Fix shortbread boundary processing in regards to disputed boundaries

The way disputed boundaries are implemented was not correct. Generally
boundaries should be processed as if disputed boundaries do not exists.
But then boundary lines are flagged as disputed if they are created
from ways

* that have the tag disputed=yes, or
* that are in a relation tagged boundary=disputed with no admin_level
  set or an admin_level smaller or equal to the admin_level that
  the boundary line has (which is the smallest of any boundary relation
  the way is a member of)

Note that the relation that marks a way as disputed is not the same
relation that marks a way as part of some specific admin boundary. The
first is tagged boundary=disputed, the second is tagged
boundary=administrative.

This is based on the work in #10 but rewrites the code to be (hopefully)
easier to understand.

Note that this only fixes the version for shortbread_v1, the version for
shortbread_v1_gen also needs updating which will come later.

Author: joto

themes/shortbread_v1/topics/boundaries.lua

@@ -12,7 +12,7 @@ themepark:add_table{
     ids_type = 'way',
     geom = 'linestring',
     columns = themepark:columns({
-        { column = 'admin_level', type = 'int' },
+        { column = 'admin_level', type = 'int', not_null = true },
         { column = 'maritime', type = 'bool' },
         { column = 'disputed', type = 'bool' },
     }),
@@ -29,27 +29,33 @@ themepark:add_table{
     }
 }
 
-local rinfos = {}
+-- ---------------------------------------------------------------------------
+-- Storage of information from boundary relations for use by boundary ways
+-- (two-stage processing).
+
+-- Minimum admin level of all relations that reference a way id
+local min_admin_level = {}
+
+-- Minimum admin level of all relations tagged boundary=disputed that
+-- reference a way id
+local min_disputed_admin_level = {}
 
 -- ---------------------------------------------------------------------------
 
--- Check if this looks like a boundary and return admin_level as number
--- Return nil if this is not a valid boundary.
-local function get_admin_level(tags)
-    local type = tags.type
+-- Shortbread is only interested in level 2 and level 4 admin boundaries.
+local function is_admin_boundary(tags)
+    return (tags.type == 'boundary' or tags.type == 'multipolygon')
+           and tags.boundary == 'administrative'
+           and (tags.admin_level == '2' or tags.admin_level == '4')
+end
 
-    if type == 'boundary' or type == 'multipolygon' then
-        local boundary = tags.boundary
-        if boundary == 'administrative' or boundary == 'disputed' then
-            return tonumber(tags.admin_level)
-        end
+-- Get numerical admin level from string, default to 1 if invalid
+local function get_admin_level(value)
+    if not value or not string.match(value, '^[1-9][0-9]?$') then
+        return 1
     end
-end
 
--- Check the (numeric) admin level. Change this depending on which admin
--- levels you want to process. Shortbread only shows 2 and 4.
-local function valid_admin_level(level)
-    return level == 2 or level == 4
+    return tonumber(value)
 end
 
 -- ---------------------------------------------------------------------------
@@ -59,45 +65,61 @@ themepark:add_proc('way', function(object, data)
         return
     end
 
-    local info = rinfos[object.id]
-    if not info then
+    local admin_level = min_admin_level[object.id]
+    if not admin_level then
         return
     end
 
     local t = object.tags
+
+    -- Set disputed flag either from disputed tag on the way...
+    local disputed = (t.disputed == 'yes')
+
+    -- .. or from a parent relation with boundary=disputed
+    if min_disputed_admin_level[object.id] and min_disputed_admin_level[object.id] <= admin_level then
+        disputed = true
+    end
+
     local a = {
-        admin_level = info.admin_level,
+        admin_level = admin_level,
         maritime = (t.maritime and (t.maritime == 'yes' or t.natural == 'coastline')),
-        disputed = info.disputed or (t.disputed and t.disputed == 'yes'),
+        disputed = disputed,
         geom = object:as_linestring()
     }
 
     themepark:insert('boundaries', a, t)
 end)
 
 themepark:add_proc('select_relation_members', function(relation)
-    if valid_admin_level(get_admin_level(relation.tags)) then
+    if is_admin_boundary(relation.tags) then
         return { ways = osm2pgsql.way_member_ids(relation) }
     end
 end)
 
 themepark:add_proc('relation', function(object, data)
-    local t = object.tags
+    if is_admin_boundary(object.tags) then
+        local admin_level = tonumber(object.tags.admin_level)
 
-    local admin_level = get_admin_level(t)
+        for _, id in ipairs(osm2pgsql.way_member_ids(object)) do
+            if not min_admin_level[id] or min_admin_level[id] > admin_level then
+                min_admin_level[id] = admin_level
+            end
+        end
 
-    if not valid_admin_level(admin_level) then
         return
     end
 
-    for _, member in ipairs(object.members) do
-        if member.type == 'w' then
-            if not rinfos[member.ref] then
-                rinfos[member.ref] = { admin_level = admin_level }
-            elseif rinfos[member.ref].admin_level > admin_level then
-                rinfos[member.ref].admin_level = admin_level
+    if object.tags.boundary == 'disputed' then
+        -- Ways in relations tagged boundary=disputed are flagged as disputed
+        -- if either the relation doesn't have an admin_level tag or the
+        -- admin_level tag is <= the admin level the way got from the
+        -- boundary=administrative relation(s).
+        local admin_level = get_admin_level(object.tags.admin_level)
+
+        for _, id in ipairs(osm2pgsql.way_member_ids(object)) do
+            if not min_disputed_admin_level[id] or min_disputed_admin_level[id] > admin_level then
+                min_disputed_admin_level[id] = admin_level
             end
-            rinfos[member.ref].disputed = (t.boundary == 'disputed')
         end
     end
 end)