From f8d8acae98879f961fa4752631215197a9a8666c Mon Sep 17 00:00:00 2001 From: minwoox Date: Wed, 11 Dec 2024 12:26:16 +0900 Subject: [PATCH] Add more test --- .../server/metadata/MetadataService.java | 2 +- .../server/metadata/PerRolePermissions.java | 10 +++---- .../server/metadata/MetadataServiceTest.java | 26 +++++++++++++++++++ 3 files changed, 31 insertions(+), 7 deletions(-) diff --git a/server/src/main/java/com/linecorp/centraldogma/server/metadata/MetadataService.java b/server/src/main/java/com/linecorp/centraldogma/server/metadata/MetadataService.java index 7450458ec..22520c8ac 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/metadata/MetadataService.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/metadata/MetadataService.java @@ -987,7 +987,7 @@ public CompletableFuture> findPermissions(String projectN User user) { requireNonNull(user, "user"); if (user.isSystemAdmin()) { - return CompletableFuture.completedFuture(PerRolePermissions.ALL_PERMISSION); + return CompletableFuture.completedFuture(PerRolePermissions.READ_WRITE); } if (user instanceof UserWithToken) { return findPermissions(projectName, repoName, ((UserWithToken) user).token().appId()); diff --git a/server/src/main/java/com/linecorp/centraldogma/server/metadata/PerRolePermissions.java b/server/src/main/java/com/linecorp/centraldogma/server/metadata/PerRolePermissions.java index 418ba90a2..4cf2eae32 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/metadata/PerRolePermissions.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/metadata/PerRolePermissions.java @@ -19,7 +19,6 @@ import static java.util.Objects.requireNonNull; import java.util.Collection; -import java.util.EnumSet; import java.util.Objects; import java.util.Set; @@ -28,6 +27,7 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.base.MoreObjects; +import com.google.common.collect.ImmutableList; import com.google.common.collect.Sets; import com.linecorp.centraldogma.common.ProjectRole; @@ -41,11 +41,9 @@ public final class PerRolePermissions { /** * {@link Permission}s for administrators. */ - public static final Collection ALL_PERMISSION = EnumSet.allOf(Permission.class); - - public static final Collection READ_WRITE = EnumSet.of(Permission.READ, Permission.WRITE); - public static final Collection READ_ONLY = EnumSet.of(Permission.READ); - public static final Collection NO_PERMISSION = EnumSet.noneOf(Permission.class); + public static final Collection READ_WRITE = ImmutableList.of(Permission.READ, Permission.WRITE); + public static final Collection READ_ONLY = ImmutableList.of(Permission.READ); + public static final Collection NO_PERMISSION = ImmutableList.of(); /** * The default permission. diff --git a/server/src/test/java/com/linecorp/centraldogma/server/metadata/MetadataServiceTest.java b/server/src/test/java/com/linecorp/centraldogma/server/metadata/MetadataServiceTest.java index b1159a69b..ef68724fd 100644 --- a/server/src/test/java/com/linecorp/centraldogma/server/metadata/MetadataServiceTest.java +++ b/server/src/test/java/com/linecorp/centraldogma/server/metadata/MetadataServiceTest.java @@ -201,6 +201,11 @@ void perRolePermissions() { assertThat(mds.findPermissions(project1, repo1, guest).join()) .containsExactlyElementsOf(NO_PERMISSION); + // Fail due to duplicated update. + assertThatThrownBy(() -> mds.updatePerRolePermissions( + author, project1, repo1, PerRolePermissions.ofPrivate()).join()) + .hasCauseInstanceOf(IllegalArgumentException.class); + assertThatThrownBy(() -> mds.updatePerRolePermissions( author, project1, REPO_DOGMA, PerRolePermissions.ofPublic()).join()) .isInstanceOf(UnsupportedOperationException.class) @@ -224,6 +229,11 @@ void perUserPermissions() { // Be a member of the project. mds.addMember(author, project1, user1, ProjectRole.MEMBER).join(); + // invalid repo. + assertThatThrownBy(() -> mds.addPerUserPermission( + author, project1, "invalid-repo", user1, READ_ONLY).join()) + .hasCauseInstanceOf(RepositoryNotFoundException.class); + // A member of the project has no permission. assertThat(mds.findPermissions(project1, repo1, user1).join()) .containsExactlyElementsOf(NO_PERMISSION); @@ -243,6 +253,14 @@ void perUserPermissions() { assertThat(mds.findPermissions(project1, repo1, user1).join()) .containsExactlyInAnyOrder(Permission.READ, Permission.WRITE); + // Update again with the same permission. + assertThatThrownBy(() -> mds.updatePerUserPermission(author, project1, repo1, user1, READ_WRITE).join()) + .hasCauseInstanceOf(IllegalArgumentException.class); + + // Update invalid user + assertThatThrownBy(() -> mds.updatePerUserPermission(author, project1, repo1, user2, READ_WRITE).join()) + .hasCauseInstanceOf(IllegalArgumentException.class); + mds.removePerUserPermission(author, project1, repo1, user1).join(); assertThatThrownBy(() -> mds.removePerUserPermission(author, project1, repo1, user1).join()) .hasCauseInstanceOf(IllegalArgumentException.class); @@ -278,11 +296,19 @@ void perTokenPermissions() { assertThat(mds.findPermissions(project1, repo1, app1).join()) .containsExactly(Permission.READ); + // Add again. + assertThatThrownBy(() -> mds.addPerTokenPermission(author, project1, repo1, app1, READ_ONLY).join()) + .hasCauseInstanceOf(IllegalArgumentException.class); + mds.updatePerTokenPermission(author, project1, repo1, app1, READ_WRITE).join(); assertThat(mds.findPermissions(project1, repo1, app1).join()) .containsExactlyInAnyOrder(Permission.READ, Permission.WRITE); + // Update again with the same permission. + assertThatThrownBy(() -> mds.updatePerTokenPermission(author, project1, repo1, app1, READ_WRITE).join()) + .hasCauseInstanceOf(IllegalArgumentException.class); + mds.removePerTokenPermission(author, project1, repo1, app1).join(); assertThatThrownBy(() -> mds.removePerTokenPermission(author, project1, repo1, app1).join()) .hasCauseInstanceOf(IllegalArgumentException.class);