Fix node constrainment when part of a way/relation

This commit is contained in:
stopflock
2026-08-03 20:21:19 -05:00
parent b231b48ac1
commit 7c5d066362
2 changed files with 89 additions and 67 deletions
+53 -20
View File
@@ -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":<nodeId>}` 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<OsmNode> _parseResponse(String responseBody) {
final data = jsonDecode(responseBody) as Map<String, dynamic>;
final elements = data['elements'] as List<dynamic>;
@@ -240,21 +277,16 @@ out ids;
final nodeElements = <Map<String, dynamic>>[];
final constrainedNodeIds = <int>{};
// First pass: collect surveillance nodes and identify constrained nodes
// First pass: collect surveillance nodes and constrained-node markers.
for (final element in elements.whereType<Map<String, dynamic>>()) {
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<dynamic>? ??
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.
+36 -47
View File
@@ -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<String, String>)['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<String, String>)['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":<nodeId>} 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([