From 936e6d6d66e7919495b7c0e5d0fe29a810f37531 Mon Sep 17 00:00:00 2001 From: Avgustin Marinov Date: Fri, 8 Mar 2024 14:58:31 +0200 Subject: [PATCH] [#1651] Implement skip DistributionSet implicit lock on DS tags (#1680) tags the implicit lock is skipped on are configured via RepositoryProperties.skipImplicitLockForTags list. By default skip tags are the ones with names: "skip-implicit-lock", "skip_implicit_lock", "SKIP_IMPLICIT_LOCK", "SKIP-IMPLICIT-LOCK" + this commit centralize the implicit lock enable/disable logic Signed-off-by: Marinov Avgustin --- .../repository/DistributionSetManagement.java | 2 +- .../repository/RepositoryProperties.java | 4 + .../RepositoryApplicationConfiguration.java | 34 ++++---- .../management/JpaDeploymentManagement.java | 4 +- .../JpaDistributionSetManagement.java | 79 +++++++++++-------- .../jpa/management/JpaRolloutManagement.java | 10 +-- .../JpaTargetFilterQueryManagement.java | 12 +-- .../DistributionSetAccessControllerTest.java | 7 +- .../DistributionSetManagementTest.java | 38 +++++++-- .../MgmtDistributionSetTagResource.java | 2 +- 10 files changed, 110 insertions(+), 82 deletions(-) diff --git a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/DistributionSetManagement.java b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/DistributionSetManagement.java index 5d4e4b0b15..20360da225 100644 --- a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/DistributionSetManagement.java +++ b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/DistributionSetManagement.java @@ -505,7 +505,7 @@ Slice findByDistributionSetFilterOrderByLinkedTarget(@NotNull P * if set or tag with given ID does not exist */ @PreAuthorize(SpringEvalExpressions.HAS_AUTH_UPDATE_REPOSITORY) - DistributionSet unAssignTag(long id, long tagId); + DistributionSet unassignTag(long id, long tagId); /** * Updates a distribution set meta data value if corresponding entry exists. diff --git a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/RepositoryProperties.java b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/RepositoryProperties.java index ea820edbd4..fb1e90be83 100644 --- a/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/RepositoryProperties.java +++ b/hawkbit-repository/hawkbit-repository-api/src/main/java/org/eclipse/hawkbit/repository/RepositoryProperties.java @@ -9,6 +9,7 @@ */ package org.eclipse.hawkbit.repository; +import java.util.List; import java.util.concurrent.TimeUnit; import lombok.Data; @@ -68,4 +69,7 @@ public class RepositoryProperties { private long dsInvalidationLockTimeout = 5; private boolean implicitTenantCreateAllowed; + + private List skipImplicitLockForTags = + List.of("skip-implicit-lock", "skip_implicit_lock", "SKIP_IMPLICIT_LOCK", "SKIP-IMPLICIT-LOCK"); } \ No newline at end of file diff --git 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 index ca083249ab..a9cc597927 100644 --- 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 @@ -164,6 +164,7 @@ import org.eclipse.hawkbit.security.SystemSecurityContext; import org.eclipse.hawkbit.tenancy.TenantAware; import org.eclipse.hawkbit.tenancy.UserAuthoritiesResolver; +import org.eclipse.hawkbit.utils.TenantConfigHelper; import org.eclipse.persistence.config.PersistenceUnitProperties; import org.hibernate.validator.BaseHibernateValidatorConfiguration; import org.springframework.beans.BeansException; @@ -532,25 +533,24 @@ SystemManagement systemManagement(final JpaProperties properties) { */ @Bean @ConditionalOnMissingBean - DistributionSetManagement distributionSetManagement(final EntityManager entityManager, - final DistributionSetRepository distributionSetRepository, - final DistributionSetTagManagement distributionSetTagManagement, final SystemManagement systemManagement, - final DistributionSetTypeManagement distributionSetTypeManagement, final QuotaManagement quotaManagement, - final DistributionSetMetadataRepository distributionSetMetadataRepository, - final TargetRepository targetRepository, - final TargetFilterQueryRepository targetFilterQueryRepository, final ActionRepository actionRepository, - final EventPublisherHolder eventPublisherHolder, final TenantAware tenantAware, - final VirtualPropertyReplacer virtualPropertyReplacer, - final SoftwareModuleRepository softwareModuleRepository, - final DistributionSetTagRepository distributionSetTagRepository, - final AfterTransactionCommitExecutor afterCommit, - final JpaProperties properties) { + DistributionSetManagement distributionSetManagement( + final EntityManager entityManager, + final DistributionSetRepository distributionSetRepository, + final DistributionSetTagManagement distributionSetTagManagement, final SystemManagement systemManagement, + final DistributionSetTypeManagement distributionSetTypeManagement, final QuotaManagement quotaManagement, + final DistributionSetMetadataRepository distributionSetMetadataRepository, + final TargetRepository targetRepository, + final TargetFilterQueryRepository targetFilterQueryRepository, final ActionRepository actionRepository, + final SystemSecurityContext systemSecurityContext, final TenantConfigurationManagement tenantConfigurationManagement, + final VirtualPropertyReplacer virtualPropertyReplacer, final SoftwareModuleRepository softwareModuleRepository, + final DistributionSetTagRepository distributionSetTagRepository, + final JpaProperties properties, final RepositoryProperties repositoryProperties) { return new JpaDistributionSetManagement(entityManager, distributionSetRepository, distributionSetTagManagement, systemManagement, distributionSetTypeManagement, quotaManagement, distributionSetMetadataRepository, - targetRepository, targetFilterQueryRepository, actionRepository, eventPublisherHolder, tenantAware, - virtualPropertyReplacer, softwareModuleRepository, distributionSetTagRepository, afterCommit, - properties.getDatabase()); - + targetRepository, targetFilterQueryRepository, actionRepository, + TenantConfigHelper.usingContext(systemSecurityContext, tenantConfigurationManagement), + virtualPropertyReplacer, softwareModuleRepository, distributionSetTagRepository, + properties.getDatabase(), repositoryProperties); } /** diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaDeploymentManagement.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaDeploymentManagement.java index 1559fb7dc6..f4fcf87824 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaDeploymentManagement.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaDeploymentManagement.java @@ -9,7 +9,6 @@ */ package org.eclipse.hawkbit.repository.jpa.management; -import static org.eclipse.hawkbit.tenancy.configuration.TenantConfigurationProperties.TenantConfigurationKey.IMPLICIT_LOCK_ENABLED; import static org.eclipse.hawkbit.tenancy.configuration.TenantConfigurationProperties.TenantConfigurationKey.REPOSITORY_ACTIONS_AUTOCLOSE_ENABLED; import java.io.Serializable; @@ -373,8 +372,7 @@ private DistributionSetAssignmentResult assignDistributionSetToTargets(final Str final JpaDistributionSet distributionSet = (JpaDistributionSet) distributionSetManagement.getValidAndComplete(dsId); - // implicit lock - if (!distributionSet.isLocked() && getConfigValue(IMPLICIT_LOCK_ENABLED, Boolean.class)) { + if (((JpaDistributionSetManagement)distributionSetManagement).isImplicitLockApplicable(distributionSet)) { // without new transaction DS changed event is not thrown DeploymentHelper.runInNewTransaction(txManager, "Implicit lock", status -> { distributionSetManagement.lock(distributionSet.getId()); diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaDistributionSetManagement.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaDistributionSetManagement.java index d02d82a6bd..8bec5c9020 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaDistributionSetManagement.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaDistributionSetManagement.java @@ -15,6 +15,7 @@ import java.util.Collections; import java.util.List; import java.util.Optional; +import java.util.Set; import java.util.function.BiFunction; import java.util.function.Supplier; import java.util.stream.Collectors; @@ -29,11 +30,11 @@ import org.eclipse.hawkbit.repository.DistributionSetTypeManagement; import org.eclipse.hawkbit.repository.OffsetBasedPageRequest; import org.eclipse.hawkbit.repository.QuotaManagement; +import org.eclipse.hawkbit.repository.RepositoryProperties; import org.eclipse.hawkbit.repository.SystemManagement; import org.eclipse.hawkbit.repository.builder.DistributionSetCreate; import org.eclipse.hawkbit.repository.builder.DistributionSetUpdate; import org.eclipse.hawkbit.repository.builder.GenericDistributionSetUpdate; -import org.eclipse.hawkbit.repository.event.remote.DistributionSetDeletedEvent; import org.eclipse.hawkbit.repository.exception.DeletedException; import org.eclipse.hawkbit.repository.exception.EntityAlreadyExistsException; import org.eclipse.hawkbit.repository.exception.EntityNotFoundException; @@ -45,7 +46,6 @@ import org.eclipse.hawkbit.repository.jpa.acm.AccessController; import org.eclipse.hawkbit.repository.jpa.builder.JpaDistributionSetCreate; import org.eclipse.hawkbit.repository.jpa.configuration.Constants; -import org.eclipse.hawkbit.repository.jpa.executor.AfterTransactionCommitExecutor; import org.eclipse.hawkbit.repository.jpa.model.DsMetadataCompositeKey; import org.eclipse.hawkbit.repository.jpa.model.JpaDistributionSet; import org.eclipse.hawkbit.repository.jpa.model.JpaDistributionSetMetadata; @@ -62,6 +62,7 @@ import org.eclipse.hawkbit.repository.jpa.rsql.RSQLUtility; import org.eclipse.hawkbit.repository.jpa.specifications.DistributionSetSpecification; import org.eclipse.hawkbit.repository.jpa.specifications.TargetSpecifications; +import org.eclipse.hawkbit.repository.jpa.utils.DeploymentHelper; import org.eclipse.hawkbit.repository.jpa.utils.QuotaHelper; import org.eclipse.hawkbit.repository.model.Action; import org.eclipse.hawkbit.repository.model.DistributionSet; @@ -73,9 +74,8 @@ import org.eclipse.hawkbit.repository.model.MetaData; import org.eclipse.hawkbit.repository.model.SoftwareModule; import org.eclipse.hawkbit.repository.model.Statistic; -import org.eclipse.hawkbit.repository.model.helper.EventPublisherHolder; import org.eclipse.hawkbit.repository.rsql.VirtualPropertyReplacer; -import org.eclipse.hawkbit.tenancy.TenantAware; +import org.eclipse.hawkbit.utils.TenantConfigHelper; import org.springframework.dao.ConcurrencyFailureException; import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; @@ -85,11 +85,14 @@ import org.springframework.orm.jpa.vendor.Database; import org.springframework.retry.annotation.Backoff; import org.springframework.retry.annotation.Retryable; +import org.springframework.transaction.PlatformTransactionManager; import org.springframework.transaction.annotation.Transactional; import org.springframework.util.CollectionUtils; import org.springframework.util.ObjectUtils; import org.springframework.validation.annotation.Validated; +import static org.eclipse.hawkbit.tenancy.configuration.TenantConfigurationProperties.TenantConfigurationKey.IMPLICIT_LOCK_ENABLED; + /** * JPA implementation of {@link DistributionSetManagement}. * @@ -99,51 +102,36 @@ public class JpaDistributionSetManagement implements DistributionSetManagement { private final EntityManager entityManager; - private final DistributionSetRepository distributionSetRepository; - private final DistributionSetTagManagement distributionSetTagManagement; - private final SystemManagement systemManagement; - private final DistributionSetTypeManagement distributionSetTypeManagement; - private final QuotaManagement quotaManagement; - private final DistributionSetMetadataRepository distributionSetMetadataRepository; private final TargetRepository targetRepository; - private final TargetFilterQueryRepository targetFilterQueryRepository; - private final ActionRepository actionRepository; - - private final EventPublisherHolder eventPublisherHolder; - - private final TenantAware tenantAware; - + private final TenantConfigHelper tenantConfigHelper; private final VirtualPropertyReplacer virtualPropertyReplacer; - private final SoftwareModuleRepository softwareModuleRepository; - private final DistributionSetTagRepository distributionSetTagRepository; - - private final AfterTransactionCommitExecutor afterCommit; - private final Database database; + private final RepositoryProperties repositoryProperties; - public JpaDistributionSetManagement(final EntityManager entityManager, + public JpaDistributionSetManagement( + final EntityManager entityManager, final DistributionSetRepository distributionSetRepository, final DistributionSetTagManagement distributionSetTagManagement, final SystemManagement systemManagement, final DistributionSetTypeManagement distributionSetTypeManagement, final QuotaManagement quotaManagement, final DistributionSetMetadataRepository distributionSetMetadataRepository, final TargetRepository targetRepository, final TargetFilterQueryRepository targetFilterQueryRepository, final ActionRepository actionRepository, - final EventPublisherHolder eventPublisherHolder, final TenantAware tenantAware, + final TenantConfigHelper tenantConfigHelper, final VirtualPropertyReplacer virtualPropertyReplacer, final SoftwareModuleRepository softwareModuleRepository, final DistributionSetTagRepository distributionSetTagRepository, - final AfterTransactionCommitExecutor afterCommit, - final Database database) { + final Database database, + final RepositoryProperties repositoryProperties) { this.entityManager = entityManager; this.distributionSetRepository = distributionSetRepository; this.distributionSetTagManagement = distributionSetTagManagement; @@ -154,13 +142,12 @@ public JpaDistributionSetManagement(final EntityManager entityManager, this.targetRepository = targetRepository; this.targetFilterQueryRepository = targetFilterQueryRepository; this.actionRepository = actionRepository; - this.eventPublisherHolder = eventPublisherHolder; - this.tenantAware = tenantAware; + this.tenantConfigHelper = tenantConfigHelper; this.virtualPropertyReplacer = virtualPropertyReplacer; this.softwareModuleRepository = softwareModuleRepository; this.distributionSetTagRepository = distributionSetTagRepository; - this.afterCommit = afterCommit; this.database = database; + this.repositoryProperties = repositoryProperties; } @Override @@ -679,7 +666,7 @@ private void checkAndThrowIfDistributionSetMetadataAlreadyExists(final DsMetadat @Transactional @Retryable(include = { ConcurrencyFailureException.class }, maxAttempts = Constants.TX_RT_MAX, backoff = @Backoff(delay = Constants.TX_RT_DELAY)) - public DistributionSet unAssignTag(final long id, final long dsTagId) { + public DistributionSet unassignTag(final long id, final long dsTagId) { final JpaDistributionSet set = (JpaDistributionSet) getWithDetails(id) .orElseThrow(() -> new EntityNotFoundException(DistributionSet.class, id)); @@ -830,6 +817,36 @@ public void invalidate(final DistributionSet distributionSet) { distributionSetRepository.save(jpaSet); } + // check if shall implicitly lock a distribution set + boolean isImplicitLockApplicable(final DistributionSet distributionSet) { + final JpaDistributionSet jpaDistributionSet = (JpaDistributionSet) distributionSet; + if (jpaDistributionSet.isLocked()) { + // already locked + return false; + } + + if (!tenantConfigHelper.getConfigValue(IMPLICIT_LOCK_ENABLED, Boolean.class)) { + // implicit lock disabled + return false; + } + + final List skipForTags = repositoryProperties.getSkipImplicitLockForTags(); + if (!ObjectUtils.isEmpty(skipForTags)) { + final Set tags = ((JpaDistributionSet)jpaDistributionSet).getTags(); + if (!ObjectUtils.isEmpty(tags)) { + for (final DistributionSetTag tag : tags) { + if (skipForTags.contains(tag.getName())) { + // has a skip tag + return false; + } + } + } + } + + // finally - implicit lock is applicable + return true; + } + private JpaDistributionSet getById(final long id) { return distributionSetRepository .findById(id) @@ -847,4 +864,4 @@ private void assertDsTagExists(final Long tagId) { throw new EntityNotFoundException(DistributionSetTag.class, tagId); } } -} +} \ No newline at end of file diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaRolloutManagement.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaRolloutManagement.java index d54037e12b..9255e94434 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaRolloutManagement.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaRolloutManagement.java @@ -10,7 +10,6 @@ package org.eclipse.hawkbit.repository.jpa.management; import static org.eclipse.hawkbit.repository.jpa.builder.JpaRolloutGroupCreate.addSuccessAndErrorConditionsAndActions; -import static org.eclipse.hawkbit.tenancy.configuration.TenantConfigurationProperties.TenantConfigurationKey.IMPLICIT_LOCK_ENABLED; import java.util.ArrayList; import java.util.Arrays; @@ -59,6 +58,7 @@ import org.eclipse.hawkbit.repository.jpa.rollout.condition.StartNextGroupRolloutGroupSuccessAction; import org.eclipse.hawkbit.repository.jpa.rsql.RSQLUtility; import org.eclipse.hawkbit.repository.jpa.specifications.RolloutSpecification; +import org.eclipse.hawkbit.repository.jpa.utils.DeploymentHelper; import org.eclipse.hawkbit.repository.jpa.utils.QuotaHelper; import org.eclipse.hawkbit.repository.jpa.utils.WeightValidationHelper; import org.eclipse.hawkbit.repository.model.DistributionSet; @@ -75,7 +75,6 @@ import org.eclipse.hawkbit.repository.model.helper.EventPublisherHolder; import org.eclipse.hawkbit.repository.rsql.VirtualPropertyReplacer; import org.eclipse.hawkbit.security.SystemSecurityContext; -import org.eclipse.hawkbit.utils.TenantConfigHelper; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.dao.ConcurrencyFailureException; import org.springframework.data.domain.Page; @@ -144,8 +143,6 @@ public class JpaRolloutManagement implements RolloutManagement { private final ContextAware contextAware; private final Database database; - private final TenantConfigHelper tenantConfigHelper; - public JpaRolloutManagement(final TargetManagement targetManagement, final DistributionSetManagement distributionSetManagement, final EventPublisherHolder eventPublisherHolder, final VirtualPropertyReplacer virtualPropertyReplacer, final Database database, @@ -162,8 +159,6 @@ public JpaRolloutManagement(final TargetManagement targetManagement, this.systemSecurityContext = systemSecurityContext; this.eventPublisherHolder = eventPublisherHolder; this.contextAware = contextAware; - - tenantConfigHelper = TenantConfigHelper.usingContext(systemSecurityContext, tenantConfigurationManagement); } @Override @@ -229,8 +224,7 @@ private JpaRollout createRollout(final JpaRollout rollout) { } rollout.setTotalTargets(totalTargets); - // implicit lock - if (!distributionSet.isLocked() && tenantConfigHelper.getConfigValue(IMPLICIT_LOCK_ENABLED, Boolean.class)) { + if (((JpaDistributionSetManagement)distributionSetManagement).isImplicitLockApplicable(distributionSet)) { distributionSetManagement.lock(distributionSet.getId()); } diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaTargetFilterQueryManagement.java b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaTargetFilterQueryManagement.java index e58f3c5df4..c8599e53af 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaTargetFilterQueryManagement.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/main/java/org/eclipse/hawkbit/repository/jpa/management/JpaTargetFilterQueryManagement.java @@ -9,8 +9,6 @@ */ package org.eclipse.hawkbit.repository.jpa.management; -import static org.eclipse.hawkbit.tenancy.configuration.TenantConfigurationProperties.TenantConfigurationKey.IMPLICIT_LOCK_ENABLED; - import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -77,20 +75,15 @@ public class JpaTargetFilterQueryManagement implements TargetFilterQueryManageme private final TargetFilterQueryRepository targetFilterQueryRepository; private final TargetManagement targetManagement; - private final VirtualPropertyReplacer virtualPropertyReplacer; - private final DistributionSetManagement distributionSetManagement; private final QuotaManagement quotaManagement; private final TenantConfigurationManagement tenantConfigurationManagement; private final RepositoryProperties repositoryProperties; private final SystemSecurityContext systemSecurityContext; private final ContextAware contextAware; - private final Database database; - private final TenantConfigHelper tenantConfigHelper; - public JpaTargetFilterQueryManagement(final TargetFilterQueryRepository targetFilterQueryRepository, final TargetManagement targetManagement, final VirtualPropertyReplacer virtualPropertyReplacer, final DistributionSetManagement distributionSetManagement, final QuotaManagement quotaManagement, @@ -107,8 +100,6 @@ public JpaTargetFilterQueryManagement(final TargetFilterQueryRepository targetFi this.repositoryProperties = repositoryProperties; this.systemSecurityContext = systemSecurityContext; this.contextAware = contextAware; - - tenantConfigHelper = TenantConfigHelper.usingContext(systemSecurityContext, tenantConfigurationManagement); } @Override @@ -285,8 +276,7 @@ public TargetFilterQuery updateAutoAssignDS(final AutoAssignDistributionSetUpdat final JpaDistributionSet distributionSet = (JpaDistributionSet) distributionSetManagement .getValidAndComplete(update.getDsId()); - // implicit lock - if (!distributionSet.isLocked() && tenantConfigHelper.getConfigValue(IMPLICIT_LOCK_ENABLED, Boolean.class)) { + if (((JpaDistributionSetManagement)distributionSetManagement).isImplicitLockApplicable(distributionSet)) { distributionSetManagement.lock(distributionSet.getId()); } diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/controller/DistributionSetAccessControllerTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/controller/DistributionSetAccessControllerTest.java index cbd4ef5961..456c25843a 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/controller/DistributionSetAccessControllerTest.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/acm/controller/DistributionSetAccessControllerTest.java @@ -31,7 +31,6 @@ import org.eclipse.hawkbit.repository.model.DistributionSetFilter; import org.eclipse.hawkbit.repository.model.DistributionSetTag; import org.eclipse.hawkbit.repository.model.SoftwareModule; -import org.eclipse.hawkbit.repository.model.Target; import org.eclipse.hawkbit.repository.model.TargetFilterQuery; import org.junit.jupiter.api.Test; import org.springframework.data.domain.Pageable; @@ -219,7 +218,7 @@ void verifyTagFilteringAndManagement() { assertThat(distributionSetManagement.assignTag(Collections.singletonList(permitted.getId()), dsTag.getId())) .hasSize(1); // verify distributionSetManagement#unAssignTag on permitted target - assertThat(distributionSetManagement.unAssignTag(permitted.getId(), dsTag.getId()).getId()) + assertThat(distributionSetManagement.unassignTag(permitted.getId(), dsTag.getId()).getId()) .isEqualTo(permitted.getId()); // assignment is denied for readOnlyTarget (read, but no update permissions) @@ -237,7 +236,7 @@ void verifyTagFilteringAndManagement() { // assignment is denied for readOnlyTarget (read, but no update permissions) assertThatThrownBy(() -> { - distributionSetManagement.unAssignTag(readOnly.getId(), dsTag.getId()); + distributionSetManagement.unassignTag(readOnly.getId(), dsTag.getId()); }).as("Missing update permissions for target to toggle tag assignment.") .isInstanceOf(InsufficientPermissionException.class); @@ -256,7 +255,7 @@ void verifyTagFilteringAndManagement() { // assignment is denied for hiddenTarget since it's hidden assertThatThrownBy(() -> { - distributionSetManagement.unAssignTag(hidden.getId(), dsTag.getId()); + distributionSetManagement.unassignTag(hidden.getId(), dsTag.getId()); }).as("Missing update permissions for target to toggle tag assignment.") .isInstanceOf(EntityNotFoundException.class); } diff --git a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/DistributionSetManagementTest.java b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/DistributionSetManagementTest.java index 68f1fd378d..b3462f7d9b 100644 --- a/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/DistributionSetManagementTest.java +++ b/hawkbit-repository/hawkbit-repository-jpa/src/test/java/org/eclipse/hawkbit/repository/jpa/management/DistributionSetManagementTest.java @@ -31,6 +31,7 @@ import org.apache.commons.lang3.RandomStringUtils; import org.assertj.core.api.Condition; import org.eclipse.hawkbit.repository.DistributionSetManagement; +import org.eclipse.hawkbit.repository.RepositoryProperties; import org.eclipse.hawkbit.repository.builder.DistributionSetCreate; import org.eclipse.hawkbit.repository.event.remote.entity.DistributionSetCreatedEvent; import org.eclipse.hawkbit.repository.event.remote.entity.DistributionSetTagCreatedEvent; @@ -67,6 +68,7 @@ import org.eclipse.hawkbit.repository.test.matcher.ExpectEvents; import org.eclipse.hawkbit.repository.test.util.WithUser; import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.data.domain.Page; import org.springframework.data.domain.PageRequest; import org.springframework.data.domain.Sort; @@ -142,10 +144,10 @@ void entityQueriesReferringToNotExistingEntitiesThrowsException() { () -> distributionSetManagement.toggleTagAssignment(singletonList(set.getId()), NOT_EXIST_ID), "DistributionSetTag"); - verifyThrownExceptionBy(() -> distributionSetManagement.unAssignTag(set.getId(), NOT_EXIST_IDL), + verifyThrownExceptionBy(() -> distributionSetManagement.unassignTag(set.getId(), NOT_EXIST_IDL), "DistributionSetTag"); - verifyThrownExceptionBy(() -> distributionSetManagement.unAssignTag(NOT_EXIST_IDL, dsTag.getId()), + verifyThrownExceptionBy(() -> distributionSetManagement.unassignTag(NOT_EXIST_IDL, dsTag.getId()), "DistributionSet"); verifyThrownExceptionBy( @@ -424,7 +426,7 @@ void assignAndUnassignDistributionSetToTag() { .isEqualTo(distributionSetManagement.findByTag(PAGE, tag.getId()).getNumberOfElements()); final JpaDistributionSet unAssignDS = (JpaDistributionSet) distributionSetManagement - .unAssignTag(assignDS.get(0), findDistributionSetTag.getId()); + .unassignTag(assignDS.get(0), findDistributionSetTag.getId()); assertThat(unAssignDS.getId()).as("unassigned ds is wrong").isEqualTo(assignDS.get(0)); assertThat(unAssignDS.getTags().size()).as("unassigned ds has wrong tag size").isZero(); assertThat(distributionSetTagManagement.getByName(TAG1_NAME)).isPresent(); @@ -1042,11 +1044,9 @@ void lockDistributionSetApplied() { assertThat(softwareModuleCount).isNotEqualTo(0); distributionSetManagement.lock(distributionSet.getId()); assertThat( - distributionSetManagement.get(distributionSet.getId()).map(DistributionSet::isLocked) - .orElse(false)) + distributionSetManagement.get(distributionSet.getId()).map(DistributionSet::isLocked).orElse(false)) .isTrue(); - // try add assertThatExceptionOfType(LockedException.class) .as("Attempt to modify a locked DS software modules should throw an exception") @@ -1066,6 +1066,32 @@ void lockDistributionSetApplied() { .isEqualTo(softwareModuleCount); } + @Autowired RepositoryProperties repositoryProperties; + @Test + @Description("Test implicit locks for a DS and skip tags.") + void isImplicitLockApplicableDistributionSet() { + final JpaDistributionSetManagement distributionSetManagement = + (JpaDistributionSetManagement)this.distributionSetManagement; + final DistributionSet distributionSet = testdataFactory.createDistributionSet("ds-non-skip"); + // assert that implicit lock is applicable for non skip tags + assertThat(distributionSetManagement.isImplicitLockApplicable(distributionSet)).isTrue(); + + assertThat(repositoryProperties.getSkipImplicitLockForTags().size()).isNotEqualTo(0); + final List skipTags = distributionSetTagManagement.create( + repositoryProperties.getSkipImplicitLockForTags().stream() + .map(skipTag -> entityFactory.tag().create().name(skipTag)) + .toList()); + // assert that implicit lock locks for every skip tag + skipTags.forEach(skipTag -> { + DistributionSet distributionSetWithSkipTag = + testdataFactory.createDistributionSet("ds-skip-" + skipTag.getName()); + distributionSetManagement.assignTag(List.of(distributionSetWithSkipTag.getId()), skipTag.getId()); + distributionSetWithSkipTag = distributionSetManagement.get(distributionSetWithSkipTag.getId()).orElseThrow(); + // assert that implicit lock isn't applicable for skip tags + assertThat(distributionSetManagement.isImplicitLockApplicable(distributionSetWithSkipTag)).isFalse(); + }); + } + @Test @Description("Locks an incomplete DS. Expected behaviour is to throw an exception and to do not lock it.") void lockIncompleteDistributionSetFails() { diff --git a/hawkbit-rest/hawkbit-mgmt-resource/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtDistributionSetTagResource.java b/hawkbit-rest/hawkbit-mgmt-resource/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtDistributionSetTagResource.java index 076a57ba94..81f8514296 100644 --- a/hawkbit-rest/hawkbit-mgmt-resource/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtDistributionSetTagResource.java +++ b/hawkbit-rest/hawkbit-mgmt-resource/src/main/java/org/eclipse/hawkbit/mgmt/rest/resource/MgmtDistributionSetTagResource.java @@ -202,7 +202,7 @@ public ResponseEntity unassignDistributionSet( @PathVariable("distributionsetTagId") final Long distributionsetTagId, @PathVariable("distributionsetId") final Long distributionsetId) { log.debug("Unassign ds {} for ds tag {}", distributionsetId, distributionsetTagId); - this.distributionSetManagement.unAssignTag(distributionsetId, distributionsetTagId); + this.distributionSetManagement.unassignTag(distributionsetId, distributionsetTagId); return ResponseEntity.ok().build(); }