Skip to content

Commit 919ee18

Browse files
committed
feat: enhance pageable validation with sort enum support
1 parent 5981639 commit 919ee18

3 files changed

Lines changed: 53 additions & 32 deletions

File tree

modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/KotlinSpringServerCodegen.java

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1059,27 +1059,21 @@ public CodegenOperation fromOperation(String path, String httpMethod, Operation
10591059
}
10601060

10611061
// #8315 Remove matching Spring Data Web default query params if 'x-spring-paginated' with Pageable is used
1062-
// Before removal, capture sort enum values for @ValidSort if generateSortValidation is enabled
1062+
// Build pageable parameter annotations (@ValidSort, @PageableDefault, @SortDefault.SortDefaults)
1063+
List<String> pageableAnnotations = new ArrayList<>();
1064+
10631065
if (generateSortValidation && useBeanValidation && sortValidationEnums.containsKey(codegenOperation.operationId)) {
10641066
List<String> allowedSortValues = sortValidationEnums.get(codegenOperation.operationId);
10651067
String allowedValuesStr = allowedSortValues.stream()
10661068
.map(v -> "\"" + v.replace("\\", "\\\\").replace("\"", "\\\"") + "\"")
10671069
.collect(Collectors.joining(", "));
1068-
String validSortAnnotation = "@ValidSort(allowedValues = [" + allowedValuesStr + "])";
1069-
1070-
Object existingAnnotation = codegenOperation.vendorExtensions.get("x-operation-extra-annotation");
1071-
List<String> existingAnnotations = DefaultCodegen.getObjectAsStringList(existingAnnotation);
1072-
List<String> updatedAnnotations = new ArrayList<>(existingAnnotations);
1073-
updatedAnnotations.add(validSortAnnotation);
1074-
codegenOperation.vendorExtensions.put("x-operation-extra-annotation", updatedAnnotations);
1075-
1070+
pageableAnnotations.add("@ValidSort(allowedValues = [" + allowedValuesStr + "])");
10761071
codegenOperation.imports.add("ValidSort");
10771072
}
10781073

10791074
// Generate @PageableDefault / @SortDefault.SortDefaults annotations if defaults are present
10801075
if (pageableDefaultsRegistry.containsKey(codegenOperation.operationId)) {
10811076
PageableDefaultsData defaults = pageableDefaultsRegistry.get(codegenOperation.operationId);
1082-
List<String> pageableAnnotations = new ArrayList<>();
10831077

10841078
if (defaults.page != null || defaults.size != null) {
10851079
List<String> attrs = new ArrayList<>();
@@ -1097,10 +1091,10 @@ public CodegenOperation fromOperation(String path, String httpMethod, Operation
10971091
codegenOperation.imports.add("SortDefault");
10981092
codegenOperation.imports.add("Sort");
10991093
}
1094+
}
11001095

1101-
if (!pageableAnnotations.isEmpty()) {
1102-
codegenOperation.vendorExtensions.put("x-pageable-extra-annotation", pageableAnnotations);
1103-
}
1096+
if (!pageableAnnotations.isEmpty()) {
1097+
codegenOperation.vendorExtensions.put("x-pageable-extra-annotation", pageableAnnotations);
11041098
}
11051099
codegenOperation.queryParams.removeIf(param -> defaultPageableQueryParams.contains(param.baseName));
11061100
codegenOperation.allParams.removeIf(param -> param.isQueryParam && defaultPageableQueryParams.contains(param.baseName));

modules/openapi-generator/src/main/resources/kotlin-spring/validSort.mustache

Lines changed: 29 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4,18 +4,25 @@ import {{javaxPackage}}.validation.Constraint
44
import {{javaxPackage}}.validation.ConstraintValidator
55
import {{javaxPackage}}.validation.ConstraintValidatorContext
66
import {{javaxPackage}}.validation.Payload
7-
import {{javaxPackage}}.validation.constraintvalidation.SupportedValidationTarget
8-
import {{javaxPackage}}.validation.constraintvalidation.ValidationTarget
97
import org.springframework.data.domain.Pageable
108

119
/**
12-
* Validates that sort properties in a [Pageable] parameter match the allowed values.
10+
* Validates that sort properties in the annotated [Pageable] parameter match the allowed values.
1311
*
14-
* This annotation can only be applied to methods that have a [Pageable] parameter.
15-
* The validator checks that each sort property and direction combination in the [Pageable]
16-
* matches one of the strings specified in [allowedValues].
12+
* Apply directly on a `pageable: Pageable` parameter. The validator checks that each sort
13+
* property and direction combination in the [Pageable] matches one of the strings specified
14+
* in [allowedValues].
1715
*
18-
* Expected value format: `"property,direction"` (e.g. `"id,asc"`, `"name,desc"`).
16+
* Two formats are accepted in [allowedValues]:
17+
* - `"property,direction"` — permits only the specific direction (e.g. `"id,asc"`, `"name,desc"`).
18+
* Direction matching is case-insensitive: `"id,ASC"` and `"id,asc"` are treated identically.
19+
* - `"property"` — permits any direction for that property (e.g. `"id"` matches `sort=id,asc`
20+
* and `sort=id,desc`). Note: because Spring always normalises a bare `sort=id` to ascending
21+
* before the validator runs, bare property names in [allowedValues] effectively allow all
22+
* directions — the original omission of a direction cannot be detected.
23+
*
24+
* Both formats may be mixed freely. For example `["id", "name,desc"]` allows `id` in any
25+
* direction but restricts `name` to descending only.
1926
*
2027
* @property allowedValues The allowed sort strings (e.g. `["id,asc", "id,desc"]`)
2128
* @property groups Validation groups (optional)
@@ -25,34 +32,38 @@ import org.springframework.data.domain.Pageable
2532
@MustBeDocumented
2633
@Retention(AnnotationRetention.RUNTIME)
2734
@Constraint(validatedBy = [SortValidator::class])
28-
@Target(AnnotationTarget.FUNCTION)
35+
@Target(AnnotationTarget.VALUE_PARAMETER)
2936
annotation class ValidSort(
3037
val allowedValues: Array<String>,
3138
val groups: Array<kotlin.reflect.KClass<*>> = [],
3239
val payload: Array<kotlin.reflect.KClass<out Payload>> = [],
3340
val message: String = "Invalid sort column"
3441
)
3542

36-
@SupportedValidationTarget(ValidationTarget.PARAMETERS)
37-
class SortValidator : ConstraintValidator<ValidSort, Array<Any?>> {
43+
class SortValidator : ConstraintValidator<ValidSort, Pageable> {
3844
3945
private lateinit var allowedValues: Set<String>
4046
4147
override fun initialize(constraintAnnotation: ValidSort) {
42-
allowedValues = constraintAnnotation.allowedValues.toSet()
48+
allowedValues = constraintAnnotation.allowedValues.map { entry ->
49+
DIRECTION_ASC_SUFFIX.replace(entry, ",asc")
50+
.let { DIRECTION_DESC_SUFFIX.replace(it, ",desc") }
51+
}.toSet()
52+
}
53+
54+
private companion object {
55+
val DIRECTION_ASC_SUFFIX = Regex(",ASC$", RegexOption.IGNORE_CASE)
56+
val DIRECTION_DESC_SUFFIX = Regex(",DESC$", RegexOption.IGNORE_CASE)
4357
}
4458

45-
override fun isValid(parameters: Array<Any?>?, context: ConstraintValidatorContext): Boolean {
46-
val pageable = parameters?.filterIsInstance<Pageable>()?.firstOrNull()
47-
?: throw IllegalStateException(
48-
"@ValidSort can only be used on methods with a Pageable parameter. " +
49-
"Ensure the annotated method has a parameter of type org.springframework.data.domain.Pageable."
50-
)
59+
override fun isValid(pageable: Pageable?, context: ConstraintValidatorContext): Boolean {
60+
if (pageable == null || pageable.sort.isUnsorted) return true
5161
5262
val invalid = pageable.sort
5363
.foldIndexed(emptyMap<Int, String>()) { index, acc, order ->
5464
val sortValue = "${order.property},${order.direction.name.lowercase()}"
55-
if (sortValue !in allowedValues) acc + (index to order.property)
65+
// Accept "property,direction" (exact match) OR "property" alone (any direction allowed)
66+
if (sortValue !in allowedValues && order.property !in allowedValues) acc + (index to order.property)
5667
else acc
5768
}
5869
.toSortedMap()

modules/openapi-generator/src/test/java/org/openapitools/codegen/kotlin/spring/KotlinSpringServerCodegenTest.java

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4177,6 +4177,21 @@ public void generateSortValidationAddsAnnotationForExplicitPaginated() throws Ex
41774177
File petApi = files.get("PetApi.kt");
41784178
assertFileContains(petApi.toPath(), "@ValidSort(allowedValues = [\"id,asc\", \"id,desc\", \"name,asc\", \"name,desc\"])");
41794179
assertFileContains(petApi.toPath(), "import org.openapitools.configuration.ValidSort");
4180+
4181+
// @ValidSort must be a parameter annotation — appears in the 500-char window AFTER `fun findPetsWithSortEnum(`
4182+
String content = Files.readString(petApi.toPath());
4183+
int methodStart = content.indexOf("fun findPetsWithSortEnum(");
4184+
Assert.assertTrue(methodStart >= 0, "findPetsWithSortEnum method should exist");
4185+
String paramBlock = content.substring(methodStart, Math.min(content.length(), methodStart + 500));
4186+
Assert.assertTrue(paramBlock.contains("@ValidSort(allowedValues = [\"id,asc\", \"id,desc\", \"name,asc\", \"name,desc\"])"),
4187+
"@ValidSort should appear as a parameter annotation (inside the method signature, after `fun`)");
4188+
Assert.assertTrue(paramBlock.contains("pageable: Pageable"),
4189+
"findPetsWithSortEnum should have a pageable: Pageable parameter");
4190+
4191+
// @ValidSort must NOT be a method-level annotation (not in the 500-char prefix before `fun`)
4192+
String prefixBlock = content.substring(Math.max(0, methodStart - 500), methodStart);
4193+
Assert.assertFalse(prefixBlock.contains("@ValidSort"),
4194+
"@ValidSort should be a parameter annotation, not a method-level annotation");
41804195
}
41814196

41824197
@Test
@@ -4265,7 +4280,8 @@ public void generateSortValidationGeneratesValidSortFile() throws Exception {
42654280
assertFileContains(validSortFile.toPath(), "annotation class ValidSort");
42664281
assertFileContains(validSortFile.toPath(), "class SortValidator");
42674282
assertFileContains(validSortFile.toPath(), "val allowedValues: Array<String>");
4268-
assertFileContains(validSortFile.toPath(), "allowedValues.toSet()");
4283+
assertFileContains(validSortFile.toPath(), "DIRECTION_ASC_SUFFIX");
4284+
assertFileContains(validSortFile.toPath(), "DIRECTION_DESC_SUFFIX");
42694285
}
42704286

42714287
@Test

0 commit comments

Comments
 (0)