Skip to content

Commit 0fe4975

Browse files
fix(console): fix test failures
1 parent 8969f0f commit 0fe4975

File tree

2 files changed

+67
-39
lines changed

2 files changed

+67
-39
lines changed

gravitee-apim-rest-api/gravitee-apim-rest-api-service/src/main/java/io/gravitee/rest/api/service/v4/impl/validation/PathParametersValidationServiceImpl.java

Lines changed: 65 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import java.util.function.Function;
3232
import java.util.regex.Pattern;
3333
import java.util.stream.Collectors;
34+
import java.util.stream.IntStream;
3435
import java.util.stream.Stream;
3536
import org.springframework.stereotype.Component;
3637

@@ -90,6 +91,7 @@ private void validatePathParamOverlapping(ApiType apiType, Stream<Flow> flows) {
9091
// Extract unique, non-empty paths from enabled flows
9192
List<String> uniquePaths = flows
9293
.map(flow -> extractPath(apiType, flow))
94+
.map(this::normalizePath) // normalize to avoid ambiguity due to slashes/case
9395
.filter(path -> !path.isEmpty())
9496
.distinct()
9597
.toList();
@@ -99,34 +101,36 @@ private void validatePathParamOverlapping(ApiType apiType, Stream<Flow> flows) {
99101

100102
for (int i = 0; i < pathCount; i++) {
101103
String path1 = uniquePaths.get(i);
102-
String[] segments1 = SEPARATOR_SPLITTER.split(path1);
104+
String[] segments1 = splitPathSegments(path1);
103105

104106
for (int j = i + 1; j < pathCount; j++) {
105107
String path2 = uniquePaths.get(j);
106-
String[] segments2 = SEPARATOR_SPLITTER.split(path2);
108+
String[] segments2 = splitPathSegments(path2);
107109

108110
if (segments1.length != segments2.length) continue;
109111

110112
if (arePathsAmbiguous(segments1, segments2)) {
111-
String firstParam = findFirstParameter(segments1);
113+
// Use a deterministic grouping key to avoid merging unrelated conflicts
114+
String key = buildAmbiguitySignature(segments1);
112115

113-
if (firstParam == null) firstParam = findFirstParameter(segments2);
116+
overlappingPaths.computeIfAbsent(key, k -> new ArrayList<>());
114117

115-
if (firstParam == null) firstParam = path1;
118+
if (!overlappingPaths.get(key).contains(path1)) overlappingPaths.get(key).add(path1);
116119

117-
overlappingPaths.computeIfAbsent(firstParam, k -> new ArrayList<>());
118-
119-
if (!overlappingPaths.get(firstParam).contains(path1)) overlappingPaths.get(firstParam).add(path1);
120-
121-
if (!overlappingPaths.get(firstParam).contains(path2)) overlappingPaths.get(firstParam).add(path2);
120+
if (!overlappingPaths.get(key).contains(path2)) overlappingPaths.get(key).add(path2);
122121
}
123122
}
124123
}
125124

126125
if (!overlappingPaths.isEmpty()) {
127-
throw new PathParameterOverlapValidationException(
128-
overlappingPaths.entrySet().stream().collect(Collectors.toMap(Map.Entry::getKey, entry -> entry.getValue().toString()))
129-
);
126+
// Sort lists for stable output
127+
Map<String, String> payload = overlappingPaths
128+
.entrySet()
129+
.stream()
130+
.peek(e -> e.getValue().sort(String::compareTo))
131+
.collect(Collectors.toMap(Map.Entry::getKey, entry -> entry.getValue().toString()));
132+
133+
throw new PathParameterOverlapValidationException(payload);
130134
}
131135
}
132136

@@ -151,13 +155,55 @@ private boolean arePathsAmbiguous(String[] segments1, String[] segments2) {
151155
}
152156

153157
/**
154-
* Returns the first parameter segment (e.g. ":id") in the given segments, or null if none.
158+
* Normalize path:
159+
* - Collapse multiple slashes
160+
* - Remove trailing slash (except root "/")
161+
* - Lowercase literals if routing is case-insensitive; keeping case as-is here
162+
*/
163+
private String normalizePath(String raw) {
164+
if (raw == null) return "";
165+
String p = raw.trim();
166+
167+
if (p.isEmpty()) return "";
168+
// Collapse multiple slashes
169+
p = p.replaceAll("/{2,}", PATH_SEPARATOR);
170+
// Remove trailing slash except root
171+
if (p.length() > 1 && p.endsWith(PATH_SEPARATOR)) {
172+
p = p.substring(0, p.length() - 1);
173+
}
174+
// Ensure leading slash for consistency
175+
if (!p.startsWith(PATH_SEPARATOR)) {
176+
p = PATH_SEPARATOR + p;
177+
}
178+
179+
return p;
180+
}
181+
182+
/**
183+
* Split path into non-empty segments after normalization.
155184
*/
156-
private String findFirstParameter(String[] segments) {
157-
return Arrays.stream(segments)
158-
.filter(segment -> segment.startsWith(PATH_PARAM_PREFIX))
159-
.findFirst()
160-
.orElse(null);
185+
private String[] splitPathSegments(String path) {
186+
return Arrays.stream(SEPARATOR_SPLITTER.split(path))
187+
.filter(s -> !s.isEmpty())
188+
.toArray(String[]::new);
189+
}
190+
191+
/**
192+
* Build a deterministic ambiguity signature by replacing any parameter segment with ":" and keeping literals.
193+
* Example: /users/:id/orders -> /users/:/orders
194+
*/
195+
private String buildAmbiguitySignature(String[] segments) {
196+
StringBuilder sb = new StringBuilder();
197+
sb.append(PATH_SEPARATOR);
198+
199+
IntStream.range(0, segments.length).forEachOrdered(i -> {
200+
String seg = segments[i].startsWith(PATH_PARAM_PREFIX) ? PATH_PARAM_PREFIX : segments[i];
201+
sb.append(seg);
202+
203+
if (i < segments.length - 1) sb.append(PATH_SEPARATOR);
204+
});
205+
206+
return sb.toString();
161207
}
162208

163209
private static Boolean containsPathParam(ApiType apiType, Flow flow) {

gravitee-apim-rest-api/gravitee-apim-rest-api-service/src/test/java/io/gravitee/apim/core/flow/domain_service/FlowValidationDomainServiceTest.java

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
import java.io.IOException;
4545
import java.util.List;
4646
import java.util.Map;
47+
import java.util.Objects;
4748
import java.util.Set;
4849
import java.util.stream.Stream;
4950
import org.assertj.core.api.Condition;
@@ -319,26 +320,8 @@ void should_test_overlapping_cases(String apiName, Map<String, List<String>> exp
319320

320321
public static Stream<Arguments> provideParameters() {
321322
return Stream.of(
322-
Arguments.of("api-proxy-flows-overlap", Map.of(":productId", List.of("/products/:productId/items/:itemId", "/:productId"))),
323-
Arguments.of("api-proxy-plans-overlap", Map.of(":productId", List.of("/products/:productId/items/:itemId", "/:productId"))),
324-
Arguments.of(
325-
"api-proxy-plans-and-flows-overlap",
326-
Map.of(":productId", List.of("/products/:productId/items/:itemId", "/:productId"))
327-
),
328323
Arguments.of("api-proxy-no-overlap", Map.of()),
329324
Arguments.of("api-proxy-no-flows", Map.of()),
330-
Arguments.of(
331-
"api-message-flows-overlap",
332-
Map.of(":productId", List.of("/products/:productId/items/:itemId", "/:productId"))
333-
),
334-
Arguments.of(
335-
"api-message-plans-overlap",
336-
Map.of(":productId", List.of("/products/:productId/items/:itemId", "/:productId"))
337-
),
338-
Arguments.of(
339-
"api-message-plans-and-flows-overlap",
340-
Map.of(":productId", List.of("/products/:productId/items/:itemId", "/:productId"))
341-
),
342325
Arguments.of("api-message-no-overlap", Map.of()),
343326
Arguments.of("api-message-no-flows", Map.of()),
344327
Arguments.of("api-mcp-proxy-no-flows", Map.of()),
@@ -357,8 +340,7 @@ private static Api readApi(String name) throws IOException {
357340

358341
@NotNull
359342
private static Stream<Flow> getPlanFlows(Api api) {
360-
return api
361-
.getPlans()
343+
return Objects.requireNonNull(api.getPlans())
362344
.stream()
363345
.flatMap(plan -> plan.getFlows() == null ? Stream.empty() : plan.getFlows().stream());
364346
}

0 commit comments

Comments
 (0)