Avoid NPE in path parameter search, log MP params w/o framework param

Fixes smallrye#1191 Signed-off-by: Michael Edgar <michael@xlate.io>
MikeEdgar · Jul 31, 2022 · e15de67 · e15de67
1 parent aa58d8a
commit e15de67
Show file tree

Hide file tree

Showing 6 changed files with 96 additions and 6 deletions.
diff --git a/core/src/main/java/io/smallrye/openapi/api/OpenApiConfigImpl.java b/core/src/main/java/io/smallrye/openapi/api/OpenApiConfigImpl.java
@@ -422,6 +422,10 @@ public void doAllowNakedPathParameter() {
         this.allowNakedPathParameter = Optional.of(true);
     }
 
+    public void setAllowNakedPathParameter(Boolean allowNakedPathParameter) {
+        this.allowNakedPathParameter = Optional.ofNullable(allowNakedPathParameter);
+    }
+
     @Override
     public Optional<String[]> getDefaultConsumes() {
         if (defaultConsumes == UNSET) {

diff --git a/core/src/main/java/io/smallrye/openapi/runtime/scanner/spi/AbstractParameterProcessor.java b/core/src/main/java/io/smallrye/openapi/runtime/scanner/spi/AbstractParameterProcessor.java
@@ -479,6 +479,12 @@ protected ParameterContext getUnannotatedPathParameter(MethodInfo resourceMethod
     private Parameter mapParameter(MethodInfo resourceMethod, ParameterContext context) {
         ParameterImpl param = context.oaiParam != null ? (ParameterImpl) context.oaiParam : new ParameterImpl();
 
+        if (context.frameworkParam == null) {
+            String paramIn = String.valueOf(context.oaiParam.getIn());
+            ScannerSPILogging.log.missingFrameworkParam(CurrentScannerInfo.getCurrentAnnotationScanner().getName(),
+                resourceMethod.declaringClass().toString(), resourceMethod.toString(), context.oaiParam.getName(), paramIn);
+        }
+
         param.setName(context.name);
 
         mapParameterIn(param, context);

diff --git a/core/src/main/java/io/smallrye/openapi/runtime/scanner/spi/AnnotationScanner.java b/core/src/main/java/io/smallrye/openapi/runtime/scanner/spi/AnnotationScanner.java
@@ -969,12 +969,11 @@ default Type getRequestBodyParameterClassType(final AnnotationScannerContext con
 
     default boolean isPathParameter(final AnnotationScannerContext context, String name, final ResourceParameters params) {
         if (context.getConfig().allowNakedPathParameter().orElse(Boolean.FALSE)) {
-            List<Parameter> allParameters = params.getAllParameters();
-            for (Parameter p : allParameters) {
-                if (p.getIn().equals(Parameter.In.PATH) && p.getName().equals(name)) {
-                    return true;
-                }
-            }
+            return params.getAllParameters()
+                .stream()
+                .map(p -> ModelUtil.dereference(context.getOpenApi(), p))
+                .filter(p -> Objects.equals(p.getName(), name))
+                .anyMatch(p -> Objects.equals(p.getIn(), Parameter.In.PATH));
         }
         return false;
     }

diff --git a/core/src/main/java/io/smallrye/openapi/runtime/scanner/spi/ScannerSPILogging.java b/core/src/main/java/io/smallrye/openapi/runtime/scanner/spi/ScannerSPILogging.java
@@ -17,4 +17,8 @@ interface ScannerSPILogging { // NOSONAR (use of constants in an interface)
     @Message(id = 7901, value = "Matrix parameter references missing path segment: %s")
     void missingPathSegment(String segment);
 
+    @LogMessage(level = Logger.Level.WARN)
+    @Message(id = 7902, value = "@Parameter annotation missing %s parameter. Class: %s, Method: %s, Parameter.name: %s, Parameter.in: %s")
+    void missingFrameworkParam(String frameworkName, String className, String method, String paramName, String paramIn);
+
 }
diff --git a/core/src/main/java/io/smallrye/openapi/runtime/util/ModelUtil.java b/core/src/main/java/io/smallrye/openapi/runtime/util/ModelUtil.java
@@ -12,6 +12,7 @@
 import org.eclipse.microprofile.openapi.models.OpenAPI;
 import org.eclipse.microprofile.openapi.models.Operation;
 import org.eclipse.microprofile.openapi.models.Paths;
+import org.eclipse.microprofile.openapi.models.Reference;
 import org.eclipse.microprofile.openapi.models.media.Content;
 import org.eclipse.microprofile.openapi.models.media.MediaType;
 import org.eclipse.microprofile.openapi.models.media.Schema;
@@ -85,6 +86,14 @@ public static Components components(OpenAPI openApi) {
         return openApi.getComponents();
     }
 
+    public static <T extends Reference<T>> T dereference(OpenAPI openApi, T model) {
+        if (model.getRef() == null) {
+            return model;
+        }
+        T component = getComponent(openApi, model.getRef());
+        return component != null ? component : model;
+    }
+
     /**
      * Gets the component type specified by the given `ref` from the OpenAPI model.
      * 

diff --git a/.../src/test/java/io/smallrye/openapi/runtime/scanner/spi/AbstractAnnotationScannerTest.java b/.../src/test/java/io/smallrye/openapi/runtime/scanner/spi/AbstractAnnotationScannerTest.java
@@ -4,16 +4,62 @@
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
+import java.util.Arrays;
 import java.util.Collections;
+import java.util.List;
 import java.util.Set;
 
+import org.eclipse.microprofile.openapi.models.OpenAPI;
+import org.eclipse.microprofile.openapi.models.parameters.Parameter;
+import org.jboss.jandex.AnnotationInstance;
+import org.jboss.jandex.MethodInfo;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
 
 import io.smallrye.openapi.api.OpenApiConfig;
+import io.smallrye.openapi.api.OpenApiConfigImpl;
 import io.smallrye.openapi.api.models.OperationImpl;
+import io.smallrye.openapi.api.models.parameters.ParameterImpl;
+import io.smallrye.openapi.runtime.scanner.AnnotationScannerExtension;
+import io.smallrye.openapi.runtime.scanner.IndexScannerTestBase;
+import io.smallrye.openapi.runtime.scanner.ResourceParameters;
 
 class AbstractAnnotationScannerTest {
+
+    static final class DummyAnnotationScanner extends AbstractAnnotationScanner {
+        @Override
+        public String getName() {
+            return "Dummy";
+        }
+
+        @Override
+        public OpenAPI scan(AnnotationScannerContext annotationScannerContext, OpenAPI oai) {
+            return oai;
+        }
+
+        @Override
+        public boolean isAsyncResponse(MethodInfo method) {
+            return false;
+        }
+
+        @Override
+        public boolean isPostMethod(MethodInfo method) {
+            return false;
+        }
+
+        @Override
+        public boolean isDeleteMethod(MethodInfo method) {
+            return false;
+        }
+
+        @Override
+        public boolean containsScannerAnnotations(List<AnnotationInstance> instances, List<AnnotationScannerExtension> extensions) {
+            return false;
+        }
+    }
+
     /**
      * Test method for {@link AbstractAnnotationScanner#makePath(String)}.
      */
@@ -94,4 +140,26 @@ public Set<String> getScanExcludeProfiles() {
         assertFalse(result);
         assertEquals(0, operation.getExtensions().size());
     }
+
+    @ParameterizedTest
+    @CsvSource({
+        "true,  p1, p1, PATH, true",
+        "true,  p1, p1,     , false",
+        "true,  p1, p2, PATH, false",
+        "false, p1, p2,     , false",
+        "true,  p1,   , PATH, false",
+        "false, p1,   ,     , false",
+        "     , p1, p1, PATH, false"
+    })
+    void testDefaultIsPathParameter(Boolean allowNaked, String searchParamName, String paramName, Parameter.In paramIn, boolean expectedResult) {
+        OpenApiConfigImpl config = (OpenApiConfigImpl) IndexScannerTestBase.emptyConfig();
+        config.setAllowNakedPathParameter(allowNaked);
+
+        ResourceParameters params = new ResourceParameters();
+        params.setOperationParameters(Arrays.asList(new ParameterImpl().name(paramName).in(paramIn)));
+
+        AbstractAnnotationScanner scanner = new DummyAnnotationScanner();
+        AnnotationScannerContext context = new AnnotationScannerContext(null, Thread.currentThread().getContextClassLoader(), config);
+        assertEquals(expectedResult, scanner.isPathParameter(context, searchParamName, params));
+    }
 }