Add OSM service policy architecture and remaining compliance fixes by dougborg · Pull Request #2 · dougborg/deflock-app

dougborg · 2026-02-25T16:19:21Z

Introduces a unified service policy system (ServiceType, ServicePolicy,
ServicePolicyResolver, ServiceRateLimiter) that resolves compliance rules
per-URL, covering OSMF official services and third-party tile providers.
Custom/self-hosted endpoints get permissive defaults.

Key changes:

ServicePolicyResolver maps URLs to policies (OSM tile server, Nominatim,
editing API, Overpass, Bing, Mapbox, custom)
Nominatim: adds 1-req/sec rate limiting and client-side result caching
with 5-minute TTL as required by Nominatim usage policy
OSM tile server: blocks offline tile downloads (explicitly prohibited by
tile usage policy) with user-facing dialog
Attribution dialog: adds tappable license link to openstreetmap.org/copyright
when using OSM-based tile providers (ODbL requirement)
OSM editing API: enforces max 2 concurrent download threads via semaphore
23 unit tests covering policy resolution, URL template parsing, custom
overrides, rate limiting, and all policy values

Builds on top of PR FoggedLens#123 (UserAgentClient) and PR FoggedLens#127 (NetworkTileProvider).
Compatible with PR FoggedLens#114 (Overpass parallelization) — Overpass policy defers
to NodeDataManager's _AsyncSemaphore.

https://claude.ai/code/session_01XyRTrax1tmtjcuT7CMoJhD

Introduces a unified service policy system (ServiceType, ServicePolicy, ServicePolicyResolver, ServiceRateLimiter) that resolves compliance rules per-URL, covering OSMF official services and third-party tile providers. Custom/self-hosted endpoints get permissive defaults. Key changes: - ServicePolicyResolver maps URLs to policies (OSM tile server, Nominatim, editing API, Overpass, Bing, Mapbox, custom) - Nominatim: adds 1-req/sec rate limiting and client-side result caching with 5-minute TTL as required by Nominatim usage policy - OSM tile server: blocks offline tile downloads (explicitly prohibited by tile usage policy) with user-facing dialog - Attribution dialog: adds tappable license link to openstreetmap.org/copyright when using OSM-based tile providers (ODbL requirement) - OSM editing API: enforces max 2 concurrent download threads via semaphore - 23 unit tests covering policy resolution, URL template parsing, custom overrides, rate limiting, and all policy values Builds on top of PR FoggedLens#123 (UserAgentClient) and PR FoggedLens#127 (NetworkTileProvider). Compatible with PR FoggedLens#114 (Overpass parallelization) — Overpass policy defers to NodeDataManager's _AsyncSemaphore. https://claude.ai/code/session_01XyRTrax1tmtjcuT7CMoJhD

Copilot

Pull request overview

This PR introduces a unified “service policy” layer for external OSM-related services and tile providers, and applies those policies to enforce usage/compliance rules (rate limits, concurrency limits, offline-download restrictions, attribution links).

Changes:

Add ServicePolicy / ServicePolicyResolver / ServiceRateLimiter to centralize compliance rules per service/URL.
Enforce Nominatim client caching + rate limiting, and OSM editing API concurrency limiting.
Update UI to block offline downloads for disallowed tile sources and add a tappable attribution/license link; add unit tests for policy resolution and limiter behavior.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
`lib/services/service_policy.dart`	Defines service types, policies, URL-to-policy resolution, and a shared rate limiter/semaphore mechanism.
`lib/services/search_service.dart`	Adds Nominatim caching and routes requests through `ServiceRateLimiter`.
`lib/services/map_data_submodules/nodes_from_osm_api.dart`	Limits OSM API concurrency via `ServiceRateLimiter`.
`lib/models/tile_provider.dart`	Exposes `allowsOfflineDownload` and `servicePolicy` derived from the tile URL template.
`lib/widgets/download_area_dialog.dart`	Blocks offline downloads for providers whose policy disallows it, showing a user-facing dialog.
`lib/widgets/map/map_overlays.dart`	Adds attribution dialog license link derived from the active tile provider policy.
`test/services/service_policy_test.dart`	Adds unit tests for policy resolution, URL template handling, overrides, and rate limiting.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-25T16:29:56Z

lib/widgets/map/map_overlays.dart

+              GestureDetector(
+                onTap: () async {


This clickable URL uses GestureDetector, which isn’t keyboard-focusable and has weaker semantics for screen readers. Prefer an accessible control (e.g., a TextButton/InkWell with semantics, or a Link widget) so the license link is discoverable and operable via accessibility tools.

Suggested change

GestureDetector(

onTap: () async {

TextButton(

style: TextButton.styleFrom(

padding: EdgeInsets.zero,

tapTargetSize: MaterialTapTargetSize.shrinkWrap,

),

onPressed: () async {

Copilot · 2026-02-25T16:29:56Z

test/services/service_policy_test.dart

+      final elapsed = DateTime.now().difference(start);
+      // Should have taken at least 900ms (allowing some tolerance)
+      expect(elapsed.inMilliseconds, greaterThanOrEqualTo(900));
+    });


This assertion relies on wall-clock timing and a tight threshold (>=900ms). On slower or loaded CI machines this can be flaky. Consider using a deterministic time source (e.g., package:fake_async) or relaxing/removing real-time assertions.

Copilot · 2026-02-25T16:29:57Z

lib/services/search_service.dart

    } catch (e) {
+      // Release the semaphore on error too
+      ServiceRateLimiter.release(ServiceType.nominatim);
      debugPrint('[SearchService] Search failed: $e');
      throw Exception('Search failed: $e');


The catch block always calls ServiceRateLimiter.release(ServiceType.nominatim), but the try block also releases on the success path. If an exception occurs after the first release, this becomes a double-release and can break semaphore accounting when there are waiters. Use a single try/finally to ensure release is invoked exactly once after a successful acquire.

Copilot · 2026-02-25T16:29:57Z

lib/services/service_policy.dart

+    final host = _extractHost(url);
+    if (host == null) return const ServicePolicy();
+
+    // Check custom overrides first
+    for (final entry in _customOverrides.entries) {
+      if (host.contains(entry.key)) {


Custom policy overrides are matched with host.contains(entry.key), which can match unintended hosts (substring matches) and is case-sensitive against the lowercased host. Normalize hostPattern to lowercase at registration and match using exact/endsWith semantics (like the built-in host patterns) to avoid false positives/negatives.

Suggested change

final host = _extractHost(url);

if (host == null) return const ServicePolicy();

// Check custom overrides first

for (final entry in _customOverrides.entries) {

if (host.contains(entry.key)) {

final host = _extractHost(url)?.toLowerCase();

if (host == null) return const ServicePolicy();

// Check custom overrides first (case-insensitive, supports subdomain matching)

for (final entry in _customOverrides.entries) {

final pattern = entry.key.toLowerCase();

if (host == pattern || host.endsWith('.$pattern')) {

Copilot · 2026-02-25T16:29:57Z

lib/widgets/download_area_dialog.dart

+                          'The ${selectedProvider?.name ?? 'current tile'} server does not permit offline downloads. '
+                          'Switch to a tile provider that allows offline use '
+                          '(e.g., Bing Maps, Mapbox, or a self-hosted tile server).',


The new "offline downloads not permitted" dialog text is hard-coded English. This file otherwise uses LocalizationService keys for user-facing strings, so this should be moved into localization (including the provider name as a parameter) to keep translations consistent.

Suggested change

'The ${selectedProvider?.name ?? 'current tile'} server does not permit offline downloads. '

'Switch to a tile provider that allows offline use '

'(e.g., Bing Maps, Mapbox, or a self-hosted tile server).',

locService.t(

'download.offlineNotPermitted',

params: [

selectedProvider?.name ?? locService.t('download.currentTile'),

],

),

Copilot · 2026-02-25T16:29:58Z

lib/services/service_policy.dart

+        final elapsed = DateTime.now().difference(lastTime);
+        final remaining = policy.minRequestInterval! - elapsed;
+        if (remaining > Duration.zero) {
+          debugPrint('[ServiceRateLimiter] Throttling $service for ${remaining.inMilliseconds}ms');
+          await Future.delayed(remaining);


Rate limiting is checked before the concurrency gate. With concurrent callers, multiple acquire() calls can all skip throttling (because _lastRequestTime hasn’t been updated yet) and then queue on the semaphore; once released, queued calls proceed without re-checking the min-interval, allowing back-to-back requests that violate minRequestInterval. The rate-limit check/update needs to be serialized with the semaphore/queue for the service.

Copilot · 2026-02-25T16:29:58Z

test/services/service_policy_test.dart

+      await ServiceRateLimiter.acquire(ServiceType.overpass);
+      ServiceRateLimiter.release(ServiceType.overpass);
+      final elapsed = DateTime.now().difference(start);
+      expect(elapsed.inMilliseconds, lessThan(100));


This test asserts the whole acquire/release sequence completes in <100ms using DateTime.now(), which is prone to CI flakiness. Consider using fake time or a much looser threshold so the test is deterministic.

Suggested change

expect(elapsed.inMilliseconds, lessThan(100));

// Use a generous upper bound to avoid CI flakiness while still catching major regressions.

expect(elapsed.inMilliseconds, lessThan(5000));

Copilot · 2026-02-25T16:29:58Z

lib/services/search_service.dart

+      ServiceRateLimiter.release(ServiceType.nominatim);
+
      if (response.statusCode != 200) {
        throw Exception('HTTP ${response.statusCode}: ${response.reasonPhrase}');
      }


ServiceRateLimiter.release(ServiceType.nominatim) is called immediately after the HTTP request completes, before status-code handling and JSON parsing. If parsing/processing throws, the limiter slot has already been released and another request may start concurrently despite maxConcurrentRequests: 1. Consider holding the slot until all response processing is complete (e.g., move the release into a finally that wraps the whole request+parse section after a successful acquire).

Copilot · 2026-02-25T16:29:59Z

lib/services/service_policy.dart

+    if (policy.maxConcurrentRequests > 0) {
+      final semaphore = _semaphores.putIfAbsent(
+        service,
+        () => _Semaphore(policy.maxConcurrentRequests),
+      );


Because the semaphore is acquired after the rate-limit logic, callers that were waiting for a concurrency slot won’t perform any throttling before proceeding. Consider acquiring the per-service semaphore/lock first, then enforcing the min-interval based on the latest _lastRequestTime, and only then allowing the request to proceed.

Copilot AI review requested due to automatic review settings February 25, 2026 16:19

Copilot started reviewing on behalf of dougborg February 25, 2026 16:19 View session

Copilot AI reviewed Feb 25, 2026

View reviewed changes

dougborg closed this Feb 26, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add OSM service policy architecture and remaining compliance fixes#2

Add OSM service policy architecture and remaining compliance fixes#2
dougborg wants to merge 1 commit intomainfrom
claude/plan-osm-compliance-up8Oj

dougborg commented Feb 25, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 25, 2026

Uh oh!

Copilot AI Feb 25, 2026

Uh oh!

Copilot AI Feb 25, 2026

Uh oh!

Copilot AI Feb 25, 2026

Uh oh!

Copilot AI Feb 25, 2026

Uh oh!

Copilot AI Feb 25, 2026

Uh oh!

Copilot AI Feb 25, 2026

Uh oh!

Copilot AI Feb 25, 2026

Uh oh!

Copilot AI Feb 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

-              GestureDetector(
-                onTap: () async {
+              TextButton(
+                style: TextButton.styleFrom(
+                  padding: EdgeInsets.zero,
+                  tapTargetSize: MaterialTapTargetSize.shrinkWrap,
+                ),
+                onPressed: () async {

-                          'The ${selectedProvider?.name ?? 'current tile'} server does not permit offline downloads. '
-                          'Switch to a tile provider that allows offline use '
-                          '(e.g., Bing Maps, Mapbox, or a self-hosted tile server).',
+                          locService.t(
+                            'download.offlineNotPermitted',
+                            params: [
+                              selectedProvider?.name ?? locService.t('download.currentTile'),
+                            ],
+                          ),

	expect(elapsed.inMilliseconds, lessThan(100));
	// Use a generous upper bound to avoid CI flakiness while still catching major regressions.
	expect(elapsed.inMilliseconds, lessThan(5000));

Conversation

dougborg commented Feb 25, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants