From 379c58d3a9f48a4cde8bccb6a3474c7320dbe448 Mon Sep 17 00:00:00 2001 From: Rene Pfeuffer Date: Tue, 3 Sep 2019 10:36:23 +0200 Subject: [PATCH] Add parameter to check current revision to prevent conflicts --- .../repository/api/ModifyCommandBuilder.java | 20 ++++++++++--------- .../repository/spi/ModifyCommandRequest.java | 9 +++++++++ .../repository/spi/AbstractGitCommand.java | 4 ++++ .../scm/repository/spi/GitModifyCommand.java | 6 ++++++ .../repository/spi/GitModifyCommandTest.java | 20 +++++++++++++++++-- 5 files changed, 48 insertions(+), 11 deletions(-) diff --git a/scm-core/src/main/java/sonia/scm/repository/api/ModifyCommandBuilder.java b/scm-core/src/main/java/sonia/scm/repository/api/ModifyCommandBuilder.java index 5baa7dcd26..1eed0697cb 100644 --- a/scm-core/src/main/java/sonia/scm/repository/api/ModifyCommandBuilder.java +++ b/scm-core/src/main/java/sonia/scm/repository/api/ModifyCommandBuilder.java @@ -57,15 +57,6 @@ public class ModifyCommandBuilder { this.workdir = workdirProvider.createNewWorkdir(); } - /** - * Set the branch that should be modified. The new commit will be made for this branch. - * @param branchToModify The branch to modify. - * @return This builder instance. - */ - public ModifyCommandBuilder setBranchToModify(String branchToModify) { - return this; - } - /** * Create a new file. The content of the file will be specified in a subsequent call to * {@link ContentLoader#withData(ByteSource)} or {@link ContentLoader#withData(InputStream)}. @@ -158,6 +149,17 @@ public class ModifyCommandBuilder { return this; } + /** + * Set the expected revision of the branch, before the changes are applied. If the branch does not have the + * expected revision, a concurrent modification exception will be thrown when the command is executed and no + * changes will be applied. + * @return This builder instance. + */ + public ModifyCommandBuilder setExpectedRevision(String expectedRevision) { + request.setExpectedRevision(expectedRevision); + return this; + } + public interface ContentLoader { /** * Specify the data of the file using a {@link ByteSource}. diff --git a/scm-core/src/main/java/sonia/scm/repository/spi/ModifyCommandRequest.java b/scm-core/src/main/java/sonia/scm/repository/spi/ModifyCommandRequest.java index e5bd7708bb..76f420803a 100644 --- a/scm-core/src/main/java/sonia/scm/repository/spi/ModifyCommandRequest.java +++ b/scm-core/src/main/java/sonia/scm/repository/spi/ModifyCommandRequest.java @@ -22,6 +22,7 @@ public class ModifyCommandRequest implements Resetable, Validateable { private Person author; private String commitMessage; private String branch; + private String expectedRevision; @Override public void reset() { @@ -63,11 +64,19 @@ public class ModifyCommandRequest implements Resetable, Validateable { return branch; } + public String getExpectedRevision() { + return expectedRevision; + } + @Override public boolean isValid() { return StringUtils.isNotEmpty(commitMessage) && !requests.isEmpty(); } + public void setExpectedRevision(String expectedRevision) { + this.expectedRevision = expectedRevision; + } + public interface PartialRequest { void execute(ModifyCommand.Worker worker) throws IOException; } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/AbstractGitCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/AbstractGitCommand.java index 6066b66371..bea110766b 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/AbstractGitCommand.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/AbstractGitCommand.java @@ -240,6 +240,10 @@ class AbstractGitCommand logger.debug("pushed changes"); } + Ref getCurrentRevision() throws IOException { + return getClone().getRepository().getRefDatabase().findRef("HEAD"); + } + private Person determineAuthor(Person author) { if (author == null) { Subject subject = SecurityUtils.getSubject(); diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitModifyCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitModifyCommand.java index c158047f53..4ff60445f7 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitModifyCommand.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitModifyCommand.java @@ -5,6 +5,7 @@ import org.eclipse.jgit.api.Git; import org.eclipse.jgit.api.errors.GitAPIException; import org.eclipse.jgit.revwalk.RevCommit; import sonia.scm.BadRequestException; +import sonia.scm.ConcurrentModificationException; import sonia.scm.ContextEntry; import sonia.scm.repository.GitWorkdirFactory; import sonia.scm.repository.InternalRepositoryException; @@ -51,6 +52,11 @@ public class GitModifyCommand extends AbstractGitCommand implements ModifyComman if (!StringUtils.isEmpty(request.getBranch())) { checkOutBranch(request.getBranch()); } + if (!StringUtils.isEmpty(request.getExpectedRevision())) { + if (!request.getExpectedRevision().equals(getCurrentRevision().getName())) { + throw new ConcurrentModificationException("branch", request.getBranch() == null? "default": request.getBranch()); + } + } for (ModifyCommandRequest.PartialRequest r : request.getRequests()) { r.execute(this); } diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitModifyCommandTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitModifyCommandTest.java index 47c15a7228..96cc256cf4 100644 --- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitModifyCommandTest.java +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitModifyCommandTest.java @@ -15,6 +15,7 @@ import org.junit.Test; import org.junit.rules.TemporaryFolder; import sonia.scm.AlreadyExistsException; import sonia.scm.BadRequestException; +import sonia.scm.ConcurrentModificationException; import sonia.scm.repository.Person; import sonia.scm.repository.util.WorkdirProvider; @@ -56,7 +57,7 @@ public class GitModifyCommandTest extends AbstractGitCommandTestBase { } @Test - public void shouldCreateCommitOnSelectedBranch() throws IOException, GitAPIException { + public void shouldCreateCommitOnSelectedBranch() throws IOException { File newFile = Files.write(temporaryFolder.newFile().toPath(), "new content".getBytes()).toFile(); GitModifyCommand command = createCommand(); @@ -95,7 +96,7 @@ public class GitModifyCommandTest extends AbstractGitCommandTestBase { } @Test(expected = AlreadyExistsException.class) - public void shouldFailIfOverwritingExistingFileWithoutOverwriteFlag() throws IOException, GitAPIException { + public void shouldFailIfOverwritingExistingFileWithoutOverwriteFlag() throws IOException { File newFile = Files.write(temporaryFolder.newFile().toPath(), "new content".getBytes()).toFile(); GitModifyCommand command = createCommand(); @@ -140,6 +141,21 @@ public class GitModifyCommandTest extends AbstractGitCommandTestBase { command.execute(request); } + @Test(expected = ConcurrentModificationException.class) + public void shouldFailBranchDoesNotHaveExpectedRevision() throws IOException { + File newFile = Files.write(temporaryFolder.newFile().toPath(), "irrelevant\n".getBytes()).toFile(); + + GitModifyCommand command = createCommand(); + + ModifyCommandRequest request = new ModifyCommandRequest(); + request.setCommitMessage("test commit"); + request.addRequest(new ModifyCommandRequest.CreateFileRequest("irrelevant", newFile, true)); + request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det")); + request.setExpectedRevision("abc"); + + command.execute(request); + } + private void assertInTree(TreeAssertions assertions) throws IOException, GitAPIException { try (Git git = new Git(createContext().open())) { RevCommit lastCommit = getLastCommit(git);