Skip to content

Commit

Permalink
[#1651] Implement skip DistributionSet implicit lock on DS tags (#1680)
Browse files Browse the repository at this point in the history
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 <Avgustin.Marinov@bosch.com>
  • Loading branch information
avgustinmm committed Mar 8, 2024
1 parent 3f060e8 commit 936e6d6
Show file tree
Hide file tree
Showing 10 changed files with 110 additions and 82 deletions.
Expand Up @@ -505,7 +505,7 @@ Slice<DistributionSet> 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.
Expand Down
Expand Up @@ -9,6 +9,7 @@
*/
package org.eclipse.hawkbit.repository;

import java.util.List;
import java.util.concurrent.TimeUnit;

import lombok.Data;
Expand Down Expand Up @@ -68,4 +69,7 @@ public class RepositoryProperties {
private long dsInvalidationLockTimeout = 5;

private boolean implicitTenantCreateAllowed;

private List<String> skipImplicitLockForTags =
List.of("skip-implicit-lock", "skip_implicit_lock", "SKIP_IMPLICIT_LOCK", "SKIP-IMPLICIT-LOCK");
}
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}

/**
Expand Down
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
Expand Down
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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}.
*
Expand All @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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));

Expand Down Expand Up @@ -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<String> skipForTags = repositoryProperties.getSkipImplicitLockForTags();
if (!ObjectUtils.isEmpty(skipForTags)) {
final Set<DistributionSetTag> 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)
Expand All @@ -847,4 +864,4 @@ private void assertDsTagExists(final Long tagId) {
throw new EntityNotFoundException(DistributionSetTag.class, tagId);
}
}
}
}
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -162,8 +159,6 @@ public JpaRolloutManagement(final TargetManagement targetManagement,
this.systemSecurityContext = systemSecurityContext;
this.eventPublisherHolder = eventPublisherHolder;
this.contextAware = contextAware;

tenantConfigHelper = TenantConfigHelper.usingContext(systemSecurityContext, tenantConfigurationManagement);
}

@Override
Expand Down Expand Up @@ -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());
}

Expand Down

0 comments on commit 936e6d6

Please sign in to comment.