Skip to content

Commit aa3a282

Browse files
jpfinnejpfinne
committed
Use constants for filter keys.
Improve exception handling and tests.
1 parent d9f3ce1 commit aa3a282

2 files changed

Lines changed: 33 additions & 34 deletions

File tree

‎modules/openapi-generator/src/main/java/org/openapitools/codegen/OpenAPINormalizer.java‎

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -276,10 +276,11 @@ public void processRules(Map<String, String> inputRules) {
276276
try {
277277
filter = new Filter(filters);
278278
} catch (RuntimeException e) {
279-
LOGGER.error("FILTER rule must be in the form of `operationId:name1|name2|name3` or `method:get|post|put` or `tag:tag1|tag2|tag3` or `path:/v1|/v2` in {}", filters);
280-
// rethrow the exception. This is a breaking change compared to pre 7.16.0
279+
String message = String.format("FILTER rule [%s] must be in the form of `%s:name1|name2|name3` or `%s:get|post|put` or `%s:tag1|tag2|tag3` or `%s:/v1|/v2`. Error: %s",
280+
filters, Filter.OPERATION_ID, Filter.METHOD, Filter.TAG, Filter.PATH, e.getMessage());
281+
// throw an exception. This is a breaking change compared to pre 7.16.0
281282
// Workaround: fix the syntax!
282-
throw e;
283+
throw new IllegalArgumentException(message);
283284
}
284285
}
285286

@@ -1782,6 +1783,10 @@ protected Schema processNormalize31Spec(Schema schema, Set<Schema> visitedSchema
17821783
// ===================== end of rules =====================
17831784

17841785
static class Filter {
1786+
public static final String OPERATION_ID = "operationId";
1787+
public static final String METHOD = "method";
1788+
public static final String TAG = "tag";
1789+
public static final String PATH = "path";
17851790
protected Set<String> operationIdFilters = Collections.emptySet();
17861791
protected Set<String> methodFilters = Collections.emptySet();
17871792
protected Set<String> tagFilters = Collections.emptySet();
@@ -1796,24 +1801,25 @@ public Filter(String filters) {
17961801
filter = filter.trim();
17971802
String[] filterStrs = filter.split(":");
17981803
if (filterStrs.length != 2) { // only support filter with : at the moment
1799-
throw new IllegalArgumentException("filter not supported :[" + filter + "]");
1804+
throw new IllegalArgumentException("filter not supported :[" + filters + "]");
18001805
} else {
18011806
String filterKey = filterStrs[0].trim();
18021807
String filterValue = filterStrs[1];
18031808
Set<String> parsedFilters = Arrays.stream(filterValue.split("[|]"))
18041809
.filter(Objects::nonNull)
18051810
.map(String::trim)
18061811
.collect(Collectors.toCollection(HashSet::new));
1807-
if ("operationId".equals(filterKey)) {
1812+
if (OPERATION_ID.equals(filterKey)) {
18081813
operationIdFilters = parsedFilters;
1809-
} else if ("method".equals(filterKey)) {
1814+
} else if (METHOD.equals(filterKey)) {
1815+
18101816
methodFilters = parsedFilters;
1811-
} else if ("tag".equals(filterKey)) {
1817+
} else if (TAG.equals(filterKey)) {
18121818
tagFilters = parsedFilters;
1813-
} else if ("path".equals(filterKey)) {
1819+
} else if (PATH.equals(filterKey)) {
18141820
pathStartingWithFilters = parsedFilters;
18151821
} else {
1816-
throw new IllegalArgumentException("filter not supported :[" + filter + "]");
1822+
throw new IllegalArgumentException("filter not supported :[" + filters + "]");
18171823
}
18181824
}
18191825
}
@@ -1828,10 +1834,10 @@ public void apply(String path, PathItem pathItem, Map<String, Function<PathItem,
18281834
Operation operation = getter.apply(pathItem);
18291835
if (operation != null) {
18301836
boolean found = false;
1831-
found |= hasMatch("path", operation, hasPathStarting(path));
1832-
found |= hasMatch("tag", operation, hasTag(operation));
1833-
found |= hasMatch("operationId", operation, hasOperationId(operation));
1834-
found |= hasMatch("method", operation, hasMethod(method));
1837+
found |= hasMatch(PATH, operation, hasPathStarting(path));
1838+
found |= hasMatch(TAG, operation, hasTag(operation));
1839+
found |= hasMatch(OPERATION_ID, operation, hasOperationId(operation));
1840+
found |= hasMatch(METHOD, operation, hasMethod(method));
18351841
operation.addExtension(X_INTERNAL, !found);
18361842
}
18371843
});

‎modules/openapi-generator/src/test/java/org/openapitools/codegen/OpenAPINormalizerTest.java‎

Lines changed: 14 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -606,8 +606,7 @@ public void testRemoveXInternal() {
606606
assertEquals(openAPI.getPaths().get("/person/display/{personId}").getDelete().getExtensions().get(X_INTERNAL), true);
607607
assertEquals(s.getExtensions().get(X_INTERNAL), true);
608608

609-
Map<String, String> options = new HashMap<>();
610-
options.put("REMOVE_X_INTERNAL", "true");
609+
Map<String, String> options = Map.of("REMOVE_X_INTERNAL", "true");
611610
OpenAPINormalizer openAPINormalizer = new OpenAPINormalizer(openAPI, options);
612611
openAPINormalizer.normalize();
613612

@@ -625,8 +624,7 @@ public void testOperationIdFilter() {
625624
assertEquals(openAPI.getPaths().get("/person/display/{personId}").getDelete().getExtensions().get(X_INTERNAL), true);
626625
assertEquals(openAPI.getPaths().get("/person/display/{personId}").getPut().getExtensions(), null);
627626

628-
Map<String, String> options = new HashMap<>();
629-
options.put("FILTER", "operationId:delete|list");
627+
Map<String, String> options = Map.of("FILTER", "operationId:delete|list");
630628
OpenAPINormalizer openAPINormalizer = new OpenAPINormalizer(openAPI, options);
631629
openAPINormalizer.normalize();
632630

@@ -643,8 +641,7 @@ public void testFilterWithMethod() {
643641
assertEquals(openAPI.getPaths().get("/person/display/{personId}").getDelete().getExtensions().get(X_INTERNAL), true);
644642
assertEquals(openAPI.getPaths().get("/person/display/{personId}").getPut().getExtensions(), null);
645643

646-
Map<String, String> options = new HashMap<>();
647-
options.put("FILTER", "method:get");
644+
Map<String, String> options = Map.of("FILTER", "method:get");
648645
OpenAPINormalizer openAPINormalizer = new OpenAPINormalizer(openAPI, options);
649646
openAPINormalizer.normalize();
650647

@@ -712,33 +709,29 @@ public void testFilterWithTag() {
712709
assertEquals(openAPI.getPaths().get("/person/display/{personId}").getDelete().getExtensions().get(X_INTERNAL), true);
713710
assertEquals(openAPI.getPaths().get("/person/display/{personId}").getPut().getExtensions(), null);
714711

715-
Map<String, String> options = new HashMap<>();
716-
options.put("FILTER", "tag:basic");
712+
Map<String, String> options = Map.of("FILTER", "tag:basic");
717713
OpenAPINormalizer openAPINormalizer = new OpenAPINormalizer(openAPI, options);
718714
openAPINormalizer.normalize();
719715

720716
assertEquals(openAPI.getPaths().get("/person/display/{personId}").getGet().getExtensions().get(X_INTERNAL), false);
721717
assertEquals(openAPI.getPaths().get("/person/display/{personId}").getDelete().getExtensions().get(X_INTERNAL), true);
722718
assertEquals(openAPI.getPaths().get("/person/display/{personId}").getPut().getExtensions().get(X_INTERNAL), true);
723719
}
720+
724721
@Test
725-
public void testFilterWithTagWithTrim() {
722+
public void testFilterInvalidDoesThrow() {
726723
OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_0/enableKeepOnlyFirstTagInOperation_test.yaml");
727724

728-
assertEquals(openAPI.getPaths().get("/person/display/{personId}").getGet().getExtensions(), null);
729-
assertEquals(openAPI.getPaths().get("/person/display/{personId}").getDelete().getExtensions().get(X_INTERNAL), true);
730-
assertEquals(openAPI.getPaths().get("/person/display/{personId}").getPut().getExtensions(), null);
731-
732-
Map<String, String> options = new HashMap<>();
733-
options.put("FILTER", "tag:basic");
734-
OpenAPINormalizer openAPINormalizer = new OpenAPINormalizer(openAPI, options);
735-
openAPINormalizer.normalize();
736-
737-
assertEquals(openAPI.getPaths().get("/person/display/{personId}").getGet().getExtensions().get(X_INTERNAL), false);
738-
assertEquals(openAPI.getPaths().get("/person/display/{personId}").getDelete().getExtensions().get(X_INTERNAL), true);
739-
assertEquals(openAPI.getPaths().get("/person/display/{personId}").getPut().getExtensions().get(X_INTERNAL), true);
725+
Map<String, String> options = Map.of("FILTER", "tag ; invalid");
726+
try {
727+
new OpenAPINormalizer(openAPI, options);
728+
fail("Expected IllegalArgumentException");
729+
} catch (IllegalArgumentException e) {
730+
assertEquals(e.getMessage(), "FILTER rule [tag ; invalid] must be in the form of `operationId:name1|name2|name3` or `method:get|post|put` or `tag:tag1|tag2|tag3` or `path:/v1|/v2`. Error: filter not supported :[tag ; invalid]");
731+
}
740732
}
741733

734+
742735
@Test
743736
public void testComposedSchemaDoesNotThrow() {
744737
OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_1/composed-schema.yaml");

0 commit comments

Comments
 (0)