Skip to content

Commit 000c259

Browse files
authored
Logs formatting improvement (#1276)
* logs formatting improvement * checkstyle * fixing IT tests * test update scenario * adressing pitest
1 parent ba8a5f7 commit 000c259

File tree

8 files changed

+84
-38
lines changed

8 files changed

+84
-38
lines changed

service/src/intTest/java/uk/nhs/adaptors/gp2gp/ehr/EhrContinueTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ public void When_EhrContinueThrowsException_Expect_EhrExtractStatusNotUpdated()
8282
Exception exception = assertThrows(UnrecognisedInteractionIdException.class,
8383
() -> ehrExtractRequestHandler.handleContinue(conversationId, CONTINUE_ACKNOWLEDGEMENT));
8484

85-
assertThat(exception.getMessage()).isEqualTo("Received an unrecognized Continue message with conversation_id: "
85+
assertThat(exception.getMessage()).isEqualTo("Received an unrecognized Continue message with conversationId: "
8686
+ conversationId);
8787
verify(taskDispatcher, never()).createTask(any());
8888
}

service/src/intTest/java/uk/nhs/adaptors/gp2gp/ehr/EhrExtractStatusServiceIT.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ void When_EhrStatusWithExceededTimeout_Expect_EhrStatusShouldNotBeUpdated() {
9292
ehrExtractStatusServiceSpy.updateEhrExtractStatusAck(inProgressConversationId, ehrReceivedAcknowledgement);
9393

9494
verify(logger, times(1))
95-
.warn("Received an ACK message with conversation_id: {}, "
95+
.warn("Received an ACK message with conversationId: {}, "
9696
+ "but it is being ignored because the EhrExtract has already been marked as failed "
9797
+ "from not receiving an acknowledgement from the requester in time.",
9898
inProgressConversationId);

service/src/main/java/uk/nhs/adaptors/gp2gp/ehr/EhrExtractStatusService.java

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ public void saveEhrExtractMessageId(String conversationId, String messageId) {
161161
ehrExtractStatusRepository.save(ehrExtractStatus);
162162
},
163163
() -> {
164-
throw new EhrExtractException("Unable to find EHR Extract status with conversation id " + conversationId);
164+
throw new EhrExtractException("Unable to find EHR Extract status with conversationId " + conversationId);
165165
});
166166
}
167167

@@ -314,8 +314,8 @@ public Optional<EhrExtractStatus> updateEhrExtractStatusContinue(String conversa
314314
EhrExtractStatus.class);
315315

316316
if (ehrExtractStatus == null) {
317-
throw new EhrExtractException("Received a Continue message with a conversation_id '" + conversationId
318-
+ "' that is not recognised");
317+
throw new EhrExtractException(format("Received a Continue message with a conversationId %s that is not recognised",
318+
conversationId));
319319
}
320320

321321
LOGGER.info("Database successfully updated with EHRContinue");
@@ -342,34 +342,34 @@ public void updateEhrExtractStatusWithEhrReceivedAckError(String conversationId,
342342

343343
if (ehrExtractStatus == null) {
344344
throw new EhrExtractException(format(
345-
"Couldn't update EHR received acknowledgement with error information because EHR status doesn't exist, conversation_id: %s",
345+
"Couldn't update EHR received acknowledgement with error information because EHR status doesn't exist, conversationId: %s",
346346
conversationId));
347347
}
348348

349349
logger().info("EHR status (EHR received acknowledgement) record successfully "
350-
+ "updated in the database with error information conversation_id: {}", conversationId);
350+
+ "updated in the database with error information conversationId: {}", conversationId);
351351

352352
}
353353

354354
public void updateEhrExtractStatusAck(String conversationId, EhrReceivedAcknowledgement ack) {
355355

356356
if (ack.getErrors() == null && !isEhrStatusWaitingForFinalAck(conversationId)) {
357-
logger().warn("Received unexpected acknowledgement of EHR Extract with conversation id=" + conversationId);
357+
logger().warn("Received unexpected acknowledgement of EHR Extract with conversationId: {}", conversationId);
358358
return;
359359
}
360360

361361
if (hasAcknowledgementExceededAckTimeoutLimit(conversationId, ack.getReceived())
362362
&& hasEhrStatusReceivedAckWithErrors(conversationId)) {
363363

364-
logger().warn("Received an ACK message with conversation_id: {}, "
364+
logger().warn("Received an ACK message with conversationId: {}, "
365365
+ "but it is being ignored because the EhrExtract has already been marked as failed "
366366
+ "from not receiving an acknowledgement from the requester in time.",
367367
conversationId);
368368
return;
369369
}
370370

371371
if (hasFinalAckBeenReceived(conversationId)) {
372-
logger().warn("Received an ACK message with a conversation_id: {} that is a duplicate", conversationId);
372+
logger().warn("Received an ACK message with a conversationId: {} that is a duplicate", conversationId);
373373
return;
374374
}
375375

@@ -396,11 +396,11 @@ && hasEhrStatusReceivedAckWithErrors(conversationId)) {
396396
EhrExtractStatus.class);
397397

398398
if (ehrExtractStatus == null) {
399-
throw new EhrExtractException("Received an ACK message with a conversation_id '" + conversationId
400-
+ "' that is not recognised");
399+
throw new EhrExtractException(format("Received an ACK message with a conversationId %s that is not recognised",
400+
conversationId));
401401
}
402402

403-
logger().info("Database successfully updated with EHRAcknowledgement, conversation_id: {}", conversationId);
403+
logger().info("Database successfully updated with EHRAcknowledgement, conversationId: {}", conversationId);
404404
}
405405

406406
public void saveAckForConversation(String conversationId, EhrReceivedAcknowledgement ack) {
@@ -502,10 +502,10 @@ public EhrExtractStatus updateEhrExtractStatusError(
502502

503503
if (ehrExtractStatus == null) {
504504
throw new EhrExtractException(format(
505-
"Couldn't update EHR status with error information because it doesn't exist conversation_id: %s", conversationId));
505+
"Couldn't update EHR status with error information because it doesn't exist conversationId: %s", conversationId));
506506
}
507507

508-
logger().info("EHR status record successfully updated in the database with error information conversation_id: {}", conversationId);
508+
logger().info("EHR status record successfully updated in the database with error information conversationId: {}", conversationId);
509509

510510
return ehrExtractStatus;
511511
}
@@ -631,8 +631,7 @@ private boolean checkForContinueOutOfOrderAndDuplicate(String conversationId) {
631631
}
632632

633633
if (ehrExtractStatus.getEhrContinue() != null) {
634-
LOGGER.warn("Received a Continue message with a conversation_id '" + conversationId
635-
+ "' that is duplicate");
634+
LOGGER.warn("Received a Continue message with a conversationId {} that is duplicate", conversationId);
636635
return true;
637636
}
638637

service/src/main/java/uk/nhs/adaptors/gp2gp/ehr/scheduling/EhrExtractTimeoutScheduler.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ public void processEhrExtractAckTimeouts() {
4545

4646
} catch (EhrExtractException exception) {
4747

48-
logger().error("An error occurred when updating EHR Extract with Ack erorrs, EHR Extract Status conversation_id: {}",
48+
logger().error("An error occurred when updating EHR Extract with Ack erorrs, EHR Extract Status conversationId: {}",
4949
ehrExtractStatus.getConversationId(), exception);
5050
throw exception;
5151
} catch (Exception exception) {

service/src/main/java/uk/nhs/adaptors/gp2gp/mhs/exception/UnrecognisedInteractionIdException.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
public class UnrecognisedInteractionIdException extends RuntimeException {
44

5-
private static final String EXCEPTION_MESSAGE = "Received an unrecognized %s message with conversation_id: %s";
5+
private static final String EXCEPTION_MESSAGE = "Received an unrecognized %s message with conversationId: %s";
66

77
public UnrecognisedInteractionIdException(String messageType, String conversationId) {
88
super(EXCEPTION_MESSAGE.formatted(messageType, conversationId));

service/src/test/java/uk/nhs/adaptors/gp2gp/ehr/EhrExtractStatusServiceTest.java

Lines changed: 61 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import org.springframework.data.mongodb.core.query.Update;
1616
import org.springframework.data.mongodb.core.query.UpdateDefinition;
1717
import uk.nhs.adaptors.gp2gp.common.service.TimestampService;
18+
import uk.nhs.adaptors.gp2gp.ehr.exception.EhrExtractException;
1819
import uk.nhs.adaptors.gp2gp.ehr.model.EhrExtractStatus;
1920
import uk.nhs.adaptors.gp2gp.mhs.exception.UnrecognisedInteractionIdException;
2021

@@ -26,6 +27,7 @@
2627
import java.util.Optional;
2728
import java.util.UUID;
2829

30+
import static java.lang.String.format;
2931
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
3032
import static org.junit.jupiter.api.Assertions.assertEquals;
3133
import static org.junit.jupiter.api.Assertions.assertFalse;
@@ -112,7 +114,7 @@ void shouldLogWarningWhenLateAcknowledgementReceivedAfter8DaysAndEhrReceivedAckH
112114
ehrExtractStatusServiceSpy.updateEhrExtractStatusAck(conversationId, ack);
113115

114116
verify(logger, times(1))
115-
.warn("Received an ACK message with conversation_id: {}, "
117+
.warn("Received an ACK message with conversationId: {}, "
116118
+ "but it is being ignored because the EhrExtract has already been marked as failed "
117119
+ "from not receiving an acknowledgement from the requester in time.",
118120
conversationId);
@@ -144,9 +146,9 @@ void shouldNotLogWarningThatAckIsIgnoredWhenAcknowledgementReceivedAfter8Days()
144146
ehrExtractStatusServiceSpy.updateEhrExtractStatusAck(conversationId, ack);
145147

146148
verify(logger, never())
147-
.warn("Received an ACK message with a conversation_id: {}, but it will be ignored", conversationId);
149+
.warn("Received an ACK message with a conversationId: {}, but it will be ignored", conversationId);
148150
verify(logger, times(1))
149-
.warn("Received an ACK message with a conversation_id: {} that is a duplicate", conversationId);
151+
.warn("Received an ACK message with a conversationId: {} that is a duplicate", conversationId);
150152
}
151153

152154
@Test
@@ -166,7 +168,7 @@ void shouldUpdateStatusWithErrorAndSpecificErrorCodeAndMessage() {
166168
ERROR_MESSAGE);
167169

168170
verify(logger).info("EHR status (EHR received acknowledgement) record successfully "
169-
+ "updated in the database with error information conversation_id: {}", inProgressConversationId);
171+
+ "updated in the database with error information conversationId: {}", inProgressConversationId);
170172
verify(mongoTemplate, times(1)).findAndModify(queryCaptor.capture(),
171173
updateCaptor.capture(),
172174
any(FindAndModifyOptions.class),
@@ -219,9 +221,32 @@ void shouldNotLogWarningThatAckIsIgnoredWhenAcknowledgementReceivedAfterWithinAn
219221
ehrExtractStatusServiceSpy.updateEhrExtractStatusAck(conversationId, ack);
220222

221223
verify(logger, never())
222-
.warn("Received an ACK message with a conversation_id: {}, but it will be ignored", conversationId);
224+
.warn("Received an ACK message with a conversationId: {}, but it will be ignored", conversationId);
223225
verify(logger, times(1))
224-
.warn("Received an ACK message with a conversation_id: {} that is a duplicate", conversationId);
226+
.warn("Received an ACK message with a conversationId: {} that is a duplicate", conversationId);
227+
}
228+
229+
@Test
230+
void receiveExceptionWhenEhrExtractStatusIsNull() {
231+
232+
EhrExtractStatusService ehrExtractStatusServiceSpy = spy(ehrExtractStatusService);
233+
String conversationId = generateRandomUppercaseUUID();
234+
Instant currentInstant = Instant.now();
235+
Optional<EhrExtractStatus> ehrExtractStatus = Optional.of(EhrExtractStatus.builder().ehrExtractCorePending(null).build());
236+
237+
doReturn(true).when(ehrExtractStatusServiceSpy).isEhrStatusWaitingForFinalAck(conversationId);
238+
doReturn(false).when(ehrExtractStatusServiceSpy).hasFinalAckBeenReceived(conversationId);
239+
doReturn(ehrExtractStatus).when(ehrExtractStatusRepository).findByConversationId(conversationId);
240+
when(ack.getErrors()).thenReturn(null);
241+
when(ack.getReceived()).thenReturn(currentInstant);
242+
doReturn(null)
243+
.when(mongoTemplate).findAndModify(any(Query.class), any(Update.class), any(FindAndModifyOptions.class), any(Class.class));
244+
245+
Exception ehrExtractException = assertThrows(EhrExtractException.class,
246+
() -> ehrExtractStatusServiceSpy.updateEhrExtractStatusAck(conversationId, ack));
247+
248+
assertTrue(ehrExtractException.getMessage()
249+
.contains(format("Received an ACK message with a conversationId %s that is not recognised", conversationId)));
225250
}
226251

227252
@Test
@@ -243,9 +268,9 @@ void updateEhrExtractStatusWhenEhrExtractCorePendingIsNull() {
243268

244269
ehrExtractStatusServiceSpy.updateEhrExtractStatusAck(conversationId, ack);
245270

246-
verify(logger, never()).warn("Received an ACK message with a conversation_id={} exceeded 8 days", conversationId);
271+
verify(logger, never()).warn("Received an ACK message with a conversationId={} exceeded 8 days", conversationId);
247272
verify(logger, times(1))
248-
.info("Database successfully updated with EHRAcknowledgement, conversation_id: {}", conversationId);
273+
.info("Database successfully updated with EHRAcknowledgement, conversationId: {}", conversationId);
249274
}
250275

251276
@Test
@@ -262,7 +287,7 @@ void ehrExtractStatusThrowsExceptionWhenConversationIdNotFound() {
262287
Exception exception = assertThrows(UnrecognisedInteractionIdException.class, () ->
263288
ehrExtractStatusServiceSpy.updateEhrExtractStatusAck(conversationId, ack));
264289

265-
assertEquals("Received an unrecognized ACK message with conversation_id: " + conversationId,
290+
assertEquals("Received an unrecognized ACK message with conversationId: " + conversationId,
266291
exception.getMessage());
267292

268293
verify(ehrExtractStatusServiceSpy, never()).updateEhrExtractStatusError(conversationId,
@@ -307,6 +332,31 @@ void doesNotUpdateEhrExtractStatusWhenEhrContinueIsPresent() {
307332
assertFalse(ehrExtractStatusResult.isPresent());
308333
}
309334

335+
@Test
336+
void throwsEhrExtractExceptionWhenEhrExtractStatusIsNotFound() {
337+
EhrExtractStatusService ehrExtractStatusServiceSpy = spy(ehrExtractStatusService);
338+
String conversationId = generateRandomUppercaseUUID();
339+
340+
Optional<EhrExtractStatus> ehrExtractStatus = Optional.of(EhrExtractStatus.builder()
341+
.ehrExtractCorePending(
342+
EhrExtractStatus
343+
.EhrExtractCorePending
344+
.builder()
345+
.sentAt(Instant.now())
346+
.taskId("22222").build())
347+
.build());
348+
doReturn(ehrExtractStatus).when(ehrExtractStatusRepository).findByConversationId(conversationId);
349+
350+
doReturn(null).when(mongoTemplate).findAndModify(any(Query.class), any(UpdateDefinition.class),
351+
any(FindAndModifyOptions.class), any());
352+
353+
Exception exception = assertThrows(EhrExtractException.class,
354+
() -> ehrExtractStatusServiceSpy.updateEhrExtractStatusContinue(conversationId));
355+
356+
assertTrue(exception.getMessage()
357+
.contains(format("Received a Continue message with a conversationId %s that is not recognised", conversationId)));
358+
}
359+
310360
@Test
311361
void expectTrueWhenEhrExtractStatusWithEhrReceivedAckWithErrorsAndExceededTimeoutLimit() {
312362

@@ -404,7 +454,7 @@ void shouldUpdateStatusWithErrorWhenEhrExtractAckTimeoutOccurs() {
404454
ERROR_MESSAGE);
405455

406456
verify(logger).info("EHR status (EHR received acknowledgement) record successfully "
407-
+ "updated in the database with error information conversation_id: {}", inProgressConversationId);
457+
+ "updated in the database with error information conversationId: {}", inProgressConversationId);
408458
}
409459

410460

@@ -521,7 +571,7 @@ void shouldLogWarningWithMsgIgnoredWhenLateAcknowledgementReceivedAfter8DaysAndE
521571
ehrExtractStatusServiceSpy.updateEhrExtractStatusAck(conversationId, ack);
522572

523573
verify(logger, times(1))
524-
.warn("Received an ACK message with conversation_id: {}, "
574+
.warn("Received an ACK message with conversationId: {}, "
525575
+ "but it is being ignored because the EhrExtract has already been marked as failed "
526576
+ "from not receiving an acknowledgement from the requester in time.",
527577
conversationId);

service/src/test/java/uk/nhs/adaptors/gp2gp/ehr/mapper/AgentPersonMapperTest.java

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@
22

33
import static org.mockito.Mockito.when;
44
import static org.assertj.core.api.Assertions.assertThat;
5-
6-
import java.io.IOException;
75
import java.util.stream.Stream;
86

97
import org.hl7.fhir.dstu3.model.Bundle;
@@ -57,8 +55,7 @@ public void setUp() {
5755

5856
@ParameterizedTest
5957
@MethodSource("readPractitionerRoleTests")
60-
public void When_MappingPractitionerToAgent_Expect_RepresentedAgentXml(String practitionerRoleJson, String outputXml)
61-
throws IOException {
58+
public void When_MappingPractitionerToAgent_Expect_RepresentedAgentXml(String practitionerRoleJson, String outputXml) {
6259

6360
var jsonInputPractitionerRole = ResourceTestFileUtils.getFileContent(practitionerRoleJson);
6461
var expectedOutput = ResourceTestFileUtils.getFileContent(outputXml);

service/src/test/java/uk/nhs/adaptors/gp2gp/ehr/scheduling/EhrExtractTimeoutSchedulerTest.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -297,9 +297,9 @@ void updateEhrExtractStatusListWithEhrReceivedAcknowledgementError() {
297297

298298
() -> assertEquals(
299299
"Couldn't update EHR received acknowledgement with error information because EHR status doesn't exist, "
300-
+ "conversation_id: " + inProgressConversationId, exception.getMessage()),
300+
+ "conversationId: " + inProgressConversationId, exception.getMessage()),
301301
() -> verify(logger)
302-
.error(eq("An error occurred when updating EHR Extract with Ack erorrs, EHR Extract Status conversation_id: {}"),
302+
.error(eq("An error occurred when updating EHR Extract with Ack erorrs, EHR Extract Status conversationId: {}"),
303303
eq(inProgressConversationId),
304304
any(EhrExtractException.class))
305305
);
@@ -342,8 +342,8 @@ void whenEhrExtractStatusIsNullInterceptExceptionAndLogErrorMsg() {
342342
var exception = assertThrows(EhrExtractException.class, ehrExtractTimeoutSchedulerSpy::processEhrExtractAckTimeouts);
343343

344344
assertEquals("Couldn't update EHR received acknowledgement with error information because EHR status doesn't exist, "
345-
+ "conversation_id: " + inProgressConversationId, exception.getMessage());
346-
verify(logger).error(eq("An error occurred when updating EHR Extract with Ack erorrs, EHR Extract Status conversation_id: {}"),
345+
+ "conversationId: " + inProgressConversationId, exception.getMessage());
346+
verify(logger).error(eq("An error occurred when updating EHR Extract with Ack erorrs, EHR Extract Status conversationId: {}"),
347347
eq(inProgressConversationId),
348348
any(EhrExtractException.class));
349349
}

0 commit comments

Comments
 (0)