Skip to content

Commit 90bc2a9

Browse files
chetanmunegowdachetanmunegowda
authored
Merge pull request #1608 from CMSgov/QPPA-11385-logging
QPPA-11385: downgrade datadog logging
2 parents 7c21bc3 + c1bc9f4 commit 90bc2a9

3 files changed

Lines changed: 60 additions & 59 deletions

File tree

‎pom.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -469,7 +469,7 @@
469469
<dependency>
470470
<groupId>ch.qos.logback</groupId>
471471
<artifactId>logback-classic</artifactId>
472-
<version>1.5.13</version>
472+
<version>1.5.25</version>
473473
</dependency>
474474

475475
<dependency>

rest-api/src/main/java/gov/cms/qpp/conversion/api/controllers/v1/ExceptionHandlerControllerV1.java renamed to rest-api/src/main/java/gov/cms/qpp/conversion/api/exceptions/GlobalExceptionHandler.java

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,10 @@
1-
package gov.cms.qpp.conversion.api.controllers.v1;
1+
package gov.cms.qpp.conversion.api.exceptions;
22

3-
import gov.cms.qpp.conversion.api.exceptions.InvalidFileTypeException;
4-
import gov.cms.qpp.conversion.api.exceptions.InvalidPurposeException;
5-
import gov.cms.qpp.conversion.api.exceptions.NoFileInDatabaseException;
3+
import com.amazonaws.AmazonServiceException;
64
import gov.cms.qpp.conversion.api.services.AuditService;
75
import gov.cms.qpp.conversion.model.error.AllErrors;
86
import gov.cms.qpp.conversion.model.error.QppValidationException;
97
import gov.cms.qpp.conversion.model.error.TransformException;
10-
11-
import com.amazonaws.AmazonServiceException;
12-
138
import org.slf4j.Logger;
149
import org.slf4j.LoggerFactory;
1510
import org.springframework.http.HttpHeaders;
@@ -22,14 +17,12 @@
2217
import org.springframework.web.multipart.MultipartException;
2318
import org.springframework.web.servlet.mvc.method.annotation.ResponseEntityExceptionHandler;
2419

25-
import gov.cms.qpp.conversion.api.exceptions.BadZipException;
26-
2720
/**
2821
* Modify the controller to send back different responses for exceptions
2922
*/
3023
@ControllerAdvice
31-
public class ExceptionHandlerControllerV1 extends ResponseEntityExceptionHandler {
32-
private static final Logger API_LOG = LoggerFactory.getLogger(ExceptionHandlerControllerV1.class);
24+
public class GlobalExceptionHandler extends ResponseEntityExceptionHandler {
25+
private static final Logger API_LOG = LoggerFactory.getLogger(GlobalExceptionHandler.class);
3326

3427
private AuditService auditService;
3528

@@ -38,7 +31,7 @@ public class ExceptionHandlerControllerV1 extends ResponseEntityExceptionHandler
3831
*
3932
* @param auditService {@link AuditService} facilitates persistence of conversion results
4033
*/
41-
public ExceptionHandlerControllerV1(final AuditService auditService) {
34+
public GlobalExceptionHandler(final AuditService auditService) {
4235
this.auditService = auditService;
4336
}
4437

@@ -52,7 +45,9 @@ public ExceptionHandlerControllerV1(final AuditService auditService) {
5245
@ExceptionHandler(TransformException.class)
5346
@ResponseBody
5447
ResponseEntity<AllErrors> handleTransformException(TransformException exception) {
55-
API_LOG.error("Transform exception occurred", exception);
48+
API_LOG.info("Transform failed validation (422): {}", exception.getMessage());
49+
API_LOG.debug("TransformException details", exception);
50+
5651
auditService.failConversion(exception.getConversionReport());
5752
return cope(exception);
5853
}
@@ -67,7 +62,9 @@ ResponseEntity<AllErrors> handleTransformException(TransformException exception)
6762
@ExceptionHandler(QppValidationException.class)
6863
@ResponseBody
6964
ResponseEntity<AllErrors> handleQppValidationException(QppValidationException exception) {
70-
API_LOG.error("Validation exception occurred", exception);
65+
API_LOG.info("Submission validation failed (422): {}", exception.getMessage());
66+
API_LOG.debug("QppValidationException details", exception);
67+
7168
auditService.failValidation(exception.getConversionReport());
7269
return cope(exception);
7370
}

rest-api/src/test/java/gov/cms/qpp/conversion/api/controllers/v1/ExceptionHandlerControllerV1Test.java renamed to rest-api/src/test/java/gov/cms/qpp/conversion/api/exceptions/GlobalExceptionHandlerTest.java

Lines changed: 48 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,26 @@
1-
package gov.cms.qpp.conversion.api.controllers.v1;
1+
package gov.cms.qpp.conversion.api.exceptions;
22

33
import com.amazonaws.AmazonServiceException;
44
import com.google.common.truth.Truth;
5+
import gov.cms.qpp.conversion.ConversionReport;
6+
import gov.cms.qpp.conversion.Converter;
7+
import gov.cms.qpp.conversion.PathSource;
8+
import gov.cms.qpp.conversion.api.controllers.v1.QrdaControllerV1;
9+
import gov.cms.qpp.conversion.api.helper.AdvancedApmHelper;
10+
import gov.cms.qpp.conversion.api.model.Constants;
11+
import gov.cms.qpp.conversion.api.services.AuditService;
12+
import gov.cms.qpp.conversion.api.services.QrdaService;
13+
import gov.cms.qpp.conversion.api.services.ValidationService;
14+
import gov.cms.qpp.conversion.model.error.AllErrors;
15+
import gov.cms.qpp.conversion.model.error.QppValidationException;
16+
import gov.cms.qpp.conversion.model.error.TransformException;
17+
import gov.cms.qpp.test.MockitoExtension;
18+
import gov.cms.qpp.test.logging.LoggerContract;
519
import org.apache.commons.lang3.ArrayUtils;
620
import org.junit.jupiter.api.BeforeAll;
721
import org.junit.jupiter.api.BeforeEach;
822
import org.junit.jupiter.api.Test;
923
import org.junit.jupiter.api.extension.ExtendWith;
10-
import org.mockito.ArgumentMatchers;
1124
import org.mockito.InjectMocks;
1225
import org.mockito.Mock;
1326
import org.mockito.Mockito;
@@ -20,45 +33,30 @@
2033
import org.springframework.test.web.servlet.request.MockMvcRequestBuilders;
2134
import org.springframework.test.web.servlet.setup.MockMvcBuilders;
2235

23-
import gov.cms.qpp.conversion.ConversionReport;
24-
import gov.cms.qpp.conversion.Converter;
25-
import gov.cms.qpp.conversion.PathSource;
26-
import gov.cms.qpp.conversion.api.exceptions.InvalidFileTypeException;
27-
import gov.cms.qpp.conversion.api.exceptions.InvalidPurposeException;
28-
import gov.cms.qpp.conversion.api.exceptions.NoFileInDatabaseException;
29-
import gov.cms.qpp.conversion.api.helper.AdvancedApmHelper;
30-
import gov.cms.qpp.conversion.api.services.AuditService;
31-
import gov.cms.qpp.conversion.model.error.AllErrors;
32-
import gov.cms.qpp.conversion.model.error.QppValidationException;
33-
import gov.cms.qpp.conversion.model.error.TransformException;
34-
import gov.cms.qpp.test.MockitoExtension;
35-
import gov.cms.qpp.test.logging.LoggerContract;
36-
3736
import java.nio.file.Path;
3837
import java.util.UUID;
3938
import java.util.concurrent.CompletableFuture;
4039

4140
import static com.google.common.truth.Truth.assertThat;
4241
import static com.google.common.truth.Truth.assertWithMessage;
4342
import static org.mockito.ArgumentMatchers.any;
44-
import static org.mockito.ArgumentMatchers.anyString;
4543
import static org.mockito.Mockito.when;
4644

47-
4845
@ExtendWith(MockitoExtension.class)
49-
class ExceptionHandlerControllerV1Test implements LoggerContract {
46+
class GlobalExceptionHandlerTest implements LoggerContract {
5047

5148
private static ConversionReport report;
52-
private static AllErrors allErrors = new AllErrors();
49+
private static final AllErrors allErrors = new AllErrors();
5350

5451
@InjectMocks
55-
private ExceptionHandlerControllerV1 objectUnderTest;
52+
private GlobalExceptionHandler objectUnderTest;
5653

5754
@Mock
5855
private AuditService auditService;
5956

6057
@BeforeAll
6158
static void setup() {
59+
// NOTE: If this path is flaky in CI, move the file to src/test/resources and load via classpath.
6260
Path path = Path.of("../qrda-files/valid-QRDA-III-latest.xml");
6361
report = new Converter(new PathSource(path)).getReport();
6462
report.setReportDetails(allErrors);
@@ -68,6 +66,8 @@ static void setup() {
6866
void before() {
6967
when(auditService.failConversion(any(ConversionReport.class)))
7068
.thenReturn(CompletableFuture.completedFuture(null));
69+
when(auditService.failValidation(any(ConversionReport.class)))
70+
.thenReturn(CompletableFuture.completedFuture(null));
7171
}
7272

7373
@Test
@@ -99,13 +99,14 @@ void testTransformExceptionBody() {
9999
new TransformException("test transform exception", new NullPointerException(), report);
100100

101101
ResponseEntity<AllErrors> responseEntity = objectUnderTest.handleTransformException(exception);
102+
102103
assertThat(responseEntity.getBody()).isEqualTo(allErrors);
103104
}
104105

105106
@Test
106107
void testQppValidationExceptionStatusCode() {
107108
QppValidationException exception =
108-
new QppValidationException("test transform exception", new NullPointerException(), report);
109+
new QppValidationException("test validation exception", new NullPointerException(), report);
109110

110111
ResponseEntity<AllErrors> responseEntity = objectUnderTest.handleQppValidationException(exception);
111112

@@ -117,7 +118,7 @@ void testQppValidationExceptionStatusCode() {
117118
@Test
118119
void testQppValidationExceptionHeaderContentType() {
119120
QppValidationException exception =
120-
new QppValidationException("test transform exception", new NullPointerException(), report);
121+
new QppValidationException("test validation exception", new NullPointerException(), report);
121122

122123
ResponseEntity<AllErrors> responseEntity = objectUnderTest.handleQppValidationException(exception);
123124

@@ -128,9 +129,10 @@ void testQppValidationExceptionHeaderContentType() {
128129
@Test
129130
void testQppValidationExceptionBody() {
130131
QppValidationException exception =
131-
new QppValidationException("test transform exception", new NullPointerException(), report);
132+
new QppValidationException("test validation exception", new NullPointerException(), report);
132133

133134
ResponseEntity<AllErrors> responseEntity = objectUnderTest.handleQppValidationException(exception);
135+
134136
assertThat(responseEntity.getBody()).isEqualTo(allErrors);
135137
}
136138

@@ -141,7 +143,7 @@ void testFileNotFoundExceptionStatusCode() {
141143

142144
ResponseEntity<String> responseEntity = objectUnderTest.handleFileNotFoundException(exception);
143145

144-
assertWithMessage("The response entity's status code must be 422.")
146+
assertWithMessage("The response entity's status code must be 404.")
145147
.that(responseEntity.getStatusCode())
146148
.isEqualTo(HttpStatus.NOT_FOUND);
147149
}
@@ -163,6 +165,7 @@ void testFileNotFoundExceptionBody() {
163165
new NoFileInDatabaseException(AdvancedApmHelper.FILE_NOT_FOUND);
164166

165167
ResponseEntity<String> responseEntity = objectUnderTest.handleFileNotFoundException(exception);
168+
166169
assertThat(responseEntity.getBody()).isEqualTo(AdvancedApmHelper.FILE_NOT_FOUND);
167170
}
168171

@@ -173,7 +176,7 @@ void testInvalidFileTypeExceptionStatusCode() {
173176

174177
ResponseEntity<String> responseEntity = objectUnderTest.handleInvalidFileTypeException(exception);
175178

176-
assertWithMessage("The response entity's status code must be 422.")
179+
assertWithMessage("The response entity's status code must be 404.")
177180
.that(responseEntity.getStatusCode())
178181
.isEqualTo(HttpStatus.NOT_FOUND);
179182
}
@@ -195,6 +198,7 @@ void testInvalidFileTypeExceptionBody() {
195198
new InvalidFileTypeException(AdvancedApmHelper.FILE_NOT_FOUND);
196199

197200
ResponseEntity<String> responseEntity = objectUnderTest.handleInvalidFileTypeException(exception);
201+
198202
assertThat(responseEntity.getBody()).isEqualTo(AdvancedApmHelper.FILE_NOT_FOUND);
199203
}
200204

@@ -208,15 +212,6 @@ void testHandleAmazonExceptionStatusCode() {
208212
Truth.assertThat(response.getStatusCodeValue()).isEqualTo(404);
209213
}
210214

211-
// @Test
212-
// void testHandleAmazonExceptionResponseBody() {
213-
// AmazonServiceException exception = new AmazonServiceException("some message");
214-
//
215-
// ResponseEntity<String> response = objectUnderTest.handleAmazonException(exception);
216-
//
217-
// Truth.assertThat(response.getBody()).contains("some message");
218-
// }
219-
220215
@Test
221216
void testHandleInvalidPurposeExceptionExceptionResponseBody() {
222217
InvalidPurposeException exception = new InvalidPurposeException("some message");
@@ -228,23 +223,32 @@ void testHandleInvalidPurposeExceptionExceptionResponseBody() {
228223

229224
@Test
230225
void testHandleInvalidPurposeExceptionResponseBodyDoesInterception() throws Exception {
231-
QrdaControllerV1 mock = Mockito.mock(QrdaControllerV1.class);
232-
MockMvc mvc = MockMvcBuilders.standaloneSetup(mock)
233-
.setControllerAdvice(new ExceptionHandlerControllerV1(auditService))
226+
// Use a real controller instance (with mocked deps) so Spring MVC can route to it reliably.
227+
QrdaService qrdaService = Mockito.mock(QrdaService.class);
228+
ValidationService validationService = Mockito.mock(ValidationService.class);
229+
230+
QrdaControllerV1 controller = new QrdaControllerV1(qrdaService, validationService, auditService);
231+
232+
MockMvc mvc = MockMvcBuilders.standaloneSetup(controller)
233+
.setControllerAdvice(new GlobalExceptionHandler(auditService))
234234
.build();
235-
when(mock.uploadQrdaFile(ArgumentMatchers.any(), ArgumentMatchers.anyString())).thenCallRealMethod();
236235

237236
String purpose = "this is an invalid purpose because it's too long" + UUID.randomUUID();
237+
238238
RequestBuilder builder = MockMvcRequestBuilders.multipart("/")
239-
.file("file", ArrayUtils.EMPTY_BYTE_ARRAY)
240-
.header("Purpose", purpose);
239+
.file("file", ArrayUtils.EMPTY_BYTE_ARRAY)
240+
.header("Purpose", purpose)
241+
.header("Accept", Constants.V1_API_ACCEPT);
242+
241243
MvcResult result = mvc.perform(builder).andReturn();
242-
Truth.assertThat(result.getResponse().getContentAsString()).isEqualTo("Given Purpose (header) is too large. Max length is "
243-
+ "25, yours was " + purpose.length());
244+
245+
Truth.assertThat(result.getResponse().getStatus()).isEqualTo(400);
246+
Truth.assertThat(result.getResponse().getContentAsString())
247+
.isEqualTo("Given Purpose (header) is too large. Max length is 25, yours was " + purpose.length());
244248
}
245249

246250
@Override
247251
public Class<?> getLoggerType() {
248-
return ExceptionHandlerControllerV1.class;
252+
return GlobalExceptionHandler.class;
249253
}
250254
}

0 commit comments

Comments
 (0)