Show parameter name in violation log (#19)

Author: pboos

openapi-validation-api/src/main/java/com/getyourguide/openapi/validation/api/model/OpenApiViolation.java

@@ -16,6 +16,7 @@ public class OpenApiViolation {
     private final Optional<String> operationId;
     private final Optional<String> normalizedPath;
     private final Optional<String> instance;
+    private final Optional<String> parameter;
     private final Optional<String> schema;
     private final Optional<Integer> responseStatus;
     private final String message;

openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/DefaultViolationLogger.java

@@ -37,6 +37,7 @@ private Map<String, String> buildLoggingContext(OpenApiViolation violation) {
         violation.getNormalizedPath().ifPresent(normalizedPath -> context.put("validation.api.path", normalizedPath));
         violation.getOperationId().ifPresent(operationId -> context.put("validation.api.operation_id", operationId));
         violation.getInstance().ifPresent(instance -> context.put("validation.instance", instance));
+        violation.getParameter().ifPresent(instance -> context.put("validation.parameter", instance));
         return context;
     }
 }

openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/ValidationReportHandler.java

@@ -9,6 +9,7 @@
 import com.getyourguide.openapi.validation.api.model.OpenApiViolation;
 import com.getyourguide.openapi.validation.api.model.RequestMetaData;
 import com.getyourguide.openapi.validation.core.throttle.ValidationReportThrottler;
+import io.swagger.v3.oas.models.parameters.Parameter;
 import java.util.Optional;
 import lombok.AllArgsConstructor;
 
@@ -48,13 +49,18 @@ private OpenApiViolation buildOpenApiViolation(
     ) {
         var requestUri = request.getUri().toString();
         var requestString = String.format("%s %s", request.getMethod(), requestUri);
-        var instance = getPointersInstance(message).map(i -> String.format("Instance: %s\n", i)).orElse("");
+        var pointersInstance = getPointersInstance(message);
+        var instance = pointersInstance.map(i -> String.format("Instance: %s\n", i)).orElse("");
+        var parameterName = getParameterName(message);
+        var parameter = parameterName.map(i -> String.format("Parameter: %s\n", i)).orElse("");
+
         var logMessage = String.format(
-            "OpenAPI spec validation error [%s]\n%s\nUser Agent: %s\n%s\n%s",
+            "OpenAPI spec validation error [%s]\n%s\nUser Agent: %s\n%s%s\n%s",
             message.getKey(),
             requestString,
             request.getHeaders().get("User-Agent"),
             instance,
+            parameter,
             message
         );
 
@@ -66,7 +72,8 @@ private OpenApiViolation buildOpenApiViolation(
             .rule(message.getKey())
             .operationId(getOperationId(message))
             .normalizedPath(getNormalizedPath(message))
-            .instance(getPointersInstance(message))
+            .instance(pointersInstance)
+            .parameter(parameterName)
             .schema(getPointersSchema(message))
             .responseStatus(getResponseStatus(message))
             .logMessage(logMessage)
@@ -94,6 +101,12 @@ private static Optional<String> getPointersSchema(ValidationReport.Message messa
             .map(ValidationReport.MessageContext.Pointers::getSchema);
     }
 
+    private static Optional<String> getParameterName(ValidationReport.Message message) {
+        return message.getContext()
+            .flatMap(ValidationReport.MessageContext::getParameter)
+            .map(Parameter::getName);
+    }
+
     private static Optional<String> getOperationId(ValidationReport.Message message) {
         return message.getContext()
             .flatMap(ValidationReport.MessageContext::getApiOperation)

openapi-validation-core/src/test/java/com/getyourguide/openapi/validation/core/ValidationReportHandlerTest.java

@@ -0,0 +1,92 @@
+package com.getyourguide.openapi.validation.core;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import com.atlassian.oai.validator.report.ValidationReport;
+import com.getyourguide.openapi.validation.api.exclusions.ViolationExclusions;
+import com.getyourguide.openapi.validation.api.log.ViolationLogger;
+import com.getyourguide.openapi.validation.api.metrics.MetricsReporter;
+import com.getyourguide.openapi.validation.api.model.Direction;
+import com.getyourguide.openapi.validation.api.model.OpenApiViolation;
+import com.getyourguide.openapi.validation.api.model.RequestMetaData;
+import com.getyourguide.openapi.validation.core.throttle.ValidationReportThrottler;
+import io.swagger.v3.oas.models.parameters.Parameter;
+import java.net.URI;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Optional;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.mockito.ArgumentCaptor;
+
+class ValidationReportHandlerTest {
+    private ValidationReportThrottler throttleHelper;
+    private ViolationLogger logger;
+
+    private ValidationReportHandler validationReportHandler;
+
+    @BeforeEach
+    public void setUp() {
+        throttleHelper = mock();
+        logger = mock();
+        MetricsReporter metrics = mock();
+        ViolationExclusions violationExclusions = mock();
+
+        validationReportHandler = new ValidationReportHandler(throttleHelper, logger, metrics, violationExclusions);
+    }
+
+    @Test
+    public void testWhenParameterNameIsPresentThenItShouldAddItToTheMessage() {
+        mockNoThrottling();
+        var request = mockRequestMetaData();
+        var validationReport = mockValidationReport("parameterName");
+
+        validationReportHandler.handleValidationReport(request, Direction.REQUEST, null, validationReport);
+
+        var argumentCaptor = ArgumentCaptor.forClass(OpenApiViolation.class);
+        verify(logger).log(argumentCaptor.capture());
+        var openApiViolation = argumentCaptor.getValue();
+        assertEquals(Optional.of("parameterName"), openApiViolation.getParameter());
+        assertEquals(
+            String.join("\n",
+                "OpenAPI spec validation error [key]",
+                "GET https://api.example.com/index",
+                "User Agent: null",
+                "Parameter: parameterName",
+                "",
+                "Violation message (toString)"),
+            openApiViolation.getLogMessage());
+    }
+
+    private static RequestMetaData mockRequestMetaData() {
+        var request = new RequestMetaData("GET", URI.create("https://api.example.com/index"), new HashMap<>());
+        return request;
+    }
+
+    private static ValidationReport mockValidationReport(String parameterName) {
+        var validationReport = mock(ValidationReport.class);
+        var message = mock(ValidationReport.Message.class);
+        when(message.getKey()).thenReturn("key");
+        when(message.getMessage()).thenReturn("Violation message");
+        when(message.toString()).thenReturn("Violation message (toString)");
+        var context = mock(ValidationReport.MessageContext.class);
+        var parameter = mock(Parameter.class);
+        when(parameter.getName()).thenReturn(parameterName);
+        when(context.getParameter()).thenReturn(Optional.of(parameter));
+        when(message.getContext()).thenReturn(Optional.of(context));
+        when(validationReport.getMessages()).thenReturn(List.of(message));
+        return validationReport;
+    }
+
+    private void mockNoThrottling() {
+        doAnswer(invocation -> {
+            ((Runnable) invocation.getArguments()[1]).run();
+            return null;
+        }).when(throttleHelper).throttle(any(), any());
+    }
+}