Skip to content

Commit f907876

Browse files
ORybak5Jonopono123
andauthored
NIAD-3418: introducing retriable mechanism (#1329)
* introducing retriable mechanism * fixing a failing test * fixing flakiness of tests * fixing flakiness of tests * update changelog --------- Co-authored-by: Jon Hardy <Jonathan.Hardy97@gmail.com>
1 parent 067847f commit f907876

6 files changed

Lines changed: 90 additions & 8 deletions

File tree

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@ All notable changes to this project will be documented in this file.
33

44
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
55

6+
## [Unreleased]
7+
8+
* Add Spring retry to MHS queue consumer to facilitate a more reasonable retry delay.
69

710
## [3.3.2] - 2026-08-19
811

gp2gp-translator/build.gradle

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ dependencies {
4040
implementation 'org.jdbi:jdbi3-core:3.49.5'
4141
implementation 'org.jetbrains:annotations:26.0.2'
4242
implementation 'org.springframework.cloud:spring-cloud-starter-gateway-server-webflux'
43+
implementation 'org.springframework.retry:spring-retry:2.0.3'
44+
implementation 'org.springframework.boot:spring-boot-starter-aop'
4345

4446
testImplementation 'org.springframework.boot:spring-boot-starter-test'
4547
testImplementation 'org.junit.jupiter:junit-jupiter:6.0.0'
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
package uk.nhs.adaptors.pss.translator.amqp;
2+
3+
import static org.junit.jupiter.api.Assertions.assertThrows;
4+
import static org.mockito.Mockito.doThrow;
5+
import static org.mockito.Mockito.mock;
6+
import static org.mockito.Mockito.times;
7+
import static org.mockito.Mockito.verify;
8+
import static org.mockito.Mockito.verifyNoInteractions;
9+
import static org.mockito.Mockito.when;
10+
11+
import java.util.UUID;
12+
13+
import jakarta.jms.Message;
14+
import jakarta.jms.Session;
15+
16+
import org.junit.jupiter.api.Test;
17+
import org.springframework.beans.factory.annotation.Autowired;
18+
import org.springframework.boot.test.context.SpringBootTest;
19+
import org.springframework.test.context.TestPropertySource;
20+
import org.springframework.test.context.bean.override.mockito.MockitoBean;
21+
import org.springframework.test.context.bean.override.mockito.MockitoSpyBean;
22+
23+
import uk.nhs.adaptors.common.service.MDCService;
24+
import uk.nhs.adaptors.pss.translator.Gp2gpTranslatorApplication;
25+
import uk.nhs.adaptors.pss.translator.task.MhsQueueMessageHandler;
26+
27+
@SpringBootTest(classes = Gp2gpTranslatorApplication.class)
28+
@TestPropertySource(properties = "amqp.daisyChaining=false")
29+
public class MhsQueueConsumerIT {
30+
31+
private static final String DELIVERY_COUNT_PROPERTY = "JMSXDeliveryCount";
32+
public static final int RETRY_ATTEMPTS = 3;
33+
34+
@Autowired
35+
private MhsQueueConsumer mhsQueueConsumer;
36+
37+
@MockitoSpyBean
38+
private MhsQueueMessageHandler mhsQueueMessageHandler;
39+
40+
@MockitoBean
41+
private MhsDlqPublisher mhsDlqPublisher;
42+
43+
@MockitoBean
44+
private MDCService mdcService;
45+
46+
@Test
47+
public void When_ReceiveThrowsRuntimeException_Expect_ReceiveMethodRetriedThreeTimes() throws Exception {
48+
Message message = mock(Message.class);
49+
Session session = mock(Session.class);
50+
51+
when(message.getJMSMessageID()).thenReturn(UUID.randomUUID().toString());
52+
when(message.getIntProperty(DELIVERY_COUNT_PROPERTY)).thenReturn(1);
53+
doThrow(new RuntimeException("Test failure"))
54+
.when(mhsQueueMessageHandler).handleMessage(message);
55+
56+
assertThrows(RuntimeException.class, () -> mhsQueueConsumer.receive(message, session));
57+
58+
verify(mhsQueueMessageHandler, times(RETRY_ATTEMPTS)).handleMessage(message);
59+
verifyNoInteractions(mhsDlqPublisher);
60+
verify(mdcService, times(RETRY_ATTEMPTS)).resetAllMdcKeys();
61+
}
62+
63+
}

gp2gp-translator/src/integrationTest/java/uk/nhs/adaptors/pss/translator/amqp/ServiceFailureIT.java

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import static org.junit.jupiter.api.Assertions.assertNotNull;
99
import static org.mockito.ArgumentMatchers.any;
1010
import static org.mockito.Mockito.doThrow;
11+
import static org.mockito.Mockito.clearInvocations;
1112
import static org.mockito.Mockito.timeout;
1213
import static org.mockito.Mockito.times;
1314
import static org.mockito.Mockito.verify;
@@ -41,6 +42,7 @@
4142
import org.springframework.http.HttpHeaders;
4243
import org.springframework.jms.core.JmsTemplate;
4344
import org.springframework.test.annotation.DirtiesContext;
45+
import org.springframework.test.context.TestPropertySource;
4446
import org.springframework.test.context.bean.override.mockito.MockitoSpyBean;
4547
import org.springframework.test.context.junit.jupiter.SpringExtension;
4648
import org.springframework.web.reactive.function.client.WebClientRequestException;
@@ -52,6 +54,7 @@
5254
import uk.nhs.adaptors.common.enums.MigrationStatus;
5355
import uk.nhs.adaptors.common.model.TransferRequestMessage;
5456
import uk.nhs.adaptors.pss.translator.Gp2gpTranslatorApplication;
57+
import uk.nhs.adaptors.pss.translator.config.MhsQueueProperties;
5558
import uk.nhs.adaptors.pss.translator.config.PssQueueProperties;
5659
import uk.nhs.adaptors.pss.translator.exception.MhsServerErrorException;
5760
import uk.nhs.adaptors.pss.translator.service.MhsClientService;
@@ -67,26 +70,29 @@
6770
@ExtendWith({SpringExtension.class, MockitoExtension.class})
6871
@DirtiesContext(classMode = DirtiesContext.ClassMode.BEFORE_EACH_TEST_METHOD)
6972
@AutoConfigureMockMvc
73+
@TestPropertySource(properties = {
74+
"amqp.pss.queueName=pssQueueServiceFailureIT",
75+
"amqp.mhs.queueName=mhsQueueServiceFailureIT"
76+
})
7077
public class ServiceFailureIT extends BaseEhrHandler {
7178

72-
public static final int TEN_SECONDS = 10000;
7379
private static final String LOSING_ASID = "LOSING_ASID";
7480
private static final String WINNING_ASID = "WINNING_ASID";
7581
private static final String STUB_BODY = "test Body";
7682
private static final int THIRTY_SECONDS = 30000;
7783
private static final long TWO_MINUTES_LONG = 2L;
78-
private static final int FIVE_WANTED_NUMBER_OF_INVOCATIONS = 5;
7984
public static final String JSON_LARGE_MESSAGE_SCENARIO_3_UK_06_JSON = "/json/LargeMessage/Scenario_3/uk06.json";
8085
public static final String JSON_LARGE_MESSAGE_SCENARIO_3_COPC_JSON = "/json/LargeMessage/Scenario_3/copc.json";
8186
public static final String JSON_LARGE_MESSAGE_EXPECTED_BUNDLE_SCENARIO_3_JSON = "/json/LargeMessage/expectedBundleScenario3.json";
8287
public static final int RECEIVE_TIMEOUT_LIMIT = 50;
83-
public static final int TWENTY = 20;
8488
private String conversationId;
8589

8690
@Autowired
8791
private ObjectMapper objectMapper;
8892
@Autowired
8993
private PssQueueProperties pssQueueProperties;
94+
@Autowired
95+
private MhsQueueProperties mhsQueueProperties;
9096

9197
@Autowired
9298
@Qualifier("jmsTemplateMhsDLQ")
@@ -134,7 +140,8 @@ public void When_SendingInitialRequest_WithMhsOutboundServerError_Expect_Migrati
134140

135141
sendRequestToPssQueue(conversationId, patientNhsNumber);
136142

137-
await().until(() -> hasMigrationStatus(EHR_EXTRACT_REQUEST_ERROR, conversationId));
143+
await().atMost(Duration.ofMinutes(TWO_MINUTES_LONG))
144+
.until(() -> hasMigrationStatus(EHR_EXTRACT_REQUEST_ERROR, conversationId));
138145

139146
verify(mhsClientService, timeout(THIRTY_SECONDS).times(pssQueueProperties.getMaxRedeliveries() + 1)
140147
).send(any());
@@ -146,15 +153,16 @@ public void When_SendingInitialRequest_WithMhsOutboundServerError_Expect_Migrati
146153
@Test
147154
public void When_ReceivingEhrExtract_WithMhsOutboundServerError_Expect_MigrationHasProcessingError() {
148155
doThrow(MhsServerErrorException.class)
149-
.doNothing()
150156
.when(sendContinueRequestHandler)
151157
.prepareAndSendRequest(any());
152158

153159
sendInboundMessageToQueue(JSON_LARGE_MESSAGE_SCENARIO_3_UK_06_JSON);
154160

155-
await().until(() -> hasMigrationStatus(EHR_GENERAL_PROCESSING_ERROR, getConversationId()));
161+
await().atMost(Duration.ofMinutes(TWO_MINUTES_LONG))
162+
.until(() -> hasMigrationStatus(EHR_GENERAL_PROCESSING_ERROR, getConversationId()));
156163

157-
verify(sendContinueRequestHandler, times(1)).prepareAndSendRequest(any());
164+
verify(sendContinueRequestHandler, timeout(THIRTY_SECONDS).atLeast(MhsQueueConsumer.RETRY_ATTEMPTS))
165+
.prepareAndSendRequest(any());
158166

159167
assertThat(getCurrentMigrationStatus(getConversationId())).isEqualTo(EHR_GENERAL_PROCESSING_ERROR);
160168
}
@@ -189,6 +197,7 @@ public void When_ReceivingCOPC_WithMhsServerErrorException_Expect_MessageSentToD
189197
sendInboundMessageToQueue(JSON_LARGE_MESSAGE_SCENARIO_3_UK_06_JSON);
190198

191199
await().until(this::hasContinueMessageBeenReceived);
200+
clearInvocations(mhsClientService);
192201

193202
doThrow(MhsServerErrorException.class).when(mhsClientService).send(any());
194203

@@ -200,7 +209,7 @@ public void When_ReceivingCOPC_WithMhsServerErrorException_Expect_MessageSentToD
200209

201210
assertNotNull(messageSentToDlq);
202211
assertEquals(copcMessageInJsonFormat, ((JmsTextMessage) messageSentToDlq).getText());
203-
verify(mhsClientService, times(FIVE_WANTED_NUMBER_OF_INVOCATIONS)).send(any());
212+
verify(mhsClientService, times((mhsQueueProperties.getMaxRedeliveries() + 1) * MhsQueueConsumer.RETRY_ATTEMPTS)).send(any());
204213
}
205214

206215
@Test

gp2gp-translator/src/main/java/uk/nhs/adaptors/pss/translator/Gp2gpTranslatorApplication.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22

33
import org.springframework.boot.SpringApplication;
44
import org.springframework.boot.autoconfigure.SpringBootApplication;
5+
import org.springframework.retry.annotation.EnableRetry;
56

7+
@EnableRetry
68
@SpringBootApplication
79
public class Gp2gpTranslatorApplication {
810

gp2gp-translator/src/main/java/uk/nhs/adaptors/pss/translator/amqp/MhsQueueConsumer.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import org.springframework.beans.factory.annotation.Autowired;
77
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
88
import org.springframework.jms.annotation.JmsListener;
9+
import org.springframework.retry.annotation.Retryable;
910
import org.springframework.stereotype.Component;
1011

1112
import lombok.RequiredArgsConstructor;
@@ -25,9 +26,11 @@ public class MhsQueueConsumer {
2526
private final MhsDlqPublisher mhsDlqPublisher;
2627
private final MhsQueueProperties mhsQueueProperties;
2728
private final MDCService mdcService;
29+
public static final int RETRY_ATTEMPTS = 3;
2830

2931
@JmsListener(destination = "${amqp.mhs.queueName}", containerFactory = "mhsQueueJmsListenerFactory")
3032
@SneakyThrows
33+
@Retryable(maxAttempts = RETRY_ATTEMPTS)
3134
public void receive(Message message, Session session) {
3235
String messageId = message.getJMSMessageID();
3336
int deliveryCount = message.getIntProperty("JMSXDeliveryCount");

0 commit comments

Comments
 (0)