Skip to content

Commit

Permalink
Improve handling of parameter refs and missing parameter key properties
Browse files Browse the repository at this point in the history
  • Loading branch information
@MikeEdgar
MikeEdgar committed Feb 7, 2021
1 parent 4b6956f commit a444d66
Show file tree
Hide file tree
Showing 9 changed files with 256 additions and 67 deletions.
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
142 changes: 106 additions & 36 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 @@ -661,10 +682,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 +702,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 +813,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 +1086,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 +1199,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
5 changes: 4 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 @@ -510,7 +510,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
2 changes: 2 additions & 0 deletions core/src/test/java/io/smallrye/openapi/runtime/scanner/IndexScannerTestBase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import io.smallrye.openapi.api.OpenApiConfigImpl;
import io.smallrye.openapi.api.models.ComponentsImpl;
import io.smallrye.openapi.api.models.OpenAPIImpl;
import io.smallrye.openapi.runtime.io.CurrentScannerInfo;
import io.smallrye.openapi.runtime.io.Format;
import io.smallrye.openapi.runtime.io.OpenApiSerializer;

Expand All @@ -39,6 +40,7 @@ public class IndexScannerTestBase {
@After
public void removeSchemaRegistry() {
SchemaRegistry.remove();
CurrentScannerInfo.remove();
}

protected static String pathOf(Class<?> clazz) {
Expand Down
11 changes: 2 additions & 9 deletions extension-jaxrs/src/main/java/io/smallrye/openapi/jaxrs/JaxRsParameterProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,15 +158,8 @@ protected void readAnnotatedType(AnnotationInstance annotation, AnnotationInstan
boolean overriddenParametersOnly) {
DotName name = annotation.name();

if (ParameterConstant.DOTNAME_PARAMETER.equals(name) && readerFunction != null) {
Parameter oaiParam = readerFunction.apply(annotation);

readParameter(new ParameterContextKey(oaiParam.getName(), oaiParam.getIn(), styleOf(oaiParam)),
oaiParam,
null,
null,
annotation.target(),
overriddenParametersOnly);
if (isReadableParameterAnnotation(name)) {
readParameterAnnotation(annotation, overriddenParametersOnly);
} else {
FrameworkParameter frameworkParam = JaxRsParameter.forName(name);

Expand Down
Loading

0 comments on commit a444d66

Please sign in to comment.