Specification and BaseRepositotyProvider improvements (#1182)

* Introduce custom base repository factory bean Signed-off-by: Alexander Dobler <alexander.dobler3@bosch.io> * Simplify ordered DS service call by using specs Signed-off-by: Alexander Dobler <alexander.dobler3@bosch.io> * Annotate getBaseRepoType interface as FunctionalInterface Signed-off-by: Alexander Dobler <alexander.dobler3@bosch.io> * Use CustomBaseRepositoryFactory instead of static BaseClass Signed-off-by: Alexander Dobler <alexander.dobler3@bosch.io> * Fix license headers Signed-off-by: Alexander Dobler <alexander.dobler3@bosch.io> * Add ordered specification back Signed-off-by: Alexander Dobler <alexander.dobler3@bosch.io>
2021-10-21 15:46:13 +02:00
parent d80d0d6349
commit 3621015f93
6 changed files with 169 additions and 42 deletions
--- a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/BaseRepositoryTypeProvider.java
+++ b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/BaseRepositoryTypeProvider.java
@@ -0,0 +1,23 @@
+/**
+ * Copyright (c) 2021 Bosch.IO GmbH and others.
+ *
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ */
+package org.eclipse.hawkbit.repository;
+
+/**
+ * Provider that returns a base repository implementation dynamically based on repository type
+ */
+@FunctionalInterface
+public interface BaseRepositoryTypeProvider {
+
+    /**
+     * Return a base repository implementation that shall be used based on provided repository type
+     * @param repositoryType type of repository
+     * @return base repository implementation class
+     */
+    Class<?> getBaseRepositoryType(final Class<?> repositoryType);
+}
--- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/CustomBaseRepositoryFactoryBean.java
+++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/CustomBaseRepositoryFactoryBean.java
@@ -0,0 +1,51 @@
+/**
+ * Copyright (c) 2021 Bosch.IO GmbH and others.
+ *
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ */
+package org.eclipse.hawkbit.repository.jpa;
+
+import javax.persistence.EntityManager;
+
+import org.eclipse.hawkbit.repository.BaseRepositoryTypeProvider;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.data.jpa.repository.support.JpaRepositoryFactoryBean;
+import org.springframework.data.repository.Repository;
+import org.springframework.data.repository.core.support.RepositoryFactorySupport;
+
+/**
+ * A {@link JpaRepositoryFactoryBean} extension that allow injection of custom
+ * repository factories by using a {@link BaseRepositoryTypeProvider}
+ * implementation, allows injecting different base repository implementations based on repository type
+ * 
+ * @param <T>
+ * @param <S>
+ * @param <ID>
+ */
+public class CustomBaseRepositoryFactoryBean<T extends Repository<S, ID>, S, ID>
+        extends JpaRepositoryFactoryBean<T, S, ID> {
+
+    @Autowired
+    BaseRepositoryTypeProvider baseRepoProvider;
+
+    /**
+     * Creates a new {@link JpaRepositoryFactoryBean} for the given repository
+     * interface.
+     *
+     * @param repositoryInterface
+     *            must not be {@literal null}.
+     */
+    public CustomBaseRepositoryFactoryBean(final Class<? extends T> repositoryInterface) {
+        super(repositoryInterface);
+    }
+
+    @Override
+    protected RepositoryFactorySupport createRepositoryFactory(final EntityManager entityManager) {
+        final RepositoryFactorySupport rfs = super.createRepositoryFactory(entityManager);
+        rfs.setRepositoryBaseClass(baseRepoProvider.getBaseRepositoryType(getObjectType()));
+        return rfs;
+    }
+}
--- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaTargetManagement.java
+++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaTargetManagement.java
@@ -8,6 +8,8 @@
 */
 package org.eclipse.hawkbit.repository.jpa;

+import static org.eclipse.hawkbit.repository.jpa.specifications.TargetSpecifications.orderedByLinkedDistributionSet;
+
 import java.net.URI;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -21,12 +23,11 @@ import java.util.stream.Collectors;
 import javax.persistence.EntityManager;
 import javax.persistence.criteria.CriteriaBuilder;
 import javax.persistence.criteria.CriteriaQuery;
-import javax.persistence.criteria.Expression;
 import javax.persistence.criteria.MapJoin;
-import javax.persistence.criteria.Predicate;
 import javax.persistence.criteria.Root;
 import org.eclipse.hawkbit.repository.DistributionSetManagement;
 import org.eclipse.hawkbit.repository.FilterParams;
+import org.eclipse.hawkbit.repository.OffsetBasedPageRequest;
 import org.eclipse.hawkbit.repository.QuotaManagement;
 import org.eclipse.hawkbit.repository.TargetFields;
 import org.eclipse.hawkbit.repository.TargetManagement;
@@ -43,7 +44,6 @@ import org.eclipse.hawkbit.repository.jpa.builder.JpaTargetCreate;
 import org.eclipse.hawkbit.repository.jpa.builder.JpaTargetUpdate;
 import org.eclipse.hawkbit.repository.jpa.configuration.Constants;
 import org.eclipse.hawkbit.repository.jpa.executor.AfterTransactionCommitExecutor;
-import org.eclipse.hawkbit.repository.jpa.model.JpaDistributionSet_;
 import org.eclipse.hawkbit.repository.jpa.model.JpaTarget;
 import org.eclipse.hawkbit.repository.jpa.model.JpaTargetMetadata;
 import org.eclipse.hawkbit.repository.jpa.model.JpaTargetMetadata_;
@@ -74,7 +74,7 @@ import org.springframework.data.domain.Page;
 import org.springframework.data.domain.PageImpl;
 import org.springframework.data.domain.Pageable;
 import org.springframework.data.domain.Slice;
-import org.springframework.data.domain.SliceImpl;
+import org.springframework.data.domain.Sort;
 import org.springframework.data.jpa.domain.Specification;
 import org.springframework.orm.jpa.vendor.Database;
 import org.springframework.retry.annotation.Backoff;
@@ -615,46 +615,30 @@ public class JpaTargetManagement implements TargetManagement {
    @Override
    public Slice<Target> findByFilterOrderByLinkedDistributionSet(final Pageable pageable,
            final long orderByDistributionId, final FilterParams filterParams) {
-        final CriteriaBuilder cb = entityManager.getCriteriaBuilder();
-        final CriteriaQuery<JpaTarget> query = cb.createQuery(JpaTarget.class);
-        final Root<JpaTarget> targetRoot = query.from(JpaTarget.class);
-
-        // select case expression to retrieve the case value as a column to be
-        // able to order based on
-        // this column, installed first,...
-        final Expression<Object> selectCase = cb.selectCase()
-                .when(cb.equal(targetRoot.get(JpaTarget_.installedDistributionSet).get(JpaDistributionSet_.id),
-                        orderByDistributionId), 1)
-                .when(cb.equal(targetRoot.get(JpaTarget_.assignedDistributionSet).get(JpaDistributionSet_.id),
-                        orderByDistributionId), 2)
-                .otherwise(100);
-        // build the specifications and then to predicates necessary by the
-        // given filters
-        final Predicate[] specificationsForMultiSelect = specificationsToPredicate(buildSpecificationList(filterParams),
-                targetRoot, query, cb);
-
-        // if we have some predicates then add it to the where clause of the
-        // multiselect
-        if (specificationsForMultiSelect.length > 0) {
-            query.where(specificationsForMultiSelect);
-        }
-        // add the order to the multi select first based on the selectCase
-        query.orderBy(cb.asc(selectCase), cb.desc(targetRoot.get(JpaTarget_.id)));
-
-        final int pageSize = pageable.getPageSize();
-        final List<JpaTarget> resultList = entityManager.createQuery(query).setFirstResult((int) pageable.getOffset())
-                .setMaxResults(pageSize).getResultList();
-        final boolean hasNext = resultList.size() > pageSize;
-        return new SliceImpl<>(Collections.unmodifiableList(resultList), pageable, hasNext);
+        Specification<JpaTarget> orderedFilterSpec = combineFiltersAndDsOrder(orderByDistributionId, filterParams);
+        // remove default sort from pageable to not overwrite sorted spec
+        final OffsetBasedPageRequest unsortedPage = new OffsetBasedPageRequest(pageable.getOffset(),
+                pageable.getPageSize(), Sort.unsorted());
+        return convertPage(targetRepository.findAllWithoutCount(orderedFilterSpec, unsortedPage), unsortedPage);
    }

-    private static Predicate[] specificationsToPredicate(final List<Specification<JpaTarget>> specifications,
-            final Root<JpaTarget> root, final CriteriaQuery<?> query, final CriteriaBuilder cb) {
-        final Predicate[] predicates = new Predicate[specifications.size()];
-        for (int index = 0; index < predicates.length; index++) {
-            predicates[index] = specifications.get(index).toPredicate(root, query, cb);
+    /**
+     * Applies orderedByLinkedDistributionSet spec for order and adds additional
+     * specs based on filters
+     * 
+     * @param orderByDistributionId
+     *            distribution set used for order
+     * @param filterParams
+     *            filters to generate additional specs for
+     * @return specification combining order and filter specs
+     */
+    private Specification<JpaTarget> combineFiltersAndDsOrder(final long orderByDistributionId,
+            final FilterParams filterParams) {
+        Specification<JpaTarget> orderedFilterSpec = orderedByLinkedDistributionSet(orderByDistributionId);
+        for (Specification<JpaTarget> spec : buildSpecificationList(filterParams)) {
+            orderedFilterSpec = orderedFilterSpec.and(spec);
        }
-        return predicates;
+        return orderedFilterSpec;
    }

    @Override
--- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/NoCountBaseRepositoryTypeProvider.java
+++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/NoCountBaseRepositoryTypeProvider.java
@@ -0,0 +1,24 @@
+/**
+ * Copyright (c) 2021 Bosch.IO GmbH and others.
+ *
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ */
+package org.eclipse.hawkbit.repository.jpa;
+
+import org.eclipse.hawkbit.repository.BaseRepositoryTypeProvider;
+
+/**
+ * Simple implementation of {@link BaseRepositoryTypeProvider} leveraging our
+ * {@link SimpleJpaWithNoCountRepository} for all current use cases
+ */
+public class NoCountBaseRepositoryTypeProvider implements BaseRepositoryTypeProvider {
+
+    @Override
+    public Class<?> getBaseRepositoryType(final Class<?> repositoryType) {
+        return SimpleJpaWithNoCountRepository.class;
+    }
+
+}
--- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/RepositoryApplicationConfiguration.java
+++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/RepositoryApplicationConfiguration.java
@@ -17,6 +17,7 @@ import javax.sql.DataSource;

 import org.eclipse.hawkbit.artifact.repository.ArtifactRepository;
 import org.eclipse.hawkbit.repository.ArtifactManagement;
+import org.eclipse.hawkbit.repository.BaseRepositoryTypeProvider;
 import org.eclipse.hawkbit.repository.ControllerManagement;
 import org.eclipse.hawkbit.repository.DeploymentManagement;
 import org.eclipse.hawkbit.repository.DistributionSetManagement;
@@ -140,7 +141,7 @@ import com.google.common.collect.Maps;
 * General configuration for hawkBit's Repository.
 *
 */
-@EnableJpaRepositories(value = "org.eclipse.hawkbit.repository.jpa", repositoryBaseClass = SimpleJpaWithNoCountRepository.class)
+@EnableJpaRepositories(value = "org.eclipse.hawkbit.repository.jpa", repositoryFactoryBeanClass = CustomBaseRepositoryFactoryBean.class)
@EnableTransactionManagement
@EnableJpaAuditing
@EnableAspectJAutoProxy
@@ -940,4 +941,15 @@ public class RepositoryApplicationConfiguration extends JpaBaseConfiguration {
                lockRegistry);
    }

+    /**
+     * Our default {@link BaseRepositoryTypeProvider} bean always provides the NoCountBaseRepository
+     *
+     * @return a {@link BaseRepositoryTypeProvider} bean
+     */
+    @Bean
+    @ConditionalOnMissingBean
+    BaseRepositoryTypeProvider baseRepositoryTypeProvider() {
+        return new NoCountBaseRepositoryTypeProvider();
+
+    }
 }
--- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/specifications/TargetSpecifications.java
+++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/specifications/TargetSpecifications.java
@@ -13,6 +13,7 @@ import java.util.Collection;
 import java.util.List;

 import javax.persistence.criteria.CriteriaBuilder;
+import javax.persistence.criteria.Expression;
 import javax.persistence.criteria.Join;
 import javax.persistence.criteria.JoinType;
 import javax.persistence.criteria.ListJoin;
@@ -537,4 +538,36 @@ public final class TargetSpecifications {
    public static Specification<JpaTarget> hasTargetType() {
        return (targetRoot, query, cb) -> cb.isNotNull(targetRoot.get(JpaTarget_.targetType));
    }
+
+    /**
+     * Can be added to specification chain to order result by provided distribution
+     * set
+     *
+     * Order: 1. Targets with DS installed, 2. Targets with DS assigned, 3. Based on
+     * target id
+     *
+     * NOTE: Other specs, pagables and sort objects may alter the queries orderBy
+     * entry too, possibly invalidating the applied order, keep in mind when using
+     * this
+     *
+     * @param distributionSetIdForOrder
+     *            distribution set to consider
+     * @return specification that applies order by ds, may be overwritten
+     */
+    public static Specification<JpaTarget> orderedByLinkedDistributionSet(long distributionSetIdForOrder) {
+        return (targetRoot, query, cb) -> {
+            // Enhance query with custom select based sort
+            final Expression<Object> selectCase = cb.selectCase()
+                    .when(cb.equal(targetRoot.get(JpaTarget_.installedDistributionSet).get(JpaDistributionSet_.id),
+                            distributionSetIdForOrder), 1)
+                    .when(cb.equal(targetRoot.get(JpaTarget_.assignedDistributionSet).get(JpaDistributionSet_.id),
+                            distributionSetIdForOrder), 2)
+                    .otherwise(100);
+            query.orderBy(cb.asc(selectCase), cb.desc(targetRoot.get(JpaTarget_.id)));
+
+            // Spec only provides order, so no further filtering
+            return query.getRestriction();
+        };
+    }
+
 }