[#1778] Prevent deletion of Software Module of locked DS (#1793)

Signed-off-by: Marinov Avgustin <Avgustin.Marinov@bosch.com>

hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/exception/LockedException.java

@@ -31,4 +31,11 @@ public class LockedException extends AbstractServerRtException {
                         " is forbidden!",
                 THIS_ERROR);
     }
+
+    public LockedException(
+            final Class<? extends BaseEntity> type, final Object entityId, final String operation, final String reason) {
+        super(type.getSimpleName() + " with given identifier {" + entityId + "} is locked and " + operation +
+                        " is forbidden! Reason: " + reason,
+                THIS_ERROR);
+    }
 }

hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/model/JpaSoftwareModule.java

@@ -35,6 +35,7 @@ import jakarta.validation.constraints.Size;
 import lombok.AccessLevel;
 import lombok.Getter;
 import lombok.NoArgsConstructor;
+import lombok.Setter;
 import lombok.ToString;
 import org.eclipse.hawkbit.repository.event.remote.SoftwareModuleDeletedEvent;
 import org.eclipse.hawkbit.repository.event.remote.entity.SoftwareModuleCreatedEvent;
@@ -75,20 +76,22 @@ public class JpaSoftwareModule extends AbstractJpaNamedVersionedEntity implement
 
     private static final String DELETED_PROPERTY = "deleted";
 
+    @Setter
     @ManyToOne
     @JoinColumn(name = "module_type", nullable = false, updatable = false, foreignKey = @ForeignKey(value = ConstraintMode.CONSTRAINT, name = "fk_module_type"))
     @NotNull
     private JpaSoftwareModuleType type;
 
     @CascadeOnDelete
-    @OneToMany(fetch = FetchType.LAZY, mappedBy = "softwareModule", cascade = {
+    @OneToMany(fetch = FetchType.LAZY, mappedBy = "softwareModule", cascade = { CascadeType.PERSIST }, targetEntity = JpaArtifact.class, orphanRemoval = true)
-            CascadeType.PERSIST }, targetEntity = JpaArtifact.class, orphanRemoval = true)
     private List<JpaArtifact> artifacts;
 
-    @Column(name = "vendor", nullable = true, length = SoftwareModule.VENDOR_MAX_SIZE)
+    @Setter
+    @Column(name = "vendor", length = SoftwareModule.VENDOR_MAX_SIZE)
     @Size(max = SoftwareModule.VENDOR_MAX_SIZE)
     private String vendor;
 
+    @Setter
     @Column(name = "encrypted")
     private boolean encrypted;
 
@@ -127,17 +130,9 @@ public class JpaSoftwareModule extends AbstractJpaNamedVersionedEntity implement
         this.encrypted = encrypted;
     }
 
-    public void setType(final JpaSoftwareModuleType type) {
-        this.type = type;
-    }
-
     @Override
     public List<Artifact> getArtifacts() {
-        if (artifacts == null) {
+        return artifacts == null ? Collections.emptyList() : Collections.unmodifiableList(artifacts);
-            return Collections.emptyList();
-        }
-
-        return Collections.unmodifiableList(artifacts);
     }
 
     public void addArtifact(final Artifact artifact) {
@@ -145,14 +140,15 @@ public class JpaSoftwareModule extends AbstractJpaNamedVersionedEntity implement
             throw new LockedException(JpaSoftwareModule.class, getId(), "ADD_ARTIFACT");
         }
 
-        if (artifacts == null) {
+        if (artifact instanceof JpaArtifact jpaArtifact) {
-            artifacts = new ArrayList<>(4);
+            if (artifacts == null) {
-            artifacts.add((JpaArtifact) artifact);
+                artifacts = new ArrayList<>(4);
-            return;
+                artifacts.add(jpaArtifact);
-        }
+            } else if (!artifacts.contains(jpaArtifact)) {
-
+                artifacts.add(jpaArtifact);
-        if (!artifacts.contains(artifact)) {
+            }
-            artifacts.add((JpaArtifact) artifact);
+        } else {
+            throw new UnsupportedOperationException("Only JpaArtifact is supported");
         }
     }
 
@@ -169,10 +165,6 @@ public class JpaSoftwareModule extends AbstractJpaNamedVersionedEntity implement
         }
     }
 
-    public void setVendor(final String vendor) {
-        this.vendor = vendor;
-    }
-
     public void lock() {
         locked = true;
     }
@@ -181,28 +173,29 @@ public class JpaSoftwareModule extends AbstractJpaNamedVersionedEntity implement
         locked = false;
     }
 
-    /**
-     * Marks or un-marks this software module as deleted.
-     * 
-     * @param deleted
-     *            {@code true} if the software module should be marked as deleted
-     *            otherwise {@code false}
-     */
     public void setDeleted(final boolean deleted) {
+        if (assignedTo != null) {
+            final List<DistributionSet> lockedDS = assignedTo.stream()
+                    .filter(DistributionSet::isLocked)
+                    .filter(ds -> !ds.isDeleted())
+                    .toList();
+            if (!lockedDS.isEmpty()) {
+                final StringBuilder sb = new StringBuilder("Part of ");
+                if (lockedDS.size() == 1) {
+                    sb.append("a locked distribution set: ");
+                } else {
+                    sb.append(lockedDS.size()).append(" locked distribution sets: ");
+                }
+                for (final DistributionSet ds : lockedDS) {
+                    sb.append(ds.getName()).append(":").append(ds.getVersion()).append(" (").append(ds.getId()).append("), ");
+                }
+                sb.delete(sb.length() - 2, sb.length());
+                throw new LockedException(JpaSoftwareModule.class, getId(), "DELETE", sb.toString());
+            };
+        }
         this.deleted = deleted;
     }
 
-    /**
-     * Marks this software module as encrypted.
-     * 
-     * @param encrypted
-     *            {@code true} if the software module should be marked as encrypted
-     *            otherwise {@code false}
-     */
-    public void setEncrypted(final boolean encrypted) {
-        this.encrypted = encrypted;
-    }
-
     @Override
     public void fireCreateEvent(final DescriptorEvent descriptorEvent) {
         EventPublisherHolder.getInstance().getEventPublisher().publishEvent(

hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/AbstractJpaIntegrationTest.java

@@ -231,10 +231,12 @@ public abstract class AbstractJpaIntegrationTest extends AbstractIntegrationTest
         return array;
     }
 
+    // just increase the opt lock revision if the instance in order to match it against locked db instance - not really locking
     protected static void implicitLock(final DistributionSet set) {
         ((JpaDistributionSet) set).setOptLockRevision(set.getOptLockRevision() + 1);
     }
 
+    // just increase the opt lock revision if the instance in order to match it against locked db instance - not really locking
     protected static void implicitLock(final SoftwareModule module) {
         ((JpaSoftwareModule) module).setOptLockRevision(module.getOptLockRevision() + 1);
     }

hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/SoftwareModuleManagementTest.java

@@ -212,6 +212,7 @@ public class SoftwareModuleManagementTest extends AbstractJpaIntegrationTest {
                 .first().isEqualTo(ah);
         assertThat(softwareModuleManagement.findByTextAndType(PAGE, ":1.0", appType.getId()).getContent()).hasSize(2);
 
+        distributionSetManagement.unlock(ds.getId()); // otherwise delete will be rejected as a part of a locked DS
         softwareModuleManagement.delete(ah2.getId());
 
         assertThat(softwareModuleManagement.findByTextAndType(PAGE, ":1.0", appType.getId()).getContent()).hasSize(1);
@@ -424,8 +425,7 @@ public class SoftwareModuleManagementTest extends AbstractJpaIntegrationTest {
 
     @Test
     @Description("Delete two assigned softwaremodules which share an artifact.")
-    public void deleteMultipleSoftwareModulesWhichShareAnArtifact() throws IOException {
+    public void deleteMultipleSoftwareModulesWhichShareAnArtifact() {
-
         // Init artifact binary data, target and DistributionSets
         final int artifactSize = 1024;
         final byte[] source = RandomUtils.nextBytes(artifactSize);
@@ -456,6 +456,8 @@ public class SoftwareModuleManagementTest extends AbstractJpaIntegrationTest {
         assignDistributionSet(disSetY, Collections.singletonList(target));
 
         // [STEP5]: Delete SoftwareModuleX
+        distributionSetManagement.unlock(disSetX.getId()); // otherwise delete will be rejected as a part of a locked DS
+        distributionSetManagement.unlock(disSetY.getId()); // otherwise delete will be rejected as a part of a locked DS
         softwareModuleManagement.delete(moduleX.getId());
 
         // [STEP6]: Delete SoftwareModuleY
@@ -892,6 +894,27 @@ public class SoftwareModuleManagementTest extends AbstractJpaIntegrationTest {
                 .isPresent();
     }
 
+    @Test
+    @Description("Artifacts of a locked SM can't be modified. Expected behaviour is to throw an exception and to do not modify them.")
+    void lockedContainingDistributionSetApplied() {
+        final DistributionSet distributionSet = testdataFactory.createDistributionSet("ds-1");
+        final List<SoftwareModule> modules = distributionSet.getModules().stream().toList();
+        assertThat(modules.size()).isGreaterThan(1);
+
+        // try delete while DS is not locked
+        softwareModuleManagement.delete(modules.get(0).getId());
+
+        distributionSetManagement.lock(distributionSet.getId());
+        assertThat(
+                distributionSetManagement.get(distributionSet.getId()).map(DistributionSet::isLocked).orElse(false))
+                .isTrue();
+
+        // try delete SM of a locked DS
+        assertThatExceptionOfType(LockedException.class)
+                .as("Attempt to delete a software module of a locked DS should throw an exception")
+                .isThrownBy(() -> softwareModuleManagement.delete(modules.get(1).getId()));
+    }
+
     @Test
     @Description("Verifies that non existing metadata find results in exception.")
     public void findSoftwareModuleMetadataFailsIfEntryDoesNotExist() {