From 6ea77b42ca38529d298d307ceb47a1d8ce47d871 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Tue, 30 Nov 2021 08:49:47 +0100 Subject: [PATCH] Fix edge cases in move (#1874) Fixes edge cases for "move" in the modify command, like - reject backslashes in validation - add overwrite option - check for invalid source and target options This is an update to the implementation of "move" in #1859. Co-authored-by: Matthias Thieroff --- .../fix_move_command_for_windows.yaml | 2 - .../repository/api/ModifyCommandBuilder.java | 38 +++- .../spi/ModificationFailedException.java | 44 +++++ .../scm/repository/spi/ModifyCommand.java | 8 +- .../repository/spi/ModifyCommandRequest.java | 6 +- .../repository/spi/ModifyWorkerHelper.java | 22 ++- .../java/sonia/scm/util/ValidationUtil.java | 1 + .../spi/ModifyWorkerHelperTest.java | 27 ++- .../sonia/scm/util/ValidationUtilTest.java | 1 + .../scm/repository/spi/GitModifyCommand.java | 17 +- .../repository/spi/GitModifyCommandTest.java | 165 ++++++++---------- .../spi/GitModifyCommand_LFSTest.java | 2 +- .../scm/repository/spi/HgModifyCommand.java | 12 +- .../repository/spi/HgModifyCommandTest.java | 127 +++++++------- .../scm/repository/spi/SvnModifyCommand.java | 7 + .../repository/spi/SvnModifyCommandTest.java | 115 ++++++------ .../ModificationFailedExceptionMapper.java | 40 +++++ .../main/resources/locales/de/plugins.json | 4 + .../main/resources/locales/en/plugins.json | 4 + 19 files changed, 400 insertions(+), 242 deletions(-) delete mode 100644 gradle/changelog/fix_move_command_for_windows.yaml create mode 100644 scm-core/src/main/java/sonia/scm/repository/spi/ModificationFailedException.java create mode 100644 scm-webapp/src/main/java/sonia/scm/api/ModificationFailedExceptionMapper.java diff --git a/gradle/changelog/fix_move_command_for_windows.yaml b/gradle/changelog/fix_move_command_for_windows.yaml deleted file mode 100644 index 41cff90054..0000000000 --- a/gradle/changelog/fix_move_command_for_windows.yaml +++ /dev/null @@ -1,2 +0,0 @@ -- type: fixed - description: Move command for Windows ([#1872](https://github.com/scm-manager/scm-manager/pull/1872)) 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 c8bc6908a5..20e85786bf 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 @@ -115,11 +115,14 @@ public class ModifyCommandBuilder { } /** + * Move existing directories or files. + * + * @param fromPath The source file or directory. + * @return A sub builder to specify the target and further options for the move. * @since 2.28.0 */ - public ModifyCommandBuilder move(String fromPath, String toPath) { - request.addRequest(new ModifyCommandRequest.MoveRequest(fromPath, toPath)); - return this; + public MoveBuilder move(String fromPath) { + return new MoveBuilder(fromPath); } /** @@ -277,6 +280,35 @@ public class ModifyCommandBuilder { } } + public class MoveBuilder { + + private final String fromPath; + private boolean overwrite = false; + + public MoveBuilder(String fromPath) { + this.fromPath = fromPath; + } + + /** + * Set this to true to overwrite the path if it already exists. Otherwise an + * {@link sonia.scm.AlreadyExistsException} will be thrown. + * @return This move builder instance. + */ + public MoveBuilder withOverwrite(boolean overwrite) { + this.overwrite = overwrite; + return this; + } + + /** + * Sets the target path the file or directory should be moved to. + * @return The builder instance. + */ + public ModifyCommandBuilder to(String toPath) { + request.addRequest(new ModifyCommandRequest.MoveRequest(fromPath, toPath, overwrite)); + return ModifyCommandBuilder.this; + } + } + private File loadData(ByteSource data) throws IOException { File file = createTemporaryFile(); data.copyTo(Files.asByteSink(file)); diff --git a/scm-core/src/main/java/sonia/scm/repository/spi/ModificationFailedException.java b/scm-core/src/main/java/sonia/scm/repository/spi/ModificationFailedException.java new file mode 100644 index 0000000000..a92b31b017 --- /dev/null +++ b/scm-core/src/main/java/sonia/scm/repository/spi/ModificationFailedException.java @@ -0,0 +1,44 @@ +/* + * MIT License + * + * Copyright (c) 2020-present Cloudogu GmbH and Contributors + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +package sonia.scm.repository.spi; + +import sonia.scm.ContextEntry; +import sonia.scm.ExceptionWithContext; + +import java.util.List; + +public class ModificationFailedException extends ExceptionWithContext { + + public static final String CODE = "8wSpi62oJ1"; + + public ModificationFailedException(List context, String message) { + super(context, message); + } + + @Override + public String getCode() { + return CODE; + } +} diff --git a/scm-core/src/main/java/sonia/scm/repository/spi/ModifyCommand.java b/scm-core/src/main/java/sonia/scm/repository/spi/ModifyCommand.java index cd4631ca0b..b3f5940bb8 100644 --- a/scm-core/src/main/java/sonia/scm/repository/spi/ModifyCommand.java +++ b/scm-core/src/main/java/sonia/scm/repository/spi/ModifyCommand.java @@ -24,6 +24,9 @@ package sonia.scm.repository.spi; +import sonia.scm.FeatureNotSupportedException; +import sonia.scm.repository.Feature; + import java.io.File; import java.io.IOException; @@ -31,6 +34,9 @@ public interface ModifyCommand { String execute(ModifyCommandRequest request); + /** + * Implementations should use the {@link ModifyWorkerHelper} for this. + */ interface Worker { void delete(String toBeDeleted, boolean recursive) throws IOException; @@ -41,6 +47,6 @@ public interface ModifyCommand { /** * @since 2.28.0 */ - void move(String path, String newPath) throws IOException; + void move(String path, String newPath, boolean overwrite) throws IOException; } } 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 85e584e9db..6a255fe645 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 @@ -212,16 +212,18 @@ public class ModifyCommandRequest implements Resetable, Validateable, CommandWit public static class MoveRequest implements PartialRequest { private final String fromPath; + private final boolean overwrite; private final String toPath; - public MoveRequest(String fromPath, String toPath) { + public MoveRequest(String fromPath, String toPath, boolean overwrite) { this.toPath = toPath; this.fromPath = fromPath; + this.overwrite = overwrite; } @Override public void execute(ModifyCommand.Worker worker) throws IOException { - worker.move(fromPath, toPath); + worker.move(fromPath, toPath, overwrite); } } } diff --git a/scm-core/src/main/java/sonia/scm/repository/spi/ModifyWorkerHelper.java b/scm-core/src/main/java/sonia/scm/repository/spi/ModifyWorkerHelper.java index 6ad96a5eb3..784be5b552 100644 --- a/scm-core/src/main/java/sonia/scm/repository/spi/ModifyWorkerHelper.java +++ b/scm-core/src/main/java/sonia/scm/repository/spi/ModifyWorkerHelper.java @@ -27,7 +27,6 @@ package sonia.scm.repository.spi; import org.apache.commons.lang.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import sonia.scm.AlreadyExistsException; import sonia.scm.ContextEntry; import sonia.scm.repository.InternalRepositoryException; import sonia.scm.repository.Repository; @@ -74,14 +73,29 @@ public interface ModifyWorkerHelper extends ModifyCommand.Worker { * @since 2.28.0 */ @Override - default void move(String source, String target) throws IOException { + default void move(String source, String target, boolean overwrite) throws IOException { + Path sourceFile = getTargetFile(source); + + if (!Files.exists(sourceFile)) { + throw notFound(entity("Path", source).in(getRepository())); + } + Path targetFile = getTargetFile(target); + + if (!overwrite && Files.exists(targetFile)) { + throw alreadyExists(entity("Path", target).in(getRepository())); + } + + doThrow() + .violation("Cannot move if new path would be a child directory of path") + .when(Files.isDirectory(sourceFile) && targetFile.startsWith(sourceFile)); + Files.createDirectories(targetFile.getParent()); try { - Path pathAfterMove = Files.move(getTargetFile(source), targetFile); + Path pathAfterMove = Files.move(sourceFile, targetFile, REPLACE_EXISTING); doScmMove(source, toScmPath(pathAfterMove)); } catch (FileAlreadyExistsException e) { - throw AlreadyExistsException.alreadyExists(ContextEntry.ContextBuilder.entity("File", target).in(getRepository())); + throw alreadyExists(entity("File", target).in(getRepository())); } } diff --git a/scm-core/src/main/java/sonia/scm/util/ValidationUtil.java b/scm-core/src/main/java/sonia/scm/util/ValidationUtil.java index 9c28e6d55e..34779e9ffa 100644 --- a/scm-core/src/main/java/sonia/scm/util/ValidationUtil.java +++ b/scm-core/src/main/java/sonia/scm/util/ValidationUtil.java @@ -63,6 +63,7 @@ public final class ValidationUtil { return !path.equals(".") && !path.contains("../") && !path.contains("//") + && !path.contains("\\") && !path.equals(".."); } diff --git a/scm-core/src/test/java/sonia/scm/repository/spi/ModifyWorkerHelperTest.java b/scm-core/src/test/java/sonia/scm/repository/spi/ModifyWorkerHelperTest.java index 1d04c48a25..ee6d0f1939 100644 --- a/scm-core/src/test/java/sonia/scm/repository/spi/ModifyWorkerHelperTest.java +++ b/scm-core/src/test/java/sonia/scm/repository/spi/ModifyWorkerHelperTest.java @@ -26,6 +26,8 @@ package sonia.scm.repository.spi; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; +import sonia.scm.AlreadyExistsException; +import sonia.scm.NotFoundException; import sonia.scm.ScmConstraintViolationException; import sonia.scm.repository.Repository; @@ -39,6 +41,8 @@ import static org.junit.jupiter.api.Assertions.assertThrows; class ModifyWorkerHelperTest { + private static final Repository REPOSITORY = new Repository("42", "git", "hitchhiker", "hog"); + private boolean pathProtected = false; @Test @@ -83,6 +87,27 @@ class ModifyWorkerHelperTest { ); } + @Test + void shouldNotMoveNotExistingFile(@TempDir Path temp) throws IOException { + ModifyWorkerHelper helper = new MinimalModifyWorkerHelper(temp); + + assertThrows( + NotFoundException.class, + () -> helper.move("no-such-file", "irrelevant", false)); + } + + @Test + void shouldNotOverwriteExistingFileInMoveWithoutOverwrite(@TempDir Path temp) throws IOException { + createFile(temp, "existing-source.txt"); + createFile(temp, "existing-target.txt"); + + ModifyWorkerHelper helper = new MinimalModifyWorkerHelper(temp); + + assertThrows( + AlreadyExistsException.class, + () -> helper.move("existing-source.txt", "existing-target.txt", false)); + } + private File createFile(Path temp, String fileName) throws IOException { File file = new File(temp.toFile(), fileName); FileWriter source = new FileWriter(file); @@ -116,7 +141,7 @@ class ModifyWorkerHelperTest { @Override public Repository getRepository() { - return null; + return REPOSITORY; } @Override diff --git a/scm-core/src/test/java/sonia/scm/util/ValidationUtilTest.java b/scm-core/src/test/java/sonia/scm/util/ValidationUtilTest.java index 5792231bd8..cd4aaf5121 100644 --- a/scm-core/src/test/java/sonia/scm/util/ValidationUtilTest.java +++ b/scm-core/src/test/java/sonia/scm/util/ValidationUtilTest.java @@ -69,6 +69,7 @@ class ValidationUtilTest { @ValueSource(strings = { "..", "../", + "some\\windows", "../../", "../ka", "test/../.." 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 66f765004a..c33c6eec15 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 @@ -29,6 +29,7 @@ import org.apache.commons.lang.StringUtils; import org.eclipse.jgit.api.Git; import org.eclipse.jgit.api.errors.GitAPIException; import org.eclipse.jgit.attributes.FilterCommandRegistry; +import org.eclipse.jgit.dircache.DirCache; import org.eclipse.jgit.revwalk.RevCommit; import sonia.scm.ConcurrentModificationException; import sonia.scm.ContextEntry; @@ -173,13 +174,21 @@ public class GitModifyCommand extends AbstractGitCommand implements ModifyComman } private void addFileToGit(String toBeCreated) throws GitAPIException { - getClone().add().addFilepattern(removeStartingPathSeparators(toBeCreated)).call(); + String toBeCreatedWithoutLeadingSlash = removeStartingPathSeparators(toBeCreated); + DirCache addResult = getClone().add().addFilepattern(toBeCreatedWithoutLeadingSlash).call(); + if (addResult.findEntry(toBeCreatedWithoutLeadingSlash) < 0) { + throw new ModificationFailedException(ContextEntry.ContextBuilder.entity("File", toBeCreated).in(repository).build(), "Could not add file to repository"); + } } @Override public void doScmDelete(String toBeDeleted) { try { - getClone().rm().addFilepattern(removeStartingPathSeparators(toBeDeleted)).call(); + String toBeDeletedWithoutLeadingSlash = removeStartingPathSeparators(toBeDeleted); + DirCache deleteResult = getClone().rm().addFilepattern(toBeDeletedWithoutLeadingSlash).call(); + if (deleteResult.findEntry(toBeDeletedWithoutLeadingSlash) >= 0) { + throw new ModificationFailedException(ContextEntry.ContextBuilder.entity("File", toBeDeleted).in(repository).build(), "Could not delete file from repository"); + } } catch (GitAPIException e) { throwInternalRepositoryException("could not remove file from index", e); } @@ -206,8 +215,8 @@ public class GitModifyCommand extends AbstractGitCommand implements ModifyComman } private String removeStartingPathSeparators(String path) { - while (path.startsWith(File.separator)) { - path = path.substring(1); + if (path.startsWith("/")) { + return path.substring(1); } return path; } 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 d17749448d..006c4315cb 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 @@ -66,16 +66,14 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase { GitModifyCommand command = createCommand(); - ModifyCommandRequest request = new ModifyCommandRequest(); - request.setCommitMessage("test commit"); + ModifyCommandRequest request = prepareModifyCommandRequest(); request.addRequest(new ModifyCommandRequest.CreateFileRequest("new_file", newFile, false)); - request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det")); String newRef = command.execute(request); try (Git git = new Git(createContext().open())) { RevCommit lastCommit = getLastCommit(git); - assertThat(lastCommit.getFullMessage()).isEqualTo("test commit"); + assertThat(lastCommit.getFullMessage()).isEqualTo("Make some change"); assertThat(lastCommit.getAuthorIdent().getName()).isEqualTo("Dirk Gently"); assertThat(newRef).isEqualTo(lastCommit.toObjectId().name()); } @@ -87,11 +85,9 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase { GitModifyCommand command = createCommand(); - ModifyCommandRequest request = new ModifyCommandRequest(); - request.setCommitMessage("test commit"); + ModifyCommandRequest request = prepareModifyCommandRequest(); request.setBranch("test-branch"); request.addRequest(new ModifyCommandRequest.CreateFileRequest("new_file", newFile, false)); - request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det")); String newRef = command.execute(request); @@ -108,10 +104,8 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase { GitModifyCommand command = createCommand(); - ModifyCommandRequest request = new ModifyCommandRequest(); - request.setCommitMessage("test commit"); + ModifyCommandRequest request = prepareModifyCommandRequest(); request.addRequest(new ModifyCommandRequest.CreateFileRequest("new_file", newFile, false)); - request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det")); command.execute(request); @@ -126,10 +120,8 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase { GitModifyCommand command = createCommand(); - ModifyCommandRequest request = new ModifyCommandRequest(); - request.setCommitMessage("test commit"); + ModifyCommandRequest request = prepareModifyCommandRequest(); request.addRequest(new ModifyCommandRequest.CreateFileRequest("/new_file", newFile, false)); - request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det")); command.execute(request); @@ -144,10 +136,8 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase { GitModifyCommand command = createCommand(); - ModifyCommandRequest request = new ModifyCommandRequest(); - request.setCommitMessage("test commit"); + ModifyCommandRequest request = prepareModifyCommandRequest(); request.addRequest(new ModifyCommandRequest.CreateFileRequest("a.txt", newFile, false)); - request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det")); command.execute(request); } @@ -158,10 +148,8 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase { GitModifyCommand command = createCommand(); - ModifyCommandRequest request = new ModifyCommandRequest(); - request.setCommitMessage("test commit"); + ModifyCommandRequest request = prepareModifyCommandRequest(); request.addRequest(new ModifyCommandRequest.CreateFileRequest("a.txt/newFile", newFile, false)); - request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det")); command.execute(request); } @@ -172,10 +160,8 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase { GitModifyCommand command = createCommand(); - ModifyCommandRequest request = new ModifyCommandRequest(); - request.setCommitMessage("test commit"); + ModifyCommandRequest request = prepareModifyCommandRequest(); request.addRequest(new ModifyCommandRequest.CreateFileRequest("a.txt", newFile, true)); - request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det")); command.execute(request); @@ -190,10 +176,8 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase { GitModifyCommand command = createCommand(); - ModifyCommandRequest request = new ModifyCommandRequest(); - request.setCommitMessage("test commit"); + ModifyCommandRequest request = prepareModifyCommandRequest(); request.addRequest(new ModifyCommandRequest.ModifyFileRequest("a.txt", newFile)); - request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det")); command.execute(request); @@ -208,10 +192,8 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase { GitModifyCommand command = createCommand(); - ModifyCommandRequest request = new ModifyCommandRequest(); - request.setCommitMessage("test commit"); + ModifyCommandRequest request = prepareModifyCommandRequest(); request.addRequest(new ModifyCommandRequest.ModifyFileRequest("no.such.file", newFile)); - request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det")); command.execute(request); } @@ -222,10 +204,8 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase { GitModifyCommand command = createCommand(); - ModifyCommandRequest request = new ModifyCommandRequest(); - request.setCommitMessage("test commit"); + ModifyCommandRequest request = prepareModifyCommandRequest(); request.addRequest(new ModifyCommandRequest.CreateFileRequest("b.txt", newFile, true)); - request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det")); command.execute(request); } @@ -236,10 +216,8 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase { GitModifyCommand command = createCommand(); - ModifyCommandRequest request = new ModifyCommandRequest(); - request.setCommitMessage("test commit"); + ModifyCommandRequest request = prepareModifyCommandRequest(); request.addRequest(new ModifyCommandRequest.CreateFileRequest("irrelevant", newFile, true)); - request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det")); request.setExpectedRevision("abc"); command.execute(request); @@ -249,10 +227,8 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase { public void shouldDeleteExistingFile() throws IOException, GitAPIException { GitModifyCommand command = createCommand(); - ModifyCommandRequest request = new ModifyCommandRequest(); - request.setCommitMessage("test commit"); + ModifyCommandRequest request = prepareModifyCommandRequest(); request.addRequest(new ModifyCommandRequest.DeleteFileRequest("a.txt", false)); - request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det")); command.execute(request); @@ -265,10 +241,8 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase { public void shouldDeleteExistingDirectory() throws IOException, GitAPIException { GitModifyCommand command = createCommand(); - ModifyCommandRequest request = new ModifyCommandRequest(); - request.setCommitMessage("test commit"); + ModifyCommandRequest request = prepareModifyCommandRequest(); request.addRequest(new ModifyCommandRequest.DeleteFileRequest("c", true)); - request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det")); command.execute(request); @@ -293,10 +267,8 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase { public void shouldThrowNotFoundExceptionWhenFileToDeleteDoesNotExist() { GitModifyCommand command = createCommand(); - ModifyCommandRequest request = new ModifyCommandRequest(); - request.setCommitMessage("test commit"); + ModifyCommandRequest request = prepareModifyCommandRequest(); request.addRequest(new ModifyCommandRequest.DeleteFileRequest("no/such/file", false)); - request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det")); command.execute(request); } @@ -305,11 +277,9 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase { public void shouldThrowNotFoundExceptionWhenBranchDoesNotExist() { GitModifyCommand command = createCommand(); - ModifyCommandRequest request = new ModifyCommandRequest(); + ModifyCommandRequest request = prepareModifyCommandRequest(); request.setBranch("does-not-exist"); - request.setCommitMessage("test commit"); request.addRequest(new ModifyCommandRequest.DeleteFileRequest("a.txt", false)); - request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det")); command.execute(request); } @@ -320,11 +290,9 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase { GitModifyCommand command = createCommand(); - ModifyCommandRequest request = new ModifyCommandRequest(); - request.setCommitMessage("test commit"); + ModifyCommandRequest request = prepareModifyCommandRequest(); request.setBranch("3f76a12f08a6ba0dc988c68b7f0b2cd190efc3c4"); request.addRequest(new ModifyCommandRequest.CreateFileRequest("irrelevant", newFile, true)); - request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det")); command.execute(request); } @@ -335,10 +303,8 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase { GitModifyCommand command = createCommand(); - ModifyCommandRequest request = new ModifyCommandRequest(); - request.setCommitMessage("test commit"); + ModifyCommandRequest request = prepareModifyCommandRequest(); request.addRequest(new ModifyCommandRequest.CreateFileRequest("new_file", newFile, false)); - request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det")); command.execute(request); @@ -356,11 +322,9 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase { GitModifyCommand command = createCommand(); - ModifyCommandRequest request = new ModifyCommandRequest(); - request.setCommitMessage("test commit"); + ModifyCommandRequest request = prepareModifyCommandRequest(); request.setSign(false); request.addRequest(new ModifyCommandRequest.CreateFileRequest("new_file", newFile, false)); - request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det")); command.execute(request); @@ -377,10 +341,8 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase { GitModifyCommand command = createCommand(); - ModifyCommandRequest request = new ModifyCommandRequest(); - request.setCommitMessage("test commit"); + ModifyCommandRequest request = prepareModifyCommandRequest(); request.addRequest(new ModifyCommandRequest.CreateFileRequest("new_file", newFile, false)); - request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det")); command.execute(request); @@ -398,10 +360,8 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase { GitModifyCommand command = createCommand(); - ModifyCommandRequest request = new ModifyCommandRequest(); - request.setCommitMessage("test commit"); + ModifyCommandRequest request = prepareModifyCommandRequest(); request.addRequest(new ModifyCommandRequest.CreateFileRequest(".git/ome.txt", newFile, true)); - request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det")); command.execute(request); } @@ -410,10 +370,8 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase { public void shouldThrowErrorIfRelativePathIsOutsideOfWorkdir() { GitModifyCommand command = createCommand(); - ModifyCommandRequest request = new ModifyCommandRequest(); - request.setCommitMessage("please rename this file"); - request.setAuthor(new Person("Peter Pan", "peter@pan.net")); - request.addRequest(new ModifyCommandRequest.MoveRequest("g/h/c", "/../../../../b")); + ModifyCommandRequest request = prepareModifyCommandRequest(); + request.addRequest(new ModifyCommandRequest.MoveRequest("g/h/c", "/../../../../b", false)); command.execute(request); } @@ -422,10 +380,8 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase { public void shouldRenameFile() throws GitAPIException, IOException { GitModifyCommand command = createCommand(); - ModifyCommandRequest request = new ModifyCommandRequest(); - request.setCommitMessage("please rename this file"); - request.setAuthor(new Person("Peter Pan", "peter@pan.net")); - request.addRequest(new ModifyCommandRequest.MoveRequest("b.txt", "/d.txt")); + ModifyCommandRequest request = prepareModifyCommandRequest(); + request.addRequest(new ModifyCommandRequest.MoveRequest("b.txt", "/d.txt", false)); command.execute(request); @@ -441,10 +397,8 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase { public void shouldThrowAlreadyExistsException() { GitModifyCommand command = createCommand(); - ModifyCommandRequest request = new ModifyCommandRequest(); - request.addRequest(new ModifyCommandRequest.MoveRequest("a.txt", "/c")); - request.setCommitMessage("please rename my file pretty please"); - request.setAuthor(new Person("Arthur Dent", "dent@hitchhiker.com")); + ModifyCommandRequest request = prepareModifyCommandRequest(); + request.addRequest(new ModifyCommandRequest.MoveRequest("a.txt", "/c", false)); command.execute(request); } @@ -453,10 +407,8 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase { public void shouldRenameFolder() throws GitAPIException, IOException { GitModifyCommand command = createCommand(); - ModifyCommandRequest request = new ModifyCommandRequest(); - request.setCommitMessage("please move this folder"); - request.setAuthor(new Person("Peter Pan", "peter@pan.net")); - request.addRequest(new ModifyCommandRequest.MoveRequest("c", "/notc")); + ModifyCommandRequest request = prepareModifyCommandRequest(); + request.addRequest(new ModifyCommandRequest.MoveRequest("c", "/notc", false)); command.execute(request); @@ -474,10 +426,8 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase { public void shouldMoveFileToExistingFolder() throws GitAPIException, IOException { GitModifyCommand command = createCommand(); - ModifyCommandRequest request = new ModifyCommandRequest(); - request.setCommitMessage("please move this file"); - request.setAuthor(new Person("Peter Pan", "peter@pan.net")); - request.addRequest(new ModifyCommandRequest.MoveRequest("a.txt", "/c/z.txt")); + ModifyCommandRequest request = prepareModifyCommandRequest(); + request.addRequest(new ModifyCommandRequest.MoveRequest("a.txt", "/c/z.txt", false)); command.execute(request); @@ -495,10 +445,8 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase { public void shouldMoveFolderToExistingFolder() throws GitAPIException, IOException { GitModifyCommand command = createCommand(); - ModifyCommandRequest request = new ModifyCommandRequest(); - request.setCommitMessage("please rename this file"); - request.setAuthor(new Person("Peter Pan", "peter@pan.net")); - request.addRequest(new ModifyCommandRequest.MoveRequest("g/h", "/g/k/h")); + ModifyCommandRequest request = prepareModifyCommandRequest(); + request.addRequest(new ModifyCommandRequest.MoveRequest("g/h", "/g/k/h", false)); command.execute(request); @@ -514,10 +462,8 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase { public void shouldMoveFileToNonExistentFolder() throws GitAPIException, IOException { GitModifyCommand command = createCommand(); - ModifyCommandRequest request = new ModifyCommandRequest(); - request.setCommitMessage("please move this file"); - request.setAuthor(new Person("Peter Pan", "peter@pan.net")); - request.addRequest(new ModifyCommandRequest.MoveRequest("a.txt", "/y/z.txt")); + ModifyCommandRequest request = prepareModifyCommandRequest(); + request.addRequest(new ModifyCommandRequest.MoveRequest("a.txt", "/y/z.txt", false)); command.execute(request); @@ -529,14 +475,29 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase { assertInTree(assertions); } + @Test + public void shouldMoveFileWithOverwrite() throws GitAPIException, IOException { + GitModifyCommand command = createCommand(); + + ModifyCommandRequest request = prepareModifyCommandRequest(); + request.addRequest(new ModifyCommandRequest.MoveRequest("a.txt", "/b.txt", true)); + + command.execute(request); + + TreeAssertions assertions = fileFinder -> { + assertThat(fileFinder.findFile("a.txt")).isFalse(); + assertThat(fileFinder.findFile("b.txt")).isTrue(); + }; + + assertInTree(assertions); + } + @Test public void shouldMoveFolderToNonExistentFolder() throws GitAPIException, IOException { GitModifyCommand command = createCommand(); - ModifyCommandRequest request = new ModifyCommandRequest(); - request.setCommitMessage("please move this file"); - request.setAuthor(new Person("Peter Pan", "peter@pan.net")); - request.addRequest(new ModifyCommandRequest.MoveRequest("c", "/j/k/c")); + ModifyCommandRequest request = prepareModifyCommandRequest(); + request.addRequest(new ModifyCommandRequest.MoveRequest("c", "/j/k/c", false)); command.execute(request); @@ -549,4 +510,22 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase { assertInTree(assertions); } + + @Test(expected = AlreadyExistsException.class) + public void shouldFailMoveAndKeepFilesWhenSourceAndTargetAreTheSame() { + GitModifyCommand command = createCommand(); + + ModifyCommandRequest request = prepareModifyCommandRequest(); + request.addRequest(new ModifyCommandRequest.MoveRequest("c", "c", false)); + + command.execute(request); + } + + private ModifyCommandRequest prepareModifyCommandRequest() { + ModifyCommandRequest request = new ModifyCommandRequest(); + request.setCommitMessage("Make some change"); + request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det")); + return request; + } + } diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitModifyCommand_LFSTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitModifyCommand_LFSTest.java index ab2ae11665..a738a3c096 100644 --- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitModifyCommand_LFSTest.java +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitModifyCommand_LFSTest.java @@ -102,7 +102,7 @@ public class GitModifyCommand_LFSTest extends GitModifyCommandTestBase { GitModifyCommand command = createCommand(); ModifyCommandRequest request = new ModifyCommandRequest(); request.setCommitMessage("Move file"); - request.addRequest(new ModifyCommandRequest.MoveRequest("new_lfs.png", "moved_lfs.png")); + request.addRequest(new ModifyCommandRequest.MoveRequest("new_lfs.png", "moved_lfs.png", false)); request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det")); command.execute(request); diff --git a/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/HgModifyCommand.java b/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/HgModifyCommand.java index d49c50dd9c..a96520c19f 100644 --- a/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/HgModifyCommand.java +++ b/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/HgModifyCommand.java @@ -26,6 +26,7 @@ package sonia.scm.repository.spi; import org.javahg.Changeset; import org.javahg.Repository; +import org.javahg.commands.AddCommand; import org.javahg.commands.CommitCommand; import org.javahg.commands.ExecutionException; import org.javahg.commands.RemoveCommand; @@ -33,6 +34,7 @@ import org.javahg.commands.StatusCommand; import com.google.common.annotations.VisibleForTesting; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import sonia.scm.ContextEntry; import sonia.scm.NoChangesMadeException; import sonia.scm.repository.HgRepositoryHandler; import sonia.scm.repository.InternalRepositoryException; @@ -81,7 +83,10 @@ public class HgModifyCommand extends AbstractWorkingCopyCommand implements Modif @Override public void doScmDelete(String toBeDeleted) { - RemoveCommand.on(workingRepository).execute(toBeDeleted); + List execute = RemoveCommand.on(workingRepository).execute(toBeDeleted); + if (execute.isEmpty()) { + throw new ModificationFailedException(ContextEntry.ContextBuilder.entity("File", toBeDeleted).in(repository).build(), "Could not delete file from repository"); + } } @Override @@ -99,7 +104,10 @@ public class HgModifyCommand extends AbstractWorkingCopyCommand implements Modif } private void addFileToHg(File file) { - workingRepository.workingCopy().add(file.getAbsolutePath()); + List execute = AddCommand.on(workingRepository).execute(file.getAbsolutePath()); + if (execute.isEmpty()) { + throw new ModificationFailedException(ContextEntry.ContextBuilder.entity("File", file.getName()).in(repository).build(), "Could not add file to repository"); + } } @Override diff --git a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/spi/HgModifyCommandTest.java b/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/spi/HgModifyCommandTest.java index 8ecc0bb18f..a87a95bd23 100644 --- a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/spi/HgModifyCommandTest.java +++ b/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/spi/HgModifyCommandTest.java @@ -69,10 +69,8 @@ public class HgModifyCommandTest extends AbstractHgCommandTestBase { @Test public void shouldRemoveFiles() { - ModifyCommandRequest request = new ModifyCommandRequest(); + ModifyCommandRequest request = prepareModifyCommandRequest(); request.addRequest(new ModifyCommandRequest.DeleteFileRequest("a.txt", false)); - request.setCommitMessage("this is great"); - request.setAuthor(new Person("Arthur Dent", "dent@hitchhiker.com")); String result = hgModifyCommand.execute(request); @@ -82,10 +80,8 @@ public class HgModifyCommandTest extends AbstractHgCommandTestBase { @Test public void shouldRemoveDirectoriesRecursively() { - ModifyCommandRequest request = new ModifyCommandRequest(); + ModifyCommandRequest request = prepareModifyCommandRequest(); request.addRequest(new ModifyCommandRequest.DeleteFileRequest("c", true)); - request.setCommitMessage("this is great"); - request.setAuthor(new Person("Arthur Dent", "dent@hitchhiker.com")); String result = hgModifyCommand.execute(request); @@ -97,10 +93,8 @@ public class HgModifyCommandTest extends AbstractHgCommandTestBase { public void shouldCreateFilesWithoutOverwrite() throws IOException { File testFile = temporaryFolder.newFile(); - ModifyCommandRequest request = new ModifyCommandRequest(); + ModifyCommandRequest request = prepareModifyCommandRequest(); request.addRequest(new ModifyCommandRequest.CreateFileRequest("Answer.txt", testFile, false)); - request.setCommitMessage("I found the answer"); - request.setAuthor(new Person("Trillian Astra", "trillian@hitchhiker.com")); String changeSet = hgModifyCommand.execute(request); @@ -115,12 +109,10 @@ public class HgModifyCommandTest extends AbstractHgCommandTestBase { try (FileOutputStream fos = new FileOutputStream(testFile)) { fos.write(42); } - ModifyCommandRequest request2 = new ModifyCommandRequest(); - request2.addRequest(new ModifyCommandRequest.CreateFileRequest("a.txt", testFile, true)); - request2.setCommitMessage(" Now i really found the answer"); - request2.setAuthor(new Person("Trillian Astra", "trillian@hitchhiker.com")); + ModifyCommandRequest request = prepareModifyCommandRequest(); + request.addRequest(new ModifyCommandRequest.CreateFileRequest("a.txt", testFile, true)); - String changeSet2 = hgModifyCommand.execute(request2); + String changeSet2 = hgModifyCommand.execute(request); assertThat(cmdContext.open().tip().getNode()).isEqualTo(changeSet2); assertThat(cmdContext.open().tip().getModifiedFiles().size()).isEqualTo(1); @@ -135,10 +127,8 @@ public class HgModifyCommandTest extends AbstractHgCommandTestBase { fos.write(21); } - ModifyCommandRequest request = new ModifyCommandRequest(); + ModifyCommandRequest request = prepareModifyCommandRequest(); request.addRequest(new ModifyCommandRequest.CreateFileRequest("Answer.txt", testFile, false)); - request.setCommitMessage("I found the answer"); - request.setAuthor(new Person("Trillian Astra", "trillian@hitchhiker.com")); hgModifyCommand.execute(request); @@ -146,9 +136,9 @@ public class HgModifyCommandTest extends AbstractHgCommandTestBase { fos.write(42); } ModifyCommandRequest request2 = new ModifyCommandRequest(); - request2.addRequest(new ModifyCommandRequest.CreateFileRequest("Answer.txt", testFile, false)); request2.setCommitMessage(" Now i really found the answer"); - request2.setAuthor(new Person("Trillian Astra", "trillian@hitchhiker.com")); + request.setAuthor(new Person("Arthur Dent", "dent@hitchhiker.com")); + request2.addRequest(new ModifyCommandRequest.CreateFileRequest("Answer.txt", testFile, false)); hgModifyCommand.execute(request2); } @@ -160,12 +150,10 @@ public class HgModifyCommandTest extends AbstractHgCommandTestBase { try (FileOutputStream fos = new FileOutputStream(testFile)) { fos.write(42); } - ModifyCommandRequest request2 = new ModifyCommandRequest(); - request2.addRequest(new ModifyCommandRequest.ModifyFileRequest("a.txt", testFile)); - request2.setCommitMessage(" Now i really found the answer"); - request2.setAuthor(new Person("Trillian Astra", "trillian@hitchhiker.com")); + ModifyCommandRequest request = prepareModifyCommandRequest(); + request.addRequest(new ModifyCommandRequest.ModifyFileRequest("a.txt", testFile)); - String changeSet2 = hgModifyCommand.execute(request2); + String changeSet2 = hgModifyCommand.execute(request); assertThat(cmdContext.open().tip().getNode()).isEqualTo(changeSet2); assertThat(cmdContext.open().tip().getModifiedFiles().size()).isEqualTo(1); @@ -179,12 +167,10 @@ public class HgModifyCommandTest extends AbstractHgCommandTestBase { try (FileOutputStream fos = new FileOutputStream(testFile)) { fos.write(42); } - ModifyCommandRequest request2 = new ModifyCommandRequest(); - request2.addRequest(new ModifyCommandRequest.ModifyFileRequest("Answer.txt", testFile)); - request2.setCommitMessage(" Now i really found the answer"); - request2.setAuthor(new Person("Trillian Astra", "trillian@hitchhiker.com")); + ModifyCommandRequest request = prepareModifyCommandRequest(); + request.addRequest(new ModifyCommandRequest.ModifyFileRequest("Answer.txt", testFile)); - hgModifyCommand.execute(request2); + hgModifyCommand.execute(request); } @Test(expected = NullPointerException.class) @@ -203,7 +189,7 @@ public class HgModifyCommandTest extends AbstractHgCommandTestBase { ModifyCommandRequest request = new ModifyCommandRequest(); request.addRequest(new ModifyCommandRequest.CreateFileRequest("Answer.txt", testFile, false)); - request.setAuthor(new Person("Trillian Astra", "trillian@hitchhiker.com")); + request.setAuthor(new Person("Arthur Dent", "dent@hitchhiker.com")); hgModifyCommand.execute(request); } @@ -233,30 +219,24 @@ public class HgModifyCommandTest extends AbstractHgCommandTestBase { try (FileOutputStream fos = new FileOutputStream(testFile)) { fos.write(42); } - ModifyCommandRequest request2 = new ModifyCommandRequest(); - request2.addRequest(new ModifyCommandRequest.CreateFileRequest(".hg/some.txt", testFile, true)); - request2.setCommitMessage("Now i really found the answer"); - request2.setAuthor(new Person("Trillian Astra", "trillian@hitchhiker.com")); + ModifyCommandRequest request = prepareModifyCommandRequest(); + request.addRequest(new ModifyCommandRequest.CreateFileRequest(".hg/some.txt", testFile, true)); - hgModifyCommand.execute(request2); + hgModifyCommand.execute(request); } @Test(expected = ScmConstraintViolationException.class) public void shouldThrowErrorIfRelativePathIsOutsideOfWorkdir() { - ModifyCommandRequest request = new ModifyCommandRequest(); - request.addRequest(new ModifyCommandRequest.MoveRequest("a.txt", "/../../../../g.txt")); - request.setCommitMessage("Now i really found the answer"); - request.setAuthor(new Person("Trillian Astra", "trillian@hitchhiker.com")); + ModifyCommandRequest request = prepareModifyCommandRequest(); + request.addRequest(new ModifyCommandRequest.MoveRequest("a.txt", "/../../../../g.txt", false)); hgModifyCommand.execute(request); } @Test public void shouldRenameFile() { - ModifyCommandRequest request = new ModifyCommandRequest(); - request.addRequest(new ModifyCommandRequest.MoveRequest("a.txt", "/g.txt")); - request.setCommitMessage("Now i really found the answer"); - request.setAuthor(new Person("Trillian Astra", "trillian@hitchhiker.com")); + ModifyCommandRequest request = prepareModifyCommandRequest(); + request.addRequest(new ModifyCommandRequest.MoveRequest("a.txt", "/g.txt", false)); hgModifyCommand.execute(request); assertThat(cmdContext.open().tip().getDeletedFiles()).contains(new File("a.txt").toString()); @@ -265,20 +245,16 @@ public class HgModifyCommandTest extends AbstractHgCommandTestBase { @Test(expected = AlreadyExistsException.class) public void shouldThrowAlreadyExistsException() { - ModifyCommandRequest request = new ModifyCommandRequest(); - request.addRequest(new ModifyCommandRequest.MoveRequest("a.txt", "/c")); - request.setCommitMessage("please rename my file pretty please"); - request.setAuthor(new Person("Arthur Dent", "dent@hitchhiker.com")); + ModifyCommandRequest request = prepareModifyCommandRequest(); + request.addRequest(new ModifyCommandRequest.MoveRequest("a.txt", "/c", false)); hgModifyCommand.execute(request); } @Test public void shouldRenameFolder() { - ModifyCommandRequest request = new ModifyCommandRequest(); - request.addRequest(new ModifyCommandRequest.MoveRequest("c", "/notc")); - request.setCommitMessage("Now i really found the answer"); - request.setAuthor(new Person("Trillian Astra", "trillian@hitchhiker.com")); + ModifyCommandRequest request = prepareModifyCommandRequest(); + request.addRequest(new ModifyCommandRequest.MoveRequest("c", "/notc", false)); hgModifyCommand.execute(request); assertThat(cmdContext.open().tip().getDeletedFiles()).contains(new File("c/d.txt").toString()); @@ -289,10 +265,8 @@ public class HgModifyCommandTest extends AbstractHgCommandTestBase { @Test public void shouldMoveFileToExistingFolder() { - ModifyCommandRequest request = new ModifyCommandRequest(); - request.addRequest(new ModifyCommandRequest.MoveRequest("a.txt", "/c/z.txt")); - request.setCommitMessage("Now i really found the answer"); - request.setAuthor(new Person("Trillian Astra", "trillian@hitchhiker.com")); + ModifyCommandRequest request = prepareModifyCommandRequest(); + request.addRequest(new ModifyCommandRequest.MoveRequest("a.txt", "/c/z.txt", false)); hgModifyCommand.execute(request); assertThat(cmdContext.open().tip().getDeletedFiles()).contains(new File("a.txt").toString()); @@ -303,10 +277,8 @@ public class HgModifyCommandTest extends AbstractHgCommandTestBase { @Test public void shouldMoveFolderToExistingFolder() { - ModifyCommandRequest request = new ModifyCommandRequest(); - request.addRequest(new ModifyCommandRequest.MoveRequest("g/h", "/y/h")); - request.setCommitMessage("Now i really found the answer"); - request.setAuthor(new Person("Trillian Astra", "trillian@hitchhiker.com")); + ModifyCommandRequest request = prepareModifyCommandRequest(); + request.addRequest(new ModifyCommandRequest.MoveRequest("g/h", "/y/h", false)); hgModifyCommand.execute(request); assertThat(cmdContext.open().tip().getDeletedFiles()).contains(new File("g/h/j.txt").toString()); @@ -315,10 +287,8 @@ public class HgModifyCommandTest extends AbstractHgCommandTestBase { @Test public void shouldMoveFileToNonExistentFolder() { - ModifyCommandRequest request = new ModifyCommandRequest(); - request.addRequest(new ModifyCommandRequest.MoveRequest("a.txt", "/y/z.txt")); - request.setCommitMessage("Now i really found the answer"); - request.setAuthor(new Person("Trillian Astra", "trillian@hitchhiker.com")); + ModifyCommandRequest request = prepareModifyCommandRequest(); + request.addRequest(new ModifyCommandRequest.MoveRequest("a.txt", "/y/z.txt", false)); hgModifyCommand.execute(request); assertThat(cmdContext.open().tip().getDeletedFiles()).contains(new File("a.txt").toString()); @@ -327,10 +297,8 @@ public class HgModifyCommandTest extends AbstractHgCommandTestBase { @Test public void shouldMoveFolderToNonExistentFolder() { - ModifyCommandRequest request = new ModifyCommandRequest(); - request.addRequest(new ModifyCommandRequest.MoveRequest("c", "/j/k/c")); - request.setCommitMessage("Now i really found the answer"); - request.setAuthor(new Person("Trillian Astra", "trillian@hitchhiker.com")); + ModifyCommandRequest request = prepareModifyCommandRequest(); + request.addRequest(new ModifyCommandRequest.MoveRequest("c", "/j/k/c", false)); hgModifyCommand.execute(request); assertThat(cmdContext.open().tip().getDeletedFiles()).contains(new File("c/d.txt").toString()); @@ -338,4 +306,29 @@ public class HgModifyCommandTest extends AbstractHgCommandTestBase { assertThat(cmdContext.open().tip().getAddedFiles()).contains(new File("j/k/c/d.txt").toString()); assertThat(cmdContext.open().tip().getAddedFiles()).contains(new File("j/k/c/e.txt").toString()); } + + @Test + public void shouldMoveFileWithOverwrite() { + ModifyCommandRequest request = prepareModifyCommandRequest(); + request.addRequest(new ModifyCommandRequest.MoveRequest("a.txt", "b.txt", true)); + + hgModifyCommand.execute(request); + assertThat(cmdContext.open().tip().getDeletedFiles()).contains(new File("a.txt").toString()); + assertThat(cmdContext.open().tip().getModifiedFiles()).contains(new File("b.txt").toString()); + } + + @Test(expected = AlreadyExistsException.class) + public void shouldFailMoveAndKeepFilesWhenSourceAndTargetAreTheSame() { + ModifyCommandRequest request = prepareModifyCommandRequest(); + request.addRequest(new ModifyCommandRequest.MoveRequest("c", "c", false)); + + hgModifyCommand.execute(request); + } + + private ModifyCommandRequest prepareModifyCommandRequest() { + ModifyCommandRequest request = new ModifyCommandRequest(); + request.setCommitMessage("Make some changes"); + request.setAuthor(new Person("Arthur Dent", "dent@hitchhiker.com")); + return request; + } } diff --git a/scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/repository/spi/SvnModifyCommand.java b/scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/repository/spi/SvnModifyCommand.java index 58d3df7a06..56a820a93d 100644 --- a/scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/repository/spi/SvnModifyCommand.java +++ b/scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/repository/spi/SvnModifyCommand.java @@ -34,6 +34,7 @@ import org.tmatesoft.svn.core.wc.SVNClientManager; import org.tmatesoft.svn.core.wc.SVNWCClient; import org.tmatesoft.svn.core.wc.SVNWCUtil; import sonia.scm.ConcurrentModificationException; +import sonia.scm.ContextEntry; import sonia.scm.repository.InternalRepositoryException; import sonia.scm.repository.Repository; import sonia.scm.repository.SvnWorkingCopyFactory; @@ -143,6 +144,9 @@ public class SvnModifyCommand implements ModifyCommand { try { wcClient.doDelete(new File(workingDirectory, toBeDeleted), true, true, false); } catch (SVNException e) { + if (e.getErrorMessage().getErrorCode().getCode() == 125001) { + throw new ModificationFailedException(ContextEntry.ContextBuilder.entity("File", toBeDeleted).in(repository).build(), "Could not remove file from repository"); + } throw new InternalRepositoryException(repository, "could not delete file from repository"); } } @@ -161,6 +165,9 @@ public class SvnModifyCommand implements ModifyCommand { true ); } catch (SVNException e) { + if (e.getErrorMessage().getErrorCode().getCode() == 155010) { + throw new ModificationFailedException(ContextEntry.ContextBuilder.entity("File", name).in(repository).build(), "Could not add file to repository"); + } throw new InternalRepositoryException(repository, "could not add file to repository"); } } diff --git a/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/spi/SvnModifyCommandTest.java b/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/spi/SvnModifyCommandTest.java index c2a7c71fd8..c42a150b22 100644 --- a/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/spi/SvnModifyCommandTest.java +++ b/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/spi/SvnModifyCommandTest.java @@ -86,10 +86,8 @@ public class SvnModifyCommandTest extends AbstractSvnCommandTestBase { @Test public void shouldRemoveFiles() { - ModifyCommandRequest request = new ModifyCommandRequest(); + ModifyCommandRequest request = prepareModifyCommandRequest(); request.addRequest(new ModifyCommandRequest.DeleteFileRequest("a.txt", false)); - request.setCommitMessage("this is great"); - request.setAuthor(new Person("Arthur Dent", "dent@hitchhiker.com")); svnModifyCommand.execute(request); WorkingCopy workingCopy = workingCopyFactory.createWorkingCopy(context, null); @@ -99,10 +97,8 @@ public class SvnModifyCommandTest extends AbstractSvnCommandTestBase { @Test public void shouldRemoveDirectory() { - ModifyCommandRequest request = new ModifyCommandRequest(); + ModifyCommandRequest request = prepareModifyCommandRequest(); request.addRequest(new ModifyCommandRequest.DeleteFileRequest("c", true)); - request.setCommitMessage("this is great"); - request.setAuthor(new Person("Arthur Dent", "dent@hitchhiker.com")); svnModifyCommand.execute(request); WorkingCopy workingCopy = workingCopyFactory.createWorkingCopy(context, null); @@ -114,10 +110,8 @@ public class SvnModifyCommandTest extends AbstractSvnCommandTestBase { public void shouldAddNewFile() throws IOException { File testfile = temporaryFolder.newFile("Test123"); - ModifyCommandRequest request = new ModifyCommandRequest(); + ModifyCommandRequest request = prepareModifyCommandRequest(); request.addRequest(new ModifyCommandRequest.CreateFileRequest("Test123", testfile, false)); - request.setCommitMessage("this is great"); - request.setAuthor(new Person("Arthur Dent", "dent@hitchhiker.com")); svnModifyCommand.execute(request); @@ -129,11 +123,9 @@ public class SvnModifyCommandTest extends AbstractSvnCommandTestBase { public void shouldAddNewFileInDefaultPath() throws IOException { File testfile = temporaryFolder.newFile("Test123"); - ModifyCommandRequest request = new ModifyCommandRequest(); + ModifyCommandRequest request = prepareModifyCommandRequest(); request.setDefaultPath(true); request.addRequest(new ModifyCommandRequest.CreateFileRequest("Test123", testfile, false)); - request.setCommitMessage("this is great"); - request.setAuthor(new Person("Arthur Dent", "dent@hitchhiker.com")); svnModifyCommand.execute(request); @@ -145,10 +137,8 @@ public class SvnModifyCommandTest extends AbstractSvnCommandTestBase { public void shouldThrowFileAlreadyExistsException() throws IOException { File testfile = temporaryFolder.newFile("a.txt"); - ModifyCommandRequest request = new ModifyCommandRequest(); + ModifyCommandRequest request = prepareModifyCommandRequest(); request.addRequest(new ModifyCommandRequest.CreateFileRequest("a.txt", testfile, false)); - request.setCommitMessage("this is great"); - request.setAuthor(new Person("Arthur Dent", "dent@hitchhiker.com")); assertThrows(AlreadyExistsException.class, () -> svnModifyCommand.execute(request)); } @@ -157,10 +147,8 @@ public class SvnModifyCommandTest extends AbstractSvnCommandTestBase { public void shouldUpdateExistingFile() throws IOException { File testfile = temporaryFolder.newFile("a.txt"); - ModifyCommandRequest request = new ModifyCommandRequest(); + ModifyCommandRequest request = prepareModifyCommandRequest(); request.addRequest(new ModifyCommandRequest.CreateFileRequest("a.txt", testfile, true)); - request.setCommitMessage("this is great"); - request.setAuthor(new Person("Arthur Dent", "dent@hitchhiker.com")); svnModifyCommand.execute(request); @@ -173,10 +161,8 @@ public class SvnModifyCommandTest extends AbstractSvnCommandTestBase { public void shouldThrowExceptionIfExpectedRevisionDoesNotMatch() throws IOException { File testfile = temporaryFolder.newFile("Test123"); - ModifyCommandRequest request = new ModifyCommandRequest(); + ModifyCommandRequest request = prepareModifyCommandRequest(); request.addRequest(new ModifyCommandRequest.CreateFileRequest("Test123", testfile, false)); - request.setCommitMessage("this should not pass"); - request.setAuthor(new Person("Arthur Dent", "dent@hitchhiker.com")); request.setExpectedRevision("42"); assertThrows(ConcurrentModificationException.class, () -> svnModifyCommand.execute(request)); @@ -190,10 +176,8 @@ public class SvnModifyCommandTest extends AbstractSvnCommandTestBase { public void shouldPassIfExpectedRevisionMatchesCurrentRevision() throws IOException { File testfile = temporaryFolder.newFile("Test123"); - ModifyCommandRequest request = new ModifyCommandRequest(); + ModifyCommandRequest request = prepareModifyCommandRequest(); request.addRequest(new ModifyCommandRequest.CreateFileRequest("Test123", testfile, false)); - request.setCommitMessage("this should not pass"); - request.setAuthor(new Person("Arthur Dent", "dent@hitchhiker.com")); request.setExpectedRevision("6"); svnModifyCommand.execute(request); @@ -203,20 +187,16 @@ public class SvnModifyCommandTest extends AbstractSvnCommandTestBase { @Test(expected = ScmConstraintViolationException.class) public void shouldThrowErrorIfRelativePathIsOutsideOfWorkdir() { - ModifyCommandRequest request = new ModifyCommandRequest(); - request.addRequest(new ModifyCommandRequest.MoveRequest("a.txt", "/../../../../b.txt")); - request.setCommitMessage("please rename my file pretty please"); - request.setAuthor(new Person("Arthur Dent", "dent@hitchhiker.com")); + ModifyCommandRequest request = prepareModifyCommandRequest(); + request.addRequest(new ModifyCommandRequest.MoveRequest("a.txt", "/../../../../b.txt", false)); svnModifyCommand.execute(request); } @Test public void shouldRenameFile() { - ModifyCommandRequest request = new ModifyCommandRequest(); - request.addRequest(new ModifyCommandRequest.MoveRequest("a.txt", "/b.txt")); - request.setCommitMessage("please rename my file pretty please"); - request.setAuthor(new Person("Arthur Dent", "dent@hitchhiker.com")); + ModifyCommandRequest request = prepareModifyCommandRequest(); + request.addRequest(new ModifyCommandRequest.MoveRequest("a.txt", "/b.txt", false)); svnModifyCommand.execute(request); @@ -227,20 +207,16 @@ public class SvnModifyCommandTest extends AbstractSvnCommandTestBase { @Test(expected = AlreadyExistsException.class) public void shouldThrowAlreadyExistsException() { - ModifyCommandRequest request = new ModifyCommandRequest(); - request.addRequest(new ModifyCommandRequest.MoveRequest("a.txt", "/c")); - request.setCommitMessage("please rename my file pretty please"); - request.setAuthor(new Person("Arthur Dent", "dent@hitchhiker.com")); + ModifyCommandRequest request = prepareModifyCommandRequest(); + request.addRequest(new ModifyCommandRequest.MoveRequest("a.txt", "/c", false)); svnModifyCommand.execute(request); } @Test public void shouldRenameFolder() { - ModifyCommandRequest request = new ModifyCommandRequest(); - request.addRequest(new ModifyCommandRequest.MoveRequest("c", "/notc")); - request.setCommitMessage("please rename my file pretty please"); - request.setAuthor(new Person("Arthur Dent", "dent@hitchhiker.com")); + ModifyCommandRequest request = prepareModifyCommandRequest(); + request.addRequest(new ModifyCommandRequest.MoveRequest("c", "/notc", false)); svnModifyCommand.execute(request); @@ -253,10 +229,8 @@ public class SvnModifyCommandTest extends AbstractSvnCommandTestBase { @Test public void shouldMoveFileToExistingFolder() { - ModifyCommandRequest request = new ModifyCommandRequest(); - request.addRequest(new ModifyCommandRequest.MoveRequest("a.txt", "/c/z.txt")); - request.setCommitMessage("please rename my file pretty please"); - request.setAuthor(new Person("Arthur Dent", "dent@hitchhiker.com")); + ModifyCommandRequest request = prepareModifyCommandRequest(); + request.addRequest(new ModifyCommandRequest.MoveRequest("a.txt", "/c/z.txt", false)); svnModifyCommand.execute(request); @@ -269,10 +243,8 @@ public class SvnModifyCommandTest extends AbstractSvnCommandTestBase { @Test public void shouldMoveFolderToExistingFolder() { - ModifyCommandRequest request = new ModifyCommandRequest(); - request.addRequest(new ModifyCommandRequest.MoveRequest("g/h", "/h/h")); - request.setCommitMessage("please rename my file pretty please"); - request.setAuthor(new Person("Arthur Dent", "dent@hitchhiker.com")); + ModifyCommandRequest request = prepareModifyCommandRequest(); + request.addRequest(new ModifyCommandRequest.MoveRequest("g/h", "/h/h", false)); svnModifyCommand.execute(request); @@ -283,10 +255,8 @@ public class SvnModifyCommandTest extends AbstractSvnCommandTestBase { @Test public void shouldMoveFileToNonExistentFolder() { - ModifyCommandRequest request = new ModifyCommandRequest(); - request.addRequest(new ModifyCommandRequest.MoveRequest("a.txt", "/y/z.txt")); - request.setCommitMessage("please rename my file pretty please"); - request.setAuthor(new Person("Arthur Dent", "dent@hitchhiker.com")); + ModifyCommandRequest request = prepareModifyCommandRequest(); + request.addRequest(new ModifyCommandRequest.MoveRequest("a.txt", "/y/z.txt", false)); svnModifyCommand.execute(request); @@ -295,12 +265,22 @@ public class SvnModifyCommandTest extends AbstractSvnCommandTestBase { assertThat(new File(workingCopy.getWorkingRepository(), "y/z.txt")).exists(); } + @Test + public void shouldMoveFileWithOverwrite() { + ModifyCommandRequest request = prepareModifyCommandRequest(); + request.addRequest(new ModifyCommandRequest.MoveRequest("a.txt", "b.txt", true)); + + svnModifyCommand.execute(request); + + WorkingCopy workingCopy = workingCopyFactory.createWorkingCopy(context, null); + assertThat(new File(workingCopy.getWorkingRepository(), "a.txt")).doesNotExist(); + assertThat(new File(workingCopy.getWorkingRepository(), "b.txt")).exists(); + } + @Test public void shouldMoveFolderToNonExistentFolder() { - ModifyCommandRequest request = new ModifyCommandRequest(); - request.addRequest(new ModifyCommandRequest.MoveRequest("c", "/j/k/c")); - request.setCommitMessage("please rename my file pretty please"); - request.setAuthor(new Person("Arthur Dent", "dent@hitchhiker.com")); + ModifyCommandRequest request = prepareModifyCommandRequest(); + request.addRequest(new ModifyCommandRequest.MoveRequest("c", "/j/k/c", false)); svnModifyCommand.execute(request); @@ -311,6 +291,14 @@ public class SvnModifyCommandTest extends AbstractSvnCommandTestBase { assertThat(new File(workingCopy.getWorkingRepository(), "j/k/c/e.txt")).exists(); } + @Test(expected = AlreadyExistsException.class) + public void shouldFailMoveAndKeepFilesWhenSourceAndTargetAreTheSame() { + ModifyCommandRequest request = prepareModifyCommandRequest(); + request.addRequest(new ModifyCommandRequest.MoveRequest("c", "c", false)); + + svnModifyCommand.execute(request); + } + @Test public void shouldFailIfLockedByOtherPerson() { Subject subject = mock(Subject.class); @@ -321,10 +309,8 @@ public class SvnModifyCommandTest extends AbstractSvnCommandTestBase { initSecurityManager(); - ModifyCommandRequest request = new ModifyCommandRequest(); + ModifyCommandRequest request = prepareModifyCommandRequest(); request.addRequest(new ModifyCommandRequest.DeleteFileRequest("a.txt", false)); - request.setCommitMessage("this should not happen"); - request.setAuthor(new Person("Arthur Dent", "dent@hitchhiker.com")); assertThrows(FileLockedException.class, () -> svnModifyCommand.execute(request)); WorkingCopy workingCopy = workingCopyFactory.createWorkingCopy(context, null); @@ -335,10 +321,8 @@ public class SvnModifyCommandTest extends AbstractSvnCommandTestBase { public void shouldSucceedIfLockedByUser() { lockFile(); - ModifyCommandRequest request = new ModifyCommandRequest(); + ModifyCommandRequest request = prepareModifyCommandRequest(); request.addRequest(new ModifyCommandRequest.DeleteFileRequest("a.txt", false)); - request.setCommitMessage("this should not happen"); - request.setAuthor(new Person("Arthur Dent", "dent@hitchhiker.com")); svnModifyCommand.execute(request); @@ -347,6 +331,13 @@ public class SvnModifyCommandTest extends AbstractSvnCommandTestBase { assertThat(getLock()).isEmpty(); } + private ModifyCommandRequest prepareModifyCommandRequest() { + ModifyCommandRequest request = new ModifyCommandRequest(); + request.setCommitMessage("modify some things"); + request.setAuthor(new Person("Arthur Dent", "dent@hitchhiker.com")); + return request; + } + private void lockFile() { SvnFileLockCommand svnFileLockCommand = new SvnFileLockCommand(context); LockCommandRequest lockRequest = new LockCommandRequest(); diff --git a/scm-webapp/src/main/java/sonia/scm/api/ModificationFailedExceptionMapper.java b/scm-webapp/src/main/java/sonia/scm/api/ModificationFailedExceptionMapper.java new file mode 100644 index 0000000000..d6b62f9767 --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/api/ModificationFailedExceptionMapper.java @@ -0,0 +1,40 @@ +/* + * MIT License + * + * Copyright (c) 2020-present Cloudogu GmbH and Contributors + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +package sonia.scm.api; + +import sonia.scm.api.rest.ContextualExceptionMapper; +import sonia.scm.api.v2.resources.ExceptionWithContextToErrorDtoMapper; +import sonia.scm.repository.spi.ModificationFailedException; + +import javax.inject.Inject; +import javax.ws.rs.core.Response; + +public class ModificationFailedExceptionMapper extends ContextualExceptionMapper { + + @Inject + public ModificationFailedExceptionMapper(ExceptionWithContextToErrorDtoMapper mapper) { + super(ModificationFailedException.class, Response.Status.BAD_REQUEST, mapper); + } +} diff --git a/scm-webapp/src/main/resources/locales/de/plugins.json b/scm-webapp/src/main/resources/locales/de/plugins.json index 43069c5175..90a7c608c5 100644 --- a/scm-webapp/src/main/resources/locales/de/plugins.json +++ b/scm-webapp/src/main/resources/locales/de/plugins.json @@ -429,6 +429,10 @@ "3mSmwOtOd1": { "displayName": "Datei gesperrt", "description": "Die Datei oder ihre Sperre kann nicht bearbeitet werden, da eine andere Sperre existiert. Die andere Sperre kann ggf. durch eine 'forcierte' Aktion umgangen werden." + }, + "8wSpi62oJ1": { + "displayName": "Änderung fehlgeschlagen", + "description": "Die Änderung konnte nicht durchgeführt werden. Dieses kann mehrere Ursachen haben, z. B. eine Datei die nicht verschoben werden kann, ungültige Dateinamen o. ä." } }, "namespaceStrategies": { diff --git a/scm-webapp/src/main/resources/locales/en/plugins.json b/scm-webapp/src/main/resources/locales/en/plugins.json index c3f9f04860..4045fb2c68 100644 --- a/scm-webapp/src/main/resources/locales/en/plugins.json +++ b/scm-webapp/src/main/resources/locales/en/plugins.json @@ -370,6 +370,10 @@ "3mSmwOtOd1": { "displayName": "File locked", "description": "The file or its lock cannot be modified, because another lock exists. This other lock may be ignored by using a 'forced' action." + }, + "8wSpi62oJ1": { + "displayName": "Modification failed", + "description": "The modification could not be applied. This can have many reasons, for example a file that could not be moved, invalid file names, etc." } }, "healthChecksFailures": {