Skip to content

Commit

Permalink
Merge pull request #661 from MikeEdgar/659_improve_param_scan_2.0.x
Browse files Browse the repository at this point in the history 
Parameter schema improvement & bug fix (2.0.x)
  • Loading branch information
@phillip-kruger
phillip-kruger authored Feb 7, 2021
2 parents 4b6956f + fa8607c commit 4a094ed
Show file tree
Hide file tree
Showing 12 changed files with 525 additions and 79 deletions.
58 changes: 58 additions & 0 deletions core/src/main/java/io/smallrye/openapi/api/models/media/SchemaImpl.java
View file

Large diffs are not rendered by default.

6 changes: 4 additions & 2 deletions core/src/main/java/io/smallrye/openapi/runtime/scanner/ResourceParameters.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,10 @@
*/
public class ResourceParameters {

private static final Comparator<Parameter> PARAMETER_COMPARATOR = Comparator.comparing(Parameter::getIn)
.thenComparing(Parameter::getName);
public static final Comparator<Parameter> PARAMETER_COMPARATOR = Comparator
.comparing(Parameter::getRef, Comparator.nullsFirst(Comparator.naturalOrder()))
.thenComparing(Parameter::getIn, Comparator.nullsLast(Comparator.naturalOrder()))
.thenComparing(Parameter::getName, Comparator.nullsLast(Comparator.naturalOrder()));

static final Pattern TEMPLATE_PARAM_PATTERN = Pattern.compile("\\{(\\w[\\w\\.-]*)\\}");

Expand Down
211 changes: 163 additions & 48 deletions core/src/main/java/io/smallrye/openapi/runtime/scanner/spi/AbstractParameterProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashMap;
Expand Down Expand Up @@ -156,39 +155,62 @@ protected static class ParameterContextKey {
final String name;
final In location;
final Style style;
final String ref;

public ParameterContextKey(String name, In location, Style style) {
this.name = name;
this.location = location;
this.style = style;
this.ref = null;
}

public ParameterContextKey(Parameter oaiParam) {
this.name = oaiParam.getName();
this.location = oaiParam.getIn();
this.style = styleOf(oaiParam);
this.ref = oaiParam.getRef();
}

public ParameterContextKey(ParameterContext context) {
this.name = context.name;
this.location = context.location;
this.style = context.style;
this.ref = context.oaiParam != null ? context.oaiParam.getRef() : null;
}

@Override
public boolean equals(Object obj) {
if (obj instanceof ParameterContextKey) {
ParameterContextKey other = (ParameterContextKey) obj;

if (isNull() && other.isNull()) {
return this == other;
}

if (ref != null) {
return ref.equals(other.ref);
}

return Objects.equals(this.name, other.name) &&
Objects.equals(this.location, other.location) &&
Objects.equals(this.style, other.style);
}

return false;
}

@Override
public int hashCode() {
return Objects.hash(name, location, style);
return isNull() ? super.hashCode() : Objects.hash(name, location, style, ref);
}

@Override
public String toString() {
return "name: " + name + "; in: " + location;
return "name: " + name + "; in: " + location + "; style: " + style + "; ref: " + ref;
}

public boolean isNull() {
return name == null && location == null && style == null && ref == null;
}
}

Expand Down Expand Up @@ -232,7 +254,7 @@ protected void processOperationParameters(MethodInfo resourceMethod, ResourcePar
// Phase II - Read method argument @Parameter and framework's annotations
resourceMethod.annotations()
.stream()
.filter(a -> !a.target().equals(resourceMethod))
.filter(a -> !JandexUtil.equals(a.target(), resourceMethod))
.forEach(annotation -> {
/*
* This condition exists to support @Parameters wrapper annotation
Expand All @@ -248,7 +270,7 @@ protected void processOperationParameters(MethodInfo resourceMethod, ResourcePar
// Phase III - Read method @Parameter(s) annotations
resourceMethod.annotations()
.stream()
.filter(a -> a.target().equals(resourceMethod))
.filter(a -> JandexUtil.equals(a.target(), resourceMethod))
.filter(a -> openApiParameterAnnotations.contains(a.name()))
.forEach(this::readParameterAnnotation);

Expand Down Expand Up @@ -391,8 +413,7 @@ protected List<Parameter> getParameters(MethodInfo resourceMethod) {
.stream()
.map(context -> this.mapParameter(resourceMethod, context))
.filter(Objects::nonNull)
.sorted(Comparator.comparing(Parameter::getIn)
.thenComparing(Parameter::getName))
.sorted(ResourceParameters.PARAMETER_COMPARATOR)
.collect(Collectors.toList());

return parameters.isEmpty() ? null : parameters;
Expand Down Expand Up @@ -499,17 +520,7 @@ private Parameter mapParameter(MethodInfo resourceMethod, ParameterContext conte
}

if (param.getSchema() != null) {
//TODO: Test BV annotations on all target types
BeanValidationScanner.applyConstraints(context.target,
param.getSchema(),
param.getName(),
(target, name) -> {
if (param.getRequired() == null) {
param.setRequired(Boolean.TRUE);
}
});

setDefaultValue(param.getSchema(), context.defaultValue);
augmentParamSchema(param, context);
}

if (param.getRequired() == null && TypeUtil.isOptional(context.targetType)) {
Expand All @@ -519,6 +530,61 @@ private Parameter mapParameter(MethodInfo resourceMethod, ParameterContext conte
return param;
}

/**
* Scan the AnnotationTarget associated with Parameter <code>param</code> for supported
* default value or Bean Validation annotations that may alter the schema. When
* found, update param's schema either directly or, when a <code>ref</code> is present, by
* creating a union of the local and reference schemas using <code>allOf</code>.
*
* @param param the Parameter containing a schema for update
* @param context ParameterContext associated with param
*/
void augmentParamSchema(Parameter param, ParameterContext context) {
Schema paramSchema = param.getSchema();
String ref = paramSchema.getRef();
Schema localSchema;

if (ref != null) {
Type paramType = TypeUtil.unwrapType(context.targetType);

/*
* Lookup the schema `type` from components (if available) or guess the type if
* the `ref` is not available.
*/
Schema refSchema = ModelUtil.getComponent(scannerContext.getOpenApi(), ref);

if (refSchema != null) {
localSchema = new SchemaImpl().type(refSchema.getType());
} else {
localSchema = new SchemaImpl().type(SchemaType
.valueOf(TypeUtil.getTypeAttributes(paramType).get(SchemaConstant.PROP_TYPE).toString().toUpperCase()));
}
} else {
localSchema = paramSchema;
}

int modCount = SchemaImpl.getModCount(localSchema);

BeanValidationScanner.applyConstraints(context.target, localSchema, param.getName(),
(target, name) -> {
if (param.getRequired() == null) {
param.setRequired(Boolean.TRUE);
}
});

setDefaultValue(localSchema, context.defaultValue);

if (localOnlySchemaModified(paramSchema, localSchema, modCount)) {
// Add new `allOf` schema, erasing `type` derived above from the local schema
Schema allOf = new SchemaImpl().addAllOf(paramSchema).addAllOf(localSchema.type(null));
param.setSchema(allOf);
}
}

boolean localOnlySchemaModified(Schema paramSchema, Schema localSchema, int originalModCount) {
return localSchema != paramSchema && originalModCount != -1 && SchemaImpl.getModCount(localSchema) > originalModCount;
}

/**
* Converts the collection of parameter annotations to properties set on the
* given schema.
Expand Down Expand Up @@ -587,7 +653,7 @@ private void setRequired(String name, Schema schema) {
}

private void setDefaultValue(Schema schema, Object defaultValue) {
if (schema.getDefaultValue() == null) {
if (schema.getDefaultValue() == null && defaultValue != null) {
schema.setDefaultValue(defaultValue);
}
}
Expand Down Expand Up @@ -661,10 +727,11 @@ protected void addEncoding(Map<String, Encoding> encodings, String paramName, An
* @see org.eclipse.microprofile.openapi.annotations.parameters.Parameter#in()
*/
protected boolean isIgnoredParameter(Parameter parameter, AnnotationTarget resourceMethod) {
String paramName = parameter.getName();
In paramIn = parameter.getIn();
final String ref = parameter.getRef();
final String paramName = parameter.getName();
final In paramIn = parameter.getIn();

if (paramIn == null) {
if (paramIn == null && ref == null) {
/*
* Per @Parameter JavaDoc, ignored when empty string (i.e., unspecified).
* This may occur when @Parameter is specified without a matching framework
Expand All @@ -680,7 +747,7 @@ protected boolean isIgnoredParameter(Parameter parameter, AnnotationTarget resou
/*
* Name is REQUIRED unless it is a reference.
*/
if ((paramName == null || paramName.trim().isEmpty()) && parameter.getRef() == null) {
if ((paramName == null || paramName.trim().isEmpty()) && ref == null) {
return true;
}

Expand Down Expand Up @@ -791,7 +858,7 @@ protected abstract void readAnnotatedType(AnnotationInstance annotation, Annotat
* @param param the {@link Parameter}
* @return the param's style, the default style defined based on <code>in</code>, or null if <code>in</code> is not defined.
*/
protected Style styleOf(Parameter param) {
protected static Style styleOf(Parameter param) {
if (param.getStyle() != null) {
return param.getStyle();
}
Expand Down Expand Up @@ -1064,6 +1131,21 @@ protected static Type getType(AnnotationTarget target) {
return type;
}

protected boolean isReadableParameterAnnotation(DotName name) {
return ParameterConstant.DOTNAME_PARAMETER.equals(name) && readerFunction != null;
}

protected void readParameterAnnotation(AnnotationInstance annotation, boolean overriddenParametersOnly) {
Parameter oaiParam = readerFunction.apply(annotation);

readParameter(new ParameterContextKey(oaiParam),
oaiParam,
null,
null,
annotation.target(),
overriddenParametersOnly);
}

/**
* Merges MP-OAI {@link Parameter}s and framework-specific parameters for the same {@link In} and name,
* and {@link Style}. When overriddenParametersOnly is true, new parameters not already known
Expand Down Expand Up @@ -1162,49 +1244,82 @@ ParameterContext getParameterContext(ParameterContextKey key, AnnotationTarget t
context = params
.values()
.stream()
// Only attempt to match parameters without `ref` specified
.filter(c -> c.oaiParam == null || c.oaiParam.getRef() == null)
.filter(c -> haveSameAnnotatedTarget(c, target, key.name))
.filter(c -> attributesCompatible(c.location, key.location))
.filter(c -> attributesCompatible(c.style, key.style))
.findFirst()
.orElse(null);
}

if (context == null) {
/*
* Allow a match on just the name and style if one of the Parameter.In values
* is not specified
*/
context = params.values().stream().filter(c -> {
if (c.location == null || key.location == null) {
return Objects.equals(c.name, key.name) && Objects.equals(c.style, key.style);
}
return false;
}).findFirst().orElse(null);

.orElseGet(() -> params.values()
.stream()
.filter(c -> nameAndStyleMatch(c, key))
.findFirst()
.orElse(null));
}

return context;
}

boolean haveSameAnnotatedTarget(ParameterContext context, AnnotationTarget target, String name) {
/*
* Consider names to match if one is unspecified or they are equal.
*/
boolean nameMatches = (context.name == null || name == null || Objects.equals(context.name, name));
boolean sameTarget = false;

if (JandexUtil.equals(target, context.target)) {
if (target.kind() != Kind.METHOD) {
// Both annotations are set on the same method argument or field
sameTarget = true;
} else {
/*
* Method targets: consider names to match if *one* is unspecified or they are equal.
*/
if (name != null) {
sameTarget = context.name == null || Objects.equals(name, context.name);
} else if (context.name != null) {
sameTarget = true;
}
}
} else if (JandexUtil.equals(targetMethod(target), targetMethod(context.target))) {
/*
* The name must match for annotations on a method because it is
* ambiguous which parameters is being referenced.
* One annotation is on the method and the other is on a parameter. Require that
* the names are both given and match.
*/
return nameMatches || target.kind() != Kind.METHOD;
sameTarget = name != null && name.equals(context.name);
}

if (nameMatches && target.kind() == Kind.METHOD && context.target.kind() == Kind.METHOD_PARAMETER) {
return context.target.asMethodParameter().method().equals(target);
return sameTarget;
}

boolean attributesCompatible(Object a1, Object a2) {
return a1 == null || a2 == null || a1.equals(a2);
}

boolean nameAndStyleMatch(ParameterContext context, ParameterContextKey key) {
/*
* Allow a match on just the name and style if one of the Parameter.In values
* is not specified
*/
if ((context.location == null || key.location == null) && key.name != null && key.style != null) {
return key.name.equals(context.name) && key.style.equals(context.style);
}

return false;
}

/**
* Obtain the MethodInfo associated with the annotation target.
*
* @param target annotated item. Only method and method parameter targets.
* @return the MethodInfo associated with the target, or null if target is not a method or parameter.
*/
static MethodInfo targetMethod(AnnotationTarget target) {
if (target.kind() == Kind.METHOD) {
return target.asMethod();
}
if (target.kind() == Kind.METHOD_PARAMETER) {
return target.asMethodParameter().method();
}
return null;
}

/**
* Scans for class level parameters on the given class argument and its ancestors.
*
Expand Down
14 changes: 13 additions & 1 deletion core/src/main/java/io/smallrye/openapi/runtime/util/JandexUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,15 @@ public enum RefType {
RefType(String componentPath) {
this.componentPath = componentPath;
}

public static RefType fromComponentPath(String path) {
for (RefType ref : values()) {
if (ref.componentPath.equals(path)) {
return ref;
}
}
return null;
}
}

/**
Expand Down Expand Up @@ -510,7 +519,10 @@ public static Map<ClassInfo, Type> inheritanceChain(IndexView index, ClassInfo k
}

public static boolean equals(AnnotationTarget t1, AnnotationTarget t2) {
if (t1.kind() != t2.kind()) {
if (t1 == t2) {
return true;
}
if (t1 == null || t2 == null || t1.kind() != t2.kind()) {
return false;
}

Expand Down
Loading

0 comments on commit 4a094ed

Please sign in to comment.