From 326fda0bc7e831028c2626ce91cea97f6d11d1e2 Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Fri, 1 Nov 2024 14:59:48 -0700 Subject: [PATCH 01/23] Bubble up exceptions, add context, and let test fail --- .../external/hapi/HapiHL7FileMatcher.java | 20 +++++++++---------- .../hapi/HapiHL7FileMatcherTest.groovy | 15 +++++++------- 2 files changed, 17 insertions(+), 18 deletions(-) diff --git a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcher.java b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcher.java index b9151a1e4..7d0014fde 100644 --- a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcher.java +++ b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcher.java @@ -35,7 +35,8 @@ public static HapiHL7FileMatcher getInstance() { } public Map matchFiles( - List outputFiles, List inputFiles) { + List outputFiles, List inputFiles) + throws HL7Exception, IOException { // We pair up output and input files based on the control ID, which is in MSH-10 // Any files (either input or output) that don't have a match are logged Map inputMap = mapMessageByControlId(inputFiles); @@ -67,7 +68,8 @@ public Map matchFiles( return messageMap; } - public Map mapMessageByControlId(List files) { + public Map mapMessageByControlId(List files) + throws HL7Exception, IOException { Map messageMap = new HashMap<>(); @@ -75,25 +77,23 @@ public Map mapMessageByControlId(List files) { Parser parser = context.getPipeParser(); for (HL7FileStream hl7FileStream : files) { + String fileName = hl7FileStream.fileName(); try (InputStream inputStream = hl7FileStream.inputStream()) { String content = new String(inputStream.readAllBytes(), StandardCharsets.UTF_8); Message message = parser.parse(content); MSH mshSegment = (MSH) message.get("MSH"); String msh10 = mshSegment.getMessageControlID().getValue(); if (msh10 == null || msh10.isEmpty()) { - logger.logError("MSH-10 is empty for : " + hl7FileStream.fileName()); - continue; + throw new IllegalArgumentException( + String.format("MSH-10 is empty for file: %s", fileName)); } messageMap.put(msh10, message); - } catch (IOException | HL7Exception e) { - logger.logError( - "An error occurred while parsing the message: " - + hl7FileStream.fileName(), + } catch (HL7Exception e) { + throw new HL7Exception( + String.format("Failed to parse HL7 message from file: %s", fileName), e); } } - } catch (IOException e) { - logger.logError("An error occurred while constructing the DefaultHapiContext", e); } return messageMap; diff --git a/rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcherTest.groovy b/rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcherTest.groovy index 889038a18..0e38cf13f 100644 --- a/rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcherTest.groovy +++ b/rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcherTest.groovy @@ -1,6 +1,7 @@ package gov.hhs.cdc.trustedintermediary.rse2e.external.hapi import ca.uhn.hl7v2.model.Message +import ca.uhn.hl7v2.HL7Exception import gov.hhs.cdc.trustedintermediary.context.TestApplicationContext import gov.hhs.cdc.trustedintermediary.rse2e.HL7FileStream import gov.hhs.cdc.trustedintermediary.wrappers.Logger @@ -70,7 +71,7 @@ class HapiHL7FileMatcherTest extends Specification { file2MshSegment == result[file2Msh10].encode().trim() } - def "should log an error and continue when MSH-10 is empty"() { + def "should throw IllegalArgumentException when MSH-10 is empty"() { given: def msh1to9 = "MSH|^~\\&|Sender Application^sender.test.com^DNS|Sender Facility^0.0.0.0.0.0.0.0^ISO|Receiver Application^0.0.0.0.0.0.0.0^ISO|Receiver Facility^simulated-lab-id^DNS|20230101010000-0000||ORM^O01^ORM_O01|" def msh11to12 = "|T|2.5.1" @@ -80,23 +81,21 @@ class HapiHL7FileMatcherTest extends Specification { def hl7FileStream = new HL7FileStream("file1", inputStream) when: - def result = fileMatcher.mapMessageByControlId([hl7FileStream]) + fileMatcher.mapMessageByControlId([hl7FileStream]) then: - result.size() == 0 - 1 * mockLogger.logError({ it.contains("MSH-10 is empty") }) + thrown(IllegalArgumentException) } - def "should log an error when not able to parse the file as HL7 message"() { + def "should throw HL7Exception when not able to parse the file as HL7 message"() { given: def inputStream = new ByteArrayInputStream("".bytes) def hl7FileStream = new HL7FileStream("badFile", inputStream) when: - def result = fileMatcher.mapMessageByControlId([hl7FileStream]) + fileMatcher.mapMessageByControlId([hl7FileStream]) then: - result.size() == 0 - 1 * mockLogger.logError({ it.contains("An error occurred while parsing the message") }, _ as Exception) + thrown(HL7Exception) } } From 353c71f017d38bfd904c2732f2e5aeab75d19642 Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Fri, 1 Nov 2024 15:40:57 -0700 Subject: [PATCH 02/23] Throw exception when no matching files found and simplify logic --- .../external/hapi/HapiHL7FileMatcher.java | 34 ++++++------------- 1 file changed, 11 insertions(+), 23 deletions(-) diff --git a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcher.java b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcher.java index 7d0014fde..0b7c76cec 100644 --- a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcher.java +++ b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcher.java @@ -6,8 +6,8 @@ import ca.uhn.hl7v2.model.Message; import ca.uhn.hl7v2.model.v251.segment.MSH; import ca.uhn.hl7v2.parser.Parser; +import com.google.common.collect.Sets; import gov.hhs.cdc.trustedintermediary.rse2e.HL7FileStream; -import gov.hhs.cdc.trustedintermediary.wrappers.Logger; import java.io.IOException; import java.io.InputStream; import java.nio.charset.StandardCharsets; @@ -15,8 +15,9 @@ import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.NoSuchElementException; import java.util.Set; -import javax.inject.Inject; +import java.util.stream.Collectors; /** * The HapiHL7FileMatcher class is responsible for matching input and output HL7 files based on the @@ -26,8 +27,6 @@ public class HapiHL7FileMatcher { private static final HapiHL7FileMatcher INSTANCE = new HapiHL7FileMatcher(); - @Inject Logger logger; - private HapiHL7FileMatcher() {} public static HapiHL7FileMatcher getInstance() { @@ -42,30 +41,19 @@ public Map matchFiles( Map inputMap = mapMessageByControlId(inputFiles); Map outputMap = mapMessageByControlId(outputFiles); - Set unmatchedInputKeys = new HashSet<>(inputMap.keySet()); - unmatchedInputKeys.removeAll(outputMap.keySet()); - - Set unmatchedOutputKeys = new HashSet<>(outputMap.keySet()); - unmatchedOutputKeys.removeAll(inputMap.keySet()); - + Set inputKeys = inputMap.keySet(); + Set outputKeys = outputMap.keySet(); Set unmatchedKeys = new HashSet<>(); - unmatchedKeys.addAll(unmatchedInputKeys); - unmatchedKeys.addAll(unmatchedOutputKeys); + unmatchedKeys.addAll(Sets.difference(inputKeys, outputKeys)); // in input but not output + unmatchedKeys.addAll(Sets.difference(outputKeys, inputKeys)); // in output but not input if (!unmatchedKeys.isEmpty()) { - logger.logError( - "Found no match for the following messages with MSH-10: " + unmatchedKeys); + throw new NoSuchElementException( + "Found no match for messages with the following MSH-10 values: " + + unmatchedKeys); } - Map messageMap = new HashMap<>(); - inputMap.keySet().retainAll(outputMap.keySet()); - inputMap.forEach( - (key, inputMessage) -> { - Message outputMessage = outputMap.get(key); - messageMap.put(inputMessage, outputMessage); - }); - - return messageMap; + return inputKeys.stream().collect(Collectors.toMap(inputMap::get, outputMap::get)); } public Map mapMessageByControlId(List files) From f29212285c79fbb8570fbec17feb59b15ba8bfd8 Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Mon, 4 Nov 2024 09:20:42 -0800 Subject: [PATCH 03/23] Fixed failing test --- .../rse2e/external/hapi/HapiHL7FileMatcher.java | 4 ++-- .../rse2e/external/hapi/HapiHL7FileMatcherTest.groovy | 11 +++++++---- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcher.java b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcher.java index 0b7c76cec..4859b8a8c 100644 --- a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcher.java +++ b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcher.java @@ -44,8 +44,8 @@ public Map matchFiles( Set inputKeys = inputMap.keySet(); Set outputKeys = outputMap.keySet(); Set unmatchedKeys = new HashSet<>(); - unmatchedKeys.addAll(Sets.difference(inputKeys, outputKeys)); // in input but not output - unmatchedKeys.addAll(Sets.difference(outputKeys, inputKeys)); // in output but not input + unmatchedKeys.addAll(Sets.difference(inputKeys, outputKeys)); + unmatchedKeys.addAll(Sets.difference(outputKeys, inputKeys)); if (!unmatchedKeys.isEmpty()) { throw new NoSuchElementException( diff --git a/rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcherTest.groovy b/rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcherTest.groovy index 0e38cf13f..318d70db2 100644 --- a/rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcherTest.groovy +++ b/rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcherTest.groovy @@ -35,12 +35,15 @@ class HapiHL7FileMatcherTest extends Specification { spyFileMatcher.mapMessageByControlId(mockOutputFiles) >> [ "2": mockOutputMessage2, "3": Mock(Message) ] when: - def result = spyFileMatcher.matchFiles(mockOutputFiles, mockInputFiles) + spyFileMatcher.matchFiles(mockOutputFiles, mockInputFiles) then: - result.size() == 1 - result == Map.of(mockInputMessage2, mockOutputMessage2) - 1 * mockLogger.logError({ it.contains("Found no match") && it.contains("1") && it.contains("3") }) + def exception = thrown(NoSuchElementException) + with(exception.getMessage()) { + contains("Found no match") + contains("1") + contains("3") + } } def "should map message by control ID"() { From 1b16688bc70fcbf93875443632f04c97c9b1b0be Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Mon, 4 Nov 2024 10:08:14 -0800 Subject: [PATCH 04/23] Added HapiHL7FileMatcherException custom exception and fixed tests --- .../external/hapi/HapiHL7FileMatcher.java | 16 ++-- .../hapi/HapiHL7FileMatcherException.java | 12 +++ .../HapiHL7FileMatcherExceptionTest.groovy | 30 +++++++ .../hapi/HapiHL7FileMatcherTest.groovy | 80 +++++++++++++++---- 4 files changed, 115 insertions(+), 23 deletions(-) create mode 100644 rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcherException.java create mode 100644 rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcherExceptionTest.groovy diff --git a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcher.java b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcher.java index 4859b8a8c..4ab050ef2 100644 --- a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcher.java +++ b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcher.java @@ -15,7 +15,6 @@ import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.NoSuchElementException; import java.util.Set; import java.util.stream.Collectors; @@ -35,7 +34,7 @@ public static HapiHL7FileMatcher getInstance() { public Map matchFiles( List outputFiles, List inputFiles) - throws HL7Exception, IOException { + throws HapiHL7FileMatcherException { // We pair up output and input files based on the control ID, which is in MSH-10 // Any files (either input or output) that don't have a match are logged Map inputMap = mapMessageByControlId(inputFiles); @@ -48,7 +47,7 @@ public Map matchFiles( unmatchedKeys.addAll(Sets.difference(outputKeys, inputKeys)); if (!unmatchedKeys.isEmpty()) { - throw new NoSuchElementException( + throw new HapiHL7FileMatcherException( "Found no match for messages with the following MSH-10 values: " + unmatchedKeys); } @@ -57,7 +56,7 @@ public Map matchFiles( } public Map mapMessageByControlId(List files) - throws HL7Exception, IOException { + throws HapiHL7FileMatcherException { Map messageMap = new HashMap<>(); @@ -72,16 +71,21 @@ public Map mapMessageByControlId(List files) MSH mshSegment = (MSH) message.get("MSH"); String msh10 = mshSegment.getMessageControlID().getValue(); if (msh10 == null || msh10.isEmpty()) { - throw new IllegalArgumentException( + throw new HapiHL7FileMatcherException( String.format("MSH-10 is empty for file: %s", fileName)); } messageMap.put(msh10, message); } catch (HL7Exception e) { - throw new HL7Exception( + throw new HapiHL7FileMatcherException( String.format("Failed to parse HL7 message from file: %s", fileName), e); + } catch (IOException e) { + throw new HapiHL7FileMatcherException( + String.format("Failed to read file: %s", fileName), e); } } + } catch (IOException e) { + throw new HapiHL7FileMatcherException("Failed to close input stream", e); } return messageMap; diff --git a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcherException.java b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcherException.java new file mode 100644 index 000000000..6e710f101 --- /dev/null +++ b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcherException.java @@ -0,0 +1,12 @@ +package gov.hhs.cdc.trustedintermediary.rse2e.external.hapi; + +public class HapiHL7FileMatcherException extends Exception { + + public HapiHL7FileMatcherException(String message, Throwable cause) { + super(message, cause); + } + + public HapiHL7FileMatcherException(String message) { + super(message); + } +} diff --git a/rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcherExceptionTest.groovy b/rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcherExceptionTest.groovy new file mode 100644 index 000000000..07e65529e --- /dev/null +++ b/rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcherExceptionTest.groovy @@ -0,0 +1,30 @@ +package gov.hhs.cdc.trustedintermediary.rse2e.external.hapi + +import spock.lang.Specification + +class HapiHL7FileMatcherExceptionTest extends Specification { + + def "two param constructor works"() { + given: + def message = "something blew up!" + def cause = new NullPointerException() + + when: + def exception = new HapiHL7FileMatcherException(message, cause) + + then: + exception.getMessage() == message + exception.getCause() == cause + } + + def "single param constructor works"() { + given: + def message = "something blew up!" + + when: + def exception = new HapiHL7FileMatcherException(message) + + then: + exception.getMessage() == message + } +} diff --git a/rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcherTest.groovy b/rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcherTest.groovy index 318d70db2..8e0f98c20 100644 --- a/rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcherTest.groovy +++ b/rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcherTest.groovy @@ -1,7 +1,6 @@ package gov.hhs.cdc.trustedintermediary.rse2e.external.hapi import ca.uhn.hl7v2.model.Message -import ca.uhn.hl7v2.HL7Exception import gov.hhs.cdc.trustedintermediary.context.TestApplicationContext import gov.hhs.cdc.trustedintermediary.rse2e.HL7FileStream import gov.hhs.cdc.trustedintermediary.wrappers.Logger @@ -21,28 +20,75 @@ class HapiHL7FileMatcherTest extends Specification { TestApplicationContext.injectRegisteredImplementations() } - def "should correctly match input and output files and log unmatched files"() { + def "should correctly match input and output files"() { given: def spyFileMatcher = Spy(HapiHL7FileMatcher.getInstance()) - def fileStream1 = new HL7FileStream("file1", Mock(InputStream)) - def fileStream2 = new HL7FileStream("file2", Mock(InputStream)) - def fileStream3 = new HL7FileStream("file3", Mock(InputStream)) - def mockInputFiles = [fileStream1, fileStream2] - def mockOutputFiles = [fileStream2, fileStream3] + def mockInputFiles = [ + new HL7FileStream("inputFileStream1", Mock(InputStream)), + new HL7FileStream("inputFileStream2", Mock(InputStream)) + ] + def mockOutputFiles = [ + new HL7FileStream("outputFileStream1", Mock(InputStream)), + new HL7FileStream("outputFileStream2", Mock(InputStream)) + ] + def mockInputMessage1 = Mock(Message) def mockInputMessage2 = Mock(Message) + def mockOutputMessage1 = Mock(Message) def mockOutputMessage2 = Mock(Message) - spyFileMatcher.mapMessageByControlId(mockInputFiles) >> [ "1": Mock(Message), "2": mockInputMessage2 ] - spyFileMatcher.mapMessageByControlId(mockOutputFiles) >> [ "2": mockOutputMessage2, "3": Mock(Message) ] + spyFileMatcher.mapMessageByControlId(mockInputFiles) >> [ "001": mockInputMessage1, "002": mockInputMessage2 ] + spyFileMatcher.mapMessageByControlId(mockOutputFiles) >> [ "001": mockOutputMessage1, "002": mockOutputMessage2 ] + + when: + def result =spyFileMatcher.matchFiles(mockOutputFiles, mockInputFiles) + + then: + result.size() == 2 + result == Map.of(mockInputMessage1, mockOutputMessage1, mockInputMessage2, mockOutputMessage2) + } + + + def "should throw HapiHL7FileMatcherException if didn't find a match for at least one file in either input or output"() { + given: + def mockInputFiles + def mockOutputFiles + def spyFileMatcher = Spy(HapiHL7FileMatcher.getInstance()) when: + mockInputFiles = [ + new HL7FileStream("nonMatchingInputFileStream", Mock(InputStream)), + new HL7FileStream("matchingInputFileStream", Mock(InputStream)) + ] + mockOutputFiles = [ + new HL7FileStream("matchingOutputFileStream", Mock(InputStream)) + ] + spyFileMatcher.mapMessageByControlId(mockInputFiles) >> [ "001": Mock(Message), "002": Mock(Message) ] + spyFileMatcher.mapMessageByControlId(mockOutputFiles) >> [ "001": Mock(Message) ] + spyFileMatcher.matchFiles(mockOutputFiles, mockInputFiles) + + then: + def nonMatchingInputException = thrown(HapiHL7FileMatcherException) + with(nonMatchingInputException.getMessage()) { + contains("Found no match") + contains("002") + } + + when: + mockInputFiles = [ + new HL7FileStream("matchingInputFileStream", Mock(InputStream)) + ] + mockOutputFiles = [ + new HL7FileStream("matchingOutputFileStream", Mock(InputStream)), + new HL7FileStream("nonMatchingOutputFileStream", Mock(InputStream)) + ] + spyFileMatcher.mapMessageByControlId(mockInputFiles) >> [ "001": Mock(Message) ] + spyFileMatcher.mapMessageByControlId(mockOutputFiles) >> [ "001": Mock(Message), "003": Mock(Message) ] spyFileMatcher.matchFiles(mockOutputFiles, mockInputFiles) then: - def exception = thrown(NoSuchElementException) - with(exception.getMessage()) { + def nonMatchingOutputException = thrown(HapiHL7FileMatcherException) + with(nonMatchingOutputException.getMessage()) { contains("Found no match") - contains("1") - contains("3") + contains("003") } } @@ -74,7 +120,7 @@ class HapiHL7FileMatcherTest extends Specification { file2MshSegment == result[file2Msh10].encode().trim() } - def "should throw IllegalArgumentException when MSH-10 is empty"() { + def "should throw HapiHL7FileMatcherException when MSH-10 is empty"() { given: def msh1to9 = "MSH|^~\\&|Sender Application^sender.test.com^DNS|Sender Facility^0.0.0.0.0.0.0.0^ISO|Receiver Application^0.0.0.0.0.0.0.0^ISO|Receiver Facility^simulated-lab-id^DNS|20230101010000-0000||ORM^O01^ORM_O01|" def msh11to12 = "|T|2.5.1" @@ -87,10 +133,10 @@ class HapiHL7FileMatcherTest extends Specification { fileMatcher.mapMessageByControlId([hl7FileStream]) then: - thrown(IllegalArgumentException) + thrown(HapiHL7FileMatcherException) } - def "should throw HL7Exception when not able to parse the file as HL7 message"() { + def "should throw HapiHL7FileMatcherException when not able to parse the file as HL7 message"() { given: def inputStream = new ByteArrayInputStream("".bytes) def hl7FileStream = new HL7FileStream("badFile", inputStream) @@ -99,6 +145,6 @@ class HapiHL7FileMatcherTest extends Specification { fileMatcher.mapMessageByControlId([hl7FileStream]) then: - thrown(HL7Exception) + thrown(HapiHL7FileMatcherException) } } From 694e44739a3cacf2cb860cd99fa96ecec80510e5 Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Mon, 4 Nov 2024 10:15:16 -0800 Subject: [PATCH 05/23] Added javadocs --- .../rse2e/external/hapi/HapiHL7FileMatcherException.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcherException.java b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcherException.java index 6e710f101..fc3e0956d 100644 --- a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcherException.java +++ b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcherException.java @@ -1,5 +1,9 @@ package gov.hhs.cdc.trustedintermediary.rse2e.external.hapi; +/** + * The HapiHL7FileMatcherException class is responsible for handling exceptions that occur in the + * HapiHL7FileMatcher class. + */ public class HapiHL7FileMatcherException extends Exception { public HapiHL7FileMatcherException(String message, Throwable cause) { From a81a097d58d9dbf8e3a8652a65348021d0bb58c1 Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Mon, 11 Nov 2024 15:10:38 -0800 Subject: [PATCH 06/23] WIP: Added AzureBlobOrganizer to reorganize blobs by dated folders --- .../rse2e/AzureBlobFileFetcher.java | 8 ++- .../rse2e/AzureBlobOrganizer.java | 57 +++++++++++++++++++ 2 files changed, 63 insertions(+), 2 deletions(-) create mode 100644 rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobOrganizer.java diff --git a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobFileFetcher.java b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobFileFetcher.java index 3245bb6d1..8e406c4d5 100644 --- a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobFileFetcher.java +++ b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobFileFetcher.java @@ -21,7 +21,7 @@ public class AzureBlobFileFetcher implements FileFetcher { private final BlobContainerClient blobContainerClient; private AzureBlobFileFetcher() { - String azureStorageConnectionName = "automated"; + String azureStorageContainerName = "automated"; String azureStorageConnectionString = System.getenv("AZURE_STORAGE_CONNECTION_STRING"); if (azureStorageConnectionString == null || azureStorageConnectionString.isEmpty()) { @@ -31,8 +31,12 @@ private AzureBlobFileFetcher() { this.blobContainerClient = new BlobContainerClientBuilder() .connectionString(azureStorageConnectionString) - .containerName(azureStorageConnectionName) + .containerName(azureStorageContainerName) .buildClient(); + + AzureBlobOrganizer blobOrganizer = + new AzureBlobOrganizer(blobContainerClient, azureStorageContainerName); + blobOrganizer.organizeBlobsByDate(); } public static FileFetcher getInstance() { diff --git a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobOrganizer.java b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobOrganizer.java new file mode 100644 index 000000000..001391277 --- /dev/null +++ b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobOrganizer.java @@ -0,0 +1,57 @@ +package gov.hhs.cdc.trustedintermediary.rse2e; + +import com.azure.storage.blob.BlobClient; +import com.azure.storage.blob.BlobContainerClient; +import com.azure.storage.blob.models.BlobItem; +import com.azure.storage.blob.models.BlobProperties; +import java.time.LocalDate; +import java.time.ZoneOffset; + +public class AzureBlobOrganizer { + + private final BlobContainerClient blobContainerClient; + private final String azureStorageContainerName; + + public AzureBlobOrganizer( + BlobContainerClient blobContainerClient, String azureStorageContainerName) { + this.blobContainerClient = blobContainerClient; + this.azureStorageContainerName = azureStorageContainerName; + } + + public void organizeBlobsByDate() { + for (BlobItem blobItem : blobContainerClient.listBlobs()) { + BlobClient sourceBlob = blobContainerClient.getBlobClient(blobItem.getName()); + BlobProperties properties = sourceBlob.getProperties(); + + if (isInDateFolder(blobItem.getName())) { + continue; + } + + LocalDate blobDate = + properties.getLastModified().toInstant().atZone(ZoneOffset.UTC).toLocalDate(); + String newPath = createDateBasedPath(blobDate, blobItem.getName()); + BlobClient destinationBlob = blobContainerClient.getBlobClient(newPath); + destinationBlob.beginCopy(sourceBlob.getBlobUrl(), null); + sourceBlob.delete(); + } + } + + private String createDateBasedPath(LocalDate date, String originalName) { + return String.format( + "%s/%d/%02d/%02d/%s", + azureStorageContainerName, + date.getYear(), + date.getMonthValue(), + date.getDayOfMonth(), + originalName); + } + + private boolean isInDateFolder(String blobName) { + String[] parts = blobName.split("/"); + return parts.length >= 4 + && parts[0].equals(azureStorageContainerName) + && parts[1].matches("\\d{4}") + && parts[2].matches("\\d{2}") + && parts[3].matches("\\d{2}"); + } +} From ade9cda8f0f431586fe3122815553d11617e65b4 Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Tue, 12 Nov 2024 11:47:27 -0800 Subject: [PATCH 07/23] Added cleanup logic, fixed storage paths and misc cleanup --- .../rse2e/AzureBlobFileFetcher.java | 16 ++++---- .../rse2e/AzureBlobOrganizer.java | 38 +++++++++---------- 2 files changed, 27 insertions(+), 27 deletions(-) diff --git a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobFileFetcher.java b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobFileFetcher.java index 8e406c4d5..c4ea3c050 100644 --- a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobFileFetcher.java +++ b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobFileFetcher.java @@ -18,10 +18,12 @@ public class AzureBlobFileFetcher implements FileFetcher { private static final FileFetcher INSTANCE = new AzureBlobFileFetcher(); + private static final int RETENTION_DAYS = 90; + private static final String CONTAINER_NAME = "automated"; + private final BlobContainerClient blobContainerClient; private AzureBlobFileFetcher() { - String azureStorageContainerName = "automated"; String azureStorageConnectionString = System.getenv("AZURE_STORAGE_CONNECTION_STRING"); if (azureStorageConnectionString == null || azureStorageConnectionString.isEmpty()) { @@ -31,12 +33,11 @@ private AzureBlobFileFetcher() { this.blobContainerClient = new BlobContainerClientBuilder() .connectionString(azureStorageConnectionString) - .containerName(azureStorageContainerName) + .containerName(CONTAINER_NAME) .buildClient(); - AzureBlobOrganizer blobOrganizer = - new AzureBlobOrganizer(blobContainerClient, azureStorageContainerName); - blobOrganizer.organizeBlobsByDate(); + AzureBlobOrganizer blobOrganizer = new AzureBlobOrganizer(blobContainerClient); + blobOrganizer.organizeAndCleanupBlobsByDate(RETENTION_DAYS); } public static FileFetcher getInstance() { @@ -53,9 +54,8 @@ public List fetchFiles() { BlobProperties properties = blobClient.getProperties(); // Currently we're doing everything in UTC. If we start uploading files manually and - // running - // this test manually, we may want to revisit this logic and/or the file structure - // because midnight UTC is 5pm PDT on the previous day + // running this test manually, we may want to revisit this logic and/or the file + // structure because midnight UTC is 5pm PDT on the previous day LocalDate blobCreationDate = properties.getLastModified().toInstant().atZone(ZoneOffset.UTC).toLocalDate(); diff --git a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobOrganizer.java b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobOrganizer.java index 001391277..d3effe05b 100644 --- a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobOrganizer.java +++ b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobOrganizer.java @@ -10,46 +10,46 @@ public class AzureBlobOrganizer { private final BlobContainerClient blobContainerClient; - private final String azureStorageContainerName; - public AzureBlobOrganizer( - BlobContainerClient blobContainerClient, String azureStorageContainerName) { + public AzureBlobOrganizer(BlobContainerClient blobContainerClient) { this.blobContainerClient = blobContainerClient; - this.azureStorageContainerName = azureStorageContainerName; } - public void organizeBlobsByDate() { + public void organizeAndCleanupBlobsByDate(int retentionDays) { for (BlobItem blobItem : blobContainerClient.listBlobs()) { - BlobClient sourceBlob = blobContainerClient.getBlobClient(blobItem.getName()); - BlobProperties properties = sourceBlob.getProperties(); + BlobClient blobClient = blobContainerClient.getBlobClient(blobItem.getName()); + + BlobProperties properties = blobClient.getProperties(); + LocalDate blobDate = + properties.getLastModified().toInstant().atZone(ZoneOffset.UTC).toLocalDate(); + + LocalDate retentionDate = LocalDate.now().minusDays(retentionDays); + if (blobDate.isBefore(retentionDate)) { + blobClient.delete(); + continue; + } if (isInDateFolder(blobItem.getName())) { continue; } - LocalDate blobDate = - properties.getLastModified().toInstant().atZone(ZoneOffset.UTC).toLocalDate(); - String newPath = createDateBasedPath(blobDate, blobItem.getName()); + String newPath = createDateBasedPath(blobDate, blobClient.getBlobName()); BlobClient destinationBlob = blobContainerClient.getBlobClient(newPath); - destinationBlob.beginCopy(sourceBlob.getBlobUrl(), null); - sourceBlob.delete(); + + destinationBlob.beginCopy(blobClient.getBlobUrl(), null); + blobClient.delete(); } } private String createDateBasedPath(LocalDate date, String originalName) { return String.format( - "%s/%d/%02d/%02d/%s", - azureStorageContainerName, - date.getYear(), - date.getMonthValue(), - date.getDayOfMonth(), - originalName); + "%d/%02d/%02d/%s", + date.getYear(), date.getMonthValue(), date.getDayOfMonth(), originalName); } private boolean isInDateFolder(String blobName) { String[] parts = blobName.split("/"); return parts.length >= 4 - && parts[0].equals(azureStorageContainerName) && parts[1].matches("\\d{4}") && parts[2].matches("\\d{2}") && parts[3].matches("\\d{2}"); From 7fd09f39806d34454c78dde8517726a7528bfbfb Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Tue, 12 Nov 2024 15:54:08 -0800 Subject: [PATCH 08/23] Refactored AzureBlobOrganizer to better handle cooying and deleting blobs, imrpoved time zone handling, added logging, more accurately check if blob was moved --- .../rse2e/AzureBlobOrganizer.java | 77 +++++++++++++------ .../rse2e/AutomatedTest.groovy | 13 ++-- 2 files changed, 61 insertions(+), 29 deletions(-) diff --git a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobOrganizer.java b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobOrganizer.java index d3effe05b..8bfadc56c 100644 --- a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobOrganizer.java +++ b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobOrganizer.java @@ -4,40 +4,71 @@ import com.azure.storage.blob.BlobContainerClient; import com.azure.storage.blob.models.BlobItem; import com.azure.storage.blob.models.BlobProperties; +import com.azure.storage.blob.options.BlobBeginCopyOptions; +import gov.hhs.cdc.trustedintermediary.context.ApplicationContext; +import gov.hhs.cdc.trustedintermediary.wrappers.Logger; +import java.time.Duration; import java.time.LocalDate; +import java.time.ZoneId; import java.time.ZoneOffset; +import java.util.Map; public class AzureBlobOrganizer { private final BlobContainerClient blobContainerClient; + private static final ZoneId TIME_ZONE = ZoneOffset.UTC; + + protected final Logger logger = ApplicationContext.getImplementation(Logger.class); + public AzureBlobOrganizer(BlobContainerClient blobContainerClient) { this.blobContainerClient = blobContainerClient; } public void organizeAndCleanupBlobsByDate(int retentionDays) { for (BlobItem blobItem : blobContainerClient.listBlobs()) { - BlobClient blobClient = blobContainerClient.getBlobClient(blobItem.getName()); + String sourcePath = blobItem.getName(); + try { + BlobClient sourceBlob = blobContainerClient.getBlobClient(sourcePath); + BlobProperties sourceProperties = sourceBlob.getProperties(); + LocalDate sourceCreationDate = + sourceProperties + .getCreationTime() + .toInstant() + .atZone(TIME_ZONE) + .toLocalDate(); - BlobProperties properties = blobClient.getProperties(); - LocalDate blobDate = - properties.getLastModified().toInstant().atZone(ZoneOffset.UTC).toLocalDate(); + LocalDate retentionDate = LocalDate.now(TIME_ZONE).minusDays(retentionDays); + if (sourceCreationDate.isBefore(retentionDate)) { + sourceBlob.delete(); + logger.logInfo("Deleted old blob: {}", sourcePath); + continue; + } - LocalDate retentionDate = LocalDate.now().minusDays(retentionDays); - if (blobDate.isBefore(retentionDate)) { - blobClient.delete(); - continue; - } + if (isInDateFolder(sourcePath, sourceCreationDate)) { + continue; + } - if (isInDateFolder(blobItem.getName())) { - continue; - } + String sourceUrl = sourceBlob.getBlobUrl(); + Map sourceMetadata = sourceProperties.getMetadata(); + BlobBeginCopyOptions copyOptions = + new BlobBeginCopyOptions(sourceUrl).setMetadata(sourceMetadata); - String newPath = createDateBasedPath(blobDate, blobClient.getBlobName()); - BlobClient destinationBlob = blobContainerClient.getBlobClient(newPath); + String destinationPath = createDateBasedPath(sourceCreationDate, sourcePath); + BlobClient destinationBlob = blobContainerClient.getBlobClient(destinationPath); + destinationBlob.beginCopy(copyOptions).waitForCompletion(Duration.ofSeconds(30)); - destinationBlob.beginCopy(blobClient.getBlobUrl(), null); - blobClient.delete(); + if (sourceBlob.getProperties().getBlobSize() + == destinationBlob.getProperties().getBlobSize()) { + sourceBlob.delete(); + logger.logInfo("Moved blob {} to {}", sourcePath, destinationPath); + } else { + destinationBlob.delete(); + logger.logError("Failed to copy blob: " + sourcePath); + } + } catch (Exception e) { + logger.logError("Error processing blob: " + sourcePath, e); + } } } @@ -47,11 +78,13 @@ private String createDateBasedPath(LocalDate date, String originalName) { date.getYear(), date.getMonthValue(), date.getDayOfMonth(), originalName); } - private boolean isInDateFolder(String blobName) { - String[] parts = blobName.split("/"); - return parts.length >= 4 - && parts[1].matches("\\d{4}") - && parts[2].matches("\\d{2}") - && parts[3].matches("\\d{2}"); + private boolean isInDateFolder(String blobPath, LocalDate creationDate) { + String expectedPath = + String.format( + "%d/%02d/%02d/", + creationDate.getYear(), + creationDate.getMonthValue(), + creationDate.getDayOfMonth()); + return blobPath.startsWith(expectedPath); } } diff --git a/rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/AutomatedTest.groovy b/rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/AutomatedTest.groovy index 5cb1cd713..89b3f64bb 100644 --- a/rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/AutomatedTest.groovy +++ b/rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/AutomatedTest.groovy @@ -21,12 +21,6 @@ class AutomatedTest extends Specification { def mockLogger = Mock(Logger) def setup() { - FileFetcher azureFileFetcher = AzureBlobFileFetcher.getInstance() - recentAzureFiles = azureFileFetcher.fetchFiles() - - FileFetcher localFileFetcher = LocalFileFetcher.getInstance() - recentLocalFiles = localFileFetcher.fetchFiles() - engine = AssertionRuleEngine.getInstance() fileMatcher = HapiHL7FileMatcher.getInstance() @@ -38,9 +32,14 @@ class AutomatedTest extends Specification { TestApplicationContext.register(Formatter, Jackson.getInstance()) TestApplicationContext.register(HapiHL7FileMatcher, fileMatcher) TestApplicationContext.register(HealthDataExpressionEvaluator, HapiHL7ExpressionEvaluator.getInstance()) - TestApplicationContext.register(AzureBlobFileFetcher, azureFileFetcher) TestApplicationContext.register(LocalFileFetcher, LocalFileFetcher.getInstance()) TestApplicationContext.injectRegisteredImplementations() + + FileFetcher azureFileFetcher = AzureBlobFileFetcher.getInstance() + recentAzureFiles = azureFileFetcher.fetchFiles() + + FileFetcher localFileFetcher = LocalFileFetcher.getInstance() + recentLocalFiles = localFileFetcher.fetchFiles() } def cleanup() { From 2c00ae0d6ad1508a42f0d4344b2f83a4157ef11e Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Wed, 13 Nov 2024 10:30:40 -0800 Subject: [PATCH 09/23] Removed non existing metadata and improved how we're checking the blob got copied --- .../rse2e/AzureBlobOrganizer.java | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobOrganizer.java b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobOrganizer.java index 8bfadc56c..ce84470eb 100644 --- a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobOrganizer.java +++ b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobOrganizer.java @@ -4,14 +4,13 @@ import com.azure.storage.blob.BlobContainerClient; import com.azure.storage.blob.models.BlobItem; import com.azure.storage.blob.models.BlobProperties; -import com.azure.storage.blob.options.BlobBeginCopyOptions; +import com.azure.storage.blob.models.CopyStatusType; import gov.hhs.cdc.trustedintermediary.context.ApplicationContext; import gov.hhs.cdc.trustedintermediary.wrappers.Logger; import java.time.Duration; import java.time.LocalDate; import java.time.ZoneId; import java.time.ZoneOffset; -import java.util.Map; public class AzureBlobOrganizer { @@ -49,17 +48,14 @@ public void organizeAndCleanupBlobsByDate(int retentionDays) { continue; } - String sourceUrl = sourceBlob.getBlobUrl(); - Map sourceMetadata = sourceProperties.getMetadata(); - BlobBeginCopyOptions copyOptions = - new BlobBeginCopyOptions(sourceUrl).setMetadata(sourceMetadata); - String destinationPath = createDateBasedPath(sourceCreationDate, sourcePath); BlobClient destinationBlob = blobContainerClient.getBlobClient(destinationPath); - destinationBlob.beginCopy(copyOptions).waitForCompletion(Duration.ofSeconds(30)); + destinationBlob + .beginCopy(sourceBlob.getBlobUrl(), null) + .waitForCompletion(Duration.ofSeconds(30)); - if (sourceBlob.getProperties().getBlobSize() - == destinationBlob.getProperties().getBlobSize()) { + CopyStatusType copyStatus = destinationBlob.getProperties().getCopyStatus(); + if (copyStatus == CopyStatusType.SUCCESS) { sourceBlob.delete(); logger.logInfo("Moved blob {} to {}", sourcePath, destinationPath); } else { From 1cb3dbff9e5032675d6e2b3a3302d944156bb728 Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Wed, 13 Nov 2024 10:55:18 -0800 Subject: [PATCH 10/23] Renaming and cleanup --- .../trustedintermediary/rse2e/AutomatedTest.groovy | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/AutomatedTest.groovy b/rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/AutomatedTest.groovy index 89b3f64bb..295f84784 100644 --- a/rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/AutomatedTest.groovy +++ b/rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/AutomatedTest.groovy @@ -14,11 +14,11 @@ import spock.lang.Specification class AutomatedTest extends Specification { - List recentAzureFiles - List recentLocalFiles + List azureFiles + List localFiles AssertionRuleEngine engine HapiHL7FileMatcher fileMatcher - def mockLogger = Mock(Logger) + Logger mockLogger = Mock(Logger) def setup() { engine = AssertionRuleEngine.getInstance() @@ -36,21 +36,21 @@ class AutomatedTest extends Specification { TestApplicationContext.injectRegisteredImplementations() FileFetcher azureFileFetcher = AzureBlobFileFetcher.getInstance() - recentAzureFiles = azureFileFetcher.fetchFiles() + azureFiles = azureFileFetcher.fetchFiles() FileFetcher localFileFetcher = LocalFileFetcher.getInstance() - recentLocalFiles = localFileFetcher.fetchFiles() + localFiles = localFileFetcher.fetchFiles() } def cleanup() { - for (HL7FileStream fileStream : recentLocalFiles + recentAzureFiles) { + for (HL7FileStream fileStream : localFiles + azureFiles) { fileStream.inputStream().close() } } def "test defined assertions on relevant messages"() { given: - def matchedFiles = fileMatcher.matchFiles(recentAzureFiles, recentLocalFiles) + def matchedFiles = fileMatcher.matchFiles(azureFiles, localFiles) when: for (messagePair in matchedFiles) { From 640dd219d1d64873fc39be70eacf039edf4037a7 Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Wed, 13 Nov 2024 10:56:06 -0800 Subject: [PATCH 11/23] Extracted time zone to use the same one everywhere --- .../rse2e/AzureBlobFileFetcher.java | 15 ++++++++------- .../rse2e/AzureBlobOrganizer.java | 9 +++------ 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobFileFetcher.java b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobFileFetcher.java index c4ea3c050..a2061164a 100644 --- a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobFileFetcher.java +++ b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobFileFetcher.java @@ -6,6 +6,7 @@ import com.azure.storage.blob.models.BlobItem; import com.azure.storage.blob.models.BlobProperties; import java.time.LocalDate; +import java.time.ZoneId; import java.time.ZoneOffset; import java.util.ArrayList; import java.util.List; @@ -16,13 +17,16 @@ */ public class AzureBlobFileFetcher implements FileFetcher { - private static final FileFetcher INSTANCE = new AzureBlobFileFetcher(); - + // We're using UTC for now, but we plan to change the timezone to be more realistic to the + // working timezones in our teams + private static final ZoneId TIME_ZONE = ZoneOffset.UTC; private static final int RETENTION_DAYS = 90; private static final String CONTAINER_NAME = "automated"; private final BlobContainerClient blobContainerClient; + private static final FileFetcher INSTANCE = new AzureBlobFileFetcher(); + private AzureBlobFileFetcher() { String azureStorageConnectionString = System.getenv("AZURE_STORAGE_CONNECTION_STRING"); @@ -37,7 +41,7 @@ private AzureBlobFileFetcher() { .buildClient(); AzureBlobOrganizer blobOrganizer = new AzureBlobOrganizer(blobContainerClient); - blobOrganizer.organizeAndCleanupBlobsByDate(RETENTION_DAYS); + blobOrganizer.organizeAndCleanupBlobsByDate(RETENTION_DAYS, TIME_ZONE); } public static FileFetcher getInstance() { @@ -53,11 +57,8 @@ public List fetchFiles() { BlobClient blobClient = blobContainerClient.getBlobClient(blobItem.getName()); BlobProperties properties = blobClient.getProperties(); - // Currently we're doing everything in UTC. If we start uploading files manually and - // running this test manually, we may want to revisit this logic and/or the file - // structure because midnight UTC is 5pm PDT on the previous day LocalDate blobCreationDate = - properties.getLastModified().toInstant().atZone(ZoneOffset.UTC).toLocalDate(); + properties.getLastModified().toInstant().atZone(TIME_ZONE).toLocalDate(); if (mostRecentDay != null && blobCreationDate.isBefore(mostRecentDay)) { continue; diff --git a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobOrganizer.java b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobOrganizer.java index ce84470eb..3b3355b82 100644 --- a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobOrganizer.java +++ b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobOrganizer.java @@ -10,21 +10,18 @@ import java.time.Duration; import java.time.LocalDate; import java.time.ZoneId; -import java.time.ZoneOffset; public class AzureBlobOrganizer { private final BlobContainerClient blobContainerClient; - private static final ZoneId TIME_ZONE = ZoneOffset.UTC; - protected final Logger logger = ApplicationContext.getImplementation(Logger.class); public AzureBlobOrganizer(BlobContainerClient blobContainerClient) { this.blobContainerClient = blobContainerClient; } - public void organizeAndCleanupBlobsByDate(int retentionDays) { + public void organizeAndCleanupBlobsByDate(int retentionDays, ZoneId timeZone) { for (BlobItem blobItem : blobContainerClient.listBlobs()) { String sourcePath = blobItem.getName(); try { @@ -34,10 +31,10 @@ public void organizeAndCleanupBlobsByDate(int retentionDays) { sourceProperties .getCreationTime() .toInstant() - .atZone(TIME_ZONE) + .atZone(timeZone) .toLocalDate(); - LocalDate retentionDate = LocalDate.now(TIME_ZONE).minusDays(retentionDays); + LocalDate retentionDate = LocalDate.now(timeZone).minusDays(retentionDays); if (sourceCreationDate.isBefore(retentionDate)) { sourceBlob.delete(); logger.logInfo("Deleted old blob: {}", sourcePath); From 05847e7b65d4371ecdabd905e5030da5207214a2 Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Wed, 13 Nov 2024 12:04:49 -0800 Subject: [PATCH 12/23] Moved methods to helper class --- .../rse2e/AzureBlobHelper.java | 20 ++++++++++++++++++ .../rse2e/AzureBlobOrganizer.java | 21 +++---------------- 2 files changed, 23 insertions(+), 18 deletions(-) create mode 100644 rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobHelper.java diff --git a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobHelper.java b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobHelper.java new file mode 100644 index 000000000..a5796cd8a --- /dev/null +++ b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobHelper.java @@ -0,0 +1,20 @@ +package gov.hhs.cdc.trustedintermediary.rse2e; + +import java.time.LocalDate; + +public class AzureBlobHelper { + + public static String buildDatePath(LocalDate date) { + return String.format( + "%d/%02d/%02d/", date.getYear(), date.getMonthValue(), date.getDayOfMonth()); + } + + public static String createDateBasedPath(LocalDate date, String originalName) { + return buildDatePath(date) + originalName; + } + + public static boolean isInDateFolder(String blobPath, LocalDate creationDate) { + String expectedPath = buildDatePath(creationDate); + return blobPath.startsWith(expectedPath); + } +} diff --git a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobOrganizer.java b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobOrganizer.java index 3b3355b82..71ed76bab 100644 --- a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobOrganizer.java +++ b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobOrganizer.java @@ -41,11 +41,12 @@ public void organizeAndCleanupBlobsByDate(int retentionDays, ZoneId timeZone) { continue; } - if (isInDateFolder(sourcePath, sourceCreationDate)) { + if (AzureBlobHelper.isInDateFolder(sourcePath, sourceCreationDate)) { continue; } - String destinationPath = createDateBasedPath(sourceCreationDate, sourcePath); + String destinationPath = + AzureBlobHelper.createDateBasedPath(sourceCreationDate, sourcePath); BlobClient destinationBlob = blobContainerClient.getBlobClient(destinationPath); destinationBlob .beginCopy(sourceBlob.getBlobUrl(), null) @@ -64,20 +65,4 @@ public void organizeAndCleanupBlobsByDate(int retentionDays, ZoneId timeZone) { } } } - - private String createDateBasedPath(LocalDate date, String originalName) { - return String.format( - "%d/%02d/%02d/%s", - date.getYear(), date.getMonthValue(), date.getDayOfMonth(), originalName); - } - - private boolean isInDateFolder(String blobPath, LocalDate creationDate) { - String expectedPath = - String.format( - "%d/%02d/%02d/", - creationDate.getYear(), - creationDate.getMonthValue(), - creationDate.getDayOfMonth()); - return blobPath.startsWith(expectedPath); - } } From 6a025fa1c039371948c1df6961c14867e0fdc3d2 Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Wed, 13 Nov 2024 12:16:50 -0800 Subject: [PATCH 13/23] Changed azure fetch logic to get blobs in expected path for todays date --- .../rse2e/AzureBlobFileFetcher.java | 28 ++++++------------- .../rse2e/AzureBlobHelper.java | 6 ++-- 2 files changed, 11 insertions(+), 23 deletions(-) diff --git a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobFileFetcher.java b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobFileFetcher.java index a2061164a..cb67d0d3c 100644 --- a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobFileFetcher.java +++ b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobFileFetcher.java @@ -4,7 +4,7 @@ import com.azure.storage.blob.BlobContainerClient; import com.azure.storage.blob.BlobContainerClientBuilder; import com.azure.storage.blob.models.BlobItem; -import com.azure.storage.blob.models.BlobProperties; +import com.azure.storage.blob.models.ListBlobsOptions; import java.time.LocalDate; import java.time.ZoneId; import java.time.ZoneOffset; @@ -50,29 +50,17 @@ public static FileFetcher getInstance() { @Override public List fetchFiles() { - List recentFiles = new ArrayList<>(); - LocalDate mostRecentDay = null; + List relevantFiles = new ArrayList<>(); - for (BlobItem blobItem : blobContainerClient.listBlobs()) { + LocalDate today = LocalDate.now(TIME_ZONE); + String pathPrefix = AzureBlobHelper.buildDatePathPrefix(today); + ListBlobsOptions options = new ListBlobsOptions().setPrefix(pathPrefix); + for (BlobItem blobItem : blobContainerClient.listBlobs(options, null)) { BlobClient blobClient = blobContainerClient.getBlobClient(blobItem.getName()); - BlobProperties properties = blobClient.getProperties(); - - LocalDate blobCreationDate = - properties.getLastModified().toInstant().atZone(TIME_ZONE).toLocalDate(); - - if (mostRecentDay != null && blobCreationDate.isBefore(mostRecentDay)) { - continue; - } - - if (mostRecentDay == null || blobCreationDate.isAfter(mostRecentDay)) { - mostRecentDay = blobCreationDate; - recentFiles.clear(); - } - - recentFiles.add( + relevantFiles.add( new HL7FileStream(blobClient.getBlobName(), blobClient.openInputStream())); } - return recentFiles; + return relevantFiles; } } diff --git a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobHelper.java b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobHelper.java index a5796cd8a..03882572a 100644 --- a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobHelper.java +++ b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobHelper.java @@ -4,17 +4,17 @@ public class AzureBlobHelper { - public static String buildDatePath(LocalDate date) { + public static String buildDatePathPrefix(LocalDate date) { return String.format( "%d/%02d/%02d/", date.getYear(), date.getMonthValue(), date.getDayOfMonth()); } public static String createDateBasedPath(LocalDate date, String originalName) { - return buildDatePath(date) + originalName; + return buildDatePathPrefix(date) + originalName; } public static boolean isInDateFolder(String blobPath, LocalDate creationDate) { - String expectedPath = buildDatePath(creationDate); + String expectedPath = buildDatePathPrefix(creationDate); return blobPath.startsWith(expectedPath); } } From 4a86f7f753ae036716d1b733912726051422c45e Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Wed, 13 Nov 2024 14:45:58 -0800 Subject: [PATCH 14/23] Added tests for AzureBlobHelper --- .../rse2e/AzureBlobHelper.java | 2 + .../rse2e/LocalFileFetcher.java | 1 + .../rse2e/AzureBlobHelperTest.groovy | 40 +++++++++++++++++++ 3 files changed, 43 insertions(+) create mode 100644 rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobHelperTest.groovy diff --git a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobHelper.java b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobHelper.java index 03882572a..1bc68ad50 100644 --- a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobHelper.java +++ b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobHelper.java @@ -4,6 +4,8 @@ public class AzureBlobHelper { + private AzureBlobHelper() {} + public static String buildDatePathPrefix(LocalDate date) { return String.format( "%d/%02d/%02d/", date.getYear(), date.getMonthValue(), date.getDayOfMonth()); diff --git a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/LocalFileFetcher.java b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/LocalFileFetcher.java index e8cb9cada..3172c5e36 100644 --- a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/LocalFileFetcher.java +++ b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/LocalFileFetcher.java @@ -17,6 +17,7 @@ public class LocalFileFetcher implements FileFetcher { private static final String FILES_PATH = "../examples/Test/Automated/"; private static final String EXTENSION = "hl7"; + private static final FileFetcher INSTANCE = new LocalFileFetcher(); private LocalFileFetcher() {} diff --git a/rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobHelperTest.groovy b/rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobHelperTest.groovy new file mode 100644 index 000000000..29b39b674 --- /dev/null +++ b/rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobHelperTest.groovy @@ -0,0 +1,40 @@ +package gov.hhs.cdc.trustedintermediary.rse2e + +import spock.lang.Specification + +import java.time.LocalDate + +class AzureBlobHelperTest extends Specification { + + def "buildDatePathPrefix should create correct path format"() { + given: + def date = LocalDate.of(2024, 3, 15) + + when: + def result = AzureBlobHelper.buildDatePathPrefix(date) + + then: + result == "2024/03/15/" + } + + def "createDateBasedPath should combine date prefix with filename"() { + given: + def date = LocalDate.of(2024, 3, 15) + def fileName = "test.hl7" + + when: + def result = AzureBlobHelper.createDateBasedPath(date, fileName) + + then: + result == "2024/03/15/test.hl7" + } + + def "isInDateFolder should return true for matching date folder"() { + given: + def date = LocalDate.of(2024, 3, 15) + def path = "2024/03/15/test.hl7" + + expect: + AzureBlobHelper.isInDateFolder(path, date) + } +} From 1710fae97c278ee2981d8e86a6d658637ead48df Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Wed, 13 Nov 2024 15:35:19 -0800 Subject: [PATCH 15/23] Renamed sourcePath => sourceName --- .../rse2e/AzureBlobOrganizer.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobOrganizer.java b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobOrganizer.java index 71ed76bab..52f547e78 100644 --- a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobOrganizer.java +++ b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobOrganizer.java @@ -23,9 +23,9 @@ public AzureBlobOrganizer(BlobContainerClient blobContainerClient) { public void organizeAndCleanupBlobsByDate(int retentionDays, ZoneId timeZone) { for (BlobItem blobItem : blobContainerClient.listBlobs()) { - String sourcePath = blobItem.getName(); + String sourceName = blobItem.getName(); try { - BlobClient sourceBlob = blobContainerClient.getBlobClient(sourcePath); + BlobClient sourceBlob = blobContainerClient.getBlobClient(sourceName); BlobProperties sourceProperties = sourceBlob.getProperties(); LocalDate sourceCreationDate = sourceProperties @@ -37,17 +37,17 @@ public void organizeAndCleanupBlobsByDate(int retentionDays, ZoneId timeZone) { LocalDate retentionDate = LocalDate.now(timeZone).minusDays(retentionDays); if (sourceCreationDate.isBefore(retentionDate)) { sourceBlob.delete(); - logger.logInfo("Deleted old blob: {}", sourcePath); + logger.logInfo("Deleted old blob: {}", sourceName); continue; } - if (AzureBlobHelper.isInDateFolder(sourcePath, sourceCreationDate)) { + if (AzureBlobHelper.isInDateFolder(sourceName, sourceCreationDate)) { continue; } - String destinationPath = - AzureBlobHelper.createDateBasedPath(sourceCreationDate, sourcePath); - BlobClient destinationBlob = blobContainerClient.getBlobClient(destinationPath); + String destinationName = + AzureBlobHelper.createDateBasedPath(sourceCreationDate, sourceName); + BlobClient destinationBlob = blobContainerClient.getBlobClient(destinationName); destinationBlob .beginCopy(sourceBlob.getBlobUrl(), null) .waitForCompletion(Duration.ofSeconds(30)); @@ -55,13 +55,13 @@ public void organizeAndCleanupBlobsByDate(int retentionDays, ZoneId timeZone) { CopyStatusType copyStatus = destinationBlob.getProperties().getCopyStatus(); if (copyStatus == CopyStatusType.SUCCESS) { sourceBlob.delete(); - logger.logInfo("Moved blob {} to {}", sourcePath, destinationPath); + logger.logInfo("Moved blob {} to {}", sourceName, destinationName); } else { destinationBlob.delete(); - logger.logError("Failed to copy blob: " + sourcePath); + logger.logError("Failed to copy blob: " + sourceName); } } catch (Exception e) { - logger.logError("Error processing blob: " + sourcePath, e); + logger.logError("Error processing blob: " + sourceName, e); } } } From 551d9297a7e69cb4acda3756083cd807b0c84dd7 Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Thu, 14 Nov 2024 09:09:16 -0800 Subject: [PATCH 16/23] Added javadocs --- .../hhs/cdc/trustedintermediary/rse2e/AzureBlobFileFetcher.java | 2 +- .../gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobHelper.java | 1 + .../hhs/cdc/trustedintermediary/rse2e/AzureBlobOrganizer.java | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobFileFetcher.java b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobFileFetcher.java index cb67d0d3c..18b790580 100644 --- a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobFileFetcher.java +++ b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobFileFetcher.java @@ -13,7 +13,7 @@ /** * The AzureBlobFileFetcher class implements the {@link FileFetcher FileFetcher} interface and - * fetches files from an Azure Blob Storage container. + * retrieves files from an Azure Blob Storage container. */ public class AzureBlobFileFetcher implements FileFetcher { diff --git a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobHelper.java b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobHelper.java index 1bc68ad50..67ce4a1b0 100644 --- a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobHelper.java +++ b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobHelper.java @@ -2,6 +2,7 @@ import java.time.LocalDate; +/* The AzureBlobHelper is a utility class that provides helper methods for working with Azure Blob Storage. */ public class AzureBlobHelper { private AzureBlobHelper() {} diff --git a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobOrganizer.java b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobOrganizer.java index 52f547e78..4f61997e0 100644 --- a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobOrganizer.java +++ b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobOrganizer.java @@ -11,6 +11,7 @@ import java.time.LocalDate; import java.time.ZoneId; +/* AzureBlobOrganizer is responsible for organizing and cleaning up blobs in an Azure container */ public class AzureBlobOrganizer { private final BlobContainerClient blobContainerClient; From c8ac0a1ce644cfb8e0874e86e6e785de5f67ccaf Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Thu, 14 Nov 2024 10:15:54 -0800 Subject: [PATCH 17/23] Added assertion to ensure all rules ran --- .../rse2e/ruleengine/AssertionRuleEngine.java | 13 +++++++++++-- .../trustedintermediary/rse2e/AutomatedTest.groovy | 7 ++++++- .../rse2e/ruleengine/AssertionRuleEngineTest.groovy | 4 ++-- 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/ruleengine/AssertionRuleEngine.java b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/ruleengine/AssertionRuleEngine.java index a040e67f9..cf5a41178 100644 --- a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/ruleengine/AssertionRuleEngine.java +++ b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/ruleengine/AssertionRuleEngine.java @@ -9,7 +9,9 @@ import java.io.IOException; import java.io.InputStream; import java.util.ArrayList; +import java.util.HashSet; import java.util.List; +import java.util.Set; import javax.inject.Inject; /** @@ -58,21 +60,28 @@ public void ensureRulesLoaded() throws RuleLoaderException { } } - public void runRules(Message outputMessage, Message inputMessage) { + public List getRules() { + return assertionRules; + } + + public Set runRules(Message outputMessage, Message inputMessage) { try { ensureRulesLoaded(); } catch (RuleLoaderException e) { logger.logError("Failed to load rules definitions", e); - return; + return Set.of(); } HapiHL7Message outputHapiMessage = new HapiHL7Message(outputMessage); HapiHL7Message inputHapiMessage = new HapiHL7Message(inputMessage); + Set runRules = new HashSet<>(); for (AssertionRule rule : assertionRules) { if (rule.shouldRun(outputHapiMessage)) { rule.runRule(outputHapiMessage, inputHapiMessage); + runRules.add(rule); } } + return runRules; } } diff --git a/rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/AutomatedTest.groovy b/rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/AutomatedTest.groovy index 295f84784..cc22b90e0 100644 --- a/rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/AutomatedTest.groovy +++ b/rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/AutomatedTest.groovy @@ -40,6 +40,8 @@ class AutomatedTest extends Specification { FileFetcher localFileFetcher = LocalFileFetcher.getInstance() localFiles = localFileFetcher.fetchFiles() + + engine.ensureRulesLoaded() } def cleanup() { @@ -50,16 +52,19 @@ class AutomatedTest extends Specification { def "test defined assertions on relevant messages"() { given: + def toRunRules = engine.getRules() def matchedFiles = fileMatcher.matchFiles(azureFiles, localFiles) when: for (messagePair in matchedFiles) { Message inputMessage = messagePair.getKey() as Message Message outputMessage = messagePair.getValue() as Message - engine.runRules(outputMessage, inputMessage) + def runRules = engine.runRules(outputMessage, inputMessage) + toRunRules.removeAll(runRules) } then: + toRunRules.collect { it.name }.isEmpty() 0 * mockLogger.logError(_ as String, _ as Exception) 0 * mockLogger.logWarning(_ as String) } diff --git a/rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/ruleengine/AssertionRuleEngineTest.groovy b/rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/ruleengine/AssertionRuleEngineTest.groovy index ce7250ca8..7a991a1ce 100644 --- a/rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/ruleengine/AssertionRuleEngineTest.groovy +++ b/rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/ruleengine/AssertionRuleEngineTest.groovy @@ -37,7 +37,7 @@ class AssertionRuleEngineTest extends Specification { then: 1 * mockRuleLoader.loadRules(_ as InputStream, _ as TypeReference) >> [mockRule] - ruleEngine.assertionRules.size() == 1 + ruleEngine.getRules().size() == 1 } def "ensureRulesLoaded loads rules only once by default"() { @@ -50,7 +50,7 @@ class AssertionRuleEngineTest extends Specification { then: 1 * mockRuleLoader.loadRules(_ as InputStream, _ as TypeReference) >> [mockRule] - ruleEngine.assertionRules.size() == 1 + ruleEngine.rules.size() == 1 } def "ensureRulesLoaded loads rules only once on multiple threads"() { From 0f6b87366d499df71fba46145a1ed9b3f0661842 Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Thu, 14 Nov 2024 11:49:23 -0800 Subject: [PATCH 18/23] Added test coverage --- .../ruleengine/AssertionRuleEngineTest.groovy | 36 ++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/ruleengine/AssertionRuleEngineTest.groovy b/rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/ruleengine/AssertionRuleEngineTest.groovy index 7a991a1ce..c7582c80e 100644 --- a/rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/ruleengine/AssertionRuleEngineTest.groovy +++ b/rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/ruleengine/AssertionRuleEngineTest.groovy @@ -1,6 +1,7 @@ package gov.hhs.cdc.trustedintermediary.rse2e.ruleengine import ca.uhn.hl7v2.model.Message +import gov.hhs.cdc.trustedintermediary.rse2e.external.hapi.HapiHL7Message import gov.hhs.cdc.trustedintermediary.ruleengine.RuleLoader import gov.hhs.cdc.trustedintermediary.ruleengine.RuleLoaderException import gov.hhs.cdc.trustedintermediary.context.TestApplicationContext @@ -50,7 +51,7 @@ class AssertionRuleEngineTest extends Specification { then: 1 * mockRuleLoader.loadRules(_ as InputStream, _ as TypeReference) >> [mockRule] - ruleEngine.rules.size() == 1 + ruleEngine.getRules().size() == 1 } def "ensureRulesLoaded loads rules only once on multiple threads"() { @@ -101,4 +102,37 @@ class AssertionRuleEngineTest extends Specification { then: 1 * mockLogger.logError(_ as String, exception) } + + def "runRules returns nothing when there are no rules"() { + when: + def result = ruleEngine.runRules(Mock(Message), Mock(Message)) + + then: + result.isEmpty() + } + + def "runRules returns rules that have been run"() { + given: + def rule = new AssertionRule("testRule", [], []) + mockRuleLoader.loadRules(_ as InputStream, _ as TypeReference) >> [rule] + + when: + def result = ruleEngine.runRules(Mock(Message), Mock(Message)) + + then: + result.size() == 1 + result[0] == rule + } + + def "runRules doesn't return rules that shouldn't run"() { + given: + def rule = new AssertionRule("testRule", ["false"], []) + mockRuleLoader.loadRules(_ as InputStream, _ as TypeReference) >> [rule] + + when: + def result = ruleEngine.runRules(Mock(Message), Mock(Message)) + + then: + result.isEmpty() + } } From 06cb089cad4f9269da05d277fce64afc2feb4e48 Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Fri, 15 Nov 2024 07:37:51 -0800 Subject: [PATCH 19/23] Removed UTC timezone comment. Planning to keep using UTC --- .../hhs/cdc/trustedintermediary/rse2e/AzureBlobFileFetcher.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobFileFetcher.java b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobFileFetcher.java index 18b790580..ceefe5c4f 100644 --- a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobFileFetcher.java +++ b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobFileFetcher.java @@ -17,8 +17,6 @@ */ public class AzureBlobFileFetcher implements FileFetcher { - // We're using UTC for now, but we plan to change the timezone to be more realistic to the - // working timezones in our teams private static final ZoneId TIME_ZONE = ZoneOffset.UTC; private static final int RETENTION_DAYS = 90; private static final String CONTAINER_NAME = "automated"; From 572529d227015f2f5572d41ecf77ff31e3cd5e14 Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Fri, 15 Nov 2024 07:38:50 -0800 Subject: [PATCH 20/23] Added comment explaining date path formatting --- .../gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobHelper.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobHelper.java b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobHelper.java index 67ce4a1b0..d6548229b 100644 --- a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobHelper.java +++ b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/AzureBlobHelper.java @@ -7,6 +7,8 @@ public class AzureBlobHelper { private AzureBlobHelper() {} + // Builds a path prefix for a given date in the format "YYYY/MM/DD/". This is meant to make it + // easier for people in the team to find files in the Azure Blob Storage public static String buildDatePathPrefix(LocalDate date) { return String.format( "%d/%02d/%02d/", date.getYear(), date.getMonthValue(), date.getDayOfMonth()); From c7521b7ab851ad4ce8de4beddeaccc711b29fa5c Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Fri, 15 Nov 2024 09:57:11 -0800 Subject: [PATCH 21/23] Added note about blob retention policy --- rs-e2e/README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rs-e2e/README.md b/rs-e2e/README.md index 242909bd0..debc7bf60 100644 --- a/rs-e2e/README.md +++ b/rs-e2e/README.md @@ -6,6 +6,8 @@ Intermediary and ReportStream. It's scheduled to run daily using the Information on how to set up the sample files evaluated by the tests can be found [here](/examples/Test/Automated/README.md) +**Note**: the output files generated by the framework are stored in an Azure blob storage container. The files are retained in the container for 90 days before being deleted + ## Running the tests - Automatically - these are scheduled to run every weekday From cf3b28538bbfcb46ec6781239cb9f8bc4a52b82f Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Fri, 15 Nov 2024 10:08:30 -0800 Subject: [PATCH 22/23] Added more context on file org --- rs-e2e/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rs-e2e/README.md b/rs-e2e/README.md index debc7bf60..3037887a6 100644 --- a/rs-e2e/README.md +++ b/rs-e2e/README.md @@ -6,7 +6,7 @@ Intermediary and ReportStream. It's scheduled to run daily using the Information on how to set up the sample files evaluated by the tests can be found [here](/examples/Test/Automated/README.md) -**Note**: the output files generated by the framework are stored in an Azure blob storage container. The files are retained in the container for 90 days before being deleted +**Note**: the output files generated by the framework are stored in an Azure blob storage container. Every time the tests are run, the files are moved to a folder with the year/month/day format for better organization. The files are retained in the container for 90 days before being deleted ## Running the tests From fdc062a81a8134fa1a30a18d200a612150617904 Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Fri, 15 Nov 2024 12:12:03 -0800 Subject: [PATCH 23/23] Renamed and added comment for clarity --- .../cdc/trustedintermediary/rse2e/AutomatedTest.groovy | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/AutomatedTest.groovy b/rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/AutomatedTest.groovy index cc22b90e0..73c62db06 100644 --- a/rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/AutomatedTest.groovy +++ b/rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/AutomatedTest.groovy @@ -52,19 +52,19 @@ class AutomatedTest extends Specification { def "test defined assertions on relevant messages"() { given: - def toRunRules = engine.getRules() + def rulesToEvaluate = engine.getRules() def matchedFiles = fileMatcher.matchFiles(azureFiles, localFiles) when: for (messagePair in matchedFiles) { Message inputMessage = messagePair.getKey() as Message Message outputMessage = messagePair.getValue() as Message - def runRules = engine.runRules(outputMessage, inputMessage) - toRunRules.removeAll(runRules) + def evaluatedRules = engine.runRules(outputMessage, inputMessage) + rulesToEvaluate.removeAll(evaluatedRules) } then: - toRunRules.collect { it.name }.isEmpty() + rulesToEvaluate.collect { it.name }.isEmpty() //Check whether all the rules in the assertions file have been run 0 * mockLogger.logError(_ as String, _ as Exception) 0 * mockLogger.logWarning(_ as String) }