Fixed NPE in Precondition (#135)

The DescriptionModificationUptPrecondition (amongst others) threw a NullPointerException when encountering a DescriptionModificationReport containing a ReportPart without a ModificationType attribute. This should fix the problem. # Checklist The following aspects have been respected by the author of this pull request, confirmed by both pull request assignee **and** reviewer: * Adherence to coding conventions * [x] Pull Request Assignee * [x] Reviewer * Adherence to javadoc conventions * [x] Pull Request Assignee * [x] Reviewer * Changelog update (necessity checked and entry added or not added respectively) * [x] Pull Request Assignee * [x] Reviewer * README update (necessity checked and entry added or not added respectively) * [x] Pull Request Assignee * [x] Reviewer * config update (necessity checked and entry added or not added respectively) * [x] Pull Request Assignee * [x] Reviewer * SDCcc executable ran against a test device (if necessary) * [x] Pull Request Assignee * [x] Reviewer --------- Co-authored-by: Maximilian Pilz <[email protected]>
Draegerwerk · Feb 15, 2024 · c0057f4 · c0057f4
1 parent 2a55741
commit c0057f4
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 9 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - autocommit of the database being on, it is now always off
 - local address resolver sometimes giving a wrong address
 - local address resolver doing a probe which may not be tolerated by some peers
+- potential NullPointerException in DescriptionModificationUptPrecondition
 
 ## [8.0.1] - 2023-09-13
 

diff --git a/...va/com/draeger/medical/sdccc/manipulation/precondition/impl/ConditionalPreconditions.java b/...va/com/draeger/medical/sdccc/manipulation/precondition/impl/ConditionalPreconditions.java
@@ -1,6 +1,6 @@
 /*
  * This Source Code Form is subject to the terms of the MIT License.
- * Copyright (c) 2023 Draegerwerk AG & Co. KGaA.
+ * Copyright (c) 2023, 2024 Draegerwerk AG & Co. KGaA.
  *
  * SPDX-License-Identifier: MIT
  */
@@ -96,7 +96,7 @@ private static boolean descriptionModificationPreconditionCheck(
                             .orElseThrow(() -> new RuntimeException(
                                     "Could not retrieve description modification report body from message")))
                     .anyMatch(message -> message.getReportPart().stream()
-                            .map(DescriptionModificationReport.ReportPart::getModificationType)
+                            .map(ImpliedValueUtil::getModificationType)
                             .anyMatch(modificationTypesList::contains));
         } catch (final IOException e) {
             throw new PreconditionException(
@@ -683,7 +683,6 @@ static boolean preconditionCheck(final Injector injector) throws PreconditionExc
          *
          * @param injector to analyze mdib on
          * @return true if successful, false otherwise
-         * @throws PreconditionException on errors
          */
         static boolean manipulation(final Injector injector) {
             return descriptionModificationManipulation(injector, LOG);
@@ -745,7 +744,7 @@ static boolean preconditionCheck(final Injector injector) throws PreconditionExc
                                         "Could not retrieve episodic context report body from message")))
                         .forEach(message -> message.getReportPart().stream()
                                 .map(AbstractContextReport.ReportPart::getContextState)
-                                .collect(Collectors.toList())
+                                .toList()
                                 .forEach(contextStates::addAll));
                 for (var state : contextStates) {
                     if (ImpliedValueUtil.getContextAssociation(state) == ContextAssociation.ASSOC) {

diff --git a/...om/draeger/medical/sdccc/manipulation/precondition/impl/ConditionalPreconditionsTest.java b/...om/draeger/medical/sdccc/manipulation/precondition/impl/ConditionalPreconditionsTest.java
@@ -1,6 +1,6 @@
 /*
  * This Source Code Form is subject to the terms of the MIT License.
- * Copyright (c) 2023 Draegerwerk AG & Co. KGaA.
+ * Copyright (c) 2023, 2024 Draegerwerk AG & Co. KGaA.
  *
  * SPDX-License-Identifier: MIT
  */
@@ -223,6 +223,7 @@ void tearDown() throws TimeoutException {
      */
     @Test
     @DisplayName("HelloMessagePrecondition correctly checks for precondition")
+    @SuppressWarnings("resource")
     public void testHelloMessagePreconditionCheck() throws PreconditionException, IOException {
         final var mockStorage = mock(MessageStorage.class);
         final var mockMessage = mock(MessageContent.class);
@@ -515,6 +516,45 @@ public void testDescriptionModificationUptPreconditionCheck()
         assertTrue(ConditionalPreconditions.DescriptionModificationUptPrecondition.preconditionCheck(testInjector));
     }
 
+    /**
+     * Tests that DescriptionModificationUptPrecondition does not fail when a DescriptionModificationReport
+     * contains ReportParts without a ModificationType attribute.
+     *
+     * @throws PreconditionException on precondition exceptions
+     * @throws IOException           on io exceptions
+     * @throws JAXBException         on marshalling failures
+     */
+    @Test
+    @DisplayName("DescriptionModificationCrtPrecondition when @ModificationType is omitted")
+    public void testDescriptionModificationUptPreconditionCheckWhenModificationTypeIsNotSet()
+            throws PreconditionException, IOException, JAXBException {
+        // no messagess
+        assertFalse(ConditionalPreconditions.DescriptionModificationUptPrecondition.preconditionCheck(testInjector));
+
+        final var reportPartDel = messageBuilder.buildDescriptionModificationReportReportPart();
+
+        final var reportPartCrt = messageBuilder.buildDescriptionModificationReportReportPart();
+
+        final var firstReport = messageBuilder.buildDescriptionModificationReport(
+                "SomeSequence", List.of(reportPartDel, reportPartCrt));
+
+        final var messageWithDelReportPart = messageBuilder.createSoapMessageWithBody(
+                ActionConstants.ACTION_DESCRIPTION_MODIFICATION_REPORT, firstReport);
+
+        messageStorageUtil.addInboundSecureHttpMessage(storage, messageWithDelReportPart);
+        // no messages with report parts crt
+        final var reportPartNoModificationType = messageBuilder.buildDescriptionModificationReportReportPart();
+
+        final var secondReport = messageBuilder.buildDescriptionModificationReport(
+                "SomeSequence", List.of(reportPartNoModificationType));
+
+        final var messageWithUpdReportPart = messageBuilder.createSoapMessageWithBody(
+                ActionConstants.ACTION_DESCRIPTION_MODIFICATION_REPORT, secondReport);
+
+        messageStorageUtil.addInboundSecureHttpMessage(storage, messageWithUpdReportPart);
+        assertTrue(ConditionalPreconditions.DescriptionModificationUptPrecondition.preconditionCheck(testInjector));
+    }
+
     /**
      * Tests whether DescriptionModificationUptPrecondition correctly calls manipulation.
      */
@@ -1510,7 +1550,7 @@ public void testAllKindsOfContextStatesAssociatedPreconditionCheck() throws Exce
 
         final var report = messageBuilder.buildEpisodicContextReport("SomeSequence");
         report.getReportPart().clear();
-        report.getReportPart().addAll(List.of(reportPart));
+        report.getReportPart().add(reportPart);
 
         final var message =
                 messageBuilder.createSoapMessageWithBody(ActionConstants.ACTION_EPISODIC_CONTEXT_REPORT, report);
@@ -1566,7 +1606,7 @@ public void testAllKindsOfContextStatesAssociatedPreconditionCheck() throws Exce
                         workflowContextState, workflowContextState2));
         final var secondReport = messageBuilder.buildEpisodicContextReport("SomeSequence");
         secondReport.getReportPart().clear();
-        secondReport.getReportPart().addAll(List.of(secondReportPart));
+        secondReport.getReportPart().add(secondReportPart);
 
         final var secondMessage =
                 messageBuilder.createSoapMessageWithBody(ActionConstants.ACTION_EPISODIC_CONTEXT_REPORT, secondReport);
@@ -1621,7 +1661,7 @@ void testAssociateAllKindsOfContextsNotAllNeeded() throws Exception {
                 ConditionalPreconditions.AllKindsOfContextStatesAssociatedPrecondition.ALREADY_ASSOCIATED_CONTEXTS
                         .entrySet()) {
             if (entry.getKey().equals(PatientContextState.class)) {
-                entry.getValue().addAll(List.of(PATIENT_CONTEXT_STATE_HANDLE));
+                entry.getValue().add(PATIENT_CONTEXT_STATE_HANDLE);
             } else if (entry.getKey().equals(LocationContextState.class)) {
                 entry.getValue().addAll(List.of(LOCATION_CONTEXT_STATE_HANDLE, LOCATION_CONTEXT_STATE_HANDLE2));
             } else if (entry.getKey().equals(EnsembleContextState.class)) {
@@ -2150,6 +2190,7 @@ private void allKindsOfContextStatesAssociatedManipulationSucceeded() throws Exc
     @Test
     @DisplayName("Different TriggerReportPreconditions correctly check for precondition")
     public void testTriggerReportPreconditionCheck() throws IOException, PreconditionException {
+        @SuppressWarnings("resource")
         final var mockStorage = mock(MessageStorage.class);
         @SuppressWarnings("unchecked")
         final MessageStorage.GetterResult<MessageContent> mockGetter = mock(MessageStorage.GetterResult.class);
@@ -2454,8 +2495,8 @@ protected void configure() {
     @Test
     @DisplayName("StateChangedPrecondition correctly checks for preconditions")
     public void testStateChangedPreconditionCheck() throws Exception {
+        @SuppressWarnings("resource")
         final var mockStorage = mock(MessageStorage.class);
-        @SuppressWarnings("unchecked")
         final MessageStorage.GetterResult<MessageContent> mockGetter = mock(MessageStorage.GetterResult.class);
         {
             when(mockGetter.areObjectsPresent()).thenReturn(true).thenReturn(false);