Skip to content

Commit

Permalink
swagger-api#4804: Only include validation constraints with no groups …
Browse files Browse the repository at this point in the history
…by default

The current process for adding constraints into the schema presumes that
all javax or jakarta validation annotations enforce a constraint, and
doesn't take into account the 'groups' property of the annotation. This
results in constrains being added to the schema that would only be
applied for certain request, so incorrectly marks fields as required in
the resulting schema where they may only be mandatory on a subset of
requests. To overcome this, a new `ValidationAnnotationFilter` is being
introduced which only treats a validation annotation as constraining the
schema where the conditions on the filter are met be the annotation. The
default implementation of this filter only treats constraints from
annotations with no groups as being enforced to bring it inline with how
the default Java Beans validation operates.
  • Loading branch information
mc1arke committed Dec 7, 2024
1 parent df88c8a commit 74c49f5
Show file tree
Hide file tree
Showing 10 changed files with 268 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

import com.fasterxml.jackson.databind.type.TypeFactory;
import io.swagger.v3.core.jackson.ModelResolver;
import io.swagger.v3.core.jackson.ValidationAnnotationFilter;
import io.swagger.v3.core.jackson.ValidationAnnotationFilter.DefaultValidationAnnotationFilter;
import io.swagger.v3.core.util.Json;
import io.swagger.v3.core.util.Json31;
import io.swagger.v3.oas.models.media.Schema;
Expand Down Expand Up @@ -51,6 +53,16 @@ public ModelConverters(boolean openapi31, Schema.SchemaResolution schemaResoluti
}
}

public ModelConverters(boolean openapi31, Schema.SchemaResolution schemaResolution, ValidationAnnotationFilter validationAnnotationFilter) {
converters = new CopyOnWriteArrayList<>();
if (openapi31) {
converters.add(new ModelResolver(Json31.mapper()).openapi31(true).schemaResolution(schemaResolution).validationAnnotationFilter(validationAnnotationFilter));
} else {
converters.add(new ModelResolver(Json.mapper()).schemaResolution(schemaResolution).validationAnnotationFilter(validationAnnotationFilter));
}
}


public Set<String> getSkippedPackages() {
return skippedPackages;
}
Expand Down Expand Up @@ -78,17 +90,21 @@ public static void reset() {
}

public static ModelConverters getInstance(boolean openapi31, Schema.SchemaResolution schemaResolution) {
return getInstance(openapi31, schemaResolution, new DefaultValidationAnnotationFilter());
}

public static ModelConverters getInstance(boolean openapi31, Schema.SchemaResolution schemaResolution, ValidationAnnotationFilter validationAnnotationFilter) {
synchronized (ModelConverters.class) {
if (openapi31) {
if (SINGLETON31 == null) {
boolean applySchemaResolution = Boolean.parseBoolean(System.getProperty(Schema.APPLY_SCHEMA_RESOLUTION_PROPERTY, "false")) || Boolean.parseBoolean(System.getenv(Schema.APPLY_SCHEMA_RESOLUTION_PROPERTY));
SINGLETON31 = new ModelConverters(openapi31, applySchemaResolution ? schemaResolution : Schema.SchemaResolution.DEFAULT);
SINGLETON31 = new ModelConverters(openapi31, applySchemaResolution ? schemaResolution : Schema.SchemaResolution.DEFAULT, validationAnnotationFilter);
init(SINGLETON31);
}
return SINGLETON31;
}
if (SINGLETON == null) {
SINGLETON = new ModelConverters(openapi31, schemaResolution);
SINGLETON = new ModelConverters(openapi31, schemaResolution, validationAnnotationFilter);
init(SINGLETON);
}
return SINGLETON;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import io.swagger.v3.core.converter.AnnotatedType;
import io.swagger.v3.core.converter.ModelConverter;
import io.swagger.v3.core.converter.ModelConverterContext;
import io.swagger.v3.core.jackson.ValidationAnnotationFilter.DefaultValidationAnnotationFilter;
import io.swagger.v3.core.util.AnnotationsUtils;
import io.swagger.v3.core.util.Constants;
import io.swagger.v3.core.util.Json;
Expand All @@ -56,7 +57,6 @@
import io.swagger.v3.oas.models.media.IntegerSchema;
import io.swagger.v3.oas.models.media.JsonSchema;
import io.swagger.v3.oas.models.media.MapSchema;
import io.swagger.v3.oas.models.media.NumberSchema;
import io.swagger.v3.oas.models.media.ObjectSchema;
import io.swagger.v3.oas.models.media.Schema;
import io.swagger.v3.oas.models.media.StringSchema;
Expand All @@ -71,6 +71,9 @@
import javax.validation.constraints.DecimalMin;
import javax.validation.constraints.Max;
import javax.validation.constraints.Min;
import javax.validation.constraints.NotBlank;
import javax.validation.constraints.NotEmpty;
import javax.validation.constraints.NotNull;
import javax.validation.constraints.Pattern;
import javax.validation.constraints.Size;
import javax.xml.bind.annotation.XmlAccessType;
Expand Down Expand Up @@ -111,6 +114,11 @@
public class ModelResolver extends AbstractModelConverter implements ModelConverter {

Logger LOGGER = LoggerFactory.getLogger(ModelResolver.class);

/**
* @deprecated Use {@link ModelResolver#hasNotNullAnnotation(Annotation[], ValidationAnnotationFilter)} instead
*/
@Deprecated
public static List<String> NOT_NULL_ANNOTATIONS = Arrays.asList("NotNull", "NonNull", "NotBlank", "NotEmpty");

public static final String SET_PROPERTY_OF_COMPOSED_MODEL_AS_SIBLING = "composed-model-properties-as-sibiling";
Expand All @@ -125,6 +133,8 @@ public class ModelResolver extends AbstractModelConverter implements ModelConver

private boolean openapi31;

private ValidationAnnotationFilter validationAnnotationFilter = new DefaultValidationAnnotationFilter();

private Schema.SchemaResolution schemaResolution = Schema.SchemaResolution.DEFAULT;

public ModelResolver(ObjectMapper mapper) {
Expand Down Expand Up @@ -1574,57 +1584,56 @@ protected void applyBeanValidatorAnnotations(Schema property, Annotation[] annot
}
}
if (parent != null && annotations != null && applyNotNullAnnotations) {
boolean requiredItem = Arrays.stream(annotations).anyMatch(annotation ->
NOT_NULL_ANNOTATIONS.contains(annotation.annotationType().getSimpleName())
);
boolean requiredItem = hasNotNullAnnotation(annotations, validationAnnotationFilter);
if (requiredItem) {
addRequiredItem(parent, property.getName());
}
}
if (annos.containsKey("javax.validation.constraints.Min")) {
if (isNumberSchema(property)) {
Min min = (Min) annos.get("javax.validation.constraints.Min");
Min min = (Min) annos.get("javax.validation.constraints.Min");
if (isNumberSchema(property) && validationAnnotationFilter.isMinAnnotationApplicable(min)) {
property.setMinimum(new BigDecimal(min.value()));
}
}
if (annos.containsKey("javax.validation.constraints.Max")) {
if (isNumberSchema(property)) {
Max max = (Max) annos.get("javax.validation.constraints.Max");
Max max = (Max) annos.get("javax.validation.constraints.Max");
if (isNumberSchema(property) && validationAnnotationFilter.isMaxAnnotationApplicable(max)) {
property.setMaximum(new BigDecimal(max.value()));
}
}
if (annos.containsKey("javax.validation.constraints.Size")) {
Size size = (Size) annos.get("javax.validation.constraints.Size");
if (isNumberSchema(property)) {
boolean isApplicable = validationAnnotationFilter.isSizeAnnotationApplicable(size);
if (isApplicable && isNumberSchema(property)) {
property.setMinimum(new BigDecimal(size.min()));
property.setMaximum(new BigDecimal(size.max()));
}
if (isStringSchema(property)) {
if (isApplicable && isStringSchema(property)) {
property.setMinLength(Integer.valueOf(size.min()));
property.setMaxLength(Integer.valueOf(size.max()));
}
if (isArraySchema(property)) {
if (isApplicable && isArraySchema(property)) {
property.setMinItems(size.min());
property.setMaxItems(size.max());
}
}
if (annos.containsKey("javax.validation.constraints.DecimalMin")) {
DecimalMin min = (DecimalMin) annos.get("javax.validation.constraints.DecimalMin");
if (isNumberSchema(property)) {
if (isNumberSchema(property) && validationAnnotationFilter.isDecimalMinAnnotationApplicable(min)) {
property.setMinimum(new BigDecimal(min.value()));
property.setExclusiveMinimum(!min.inclusive());
}
}
if (annos.containsKey("javax.validation.constraints.DecimalMax")) {
DecimalMax max = (DecimalMax) annos.get("javax.validation.constraints.DecimalMax");
if (isNumberSchema(property)) {
if (isNumberSchema(property) && validationAnnotationFilter.isDecimalMaxAnnotationApplicable(max)) {
property.setMaximum(new BigDecimal(max.value()));
property.setExclusiveMaximum(!max.inclusive());
}
}
if (annos.containsKey("javax.validation.constraints.Pattern")) {
Pattern pattern = (Pattern) annos.get("javax.validation.constraints.Pattern");
if (isStringSchema(property)) {
if (isStringSchema(property) && validationAnnotationFilter.isPatternAnnotationApplicable(pattern)) {
property.setPattern(pattern.regexp());
}
if(property.getItems() != null && isStringSchema(property.getItems())) {
Expand Down Expand Up @@ -2416,9 +2425,9 @@ protected Set<String> resolveIgnoredProperties(Annotations a, Annotation[] annot
Set<String> propertiesToIgnore = new HashSet<>();
JsonIgnoreProperties ignoreProperties = a.get(JsonIgnoreProperties.class);
if (ignoreProperties != null) {
if(!ignoreProperties.allowGetters()) {
propertiesToIgnore.addAll(Arrays.asList(ignoreProperties.value()));
}
if(!ignoreProperties.allowGetters()) {
propertiesToIgnore.addAll(Arrays.asList(ignoreProperties.value()));
}
}
propertiesToIgnore.addAll(resolveIgnoredProperties(annotations));
return propertiesToIgnore;
Expand Down Expand Up @@ -3079,6 +3088,11 @@ public ModelResolver openapi31(boolean openapi31) {
return this;
}

public ModelResolver validationAnnotationFilter(ValidationAnnotationFilter validationAnnotationFilter) {
this.validationAnnotationFilter = validationAnnotationFilter;
return this;
}

public boolean isOpenapi31() {
return openapi31;
}
Expand Down Expand Up @@ -3126,4 +3140,38 @@ protected boolean applySchemaResolution() {
(Boolean.parseBoolean(System.getProperty(Schema.APPLY_SCHEMA_RESOLUTION_PROPERTY, "false")) ||
Boolean.parseBoolean(System.getenv(Schema.APPLY_SCHEMA_RESOLUTION_PROPERTY)));
}


/**
* Check if the parameter has any of the annotations that make it non-optional
*
* @param annotations the annotations to check
* @param validationAnnotationFilter the filters to use to remove annotations that don't match the current group
* @return whether any of the known NotNull annotations are present
*/
public static boolean hasNotNullAnnotation(Annotation[] annotations, ValidationAnnotationFilter validationAnnotationFilter) {
for (Annotation annotation : annotations) {
if (hasNotNullAnnotation(annotation, validationAnnotationFilter)) {
return true;
}
}
return false;
}

public static boolean hasNotNullAnnotation(Annotation annotation, ValidationAnnotationFilter validationAnnotationFilter) {
if (annotation instanceof NotEmpty && validationAnnotationFilter.isNotEmptyAnnotationApplicable((NotEmpty) annotation)) {
return true;
}
if (annotation instanceof NotBlank && validationAnnotationFilter.isNotBlankAnnotationApplicable((NotBlank) annotation)) {
return true;
}
if (annotation instanceof NotNull && validationAnnotationFilter.isNotNullAnnotationApplicable((NotNull) annotation)) {
return true;
}
if (annotation.annotationType().getSimpleName().equals("NonNull")) {
return true;
}
return false;
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
package io.swagger.v3.core.jackson;

import javax.validation.constraints.DecimalMax;
import javax.validation.constraints.DecimalMin;
import javax.validation.constraints.Max;
import javax.validation.constraints.Min;
import javax.validation.constraints.NotBlank;
import javax.validation.constraints.NotEmpty;
import javax.validation.constraints.NotNull;
import javax.validation.constraints.Pattern;
import javax.validation.constraints.Size;

public interface ValidationAnnotationFilter {

default boolean isNotEmptyAnnotationApplicable(NotEmpty notEmpty) {
return annotationGroupMatches(notEmpty.groups());
}

default boolean isNotBlankAnnotationApplicable(NotBlank notBlank) {
return annotationGroupMatches(notBlank.groups());
}

default boolean isNotNullAnnotationApplicable(NotNull notNull) {
return annotationGroupMatches(notNull.groups());
}

default boolean isMaxAnnotationApplicable(Max max) {
return annotationGroupMatches(max.groups());
}

default boolean isMinAnnotationApplicable(Min min) {
return annotationGroupMatches(min.groups());
}

default boolean isDecimalMaxAnnotationApplicable(DecimalMax decimalMax) {
return annotationGroupMatches(decimalMax.groups());
}

default boolean isDecimalMinAnnotationApplicable(DecimalMin decimalMin) {
return annotationGroupMatches(decimalMin.groups());
}

default boolean isSizeAnnotationApplicable(Size size) {
return annotationGroupMatches(size.groups());
}

default boolean isPatternAnnotationApplicable(Pattern pattern) {
return annotationGroupMatches(pattern.groups());
}

boolean annotationGroupMatches(Class<?>[] annotationGroups);

class DefaultValidationAnnotationFilter implements ValidationAnnotationFilter {

@Override
public boolean annotationGroupMatches(Class<?>[] annotationGroups) {
return annotationGroups.length == 0;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
import io.swagger.v3.core.converter.ModelConverters;
import io.swagger.v3.core.converter.ResolvedSchema;
import io.swagger.v3.core.jackson.ModelResolver;
import io.swagger.v3.core.jackson.ValidationAnnotationFilter;
import io.swagger.v3.core.jackson.ValidationAnnotationFilter.DefaultValidationAnnotationFilter;
import io.swagger.v3.oas.annotations.enums.Explode;
import io.swagger.v3.oas.annotations.media.ExampleObject;
import io.swagger.v3.oas.models.Components;
Expand Down Expand Up @@ -55,7 +57,20 @@ public static Parameter applyAnnotations(
JsonView jsonViewAnnotation,
boolean openapi31,
Schema.SchemaResolution schemaResolution) {
return applyAnnotations(parameter, type, annotations, components, classTypes, methodTypes, jsonViewAnnotation, openapi31, schemaResolution, new DefaultValidationAnnotationFilter());
}

public static Parameter applyAnnotations(
Parameter parameter,
Type type,
List<Annotation> annotations,
Components components,
String[] classTypes,
String[] methodTypes,
JsonView jsonViewAnnotation,
boolean openapi31,
Schema.SchemaResolution schemaResolution,
ValidationAnnotationFilter validationAnnotationFilter) {
final AnnotationsHelper helper = new AnnotationsHelper(annotations, type);
if (helper.isContext()) {
return null;
Expand Down Expand Up @@ -252,7 +267,7 @@ public static Parameter applyAnnotations(
} catch (Exception e) {
LOGGER.error("failed on " + annotation.annotationType().getName(), e);
}
} else if (ModelResolver.NOT_NULL_ANNOTATIONS.contains(annotation.annotationType().getSimpleName())) {
} else if (ModelResolver.hasNotNullAnnotation(annotation, validationAnnotationFilter)) {
parameter.setRequired(true);
}
}
Expand Down
Loading

0 comments on commit 74c49f5

Please sign in to comment.