Improve audit properties handling to be hibernate comapttible (#2140)

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

hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaManagementHelper.java

@@ -90,7 +90,7 @@ public final class JpaManagementHelper {
         // log written because modifying e.g. metadata is modifying the base
         // entity itself for auditing purposes.
         final J result = entityManager.merge(entity);
-        result.setLastModifiedAt(0L);
+        result.setLastModifiedAt(1);
 
         return repository.save(result);
     }

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

@@ -42,8 +42,6 @@ import org.springframework.security.core.context.SecurityContextHolder;
  * Base hawkBit entity class containing the common attributes.
  */
 @NoArgsConstructor(access = AccessLevel.PROTECTED) // Default constructor needed for JPA entities.
-@Setter
-@Getter
 @MappedSuperclass
 @EntityListeners({ AuditingEntityListener.class, EntityInterceptorListener.class })
 public abstract class AbstractJpaBaseEntity implements BaseEntity {
@@ -53,17 +51,21 @@ public abstract class AbstractJpaBaseEntity implements BaseEntity {
     @Serial
     private static final long serialVersionUID = 1L;
 
+    @Setter // should be used just for test purposes
+    @Getter
     @Id
     @GeneratedValue(strategy = GenerationType.IDENTITY)
     @Column(name = "id")
     private Long id;
 
+    @Setter // should be used just for test purposes
+    @Getter
     @Version
     @Column(name = "optlock_revision")
     private int optLockRevision;
 
     // Audit fields. use property access to ensure that setters will be called and checked for modification
-    // (touch implementation depends on setLastModifiedAt(0).
+    // (touch implementation depends on setLastModifiedAt(1).
     @Column(name = "created_by", updatable = false, nullable = false, length = USERNAME_FIELD_LENGTH)
     private String createdBy;
     @Column(name = "created_at", updatable = false, nullable = false)
@@ -75,15 +77,6 @@ public abstract class AbstractJpaBaseEntity implements BaseEntity {
 
     @CreatedBy
     public void setCreatedBy(final String createdBy) {
-        if (isController()) {
-            this.createdBy = "CONTROLLER_PLUG_AND_PLAY";
-
-            // In general modification audit entry is not changed by the controller.
-            // However, we want to stay consistent with EnableJpaAuditing#modifyOnCreate=true.
-            this.lastModifiedBy = this.createdBy;
-            return;
-        }
-
         this.createdBy = createdBy;
     }
 
@@ -96,15 +89,9 @@ public abstract class AbstractJpaBaseEntity implements BaseEntity {
     @CreatedDate
     public void setCreatedAt(final long createdAt) {
         this.createdAt = createdAt;
-
-        // In general modification audit entry is not changed by the controller.
-        // However, we want to stay consistent with EnableJpaAuditing#modifyOnCreate=true.
-        if (isController()) {
-            this.lastModifiedAt = createdAt;
-        }
     }
 
-    @Column(name = "created_at", updatable = false, nullable = false)
+    // property access to make entity manager to detect touch
     @Access(AccessType.PROPERTY)
     public long getCreatedAt() {
         return createdAt;
@@ -112,22 +99,24 @@ public abstract class AbstractJpaBaseEntity implements BaseEntity {
 
     @LastModifiedBy
     public void setLastModifiedBy(final String lastModifiedBy) {
-        if (isController()) {
+        if (lastModifiedBy != null && isController()) {
+            // initialized and controller = doesn't update
             return;
         }
 
         this.lastModifiedBy = lastModifiedBy;
     }
 
-    @Column(name = "last_modified_by", nullable = false, length = USERNAME_FIELD_LENGTH)
+    // property access to make entity manager to detect touch
     @Access(AccessType.PROPERTY)
     public String getLastModifiedBy() {
-        return lastModifiedBy;
+        return lastModifiedBy == null ? createdBy : lastModifiedBy;
     }
 
     @LastModifiedDate
     public void setLastModifiedAt(final long lastModifiedAt) {
-        if (isController()) {
+        if (lastModifiedAt != 0 && isController()) {
+            // initialized and controller = doesn't update
             return;
         }
 
@@ -137,7 +126,7 @@ public abstract class AbstractJpaBaseEntity implements BaseEntity {
     // property access to make entity manager to detect touch
     @Access(AccessType.PROPERTY)
     public long getLastModifiedAt() {
-        return lastModifiedAt;
+        return lastModifiedAt == 0 ? createdAt : lastModifiedAt;
     }
 
     /**

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

@@ -10,7 +10,6 @@
 package org.eclipse.hawkbit.repository.jpa.model;
 
 import java.io.Serial;
-import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 

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

@@ -104,7 +104,7 @@ class ControllerManagementTest extends AbstractJpaIntegrationTest {
     @ExpectEvents({
             @Expect(type = TargetCreatedEvent.class, count = 1),
             @Expect(type = TargetUpdatedEvent.class, count = 2) })
-    public void updateTargetAttributesFailsIfTooManyEntries() throws Exception {
+    void updateTargetAttributesFailsIfTooManyEntries() throws Exception {
         final String controllerId = "test123";
         final int allowedAttributes = quotaManagement.getMaxAttributeEntriesPerTarget();
         testdataFactory.createTarget(controllerId);
@@ -139,7 +139,7 @@ class ControllerManagementTest extends AbstractJpaIntegrationTest {
 
     @Test
     @Description("Checks if invalid values of attribute-key and attribute-value are handled correctly")
-    public void updateTargetAttributesFailsForInvalidAttributes() {
+    void updateTargetAttributesFailsForInvalidAttributes() {
         final String controllerId = "targetId123";
         testdataFactory.createTarget(controllerId);
 
@@ -175,22 +175,23 @@ class ControllerManagementTest extends AbstractJpaIntegrationTest {
             @Expect(type = ActionCreatedEvent.class, count = 1),
             @Expect(type = TargetUpdatedEvent.class, count = 1),
             @Expect(type = TargetAssignDistributionSetEvent.class, count = 1) })
-    public void controllerProvidesIntermediateFeedbackFailsIfQuotaHit() {
+    void controllerProvidesIntermediateFeedbackFailsIfQuotaHit() throws Exception {
         final int allowStatusEntries = 10;
         final Long actionId = createTargetAndAssignDs();
 
-        // Fails as one entry is already in there from the assignment
+        SecurityContextSwitch
-        assertThatExceptionOfType(AssignmentQuotaExceededException.class)
+                .runAs(SecurityContextSwitch.withController("controller", CONTROLLER_ROLE_ANONYMOUS), () -> {
-                .isThrownBy(() -> SecurityContextSwitch
+                    // Fails as one entry is already in there from the assignment
-                        .runAs(SecurityContextSwitch.withController("controller", CONTROLLER_ROLE_ANONYMOUS), () -> {
+                    assertThatExceptionOfType(AssignmentQuotaExceededException.class)
-                            writeStatus(actionId, allowStatusEntries);
+                            .isThrownBy(() -> writeStatus(actionId, allowStatusEntries))
-                            return null;
+                            .withMessageContaining(String.valueOf(allowStatusEntries));
-                        })).withMessageContaining("" + allowStatusEntries);
+                    return null;
+                });
     }
 
     @Test
     @Description("Test to verify the storage and retrieval of action history.")
-    public void findMessagesByActionStatusId() {
+    void findMessagesByActionStatusId() {
         final DistributionSet testDs = testdataFactory.createDistributionSet("1");
         final List<Target> testTarget = testdataFactory.createTargets(1);
 
@@ -225,7 +226,7 @@ class ControllerManagementTest extends AbstractJpaIntegrationTest {
             @Expect(type = ActionCreatedEvent.class, count = 2),
             @Expect(type = TargetUpdatedEvent.class, count = 2),
             @Expect(type = TargetAssignDistributionSetEvent.class, count = 2) })
-    public void addActionStatusUpdatesUntilQuotaIsExceeded() {
+    void addActionStatusUpdatesUntilQuotaIsExceeded() {
 
         // any distribution set assignment causes 1 status entity to be created
         final int maxStatusEntries = quotaManagement.getMaxStatusEntriesPerAction() - 1;
@@ -265,7 +266,7 @@ class ControllerManagementTest extends AbstractJpaIntegrationTest {
             @Expect(type = ActionCreatedEvent.class, count = 1),
             @Expect(type = TargetUpdatedEvent.class, count = 1),
             @Expect(type = TargetAssignDistributionSetEvent.class, count = 1) })
-    public void createActionStatusWithTooManyMessages() {
+    void createActionStatusWithTooManyMessages() {
 
         final int maxMessages = quotaManagement.getMaxMessagesPerActionStatus();
 
@@ -297,7 +298,7 @@ class ControllerManagementTest extends AbstractJpaIntegrationTest {
             @Expect(type = ActionCreatedEvent.class, count = 1),
             @Expect(type = TargetUpdatedEvent.class, count = 1),
             @Expect(type = TargetAssignDistributionSetEvent.class, count = 1) })
-    public void controllerReportsDownloadForDownloadOnlyAction() {
+    void controllerReportsDownloadForDownloadOnlyAction() {
         testdataFactory.createTarget();
         final Long actionId = createAndAssignDsAsDownloadOnly("downloadOnlyDs", DEFAULT_CONTROLLER_ID);
         assertThat(actionId).isNotNull();
@@ -932,8 +933,7 @@ class ControllerManagementTest extends AbstractJpaIntegrationTest {
         testdataFactory.addSoftwareModuleMetadata(set);
 
         final Map<Long, List<SoftwareModuleMetadata>> result = controllerManagement
-                .findTargetVisibleMetaDataBySoftwareModuleId(
+                .findTargetVisibleMetaDataBySoftwareModuleId(set.getModules().stream().map(SoftwareModule::getId).toList());
-                        set.getModules().stream().map(SoftwareModule::getId).collect(Collectors.toList()));
 
         assertThat(result).hasSize(3);
         result.forEach((key, value) -> assertThat(value).hasSize(1));