diff --git a/lib/services/overpass_service.dart b/lib/services/overpass_service.dart index b354fd1..90299d7 100644 --- a/lib/services/overpass_service.dart +++ b/lib/services/overpass_service.dart @@ -143,27 +143,55 @@ class OverpassService { '[OverpassService] Deduplicated ${profiles.length} profiles to ${profilesToQuery.length} for query efficiency'); } - final nodeClauses = profilesToQuery.map((profile) { - // Convert profile tags to Overpass filter format, excluding empty values - final tagFilters = profile.tags.entries + // Per-profile tag filter strings (e.g. ["man_made"="surveillance"]["surveillance:type"="ALPR"]), + // excluding empty tag values. Reused twice below: once to select our nodes + // in the bbox, and again to re-select just our nodes out of the pool of + // way/relation-connected nodes (which may include unrelated node types). + final tagFilterStrings = profilesToQuery.map((profile) { + return profile.tags.entries .where((entry) => entry.value.trim().isNotEmpty) .map((entry) => '["${entry.key}"="${entry.value}"]') .join(); + }).toList(); - return 'node$tagFilters(${bounds.southWest.latitude},${bounds.southWest.longitude},${bounds.northEast.latitude},${bounds.northEast.longitude});'; - }).join('\n '); + final bboxStr = '${bounds.southWest.latitude},${bounds.southWest.longitude},' + '${bounds.northEast.latitude},${bounds.northEast.longitude}'; + final allNodesClauses = + tagFilterStrings.map((filters) => 'node$filters($bboxStr);').join('\n '); + final resNodesClauses = + tagFilterStrings.map((filters) => 'node.resNodes$filters;').join('\n '); + + // Query strategy: + // 1. Find all matching nodes in bbox -> .allNodes, output them with tags ("out body"). + // 2. Find any way/relation that references one of .allNodes ("bn." = "by node"), + // then pull the node IDs those ways/relations reference -> .wayNodes/.relNodes. + // (`node(w)`/`node(r)` on a way/relation set returns only node IDs, not + // full tags/coords, keeping the query/response cheap.) + // 3. Re-filter that combined node pool down to just our own matching + // surveillance nodes (.resNodes may contain unrelated node types too). + // 4. `convert restriction ::id = id();` emits each of those node IDs as a + // lightweight synthetic `{"type":"restriction","id":}` element — + // a flat, unambiguous list of which node IDs are "constrained" (part of + // some way/relation), without needing the full way/relation node lists. return ''' [out:json][timeout:${kOverpassQueryTimeout.inSeconds}]; ( - $nodeClauses + $allNodesClauses +)->.allNodes; +.allNodes out body; +way(bn.allNodes); node(w)->.wayNodes; +relation(bn.allNodes); node(r)->.relNodes; +(.wayNodes;.relNodes;)->.resNodes; +( + $resNodesClauses ); -out body; -<; -out ids; +convert restriction ::id = id(); +out; '''; } + /// Deduplicate profiles for Overpass queries by removing profiles that are /// subsumed by others. A profile A subsumes profile B if all of A's /// non-empty tags exist in B with identical values — meaning every node @@ -232,7 +260,16 @@ out ids; } - /// Parse Overpass JSON response into OsmNode objects + /// Parse Overpass JSON response into OsmNode objects. + /// + /// The query (see [_buildQuery]) produces two kinds of elements: + /// - `type: "node"` — our matched surveillance nodes, with full tags/coords + /// (from `.allNodes out body;`). + /// - `type: "restriction"` — synthetic elements (from + /// `convert restriction ::id = id();`) whose `id` is the OSM node ID of + /// one of our nodes that Overpass determined is referenced by some way + /// or relation. These carry no lat/lon/tags — just the id — and exist + /// purely to tell us which node IDs are "constrained". List _parseResponse(String responseBody) { final data = jsonDecode(responseBody) as Map; final elements = data['elements'] as List; @@ -240,21 +277,16 @@ out ids; final nodeElements = >[]; final constrainedNodeIds = {}; - // First pass: collect surveillance nodes and identify constrained nodes + // First pass: collect surveillance nodes and constrained-node markers. for (final element in elements.whereType>()) { final type = element['type'] as String?; if (type == 'node') { nodeElements.add(element); - } else if (type == 'way' || type == 'relation') { - // Mark referenced nodes as constrained - final refs = element['nodes'] as List? ?? - element['members']?.where((m) => m['type'] == 'node').map((m) => m['ref']) ?? []; - - for (final ref in refs) { - final nodeId = ref is int ? ref : int.tryParse(ref.toString()); - if (nodeId != null) constrainedNodeIds.add(nodeId); - } + } else if (type == 'restriction') { + final rawId = element['id']; + final nodeId = rawId is int ? rawId : int.tryParse(rawId.toString()); + if (nodeId != null) constrainedNodeIds.add(nodeId); } } @@ -274,6 +306,7 @@ out ids; } } + /// Error thrown when a query would exceed Overpass's 50k node limit. /// The caller should split the query area into smaller regions to resolve this /// — it's a deterministic function of how much data lives within the bounds. diff --git a/test/services/overpass_service_test.dart b/test/services/overpass_service_test.dart index 6c52566..bfcb2cf 100644 --- a/test/services/overpass_service_test.dart +++ b/test/services/overpass_service_test.dart @@ -57,7 +57,7 @@ void main() { } group('query building', () { - test('uses out ids for way/relation pass, out body for node pass', + test('uses the way/relation membership pass with restriction conversion', () async { stubOverpassResponse([]); @@ -68,11 +68,16 @@ void main() { ).captured; final query = (captured.last as Map)['data']!; - expect(query, contains('out body;')); - expect(query, contains('out ids;')); + expect(query, contains('.allNodes out body;')); + expect(query, contains('way(bn.allNodes);')); + expect(query, contains('relation(bn.allNodes);')); + expect(query, contains('.resNodes')); + expect(query, contains('convert restriction ::id = id();')); expect(query, isNot(contains('out meta;'))); + expect(query, isNot(contains('out ids;'))); }); + test('empty tag values are excluded from filters', () async { final profileWithEmpty = [ NodeProfile( @@ -137,12 +142,14 @@ void main() { expect(query, isNot(contains('manufacturer'))); expect(query, isNot(contains('Flock Safety'))); - // Only one node clause total. - final nodeClauseCount = RegExp(r'node\[').allMatches(query).length; - expect(nodeClauseCount, equals(1)); + // Only one distinct tag-filter clause, appearing twice (once for + // .allNodes, once for the .resNodes re-filter). + final nodeClauseCount = RegExp(r'node[.\[]').allMatches(query).length; + expect(nodeClauseCount, equals(2)); }); + test('unrelated profiles are all kept', () async { final alprGeneric = NodeProfile( id: 'generic-alpr', @@ -186,11 +193,14 @@ void main() { ).captured; final query = (captured.last as Map)['data']!; - final nodeClauseCount = RegExp(r'node\[').allMatches(query).length; - expect(nodeClauseCount, equals(2)); + // Each of the 2 generic clauses appears twice (once in .allNodes, + // once in the .resNodes re-filter) = 4 total tag-filter clauses. + final nodeClauseCount = RegExp(r'node[.\[]').allMatches(query).length; + expect(nodeClauseCount, equals(4)); expect(query, contains('["man_made"="surveillance"]["surveillance:type"="ALPR"]')); expect(query, contains('["man_made"="surveillance"]["surveillance:type"="gunshot_detector"]')); + // Brand-specific tags should not appear anywhere in the reduced query. expect(query, isNot(contains('manufacturer'))); expect(query, isNot(contains('ShotSpotter'))); @@ -223,7 +233,13 @@ void main() { group('response parsing — constraint detection', () { - test('nodes referenced by a way are constrained', () async { + test('a node referenced as a "restriction" element is constrained', + () async { + // The Overpass query converts each way/relation-referenced node ID + // into a synthetic {"type":"restriction","id":} element + // (via `convert restriction ::id = id();`) — this is how the server + // tells us a node is part of some way/relation, regardless of whether + // that way/relation came from a "way" or a "relation" in Overpass. stubOverpassResponse([ { 'type': 'node', @@ -240,9 +256,8 @@ void main() { 'tags': {'man_made': 'surveillance'}, }, { - 'type': 'way', - 'id': 100, - 'nodes': [1], + 'type': 'restriction', + 'id': 1, }, ]); @@ -256,32 +271,8 @@ void main() { expect(node2.isConstrained, isFalse); }); - test('nodes referenced by a relation member are constrained', () async { - stubOverpassResponse([ - { - 'type': 'node', - 'id': 3, - 'lat': 38.9, - 'lon': -77.0, - 'tags': {'man_made': 'surveillance'}, - }, - { - 'type': 'relation', - 'id': 200, - 'members': [ - {'type': 'node', 'ref': 3, 'role': ''}, - ], - }, - ]); - - final nodes = - await service.fetchNodes(bounds: bounds, profiles: profiles); - - expect(nodes, hasLength(1)); - expect(nodes.first.isConstrained, isTrue); - }); - - test('nodes not in any way or relation are unconstrained', () async { + test('nodes not marked with a "restriction" element are unconstrained', + () async { stubOverpassResponse([ { 'type': 'node', @@ -299,7 +290,8 @@ void main() { expect(nodes.first.isConstrained, isFalse); }); - test('mixed response with nodes, ways, and relations', () async { + test('mixed response with multiple constrained and unconstrained nodes', + () async { stubOverpassResponse([ { 'type': 'node', @@ -323,16 +315,12 @@ void main() { 'tags': {'man_made': 'surveillance'}, }, { - 'type': 'way', - 'id': 300, - 'nodes': [10], + 'type': 'restriction', + 'id': 10, }, { - 'type': 'relation', - 'id': 400, - 'members': [ - {'type': 'node', 'ref': 11, 'role': ''}, - ], + 'type': 'restriction', + 'id': 11, }, ]); @@ -346,6 +334,7 @@ void main() { }); }); + group('error handling', () { test('HTTP 200 returns parsed nodes', () async { stubOverpassResponse([