Fix sonar/repository for target type (#1169)

* fix sonar/repository for target type changes

Signed-off-by: Bogdan Bondar <Bogdan.Bondar@bosch.io>

* review finding

Signed-off-by: Bogdan Bondar <Bogdan.Bondar@bosch.io>

docs/content/apis/management_api.md

@@ -29,7 +29,7 @@ Supported HTTP-methods are:
 Available Management APIs resources are:
 
 - [Targets](/hawkbit/apis/mgmt/targets/)
-- [Target types](/hawkbit/apis/mgmt/targettypess/)
+- [Target types](/hawkbit/apis/mgmt/targettypes/)
 - [Distribution sets](/hawkbit/apis/mgmt/distributionsets/)
 - [Distribution set types](/hawkbit/apis/mgmt/distributionsettypes/)
 - [Software modules](/hawkbit/apis/mgmt/softwaremodules/)

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

@@ -123,14 +123,14 @@ public class JpaTargetManagement implements TargetManagement {
     private final Database database;
 
     public JpaTargetManagement(final EntityManager entityManager, final QuotaManagement quotaManagement,
-            final TargetRepository targetRepository, final TargetTypeRepository targetTypeRepository, final TargetMetadataRepository targetMetadataRepository,
+            final TargetRepository targetRepository, final TargetTypeRepository targetTypeRepository,
+            final TargetMetadataRepository targetMetadataRepository,
             final RolloutGroupRepository rolloutGroupRepository,
             final DistributionSetRepository distributionSetRepository,
             final TargetFilterQueryRepository targetFilterQueryRepository,
-            final TargetTagRepository targetTagRepository,
+            final TargetTagRepository targetTagRepository, final EventPublisherHolder eventPublisherHolder,
-            final EventPublisherHolder eventPublisherHolder, final TenantAware tenantAware,
+            final TenantAware tenantAware, final AfterTransactionCommitExecutor afterCommit,
-            final AfterTransactionCommitExecutor afterCommit, final VirtualPropertyReplacer virtualPropertyReplacer,
+            final VirtualPropertyReplacer virtualPropertyReplacer, final Database database) {
-            final Database database) {
         this.entityManager = entityManager;
         this.quotaManagement = quotaManagement;
         this.targetRepository = targetRepository;
@@ -288,8 +288,8 @@ public class JpaTargetManagement implements TargetManagement {
 
         final Long targetId = getByControllerIdAndThrowIfNotFound(controllerId).getId();
 
-        final Specification<JpaTargetMetadata> spec = RSQLUtility.buildRsqlSpecification(rsqlParam, TargetMetadataFields.class,
+        final Specification<JpaTargetMetadata> spec = RSQLUtility.buildRsqlSpecification(rsqlParam,
-                virtualPropertyReplacer, database);
+                TargetMetadataFields.class, virtualPropertyReplacer, database);
 
         return convertMdPage(targetMetadataRepository.findAll((Specification<JpaTargetMetadata>) (root, query, cb) -> cb
                 .and(cb.equal(root.get(JpaTargetMetadata_.target).get(JpaTarget_.id), targetId),
@@ -314,15 +314,14 @@ public class JpaTargetManagement implements TargetManagement {
         final TargetFilterQuery targetFilterQuery = targetFilterQueryRepository.findById(targetFilterQueryId)
                 .orElseThrow(() -> new EntityNotFoundException(TargetFilterQuery.class, targetFilterQueryId));
 
-        return findTargetsBySpec(
+        return findTargetsBySpec(RSQLUtility.buildRsqlSpecification(targetFilterQuery.getQuery(), TargetFields.class,
-                RSQLUtility.buildRsqlSpecification(targetFilterQuery.getQuery(), TargetFields.class, virtualPropertyReplacer, database),
+                virtualPropertyReplacer, database), pageable);
-                pageable);
     }
 
     @Override
     public Page<Target> findByRsql(final Pageable pageable, final String targetFilterQuery) {
-        return findTargetsBySpec(
+        return findTargetsBySpec(RSQLUtility.buildRsqlSpecification(targetFilterQuery, TargetFields.class,
-                RSQLUtility.buildRsqlSpecification(targetFilterQuery, TargetFields.class, virtualPropertyReplacer, database), pageable);
+                virtualPropertyReplacer, database), pageable);
     }
 
     private Page<Target> findTargetsBySpec(final Specification<JpaTarget> spec, final Pageable pageable) {
@@ -342,8 +341,8 @@ public class JpaTargetManagement implements TargetManagement {
         update.getDescription().ifPresent(target::setDescription);
         update.getAddress().ifPresent(target::setAddress);
         update.getSecurityToken().ifPresent(target::setSecurityToken);
-        if (update.getTargetTypeId() != null){
+        if (update.getTargetTypeId() != null) {
-            TargetType targetType = getTargetTypeByIdAndThrowIfNotFound(update.getTargetTypeId());
+            final TargetType targetType = getTargetTypeByIdAndThrowIfNotFound(update.getTargetTypeId());
             target.setTargetType(targetType);
         }
 
@@ -396,8 +395,8 @@ public class JpaTargetManagement implements TargetManagement {
             final String rsqlParam) {
         throwEntityNotFoundIfDsDoesNotExist(distributionSetID);
 
-        final Specification<JpaTarget> spec = RSQLUtility.buildRsqlSpecification(rsqlParam, TargetFields.class, virtualPropertyReplacer,
+        final Specification<JpaTarget> spec = RSQLUtility.buildRsqlSpecification(rsqlParam, TargetFields.class,
-                database);
+                virtualPropertyReplacer, database);
 
         return convertPage(
                 targetRepository
@@ -435,8 +434,8 @@ public class JpaTargetManagement implements TargetManagement {
             final String rsqlParam) {
         throwEntityNotFoundIfDsDoesNotExist(distributionSetId);
 
-        final Specification<JpaTarget> spec = RSQLUtility.buildRsqlSpecification(rsqlParam, TargetFields.class, virtualPropertyReplacer,
+        final Specification<JpaTarget> spec = RSQLUtility.buildRsqlSpecification(rsqlParam, TargetFields.class,
-                database);
+                virtualPropertyReplacer, database);
 
         return convertPage(
                 targetRepository
@@ -613,7 +612,10 @@ public class JpaTargetManagement implements TargetManagement {
     }
 
     @Override
-    public Target assignType(String controllerID, long targetTypeId) {
+    @Transactional
+    @Retryable(include = {
+            ConcurrencyFailureException.class }, maxAttempts = Constants.TX_RT_MAX, backoff = @Backoff(delay = Constants.TX_RT_DELAY))
+    public Target assignType(final String controllerID, final long targetTypeId) {
         final JpaTarget target = getByControllerIdAndThrowIfNotFound(controllerID);
         final JpaTargetType targetType = getTargetTypeByIdAndThrowIfNotFound(targetTypeId);
         target.setTargetType(targetType);
@@ -783,8 +785,8 @@ public class JpaTargetManagement implements TargetManagement {
 
         throwEntityNotFoundExceptionIfTagDoesNotExist(tagId);
 
-        final Specification<JpaTarget> spec = RSQLUtility.buildRsqlSpecification(rsqlParam, TargetFields.class, virtualPropertyReplacer,
+        final Specification<JpaTarget> spec = RSQLUtility.buildRsqlSpecification(rsqlParam, TargetFields.class,
-                database);
+                virtualPropertyReplacer, database);
 
         return convertPage(targetRepository.findAll((Specification<JpaTarget>) (root, query, cb) -> cb.and(
                 TargetSpecifications.hasTag(tagId).toPredicate(root, query, cb), spec.toPredicate(root, query, cb)),
@@ -796,8 +798,8 @@ public class JpaTargetManagement implements TargetManagement {
         final TargetFilterQuery targetFilterQuery = targetFilterQueryRepository.findById(targetFilterQueryId)
                 .orElseThrow(() -> new EntityNotFoundException(TargetFilterQuery.class, targetFilterQueryId));
 
-        final Specification<JpaTarget> specs = RSQLUtility.buildRsqlSpecification(targetFilterQuery.getQuery(), TargetFields.class,
+        final Specification<JpaTarget> specs = RSQLUtility.buildRsqlSpecification(targetFilterQuery.getQuery(),
-                virtualPropertyReplacer, database);
+                TargetFields.class, virtualPropertyReplacer, database);
         return targetRepository.count(specs);
     }
 

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

@@ -8,6 +8,12 @@
  */
 package org.eclipse.hawkbit.repository.jpa;
 
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+import java.util.Optional;
+import java.util.stream.Collectors;
+
 import org.eclipse.hawkbit.repository.QuotaManagement;
 import org.eclipse.hawkbit.repository.TargetTypeFields;
 import org.eclipse.hawkbit.repository.TargetTypeManagement;
@@ -31,7 +37,6 @@ import org.springframework.dao.ConcurrencyFailureException;
 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.jpa.domain.Specification;
 import org.springframework.orm.jpa.vendor.Database;
 import org.springframework.retry.annotation.Backoff;
@@ -39,12 +44,6 @@ import org.springframework.retry.annotation.Retryable;
 import org.springframework.transaction.annotation.Transactional;
 import org.springframework.validation.annotation.Validated;
 
-import java.util.Collection;
-import java.util.Collections;
-import java.util.List;
-import java.util.Optional;
-import java.util.stream.Collectors;
-
 /**
  * JPA implementation of {@link TargetTypeManagement}.
  *
@@ -87,7 +86,7 @@ public class JpaTargetTypeManagement implements TargetTypeManagement {
     }
 
     @Override
-    public Optional<TargetType> getByName(String name) {
+    public Optional<TargetType> getByName(final String name) {
         return targetTypeRepository.findByName(name).map(TargetType.class::cast);
     }
 
@@ -97,13 +96,19 @@ public class JpaTargetTypeManagement implements TargetTypeManagement {
     }
 
     @Override
-    public TargetType create(TargetTypeCreate create) {
+    @Transactional
+    @Retryable(include = {
+            ConcurrencyFailureException.class }, maxAttempts = Constants.TX_RT_MAX, backoff = @Backoff(delay = Constants.TX_RT_DELAY))
+    public TargetType create(final TargetTypeCreate create) {
         final JpaTargetTypeCreate typeCreate = (JpaTargetTypeCreate) create;
         return targetTypeRepository.save(typeCreate.build());
     }
 
     @Override
-    public List<TargetType> create(Collection<TargetTypeCreate> creates) {
+    @Transactional
+    @Retryable(include = {
+            ConcurrencyFailureException.class }, maxAttempts = Constants.TX_RT_MAX, backoff = @Backoff(delay = Constants.TX_RT_DELAY))
+    public List<TargetType> create(final Collection<TargetTypeCreate> creates) {
         return creates.stream().map(this::create).collect(Collectors.toList());
     }
 
@@ -122,12 +127,12 @@ public class JpaTargetTypeManagement implements TargetTypeManagement {
     }
 
     @Override
-    public Page<TargetType> findAll(Pageable pageable) {
+    public Page<TargetType> findAll(final Pageable pageable) {
         return convertPage(targetTypeRepository.findAll(pageable), pageable);
     }
 
     @Override
-    public Page<TargetType> findByRsql(Pageable pageable, String rsqlParam) {
+    public Page<TargetType> findByRsql(final Pageable pageable, final String rsqlParam) {
         final Specification<JpaTargetType> spec = RSQLUtility.buildRsqlSpecification(rsqlParam, TargetTypeFields.class,
                 virtualPropertyReplacer, database);
 
@@ -135,41 +140,43 @@ public class JpaTargetTypeManagement implements TargetTypeManagement {
     }
 
     @Override
-    public Optional<TargetType> get(long id) {
+    public Optional<TargetType> get(final long id) {
         return targetTypeRepository.findById(id).map(targetType -> targetType);
     }
 
     @Override
-    public Optional<TargetType> findByTargetId(long targetId) {
+    public Optional<TargetType> findByTargetId(final long targetId) {
-        return targetTypeRepository
+        return targetTypeRepository.findOne(TargetTypeSpecification.hasTarget(targetId)).map(TargetType.class::cast);
-                .findOne(TargetTypeSpecification.hasTarget(targetId)).map(TargetType.class::cast);
     }
 
     @Override
-    public List<TargetType> findByTargetIds(Collection<Long> targetIds) {
+    public List<TargetType> findByTargetIds(final Collection<Long> targetIds) {
-        return targetTypeRepository.findAll(TargetTypeSpecification.hasTarget(targetIds))
+        return targetTypeRepository.findAll(TargetTypeSpecification.hasTarget(targetIds)).stream()
-                .stream().map(TargetType.class::cast).collect(Collectors.toList());
+                .map(TargetType.class::cast).collect(Collectors.toList());
     }
 
     @Override
-    public Optional<TargetType> findByTargetControllerId(String controllerId) {
+    public Optional<TargetType> findByTargetControllerId(final String controllerId) {
-        return targetTypeRepository
+        return targetTypeRepository.findOne(TargetTypeSpecification.hasTargetControllerId(controllerId))
-                .findOne(TargetTypeSpecification.hasTargetControllerId(controllerId)).map(TargetType.class::cast);
+                .map(TargetType.class::cast);
     }
 
     @Override
-    public List<TargetType> findByTargetControllerIds(Collection<String> controllerIds) {
+    public List<TargetType> findByTargetControllerIds(final Collection<String> controllerIds) {
-        return targetTypeRepository.findAll(TargetTypeSpecification.hasTargetControllerIdIn(controllerIds))
+        return targetTypeRepository.findAll(TargetTypeSpecification.hasTargetControllerIdIn(controllerIds)).stream()
-                .stream().map(TargetType.class::cast).collect(Collectors.toList());
+                .map(TargetType.class::cast).collect(Collectors.toList());
     }
 
     @Override
-    public List<TargetType> get(Collection<Long> ids) {
+    public List<TargetType> get(final Collection<Long> ids) {
         return Collections.unmodifiableList(targetTypeRepository.findAllById(ids));
     }
 
     @Override
-    public TargetType update(TargetTypeUpdate update) {
+    @Transactional
+    @Retryable(include = {
+            ConcurrencyFailureException.class }, maxAttempts = Constants.TX_RT_MAX, backoff = @Backoff(delay = Constants.TX_RT_DELAY))
+    public TargetType update(final TargetTypeUpdate update) {
         final GenericTargetTypeUpdate typeUpdate = (GenericTargetTypeUpdate) update;
 
         final JpaTargetType type = findTargetTypeAndThrowExceptionIfNotFound(typeUpdate.getId());
@@ -182,7 +189,11 @@ public class JpaTargetTypeManagement implements TargetTypeManagement {
     }
 
     @Override
-    public TargetType assignCompatibleDistributionSetTypes(long targetTypeId, Collection<Long> distributionSetTypeIds) {
+    @Transactional
+    @Retryable(include = {
+            ConcurrencyFailureException.class }, maxAttempts = Constants.TX_RT_MAX, backoff = @Backoff(delay = Constants.TX_RT_DELAY))
+    public TargetType assignCompatibleDistributionSetTypes(final long targetTypeId,
+            final Collection<Long> distributionSetTypeIds) {
         final Collection<JpaDistributionSetType> dsTypes = distributionSetTypeRepository
                 .findAllById(distributionSetTypeIds);
 
@@ -200,7 +211,10 @@ public class JpaTargetTypeManagement implements TargetTypeManagement {
     }
 
     @Override
-    public TargetType unassignDistributionSetType(long targetTypeId, long distributionSetTypeId) {
+    @Transactional
+    @Retryable(include = {
+            ConcurrencyFailureException.class }, maxAttempts = Constants.TX_RT_MAX, backoff = @Backoff(delay = Constants.TX_RT_DELAY))
+    public TargetType unassignDistributionSetType(final long targetTypeId, final long distributionSetTypeId) {
         final JpaTargetType type = findTargetTypeAndThrowExceptionIfNotFound(targetTypeId);
         findDsTypeAndThrowExceptionIfNotFound(distributionSetTypeId);
         type.removeDistributionSetType(distributionSetTypeId);
@@ -212,8 +226,9 @@ public class JpaTargetTypeManagement implements TargetTypeManagement {
         return (JpaTargetType) get(typeId).orElseThrow(() -> new EntityNotFoundException(TargetType.class, typeId));
     }
 
-    private void findDsTypeAndThrowExceptionIfNotFound(final Long typeId) {
+    private JpaDistributionSetType findDsTypeAndThrowExceptionIfNotFound(final Long typeId) {
-        distributionSetTypeRepository.findById(typeId).orElseThrow(() -> new EntityNotFoundException(DistributionSetType.class, typeId));
+        return distributionSetTypeRepository.findById(typeId)
+                .orElseThrow(() -> new EntityNotFoundException(DistributionSetType.class, typeId));
     }
 
     private void throwExceptionIfTargetTypeDoesNotExist(final Long typeId) {
@@ -242,9 +257,4 @@ public class JpaTargetTypeManagement implements TargetTypeManagement {
     private static Page<TargetType> convertPage(final Page<JpaTargetType> findAll, final Pageable pageable) {
         return new PageImpl<>(Collections.unmodifiableList(findAll.getContent()), pageable, findAll.getTotalElements());
     }
-
-    private static Slice<TargetType> convertPage(final Slice<JpaTargetType> findAll, final Pageable pageable) {
-        return new PageImpl<>(Collections.unmodifiableList(findAll.getContent()), pageable, 0);
-    }
-
 }