Add support for native query for multiple JPA vendors (#2129)

Signed-off-by: Avgustin Marinov <[email protected]>

hawkbit-mgmt/hawkbit-mgmt-resource/src/test/java/org/eclipse/hawkbit/mgmt/rest/resource/AbstractManagementApiIntegrationTest.java

@@ -14,7 +14,7 @@
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
 
 import org.eclipse.hawkbit.mgmt.json.model.distributionset.MgmtActionType;
-import org.eclipse.hawkbit.repository.jpa.JpaConstants;
+import org.eclipse.hawkbit.repository.jpa.Jpa;
 import org.eclipse.hawkbit.repository.jpa.RepositoryApplicationConfiguration;
 import org.eclipse.hawkbit.repository.jpa.model.JpaDistributionSet;
 import org.eclipse.hawkbit.repository.model.BaseEntity;
@@ -205,7 +205,7 @@ static void implicitLock(final DistributionSet set) {
 
     // version is 1, 2 ... based
     protected int version(final int version) {
-        return switch (JpaConstants.JPA_VENDOR) {
+        return switch (Jpa.JPA_VENDOR) {
             case ECLIPSELINK -> version;
             case HIBERNATE -> version - 1;
         };

hawkbit-mgmt/hawkbit-mgmt-resource/src/test/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtDistributionSetTypeResourceTest.java

@@ -38,7 +38,6 @@
 import org.eclipse.hawkbit.mgmt.rest.api.MgmtRestConstants;
 import org.eclipse.hawkbit.repository.builder.SoftwareModuleTypeCreate;
 import org.eclipse.hawkbit.repository.exception.AssignmentQuotaExceededException;
-import org.eclipse.hawkbit.repository.jpa.JpaConstants;
 import org.eclipse.hawkbit.repository.model.DistributionSetType;
 import org.eclipse.hawkbit.repository.model.NamedEntity;
 import org.eclipse.hawkbit.repository.model.SoftwareModuleType;

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

@@ -0,0 +1,55 @@
+/**
+ * Copyright (c) 2024 Contributors to the Eclipse Foundation
+ *
+ * This program and the accompanying materials are made
+ * available under the terms of the Eclipse Public License 2.0
+ * which is available at https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ */
+package org.eclipse.hawkbit.repository.jpa;
+
+import java.util.Collection;
+import java.util.List;
+import java.util.stream.IntStream;
+
+import jakarta.persistence.Query;
+
+import lombok.NoArgsConstructor;
+
+@NoArgsConstructor(access = lombok.AccessLevel.PRIVATE)
+public class Jpa {
+
+    public enum JpaVendor {
+        ECLIPSELINK,
+        HIBERNATE // NOT SUPPORTED!
+    }
+
+    public static final JpaVendor JPA_VENDOR = JpaVendor.ECLIPSELINK;
+
+    public static char NATIVE_QUERY_PARAMETER_PREFIX = switch (JPA_VENDOR) {
+        case ECLIPSELINK -> '?';
+        case HIBERNATE -> ':';
+    };
+
+    public static <T> String formatNativeQueryInClause(final String name, final List<T> list) {
+        return switch (Jpa.JPA_VENDOR) {
+            case ECLIPSELINK -> formatEclipseLinkNativeQueryInClause(IntStream.range(0, list.size()).mapToObj(i -> name + "_" + i).toList());
+            case HIBERNATE -> ":" + name;
+        };
+    }
+
+    public static <T> void setNativeQueryInParameter(final Query deleteQuery, final String name, final List<T> list) {
+        if (Jpa.JPA_VENDOR == Jpa.JpaVendor.ECLIPSELINK) {
+            for (int i = 0, len = list.size(); i < len; i++) {
+                deleteQuery.setParameter(name + "_" + i, list.get(i));
+            }
+        } else if (Jpa.JPA_VENDOR == Jpa.JpaVendor.HIBERNATE) {
+            deleteQuery.setParameter(name, list);
+        }
+    }
+
+    private static String formatEclipseLinkNativeQueryInClause(final Collection<String> elements) {
+        return "?" + String.join(",?", elements);
+    }
+}

hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaDeploymentManagement.java

@@ -26,7 +26,6 @@
 import java.util.Set;
 import java.util.function.Function;
 import java.util.stream.Collectors;
-import java.util.stream.IntStream;
 
 import jakarta.persistence.EntityManager;
 import jakarta.persistence.Query;
@@ -53,6 +52,7 @@
 import org.eclipse.hawkbit.repository.exception.IncompleteDistributionSetException;
 import org.eclipse.hawkbit.repository.exception.InsufficientPermissionException;
 import org.eclipse.hawkbit.repository.exception.MultiAssignmentIsNotEnabledException;
+import org.eclipse.hawkbit.repository.jpa.Jpa;
 import org.eclipse.hawkbit.repository.jpa.JpaManagementHelper;
 import org.eclipse.hawkbit.repository.jpa.acm.AccessController;
 import org.eclipse.hawkbit.repository.jpa.configuration.Constants;
@@ -123,9 +123,29 @@ public class JpaDeploymentManagement extends JpaActionManagement implements Depl
      */
     private static final int ACTION_PAGE_LIMIT = 1000;
     private static final String QUERY_DELETE_ACTIONS_BY_STATE_AND_LAST_MODIFIED_DEFAULT =
-            "DELETE FROM sp_action WHERE tenant=#tenant AND status IN (%s) AND last_modified_at<#last_modified_at LIMIT " + ACTION_PAGE_LIMIT;
+            "DELETE FROM sp_action " +
+                    "WHERE tenant=" + Jpa.NATIVE_QUERY_PARAMETER_PREFIX + "tenant" +
+                    " AND status IN (%s)" +
+                    " AND last_modified_at<" + Jpa.NATIVE_QUERY_PARAMETER_PREFIX + "last_modified_at LIMIT " + ACTION_PAGE_LIMIT;
     private static final EnumMap<Database, String> QUERY_DELETE_ACTIONS_BY_STATE_AND_LAST_MODIFIED;
 
+    static {
+        QUERY_DELETE_ACTIONS_BY_STATE_AND_LAST_MODIFIED = new EnumMap<>(Database.class);
+        QUERY_DELETE_ACTIONS_BY_STATE_AND_LAST_MODIFIED.put(
+                Database.SQL_SERVER,
+                "DELETE TOP (" + ACTION_PAGE_LIMIT + ") FROM sp_action " +
+                        "WHERE tenant=" + Jpa.NATIVE_QUERY_PARAMETER_PREFIX + "tenant" +
+                        " AND status IN (%s)" +
+                        " AND last_modified_at<" + Jpa.NATIVE_QUERY_PARAMETER_PREFIX + "last_modified_at ");
+        QUERY_DELETE_ACTIONS_BY_STATE_AND_LAST_MODIFIED.put(
+                Database.POSTGRESQL,
+                "DELETE FROM sp_action " +
+                        "WHERE id IN (SELECT id FROM sp_action " +
+                        "WHERE tenant=" + Jpa.NATIVE_QUERY_PARAMETER_PREFIX + "tenant" +
+                        " AND status IN (%s)" +
+                        " AND last_modified_at<" + Jpa.NATIVE_QUERY_PARAMETER_PREFIX + "last_modified_at LIMIT " + ACTION_PAGE_LIMIT + ")");
+    }
+
     private final EntityManager entityManager;
     private final DistributionSetManagement distributionSetManagement;
     private final TargetRepository targetRepository;
@@ -141,18 +161,6 @@ public class JpaDeploymentManagement extends JpaActionManagement implements Depl
     private final Database database;
     private final RetryTemplate retryTemplate;
 
-    static {
-        QUERY_DELETE_ACTIONS_BY_STATE_AND_LAST_MODIFIED = new EnumMap<>(Database.class);
-        QUERY_DELETE_ACTIONS_BY_STATE_AND_LAST_MODIFIED.put(
-                Database.SQL_SERVER,
-                "DELETE TOP (" + ACTION_PAGE_LIMIT + ") FROM sp_action " +
-                        "WHERE tenant=#tenant AND status IN (%s) AND last_modified_at<#last_modified_at ");
-        QUERY_DELETE_ACTIONS_BY_STATE_AND_LAST_MODIFIED.put(
-                Database.POSTGRESQL,
-                "DELETE FROM sp_action " +
-                        "WHERE id IN (SELECT id FROM sp_action WHERE tenant=#tenant AND status IN (%s) AND last_modified_at<#last_modified_at LIMIT " + ACTION_PAGE_LIMIT + ")");
-    }
-
     public JpaDeploymentManagement(
             final EntityManager entityManager, final ActionRepository actionRepository,
             final DistributionSetManagement distributionSetManagement, final TargetRepository targetRepository,
@@ -500,23 +508,18 @@ public int deleteActionsByStatusAndLastModifiedBefore(final Set<Status> status,
         if (status.isEmpty()) {
             return 0;
         }
-        /*
-         * We use a native query here because Spring JPA does not support to specify a
-         * LIMIT clause on a DELETE statement. However, for this specific use case
-         * (action cleanup), we must specify a row limit to reduce the overall load on
-         * the database.
-         */
-
-        final int statusCount = status.size();
-        final Status[] statusArr = status.toArray(new Status[statusCount]);
-
-        final String queryStr = String.format(getQueryForDeleteActionsByStatusAndLastModifiedBeforeString(database),
-                formatInClauseWithNumberKeys(statusCount));
-        final Query deleteQuery = entityManager.createNativeQuery(queryStr);
-
-        IntStream.range(0, statusCount)
-                .forEach(i -> deleteQuery.setParameter(String.valueOf(i), statusArr[i].ordinal()));
+
+        // We use a native query here because Spring JPA does not support to specify a LIMIT clause on a DELETE statement.
+        // However, for this specific use case (action cleanup), we must specify a row limit to reduce the overall load of
+        // the database.
+        final List<Integer> statusList = status.stream().map(Status::ordinal).toList();
+
+        final Query deleteQuery = entityManager.createNativeQuery(String.format(
+                getQueryForDeleteActionsByStatusAndLastModifiedBeforeString(database),
+                Jpa.formatNativeQueryInClause("status", statusList)));
+
         deleteQuery.setParameter("tenant", tenantAware.getCurrentTenant().toUpperCase());
+        Jpa.setNativeQueryInParameter(deleteQuery, "status", statusList);
         deleteQuery.setParameter("last_modified_at", lastModified);
 
         log.debug("Action cleanup: Executing the following (native) query: {}", deleteQuery);
@@ -600,14 +603,6 @@ private static String getQueryForDeleteActionsByStatusAndLastModifiedBeforeStrin
                 QUERY_DELETE_ACTIONS_BY_STATE_AND_LAST_MODIFIED_DEFAULT);
     }
 
-    private static String formatInClauseWithNumberKeys(final int count) {
-        return formatInClause(IntStream.range(0, count).mapToObj(String::valueOf).collect(Collectors.toList()));
-    }
-
-    private static String formatInClause(final Collection<String> elements) {
-        return "#" + String.join(",#", elements);
-    }
-
     private static RetryTemplate createRetryTemplate() {
         final RetryTemplate template = new RetryTemplate();
 

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

@@ -150,7 +150,7 @@ public String toString() {
 
     private TypedQuery<T> withEntityGraph(final TypedQuery<T> query, final String entityGraph) {
         final EntityGraph<?> graph = ObjectUtils.isEmpty(entityGraph) ? null : entityManager.createEntityGraph(entityGraph);
-        return graph == null ? query : query.setHint("javax.persistence.loadgraph", graph);
+        return graph == null ? query : query.setHint("jakarta.persistence.loadgraph", graph);
     }
 
     private <S extends T> Page<S> readPageWithoutCount(final TypedQuery<S> query, final Pageable pageable) {

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

@@ -246,7 +246,7 @@ protected JpaRolloutGroup refresh(final RolloutGroup group) {
 
     // version is 1, 2 ... based
     protected int version(final int version) {
-        return switch (JpaConstants.JPA_VENDOR) {
+        return switch (Jpa.JPA_VENDOR) {
             case ECLIPSELINK -> version;
             case HIBERNATE -> version - 1;
         };

hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/autocleanup/AutoActionCleanupTest.java

@@ -33,15 +33,14 @@
  */
 @Feature("Component Tests - Repository")
 @Story("Action cleanup handler")
-public class AutoActionCleanupTest extends AbstractJpaIntegrationTest {
+class AutoActionCleanupTest extends AbstractJpaIntegrationTest {
 
     @Autowired
     private AutoActionCleanup autoActionCleanup;
 
     @Test
     @Description("Verifies that running actions are not cleaned up.")
-    public void runningActionsAreNotCleanedUp() {
-
+    void runningActionsAreNotCleanedUp() {
         // cleanup config for this test case
         setupCleanupConfiguration(true, 0, Action.Status.CANCELED, Action.Status.ERROR);
 
@@ -60,13 +59,11 @@ public void runningActionsAreNotCleanedUp() {
         autoActionCleanup.run();
 
         assertThat(actionRepository.count()).isEqualTo(2);
-
     }
 
     @Test
     @Description("Verifies that nothing is cleaned up if the cleanup is disabled.")
-    public void cleanupDisabled() {
-
+    void cleanupDisabled() {
         // cleanup config for this test case
         setupCleanupConfiguration(false, 0, Action.Status.CANCELED);
 
@@ -87,13 +84,11 @@ public void cleanupDisabled() {
         autoActionCleanup.run();
 
         assertThat(actionRepository.count()).isEqualTo(2);
-
     }
 
     @Test
     @Description("Verifies that canceled and failed actions are cleaned up.")
-    public void canceledAndFailedActionsAreCleanedUp() {
-
+    void canceledAndFailedActionsAreCleanedUp() {
         // cleanup config for this test case
         setupCleanupConfiguration(true, 0, Action.Status.CANCELED, Action.Status.ERROR);
 
@@ -120,13 +115,11 @@ public void canceledAndFailedActionsAreCleanedUp() {
 
         assertThat(actionRepository.count()).isEqualTo(1);
         assertThat(actionRepository.findWithDetailsById(action3)).isPresent();
-
     }
 
     @Test
     @Description("Verifies that canceled actions are cleaned up.")
-    public void canceledActionsAreCleanedUp() {
-
+    void canceledActionsAreCleanedUp() {
         // cleanup config for this test case
         setupCleanupConfiguration(true, 0, Action.Status.CANCELED);
 
@@ -154,14 +147,12 @@ public void canceledActionsAreCleanedUp() {
         assertThat(actionRepository.count()).isEqualTo(2);
         assertThat(actionRepository.findWithDetailsById(action2)).isPresent();
         assertThat(actionRepository.findWithDetailsById(action3)).isPresent();
-
     }
 
     @Test
     @Description("Verifies that canceled and failed actions are cleaned up once they expired.")
     @SuppressWarnings("squid:S2925")
-    public void canceledAndFailedActionsAreCleanedUpWhenExpired() throws InterruptedException {
-
+    void canceledAndFailedActionsAreCleanedUpWhenExpired() throws InterruptedException {
         // cleanup config for this test case
         setupCleanupConfiguration(true, 500, Action.Status.CANCELED, Action.Status.ERROR);
 
@@ -194,7 +185,6 @@ public void canceledAndFailedActionsAreCleanedUpWhenExpired() throws Interrupted
 
         assertThat(actionRepository.count()).isEqualTo(1);
         assertThat(actionRepository.findWithDetailsById(action3)).isPresent();
-
     }
 
     private void setActionToCanceled(final Long id) {
@@ -209,7 +199,8 @@ private void setActionToFailed(final Long id) {
     private void setupCleanupConfiguration(final boolean cleanupEnabled, final long expiry, final Status... status) {
         tenantConfigurationManagement.addOrUpdateConfiguration(ACTION_CLEANUP_ENABLED, cleanupEnabled);
         tenantConfigurationManagement.addOrUpdateConfiguration(ACTION_CLEANUP_ACTION_EXPIRY, expiry);
-        tenantConfigurationManagement.addOrUpdateConfiguration(ACTION_CLEANUP_ACTION_STATUS,
+        tenantConfigurationManagement.addOrUpdateConfiguration(
+                ACTION_CLEANUP_ACTION_STATUS,
                 Arrays.stream(status).map(Status::toString).collect(Collectors.joining(",")));
     }
-}
+}

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

@@ -160,8 +160,7 @@ void findAllTargetsOfRolloutGroupWithActionStatusConsidersSortingByLastActionSta
     @Description("Verifies that Rollouts in different states are handled correctly.")
     void findAllTargetsOfRolloutGroupWithActionStatus() {
         final Rollout rollout = testdataFactory.createRollout();
-        final List<RolloutGroup> rolloutGroups = rolloutGroupManagement.findByRollout(PAGE, rollout.getId())
-                .getContent();
+        final List<RolloutGroup> rolloutGroups = rolloutGroupManagement.findByRollout(PAGE, rollout.getId()).getContent();
         rolloutHandler.handleAll();
 
         // check query when no actions exist
@@ -170,8 +169,7 @@ void findAllTargetsOfRolloutGroupWithActionStatus() {
                         PageRequest.of(0, 500, Sort.by(Direction.DESC, "lastActionStatusCode")),
                         rolloutGroups.get(0).getId())
                 .getContent();
-        assertThat(targetsWithActionStatus)
-                .hasSize((int) rolloutGroupManagement.countTargetsOfRolloutsGroup(rolloutGroups.get(0).getId()));
+        assertThat(targetsWithActionStatus).hasSize((int) rolloutGroupManagement.countTargetsOfRolloutsGroup(rolloutGroups.get(0).getId()));
         assertTargetNotNullAndActionStatusNullAndActionStatusCode(targetsWithActionStatus, null);
 
         rolloutManagement.start(rollout.getId());