diff --git a/app/src/main/java/gov/hhs/cdc/trustedintermediary/external/javalin/App.java b/app/src/main/java/gov/hhs/cdc/trustedintermediary/external/javalin/App.java index cbd225963..ae2952487 100644 --- a/app/src/main/java/gov/hhs/cdc/trustedintermediary/external/javalin/App.java +++ b/app/src/main/java/gov/hhs/cdc/trustedintermediary/external/javalin/App.java @@ -17,7 +17,6 @@ import gov.hhs.cdc.trustedintermediary.external.slf4j.DeployedLogger; import gov.hhs.cdc.trustedintermediary.external.slf4j.LocalLogger; import gov.hhs.cdc.trustedintermediary.organizations.OrganizationsSettings; -import gov.hhs.cdc.trustedintermediary.utils.SyncRetryTask; import gov.hhs.cdc.trustedintermediary.wrappers.AuthEngine; import gov.hhs.cdc.trustedintermediary.wrappers.Cache; import gov.hhs.cdc.trustedintermediary.wrappers.HapiFhir; @@ -88,6 +87,5 @@ private static void registerClasses() { ApplicationContext.register( OrganizationsSettings.class, OrganizationsSettings.getInstance()); ApplicationContext.register(MetricMetadata.class, LoggingMetricMetadata.getInstance()); - ApplicationContext.register(SyncRetryTask.class, SyncRetryTask.getInstance()); } } diff --git a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/metadata/PartnerMetadataOrchestrator.java b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/metadata/PartnerMetadataOrchestrator.java index 5a930f529..9381cb704 100644 --- a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/metadata/PartnerMetadataOrchestrator.java +++ b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/metadata/PartnerMetadataOrchestrator.java @@ -91,27 +91,13 @@ public void updateMetadataForSentOrder(String receivedSubmissionId, String sentS } PartnerMetadata partnerMetadata = optionalPartnerMetadata.get(); - if (!sentSubmissionId.equals(partnerMetadata.sentSubmissionId())) { - logger.logInfo("Updating metadata with sentSubmissionId: {}", sentSubmissionId); - partnerMetadata = partnerMetadata.withSentSubmissionId(sentSubmissionId); - partnerMetadataStorage.saveMetadata(partnerMetadata); - } - logger.logInfo( - "Looking up receiver name from RS history API for sentSubmissionId: {}", - sentSubmissionId); - String receiver; - try { - String bearerToken = rsclient.getRsToken(); - String responseBody = rsclient.requestHistoryEndpoint(sentSubmissionId, bearerToken); - receiver = getReceiverName(responseBody); - } catch (ReportStreamEndpointClientException | FormatterProcessingException e) { - throw new PartnerMetadataException( - "Unable to retrieve metadata from RS history API", e); + if (sentSubmissionId.equals(partnerMetadata.sentSubmissionId())) { + return; } - logger.logInfo("Updating metadata with receiver: {}", receiver); - partnerMetadata = partnerMetadata.withReceiver(receiver); + logger.logInfo("Updating metadata with sentSubmissionId: {}", sentSubmissionId); + partnerMetadata = partnerMetadata.withSentSubmissionId(sentSubmissionId); partnerMetadataStorage.saveMetadata(partnerMetadata); } @@ -125,10 +111,26 @@ public Optional getMetadata(String receivedSubmissionId) } PartnerMetadata partnerMetadata = optionalPartnerMetadata.get(); - if (partnerMetadata.receiver() == null && partnerMetadata.sentSubmissionId() != null) { - logger.logInfo("Receiver name not found in metadata, looking up from RS history API"); - updateMetadataForSentOrder(receivedSubmissionId, partnerMetadata.sentSubmissionId()); - return partnerMetadataStorage.readMetadata(receivedSubmissionId); + var sentSubmissionId = partnerMetadata.sentSubmissionId(); + if (partnerMetadata.receiver() == null && sentSubmissionId != null) { + logger.logInfo( + "Receiver name not found in metadata, looking up {} from RS history API", + sentSubmissionId); + + String receiver; + try { + String bearerToken = rsclient.getRsToken(); + String responseBody = + rsclient.requestHistoryEndpoint(sentSubmissionId, bearerToken); + receiver = getReceiverName(responseBody); + } catch (ReportStreamEndpointClientException | FormatterProcessingException e) { + throw new PartnerMetadataException( + "Unable to retrieve metadata from RS history API", e); + } + + logger.logInfo("Updating metadata with receiver: {}", receiver); + partnerMetadata = partnerMetadata.withReceiver(receiver); + partnerMetadataStorage.saveMetadata(partnerMetadata); } return Optional.of(partnerMetadata); diff --git a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/orders/SendOrderUseCase.java b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/orders/SendOrderUseCase.java index 6e49fe692..c713efefd 100644 --- a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/orders/SendOrderUseCase.java +++ b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/orders/SendOrderUseCase.java @@ -3,11 +3,8 @@ import gov.hhs.cdc.trustedintermediary.etor.metadata.EtorMetadataStep; import gov.hhs.cdc.trustedintermediary.etor.metadata.PartnerMetadataException; import gov.hhs.cdc.trustedintermediary.etor.metadata.PartnerMetadataOrchestrator; -import gov.hhs.cdc.trustedintermediary.utils.RetryFailedException; -import gov.hhs.cdc.trustedintermediary.utils.SyncRetryTask; import gov.hhs.cdc.trustedintermediary.wrappers.Logger; import gov.hhs.cdc.trustedintermediary.wrappers.MetricMetadata; -import java.util.concurrent.Callable; import javax.inject.Inject; /** The overall logic to receive, convert to OML, and subsequently send a lab order. */ @@ -18,7 +15,6 @@ public class SendOrderUseCase { @Inject MetricMetadata metadata; @Inject PartnerMetadataOrchestrator partnerMetadataOrchestrator; @Inject Logger logger; - @Inject SyncRetryTask retryTask; private SendOrderUseCase() {} @@ -66,15 +62,11 @@ private void saveSentOrderSubmissionId(String receivedSubmissionId, String sentS "Received and/or sent submissionId is null so not saving metadata for sent order"); return; } - Callable task = - () -> { - partnerMetadataOrchestrator.updateMetadataForSentOrder( - receivedSubmissionId, sentSubmissionId); - return null; - }; + try { - retryTask.retry(task, 3, 1000); - } catch (RetryFailedException e) { + partnerMetadataOrchestrator.updateMetadataForSentOrder( + receivedSubmissionId, sentSubmissionId); + } catch (PartnerMetadataException e) { logger.logError( "Unable to update metadata for received submissionId " + receivedSubmissionId diff --git a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/metadata/PartnerMetadataOrchestratorTest.groovy b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/metadata/PartnerMetadataOrchestratorTest.groovy index 1970a7d16..4157b9340 100644 --- a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/metadata/PartnerMetadataOrchestratorTest.groovy +++ b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/metadata/PartnerMetadataOrchestratorTest.groovy @@ -147,22 +147,13 @@ class PartnerMetadataOrchestratorTest extends Specification { given: def receivedSubmissionId = "receivedSubmissionId" def sentSubmissionId = "sentSubmissionId" - def bearerToken = "token" - def receiver = "org.service" - def rsHistoryApiResponse = "{\"destinations\": [{\"organization_id\": \"org\", \"service\": \"service\"}]}" def partnerMetadata = new PartnerMetadata(receivedSubmissionId, "sender", Instant.now(), "hash") - def updatedPartnerMetadata = partnerMetadata.withSentSubmissionId(sentSubmissionId).withReceiver(receiver) - - mockFormatter.convertJsonToObject(rsHistoryApiResponse, _ as TypeReference) >> [destinations: [ - [organization_id: "org", service: "service"] - ]] + def updatedPartnerMetadata = partnerMetadata.withSentSubmissionId(sentSubmissionId) when: PartnerMetadataOrchestrator.getInstance().updateMetadataForSentOrder(receivedSubmissionId, sentSubmissionId) then: - 1 * mockClient.getRsToken() >> bearerToken - 1 * mockClient.requestHistoryEndpoint(sentSubmissionId, bearerToken) >> rsHistoryApiResponse 1 * mockPartnerMetadataStorage.readMetadata(receivedSubmissionId) >> Optional.of(partnerMetadata) 1 * mockPartnerMetadataStorage.saveMetadata(updatedPartnerMetadata) } @@ -179,29 +170,29 @@ class PartnerMetadataOrchestratorTest extends Specification { 0 * mockPartnerMetadataStorage.readMetadata(receivedSubmissionId) } - def "updateMetadataForSentOrder throws PartnerMetadataException on client error"() { + def "getMetadata throws PartnerMetadataException on client error"() { given: def receivedSubmissionId = "receivedSubmissionId" def sentSubmissionId = "sentSubmissionId" - def partnerMetadata = new PartnerMetadata(receivedSubmissionId, sentSubmissionId, "sender", "receiver", Instant.now(), "hash") + def partnerMetadata = new PartnerMetadata(receivedSubmissionId, sentSubmissionId, "sender", null, Instant.now(), "hash") mockPartnerMetadataStorage.readMetadata(receivedSubmissionId) >> Optional.of(partnerMetadata) mockClient.getRsToken() >> "token" mockClient.requestHistoryEndpoint(_ as String, _ as String) >> { throw new ReportStreamEndpointClientException("Client error", new Exception()) } when: - PartnerMetadataOrchestrator.getInstance().updateMetadataForSentOrder(receivedSubmissionId, sentSubmissionId) + PartnerMetadataOrchestrator.getInstance().getMetadata(receivedSubmissionId) then: thrown(PartnerMetadataException) } - def "updateMetadataForSentOrder throws PartnerMetadataException on formatter error"() { + def "getMetadata throws PartnerMetadataException on formatter error"() { given: def receivedSubmissionId = "receivedSubmissionId" def sentSubmissionId = "sentSubmissionId" def rsHistoryApiResponse = "{\"destinations\": [{\"organization_id\": \"org\", \"service\": \"service\"}]}" - def partnerMetadata = new PartnerMetadata(receivedSubmissionId, sentSubmissionId, "sender", "receiver", Instant.now(), "hash") + def partnerMetadata = new PartnerMetadata(receivedSubmissionId, sentSubmissionId, "sender", null, Instant.now(), "hash") mockPartnerMetadataStorage.readMetadata(receivedSubmissionId) >> Optional.of(partnerMetadata) mockClient.getRsToken() >> "token" @@ -209,13 +200,13 @@ class PartnerMetadataOrchestratorTest extends Specification { mockFormatter.convertJsonToObject(rsHistoryApiResponse, _ as TypeReference) >> { throw new FormatterProcessingException("Formatter error", new Exception()) } when: - PartnerMetadataOrchestrator.getInstance().updateMetadataForSentOrder(receivedSubmissionId, sentSubmissionId) + PartnerMetadataOrchestrator.getInstance().getMetadata(receivedSubmissionId) then: thrown(PartnerMetadataException) } - def "getMetadata retrieves metadata successfully"() { + def "getMetadata retrieves metadata successfully with the sender already filled"() { given: def receivedSubmissionId = "receivedSubmissionId" def metadata = new PartnerMetadata(receivedSubmissionId, "sentSubmissionId", "sender", "receiver", Instant.now(), "hash") @@ -227,6 +218,7 @@ class PartnerMetadataOrchestratorTest extends Specification { result.isPresent() result.get() == metadata 1 * mockPartnerMetadataStorage.readMetadata(receivedSubmissionId) >> Optional.of(metadata) + 0 * mockClient.requestHistoryEndpoint(_, _) } def "getMetadata retrieves metadata successfully when receiver is present and sentSubmissionId is missing"() { @@ -267,9 +259,8 @@ class PartnerMetadataOrchestratorTest extends Specification { then: result.isPresent() result.get() == expectedMetadata - 2 * mockPartnerMetadataStorage.readMetadata(receivedSubmissionId) >> Optional.of(missingReceiverMetadata) + 1 * mockPartnerMetadataStorage.readMetadata(receivedSubmissionId) >> Optional.of(missingReceiverMetadata) 1 * mockPartnerMetadataStorage.saveMetadata(expectedMetadata) - 1 * mockPartnerMetadataStorage.readMetadata(receivedSubmissionId) >> Optional.of(expectedMetadata) } def "getReceiverName returns correct receiver name from valid JSON response"() { diff --git a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/orders/SendOrderUsecaseTest.groovy b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/orders/SendOrderUsecaseTest.groovy index b18f0437c..e4fe92d11 100644 --- a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/orders/SendOrderUsecaseTest.groovy +++ b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/orders/SendOrderUsecaseTest.groovy @@ -5,21 +5,16 @@ import gov.hhs.cdc.trustedintermediary.context.TestApplicationContext import gov.hhs.cdc.trustedintermediary.etor.metadata.EtorMetadataStep import gov.hhs.cdc.trustedintermediary.etor.metadata.PartnerMetadataException import gov.hhs.cdc.trustedintermediary.etor.metadata.PartnerMetadataOrchestrator -import gov.hhs.cdc.trustedintermediary.utils.RetryFailedException -import gov.hhs.cdc.trustedintermediary.utils.SyncRetryTask import gov.hhs.cdc.trustedintermediary.wrappers.Logger import gov.hhs.cdc.trustedintermediary.wrappers.MetricMetadata import spock.lang.Specification -import java.util.concurrent.Callable - class SendOrderUsecaseTest extends Specification { def mockOrchestrator = Mock(PartnerMetadataOrchestrator) def mockConverter = Mock(OrderConverter) def mockSender = Mock(OrderSender) def mockLogger = Mock(Logger) - def mockRetryTask = Mock(SyncRetryTask) def setup() { TestApplicationContext.reset() @@ -30,7 +25,6 @@ class SendOrderUsecaseTest extends Specification { TestApplicationContext.register(OrderConverter, mockConverter) TestApplicationContext.register(OrderSender, mockSender) TestApplicationContext.register(Logger, mockLogger) - TestApplicationContext.register(SyncRetryTask, mockRetryTask) } def "send sends successfully"() { @@ -42,8 +36,6 @@ class SendOrderUsecaseTest extends Specification { def mockOrder = new OrderMock(null, null, null) def mockOmlOrder = Mock(Order) - mockRetryTask.retry({ it.call(); true }, _, _) >> { Callable task, int retries, int delay -> task.call(); true } - TestApplicationContext.injectRegisteredImplementations() when: @@ -100,14 +92,14 @@ class SendOrderUsecaseTest extends Specification { 1 * mockConverter.convertMetadataToOmlOrder(order) >> omlOrder 1 * mockConverter.addContactSectionToPatientResource(omlOrder) >> omlOrder 1 * mockSender.sendOrder(omlOrder) >> Optional.of("sentId") - 1 * mockRetryTask.retry(_, _, _) - noExceptionThrown() } - def "convertAndSend logs error and continues when retryTask throws exception"() { + def "convertAndSend logs error and continues when updating the metadata for the sent order throws exception"() { given: def order = Mock(Order) def omlOrder = Mock(Order) + def partnerMetadataException = new PartnerMetadataException("Error") + mockOrchestrator.updateMetadataForSentOrder("receivedId", _) >> { throw partnerMetadataException} TestApplicationContext.injectRegisteredImplementations() when: @@ -117,9 +109,7 @@ class SendOrderUsecaseTest extends Specification { 1 * mockConverter.convertMetadataToOmlOrder(order) >> omlOrder 1 * mockConverter.addContactSectionToPatientResource(omlOrder) >> omlOrder 1 * mockSender.sendOrder(omlOrder) >> Optional.of("sentId") - 1 * mockRetryTask.retry(_, _, _) >> { throw new RetryFailedException("Error", new Exception()) } - 1 * mockLogger.logError(_, _) - noExceptionThrown() + 1 * mockLogger.logError(_, partnerMetadataException) } def "convertAndSend logs event when submissionId is null"() { diff --git a/shared/src/main/java/gov/hhs/cdc/trustedintermediary/utils/RetryFailedException.java b/shared/src/main/java/gov/hhs/cdc/trustedintermediary/utils/RetryFailedException.java deleted file mode 100644 index edebd5f29..000000000 --- a/shared/src/main/java/gov/hhs/cdc/trustedintermediary/utils/RetryFailedException.java +++ /dev/null @@ -1,12 +0,0 @@ -package gov.hhs.cdc.trustedintermediary.utils; - -/** - * Custom exception that wraps the last exception thrown after the maximum number of retries has - * been reached. It could also be thrown in the case of a InterruptedException. - */ -public class RetryFailedException extends Exception { - - public RetryFailedException(String message, Throwable cause) { - super(message, cause); - } -} diff --git a/shared/src/main/java/gov/hhs/cdc/trustedintermediary/utils/SyncRetryTask.java b/shared/src/main/java/gov/hhs/cdc/trustedintermediary/utils/SyncRetryTask.java deleted file mode 100644 index 6cc4c86e2..000000000 --- a/shared/src/main/java/gov/hhs/cdc/trustedintermediary/utils/SyncRetryTask.java +++ /dev/null @@ -1,48 +0,0 @@ -package gov.hhs.cdc.trustedintermediary.utils; - -import gov.hhs.cdc.trustedintermediary.wrappers.Logger; -import java.util.concurrent.Callable; -import javax.inject.Inject; - -/** - * Provides a reusable utility for retrying a task with a specified number of retries and wait time - * between retries. - */ -public class SyncRetryTask { - private static final SyncRetryTask INSTANCE = new SyncRetryTask(); - @Inject Logger logger; - - public static SyncRetryTask getInstance() { - return INSTANCE; - } - - private SyncRetryTask() {} - - public T retry(Callable task, int maxRetries, long waitTime) - throws RetryFailedException { - Exception lastException = null; - int attempt = 0; - - while (attempt < maxRetries) { - try { - return task.call(); - } catch (Exception e) { - lastException = e; - attempt++; - - logger.logWarning("Attempt {}: Retrying in {}s", attempt, waitTime * 2 / 1000); - - try { - Thread.sleep(waitTime); - } catch (InterruptedException ie) { - Thread.currentThread().interrupt(); // Restore interrupted status - throw new RetryFailedException("Thread interrupted during retries", ie); - } - - waitTime *= 2; - } - } - - throw new RetryFailedException("Failed after " + maxRetries + " retries", lastException); - } -} diff --git a/shared/src/test/groovy/gov/hhs/cdc/trustedintermediary/utils/RetryFailedExceptionTest.groovy b/shared/src/test/groovy/gov/hhs/cdc/trustedintermediary/utils/RetryFailedExceptionTest.groovy deleted file mode 100644 index d22ec4b13..000000000 --- a/shared/src/test/groovy/gov/hhs/cdc/trustedintermediary/utils/RetryFailedExceptionTest.groovy +++ /dev/null @@ -1,18 +0,0 @@ -package gov.hhs.cdc.trustedintermediary.utils - -import spock.lang.Specification - -class RetryFailedExceptionTest extends Specification { - def "test constructor"() { - given: - def message = "DogCow" - - when: - def innerException = new IOException() - def exception = new RetryFailedException(message, innerException) - - then: - exception.getMessage() == message - exception.getCause() == innerException - } -} diff --git a/shared/src/test/groovy/gov/hhs/cdc/trustedintermediary/utils/SyncRetryTaskTest.groovy b/shared/src/test/groovy/gov/hhs/cdc/trustedintermediary/utils/SyncRetryTaskTest.groovy deleted file mode 100644 index c540d3e38..000000000 --- a/shared/src/test/groovy/gov/hhs/cdc/trustedintermediary/utils/SyncRetryTaskTest.groovy +++ /dev/null @@ -1,70 +0,0 @@ -package gov.hhs.cdc.trustedintermediary.utils - -import gov.hhs.cdc.trustedintermediary.context.TestApplicationContext -import spock.lang.Specification - -import java.util.concurrent.Callable - -class SyncRetryTaskTest extends Specification { - - def setup() { - TestApplicationContext.reset() - TestApplicationContext.init() - TestApplicationContext.register(SyncRetryTask, SyncRetryTask.getInstance()) - TestApplicationContext.injectRegisteredImplementations() - } - - def "scheduleRetry should retry the task until it succeeds"() { - given: - def maxRetries = 3 - def waitTime = 10 - def mockTask = Mock(Callable) - - when: - def result = SyncRetryTask.getInstance().retry(mockTask, maxRetries, waitTime) - - then: - (1..maxRetries - 1) * mockTask.call() >> { throw new Exception("Fail") } - 1 * mockTask.call() >> null // Succeeds on the last attempt - result == null - } - - def "scheduleRetry should give up after max retries and throw RetryFailedException"() { - given: - def maxRetries = 3 - def waitTime = 10 - def mockTask = Mock(Callable) - - when: - SyncRetryTask.getInstance().retry(mockTask, maxRetries, waitTime) - - then: - maxRetries * mockTask.call() >> { throw new Exception("Fail") } - def exception = thrown(RetryFailedException) - exception.getCause().getClass() == Exception - } - - def "should handle thread interruption"() { - given: - def mockCallable = Mock(Callable) - mockCallable.call() >> { throw new Exception("Fail") } - Exception thrown - def syncRetryTask = SyncRetryTask.getInstance() - - when: - def thread = new Thread({ - try { - syncRetryTask.retry(mockCallable, 3, 1000) - } catch (RetryFailedException e) { - thrown = e - } - }) - thread.start() - Thread.sleep(500) - thread.interrupt() - thread.join() - - then: - thrown.cause instanceof InterruptedException - } -}