Skip to content

Commit a688a69

Browse files
PostremusPostremus
authored
per: optimize AbstractParameterProcessor.readParameters (#2646)
readParameters checked each annotation on the class, if it is parameter. Then checked if the annotation instances target is FIELD|METHOD|METHOD_PARAMETER, and then checked that target if it has any parameter annotations. I turned it around. We overall only check fields and methods now for parameter annotations. For fields we only require one isParameter check for the specific annotation instance. For methods, we check if the method could even be a bean property method first. Short circuit for parameterless methods (constructors, static initializers). This saves around 2/3 of the calls towards isResourceMethod (150k -> 50k). We also save on retrieving the annotations of each target again and again, if the target has multiple parameter annotations (resource methods, constructors). Optimize JaxRsParameterProcessor.hasMethodHTTPMethodAnnotation further to save on the ArrayList allocation inside MethodInfo.declaredAnnotations(). Also compare by name directly instead of AnnotationInstance. sorted-parameters.enable=false had the problem, that parameters picked up from beanparams where not sorted in declaration order. They where instead grouped by annotation (i.e. path param, query param, etc), and in each group in declaration order. The behaviour is now so that beanparams also follow declaration order; And their parameters are inserted right between the other parameters from the "parents" (e.g. the resource methods) parameters. This is also ensured for parameters declared on constructors. Jandex sorts method parameters alphabetically, so we have to resort based on parameter position. Add a test to ensure the declaration order sorting stays consistent.
1 parent 9e6aafa commit a688a69

6 files changed

Lines changed: 300 additions & 94 deletions

File tree

‎README.adoc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ Set this boolean value to disable the merging of the deprecated `@Schema` `examp
140140
----
141141
mp.openapi.extensions.smallrye.sorted-parameters.enable
142142
----
143-
Set this boolean value to enable or disable the sorting of parameter array entries during annotation scanning. When enabled (set to `true`), parameters will be order either by their order within a `@Parameters` annotation on an operation method or (in the absence of that annotation) by their `$ref`, `in`, and `name` attributes. When disabled (set to `false`), parameters will be in the order they are encountered in the Java code. If not set, it will default to `true`.
143+
Set this boolean value to enable or disable the sorting of parameter array entries during annotation scanning. When enabled (set to `true`), parameters will be order either by their order within a `@Parameters` annotation on an operation method or (in the absence of that annotation) by their `$ref`, `in`, and `name` attributes. When disabled (set to `false`), parameters will be in the order they are encountered in the Java code. Within a class, fields are encountered before methods and constructors, each in declaration order. If not set, it will default to `true`.
144144

145145
[#generic-response-use-default]
146146
==== Generic Responses use `default` code

‎core/src/main/java/io/smallrye/openapi/runtime/scanner/spi/AbstractParameterProcessor.java‎

Lines changed: 44 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@
4343
import org.jboss.jandex.FieldInfo;
4444
import org.jboss.jandex.IndexView;
4545
import org.jboss.jandex.MethodInfo;
46-
import org.jboss.jandex.MethodParameterInfo;
4746
import org.jboss.jandex.PrimitiveType.Primitive;
4847
import org.jboss.jandex.Type;
4948

@@ -139,6 +138,20 @@ public int compare(AnnotationInstance o1, AnnotationInstance o2) {
139138

140139
private final Comparator<AnnotationInstance> frameworkParametersFirst = new FrameworkParamsFirstComparator();
141140

141+
/**
142+
* Jandex sorts method arguments alphabetically. This comparator ensures they are sorted by their declaration order instead.
143+
*/
144+
private static final Comparator<AnnotationInstance> COMPARATOR_METHOD_PARAMETER_ORDER = Comparator
145+
.comparingInt(an -> {
146+
AnnotationTarget target = an.target();
147+
if (target.kind() != Kind.METHOD_PARAMETER) {
148+
// order method level parameters before method argument level parameters
149+
return -1;
150+
}
151+
152+
return target.asMethodParameter().position();
153+
});
154+
142155
protected AbstractParameterProcessor(AnnotationScannerContext scannerContext,
143156
String contextPath,
144157
ClassInfo resourceClass,
@@ -1596,73 +1609,51 @@ protected void readParametersInherited(ClassInfo clazz, AnnotationInstance beanP
15961609
protected void readParameters(ClassInfo clazz, AnnotationInstance beanParamAnnotation, boolean overriddenParametersOnly) {
15971610
List<AnnotationInstance> paramAnnotations = new ArrayList<>();
15981611

1599-
for (Map.Entry<DotName, List<AnnotationInstance>> entry : clazz.annotationsMap().entrySet()) {
1600-
DotName name = entry.getKey();
1601-
1602-
if (Names.PARAMETER.equals(name) || isParameter(name)) {
1603-
for (AnnotationInstance annotation : entry.getValue()) {
1604-
if (isBeanPropertyParam(annotation)) {
1605-
paramAnnotations.add(annotation);
1606-
}
1612+
for (FieldInfo fieldInfo : clazz.fieldsInDeclarationOrder()) {
1613+
for (AnnotationInstance annotation : fieldInfo.annotations()) {
1614+
if (annotation.target().kind() == Kind.FIELD && isParameter(annotation.name())) {
1615+
paramAnnotations.add(annotation);
16071616
}
16081617
}
16091618
}
16101619

1611-
Collections.sort(paramAnnotations, frameworkParametersFirst);
1612-
1613-
for (AnnotationInstance annotation : paramAnnotations) {
1614-
readAnnotatedType(annotation, beanParamAnnotation, overriddenParametersOnly);
1615-
}
1616-
}
1620+
for (MethodInfo method : clazz.methodsInDeclarationOrder()) {
1621+
if (!isBeanPropertyMethod(method)) {
1622+
continue;
1623+
}
16171624

1618-
/**
1619-
* Determines if the annotation is a property parameter. Annotation targets
1620-
* must be annotated with a framework-specific parameter annotation or
1621-
* {@link org.eclipse.microprofile.openapi.annotations.parameters.Parameter @Parameter}.
1622-
*
1623-
* Method targets must not be annotated with one of the framework-specific HTTP method annotations and
1624-
* the method must have a single argument.
1625-
*
1626-
* @param annotation
1627-
* @return
1628-
*/
1629-
boolean isBeanPropertyParam(AnnotationInstance annotation) {
1630-
AnnotationTarget target = annotation.target();
1631-
boolean relevant = false;
1625+
int methodOffset = paramAnnotations.size();
16321626

1633-
switch (target.kind()) {
1634-
case FIELD: {
1635-
FieldInfo field = target.asField();
1636-
relevant = hasParameters(field.annotations());
1637-
break;
1627+
for (AnnotationInstance annotation : method.annotations()) {
1628+
AnnotationTarget target = annotation.target();
1629+
if (target.kind() == Kind.METHOD_PARAMETER) {
1630+
if (isParameter(annotation.name())) {
1631+
paramAnnotations.add(annotation);
1632+
}
1633+
} else if (target.kind() == Kind.METHOD) {
1634+
if (getType(target) != null && isParameter(annotation.name())) {
1635+
paramAnnotations.add(annotation);
1636+
}
1637+
}
16381638
}
16391639

1640-
case METHOD_PARAMETER: {
1641-
MethodParameterInfo param = target.asMethodParameter();
1642-
MethodInfo method = param.method();
1643-
relevant = nonSyntheticParameterMethod(method,
1644-
scannerContext.annotations().getMethodParameterAnnotations(method, param.position()));
1645-
break;
1640+
if (paramAnnotations.size() - methodOffset > 1) {
1641+
paramAnnotations.subList(methodOffset, paramAnnotations.size()).sort(COMPARATOR_METHOD_PARAMETER_ORDER);
16461642
}
1643+
}
16471644

1648-
case METHOD: {
1649-
MethodInfo method = target.asMethod();
1650-
relevant = nonSyntheticParameterMethod(method, method.annotations()) &&
1651-
getType(target) != null;
1652-
break;
1653-
}
1645+
paramAnnotations.sort(frameworkParametersFirst);
16541646

1655-
default:
1656-
break;
1647+
for (AnnotationInstance annotation : paramAnnotations) {
1648+
readAnnotatedType(annotation, beanParamAnnotation, overriddenParametersOnly);
16571649
}
1658-
1659-
return relevant;
16601650
}
16611651

1662-
boolean nonSyntheticParameterMethod(MethodInfo method, Collection<AnnotationInstance> annotations) {
1663-
return !method.isSynthetic() &&
1652+
boolean isBeanPropertyMethod(MethodInfo method) {
1653+
// A method without parameters is neither a "setter" nor a source of parameters
1654+
return method.parametersCount() > 0 &&
1655+
!method.isSynthetic() &&
16641656
!isResourceMethod(method) &&
1665-
hasParameters(annotations) &&
16661657
!isSubResourceLocator(method);
16671658
}
16681659

@@ -1685,18 +1676,6 @@ boolean nonSyntheticParameterMethod(MethodInfo method, Collection<AnnotationInst
16851676
*/
16861677
protected abstract boolean isResourceMethod(MethodInfo method);
16871678

1688-
/**
1689-
* Check for the existence relevant parameter annotations in the collection.
1690-
*
1691-
* @param annotations collection of annotations
1692-
* @return true if any of the annotations is a relevant parameter annotation.
1693-
*/
1694-
protected boolean hasParameters(Collection<AnnotationInstance> annotations) {
1695-
return annotations.stream()
1696-
.map(AnnotationInstance::name)
1697-
.anyMatch(this::isParameter);
1698-
}
1699-
17001679
protected abstract boolean isParameter(DotName annotationName);
17011680

17021681
/**

‎extension-jaxrs/src/main/java/io/smallrye/openapi/jaxrs/JaxRsConstants.java‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,6 @@
1212

1313
import org.eclipse.microprofile.openapi.models.PathItem;
1414
import org.eclipse.microprofile.openapi.models.PathItem.HttpMethod;
15-
import org.jboss.jandex.AnnotationInstance;
16-
import org.jboss.jandex.AnnotationValue;
1715
import org.jboss.jandex.DotName;
1816

1917
/**
@@ -125,12 +123,6 @@ public class JaxRsConstants {
125123
public static final Set<DotName> HTTP_METHODS = Collections
126124
.unmodifiableSet(methods);
127125

128-
private static final AnnotationValue[] EMPTY_VALUES = new AnnotationValue[0];
129-
public static final Set<AnnotationInstance> HTTP_METHOD_INSTANCES = methods
130-
.stream()
131-
.map(name -> AnnotationInstance.create(name, null, EMPTY_VALUES))
132-
.collect(Collectors.toUnmodifiableSet());
133-
134126
public static final Map<PathItem.HttpMethod, Set<DotName>> HTTP_METHOD_ANNOTATIONS;
135127
static {
136128
Map<PathItem.HttpMethod, Set<DotName>> annotations = new EnumMap<>(PathItem.HttpMethod.class);

‎extension-jaxrs/src/main/java/io/smallrye/openapi/jaxrs/JaxRsParameterProcessor.java‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -302,14 +302,11 @@ protected boolean isResourceMethod(MethodInfo method) {
302302
}
303303

304304
private boolean hasMethodHTTPMethodAnnotation(MethodInfo method) {
305-
for (AnnotationInstance a : method.declaredAnnotations()) {
306-
for (AnnotationInstance httpMethod : JaxRsConstants.HTTP_METHOD_INSTANCES) {
307-
if (a.equivalentTo(httpMethod)) {
308-
return true;
309-
}
305+
for (AnnotationInstance a : method.annotations()) {
306+
if (a.target().kind() == AnnotationTarget.Kind.METHOD && JaxRsConstants.HTTP_METHODS.contains(a.name())) {
307+
return true;
310308
}
311309
}
312-
313310
return false;
314311
}
315312

‎extension-jaxrs/src/test/java/io/smallrye/openapi/runtime/scanner/ParameterScanTests.java‎

Lines changed: 85 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,11 @@
1818
import java.util.Optional;
1919
import java.util.OptionalDouble;
2020
import java.util.OptionalLong;
21+
import java.util.UUID;
2122

2223
import org.eclipse.microprofile.openapi.annotations.Operation;
2324
import org.eclipse.microprofile.openapi.annotations.enums.Explode;
25+
import org.eclipse.microprofile.openapi.annotations.enums.ParameterIn;
2426
import org.eclipse.microprofile.openapi.annotations.enums.ParameterStyle;
2527
import org.eclipse.microprofile.openapi.annotations.enums.SchemaType;
2628
import org.eclipse.microprofile.openapi.annotations.media.Content;
@@ -808,22 +810,97 @@ void testNullableRefParam(String oasVersion, String expectedResource) throws IOE
808810

809811
@Test
810812
void testUnsortedParameters() throws IOException, JSONException {
811-
@jakarta.ws.rs.Path("/status")
813+
class ListOptions {
814+
815+
@jakarta.ws.rs.QueryParam("beanparam-field-1")
816+
UUID field1;
817+
818+
@jakarta.ws.rs.HeaderParam("beanparam-field-2")
819+
UUID field2;
820+
821+
@jakarta.ws.rs.PathParam("beanparam-field-3")
822+
UUID field3;
823+
824+
@jakarta.ws.rs.QueryParam("beanparam-method-4")
825+
public void field4(String field4) {
826+
}
827+
828+
@jakarta.ws.rs.HeaderParam("beanparam-method-5")
829+
public void field5(String field5) {
830+
}
831+
832+
@jakarta.ws.rs.PathParam("beanparam-method-6")
833+
public void field6(String field6) {
834+
}
835+
836+
public ListOptions(@jakarta.ws.rs.QueryParam("beanparam-constructor-c-10") String constructor10,
837+
@jakarta.ws.rs.HeaderParam("beanparam-constructor-d-11") String constructor11,
838+
@jakarta.ws.rs.PathParam("beanparam-constructor-a-12") String constructor12) {
839+
840+
}
841+
842+
@Parameter(name = "beanparam-method-42", in = ParameterIn.QUERY)
843+
public void field41(@jakarta.ws.rs.QueryParam("beanparam-method-41") String field41) {
844+
}
845+
846+
@jakarta.ws.rs.QueryParam("beanparam-field-7")
847+
UUID field7;
848+
849+
@jakarta.ws.rs.HeaderParam("beanparam-field-8")
850+
UUID field8;
851+
852+
@jakarta.ws.rs.PathParam("beanparam-field-9")
853+
UUID field9;
854+
}
855+
856+
@jakarta.ws.rs.Path("/status/{method-1-p1}/{field-3}/{method-6}/{field-9}/{beanparam-field-3}/{beanparam-method-6}/{beanparam-field-9}/{beanparam-constructor-a-12}")
812857
class Resource {
858+
@jakarta.ws.rs.QueryParam("field-1")
859+
UUID field1;
860+
861+
@jakarta.ws.rs.HeaderParam("field-2")
862+
UUID field2;
863+
864+
@jakarta.ws.rs.PathParam("field-3")
865+
UUID field3;
866+
867+
@Parameter(name = "method-additional-1", in = ParameterIn.QUERY)
868+
@Parameter(name = "method-additional-2", in = ParameterIn.HEADER)
813869
@jakarta.ws.rs.GET
814-
@jakarta.ws.rs.Path("/{resourceId}")
815870
public String get(
816-
@jakarta.ws.rs.QueryParam("q7") int q7,
817-
@jakarta.ws.rs.QueryParam("q9") int q9,
818-
@jakarta.ws.rs.HeaderParam("h6") int h8,
819-
@jakarta.ws.rs.PathParam("resourceId") String resourceId,
820-
@jakarta.ws.rs.QueryParam("q8") int q8) {
871+
@jakarta.ws.rs.QueryParam("method-1-q1") int q1,
872+
@jakarta.ws.rs.QueryParam("method-1-q2") int q2,
873+
@jakarta.ws.rs.HeaderParam("method-1-h1") int h3,
874+
@jakarta.ws.rs.BeanParam ListOptions listOptions,
875+
@jakarta.ws.rs.PathParam("method-1-p1") String resourceId,
876+
@jakarta.ws.rs.QueryParam("method-1-q4") int q4) {
821877
return null;
822878
}
879+
880+
@jakarta.ws.rs.QueryParam("method-4")
881+
public void field4(String field4) {
882+
}
883+
884+
@jakarta.ws.rs.HeaderParam("method-5")
885+
public void field5(String field5) {
886+
}
887+
888+
@jakarta.ws.rs.PathParam("method-6")
889+
public void field6(String field6) {
890+
}
891+
892+
@jakarta.ws.rs.QueryParam("field-7")
893+
String field7;
894+
895+
@jakarta.ws.rs.HeaderParam("field-8")
896+
String field8;
897+
898+
@jakarta.ws.rs.PathParam("field-9")
899+
String field9;
823900
}
824901

825902
test(dynamicConfig(SmallRyeOASConfig.SMALLRYE_SORTED_PARAMETERS_ENABLE, Boolean.FALSE),
826-
"params.unsorted-scan-order.json", Resource.class);
903+
"params.unsorted-scan-order.json", Resource.class, ListOptions.class);
827904
}
828905

829906
@Test

0 commit comments

Comments
 (0)