[#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 <Avgustin.Marinov@bosch.com>
This commit is contained in:
Avgustin Marinov
2024-03-08 14:58:31 +02:00
committed by GitHub
parent 3f060e84a3
commit 936e6d6d66
10 changed files with 110 additions and 82 deletions

View File

@@ -31,7 +31,6 @@ import org.eclipse.hawkbit.repository.model.DistributionSet;
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 @@ class DistributionSetAccessControllerTest extends AbstractAccessControllerTest {
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 @@ class DistributionSetAccessControllerTest extends AbstractAccessControllerTest {
// 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 @@ class DistributionSetAccessControllerTest extends AbstractAccessControllerTest {
// 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);
}

View File

@@ -31,6 +31,7 @@ import jakarta.validation.ConstraintViolationException;
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.Expect;
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 @@ class DistributionSetManagementTest extends AbstractJpaIntegrationTest {
() -> 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 @@ class DistributionSetManagementTest extends AbstractJpaIntegrationTest {
.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 @@ class DistributionSetManagementTest extends AbstractJpaIntegrationTest {
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 @@ class DistributionSetManagementTest extends AbstractJpaIntegrationTest {
.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<DistributionSetTag> 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() {