Merge pull request #42 from dougborg/fix/routing-empty-tags

Fix route calculation HTTP 400 caused by empty profile tag values

lib/services/routing_service.dart

@@ -27,7 +27,12 @@ class RouteResult {
 class RoutingService {
   static const String _baseUrl = 'https://alprwatch.org/api/v1/deflock/directions';
   static const String _userAgent = 'DeFlock/1.0 (OSM surveillance mapping app)';
-  
+  final http.Client _client;
+
+  RoutingService({http.Client? client}) : _client = client ?? http.Client();
+
+  void close() => _client.close();
+
   // Calculate route between two points using alprwatch
   Future<RouteResult> calculateRoute({
     required LatLng start,
@@ -40,10 +45,12 @@ class RoutingService {
 
     final enabledProfiles = AppState.instance.enabledProfiles.map((p) {
       final full = p.toJson();
+      final tags = Map<String, String>.from(full['tags'] as Map);
+      tags.removeWhere((key, value) => value.isEmpty);
       return {
         'id': full['id'],
         'name': full['name'],
-        'tags': full['tags'],
+        'tags': tags,
       };
     }).toList();
     
@@ -65,7 +72,7 @@ class RoutingService {
     debugPrint('[RoutingService] alprwatch request: $uri $params');
     
     try {
-      final response = await http.post(
+      final response = await _client.post(
         uri,
         headers: {
           'User-Agent': _userAgent,
@@ -75,6 +82,16 @@ class RoutingService {
       ).timeout(kNavigationRoutingTimeout);
 
       if (response.statusCode != 200) {
+        if (kDebugMode) {
+          debugPrint('[RoutingService] Error response body: ${response.body}');
+        } else {
+          const maxLen = 500;
+          final body = response.body;
+          final truncated = body.length > maxLen
+              ? '${body.substring(0, maxLen)}… [truncated]'
+              : body;
+          debugPrint('[RoutingService] Error response body ($maxLen char max): $truncated');
+        }
         throw RoutingException('HTTP ${response.statusCode}: ${response.reasonPhrase}');
       }
       

lib/state/navigation_state.dart

@@ -376,4 +376,10 @@ class NavigationState extends ChangeNotifier {
       notifyListeners();
     }
   }
+
+  @override
+  void dispose() {
+    _routingService.close();
+    super.dispose();
+  }
 }

test/models/node_profile_test.dart

@@ -0,0 +1,76 @@
+import 'package:flutter_test/flutter_test.dart';
+import 'package:deflockapp/models/node_profile.dart';
+
+void main() {
+  group('NodeProfile', () {
+    test('toJson/fromJson round-trip preserves all fields', () {
+      final profile = NodeProfile(
+        id: 'test-id',
+        name: 'Test Profile',
+        tags: const {'man_made': 'surveillance', 'camera:type': 'fixed'},
+        builtin: true,
+        requiresDirection: false,
+        submittable: true,
+        editable: false,
+        fov: 90.0,
+      );
+
+      final json = profile.toJson();
+      final restored = NodeProfile.fromJson(json);
+
+      expect(restored.id, equals(profile.id));
+      expect(restored.name, equals(profile.name));
+      expect(restored.tags, equals(profile.tags));
+      expect(restored.builtin, equals(profile.builtin));
+      expect(restored.requiresDirection, equals(profile.requiresDirection));
+      expect(restored.submittable, equals(profile.submittable));
+      expect(restored.editable, equals(profile.editable));
+      expect(restored.fov, equals(profile.fov));
+    });
+
+    test('getDefaults returns expected profiles', () {
+      final defaults = NodeProfile.getDefaults();
+
+      expect(defaults.length, greaterThanOrEqualTo(10));
+
+      final ids = defaults.map((p) => p.id).toSet();
+      expect(ids, contains('builtin-flock'));
+      expect(ids, contains('builtin-generic-alpr'));
+      expect(ids, contains('builtin-motorola'));
+      expect(ids, contains('builtin-shotspotter'));
+    });
+
+    test('empty tag values exist in default profiles', () {
+      // Documents that profiles like builtin-flock ship with camera:mount: ''
+      // This is the root cause of the HTTP 400 bug — the routing service must
+      // filter these out before sending to the API.
+      final defaults = NodeProfile.getDefaults();
+      final flock = defaults.firstWhere((p) => p.id == 'builtin-flock');
+
+      expect(flock.tags.containsKey('camera:mount'), isTrue);
+      expect(flock.tags['camera:mount'], equals(''));
+    });
+
+    test('equality is based on id', () {
+      final a = NodeProfile(
+        id: 'same-id',
+        name: 'Profile A',
+        tags: const {'tag': 'a'},
+      );
+      final b = NodeProfile(
+        id: 'same-id',
+        name: 'Profile B',
+        tags: const {'tag': 'b'},
+      );
+      final c = NodeProfile(
+        id: 'different-id',
+        name: 'Profile A',
+        tags: const {'tag': 'a'},
+      );
+
+      expect(a, equals(b));
+      expect(a.hashCode, equals(b.hashCode));
+      expect(a, isNot(equals(c)));
+    });
+  });
+}

test/services/routing_service_test.dart

@@ -0,0 +1,204 @@
+import 'dart:convert';
+import 'package:flutter_test/flutter_test.dart';
+import 'package:http/http.dart' as http;
+import 'package:latlong2/latlong.dart';
+import 'package:mocktail/mocktail.dart';
+import 'package:shared_preferences/shared_preferences.dart';
+
+import 'package:deflockapp/app_state.dart';
+import 'package:deflockapp/models/node_profile.dart';
+import 'package:deflockapp/services/routing_service.dart';
+
+class MockHttpClient extends Mock implements http.Client {}
+
+class MockAppState extends Mock implements AppState {}
+
+void main() {
+  late MockHttpClient mockClient;
+  late MockAppState mockAppState;
+  late RoutingService service;
+
+  final start = const LatLng(38.9, -77.0);
+  final end = const LatLng(39.0, -77.1);
+
+  setUpAll(() {
+    registerFallbackValue(Uri.parse('https://example.com'));
+  });
+
+  setUp(() {
+    SharedPreferences.setMockInitialValues({
+      'navigation_avoidance_distance': 100,
+    });
+
+    mockClient = MockHttpClient();
+    mockAppState = MockAppState();
+    AppState.instance = mockAppState;
+
+    service = RoutingService(client: mockClient);
+  });
+
+  tearDown(() {
+    AppState.instance = MockAppState();
+  });
+
+  group('RoutingService', () {
+    test('empty tags are filtered from request body', () async {
+      // Profile with empty tag values (like builtin-flock has camera:mount: '')
+      final profiles = [
+        NodeProfile(
+          id: 'test-profile',
+          name: 'Test Profile',
+          tags: const {
+            'man_made': 'surveillance',
+            'surveillance:type': 'ALPR',
+            'camera:mount': '', // empty value - should be filtered
+          },
+        ),
+      ];
+      when(() => mockAppState.enabledProfiles).thenReturn(profiles);
+
+      // Capture the request body
+      when(() => mockClient.post(
+            any(),
+            headers: any(named: 'headers'),
+            body: any(named: 'body'),
+          )).thenAnswer((invocation) async {
+        return http.Response(
+          json.encode({
+            'ok': true,
+            'result': {
+              'route': {
+                'coordinates': [
+                  [-77.0, 38.9],
+                  [-77.1, 39.0],
+                ],
+                'distance': 1000.0,
+                'duration': 600.0,
+              },
+            },
+          }),
+          200,
+        );
+      });
+
+      await service.calculateRoute(start: start, end: end);
+
+      final captured = verify(() => mockClient.post(
+            any(),
+            headers: any(named: 'headers'),
+            body: captureAny(named: 'body'),
+          )).captured;
+
+      final body = json.decode(captured.last as String) as Map<String, dynamic>;
+      final enabledProfiles = body['enabled_profiles'] as List<dynamic>;
+      final tags = enabledProfiles[0]['tags'] as Map<String, dynamic>;
+
+      // camera:mount with empty value should be stripped
+      expect(tags.containsKey('camera:mount'), isFalse);
+      // Non-empty tags should remain
+      expect(tags['man_made'], equals('surveillance'));
+      expect(tags['surveillance:type'], equals('ALPR'));
+    });
+
+    test('successful route parsing', () async {
+      when(() => mockAppState.enabledProfiles).thenReturn([]);
+
+      when(() => mockClient.post(
+            any(),
+            headers: any(named: 'headers'),
+            body: any(named: 'body'),
+          )).thenAnswer((_) async => http.Response(
+            json.encode({
+              'ok': true,
+              'result': {
+                'route': {
+                  'coordinates': [
+                    [-77.0, 38.9],
+                    [-77.05, 38.95],
+                    [-77.1, 39.0],
+                  ],
+                  'distance': 15000.0,
+                  'duration': 1200.0,
+                },
+              },
+            }),
+            200,
+          ));
+
+      final result = await service.calculateRoute(start: start, end: end);
+
+      expect(result.waypoints, hasLength(3));
+      expect(result.waypoints.first.latitude, equals(38.9));
+      expect(result.waypoints.first.longitude, equals(-77.0));
+      expect(result.distanceMeters, equals(15000.0));
+      expect(result.durationSeconds, equals(1200.0));
+    });
+
+    test('HTTP error throws RoutingException with status code', () async {
+      when(() => mockAppState.enabledProfiles).thenReturn([]);
+
+      when(() => mockClient.post(
+            any(),
+            headers: any(named: 'headers'),
+            body: any(named: 'body'),
+          )).thenAnswer((_) async => http.Response(
+            'Bad Request',
+            400,
+            reasonPhrase: 'Bad Request',
+          ));
+
+      expect(
+        () => service.calculateRoute(start: start, end: end),
+        throwsA(isA<RoutingException>().having(
+          (e) => e.message,
+          'message',
+          contains('400'),
+        )),
+      );
+    });
+
+    test('network error is wrapped in RoutingException', () async {
+      when(() => mockAppState.enabledProfiles).thenReturn([]);
+
+      when(() => mockClient.post(
+            any(),
+            headers: any(named: 'headers'),
+            body: any(named: 'body'),
+          )).thenThrow(http.ClientException('Connection refused'));
+
+      expect(
+        () => service.calculateRoute(start: start, end: end),
+        throwsA(isA<RoutingException>().having(
+          (e) => e.message,
+          'message',
+          startsWith('Network error:'),
+        )),
+      );
+    });
+
+    test('API-level error surfaces alprwatch message', () async {
+      when(() => mockAppState.enabledProfiles).thenReturn([]);
+
+      when(() => mockClient.post(
+            any(),
+            headers: any(named: 'headers'),
+            body: any(named: 'body'),
+          )).thenAnswer((_) async => http.Response(
+            json.encode({
+              'ok': false,
+              'error': 'Invalid profile configuration',
+            }),
+            200,
+          ));
+
+      expect(
+        () => service.calculateRoute(start: start, end: end),
+        throwsA(isA<RoutingException>().having(
+          (e) => e.message,
+          'message',
+          contains('Invalid profile configuration'),
+        )),
+      );
+    });
+  });
+}