Skip to content

Commit edd9c22

Browse files
pboospboos
authored
Fix: Do not return violations with LogLevel.IGNORE (#279)
1 parent 3adfed7 commit edd9c22

File tree

4 files changed

+169
-31
lines changed

4 files changed

+169
-31
lines changed

‎openapi-validation-core/src/main/java/com/getyourguide/openapi/validation/core/OpenApiRequestValidator.java‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import com.atlassian.oai.validator.model.Request;
44
import com.atlassian.oai.validator.model.SimpleRequest;
55
import com.atlassian.oai.validator.model.SimpleResponse;
6+
import com.getyourguide.openapi.validation.api.log.LogLevel;
67
import com.getyourguide.openapi.validation.api.log.OpenApiViolationHandler;
78
import com.getyourguide.openapi.validation.api.metrics.MetricsReporter;
89
import com.getyourguide.openapi.validation.api.model.Direction;
@@ -98,7 +99,7 @@ public List<OpenApiViolation> validateRequestObject(
9899
var result = validator.validateRequest(simpleRequest);
99100
var violations = mapper.map(result, request, response, Direction.REQUEST, requestBody);
100101
return violations.stream()
101-
.filter(violation -> !violationExclusions.isExcluded(violation))
102+
.filter(this::isNonExcludedViolation)
102103
.toList();
103104
} catch (Exception e) {
104105
log.error("[OpenAPI Validation] Could not validate request", e);
@@ -145,11 +146,15 @@ public List<OpenApiViolation> validateResponseObject(
145146
);
146147
var violations = mapper.map(result, request, response, Direction.RESPONSE, responseBody);
147148
return violations.stream()
148-
.filter(violation -> !violationExclusions.isExcluded(violation))
149+
.filter(this::isNonExcludedViolation)
149150
.toList();
150151
} catch (Exception e) {
151152
log.error("[OpenAPI Validation] Could not validate response", e);
152153
return List.of();
153154
}
154155
}
156+
157+
private boolean isNonExcludedViolation(OpenApiViolation violation) {
158+
return !LogLevel.IGNORE.equals(violation.getLevel()) && !violationExclusions.isExcluded(violation);
159+
}
155160
}

‎openapi-validation-core/src/test/java/com/getyourguide/openapi/validation/core/OpenApiRequestValidatorTest.java‎

Lines changed: 158 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,10 @@
88

99
import com.atlassian.oai.validator.model.SimpleRequest;
1010
import com.atlassian.oai.validator.report.ValidationReport;
11+
import com.getyourguide.openapi.validation.api.log.LogLevel;
1112
import com.getyourguide.openapi.validation.api.model.OpenApiViolation;
1213
import com.getyourguide.openapi.validation.api.model.RequestMetaData;
14+
import com.getyourguide.openapi.validation.api.model.ResponseMetaData;
1315
import com.getyourguide.openapi.validation.core.exclusions.InternalViolationExclusions;
1416
import com.getyourguide.openapi.validation.core.mapper.ValidationReportToOpenApiViolationsMapper;
1517
import com.getyourguide.openapi.validation.core.validator.OpenApiInteractionValidatorWrapper;
@@ -19,6 +21,8 @@
1921
import java.util.concurrent.Executor;
2022
import java.util.concurrent.RejectedExecutionException;
2123
import org.junit.jupiter.api.BeforeEach;
24+
import org.junit.jupiter.api.DisplayName;
25+
import org.junit.jupiter.api.Nested;
2226
import org.junit.jupiter.api.Test;
2327
import org.mockito.ArgumentCaptor;
2428
import org.mockito.Mockito;
@@ -51,49 +55,178 @@ public void setup() {
5155
}
5256

5357
@Test
58+
@DisplayName("When thread pool executor rejects execution then it should not throw")
5459
public void testWhenThreadPoolExecutorRejectsExecutionThenItShouldNotThrow() {
5560
Mockito.doThrow(new RejectedExecutionException()).when(executor).execute(any());
5661

5762
openApiRequestValidator.validateRequestObjectAsync(mock(), null, null, mock());
5863
}
5964

60-
@Test
61-
public void testWhenEncodedQueryParamIsPassedThenValidationShouldHappenWithQueryParamDecoded() {
62-
var uri = URI.create("https://api.example.com?ids=1%2C2%2C3&text=e%3Dmc2%20%26%20more&spaces=this+is+a+sparta");
63-
var request = new RequestMetaData("GET", uri, new HashMap<>());
65+
@Nested
66+
@DisplayName("validateRequestObject")
67+
public class ValidateRequestObjectTests {
68+
69+
@Test
70+
@DisplayName("When encoded query param is passed then validation should happen with query param decoded")
71+
public void testWhenEncodedQueryParamIsPassedThenValidationShouldHappenWithQueryParamDecoded() {
72+
var uri = URI.create("https://api.example.com?ids=1%2C2%2C3&text=e%3Dmc2%20%26%20more&spaces=this+is+a+sparta");
73+
var request = new RequestMetaData("GET", uri, new HashMap<>());
74+
75+
openApiRequestValidator.validateRequestObject(request, null);
76+
77+
var simpleRequestArgumentCaptor = ArgumentCaptor.forClass(SimpleRequest.class);
78+
verify(validator).validateRequest(simpleRequestArgumentCaptor.capture());
79+
verifyQueryParamValueEquals(simpleRequestArgumentCaptor, "ids", "1,2,3");
80+
verifyQueryParamValueEquals(simpleRequestArgumentCaptor, "text", "e=mc2 & more");
81+
verifyQueryParamValueEquals(simpleRequestArgumentCaptor, "spaces", "this is a sparta");
82+
}
83+
84+
@Test
85+
@DisplayName("When violation is excluded then it should not be returned")
86+
public void testWhenViolationIsExcludedThenItShouldNotBeReturned() {
87+
var validationReport = mock(ValidationReport.class);
88+
when(validator.validateRequest(any())).thenReturn(validationReport);
89+
var violationExcluded = mock(OpenApiViolation.class);
90+
var violations = List.of(violationExcluded, mock(OpenApiViolation.class));
91+
when(mapper.map(any(), any(), any(), any(), any())).thenReturn(violations);
92+
when(internalViolationExclusions.isExcluded(violationExcluded)).thenReturn(true);
93+
94+
var result = openApiRequestValidator.validateRequestObject(createRequest(), null);
95+
96+
assertEquals(1, result.size());
97+
assertEquals(violations.get(1), result.getFirst());
98+
}
99+
100+
@Test
101+
@DisplayName("When violation has log level IGNORE then it should not be returned")
102+
public void testWhenRequestViolationHasLogLevelIgnoreThenItShouldNotBeReturned() {
103+
var violationIgnored = createViolation(LogLevel.IGNORE);
104+
var violationError = createViolation(LogLevel.ERROR);
105+
mockRequestValidation(List.of(violationIgnored, violationError));
106+
107+
var result = openApiRequestValidator.validateRequestObject(createRequest(), null);
108+
109+
assertSingleViolationReturned(result, violationError);
110+
}
111+
112+
@Test
113+
@DisplayName("When violation has log level IGNORE and another is excluded then both should not be returned")
114+
public void testWhenRequestViolationHasLogLevelIgnoreAndIsExcludedThenItShouldNotBeReturned() {
115+
var violationIgnored = createViolation(LogLevel.IGNORE);
116+
var violationExcluded = createViolation(LogLevel.WARN);
117+
when(internalViolationExclusions.isExcluded(violationExcluded)).thenReturn(true);
118+
var violationError = createViolation(LogLevel.ERROR);
119+
mockRequestValidation(List.of(violationIgnored, violationExcluded, violationError));
120+
121+
var result = openApiRequestValidator.validateRequestObject(createRequest(), null);
122+
123+
assertSingleViolationReturned(result, violationError);
124+
}
125+
126+
@Test
127+
@DisplayName("When all violations are ignored then empty list is returned")
128+
public void testWhenAllRequestViolationsAreIgnoredThenEmptyListIsReturned() {
129+
var violation1 = createViolation(LogLevel.IGNORE);
130+
var violation2 = createViolation(LogLevel.IGNORE);
131+
mockRequestValidation(List.of(violation1, violation2));
132+
133+
var result = openApiRequestValidator.validateRequestObject(createRequest(), null);
134+
135+
assertNoViolationsReturned(result);
136+
}
137+
}
138+
139+
@Nested
140+
@DisplayName("validateResponseObject")
141+
public class ValidateResponseObjectTests {
142+
143+
@Test
144+
@DisplayName("When violation has log level IGNORE then it should not be returned")
145+
public void testWhenResponseViolationHasLogLevelIgnoreThenItShouldNotBeReturned() {
146+
var violationIgnored = createViolation(LogLevel.IGNORE);
147+
var violationWarn = createViolation(LogLevel.WARN);
148+
mockResponseValidation(List.of(violationIgnored, violationWarn));
149+
150+
var result = executeValidateResponseObject();
151+
152+
assertSingleViolationReturned(result, violationWarn);
153+
}
154+
155+
@Test
156+
@DisplayName("When violation has log level IGNORE and another is excluded then both should not be returned")
157+
public void testWhenResponseViolationHasLogLevelIgnoreAndIsExcludedThenItShouldNotBeReturned() {
158+
var violationIgnored = createViolation(LogLevel.IGNORE);
159+
var violationExcluded = createViolation(LogLevel.INFO);
160+
when(internalViolationExclusions.isExcluded(violationExcluded)).thenReturn(true);
161+
var violationError = createViolation(LogLevel.ERROR);
162+
mockResponseValidation(List.of(violationIgnored, violationExcluded, violationError));
163+
164+
var result = executeValidateResponseObject();
165+
166+
assertSingleViolationReturned(result, violationError);
167+
}
168+
169+
@Test
170+
@DisplayName("When all violations are ignored then empty list is returned")
171+
public void testWhenAllResponseViolationsAreIgnoredThenEmptyListIsReturned() {
172+
var violation1 = createViolation(LogLevel.IGNORE);
173+
var violation2 = createViolation(LogLevel.IGNORE);
174+
mockResponseValidation(List.of(violation1, violation2));
175+
176+
var result = executeValidateResponseObject();
177+
178+
assertNoViolationsReturned(result);
179+
}
180+
181+
private List<OpenApiViolation> executeValidateResponseObject() {
182+
var request = createRequest();
183+
var response = createResponse();
184+
return openApiRequestValidator.validateResponseObject(request, response, null);
185+
}
186+
}
64187

65-
openApiRequestValidator.validateRequestObject(request, null);
188+
private void verifyQueryParamValueEquals(
189+
ArgumentCaptor<SimpleRequest> simpleRequestArgumentCaptor,
190+
String name,
191+
String expected
192+
) {
193+
var ids = simpleRequestArgumentCaptor.getValue().getQueryParameterValues(name).iterator().next();
194+
assertEquals(expected, ids);
195+
}
66196

67-
var simpleRequestArgumentCaptor = ArgumentCaptor.forClass(SimpleRequest.class);
68-
verify(validator).validateRequest(simpleRequestArgumentCaptor.capture());
69-
verifyQueryParamValueEquals(simpleRequestArgumentCaptor, "ids", "1,2,3");
70-
verifyQueryParamValueEquals(simpleRequestArgumentCaptor, "text", "e=mc2 & more");
71-
verifyQueryParamValueEquals(simpleRequestArgumentCaptor, "spaces", "this is a sparta");
197+
private OpenApiViolation createViolation(LogLevel level) {
198+
var violation = mock(OpenApiViolation.class);
199+
when(violation.getLevel()).thenReturn(level);
200+
return violation;
72201
}
73202

74-
@Test
75-
public void testWhenViolationIsExcludedThenItShouldNotBeReturned() {
203+
private RequestMetaData createRequest() {
76204
var uri = URI.create("https://api.example.com/path");
77-
var request = new RequestMetaData("GET", uri, new HashMap<>());
205+
return new RequestMetaData("GET", uri, new HashMap<>());
206+
}
207+
208+
private ResponseMetaData createResponse() {
209+
return new ResponseMetaData(200, "application/json", new HashMap<>());
210+
}
211+
212+
private void mockRequestValidation(List<OpenApiViolation> violations) {
78213
var validationReport = mock(ValidationReport.class);
79214
when(validator.validateRequest(any())).thenReturn(validationReport);
80-
var violationExcluded = mock(OpenApiViolation.class);
81-
var violations = List.of(violationExcluded, mock(OpenApiViolation.class));
82215
when(mapper.map(any(), any(), any(), any(), any())).thenReturn(violations);
83-
when(internalViolationExclusions.isExcluded(violationExcluded)).thenReturn(true);
216+
}
84217

85-
var result = openApiRequestValidator.validateRequestObject(request, null);
218+
private void mockResponseValidation(List<OpenApiViolation> violations) {
219+
var validationReport = mock(ValidationReport.class);
220+
when(validator.validateResponse(any(), any(), any())).thenReturn(validationReport);
221+
when(mapper.map(any(), any(), any(), any(), any())).thenReturn(violations);
222+
}
86223

224+
private void assertSingleViolationReturned(List<OpenApiViolation> result, OpenApiViolation expected) {
87225
assertEquals(1, result.size());
88-
assertEquals(violations.get(1), result.getFirst());
226+
assertEquals(expected, result.getFirst());
89227
}
90228

91-
private void verifyQueryParamValueEquals(
92-
ArgumentCaptor<SimpleRequest> simpleRequestArgumentCaptor,
93-
String name,
94-
String expected
95-
) {
96-
var ids = simpleRequestArgumentCaptor.getValue().getQueryParameterValues(name).iterator().next();
97-
assertEquals(expected, ids);
229+
private void assertNoViolationsReturned(List<OpenApiViolation> result) {
230+
assertEquals(0, result.size());
98231
}
99232
}

‎spring-boot-starter/spring-boot-starter-web/src/test/java/com/getyourguide/openapi/validation/integration/FailOnViolationIntegrationTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@
2020
import org.springframework.beans.factory.annotation.Autowired;
2121
import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc;
2222
import org.springframework.boot.test.context.SpringBootTest;
23-
import org.springframework.boot.test.mock.mockito.SpyBean;
2423
import org.springframework.http.MediaType;
24+
import org.springframework.test.context.bean.override.mockito.MockitoSpyBean;
2525
import org.springframework.test.context.junit.jupiter.SpringExtension;
2626
import org.springframework.test.web.servlet.MockMvc;
2727

@@ -39,7 +39,7 @@ public class FailOnViolationIntegrationTest {
3939
@Autowired
4040
private TestViolationLogger openApiViolationLogger;
4141

42-
@SpyBean
42+
@MockitoSpyBean
4343
private DefaultRestController defaultRestController;
4444

4545
@BeforeEach

‎spring-boot-starter/spring-boot-starter-webflux/src/test/java/com/getyourguide/openapi/validation/integration/FailOnViolationIntegrationTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@
1414
import org.springframework.beans.factory.annotation.Autowired;
1515
import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc;
1616
import org.springframework.boot.test.context.SpringBootTest;
17-
import org.springframework.boot.test.mock.mockito.SpyBean;
1817
import org.springframework.http.MediaType;
18+
import org.springframework.test.context.bean.override.mockito.MockitoSpyBean;
1919
import org.springframework.test.context.junit.jupiter.SpringExtension;
2020
import org.springframework.test.web.reactive.server.WebTestClient;
2121

@@ -33,7 +33,7 @@ public class FailOnViolationIntegrationTest {
3333
@Autowired
3434
private TestViolationLogger openApiViolationLogger;
3535

36-
@SpyBean
36+
@MockitoSpyBean
3737
private DefaultRestController defaultRestController;
3838

3939
@BeforeEach

0 commit comments

Comments
 (0)