Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SDT-149: SDT: Timeout error when manually writing to dead letter queue #284

Closed
wants to merge 13 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ ext {
jaxwsVersion = "2.3.1"
activeMQVersion = '5.18.2'
testcontainers = '1.19.1'
sdtCommonVersion = '1.1.0'
sdtCommonVersion = '1.1.0-SDT-149.0.1'
limits = [
'instruction': 99,
'branch' : 99,
Expand Down
2 changes: 1 addition & 1 deletion docker/database/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FROM postgres:11
FROM postgres:15

COPY init-db.sh /docker-entrypoint-initdb.d

Expand Down
1 change: 1 addition & 0 deletions infrastructure/aat.tfvars
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
database_backup_retention_days=7
sku = "Standard"
60 changes: 53 additions & 7 deletions infrastructure/database.tf
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,19 @@ module "postgresql" {
azurerm.postgres_network = azurerm.private_endpoint
}

admin_user_object_id = var.jenkins_AAD_objectId
business_area = "cft"
common_tags = local.tags
component = var.component
env = var.env
admin_user_object_id = var.jenkins_AAD_objectId
business_area = "cft"
common_tags = local.tags
component = var.component
env = var.env
pgsql_databases = [
{
name = "civil_sdt"
}
]
pgsql_version = "11"
product = var.product
pgsql_version = "11"
product = var.product
backup_retention_days = "${var.database_backup_retention_days}"
}

# Create secret for database user
Expand All @@ -41,3 +42,48 @@ resource "azurerm_key_vault_secret" "POSTGRES-HOST" {
value = module.postgresql.fqdn
key_vault_id = module.civil_sdt_key_vault.key_vault_id
}


module "postgresql-v15" {
source = "[email protected]:hmcts/terraform-module-postgresql-flexible?ref=master"

providers = {
azurerm.postgres_network = azurerm.private_endpoint
}

admin_user_object_id = var.jenkins_AAD_objectId
business_area = "cft"
common_tags = local.tags
component = var.component
env = var.env
pgsql_databases = [
{
name = "civil_sdt"
}
]
pgsql_version = "15"
product = var.product
name = join("-", [var.product, var.component, "v15"])
backup_retention_days = "${var.database_backup_retention_days}"
}

# Create secret for database user
resource "azurerm_key_vault_secret" "POSTGRES-USER-V15" {
name = "civil-sdt-POSTGRES-USER-V15"
value = module.postgresql-v15.username
key_vault_id = module.civil_sdt_key_vault.key_vault_id
}

# Create secret for database password
resource "azurerm_key_vault_secret" "POSTGRES-PASS-V15" {
name = "civil-sdt-POSTGRES-PASS-V15"
value = module.postgresql-v15.password
key_vault_id = module.civil_sdt_key_vault.key_vault_id
}

# Create secret for database host
resource "azurerm_key_vault_secret" "POSTGRES-HOST-V15" {
name = "civil-sdt-POSTGRES-HOST-V15"
value = module.postgresql-v15.fqdn
key_vault_id = module.civil_sdt_key_vault.key_vault_id
}
1 change: 1 addition & 0 deletions infrastructure/demo.tfvars
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
database_backup_retention_days=7
sku = "Standard"
1 change: 1 addition & 0 deletions infrastructure/ithc.tfvars
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
database_backup_retention_days=7
sku = "Standard"
1 change: 1 addition & 0 deletions infrastructure/perftest.tfvars
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
database_backup_retention_days=7
sku = "Standard"
6 changes: 6 additions & 0 deletions infrastructure/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,9 @@ variable "jenkins_AAD_objectId" {
}

variable "aks_subscription_id" {}

variable "database_backup_retention_days" {
default = 35
description = "Backup retention period in days for the PGSql instance. Valid values are between 7 & 35 days"
}

Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public void setUp() {
sdtMessage.setMessageSentTimestamp(System.currentTimeMillis());
sdtMessage.setEnqueueLoggingId(1);
final IMessageWriter messageWriter = (IMessageWriter) this.applicationContext.getBean("MessageWriter");
messageWriter.queueMessage(sdtMessage, "MCOLS", false);
messageWriter.queueMessage(sdtMessage, "MCOLS");
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,14 +84,14 @@ public void testQueueMessage() throws JMSException, InterruptedException {
final String strMessage1 =
"TestMessage1" + dateFormat.format(new java.util.Date(System.currentTimeMillis()));
message1.setSdtRequestReference(strMessage1);
messageWriter.queueMessage(message1, "TEST1", false);
messageWriter.queueMessage(message1, "TEST1");

// Send the second message.
final ISdtMessage message2 = new SdtMessage();
final String strMessage2 =
"TestMessage2" + dateFormat.format(new java.util.Date(System.currentTimeMillis()));
message2.setSdtRequestReference(strMessage2);
messageWriter.queueMessage(message2, "TEST1", false);
messageWriter.queueMessage(message2, "TEST1");

readMessageFromQueue(Lists.newArrayList(strMessage1, strMessage2));

Expand All @@ -113,7 +113,7 @@ public void testAzureServiceBusDown() throws JMSException {
String requestReference = "Test message" + dateFormat.format(new java.util.Date(System.currentTimeMillis()));
message.setSdtRequestReference(requestReference);

messageWriter.queueMessage(message, "TEST1", false);
messageWriter.queueMessage(message, "TEST1");
Assert.assertTrue("Test completed", true);

readMessageFromQueue(Lists.newArrayList(requestReference));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,14 +83,14 @@ public void testQueueMessage() throws JMSException, InterruptedException {
final String strMessage1 =
"TestMessage1" + dateFormat.format(new java.util.Date(System.currentTimeMillis()));
message1.setSdtRequestReference(strMessage1);
messageWriter.queueMessage(message1, "TEST1", false);
messageWriter.queueMessage(message1, "TEST1");

// Send the second message.
final ISdtMessage message2 = new SdtMessage();
final String strMessage2 =
"TestMessage2" + dateFormat.format(new java.util.Date(System.currentTimeMillis()));
message2.setSdtRequestReference(strMessage2);
messageWriter.queueMessage(message2, "TEST1", false);
messageWriter.queueMessage(message2, "TEST1");

readMessageFromQueue(3, Lists.newArrayList(strMessage1, strMessage2));

Expand All @@ -110,7 +110,7 @@ public void testAzureServiceBusDown() throws JMSException {
final ISdtMessage message = new SdtMessage();
message.setSdtRequestReference("Test message");

messageWriter.queueMessage(message, "TEST1", false);
messageWriter.queueMessage(message, "TEST1");
Assert.assertTrue("Test completed", true);

readMessageFromQueue(1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -350,16 +350,6 @@ private void handleSoapFaultAndWebServiceException(final IIndividualRequest indi

// now persist the request.
this.getIndividualRequestDao().persist(individualRequest);

// Create a new message for DLQ.
final ISdtMessage messageObj = new SdtMessage();
messageObj.setSdtRequestReference(individualRequest.getSdtRequestReference());

final String targetAppCode =
individualRequest.getBulkSubmission().getTargetApplication().getTargetApplicationCode();

// Write to dead letter queue.
this.getMessageWriter().queueMessage(messageObj, targetAppCode, true);
}

/**
Expand Down Expand Up @@ -477,7 +467,7 @@ private void reQueueRequest(final IIndividualRequest individualRequest, Boolean
final String targetAppCode =
individualRequest.getBulkSubmission().getTargetApplication().getTargetApplicationCode();

this.getMessageWriter().queueMessage(messageObj, targetAppCode, false);
this.getMessageWriter().queueMessage(messageObj, targetAppCode);
} else {
LOGGER.error("Maximum forwarding attempts exceeded for request {}",
individualRequest.getSdtRequestReference());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,6 @@ public class MessageWriter implements IMessageWriter {
*/
private static final Logger LOGGER = LoggerFactory.getLogger(MessageWriter.class);

/**
* Default suffix for DLQ name.
*/
private static final String DLQ_SUFFIX = "/$deadletterqueue";

/**
* The last id used for a queued message.
*/
Expand All @@ -89,10 +84,10 @@ public MessageWriter(final JmsTemplate jmsTemplate,
}

@Override
public void queueMessage(final ISdtMessage sdtMessage, final String targetAppCode, final boolean deadLetter) {
public void queueMessage(final ISdtMessage sdtMessage, final String targetAppCode) {

// Check the target application code is valid and return queue name.
final String queueName = getQueueName(targetAppCode, deadLetter);
final String queueName = getQueueName(targetAppCode);

LOGGER.debug("Sending message with SDT request reference [{}] to queue [{}]",
sdtMessage.getSdtRequestReference(),
Expand Down Expand Up @@ -186,12 +181,9 @@ public void setQueueNameMap(final Map<String, String> queueNameMap) {
* that is mapped to the target application code.
*
* @param targetApplicationCode the target application code.
* @param deadLetter flags whether dead letter queue name should be returned.
* @return the queue name matching to the target application code.
*/
private String getQueueName(final String targetApplicationCode, final boolean deadLetter) {
String queueName;

private String getQueueName(final String targetApplicationCode) {
// The target application code should be supplied.
if (targetApplicationCode == null || targetApplicationCode.trim().length() == 0) {
throw new IllegalArgumentException("Target application code must be supplied.");
Expand All @@ -202,13 +194,7 @@ private String getQueueName(final String targetApplicationCode, final boolean de
throw new IllegalArgumentException("Target application code [" + targetApplicationCode +
"] does not have a JMS queue mapped.");
} else {
queueName = this.getQueueNameMap().get(targetApplicationCode);
if (deadLetter) {
// Append DLQ suffix for dead letter.
queueName += DLQ_SUFFIX;

}
return queueName;
return this.getQueueNameMap().get(targetApplicationCode);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ private void queueRequest(IIndividualRequest individualRequest) {

messageObj.setSdtRequestReference(individualRequest.getSdtRequestReference());

getMessageWriter().queueMessage(messageObj, targetAppCode, false);
getMessageWriter().queueMessage(messageObj, targetAppCode);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,7 @@ void processRequestToSubmitTimeOut() {
verify(mockErrorMsgCacheable).getValue(IErrorMessage.class, REQ_NOT_ACK);
verify(mockCacheable).getValue(IGlobalParameter.class, TARGET_APP_TIMEOUT);
verify(mockCacheable).getValue(IGlobalParameter.class, TARGET_APP_RESP_TIMEOUT);
verify(mockMessageWriter).queueMessage(any(ISdtMessage.class),any(String.class), eq(false));
verify(mockMessageWriter).queueMessage(any(ISdtMessage.class),any(String.class));
verify(mockIndividualRequestDao).getRequestBySdtReference(TEST_1);

}
Expand Down Expand Up @@ -518,7 +518,6 @@ void processRequestToSubmitForWebServiceException() {
verify(mockCacheable).getValue(IGlobalParameter.class, TARGET_APP_RESP_TIMEOUT);
verify(mockCacheable).getValue(IGlobalParameter.class, MCOL_INDV_REQ_DELAY);
verify(mockIndividualRequestDao).getRequestBySdtReference(TEST_1);
verify(mockMessageWriter).queueMessage(any(ISdtMessage.class), any(String.class), eq(true));
}

/**
Expand Down Expand Up @@ -571,7 +570,6 @@ void processRequestToSubmitSoapFault() {
verify(mockCacheable).getValue(IGlobalParameter.class, TARGET_APP_RESP_TIMEOUT);
verify(mockCacheable).getValue(IGlobalParameter.class, MCOL_INDV_REQ_DELAY);
verify(mockIndividualRequestDao).getRequestBySdtReference(TEST_1);
verify(mockMessageWriter).queueMessage(any(ISdtMessage.class), any(String.class), eq(true));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,6 @@ class MessageWriterTest extends AbstractSdtUnitTestBase {

private static final String UNIT_TEST = "UNITTEST";

private static final String DLQ_SUFFIX = "/$deadletterqueue";

private static final String NOT_EXPECTED_TO_FAIL = "Not Expected to fail";

private static final String ILLEGAL_STATE_EXCEPTION_MESSAGE =
Expand Down Expand Up @@ -87,7 +85,7 @@ public void setUp() {
@MethodSource("invalidTargetAppCodes")
void testQueueMessageInvalidArgumentException(String targetAppCode, String exceptionMessage) {
try {
messageWriter.queueMessage(sdtMessage, targetAppCode, false);
messageWriter.queueMessage(sdtMessage, targetAppCode);
fail("Should have thrown an Illegal Argument exception as the target app code is invalid");
} catch (final IllegalArgumentException e) {
// Expected exception thrown, continue with test
Expand All @@ -112,29 +110,14 @@ static Stream<Arguments> invalidTargetAppCodes() {
void testQueueMessage() {
// Send the message.
try {
messageWriter.queueMessage(sdtMessage, UNIT_TEST, false);
messageWriter.queueMessage(sdtMessage, UNIT_TEST);
assertNotEquals(0L, sdtMessage.getMessageSentTimestamp());
verify(mockJmsTemplate).convertAndSend(UNIT_TEST_QUEUE, sdtMessage);
} catch (final IllegalArgumentException e) {
fail(NOT_EXPECTED_TO_FAIL);
}
}

/**
* Test for a valid queue message on the MessageWriter for dead letter.
*/
@Test
void testQueueMessageForDeadLetter() {
// Send the message.
try {
messageWriter.queueMessage(sdtMessage, UNIT_TEST, true);
assertNotEquals(0L, sdtMessage.getMessageSentTimestamp());
verify(mockJmsTemplate).convertAndSend(UNIT_TEST_QUEUE + DLQ_SUFFIX, sdtMessage);
} catch (final IllegalArgumentException e) {
fail(NOT_EXPECTED_TO_FAIL);
}
}

@Test
void testQueueMessageUncategorizedJmsException() {
UncategorizedJmsException uncategorizedJmsException = new UncategorizedJmsException("Some JMS exception");
Expand All @@ -147,7 +130,7 @@ void testQueueMessageUncategorizedJmsException() {
listAppender.start();
messageWriterLogger.addAppender(listAppender);

messageWriter.queueMessage(sdtMessage, UNIT_TEST, false);
messageWriter.queueMessage(sdtMessage, UNIT_TEST);

List<ILoggingEvent> logsList = listAppender.list;
ILoggingEvent lastLogEntry = logsList.get(logsList.size() - 1);
Expand Down Expand Up @@ -179,7 +162,7 @@ void testResetConnectionAndQueueMessage() {
CachingConnectionFactory mockCachingConnectionFactory = mock(CachingConnectionFactory.class);
when(mockJmsTemplate.getConnectionFactory()).thenReturn(mockCachingConnectionFactory);

messageWriter.queueMessage(sdtMessage, UNIT_TEST, false);
messageWriter.queueMessage(sdtMessage, UNIT_TEST);

verify(mockJmsTemplate).getConnectionFactory();
verify(mockCachingConnectionFactory).resetConnection();
Expand All @@ -199,7 +182,7 @@ void testResetConnectionAndQueueMessageNoConnectionFactory() {
when(mockJmsTemplate.getConnectionFactory()).thenReturn(null);

try {
messageWriter.queueMessage(sdtMessage, UNIT_TEST, false);
messageWriter.queueMessage(sdtMessage, UNIT_TEST);
fail("IllegalStateException (java.lang) should be thrown");
}
catch (IllegalStateException e) {
Expand Down Expand Up @@ -234,7 +217,7 @@ void testResetConnectionAndQueueMessageUncategorizedJmsException() {
listAppender.start();
messageWriterLogger.addAppender(listAppender);

messageWriter.queueMessage(sdtMessage, UNIT_TEST, false);
messageWriter.queueMessage(sdtMessage, UNIT_TEST);

List<ILoggingEvent> logsList = listAppender.list;
ILoggingEvent lastLogEntry = logsList.get(logsList.size() - 1);
Expand Down Expand Up @@ -273,7 +256,7 @@ void testResetConnectionAndQueueMessageOtherException() {
when(mockJmsTemplate.getConnectionFactory()).thenReturn(mockCachingConnectionFactory);

try {
messageWriter.queueMessage(sdtMessage, UNIT_TEST, false);
messageWriter.queueMessage(sdtMessage, UNIT_TEST);
fail("InvalidClientIDException should be thrown");
} catch (org.springframework.jms.InvalidClientIDException e) {
// Expected exception thrown, continue with test
Expand All @@ -297,7 +280,7 @@ void testOtherIllegalStateExceptionMessage(String exceptionMessage) {
when(mockJmsTemplate).convertAndSend(UNIT_TEST_QUEUE, sdtMessage);

try {
messageWriter.queueMessage(sdtMessage, UNIT_TEST, false);
messageWriter.queueMessage(sdtMessage, UNIT_TEST);
fail("IllegalStateException (org.springframework.jms) should be thrown");
}
catch(org.springframework.jms.IllegalStateException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public void testEnqueueRequestNew() {
Runnable runnable = runnableCaptor.getValue();
runnable.run();

verify(messageWriter).queueMessage(any(SdtMessage.class), eq("targetAppCode"), eq(false));
verify(messageWriter).queueMessage(any(SdtMessage.class), eq("targetAppCode"));
}

@Test
Expand Down