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 <matthias.thieroff@cloudogu.com>
This commit is contained in:
René Pfeuffer
2021-11-30 08:49:47 +01:00
committed by GitHub
parent 5eb1d9cd22
commit 6ea77b42ca
19 changed files with 400 additions and 242 deletions

View File

@@ -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");
}
}

View File

@@ -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<File, File> 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<File, File> 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<File, File> 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<File, File> 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();