Skip to content

Commit cfc7c65

Browse files
erickzanardoerickzanardo
authored
feat: detect conflict between dynamic routes and non dynamic ones (#687)
* feat: adding folder conflict validation * improving message reporting * improving message * fix analyze * PR suggestion * fix tests
1 parent ac8b0b9 commit cfc7c65

File tree

3 files changed

+219
-5
lines changed

3 files changed

+219
-5
lines changed

‎packages/dart_frog_gen/lib/src/validate_route_configuration/route_conflicts.dart

Lines changed: 76 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,26 @@
11
import 'package:dart_frog_gen/dart_frog_gen.dart';
2+
import 'package:equatable/equatable.dart';
23
import 'package:path/path.dart' as path;
34

5+
class _RouteConflict extends Equatable {
6+
const _RouteConflict(
7+
this.originalFilePath,
8+
this.conflictingFilePath,
9+
this.conflictingEndpoint,
10+
);
11+
12+
final String originalFilePath;
13+
final String conflictingFilePath;
14+
final String conflictingEndpoint;
15+
16+
@override
17+
List<Object> get props => [
18+
originalFilePath,
19+
conflictingFilePath,
20+
conflictingEndpoint,
21+
];
22+
}
23+
424
/// Type definition for callbacks that report route conflicts.
525
typedef OnRouteConflict = void Function(
626
String originalFilePath,
@@ -20,21 +40,72 @@ void reportRouteConflicts(
2040
/// Callback called when any route conflict is found.
2141
void Function()? onViolationEnd,
2242
}) {
23-
final conflictingEndpoints =
24-
configuration.endpoints.entries.where((entry) => entry.value.length > 1);
43+
final directConflicts = configuration.endpoints.entries
44+
.where((entry) => entry.value.length > 1)
45+
.map((e) => _RouteConflict(e.value.first.path, e.value.last.path, e.key));
46+
47+
final indirectConflicts = configuration.endpoints.entries
48+
.map((entry) {
49+
final matches = configuration.endpoints.keys.where((other) {
50+
final keyParts = entry.key.split('/');
51+
if (other == entry.key) {
52+
return false;
53+
}
54+
55+
final otherParts = other.split('/');
56+
57+
var match = false;
58+
59+
if (keyParts.length == otherParts.length) {
60+
for (var i = 0; i < keyParts.length; i++) {
61+
if ((keyParts[i] == otherParts[i]) ||
62+
(keyParts[i].startsWith('<') ||
63+
otherParts[i].startsWith('<'))) {
64+
match = true;
65+
} else {
66+
match = false;
67+
break;
68+
}
69+
}
70+
}
71+
72+
return match;
73+
});
74+
75+
if (matches.isNotEmpty) {
76+
final originalFilePath =
77+
matches.first.endsWith('>') ? matches.first : entry.key;
78+
79+
final conflictingFilePath =
80+
entry.key == originalFilePath ? matches.first : entry.key;
81+
82+
return _RouteConflict(
83+
originalFilePath,
84+
conflictingFilePath,
85+
originalFilePath,
86+
);
87+
}
88+
89+
return null;
90+
})
91+
.whereType<_RouteConflict>()
92+
.toSet();
93+
94+
final conflictingEndpoints = [...directConflicts, ...indirectConflicts];
95+
2596
if (conflictingEndpoints.isNotEmpty) {
2697
onViolationStart?.call();
2798
for (final conflict in conflictingEndpoints) {
2899
final originalFilePath = path.normalize(
29-
path.join('routes', conflict.value.first.path),
100+
path.join('routes', conflict.originalFilePath),
30101
);
31102
final conflictingFilePath = path.normalize(
32-
path.join('routes', conflict.value.last.path),
103+
path.join('routes', conflict.conflictingFilePath),
33104
);
34105
onRouteConflict?.call(
35106
originalFilePath,
36107
conflictingFilePath,
37-
conflict.key,
108+
conflict.conflictingEndpoint,
38109
);
39110
}
40111
onViolationEnd?.call();

‎packages/dart_frog_gen/pubspec.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ environment:
1010
sdk: ">=3.0.0 <4.0.0"
1111

1212
dependencies:
13+
equatable: ^2.0.5
1314
path: ^1.8.1
1415
pubspec_parse: ^1.2.2
1516

‎packages/dart_frog_gen/test/src/validate_route_configuration/route_conflicts_test.dart

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import 'package:dart_frog_gen/dart_frog_gen.dart';
22

33
import 'package:mocktail/mocktail.dart';
4+
import 'package:path/path.dart' as path;
45
import 'package:test/test.dart';
56

67
class _MockRouteConfiguration extends Mock implements RouteConfiguration {}
@@ -194,5 +195,146 @@ void main() {
194195
expect(violationEndCalled, isTrue);
195196
expect(conflicts, ['/hello', '/echo']);
196197
});
198+
199+
test(
200+
'reports error when dynamic directories conflict with non dynamic files',
201+
() {
202+
when(() => configuration.endpoints).thenReturn({
203+
'/cars/<id>': const [
204+
RouteFile(
205+
name: r'cars_$id_index',
206+
path: '../routes/cars/[id]/index.dart',
207+
route: '/',
208+
params: [],
209+
wildcard: false,
210+
),
211+
],
212+
'/cars/mine': const [
213+
RouteFile(
214+
name: 'cars_mine',
215+
path: '../routes/cars/mine.dart',
216+
route: '/mine',
217+
params: [],
218+
wildcard: false,
219+
),
220+
],
221+
});
222+
223+
reportRouteConflicts(
224+
configuration,
225+
onViolationStart: () {
226+
violationStartCalled = true;
227+
},
228+
onRouteConflict: (
229+
originalFilePath,
230+
conflictingFilePath,
231+
conflictingEndpoint,
232+
) {
233+
conflicts.add('$originalFilePath and '
234+
'$conflictingFilePath -> '
235+
'$conflictingEndpoint');
236+
},
237+
onViolationEnd: () {
238+
violationEndCalled = true;
239+
},
240+
);
241+
242+
expect(violationStartCalled, isTrue);
243+
expect(violationEndCalled, isTrue);
244+
expect(
245+
conflicts,
246+
equals(
247+
[
248+
'${path.normalize('/cars/<id>')} and ${path.normalize('//cars/mine')} -> /cars/<id>',
249+
],
250+
),
251+
);
252+
},
253+
);
254+
255+
test(
256+
'reports error when dynamic directories conflict with non dynamic files, '
257+
'with multiple folders',
258+
() {
259+
when(() => configuration.endpoints).thenReturn({
260+
'/turtles/random': const [
261+
RouteFile(
262+
name: 'turtles_random',
263+
path: '../routes/turtles/random.dart',
264+
route: '/',
265+
params: [],
266+
wildcard: false,
267+
),
268+
],
269+
'/turtles/<id>': const [
270+
RouteFile(
271+
name: r'turtles_$id_index',
272+
path: '../routes/turtles/[id]/index.dart',
273+
route: '/turtles/<id>',
274+
params: [],
275+
wildcard: false,
276+
),
277+
],
278+
'/turtles/<id>/bla': const [
279+
RouteFile(
280+
name: r'turtles_$id_bla',
281+
path: '../routes/turtles/[id]/bla.dart',
282+
route: '/turtles/<id>/bla.dart',
283+
params: [],
284+
wildcard: false,
285+
),
286+
],
287+
'/turtles/<id>/<name>': const [
288+
RouteFile(
289+
name: r'turtles_$id_$name_index',
290+
path: '../routes/turtles/[id]/[name]/index.dart',
291+
route: '/turtles/<id>/<name>/index.dart',
292+
params: [],
293+
wildcard: false,
294+
),
295+
],
296+
'/turtles/<id>/<name>/ble.dart': const [
297+
RouteFile(
298+
name: r'turtles_$id_$name_ble.dart',
299+
path: '../routes/turtles/[id]/[name]/ble.dart',
300+
route: '/turtles/<id>/<name>/ble.dart',
301+
params: [],
302+
wildcard: false,
303+
),
304+
],
305+
});
306+
307+
reportRouteConflicts(
308+
configuration,
309+
onViolationStart: () {
310+
violationStartCalled = true;
311+
},
312+
onRouteConflict: (
313+
originalFilePath,
314+
conflictingFilePath,
315+
conflictingEndpoint,
316+
) {
317+
conflicts.add(
318+
'$originalFilePath and '
319+
'$conflictingFilePath -> '
320+
'$conflictingEndpoint',
321+
);
322+
},
323+
onViolationEnd: () {
324+
violationEndCalled = true;
325+
},
326+
);
327+
328+
expect(violationStartCalled, isTrue);
329+
expect(violationEndCalled, isTrue);
330+
expect(
331+
conflicts,
332+
[
333+
'${path.normalize('/turtles/<id>')} and ${path.normalize('/turtles/random')} -> /turtles/<id>',
334+
'${path.normalize('/turtles/<id>/<name>')} and ${path.normalize('/turtles/<id>/bla')} -> /turtles/<id>/<name>'
335+
],
336+
);
337+
},
338+
);
197339
});
198340
}

0 commit comments

Comments
 (0)