From 13e2c945bb49529bc37363ec9915202ebf377892 Mon Sep 17 00:00:00 2001 From: jdcs Date: Thu, 26 Jan 2023 10:39:20 +0100 Subject: [PATCH 1/6] feat: add logs for ForwardService --- pom.xml | 2 +- .../java/org/karnak/backend/service/ForwardService.java | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index a0050bb0..7fbdeedd 100644 --- a/pom.xml +++ b/pom.xml @@ -16,7 +16,7 @@ - 1.0.54 + 1.0.55 jar diff --git a/src/main/java/org/karnak/backend/service/ForwardService.java b/src/main/java/org/karnak/backend/service/ForwardService.java index 48d5fcee..1ca623fb 100644 --- a/src/main/java/org/karnak/backend/service/ForwardService.java +++ b/src/main/java/org/karnak/backend/service/ForwardService.java @@ -15,6 +15,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Set; +import java.util.stream.Collectors; import org.dcm4che3.data.Attributes; import org.dcm4che3.data.Tag; import org.dcm4che3.data.UID; @@ -159,6 +160,9 @@ public static synchronized StoreFromStreamSCU prepareTransfer( // Handle dynamically new SOPClassUID Set tss = streamSCU.getTransferSyntaxesFor(cuid); if (!tss.contains(dstTsuid)) { + LOGGER.info("New output transfer syntax "+dstTsuid+": closing streamSCU"); + LOGGER.info("Current list of transfer syntaxes for "+cuid+":"+tss.stream().collect( + Collectors.joining(","))); streamSCU.close(true); } @@ -170,6 +174,7 @@ public static synchronized StoreFromStreamSCU prepareTransfer( if (!streamSCU.isReadyForDataTransfer()) { // If connection has been closed just reopen + LOGGER.info("streamSCU not ready for data transfer, reopen streamSCU"); streamSCU.open(); } } else { @@ -182,6 +187,7 @@ public static synchronized StoreFromStreamSCU prepareTransfer( if (DicomOutputData.isAdaptableSyntax(dstTsuid)) { streamSCU.addData(cuid, UID.JPEGLosslessSV1); } + LOGGER.info("open streamSCU"); streamSCU.open(); } return streamSCU; @@ -297,6 +303,7 @@ public List transfer( e.getMessage()); LOGGER.error(ERROR_WHEN_FORWARDING, e); } finally { + LOGGER.info("streamSCU triggerCloseExecutor transfer"); streamSCU.triggerCloseExecutor(); files = cleanOrGetBulkDataFiles(in, copy == null); } @@ -469,6 +476,7 @@ public void transferOther( e.getMessage()); LOGGER.error(ERROR_WHEN_FORWARDING, e); } finally { + LOGGER.info("streamSCU triggerCloseExecutor transferOther"); streamSCU.triggerCloseExecutor(); } } From 531c8bca18584171403c2cc1bca9683540861fbf Mon Sep 17 00:00:00 2001 From: jdcs Date: Thu, 26 Jan 2023 11:51:48 +0100 Subject: [PATCH 2/6] feat: logs --- pom.xml | 2 +- .../org/karnak/backend/service/ForwardService.java | 2 -- src/main/resources/logback-spring.xml | 10 +++++----- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/pom.xml b/pom.xml index 7fbdeedd..aa610e49 100644 --- a/pom.xml +++ b/pom.xml @@ -16,7 +16,7 @@ - 1.0.55 + 1.0.56 jar diff --git a/src/main/java/org/karnak/backend/service/ForwardService.java b/src/main/java/org/karnak/backend/service/ForwardService.java index 1ca623fb..74871f13 100644 --- a/src/main/java/org/karnak/backend/service/ForwardService.java +++ b/src/main/java/org/karnak/backend/service/ForwardService.java @@ -303,7 +303,6 @@ public List transfer( e.getMessage()); LOGGER.error(ERROR_WHEN_FORWARDING, e); } finally { - LOGGER.info("streamSCU triggerCloseExecutor transfer"); streamSCU.triggerCloseExecutor(); files = cleanOrGetBulkDataFiles(in, copy == null); } @@ -476,7 +475,6 @@ public void transferOther( e.getMessage()); LOGGER.error(ERROR_WHEN_FORWARDING, e); } finally { - LOGGER.info("streamSCU triggerCloseExecutor transferOther"); streamSCU.triggerCloseExecutor(); } } diff --git a/src/main/resources/logback-spring.xml b/src/main/resources/logback-spring.xml index 627e0f33..a7be3301 100644 --- a/src/main/resources/logback-spring.xml +++ b/src/main/resources/logback-spring.xml @@ -9,10 +9,10 @@ - - - - - + + + + + From 955088b3a9c5938eed736393997bee2fc2a52d71 Mon Sep 17 00:00:00 2001 From: jdcs Date: Thu, 26 Jan 2023 12:10:37 +0100 Subject: [PATCH 3/6] feat: logs --- pom.xml | 2 +- src/main/resources/logback-spring.xml | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pom.xml b/pom.xml index aa610e49..c1e06544 100644 --- a/pom.xml +++ b/pom.xml @@ -16,7 +16,7 @@ - 1.0.56 + 1.0.57 jar diff --git a/src/main/resources/logback-spring.xml b/src/main/resources/logback-spring.xml index a7be3301..627e0f33 100644 --- a/src/main/resources/logback-spring.xml +++ b/src/main/resources/logback-spring.xml @@ -9,10 +9,10 @@ - - - - - + + + + + From 11403c6b4b96091576dbad850d8c3fa8aa76b7ca Mon Sep 17 00:00:00 2001 From: jdcs Date: Tue, 14 Feb 2023 17:02:26 +0100 Subject: [PATCH 4/6] feat: modify ForwardService/multiple destinations: gather prepare and transfer of files --- pom.xml | 4 +- .../backend/service/ForwardService.java | 178 ++++++++---------- 2 files changed, 77 insertions(+), 105 deletions(-) diff --git a/pom.xml b/pom.xml index c1e06544..6eafa6f8 100644 --- a/pom.xml +++ b/pom.xml @@ -32,8 +32,8 @@ 3.0.0 1.6.2 21.0.9 - 5.27.0.1 - 4.5.5-dcm + 5.29.0.2 + 4.6.0-dcm 1.6.0 4.2.0 1.4.200 diff --git a/src/main/java/org/karnak/backend/service/ForwardService.java b/src/main/java/org/karnak/backend/service/ForwardService.java index 74871f13..b187de69 100644 --- a/src/main/java/org/karnak/backend/service/ForwardService.java +++ b/src/main/java/org/karnak/backend/service/ForwardService.java @@ -15,11 +15,10 @@ import java.util.ArrayList; import java.util.List; import java.util.Set; -import java.util.stream.Collectors; +import java.util.stream.IntStream; import org.dcm4che3.data.Attributes; import org.dcm4che3.data.Tag; import org.dcm4che3.data.UID; -import org.dcm4che3.img.DicomOutputData; import org.dcm4che3.img.op.MaskArea; import org.dcm4che3.img.stream.BytesWithImageDescriptor; import org.dcm4che3.img.stream.ImageAdapter; @@ -63,11 +62,10 @@ @Service public class ForwardService { - private static final String ERROR_WHEN_FORWARDING = - "Error when forwarding to the final destination"; - private static final Logger LOGGER = LoggerFactory.getLogger(ForwardService.class); + private static final String ERROR_WHEN_FORWARDING = "Error when forwarding to the final destination"; + private final ApplicationEventPublisher applicationEventPublisher; @Autowired @@ -76,10 +74,10 @@ public ForwardService(final ApplicationEventPublisher applicationEventPublisher) } public void storeMultipleDestination( - ForwardDicomNode fwdNode, List destList, Params p) throws IOException { - if (destList == null || destList.isEmpty()) { + ForwardDicomNode forwardNode, List destinations, Params p) throws IOException { + if (destinations == null || destinations.isEmpty()) { throw new IllegalStateException( - "Cannot find the DICOM destination from " + fwdNode.toString()); + "Cannot find destinations from " + forwardNode.toString()); } // Exclude DICOMDIR if ("1.2.840.10008.1.3.10".equals(p.getCuid())) { @@ -87,109 +85,71 @@ public void storeMultipleDestination( return; } - if (destList.size() == 1) { - storeOneDestination(fwdNode, destList.get(0), p); - } else { - List destConList = new ArrayList<>(); - for (ForwardDestination fwDest : destList) { - try { - if (fwDest instanceof DicomForwardDestination) { - prepareTransfer((DicomForwardDestination) fwDest, p); - } - destConList.add(fwDest); - } catch (Exception e) { - LOGGER.error("Cannot connect to the final destination", e); + // Prepare and transfer files + Attributes attributes = new Attributes(); + List files = new ArrayList<>(); + try { + IntStream.range(0, destinations.size()).forEachOrdered(i -> + prepareAndTransfer(forwardNode, p, i, destinations.get(i), attributes, destinations.size(), files)); + } + finally { + if (!files.isEmpty()) { + // Force to clean if tmp bulk files + for (File file : files) { + FileUtil.delete(file); } } + } + } - if (destConList.isEmpty()) { - return; - } else if (destConList.size() == 1) { - storeOneDestination(fwdNode, destConList.get(0), p); - } else { - List files = null; - try { - Attributes attributes = new Attributes(); - ForwardDestination fistDest = destConList.get(0); - if (fistDest instanceof DicomForwardDestination) { - files = transfer(fwdNode, (DicomForwardDestination) fistDest, attributes, p); - } else if (fistDest instanceof WebForwardDestination) { - files = transfer(fwdNode, (WebForwardDestination) fistDest, attributes, p); + /** + * Prepare and transfer files + * + * @param fwdNode Forward node + * @param p Params + * @param index Current index + * @param destination Destination + * @param attributes Attributes + * @param nbDestinations Number of destinations to handle + * @param files Temp files to delete + */ + private void prepareAndTransfer(ForwardDicomNode fwdNode, Params p, int index, + ForwardDestination destination, Attributes attributes, int nbDestinations, List files) { + try { + if (destination instanceof DicomForwardDestination) { + // Prepare transfer only for dicom destination + prepareTransfer((DicomForwardDestination) destination, p); + } + if (index == 0) { + // Case first iteration: handle first destination of the forward node + Attributes attToApply = nbDestinations > 1 ? attributes : null; + if (destination instanceof DicomForwardDestination) { + files.addAll(transfer(fwdNode, (DicomForwardDestination) destination, attToApply, p)); + } else if (destination instanceof WebForwardDestination) { + files.addAll(transfer(fwdNode, (WebForwardDestination) destination, attToApply, p)); } + } else { + // Case other iterations: handle other destinations of the forward node if (!attributes.isEmpty()) { - for (int i = 1; i < destConList.size(); i++) { - ForwardDestination dest = destConList.get(i); - if (dest instanceof DicomForwardDestination) { - transferOther(fwdNode, (DicomForwardDestination) dest, attributes, p); - } else if (dest instanceof WebForwardDestination) { - transferOther(fwdNode, (WebForwardDestination) dest, attributes, p); - } - } - } - } finally { - if (files != null) { - // Force to clean if tmp bulk files - for (File file : files) { - FileUtil.delete(file); + if (destination instanceof DicomForwardDestination) { + transferOther(fwdNode, (DicomForwardDestination) destination, attributes, p); + } else if (destination instanceof WebForwardDestination) { + transferOther(fwdNode, (WebForwardDestination) destination, attributes, p); } } } + } catch (Exception e) { + LOGGER.error("Cannot connect to the final destination", e); } - } } - public void storeOneDestination( - ForwardDicomNode fwdNode, ForwardDestination destination, Params p) throws IOException { - if (destination instanceof DicomForwardDestination) { - DicomForwardDestination dest = (DicomForwardDestination) destination; - prepareTransfer(dest, p); - transfer(fwdNode, dest, null, p); - } else if (destination instanceof WebForwardDestination) { - transfer(fwdNode, (WebForwardDestination) destination, null, p); - } - } - - public static synchronized StoreFromStreamSCU prepareTransfer( + public static StoreFromStreamSCU prepareTransfer( DicomForwardDestination destination, Params p) throws IOException { String cuid = p.getCuid(); String tsuid = p.getTsuid(); String dstTsuid = destination.getOutputTransferSyntax(tsuid); StoreFromStreamSCU streamSCU = destination.getStreamSCU(); - - if (streamSCU.hasAssociation()) { - // Handle dynamically new SOPClassUID - Set tss = streamSCU.getTransferSyntaxesFor(cuid); - if (!tss.contains(dstTsuid)) { - LOGGER.info("New output transfer syntax "+dstTsuid+": closing streamSCU"); - LOGGER.info("Current list of transfer syntaxes for "+cuid+":"+tss.stream().collect( - Collectors.joining(","))); - streamSCU.close(true); - } - - // Add Presentation Context for the association - streamSCU.addData(cuid, dstTsuid); - if (DicomOutputData.isAdaptableSyntax(dstTsuid)) { - streamSCU.addData(cuid, UID.JPEGLosslessSV1); - } - - if (!streamSCU.isReadyForDataTransfer()) { - // If connection has been closed just reopen - LOGGER.info("streamSCU not ready for data transfer, reopen streamSCU"); - streamSCU.open(); - } - } else { - destination.getStreamSCUService().start(); - // Add Presentation Context for the association - streamSCU.addData(cuid, dstTsuid); - if (!dstTsuid.equals(UID.ExplicitVRLittleEndian)) { - streamSCU.addData(cuid, UID.ExplicitVRLittleEndian); - } - if (DicomOutputData.isAdaptableSyntax(dstTsuid)) { - streamSCU.addData(cuid, UID.JPEGLosslessSV1); - } - LOGGER.info("open streamSCU"); - streamSCU.open(); - } + streamSCU.prepareTransfer(destination.getStreamSCUService(), p.getIuid(), cuid, dstTsuid); return streamSCU; } @@ -256,7 +216,7 @@ public List transfer( launchCStore(p, streamSCU, dataWriter, cuid, iuid, syntax, transformedPlanarImage); progressNotify( - destination, p.getIuid(), p.getCuid(), false, streamSCU.getNumberOfSuboperations()); + destination, p.getIuid(), p.getCuid(), false, streamSCU); monitor( sourceNode.getId(), destination.getId(), @@ -266,7 +226,7 @@ public List transfer( null); } catch (AbortException e) { progressNotify( - destination, p.getIuid(), p.getCuid(), true, streamSCU.getNumberOfSuboperations()); + destination, p.getIuid(), p.getCuid(), true, streamSCU); monitor( sourceNode.getId(), destination.getId(), @@ -279,7 +239,7 @@ public List transfer( } } catch (IOException e) { progressNotify( - destination, p.getIuid(), p.getCuid(), true, streamSCU.getNumberOfSuboperations()); + destination, p.getIuid(), p.getCuid(), true, streamSCU); monitor( sourceNode.getId(), destination.getId(), @@ -293,7 +253,7 @@ public List transfer( Thread.currentThread().interrupt(); } progressNotify( - destination, p.getIuid(), p.getCuid(), true, streamSCU.getNumberOfSuboperations()); + destination, p.getIuid(), p.getCuid(), true, streamSCU); monitor( sourceNode.getId(), destination.getId(), @@ -433,12 +393,12 @@ public void transferOther( launchCStore(p, streamSCU, dataWriter, cuid, iuid, syntax, transformedPlanarImage); progressNotify( - destination, p.getIuid(), p.getCuid(), false, streamSCU.getNumberOfSuboperations()); + destination, p.getIuid(), p.getCuid(), false, streamSCU); monitor( fwdNode.getId(), destination.getId(), attributesOriginal, attributesToSend, true, null); } catch (AbortException e) { progressNotify( - destination, p.getIuid(), p.getCuid(), true, streamSCU.getNumberOfSuboperations()); + destination, p.getIuid(), p.getCuid(), true, streamSCU); monitor( fwdNode.getId(), destination.getId(), @@ -451,7 +411,7 @@ public void transferOther( } } catch (IOException e) { progressNotify( - destination, p.getIuid(), p.getCuid(), true, streamSCU.getNumberOfSuboperations()); + destination, p.getIuid(), p.getCuid(), true, streamSCU); monitor( fwdNode.getId(), destination.getId(), @@ -465,7 +425,7 @@ public void transferOther( Thread.currentThread().interrupt(); } progressNotify( - destination, p.getIuid(), p.getCuid(), true, streamSCU.getNumberOfSuboperations()); + destination, p.getIuid(), p.getCuid(), true, streamSCU); monitor( fwdNode.getId(), destination.getId(), @@ -685,6 +645,18 @@ public void transferOther( LOGGER.error(ERROR_WHEN_FORWARDING, e); } } + private static void progressNotify( + ForwardDestination destination, String iuid, String cuid, boolean failed, StoreFromStreamSCU streamSCU) { + streamSCU.removeIUIDProcessed(iuid); + ServiceUtil.notifyProgession( + destination.getState(), + iuid, + cuid, + failed ? Status.ProcessingFailure : Status.Success, + failed ? ProgressStatus.FAILED : ProgressStatus.COMPLETED, + streamSCU.getNumberOfSuboperations()); + } + private static void progressNotify( ForwardDestination destination, String iuid, String cuid, boolean failed, int subOperations) { From 3973c37d3c10b1b0e33a2b3190fb17fa236ab120 Mon Sep 17 00:00:00 2001 From: jdcs Date: Tue, 14 Feb 2023 17:06:59 +0100 Subject: [PATCH 5/6] feat: change pom version --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 6eafa6f8..9d4936f3 100644 --- a/pom.xml +++ b/pom.xml @@ -16,7 +16,7 @@ - 1.0.57 + 1.0.58 jar From 04c1fa67880a4744d355592d217a8819e708fb55 Mon Sep 17 00:00:00 2001 From: jdcs Date: Tue, 14 Feb 2023 17:46:55 +0100 Subject: [PATCH 6/6] feat: fix following unit tests --- .../backend/service/ForwardService.java | 80 ++++++++++--------- 1 file changed, 42 insertions(+), 38 deletions(-) diff --git a/src/main/java/org/karnak/backend/service/ForwardService.java b/src/main/java/org/karnak/backend/service/ForwardService.java index b187de69..18082f3e 100644 --- a/src/main/java/org/karnak/backend/service/ForwardService.java +++ b/src/main/java/org/karnak/backend/service/ForwardService.java @@ -15,7 +15,6 @@ import java.util.ArrayList; import java.util.List; import java.util.Set; -import java.util.stream.IntStream; import org.dcm4che3.data.Attributes; import org.dcm4che3.data.Tag; import org.dcm4che3.data.UID; @@ -74,7 +73,8 @@ public ForwardService(final ApplicationEventPublisher applicationEventPublisher) } public void storeMultipleDestination( - ForwardDicomNode forwardNode, List destinations, Params p) throws IOException { + ForwardDicomNode forwardNode, List destinations, Params p) + throws IOException { if (destinations == null || destinations.isEmpty()) { throw new IllegalStateException( "Cannot find destinations from " + forwardNode.toString()); @@ -89,10 +89,15 @@ public void storeMultipleDestination( Attributes attributes = new Attributes(); List files = new ArrayList<>(); try { - IntStream.range(0, destinations.size()).forEachOrdered(i -> - prepareAndTransfer(forwardNode, p, i, destinations.get(i), attributes, destinations.size(), files)); - } - finally { + int nbDestinations = destinations.size(); + for (int i = 0; i < nbDestinations; i++) { + prepareAndTransfer(forwardNode, p, i, destinations.get(i), attributes, + nbDestinations, files); + } + } catch (IOException e) { + LOGGER.error("Cannot connect to the final destination", e); + throw e; + } finally { if (!files.isEmpty()) { // Force to clean if tmp bulk files for (File file : files) { @@ -111,36 +116,33 @@ public void storeMultipleDestination( * @param destination Destination * @param attributes Attributes * @param nbDestinations Number of destinations to handle - * @param files Temp files to delete + * @param files Temp files to delete */ private void prepareAndTransfer(ForwardDicomNode fwdNode, Params p, int index, - ForwardDestination destination, Attributes attributes, int nbDestinations, List files) { - try { + ForwardDestination destination, Attributes attributes, int nbDestinations, List files) + throws IOException { + if (destination instanceof DicomForwardDestination) { + // Prepare transfer only for dicom destination + prepareTransfer((DicomForwardDestination) destination, p); + } + if (index == 0) { + // Case first iteration: handle first destination of the forward node + Attributes attToApply = nbDestinations > 1 ? attributes : null; + if (destination instanceof DicomForwardDestination) { + files.addAll(transfer(fwdNode, (DicomForwardDestination) destination, attToApply, p)); + } else if (destination instanceof WebForwardDestination) { + files.addAll(transfer(fwdNode, (WebForwardDestination) destination, attToApply, p)); + } + } else { + // Case other iterations: handle other destinations of the forward node + if (!attributes.isEmpty()) { if (destination instanceof DicomForwardDestination) { - // Prepare transfer only for dicom destination - prepareTransfer((DicomForwardDestination) destination, p); - } - if (index == 0) { - // Case first iteration: handle first destination of the forward node - Attributes attToApply = nbDestinations > 1 ? attributes : null; - if (destination instanceof DicomForwardDestination) { - files.addAll(transfer(fwdNode, (DicomForwardDestination) destination, attToApply, p)); - } else if (destination instanceof WebForwardDestination) { - files.addAll(transfer(fwdNode, (WebForwardDestination) destination, attToApply, p)); - } - } else { - // Case other iterations: handle other destinations of the forward node - if (!attributes.isEmpty()) { - if (destination instanceof DicomForwardDestination) { - transferOther(fwdNode, (DicomForwardDestination) destination, attributes, p); - } else if (destination instanceof WebForwardDestination) { - transferOther(fwdNode, (WebForwardDestination) destination, attributes, p); - } - } + transferOther(fwdNode, (DicomForwardDestination) destination, attributes, p); + } else if (destination instanceof WebForwardDestination) { + transferOther(fwdNode, (WebForwardDestination) destination, attributes, p); } - } catch (Exception e) { - LOGGER.error("Cannot connect to the final destination", e); } + } } public static StoreFromStreamSCU prepareTransfer( @@ -645,16 +647,18 @@ public void transferOther( LOGGER.error(ERROR_WHEN_FORWARDING, e); } } + private static void progressNotify( - ForwardDestination destination, String iuid, String cuid, boolean failed, StoreFromStreamSCU streamSCU) { + ForwardDestination destination, String iuid, String cuid, boolean failed, + StoreFromStreamSCU streamSCU) { streamSCU.removeIUIDProcessed(iuid); ServiceUtil.notifyProgession( - destination.getState(), - iuid, - cuid, - failed ? Status.ProcessingFailure : Status.Success, - failed ? ProgressStatus.FAILED : ProgressStatus.COMPLETED, - streamSCU.getNumberOfSuboperations()); + destination.getState(), + iuid, + cuid, + failed ? Status.ProcessingFailure : Status.Success, + failed ? ProgressStatus.FAILED : ProgressStatus.COMPLETED, + streamSCU.getNumberOfSuboperations()); }