From 20388e48f895e23065469a6efd105515ab51ed37 Mon Sep 17 00:00:00 2001 From: Bondar Bogdan <36962546+bogdan-bondar@users.noreply.github.com> Date: Wed, 25 Aug 2021 14:43:10 +0200 Subject: [PATCH] Fix sonar/repository for target type (#1169) * fix sonar/repository for target type changes Signed-off-by: Bogdan Bondar * review finding Signed-off-by: Bogdan Bondar --- docs/content/apis/management_api.md | 2 +- .../repository/jpa/JpaTargetManagement.java | 48 +++++------ .../jpa/JpaTargetTypeManagement.java | 82 +++++++++++-------- 3 files changed, 72 insertions(+), 60 deletions(-) diff --git a/docs/content/apis/management_api.md b/docs/content/apis/management_api.md index d7497c922..fc7d843c2 100644 --- a/docs/content/apis/management_api.md +++ b/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/) diff --git 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 index e9176fb30..75b557ed4 100644 --- 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 @@ -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 EventPublisherHolder eventPublisherHolder, final TenantAware tenantAware, - final AfterTransactionCommitExecutor afterCommit, final VirtualPropertyReplacer virtualPropertyReplacer, - final Database database) { + final TargetTagRepository targetTagRepository, final EventPublisherHolder eventPublisherHolder, + final TenantAware tenantAware, final AfterTransactionCommitExecutor afterCommit, + final VirtualPropertyReplacer virtualPropertyReplacer, 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 spec = RSQLUtility.buildRsqlSpecification(rsqlParam, TargetMetadataFields.class, - virtualPropertyReplacer, database); + final Specification spec = RSQLUtility.buildRsqlSpecification(rsqlParam, + TargetMetadataFields.class, virtualPropertyReplacer, database); return convertMdPage(targetMetadataRepository.findAll((Specification) (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( - RSQLUtility.buildRsqlSpecification(targetFilterQuery.getQuery(), TargetFields.class, virtualPropertyReplacer, database), - pageable); + return findTargetsBySpec(RSQLUtility.buildRsqlSpecification(targetFilterQuery.getQuery(), TargetFields.class, + virtualPropertyReplacer, database), pageable); } @Override public Page findByRsql(final Pageable pageable, final String targetFilterQuery) { - return findTargetsBySpec( - RSQLUtility.buildRsqlSpecification(targetFilterQuery, TargetFields.class, virtualPropertyReplacer, database), pageable); + return findTargetsBySpec(RSQLUtility.buildRsqlSpecification(targetFilterQuery, TargetFields.class, + virtualPropertyReplacer, database), pageable); } private Page findTargetsBySpec(final Specification 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){ - TargetType targetType = getTargetTypeByIdAndThrowIfNotFound(update.getTargetTypeId()); + if (update.getTargetTypeId() != null) { + 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 spec = RSQLUtility.buildRsqlSpecification(rsqlParam, TargetFields.class, virtualPropertyReplacer, - database); + final Specification spec = RSQLUtility.buildRsqlSpecification(rsqlParam, TargetFields.class, + virtualPropertyReplacer, database); return convertPage( targetRepository @@ -435,8 +434,8 @@ public class JpaTargetManagement implements TargetManagement { final String rsqlParam) { throwEntityNotFoundIfDsDoesNotExist(distributionSetId); - final Specification spec = RSQLUtility.buildRsqlSpecification(rsqlParam, TargetFields.class, virtualPropertyReplacer, - database); + final Specification spec = RSQLUtility.buildRsqlSpecification(rsqlParam, TargetFields.class, + 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 spec = RSQLUtility.buildRsqlSpecification(rsqlParam, TargetFields.class, virtualPropertyReplacer, - database); + final Specification spec = RSQLUtility.buildRsqlSpecification(rsqlParam, TargetFields.class, + virtualPropertyReplacer, database); return convertPage(targetRepository.findAll((Specification) (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 specs = RSQLUtility.buildRsqlSpecification(targetFilterQuery.getQuery(), TargetFields.class, - virtualPropertyReplacer, database); + final Specification specs = RSQLUtility.buildRsqlSpecification(targetFilterQuery.getQuery(), + TargetFields.class, virtualPropertyReplacer, database); return targetRepository.count(specs); } diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaTargetTypeManagement.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaTargetTypeManagement.java index 34a9a0997..84ea750f3 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/JpaTargetTypeManagement.java +++ b/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 getByName(String name) { + public Optional 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 create(Collection creates) { + @Transactional + @Retryable(include = { + ConcurrencyFailureException.class }, maxAttempts = Constants.TX_RT_MAX, backoff = @Backoff(delay = Constants.TX_RT_DELAY)) + public List create(final Collection creates) { return creates.stream().map(this::create).collect(Collectors.toList()); } @@ -122,12 +127,12 @@ public class JpaTargetTypeManagement implements TargetTypeManagement { } @Override - public Page findAll(Pageable pageable) { + public Page findAll(final Pageable pageable) { return convertPage(targetTypeRepository.findAll(pageable), pageable); } @Override - public Page findByRsql(Pageable pageable, String rsqlParam) { + public Page findByRsql(final Pageable pageable, final String rsqlParam) { final Specification spec = RSQLUtility.buildRsqlSpecification(rsqlParam, TargetTypeFields.class, virtualPropertyReplacer, database); @@ -135,41 +140,43 @@ public class JpaTargetTypeManagement implements TargetTypeManagement { } @Override - public Optional get(long id) { + public Optional get(final long id) { return targetTypeRepository.findById(id).map(targetType -> targetType); } @Override - public Optional findByTargetId(long targetId) { - return targetTypeRepository - .findOne(TargetTypeSpecification.hasTarget(targetId)).map(TargetType.class::cast); + public Optional findByTargetId(final long targetId) { + return targetTypeRepository.findOne(TargetTypeSpecification.hasTarget(targetId)).map(TargetType.class::cast); } @Override - public List findByTargetIds(Collection targetIds) { - return targetTypeRepository.findAll(TargetTypeSpecification.hasTarget(targetIds)) - .stream().map(TargetType.class::cast).collect(Collectors.toList()); + public List findByTargetIds(final Collection targetIds) { + return targetTypeRepository.findAll(TargetTypeSpecification.hasTarget(targetIds)).stream() + .map(TargetType.class::cast).collect(Collectors.toList()); } @Override - public Optional findByTargetControllerId(String controllerId) { - return targetTypeRepository - .findOne(TargetTypeSpecification.hasTargetControllerId(controllerId)).map(TargetType.class::cast); + public Optional findByTargetControllerId(final String controllerId) { + return targetTypeRepository.findOne(TargetTypeSpecification.hasTargetControllerId(controllerId)) + .map(TargetType.class::cast); } @Override - public List findByTargetControllerIds(Collection controllerIds) { - return targetTypeRepository.findAll(TargetTypeSpecification.hasTargetControllerIdIn(controllerIds)) - .stream().map(TargetType.class::cast).collect(Collectors.toList()); + public List findByTargetControllerIds(final Collection controllerIds) { + return targetTypeRepository.findAll(TargetTypeSpecification.hasTargetControllerIdIn(controllerIds)).stream() + .map(TargetType.class::cast).collect(Collectors.toList()); } @Override - public List get(Collection ids) { + public List get(final Collection 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 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 distributionSetTypeIds) { final Collection 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) { - distributionSetTypeRepository.findById(typeId).orElseThrow(() -> new EntityNotFoundException(DistributionSetType.class, typeId)); + private JpaDistributionSetType findDsTypeAndThrowExceptionIfNotFound(final Long 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 convertPage(final Page findAll, final Pageable pageable) { return new PageImpl<>(Collections.unmodifiableList(findAll.getContent()), pageable, findAll.getTotalElements()); } - - private static Slice convertPage(final Slice findAll, final Pageable pageable) { - return new PageImpl<>(Collections.unmodifiableList(findAll.getContent()), pageable, 0); - } - }