Add move to modify command (#1859)

dds a move/rename functionality to the modify command. This currently only works for absolute destination paths and does not work with backslashes. If the destination path does not exist, it is created. The action fails if the target file already exists.

scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitModifyCommand.java

@@ -163,6 +163,15 @@ public class GitModifyCommand extends AbstractGitCommand implements ModifyComman
       }
     }
 
+    @Override
+    public void addMovedFileToScm(String path, Path targetPath) {
+      try {
+        addFileToGit(path);
+      } catch (GitAPIException e) {
+        throwInternalRepositoryException("could not add file to index", e);
+      }
+    }
+
     private void addFileToGit(String toBeCreated) throws GitAPIException {
       getClone().add().addFilepattern(removeStartingPathSeparators(toBeCreated)).call();
     }

scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitModifyCommandTest.java

@@ -55,6 +55,11 @@ import static org.mockito.Mockito.verify;
 
 public class GitModifyCommandTest extends GitModifyCommandTestBase {
 
+  @Override
+  protected String getZippedRepositoryResource() {
+    return "sonia/scm/repository/spi/scm-git-spi-move-test.zip";
+  }
+
   @Test
   public void shouldCreateCommit() throws IOException, GitAPIException {
     File newFile = Files.write(tempFolder.newFile().toPath(), "new content".getBytes()).toFile();
@@ -400,4 +405,148 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase {
 
     command.execute(request);
   }
+
+  @Test(expected = ScmConstraintViolationException.class)
+  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"));
+
+    command.execute(request);
+  }
+
+  @Test
+  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"));
+
+    command.execute(request);
+
+    TreeAssertions assertions = canonicalTreeParser -> {
+      assertThat(canonicalTreeParser.findFile("b.txt")).isFalse();
+      assertThat(canonicalTreeParser.findFile("d.txt")).isTrue();
+    };
+
+    assertInTree(assertions);
+  }
+
+  @Test(expected = AlreadyExistsException.class)
+  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"));
+
+    command.execute(request);
+  }
+
+  @Test
+  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"));
+
+    command.execute(request);
+
+    TreeAssertions assertions = canonicalTreeParser -> {
+      assertThat(canonicalTreeParser.findFile("c/d.txt")).isFalse();
+      assertThat(canonicalTreeParser.findFile("c/e.txt")).isFalse();
+      assertThat(canonicalTreeParser.findFile("notc/d.txt")).isTrue();
+      assertThat(canonicalTreeParser.findFile("notc/e.txt")).isTrue();
+    };
+
+    assertInTree(assertions);
+  }
+
+  @Test
+  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"));
+
+    command.execute(request);
+
+    TreeAssertions assertions = canonicalTreeParser -> {
+      assertThat(canonicalTreeParser.findFile("a.txt")).isFalse();
+      assertThat(canonicalTreeParser.findFile("c/z.txt")).isTrue();
+      assertThat(canonicalTreeParser.findFile("c/d.txt")).isTrue();
+      assertThat(canonicalTreeParser.findFile("c/e.txt")).isTrue();
+    };
+
+    assertInTree(assertions);
+  }
+
+  @Test
+  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"));
+
+    command.execute(request);
+
+    TreeAssertions assertions = canonicalTreeParser -> {
+      assertThat(canonicalTreeParser.findFile("g/h/j.txt")).isFalse();
+      assertThat(canonicalTreeParser.findFile("g/k/h/j.txt")).isTrue();
+    };
+
+    assertInTree(assertions);
+  }
+
+  @Test
+  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"));
+
+    command.execute(request);
+
+    TreeAssertions assertions = fileFinder -> {
+      assertThat(fileFinder.findFile("a.txt")).isFalse();
+      assertThat(fileFinder.findFile("y/z.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"));
+
+    command.execute(request);
+
+    TreeAssertions assertions = fileFinder -> {
+      assertThat(fileFinder.findFile("c/d.txt")).isFalse();
+      assertThat(fileFinder.findFile("c/e.txt")).isFalse();
+      assertThat(fileFinder.findFile("j/k/c/d.txt")).isTrue();
+      assertThat(fileFinder.findFile("j/k/c/e.txt")).isTrue();
+    };
+
+    assertInTree(assertions);
+  }
 }

scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitModifyCommandTestBase.java

@@ -29,16 +29,13 @@ import com.github.sdorra.shiro.SubjectAware;
 import io.micrometer.core.instrument.simple.SimpleMeterRegistry;
 import org.eclipse.jgit.api.Git;
 import org.eclipse.jgit.api.errors.GitAPIException;
-import org.eclipse.jgit.errors.CorruptObjectException;
 import org.eclipse.jgit.lib.GpgSigner;
 import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.lib.ObjectReader;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.RevWalk;
-import org.eclipse.jgit.treewalk.CanonicalTreeParser;
+import org.eclipse.jgit.treewalk.TreeWalk;
 import org.junit.BeforeClass;
 import org.junit.Rule;
-import org.junit.rules.TemporaryFolder;
 import sonia.scm.repository.GitTestHelper;
 import sonia.scm.repository.work.NoneCachingWorkingCopyPool;
 import sonia.scm.repository.work.WorkdirProvider;
@@ -82,15 +79,18 @@ class GitModifyCommandTestBase extends AbstractGitCommandTestBase {
       try (RevWalk walk = new RevWalk(git.getRepository())) {
         RevCommit commit = walk.parseCommit(lastCommit);
         ObjectId treeId = commit.getTree().getId();
-        try (ObjectReader reader = git.getRepository().newObjectReader()) {
-          assertions.checkAssertions(new CanonicalTreeParser(null, reader, treeId));
-        }
+        assertions.checkAssertions(path -> TreeWalk.forPath(git.getRepository(), path, treeId) != null);
       }
     }
   }
 
   @FunctionalInterface
   interface TreeAssertions {
-    void checkAssertions(CanonicalTreeParser treeParser) throws CorruptObjectException;
+    void checkAssertions(FileFinder fileFinder) throws IOException;
+  }
+
+  @FunctionalInterface
+  interface FileFinder {
+    boolean findFile(String path) throws IOException;
   }
 }

scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitModifyCommand_LFSTest.java

@@ -26,7 +26,10 @@ package sonia.scm.repository.spi;
 
 import org.eclipse.jgit.api.Git;
 import org.eclipse.jgit.api.errors.GitAPIException;
+import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevWalk;
+import org.eclipse.jgit.treewalk.TreeWalk;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.Test;
@@ -42,6 +45,8 @@ import java.nio.file.Files;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 public class GitModifyCommand_LFSTest extends GitModifyCommandTestBase {
@@ -68,7 +73,7 @@ public class GitModifyCommand_LFSTest extends GitModifyCommandTestBase {
       assertThat(newRef).isEqualTo(lastCommit.toObjectId().name());
     }
 
-    assertThat(outputStream.toString()).isEqualTo("new content");
+    assertThat(outputStream).hasToString("new content");
   }
 
   @Test
@@ -85,13 +90,41 @@ public class GitModifyCommand_LFSTest extends GitModifyCommandTestBase {
       assertThat(newRef).isEqualTo(lastCommit.toObjectId().name());
     }
 
-    assertThat(outputStream.toString()).isEqualTo("more content");
+    assertThat(outputStream).hasToString("more content");
+  }
+
+  @Test
+  public void shouldMoveLfsFile() throws IOException, GitAPIException {
+    BlobStore blobStore = mockBlobStore();
+
+    createCommit("new_lfs.png", "new content", "fe32608c9ef5b6cf7e3f946480253ff76f24f4ec0678f3d0f07f9844cbff9601", new ByteArrayOutputStream());
+
+    GitModifyCommand command = createCommand();
+    ModifyCommandRequest request = new ModifyCommandRequest();
+    request.setCommitMessage("Move file");
+    request.addRequest(new ModifyCommandRequest.MoveRequest("new_lfs.png", "moved_lfs.png"));
+    request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det"));
+    command.execute(request);
+
+    verify(blobStore, never()).get(any());
+
+    // we have to assert, that the content of the new file has not changed (that is, the lfs pointer
+    // has only been moved. Therefore, we ensure that the object id (aka hash) of "moved_lfs.png"
+    // stays the same (182f...)
+    try (Git git = new Git(createContext().open())) {
+      RevCommit lastCommit = getLastCommit(git);
+      try (RevWalk walk = new RevWalk(git.getRepository())) {
+        RevCommit commit = walk.parseCommit(lastCommit);
+        ObjectId treeId = commit.getTree().getId();
+        TreeWalk treeWalk = TreeWalk.forPath(git.getRepository(), "moved_lfs.png", treeId);
+        assertThat(treeWalk.getObjectId(0).getName()).isEqualTo("182fd989777cad6d7e4c887e39e518f6a4acc5bd");
+      }
+    }
   }
 
   private String createCommit(String fileName, String content, String hashOfContent, ByteArrayOutputStream outputStream) throws IOException {
-    BlobStore blobStore = mock(BlobStore.class);
+    BlobStore blobStore = mockBlobStore();
     Blob blob = mock(Blob.class);
-    when(lfsBlobStoreFactory.getLfsBlobStore(any())).thenReturn(blobStore);
     when(blobStore.create(hashOfContent)).thenReturn(blob);
     when(blobStore.get(hashOfContent)).thenReturn(null, blob);
     when(blob.getOutputStream()).thenReturn(outputStream);
@@ -109,6 +142,12 @@ public class GitModifyCommand_LFSTest extends GitModifyCommandTestBase {
     return command.execute(request);
   }
 
+  private BlobStore mockBlobStore() {
+    BlobStore blobStore = mock(BlobStore.class);
+    when(lfsBlobStoreFactory.getLfsBlobStore(any())).thenReturn(blobStore);
+    return blobStore;
+  }
+
   @Override
   protected String getZippedRepositoryResource() {
     return "sonia/scm/repository/spi/scm-git-spi-lfs-test.zip";