From fe31752684f5d758680bda3d3cb284eee0403395 Mon Sep 17 00:00:00 2001 From: songmw725 Date: Fri, 13 Dec 2024 12:26:41 +0900 Subject: [PATCH] Address the comments from @ikhoon --- .../server/command/ContentTransformer.java | 10 +- .../internal/api/HttpApiExceptionHandler.java | 8 +- .../api/auth/ApplicationTokenAuthorizer.java | 2 +- .../git/AbstractChangesApplier.java | 13 +- .../repository/git/CommitExecutor.java | 72 +-- .../repository/git/DefaultChangesApplier.java | 4 +- .../storage/repository/git/GitRepository.java | 2 +- .../git/TransformingChangesApplier.java | 9 +- .../metadata/MemberNotFoundException.java | 34 ++ .../server/metadata/MetadataService.java | 533 +++++++----------- .../server/metadata/ProjectMetadata.java | 3 +- .../metadata/ProjectMetadataTransformer.java | 47 ++ .../RepositoryMetadataTransformer.java | 57 ++ .../server/metadata/RepositorySupport.java | 12 +- .../TokenNotFoundException.java | 26 +- .../centraldogma/server/metadata/Tokens.java | 2 - .../server/metadata/TokensTransformer.java | 49 ++ .../StandaloneCommandExecutorTest.java | 4 +- .../ZooKeeperCommandExecutorTest.java | 10 +- .../server/metadata/MetadataServiceTest.java | 93 +-- 20 files changed, 530 insertions(+), 460 deletions(-) create mode 100644 server/src/main/java/com/linecorp/centraldogma/server/metadata/MemberNotFoundException.java create mode 100644 server/src/main/java/com/linecorp/centraldogma/server/metadata/ProjectMetadataTransformer.java create mode 100644 server/src/main/java/com/linecorp/centraldogma/server/metadata/RepositoryMetadataTransformer.java rename server/src/main/java/com/linecorp/centraldogma/server/{internal/admin/service => metadata}/TokenNotFoundException.java (53%) create mode 100644 server/src/main/java/com/linecorp/centraldogma/server/metadata/TokensTransformer.java diff --git a/server/src/main/java/com/linecorp/centraldogma/server/command/ContentTransformer.java b/server/src/main/java/com/linecorp/centraldogma/server/command/ContentTransformer.java index 0b874535fc..3e8d75d091 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/command/ContentTransformer.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/command/ContentTransformer.java @@ -18,25 +18,27 @@ import static com.google.common.base.Preconditions.checkArgument; import static java.util.Objects.requireNonNull; +import java.util.function.BiFunction; import java.util.function.Function; import com.google.common.base.MoreObjects; import com.linecorp.centraldogma.common.EntryType; +import com.linecorp.centraldogma.common.Revision; /** * A {@link Function} which is used for transforming the content at the specified path of the repository. */ -public final class ContentTransformer { +public class ContentTransformer { private final String path; private final EntryType entryType; - private final Function transformer; + private final BiFunction transformer; /** * Creates a new instance. */ - public ContentTransformer(String path, EntryType entryType, Function transformer) { + public ContentTransformer(String path, EntryType entryType, BiFunction transformer) { this.path = requireNonNull(path, "path"); checkArgument(entryType == EntryType.JSON, "entryType: %s (expected: %s)", entryType, EntryType.JSON); this.entryType = requireNonNull(entryType, "entryType"); @@ -60,7 +62,7 @@ public EntryType entryType() { /** * Returns the {@link Function} which transforms the content. */ - public Function transformer() { + public BiFunction transformer() { return transformer; } diff --git a/server/src/main/java/com/linecorp/centraldogma/server/internal/api/HttpApiExceptionHandler.java b/server/src/main/java/com/linecorp/centraldogma/server/internal/api/HttpApiExceptionHandler.java index 4a8acab3da..e0d12559bb 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/internal/api/HttpApiExceptionHandler.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/internal/api/HttpApiExceptionHandler.java @@ -48,9 +48,10 @@ import com.linecorp.centraldogma.common.RepositoryNotFoundException; import com.linecorp.centraldogma.common.RevisionNotFoundException; import com.linecorp.centraldogma.common.TooManyRequestsException; -import com.linecorp.centraldogma.server.internal.admin.service.TokenNotFoundException; import com.linecorp.centraldogma.server.internal.storage.RequestAlreadyTimedOutException; import com.linecorp.centraldogma.server.internal.storage.repository.RepositoryMetadataException; +import com.linecorp.centraldogma.server.metadata.MemberNotFoundException; +import com.linecorp.centraldogma.server.metadata.TokenNotFoundException; /** * A default {@link ExceptionHandlerFunction} of HTTP API. @@ -96,8 +97,9 @@ public final class HttpApiExceptionHandler implements ServerErrorHandler { (ctx, cause) -> newResponse(ctx, HttpStatus.NOT_FOUND, cause, "Revision %s does not exist.", cause.getMessage())) .put(TokenNotFoundException.class, - (ctx, cause) -> newResponse(ctx, HttpStatus.NOT_FOUND, cause, - "Token '%s' does not exist.", cause.getMessage())) + (ctx, cause) -> newResponse(ctx, HttpStatus.NOT_FOUND, cause, cause.getMessage())) + .put(MemberNotFoundException.class, + (ctx, cause) -> newResponse(ctx, HttpStatus.NOT_FOUND, cause, cause.getMessage())) .put(QueryExecutionException.class, (ctx, cause) -> newResponse(ctx, HttpStatus.BAD_REQUEST, cause)) .put(UnsupportedOperationException.class, diff --git a/server/src/main/java/com/linecorp/centraldogma/server/internal/api/auth/ApplicationTokenAuthorizer.java b/server/src/main/java/com/linecorp/centraldogma/server/internal/api/auth/ApplicationTokenAuthorizer.java index d310ee0a06..c15ba7f677 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/internal/api/auth/ApplicationTokenAuthorizer.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/internal/api/auth/ApplicationTokenAuthorizer.java @@ -37,9 +37,9 @@ import com.linecorp.armeria.server.auth.AuthTokenExtractors; import com.linecorp.armeria.server.auth.Authorizer; import com.linecorp.centraldogma.server.internal.admin.auth.AuthUtil; -import com.linecorp.centraldogma.server.internal.admin.service.TokenNotFoundException; import com.linecorp.centraldogma.server.internal.api.HttpApiUtil; import com.linecorp.centraldogma.server.metadata.Token; +import com.linecorp.centraldogma.server.metadata.TokenNotFoundException; import com.linecorp.centraldogma.server.metadata.Tokens; import com.linecorp.centraldogma.server.metadata.UserWithToken; diff --git a/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/git/AbstractChangesApplier.java b/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/git/AbstractChangesApplier.java index f404e9b723..31e1cdac73 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/git/AbstractChangesApplier.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/git/AbstractChangesApplier.java @@ -38,14 +38,13 @@ import com.linecorp.centraldogma.common.CentralDogmaException; import com.linecorp.centraldogma.common.Revision; import com.linecorp.centraldogma.internal.Jackson; -import com.linecorp.centraldogma.server.internal.admin.service.TokenNotFoundException; import com.linecorp.centraldogma.server.storage.StorageException; abstract class AbstractChangesApplier { private static final byte[] EMPTY_BYTE = new byte[0]; - int apply(Repository jGitRepository, @Nullable Revision baseRevision, + int apply(Repository jGitRepository, Revision headRevision, @Nullable ObjectId baseTreeId, DirCache dirCache) { try (ObjectInserter inserter = jGitRepository.newObjectInserter(); ObjectReader reader = jGitRepository.newObjectReader()) { @@ -59,16 +58,16 @@ int apply(Repository jGitRepository, @Nullable Revision baseRevision, builder.finish(); } - return doApply(dirCache, reader, inserter); - } catch (CentralDogmaException | TokenNotFoundException | IllegalArgumentException e) { + return doApply(headRevision, dirCache, reader, inserter); + } catch (CentralDogmaException e) { throw e; } catch (Exception e) { - throw new StorageException("failed to apply changes on revision: " + - (baseRevision != null ? baseRevision.major() : 0), e); + throw new StorageException("failed to apply changes on revision: " + headRevision.major(), e); } } - abstract int doApply(DirCache dirCache, ObjectReader reader, ObjectInserter inserter) throws IOException; + abstract int doApply(Revision headRevision, DirCache dirCache, + ObjectReader reader, ObjectInserter inserter) throws IOException; static void applyPathEdit(DirCache dirCache, PathEdit edit) { final DirCacheEditor e = dirCache.editor(); diff --git a/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/git/CommitExecutor.java b/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/git/CommitExecutor.java index df73572cf9..b3bccb8833 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/git/CommitExecutor.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/git/CommitExecutor.java @@ -41,6 +41,7 @@ import org.eclipse.jgit.treewalk.CanonicalTreeParser; import com.google.common.collect.ImmutableList; +import com.google.common.collect.Iterables; import com.linecorp.centraldogma.common.Author; import com.linecorp.centraldogma.common.CentralDogmaException; @@ -81,8 +82,8 @@ String summary() { return summary; } - void executeInitialCommit(Iterable> changes) { - commit(null, Revision.INIT, changes); + void executeInitialCommit() { + commit(null, Revision.INIT, ImmutableList.of()); } CommitResult execute(Revision baseRevision, @@ -112,58 +113,57 @@ CommitResult execute(Revision baseRevision, return CommitResult.of(res.revision, applyingChanges); } - RevisionAndEntries commit(@Nullable Revision prevRevision, Revision nextRevision, + RevisionAndEntries commit(@Nullable Revision headRevision, Revision nextRevision, Iterable> changes) { requireNonNull(nextRevision, "nextRevision"); requireNonNull(changes, "changes"); - assert prevRevision == null || prevRevision.major() > 0; + assert (headRevision == null && Iterables.isEmpty(changes)) || + (headRevision != null && headRevision.major() > 0); assert nextRevision.major() > 0; final Repository jGitRepository = gitRepository.jGitRepository(); try (ObjectInserter inserter = jGitRepository.newObjectInserter(); ObjectReader reader = jGitRepository.newObjectReader(); RevWalk revWalk = newRevWalk(reader)) { - final CommitIdDatabase commitIdDatabase = gitRepository.commitIdDatabase(); - final ObjectId prevTreeId = - prevRevision != null ? toTree(commitIdDatabase, revWalk, prevRevision) : null; - // The staging area that keeps the entries of the new tree. - // It starts with the entries of the tree at the prevRevision (or with no entries if the - // prevRevision is the initial commit), and then this method will apply the requested changes + // It starts with the entries of the tree at the headRevision (or with no entries if the + // headRevision is the initial commit), and then this method will apply the requested changes // to build the new tree. final DirCache dirCache = DirCache.newInCore(); - - // Apply the changes and retrieve the list of the affected files. - final int numEdits = new DefaultChangesApplier(changes) - .apply(jGitRepository, prevRevision, prevTreeId, dirCache); - - // Reject empty commit if necessary. final List diffEntries; - boolean isEmpty = numEdits == 0; - if (!isEmpty) { - // Even if there are edits, the resulting tree might be identical with the previous tree. - final CanonicalTreeParser p = new CanonicalTreeParser(); - p.reset(reader, prevTreeId); - final DiffFormatter diffFormatter = new DiffFormatter(null); - diffFormatter.setRepository(jGitRepository); - diffEntries = diffFormatter.scan(p, new DirCacheIterator(dirCache)); - isEmpty = diffEntries.isEmpty(); + + if (headRevision != null) { + final ObjectId prevTreeId = toTree(commitIdDatabase, revWalk, headRevision); + // Apply the changes and retrieve the list of the affected files. + final int numEdits = new DefaultChangesApplier(changes) + .apply(jGitRepository, headRevision, prevTreeId, dirCache); + // Reject empty commit if necessary. + boolean isEmpty = numEdits == 0; + if (!isEmpty) { + // Even if there are edits, the resulting tree might be identical with the previous tree. + final CanonicalTreeParser p = new CanonicalTreeParser(); + p.reset(reader, prevTreeId); + final DiffFormatter diffFormatter = new DiffFormatter(null); + diffFormatter.setRepository(jGitRepository); + diffEntries = diffFormatter.scan(p, new DirCacheIterator(dirCache)); + isEmpty = diffEntries.isEmpty(); + } else { + diffEntries = ImmutableList.of(); + } + if (!allowEmptyCommit && isEmpty) { + throw new RedundantChangeException( + headRevision, + "changes did not change anything in " + gitRepository.parent().name() + '/' + + gitRepository.name() + " at revision " + headRevision.major() + ": " + changes); + } } else { + // initial commit. diffEntries = ImmutableList.of(); } - if (!allowEmptyCommit && isEmpty) { - // prevRevision is not null when allowEmptyCommit is false. - assert prevRevision != null; - throw new RedundantChangeException( - prevRevision, - "changes did not change anything in " + gitRepository.parent().name() + '/' + - gitRepository.name() + " at revision " + prevRevision.major() + ": " + changes); - } - // flush the current index to repository and get the result tree object id. final ObjectId nextTreeId = dirCache.writeTree(inserter); @@ -182,8 +182,8 @@ RevisionAndEntries commit(@Nullable Revision prevRevision, Revision nextRevision commitBuilder.setMessage(CommitUtil.toJsonString(summary, detail, markup, nextRevision)); // if the head commit exists, use it as the parent commit. - if (prevRevision != null) { - commitBuilder.setParentId(commitIdDatabase.get(prevRevision)); + if (headRevision != null) { + commitBuilder.setParentId(commitIdDatabase.get(headRevision)); } final ObjectId nextCommitId = inserter.insert(commitBuilder); diff --git a/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/git/DefaultChangesApplier.java b/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/git/DefaultChangesApplier.java index 269f7a66f6..86c232801e 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/git/DefaultChangesApplier.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/git/DefaultChangesApplier.java @@ -42,6 +42,7 @@ import com.linecorp.centraldogma.common.Change; import com.linecorp.centraldogma.common.ChangeConflictException; +import com.linecorp.centraldogma.common.Revision; import com.linecorp.centraldogma.internal.Jackson; import com.linecorp.centraldogma.internal.Util; import com.linecorp.centraldogma.internal.jsonpatch.JsonPatch; @@ -58,7 +59,8 @@ final class DefaultChangesApplier extends AbstractChangesApplier { } @Override - int doApply(DirCache dirCache, ObjectReader reader, ObjectInserter inserter) throws IOException { + int doApply(Revision unused, DirCache dirCache, + ObjectReader reader, ObjectInserter inserter) throws IOException { int numEdits = 0; // loop over the specified changes. for (Change change : changes) { diff --git a/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/git/GitRepository.java b/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/git/GitRepository.java index 97087c954c..ec2334e510 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/git/GitRepository.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/git/GitRepository.java @@ -249,7 +249,7 @@ class GitRepository implements Repository { new CommitExecutor(this, creationTimeMillis, author, "Create a new repository", "", Markup.PLAINTEXT, true) - .executeInitialCommit(Collections.emptyList()); + .executeInitialCommit(); headRevision = Revision.INIT; success = true; diff --git a/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/git/TransformingChangesApplier.java b/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/git/TransformingChangesApplier.java index adbd4e8b08..20c208852a 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/git/TransformingChangesApplier.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/internal/storage/repository/git/TransformingChangesApplier.java @@ -33,9 +33,9 @@ import com.linecorp.centraldogma.common.CentralDogmaException; import com.linecorp.centraldogma.common.ChangeConflictException; import com.linecorp.centraldogma.common.EntryType; +import com.linecorp.centraldogma.common.Revision; import com.linecorp.centraldogma.internal.Jackson; import com.linecorp.centraldogma.server.command.ContentTransformer; -import com.linecorp.centraldogma.server.internal.admin.service.TokenNotFoundException; final class TransformingChangesApplier extends AbstractChangesApplier { @@ -49,7 +49,8 @@ final class TransformingChangesApplier extends AbstractChangesApplier { } @Override - int doApply(DirCache dirCache, ObjectReader reader, ObjectInserter inserter) throws IOException { + int doApply(Revision headRevision, DirCache dirCache, + ObjectReader reader, ObjectInserter inserter) throws IOException { final String changePath = transformer.path().substring(1); // Strip the leading '/'. final DirCacheEntry oldEntry = dirCache.getEntry(changePath); final byte[] oldContent = oldEntry != null ? reader.open(oldEntry.getObjectId()).getBytes() @@ -57,13 +58,13 @@ int doApply(DirCache dirCache, ObjectReader reader, ObjectInserter inserter) thr final JsonNode oldJsonNode = oldContent != null ? Jackson.readTree(oldContent) : JsonNodeFactory.instance.nullNode(); try { - final JsonNode newJsonNode = transformer.transformer().apply(oldJsonNode.deepCopy()); + final JsonNode newJsonNode = transformer.transformer().apply(headRevision, oldJsonNode.deepCopy()); requireNonNull(newJsonNode, "transformer.transformer().apply() returned null"); if (!Objects.equals(newJsonNode, oldJsonNode)) { applyPathEdit(dirCache, new InsertJson(changePath, inserter, newJsonNode)); return 1; } - } catch (CentralDogmaException | TokenNotFoundException | IllegalArgumentException e) { + } catch (CentralDogmaException e) { throw e; } catch (Exception e) { throw new ChangeConflictException("failed to transform the content: " + oldJsonNode + diff --git a/server/src/main/java/com/linecorp/centraldogma/server/metadata/MemberNotFoundException.java b/server/src/main/java/com/linecorp/centraldogma/server/metadata/MemberNotFoundException.java new file mode 100644 index 0000000000..54efc34309 --- /dev/null +++ b/server/src/main/java/com/linecorp/centraldogma/server/metadata/MemberNotFoundException.java @@ -0,0 +1,34 @@ +/* + * Copyright 2019 LINE Corporation + * + * LINE Corporation licenses this file to you under the Apache License, + * version 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ +package com.linecorp.centraldogma.server.metadata; + +import com.linecorp.centraldogma.common.CentralDogmaException; + +/** + * A {@link CentralDogmaException} that is raised when failed to find a {@link Member}. + */ +public final class MemberNotFoundException extends CentralDogmaException { + + private static final long serialVersionUID = 914551040812058495L; + + MemberNotFoundException(String memberId, String projectName) { + super("failed to find member " + memberId + " in '" + projectName + '\''); + } + + MemberNotFoundException(String memberId, String projectName, String repoName) { + super("failed to find member " + memberId + " in '" + projectName + '/' + repoName + '\''); + } +} 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 22520c8ac9..d5d131b3da 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 @@ -38,9 +38,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import com.fasterxml.jackson.core.JsonParseException; import com.fasterxml.jackson.core.JsonPointer; -import com.fasterxml.jackson.databind.JsonMappingException; import com.fasterxml.jackson.databind.JsonNode; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -50,9 +48,8 @@ import com.linecorp.centraldogma.common.Author; import com.linecorp.centraldogma.common.Change; import com.linecorp.centraldogma.common.ChangeConflictException; -import com.linecorp.centraldogma.common.EntryNotFoundException; -import com.linecorp.centraldogma.common.EntryType; import com.linecorp.centraldogma.common.ProjectRole; +import com.linecorp.centraldogma.common.RedundantChangeException; import com.linecorp.centraldogma.common.RepositoryExistsException; import com.linecorp.centraldogma.common.Revision; import com.linecorp.centraldogma.internal.Jackson; @@ -62,7 +59,6 @@ import com.linecorp.centraldogma.internal.jsonpatch.TestAbsenceOperation; import com.linecorp.centraldogma.server.QuotaConfig; import com.linecorp.centraldogma.server.command.CommandExecutor; -import com.linecorp.centraldogma.server.command.ContentTransformer; import com.linecorp.centraldogma.server.storage.project.Project; import com.linecorp.centraldogma.server.storage.project.ProjectManager; @@ -256,41 +252,26 @@ public CompletableFuture removeMember(Author author, String projectNam final String commitSummary = "Remove the member '" + memberId + "' from the project '" + projectName + '\''; - final ContentTransformer transformer = new ContentTransformer<>( - METADATA_JSON, EntryType.JSON, node -> { - final ProjectMetadata projectMetadata = projectMetadata(node); - projectMetadata.member(memberId); // Raises an exception if the member does not exist. - final Map members = projectMetadata.members(); - final ImmutableMap.Builder membersBuilder = - ImmutableMap.builderWithExpectedSize(members.size() - 1); - for (Entry entry : members.entrySet()) { - if (!entry.getKey().equals(memberId)) { - membersBuilder.put(entry); - } - } - final Map newMembers = membersBuilder.build(); + final ProjectMetadataTransformer transformer = + new ProjectMetadataTransformer((headRevision, projectMetadata) -> { + projectMetadata.member(memberId); // Raises an exception if the member does not exist. + final Map newMembers = + projectMetadata.members().entrySet().stream() + .filter(entry -> !entry.getKey().equals(memberId)) + .collect(toImmutableMap(Entry::getKey, Entry::getValue)); - final ImmutableMap newRepos = - removeMemberFromRepositories(projectMetadata, memberId); - return Jackson.valueToTree(new ProjectMetadata(projectMetadata.name(), - newRepos, - newMembers, - projectMetadata.tokens(), - projectMetadata.creation(), - projectMetadata.removal())); - }); + final ImmutableMap newRepos = + removeMemberFromRepositories(projectMetadata, memberId); + return new ProjectMetadata(projectMetadata.name(), + newRepos, + newMembers, + projectMetadata.tokens(), + projectMetadata.creation(), + projectMetadata.removal()); + }); return metadataRepo.push(projectName, Project.REPO_DOGMA, author, commitSummary, transformer); } - private static ProjectMetadata projectMetadata(JsonNode node) { - try { - return Jackson.treeToValue(node, ProjectMetadata.class); - } catch (JsonParseException | JsonMappingException e) { - // Should never reach here. - throw new Error(); - } - } - private static ImmutableMap removeMemberFromRepositories( ProjectMetadata projectMetadata, String memberId) { final ImmutableMap.Builder reposBuilder = @@ -473,48 +454,25 @@ public CompletableFuture updatePerRolePermissions(Author author, } final String commitSummary = "Update the role permission of the '" + repoName + "' in the project '" + projectName + '\''; - final ContentTransformer transformer = new ContentTransformer<>( - METADATA_JSON, EntryType.JSON, node -> { - final ProjectMetadata projectMetadata = projectMetadata(node); - final RepositoryMetadata repositoryMetadata = projectMetadata.repo(repoName); - assert repositoryMetadata.name().equals(repoName); + final RepositoryMetadataTransformer transformer = new RepositoryMetadataTransformer( + repoName, (headRevision, repositoryMetadata) -> { if (repositoryMetadata.perRolePermissions().equals(perRolePermissions)) { - throw new IllegalArgumentException( + throw new RedundantChangeException( + headRevision, "the role permission of '" + projectName + '/' + repoName + "' isn't changed."); } - return newProjectMetadata(projectMetadata, - new RepositoryMetadata(repositoryMetadata.name(), - perRolePermissions, - repositoryMetadata.perUserPermissions(), - repositoryMetadata.perTokenPermissions(), - repositoryMetadata.creation(), - repositoryMetadata.removal(), - repositoryMetadata.writeQuota())); + return new RepositoryMetadata(repositoryMetadata.name(), + perRolePermissions, + repositoryMetadata.perUserPermissions(), + repositoryMetadata.perTokenPermissions(), + repositoryMetadata.creation(), + repositoryMetadata.removal(), + repositoryMetadata.writeQuota()); }); return metadataRepo.push(projectName, Project.REPO_DOGMA, author, commitSummary, transformer); } - private static JsonNode newProjectMetadata(ProjectMetadata projectMetadata, - RepositoryMetadata repositoryMetadata) { - final ImmutableMap.Builder builder = - ImmutableMap.builderWithExpectedSize(projectMetadata.repos().size()); - for (Entry entry : projectMetadata.repos().entrySet()) { - if (entry.getKey().equals(repositoryMetadata.name())) { - builder.put(entry.getKey(), repositoryMetadata); - } else { - builder.put(entry); - } - } - final ImmutableMap newRepos = builder.build(); - return Jackson.valueToTree(new ProjectMetadata(projectMetadata.name(), - newRepos, - projectMetadata.members(), - projectMetadata.tokens(), - projectMetadata.creation(), - projectMetadata.removal())); - } - /** * Adds the specified {@link Token} to the specified {@code projectName}. */ @@ -534,8 +492,7 @@ public CompletableFuture addToken(Author author, String projectName, requireNonNull(role, "role"); return getTokens().thenCompose(tokens -> { - final Token token = tokens.appIds().get(appId); - checkArgument(token != null, "Token not found: " + appId); + tokens.get(appId); // Will raise an exception if not found. final TokenRegistration registration = new TokenRegistration(appId, role, UserAndTimestamp.of(author)); @@ -574,36 +531,31 @@ private CompletableFuture removeToken(String projectName, Author autho boolean quiet) { final String commitSummary = "Remove the token '" + appId + "' from the project '" + projectName + '\''; - final ContentTransformer transformer = new ContentTransformer<>( - METADATA_JSON, EntryType.JSON, node -> { - final ProjectMetadata projectMetadata = projectMetadata(node); + final ProjectMetadataTransformer transformer = + new ProjectMetadataTransformer((headRevision, projectMetadata) -> { final Map tokens = projectMetadata.tokens(); final Map newTokens; if (tokens.get(appId) == null) { if (!quiet) { - throw new EntryNotFoundException( + throw new TokenNotFoundException( "failed to find the token " + appId + " in project " + projectName); } newTokens = tokens; } else { - final ImmutableMap.Builder tokensBuilder = - ImmutableMap.builderWithExpectedSize(tokens.size() - 1); - for (Entry entry : tokens.entrySet()) { - if (!entry.getKey().equals(appId)) { - tokensBuilder.put(entry); - } - } - newTokens = tokensBuilder.build(); + newTokens = tokens.entrySet() + .stream() + .filter(entry -> !entry.getKey().equals(appId)) + .collect(toImmutableMap(Entry::getKey, Entry::getValue)); } final ImmutableMap newRepos = removeTokenFromRepositories(appId, projectMetadata); - return Jackson.valueToTree(new ProjectMetadata(projectMetadata.name(), - newRepos, - projectMetadata.members(), - newTokens, - projectMetadata.creation(), - projectMetadata.removal())); + return new ProjectMetadata(projectMetadata.name(), + newRepos, + projectMetadata.members(), + newTokens, + projectMetadata.creation(), + projectMetadata.removal()); }); return metadataRepo.push(projectName, Project.REPO_DOGMA, author, commitSummary, transformer); } @@ -673,15 +625,12 @@ public CompletableFuture addPerUserPermission(Author author, String pr ensureProjectMember(project, member); final String commitSummary = "Add permission of '" + member.id() + "' as '" + permission + "' to '" + projectName + '/' + repoName + "'\n"; - final ContentTransformer transformer = new ContentTransformer<>( - METADATA_JSON, EntryType.JSON, node -> { - final ProjectMetadata projectMetadata = projectMetadata(node); - final RepositoryMetadata repositoryMetadata = projectMetadata.repo(repoName); - assert repositoryMetadata.name().equals(repoName); + final RepositoryMetadataTransformer transformer = new RepositoryMetadataTransformer( + repoName, (headRevision, repositoryMetadata) -> { final Map> perUserPermissions = repositoryMetadata.perUserPermissions(); if (perUserPermissions.get(member.id()) != null) { - throw new IllegalArgumentException( + throw new ChangeConflictException( "the member " + member.id() + " is already added to '" + projectName + '/' + repoName + '\''); } @@ -693,14 +642,13 @@ public CompletableFuture addPerUserPermission(Author author, String pr .put(member.id(), permission) .build(); - return newProjectMetadata(projectMetadata, - new RepositoryMetadata(repositoryMetadata.name(), - repositoryMetadata.perRolePermissions(), - newPerUserPermissions, - repositoryMetadata.perTokenPermissions(), - repositoryMetadata.creation(), - repositoryMetadata.removal(), - repositoryMetadata.writeQuota())); + return new RepositoryMetadata(repositoryMetadata.name(), + repositoryMetadata.perRolePermissions(), + newPerUserPermissions, + repositoryMetadata.perTokenPermissions(), + repositoryMetadata.creation(), + repositoryMetadata.removal(), + repositoryMetadata.writeQuota()); }); return metadataRepo.push(projectName, Project.REPO_DOGMA, author, commitSummary, transformer); }); @@ -718,33 +666,25 @@ public CompletableFuture removePerUserPermission(Author author, String requireNonNull(member, "member"); final String memberId = member.id(); - final ContentTransformer transformer = new ContentTransformer<>( - METADATA_JSON, EntryType.JSON, node -> { - final ProjectMetadata projectMetadata = projectMetadata(node); - final RepositoryMetadata repositoryMetadata = projectMetadata.repo(repoName); - assert repositoryMetadata.name().equals(repoName); + final RepositoryMetadataTransformer transformer = new RepositoryMetadataTransformer( + repoName, (headRevision, repositoryMetadata) -> { final Map> perUserPermissions = repositoryMetadata.perUserPermissions(); if (perUserPermissions.get(memberId) == null) { - throw new IllegalArgumentException( - "the member " + memberId + " doesn't exist at '" + - projectName + '/' + repoName + '\''); + throw new MemberNotFoundException(memberId, projectName, repoName); } - final ImmutableMap.Builder> builder = - ImmutableMap.builderWithExpectedSize(perUserPermissions.size() - 1); - perUserPermissions.entrySet().stream() - .filter(entry -> !entry.getKey().equals(memberId)) - .forEach(builder::put); - final ImmutableMap> newPerUserPermissions = builder.build(); - return newProjectMetadata(projectMetadata, - new RepositoryMetadata(repositoryMetadata.name(), - repositoryMetadata.perRolePermissions(), - newPerUserPermissions, - repositoryMetadata.perTokenPermissions(), - repositoryMetadata.creation(), - repositoryMetadata.removal(), - repositoryMetadata.writeQuota())); + final ImmutableMap> newPerUserPermissions = + perUserPermissions.entrySet().stream() + .filter(entry -> !entry.getKey().equals(memberId)) + .collect(toImmutableMap(Map.Entry::getKey, Map.Entry::getValue)); + return new RepositoryMetadata(repositoryMetadata.name(), + repositoryMetadata.perRolePermissions(), + newPerUserPermissions, + repositoryMetadata.perTokenPermissions(), + repositoryMetadata.creation(), + repositoryMetadata.removal(), + repositoryMetadata.writeQuota()); }); final String commitSummary = "Remove permission of the '" + memberId + "' from '" + projectName + '/' + repoName + '\''; @@ -765,50 +705,52 @@ public CompletableFuture updatePerUserPermission(Author author, String requireNonNull(permission, "permission"); final String memberId = member.id(); - final ContentTransformer transformer = new ContentTransformer<>( - METADATA_JSON, EntryType.JSON, node -> { - final ProjectMetadata projectMetadata = projectMetadata(node); - final RepositoryMetadata repositoryMetadata = projectMetadata.repo(repoName); - assert repositoryMetadata.name().equals(repoName); + final RepositoryMetadataTransformer transformer = new RepositoryMetadataTransformer( + repoName, (headRevision, repositoryMetadata) -> { final Map> perUserPermissions = repositoryMetadata.perUserPermissions(); final Collection oldPermissions = perUserPermissions.get(memberId); if (oldPermissions == null) { - throw new IllegalArgumentException( - "the member " + memberId + " doesn't exist at '" + - projectName + '/' + repoName + '\''); + throw new MemberNotFoundException(memberId, projectName, repoName); } if (oldPermissions.equals(permission)) { - throw new IllegalArgumentException( + throw new RedundantChangeException( + headRevision, "the permission of " + memberId + " in '" + projectName + '/' + repoName + "' isn't changed."); } - final ImmutableMap.Builder> builder = - ImmutableMap.builderWithExpectedSize(perUserPermissions.size()); - for (Entry> entry : perUserPermissions.entrySet()) { - if (entry.getKey().equals(memberId)) { - builder.put(memberId, permission); - } else { - builder.put(entry); - } - } - final ImmutableMap> newPerUserPermissions = builder.build(); - return newProjectMetadata(projectMetadata, - new RepositoryMetadata(repositoryMetadata.name(), - repositoryMetadata.perRolePermissions(), - newPerUserPermissions, - repositoryMetadata.perTokenPermissions(), - repositoryMetadata.creation(), - repositoryMetadata.removal(), - repositoryMetadata.writeQuota())); + final ImmutableMap> newPerUserPermissions = + updatePermissions(permission, perUserPermissions, memberId); + return new RepositoryMetadata(repositoryMetadata.name(), + repositoryMetadata.perRolePermissions(), + newPerUserPermissions, + repositoryMetadata.perTokenPermissions(), + repositoryMetadata.creation(), + repositoryMetadata.removal(), + repositoryMetadata.writeQuota()); }); final String commitSummary = "Update permission of the '" + memberId + "' as '" + permission + "' for '" + projectName + '/' + repoName + '\''; return metadataRepo.push(projectName, Project.REPO_DOGMA, author, commitSummary, transformer); } + private static ImmutableMap> updatePermissions( + Collection permission, Map> perPermissions, + String id) { + final ImmutableMap.Builder> builder = + ImmutableMap.builderWithExpectedSize(perPermissions.size()); + for (Entry> entry : perPermissions.entrySet()) { + if (entry.getKey().equals(id)) { + builder.put(id, permission); + } else { + builder.put(entry); + } + } + return builder.build(); + } + /** * Adds {@link Permission}s for the {@link Token} of the specified {@code appId} to the specified * {@code repoName} in the specified {@code projectName}. @@ -826,32 +768,28 @@ public CompletableFuture addPerTokenPermission(Author author, String p ensureProjectToken(project, appId); final String commitSummary = "Add permission of the token '" + appId + "' as '" + permission + "' to '" + projectName + '/' + repoName + "'\n"; - final ContentTransformer transformer = new ContentTransformer<>( - METADATA_JSON, EntryType.JSON, node -> { - final ProjectMetadata projectMetadata = projectMetadata(node); - final RepositoryMetadata repositoryMetadata = projectMetadata.repo(repoName); - assert repositoryMetadata.name().equals(repoName); + final RepositoryMetadataTransformer transformer = new RepositoryMetadataTransformer( + repoName, (headRevision, repositoryMetadata) -> { final Map> perTokenPermissions = repositoryMetadata.perTokenPermissions(); if (perTokenPermissions.get(appId) != null) { - throw new IllegalArgumentException( + throw new ChangeConflictException( "the token " + appId + " is already added to '" + projectName + '/' + repoName + '\''); } - final ImmutableMap.Builder> builder = - ImmutableMap.builderWithExpectedSize(perTokenPermissions.size() + 1); - builder.putAll(perTokenPermissions); - builder.put(appId, permission); - final ImmutableMap> newPerTokenPermissions = builder.build(); - return newProjectMetadata(projectMetadata, - new RepositoryMetadata(repositoryMetadata.name(), - repositoryMetadata.perRolePermissions(), - repositoryMetadata.perUserPermissions(), - newPerTokenPermissions, - repositoryMetadata.creation(), - repositoryMetadata.removal(), - repositoryMetadata.writeQuota())); + final ImmutableMap> newPerTokenPermissions = + ImmutableMap.>builderWithExpectedSize( + perTokenPermissions.size() + 1) + .putAll(perTokenPermissions) + .put(appId, permission).build(); + return new RepositoryMetadata(repositoryMetadata.name(), + repositoryMetadata.perRolePermissions(), + repositoryMetadata.perUserPermissions(), + newPerTokenPermissions, + repositoryMetadata.creation(), + repositoryMetadata.removal(), + repositoryMetadata.writeQuota()); }); return metadataRepo.push(projectName, Project.REPO_DOGMA, author, commitSummary, transformer); }); @@ -868,34 +806,27 @@ public CompletableFuture removePerTokenPermission(Author author, Strin requireNonNull(repoName, "repoName"); requireNonNull(appId, "appId"); - final ContentTransformer transformer = new ContentTransformer<>( - METADATA_JSON, EntryType.JSON, node -> { - final ProjectMetadata projectMetadata = projectMetadata(node); - final RepositoryMetadata repositoryMetadata = projectMetadata.repo(repoName); - assert repositoryMetadata.name().equals(repoName); + final RepositoryMetadataTransformer transformer = new RepositoryMetadataTransformer( + repoName, (headRevision, repositoryMetadata) -> { final Map> perTokenPermissions = repositoryMetadata.perTokenPermissions(); if (perTokenPermissions.get(appId) == null) { - throw new IllegalArgumentException( + throw new ChangeConflictException( "the token " + appId + " doesn't exist at '" + projectName + '/' + repoName + '\''); } - final ImmutableMap.Builder> builder = - ImmutableMap.builderWithExpectedSize( - perTokenPermissions.size() - 1); - perTokenPermissions.entrySet().stream() - .filter(entry -> !entry.getKey().equals(appId)) - .forEach(builder::put); - final ImmutableMap> newPerTokenPermissions = builder.build(); - return newProjectMetadata(projectMetadata, - new RepositoryMetadata(repositoryMetadata.name(), - repositoryMetadata.perRolePermissions(), - repositoryMetadata.perUserPermissions(), - newPerTokenPermissions, - repositoryMetadata.creation(), - repositoryMetadata.removal(), - repositoryMetadata.writeQuota())); + final ImmutableMap> newPerTokenPermissions = + perTokenPermissions.entrySet().stream() + .filter(entry -> !entry.getKey().equals(appId)) + .collect(toImmutableMap(Map.Entry::getKey, Map.Entry::getValue)); + return new RepositoryMetadata(repositoryMetadata.name(), + repositoryMetadata.perRolePermissions(), + repositoryMetadata.perUserPermissions(), + newPerTokenPermissions, + repositoryMetadata.creation(), + repositoryMetadata.removal(), + repositoryMetadata.writeQuota()); }); final String commitSummary = "Remove permission of the token '" + appId + "' from '" + projectName + '/' + repoName + '\''; @@ -915,44 +846,33 @@ public CompletableFuture updatePerTokenPermission(Author author, Strin requireNonNull(appId, "appId"); requireNonNull(permission, "permission"); - final ContentTransformer transformer = new ContentTransformer<>( - METADATA_JSON, EntryType.JSON, node -> { - final ProjectMetadata projectMetadata = projectMetadata(node); - final RepositoryMetadata repositoryMetadata = projectMetadata.repo(repoName); - assert repositoryMetadata.name().equals(repoName); + final RepositoryMetadataTransformer transformer = new RepositoryMetadataTransformer( + repoName, (headRevision, repositoryMetadata) -> { final Map> perTokenPermissions = repositoryMetadata.perTokenPermissions(); final Collection oldPermissions = perTokenPermissions.get(appId); if (oldPermissions == null) { - throw new IllegalArgumentException( + throw new TokenNotFoundException( "the token " + appId + " doesn't exist at '" + projectName + '/' + repoName + '\''); } if (oldPermissions.equals(permission)) { - throw new IllegalArgumentException( + throw new RedundantChangeException( + headRevision, "the permission of " + appId + " in '" + projectName + '/' + repoName + "' isn't changed."); } - final ImmutableMap.Builder> builder = - ImmutableMap.builderWithExpectedSize(perTokenPermissions.size()); - for (Entry> entry : perTokenPermissions.entrySet()) { - if (entry.getKey().equals(appId)) { - builder.put(appId, permission); - } else { - builder.put(entry); - } - } - final ImmutableMap> newPerTokenPermissions = builder.build(); - return newProjectMetadata(projectMetadata, - new RepositoryMetadata(repositoryMetadata.name(), - repositoryMetadata.perRolePermissions(), - repositoryMetadata.perUserPermissions(), - newPerTokenPermissions, - repositoryMetadata.creation(), - repositoryMetadata.removal(), - repositoryMetadata.writeQuota())); + final ImmutableMap> newPerTokenPermissions = + updatePermissions(permission, perTokenPermissions, appId); + return new RepositoryMetadata(repositoryMetadata.name(), + repositoryMetadata.perRolePermissions(), + repositoryMetadata.perUserPermissions(), + newPerTokenPermissions, + repositoryMetadata.creation(), + repositoryMetadata.removal(), + repositoryMetadata.writeQuota()); }); final String commitSummary = "Update permission of the token '" + appId + "' for '" + projectName + '/' + repoName + '\''; @@ -1147,35 +1067,38 @@ public CompletableFuture destroyToken(Author author, String appId) { requireNonNull(author, "author"); requireNonNull(appId, "appId"); - final String commitSummary = "Delete the token: " + appId; + final String commitSummary = "Destroy the token: " + appId; final UserAndTimestamp userAndTimestamp = UserAndTimestamp.of(author); - final ContentTransformer transformer = new ContentTransformer<>( - TOKEN_JSON, EntryType.JSON, node -> { - final Tokens tokens = tokens(node); + final TokensTransformer transformer = new TokensTransformer((headRevision, tokens) -> { final Token token = tokens.get(appId); // Raise an exception if not found. if (token.deletion() != null) { - throw new IllegalArgumentException("The token is already deleted: " + appId); + throw new ChangeConflictException("The token is already destroyed: " + appId); } - final ImmutableMap.Builder appIdsBuilder = ImmutableMap.builder(); - for (Entry entry : tokens.appIds().entrySet()) { - if (!entry.getKey().equals(appId)) { - appIdsBuilder.put(entry); - } else { - final String secret = token.secret(); - assert secret != null; - appIdsBuilder.put(appId, new Token(token.appId(), secret, token.isSystemAdmin(), - token.isSystemAdmin(), token.creation(), - token.deactivation(), userAndTimestamp)); - } - } - final Map newAppIds = appIdsBuilder.build(); - return Jackson.valueToTree(new Tokens(newAppIds, tokens.secrets())); + final String secret = token.secret(); + assert secret != null; + final Token newToken = new Token(token.appId(), secret, token.isSystemAdmin(), + token.isSystemAdmin(), token.creation(), + token.deactivation(), userAndTimestamp); + return new Tokens(newAppIds(tokens, appId, newToken), tokens.secrets()); }); return tokenRepo.push(INTERNAL_PROJECT_DOGMA, Project.REPO_DOGMA, author, commitSummary, transformer); } + private static Map newAppIds(Tokens tokens, String appId, Token newToken) { + final ImmutableMap.Builder appIdsBuilder = + ImmutableMap.builderWithExpectedSize(tokens.appIds().size()); + for (Entry entry : tokens.appIds().entrySet()) { + if (!entry.getKey().equals(appId)) { + appIdsBuilder.put(entry); + } else { + appIdsBuilder.put(appId, newToken); + } + } + return appIdsBuilder.build(); + } + /** * Purges the {@link Token} of the specified {@code appId} that was removed before. * @@ -1202,46 +1125,22 @@ public Revision purgeToken(Author author, String appId) { final String commitSummary = "Remove the token: " + appId; - final ContentTransformer transformer = new ContentTransformer<>( - TOKEN_JSON, EntryType.JSON, node -> { - final Tokens tokens = tokens(node); - tokens.get(appId); // Raise an exception if not found. - final ImmutableMap.Builder appIdsBuilder = ImmutableMap.builder(); - for (Entry entry : tokens.appIds().entrySet()) { - if (!entry.getKey().equals(appId)) { - appIdsBuilder.put(entry); - } - } - final Map newAppIds = appIdsBuilder.build(); - final Map newSecrets = secretsWithout(appId, tokens); - - return Jackson.valueToTree(new Tokens(newAppIds, newSecrets)); + final TokensTransformer transformer = new TokensTransformer((headRevision, tokens) -> { + final Token token = tokens.get(appId); + final Map newAppIds = tokens.appIds().entrySet().stream() + .filter(entry -> !entry.getKey().equals(appId)) + .collect(toImmutableMap(Entry::getKey, Entry::getValue)); + final String secret = token.secret(); + assert secret != null; + final Map newSecrets = + tokens.secrets().entrySet().stream().filter(entry -> !entry.getKey().equals(secret)) + .collect(toImmutableMap(Entry::getKey, Entry::getValue)); + return new Tokens(newAppIds, newSecrets); }); return tokenRepo.push(INTERNAL_PROJECT_DOGMA, Project.REPO_DOGMA, author, commitSummary, transformer) .join(); } - private static Tokens tokens(JsonNode node) { - final Tokens tokens; - try { - tokens = Jackson.treeToValue(node, Tokens.class); - } catch (JsonParseException | JsonMappingException e) { - // Should never reach here. - throw new Error(e); - } - return tokens; - } - - private static Map secretsWithout(String appId, Tokens tokens) { - final ImmutableMap.Builder secretsBuilder = ImmutableMap.builder(); - for (Entry entry : tokens.secrets().entrySet()) { - if (!entry.getValue().equals(appId)) { - secretsBuilder.put(entry); - } - } - return secretsBuilder.build(); - } - /** * Activates the {@link Token} of the specified {@code appId}. */ @@ -1251,33 +1150,19 @@ public CompletableFuture activateToken(Author author, String appId) { final String commitSummary = "Enable the token: " + appId; - final ContentTransformer transformer = new ContentTransformer<>( - TOKEN_JSON, EntryType.JSON, node -> { - final Tokens tokens = tokens(node); + final TokensTransformer transformer = new TokensTransformer((headRevision, tokens) -> { final Token token = tokens.get(appId); // Raise an exception if not found. if (token.deactivation() == null) { - throw new IllegalArgumentException("The token is already activated: " + appId); + throw new RedundantChangeException(headRevision, "The token is already activated: " + appId); } final String secret = token.secret(); assert secret != null; - - final ImmutableMap.Builder appIdsBuilder = ImmutableMap.builder(); - for (Entry entry : tokens.appIds().entrySet()) { - if (!entry.getKey().equals(appId)) { - appIdsBuilder.put(entry); - } else { - appIdsBuilder.put(appId, new Token(token.appId(), secret, token.isSystemAdmin(), - token.creation())); - } - } - final Map newAppIds = appIdsBuilder.build(); - - final ImmutableMap.Builder secretsBuilder = ImmutableMap.builder(); - secretsBuilder.putAll(tokens.secrets()); - secretsBuilder.put(secret, appId); - final Map newSecrets = secretsBuilder.build(); - - return Jackson.valueToTree(new Tokens(newAppIds, newSecrets)); + final Map newSecrets = + ImmutableMap.builderWithExpectedSize(tokens.secrets().size() + 1) + .putAll(tokens.secrets()) + .put(secret, appId).build(); + final Token newToken = new Token(token.appId(), secret, token.isSystemAdmin(), token.creation()); + return new Tokens(newAppIds(tokens, appId, newToken), newSecrets); }); return tokenRepo.push(INTERNAL_PROJECT_DOGMA, Project.REPO_DOGMA, author, commitSummary, transformer); } @@ -1292,30 +1177,20 @@ public CompletableFuture deactivateToken(Author author, String appId) final String commitSummary = "Deactivate the token: " + appId; final UserAndTimestamp userAndTimestamp = UserAndTimestamp.of(author); - final ContentTransformer transformer = new ContentTransformer<>( - TOKEN_JSON, EntryType.JSON, node -> { - final Tokens tokens = tokens(node); - final Token token = tokens.get(appId); // Raise an exception if not found. + final TokensTransformer transformer = new TokensTransformer((headRevision, tokens) -> { + final Token token = tokens.get(appId); if (token.deactivation() != null) { - throw new IllegalArgumentException("The token is already deactivated: " + appId); + throw new RedundantChangeException(headRevision, "The token is already deactivated: " + appId); } final String secret = token.secret(); assert secret != null; - - final ImmutableMap.Builder appIdsBuilder = ImmutableMap.builder(); - for (Entry entry : tokens.appIds().entrySet()) { - if (!entry.getKey().equals(appId)) { - appIdsBuilder.put(entry); - } else { - appIdsBuilder.put(appId, new Token(token.appId(), secret, token.isSystemAdmin(), - token.isSystemAdmin(), - token.creation(), userAndTimestamp, null)); - } - } - final Map newAppIds = appIdsBuilder.build(); - final Map newSecrets = secretsWithout(appId, tokens); - - return Jackson.valueToTree(new Tokens(newAppIds, newSecrets)); + final Token newToken = new Token(token.appId(), secret, token.isSystemAdmin(), + token.isSystemAdmin(), token.creation(), userAndTimestamp, null); + final Map newAppIds = newAppIds(tokens, appId, newToken); + final Map newSecrets = + tokens.secrets().entrySet().stream().filter(entry -> !entry.getKey().equals(secret)) + .collect(toImmutableMap(Entry::getKey, Entry::getValue)); + return new Tokens(newAppIds, newSecrets); }); return tokenRepo.push(INTERNAL_PROJECT_DOGMA, Project.REPO_DOGMA, author, commitSummary, transformer); } @@ -1328,25 +1203,16 @@ public CompletableFuture updateTokenLevel(Author author, String appId, requireNonNull(appId, "appId"); final String commitSummary = "Update the token level: " + appId + " to " + (toBeSystemAdmin ? "admin" : "user"); - final ContentTransformer transformer = new ContentTransformer<>( - TOKEN_JSON, EntryType.JSON, node -> { - final Tokens tokens = tokens(node); + final TokensTransformer transformer = new TokensTransformer((headRevision, tokens) -> { final Token token = tokens.get(appId); // Raise an exception if not found. if (toBeSystemAdmin == token.isSystemAdmin()) { - throw new IllegalArgumentException( + throw new RedundantChangeException( + headRevision, "The token is already " + (toBeSystemAdmin ? "admin" : "user")); } - final ImmutableMap.Builder appIdsBuilder = ImmutableMap.builder(); - for (Entry entry : tokens.appIds().entrySet()) { - if (!entry.getKey().equals(appId)) { - appIdsBuilder.put(entry); - } else { - appIdsBuilder.put(appId, token.withSystemAdmin(toBeSystemAdmin)); - } - } - final Map newAppIds = appIdsBuilder.build(); - return Jackson.valueToTree(new Tokens(newAppIds, tokens.secrets())); + return new Tokens(newAppIds(tokens, appId, token.withSystemAdmin(toBeSystemAdmin)), + tokens.secrets()); }); return tokenRepo.push(INTERNAL_PROJECT_DOGMA, Project.REPO_DOGMA, author, commitSummary, transformer); } @@ -1377,8 +1243,9 @@ private static void ensureProjectMember(ProjectMetadata project, User user) { requireNonNull(project, "project"); requireNonNull(user, "user"); - checkArgument(project.members().values().stream().anyMatch(member -> member.login().equals(user.id())), - user.id() + " is not a member of the project '" + project.name() + '\''); + if (project.members().values().stream().noneMatch(member -> member.login().equals(user.id()))) { + throw new MemberNotFoundException(user.id(), project.name()); + } } /** @@ -1388,7 +1255,9 @@ private static void ensureProjectToken(ProjectMetadata project, String appId) { requireNonNull(project, "project"); requireNonNull(appId, "appId"); - checkArgument(project.tokens().containsKey(appId), - appId + " is not a token of the project '" + project.name() + '\''); + if (!project.tokens().containsKey(appId)) { + throw new TokenNotFoundException( + appId + " is not a token of the project '" + project.name() + '\''); + } } } diff --git a/server/src/main/java/com/linecorp/centraldogma/server/metadata/ProjectMetadata.java b/server/src/main/java/com/linecorp/centraldogma/server/metadata/ProjectMetadata.java index 811d9f33fd..2a1fec8f60 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/metadata/ProjectMetadata.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/metadata/ProjectMetadata.java @@ -29,7 +29,6 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.base.MoreObjects; -import com.linecorp.centraldogma.common.EntryNotFoundException; import com.linecorp.centraldogma.common.RepositoryNotFoundException; import com.linecorp.centraldogma.server.storage.project.Project; @@ -163,7 +162,7 @@ public Member member(String memberId) { if (member != null) { return member; } - throw new EntryNotFoundException("failed to find member " + memberId + " in project " + name()); + throw new MemberNotFoundException(memberId, name()); } /** diff --git a/server/src/main/java/com/linecorp/centraldogma/server/metadata/ProjectMetadataTransformer.java b/server/src/main/java/com/linecorp/centraldogma/server/metadata/ProjectMetadataTransformer.java new file mode 100644 index 0000000000..574f5fa05a --- /dev/null +++ b/server/src/main/java/com/linecorp/centraldogma/server/metadata/ProjectMetadataTransformer.java @@ -0,0 +1,47 @@ +/* + * Copyright 2024 LINE Corporation + * + * LINE Corporation licenses this file to you under the Apache License, + * version 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ +package com.linecorp.centraldogma.server.metadata; + +import static com.linecorp.centraldogma.server.metadata.MetadataService.METADATA_JSON; + +import java.util.function.BiFunction; + +import com.fasterxml.jackson.core.JsonParseException; +import com.fasterxml.jackson.databind.JsonMappingException; +import com.fasterxml.jackson.databind.JsonNode; + +import com.linecorp.centraldogma.common.EntryType; +import com.linecorp.centraldogma.common.Revision; +import com.linecorp.centraldogma.internal.Jackson; +import com.linecorp.centraldogma.server.command.ContentTransformer; + +class ProjectMetadataTransformer extends ContentTransformer { + + ProjectMetadataTransformer(BiFunction transformer) { + super(METADATA_JSON, EntryType.JSON, + (headRevision, jsonNode) -> Jackson.valueToTree( + transformer.apply(headRevision, projectMetadata(jsonNode)))); + } + + private static ProjectMetadata projectMetadata(JsonNode node) { + try { + return Jackson.treeToValue(node, ProjectMetadata.class); + } catch (JsonParseException | JsonMappingException e) { + // Should never reach here. + throw new Error(); + } + } +} diff --git a/server/src/main/java/com/linecorp/centraldogma/server/metadata/RepositoryMetadataTransformer.java b/server/src/main/java/com/linecorp/centraldogma/server/metadata/RepositoryMetadataTransformer.java new file mode 100644 index 0000000000..7e2029af95 --- /dev/null +++ b/server/src/main/java/com/linecorp/centraldogma/server/metadata/RepositoryMetadataTransformer.java @@ -0,0 +1,57 @@ +/* + * Copyright 2024 LINE Corporation + * + * LINE Corporation licenses this file to you under the Apache License, + * version 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ +package com.linecorp.centraldogma.server.metadata; + +import java.util.Map.Entry; +import java.util.function.BiFunction; + +import com.google.common.collect.ImmutableMap; + +import com.linecorp.centraldogma.common.Revision; + +final class RepositoryMetadataTransformer extends ProjectMetadataTransformer { + + RepositoryMetadataTransformer(String repoName, + BiFunction transformer) { + super((headRevision, projectMetadata) -> { + final RepositoryMetadata repositoryMetadata = projectMetadata.repo(repoName); + assert repositoryMetadata.name().equals(repoName); + final RepositoryMetadata newRepositoryMetadata = + transformer.apply(headRevision, repositoryMetadata); + return newProjectMetadata(projectMetadata, newRepositoryMetadata); + }); + } + + private static ProjectMetadata newProjectMetadata(ProjectMetadata projectMetadata, + RepositoryMetadata repositoryMetadata) { + final ImmutableMap.Builder builder = + ImmutableMap.builderWithExpectedSize(projectMetadata.repos().size()); + for (Entry entry : projectMetadata.repos().entrySet()) { + if (entry.getKey().equals(repositoryMetadata.name())) { + builder.put(entry.getKey(), repositoryMetadata); + } else { + builder.put(entry); + } + } + final ImmutableMap newRepos = builder.build(); + return new ProjectMetadata(projectMetadata.name(), + newRepos, + projectMetadata.members(), + projectMetadata.tokens(), + projectMetadata.creation(), + projectMetadata.removal()); + } +} diff --git a/server/src/main/java/com/linecorp/centraldogma/server/metadata/RepositorySupport.java b/server/src/main/java/com/linecorp/centraldogma/server/metadata/RepositorySupport.java index f31792e42c..bbeb3df24e 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/metadata/RepositorySupport.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/metadata/RepositorySupport.java @@ -29,6 +29,7 @@ import com.linecorp.centraldogma.common.Change; import com.linecorp.centraldogma.common.Entry; import com.linecorp.centraldogma.common.Markup; +import com.linecorp.centraldogma.common.RedundantChangeException; import com.linecorp.centraldogma.common.Revision; import com.linecorp.centraldogma.internal.Jackson; import com.linecorp.centraldogma.server.command.Command; @@ -114,7 +115,16 @@ CompletableFuture push(String projectName, String repoName, return executor.execute(Command.transform(null, author, projectName, repoName, Revision.HEAD, commitSummary, "", Markup.PLAINTEXT, transformer)) - .thenApply(CommitResult::revision); + .thenApply(CommitResult::revision) + .exceptionally(cause -> { + final Throwable peeled = Exceptions.peel(cause); + if (peeled instanceof RedundantChangeException) { + final Revision revision = ((RedundantChangeException) peeled).headRevision(); + assert revision != null; + return revision; + } + return Exceptions.throwUnsafely(peeled); + }); } Revision normalize(Repository repository) { diff --git a/server/src/main/java/com/linecorp/centraldogma/server/internal/admin/service/TokenNotFoundException.java b/server/src/main/java/com/linecorp/centraldogma/server/metadata/TokenNotFoundException.java similarity index 53% rename from server/src/main/java/com/linecorp/centraldogma/server/internal/admin/service/TokenNotFoundException.java rename to server/src/main/java/com/linecorp/centraldogma/server/metadata/TokenNotFoundException.java index dfc041cf4c..f0b839066f 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/internal/admin/service/TokenNotFoundException.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/metadata/TokenNotFoundException.java @@ -14,28 +14,18 @@ * under the License. */ -package com.linecorp.centraldogma.server.internal.admin.service; +package com.linecorp.centraldogma.server.metadata; -public class TokenNotFoundException extends RuntimeException { +import com.linecorp.centraldogma.common.CentralDogmaException; - private static final long serialVersionUID = 7795045154004749414L; +/** + * A {@link CentralDogmaException} that is raised when failed to find a {@link Token}. + */ +public final class TokenNotFoundException extends CentralDogmaException { - public TokenNotFoundException() {} + private static final long serialVersionUID = 7795045154004749414L; - public TokenNotFoundException(String message) { + TokenNotFoundException(String message) { super(message); } - - public TokenNotFoundException(Throwable cause) { - super(cause); - } - - public TokenNotFoundException(String message, Throwable cause) { - super(message, cause); - } - - protected TokenNotFoundException(String message, Throwable cause, boolean enableSuppression, - boolean writableStackTrace) { - super(message, cause, enableSuppression, writableStackTrace); - } } diff --git a/server/src/main/java/com/linecorp/centraldogma/server/metadata/Tokens.java b/server/src/main/java/com/linecorp/centraldogma/server/metadata/Tokens.java index 6f008358ad..f5d2f4527f 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/metadata/Tokens.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/metadata/Tokens.java @@ -33,8 +33,6 @@ import com.google.common.base.MoreObjects; import com.google.common.collect.ImmutableMap; -import com.linecorp.centraldogma.server.internal.admin.service.TokenNotFoundException; - /** * Holds a token map and a secret map for fast lookup. */ diff --git a/server/src/main/java/com/linecorp/centraldogma/server/metadata/TokensTransformer.java b/server/src/main/java/com/linecorp/centraldogma/server/metadata/TokensTransformer.java new file mode 100644 index 0000000000..12b6ab2e39 --- /dev/null +++ b/server/src/main/java/com/linecorp/centraldogma/server/metadata/TokensTransformer.java @@ -0,0 +1,49 @@ +/* + * Copyright 2024 LINE Corporation + * + * LINE Corporation licenses this file to you under the Apache License, + * version 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ +package com.linecorp.centraldogma.server.metadata; + +import static com.linecorp.centraldogma.server.metadata.MetadataService.TOKEN_JSON; + +import java.util.function.BiFunction; + +import com.fasterxml.jackson.core.JsonParseException; +import com.fasterxml.jackson.databind.JsonMappingException; +import com.fasterxml.jackson.databind.JsonNode; + +import com.linecorp.centraldogma.common.EntryType; +import com.linecorp.centraldogma.common.Revision; +import com.linecorp.centraldogma.internal.Jackson; +import com.linecorp.centraldogma.server.command.ContentTransformer; + +class TokensTransformer extends ContentTransformer { + + TokensTransformer(BiFunction transformer) { + super(TOKEN_JSON, EntryType.JSON, + (headRevision, jsonNode) -> Jackson.valueToTree( + transformer.apply(headRevision, tokens(jsonNode)))); + } + + private static Tokens tokens(JsonNode node) { + final Tokens tokens; + try { + tokens = Jackson.treeToValue(node, Tokens.class); + } catch (JsonParseException | JsonMappingException e) { + // Should never reach here. + throw new Error(e); + } + return tokens; + } +} diff --git a/server/src/test/java/com/linecorp/centraldogma/server/command/StandaloneCommandExecutorTest.java b/server/src/test/java/com/linecorp/centraldogma/server/command/StandaloneCommandExecutorTest.java index 6c913bf9e2..52fe27dfc5 100644 --- a/server/src/test/java/com/linecorp/centraldogma/server/command/StandaloneCommandExecutorTest.java +++ b/server/src/test/java/com/linecorp/centraldogma/server/command/StandaloneCommandExecutorTest.java @@ -21,7 +21,7 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; import java.util.List; -import java.util.function.Function; +import java.util.function.BiFunction; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; @@ -190,7 +190,7 @@ void transformCommandConvertedIntoJsonPatch() { final Revision previousRevision = commitResult.revision(); assertThat(commitResult).isEqualTo(CommitResult.of(previousRevision, ImmutableList.of(change))); - final Function transformer = jsonNode -> { + final BiFunction transformer = (revision, jsonNode) -> { if (jsonNode.has("a")) { ((ObjectNode) jsonNode).put("a", "c"); } diff --git a/server/src/test/java/com/linecorp/centraldogma/server/internal/replication/ZooKeeperCommandExecutorTest.java b/server/src/test/java/com/linecorp/centraldogma/server/internal/replication/ZooKeeperCommandExecutorTest.java index e3f10e9ac5..75f6da9683 100644 --- a/server/src/test/java/com/linecorp/centraldogma/server/internal/replication/ZooKeeperCommandExecutorTest.java +++ b/server/src/test/java/com/linecorp/centraldogma/server/internal/replication/ZooKeeperCommandExecutorTest.java @@ -42,6 +42,7 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; +import java.util.function.BiFunction; import java.util.function.Function; import java.util.function.Supplier; @@ -375,7 +376,7 @@ private static PushAsIsCommand executeCommand(Replica replica1, boolean normaliz .isEqualTo(new Revision(2)); return asIsCommand; } else { - final Function transformer = jsonNode -> { + final BiFunction transformer = (revision, jsonNode) -> { final JsonNode oldContent = pushChange.content(); assertThat(jsonNode).isEqualTo(oldContent); final JsonNode newContent = oldContent.deepCopy(); @@ -683,9 +684,10 @@ static Function, CompletableFuture> newMockDelegate() { final TransformCommand pushCommand = (TransformCommand) argument; assertThat(pushCommand.type()).isSameAs(CommandType.TRANSFORM); - final Function transformer = - (Function) pushCommand.transformer().transformer(); - final JsonNode applied = transformer.apply(pushChange.content()); + final BiFunction transformer = + (BiFunction) pushCommand.transformer() + .transformer(); + final JsonNode applied = transformer.apply(null, pushChange.content()); assertThat(applied).isEqualTo(JsonNodeFactory.instance.objectNode().put("a", "c")); return completedFuture( CommitResult.of(revision, ImmutableList.of(normalizedChange))); 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 ef68724fd3..52b16b17eb 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 @@ -35,14 +35,13 @@ import com.linecorp.centraldogma.common.Author; import com.linecorp.centraldogma.common.ChangeConflictException; -import com.linecorp.centraldogma.common.EntryNotFoundException; import com.linecorp.centraldogma.common.ProjectExistsException; import com.linecorp.centraldogma.common.ProjectRole; import com.linecorp.centraldogma.common.RepositoryExistsException; import com.linecorp.centraldogma.common.RepositoryNotFoundException; +import com.linecorp.centraldogma.common.Revision; import com.linecorp.centraldogma.server.QuotaConfig; import com.linecorp.centraldogma.server.command.Command; -import com.linecorp.centraldogma.server.internal.admin.service.TokenNotFoundException; import com.linecorp.centraldogma.testing.internal.ProjectManagerExtension; class MetadataServiceTest { @@ -186,7 +185,8 @@ void perRolePermissions() { assertThat(repositoryMetadata.perRolePermissions().guest()) .containsExactlyInAnyOrder(Permission.READ, Permission.WRITE); - mds.updatePerRolePermissions(author, project1, repo1, PerRolePermissions.ofPrivate()).join(); + final Revision revision = + mds.updatePerRolePermissions(author, project1, repo1, PerRolePermissions.ofPrivate()).join(); repositoryMetadata = getRepo1(mds); assertThat(repositoryMetadata.perRolePermissions().owner()) @@ -201,10 +201,10 @@ void perRolePermissions() { assertThat(mds.findPermissions(project1, repo1, guest).join()) .containsExactlyElementsOf(NO_PERMISSION); - // Fail due to duplicated update. - assertThatThrownBy(() -> mds.updatePerRolePermissions( + // Updating the same permission is ok. + assertThat(mds.updatePerRolePermissions( author, project1, repo1, PerRolePermissions.ofPrivate()).join()) - .hasCauseInstanceOf(IllegalArgumentException.class); + .isEqualTo(revision); assertThatThrownBy(() -> mds.updatePerRolePermissions( author, project1, REPO_DOGMA, PerRolePermissions.ofPublic()).join()) @@ -224,10 +224,14 @@ void perUserPermissions() { // Not a member yet. assertThatThrownBy(() -> mds.addPerUserPermission(author, project1, repo1, user1, READ_ONLY).join()) - .hasCauseInstanceOf(IllegalArgumentException.class); + .hasCauseInstanceOf(MemberNotFoundException.class); // Be a member of the project. mds.addMember(author, project1, user1, ProjectRole.MEMBER).join(); + // Try once more. + assertThatThrownBy(() -> mds.addMember(author, project1, user1, ProjectRole.MEMBER).join()) + // TODO(minwoox): Consider removing JSON path operation from MetadataService completely. + .hasCauseInstanceOf(ChangeConflictException.class); // invalid repo. assertThatThrownBy(() -> mds.addPerUserPermission( @@ -239,31 +243,33 @@ void perUserPermissions() { .containsExactlyElementsOf(NO_PERMISSION); // Add 'user1' to per-user permissions table. - mds.addPerUserPermission(author, project1, repo1, user1, READ_ONLY).join(); + final Revision revision = mds.addPerUserPermission(author, project1, repo1, user1, READ_ONLY).join(); - // Fail due to duplicated addition. + // Try once more. assertThatThrownBy(() -> mds.addPerUserPermission(author, project1, repo1, user1, READ_ONLY).join()) - .hasCauseInstanceOf(IllegalArgumentException.class); + .hasCauseInstanceOf(ChangeConflictException.class); assertThat(mds.findPermissions(project1, repo1, user1).join()) .containsExactly(Permission.READ); - mds.updatePerUserPermission(author, project1, repo1, user1, READ_WRITE).join(); + assertThat(mds.updatePerUserPermission(author, project1, repo1, user1, READ_WRITE).join().major()) + .isEqualTo(revision.major() + 1); 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); + // Updating the same operation will return the same revision. + assertThat(mds.updatePerUserPermission(author, project1, repo1, user1, READ_WRITE).join().major()) + .isEqualTo(revision.major() + 1); // Update invalid user assertThatThrownBy(() -> mds.updatePerUserPermission(author, project1, repo1, user2, READ_WRITE).join()) - .hasCauseInstanceOf(IllegalArgumentException.class); + .hasCauseInstanceOf(MemberNotFoundException.class); - mds.removePerUserPermission(author, project1, repo1, user1).join(); + assertThat(mds.removePerUserPermission(author, project1, repo1, user1).join().major()) + .isEqualTo(revision.major() + 2); assertThatThrownBy(() -> mds.removePerUserPermission(author, project1, repo1, user1).join()) - .hasCauseInstanceOf(IllegalArgumentException.class); + .hasCauseInstanceOf(MemberNotFoundException.class); assertThat(mds.findPermissions(project1, repo1, user1).join()) .containsExactlyElementsOf(NO_PERMISSION); @@ -275,16 +281,20 @@ void perTokenPermissions() { mds.addRepo(author, project1, repo1, ownerOnly).join(); mds.createToken(author, app1).join(); + // Try once more. + assertThatThrownBy(() -> mds.createToken(author, app1).join()) + .hasCauseInstanceOf(ChangeConflictException.class); + final Token token = mds.findTokenByAppId(app1).join(); assertThat(token).isNotNull(); // Token 'app2' is not created yet. assertThatThrownBy(() -> mds.addToken(author, project1, app2, ProjectRole.MEMBER).join()) - .hasCauseInstanceOf(IllegalArgumentException.class); + .hasCauseInstanceOf(TokenNotFoundException.class); - // Not a member yet. + // Token is not registered to the project yet. assertThatThrownBy(() -> mds.addPerTokenPermission(author, project1, repo1, app1, READ_ONLY).join()) - .hasCauseInstanceOf(IllegalArgumentException.class); + .hasCauseInstanceOf(TokenNotFoundException.class); assertThat(mds.findPermissions(project1, repo1, app1).join()) .containsExactlyElementsOf(NO_PERMISSION); @@ -296,22 +306,25 @@ void perTokenPermissions() { assertThat(mds.findPermissions(project1, repo1, app1).join()) .containsExactly(Permission.READ); - // Add again. + // Try once more + assertThatThrownBy(() -> mds.addToken(author, project1, app1, ProjectRole.MEMBER).join()) + .hasCauseInstanceOf(ChangeConflictException.class); assertThatThrownBy(() -> mds.addPerTokenPermission(author, project1, repo1, app1, READ_ONLY).join()) - .hasCauseInstanceOf(IllegalArgumentException.class); + .hasCauseInstanceOf(ChangeConflictException.class); - mds.updatePerTokenPermission(author, project1, repo1, app1, READ_WRITE).join(); + final Revision revision = + 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); + assertThat(mds.updatePerTokenPermission(author, project1, repo1, app1, READ_WRITE).join()) + .isEqualTo(revision); mds.removePerTokenPermission(author, project1, repo1, app1).join(); assertThatThrownBy(() -> mds.removePerTokenPermission(author, project1, repo1, app1).join()) - .hasCauseInstanceOf(IllegalArgumentException.class); + .hasCauseInstanceOf(ChangeConflictException.class); assertThat(mds.findPermissions(project1, repo1, app1).join()) .containsExactlyElementsOf(NO_PERMISSION); @@ -336,7 +349,6 @@ void removeMember() { // Remove 'user1' from the project. mds.removeMember(author, project1, user1).join(); - // Remove per-user permission of 'user1', too. assertThat(mds.findPermissions(project1, repo1, user1).join()) .containsExactlyElementsOf(NO_PERMISSION); @@ -345,7 +357,7 @@ void removeMember() { // Remove 'user1' again. assertThatThrownBy(() -> mds.removeMember(author, project1, user1).join()) - .hasCauseInstanceOf(EntryNotFoundException.class); + .hasCauseInstanceOf(MemberNotFoundException.class); } @Test @@ -366,7 +378,6 @@ void removeToken() { // Remove 'app1' from the project. mds.removeToken(author, project1, app1).join(); - // Remove per-token permission of 'app1', too. assertThat(mds.findPermissions(project1, repo1, app1).join()) .containsExactlyElementsOf(NO_PERMISSION); @@ -375,7 +386,7 @@ void removeToken() { // Remove 'app1' again. assertThatThrownBy(() -> mds.removeToken(author, project1, app1).join()) - .hasCauseInstanceOf(EntryNotFoundException.class); + .hasCauseInstanceOf(TokenNotFoundException.class); } @Test @@ -421,20 +432,20 @@ void tokenActivationAndDeactivation() { assertThat(token).isNotNull(); assertThat(token.creation().user()).isEqualTo(owner.id()); - mds.deactivateToken(author, app1).join(); + final Revision revision = mds.deactivateToken(author, app1).join(); token = mds.getTokens().join().get(app1); assertThat(token.isActive()).isFalse(); assertThat(token.deactivation()).isNotNull(); assertThat(token.deactivation().user()).isEqualTo(owner.id()); - assertThatThrownBy(() -> mds.deactivateToken(author, app1).join()) - .hasCauseInstanceOf(IllegalArgumentException.class); + // Executing the same operation will return the same revision. + assertThat(mds.deactivateToken(author, app1).join()).isEqualTo(revision); - mds.activateToken(author, app1).join(); + assertThat(mds.activateToken(author, app1).join().major()).isEqualTo(revision.major() + 1); assertThat(mds.getTokens().join().get(app1).isActive()).isTrue(); - assertThatThrownBy(() -> mds.activateToken(author, app1).join()) - .hasCauseInstanceOf(IllegalArgumentException.class); + // Executing the same operation will return the same revision. + assertThat(mds.activateToken(author, app1).join().major()).isEqualTo(revision.major() + 1); } @Test @@ -465,17 +476,15 @@ void updateUser() { assertThat(token).isNotNull(); assertThat(token.isSystemAdmin()).isFalse(); - mds.updateTokenLevel(author, app1, true).join(); + final Revision revision = mds.updateTokenLevel(author, app1, true).join(); token = mds.getTokens().join().get(app1); assertThat(token.isSystemAdmin()).isTrue(); - assertThatThrownBy(() -> mds.updateTokenLevel(author, app1, true).join()) - .hasCauseInstanceOf(IllegalArgumentException.class); + assertThat(mds.updateTokenLevel(author, app1, true).join()).isEqualTo(revision); - mds.updateTokenLevel(author, app1, false).join(); + assertThat(mds.updateTokenLevel(author, app1, false).join()).isEqualTo(revision.forward(1)); token = mds.getTokens().join().get(app1); assertThat(token.isSystemAdmin()).isFalse(); - assertThatThrownBy(() -> mds.updateTokenLevel(author, app1, false).join()) - .hasCauseInstanceOf(IllegalArgumentException.class); + assertThat(mds.updateTokenLevel(author, app1, false).join()).isEqualTo(revision.forward(1)); } private static RepositoryMetadata getRepo1(MetadataService mds) {