Make change types explicit

Without explicit change types, we cannot tell copy and rename apart.

scm-core/src/main/java/sonia/scm/repository/api/DiffFile.java

@@ -21,7 +21,7 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
  * SOFTWARE.
  */
-    
+
 package sonia.scm.repository.api;
 
 public interface DiffFile extends Iterable<Hunk> {
@@ -33,4 +33,10 @@ public interface DiffFile extends Iterable<Hunk> {
   String getOldPath();
 
   String getNewPath();
+
+  ChangeType getChangeType();
+
+  enum ChangeType {
+    ADD, MODIFY, DELETE, RENAME, COPY
+  }
 }

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

@@ -57,7 +57,7 @@ public class GitDiffCommand extends AbstractGitCommand implements DiffCommand {
         formatter.setRepository(repository);
 
         for (DiffEntry e : diff.getEntries()) {
-          if (!e.getOldId().equals(e.getNewId()) || !e.getNewPath().equals(e.getOldPath())) {
+          if (idOrPathChanged(e)) {
             formatter.format(e);
           }
         }
@@ -67,6 +67,10 @@ public class GitDiffCommand extends AbstractGitCommand implements DiffCommand {
     };
   }
 
+  private boolean idOrPathChanged(DiffEntry e) {
+    return !e.getOldId().equals(e.getNewId()) || !e.getNewPath().equals(e.getOldPath());
+  }
+
   static class DequoteOutputStream extends OutputStream {
 
     private static final String[] DEQUOTE_STARTS = {

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

@@ -108,6 +108,24 @@ public class GitDiffResultCommand extends AbstractGitCommand implements DiffResu
       return diffEntry.getNewPath();
     }
 
+    @Override
+    public ChangeType getChangeType() {
+      switch (diffEntry.getChangeType()) {
+        case ADD:
+          return ChangeType.ADD;
+        case MODIFY:
+          return ChangeType.MODIFY;
+        case RENAME:
+          return ChangeType.RENAME;
+        case DELETE:
+          return ChangeType.DELETE;
+        case COPY:
+          return ChangeType.COPY;
+        default:
+          throw new IllegalArgumentException("Unknown change type: " + diffEntry.getChangeType());
+      }
+    }
+
     @Override
     public Iterator<Hunk> iterator() {
       String content = format(repository, diffEntry);

scm-webapp/src/main/java/sonia/scm/api/v2/resources/DiffResultToDiffResultDtoMapper.java

@@ -26,7 +26,6 @@ package sonia.scm.api.v2.resources;
 
 import com.github.sdorra.spotter.ContentTypes;
 import com.github.sdorra.spotter.Language;
-import com.google.common.base.Strings;
 import com.google.inject.Inject;
 import sonia.scm.repository.Repository;
 import sonia.scm.repository.api.DiffFile;
@@ -42,7 +41,7 @@ import java.util.OptionalInt;
 import static de.otto.edison.hal.Links.linkingTo;
 
 /**
- * TODO conflicts, copy and rename
+ * TODO conflicts
  */
 class DiffResultToDiffResultDtoMapper {
 
@@ -83,21 +82,29 @@ class DiffResultToDiffResultDtoMapper {
     String oldPath = file.getOldPath();
 
     String path;
-    if (isFilePath(newPath) && isFileNull(oldPath)) {
-      path = newPath;
-      dto.setType("add");
-    } else if (isFileNull(newPath) && isFilePath(oldPath)) {
-      path = oldPath;
-      dto.setType("delete");
-    } else if (!newPath.equals(oldPath)) {
-      path = newPath;
-      dto.setType("rename");
-    } else if (isFilePath(newPath) && isFilePath(oldPath)) {
-      path = newPath;
-      dto.setType("modify");
-    } else {
-      // TODO copy?
-      throw new IllegalStateException("no file without path");
+    switch (file.getChangeType()) {
+      case ADD:
+        path = newPath;
+        dto.setType("add");
+        break;
+      case DELETE:
+        path = oldPath;
+        dto.setType("delete");
+        break;
+      case RENAME:
+        path = newPath;
+        dto.setType("rename");
+        break;
+      case MODIFY:
+        path = newPath;
+        dto.setType("modify");
+        break;
+      case COPY:
+        path = newPath;
+        dto.setType("copy");
+        break;
+      default:
+        throw new IllegalArgumentException("unknown change type: " + file.getChangeType());
     }
 
     dto.setNewPath(newPath);
@@ -119,14 +126,6 @@ class DiffResultToDiffResultDtoMapper {
     return dto;
   }
 
-  private boolean isFilePath(String path) {
-    return !isFileNull(path);
-  }
-
-  private boolean isFileNull(String path) {
-    return Strings.isNullOrEmpty(path) || "/dev/null".equals(path);
-  }
-
   private DiffResultDto.HunkDto mapHunk(Hunk hunk) {
     DiffResultDto.HunkDto dto = new DiffResultDto.HunkDto();
     dto.setContent(hunk.getRawHeader());

scm-webapp/src/test/java/sonia/scm/api/v2/resources/DiffResultToDiffResultDtoMapperTest.java

@@ -63,6 +63,7 @@ class DiffResultToDiffResultDtoMapperTest {
     assertModifiedFile(files.get(1), "B.ts", "abc", "def", "typescript");
     assertDeletedFile(files.get(2), "C.go", "ghi", "golang");
     assertRenamedFile(files.get(3), "typo.ts", "okay.ts", "def",  "fixed", "typescript");
+    assertCopiedFile(files.get(4), "good.ts", "better.ts", "def",  "fixed", "typescript");
 
     DiffResultDto.HunkDto hunk = files.get(1).getHunks().get(0);
     assertHunk(hunk, "@@ -3,4 1,2 @@", 1, 2, 3, 4);
@@ -108,7 +109,8 @@ class DiffResultToDiffResultDtoMapperTest {
         )
       ),
       deletedFile("C.go", "ghi"),
-      renamedFile("okay.ts", "typo.ts", "fixed", "def")
+      renamedFile("okay.ts", "typo.ts", "fixed", "def"),
+      copiedFile("better.ts", "good.ts", "fixed", "def")
     );
   }
 
@@ -174,6 +176,15 @@ class DiffResultToDiffResultDtoMapperTest {
     assertThat(file.getLanguage()).isEqualTo(language);
   }
 
+  private void assertCopiedFile(DiffResultDto.FileDto file, String oldPath, String newPath, String oldRevision, String newRevision, String language) {
+    assertThat(file.getOldPath()).isEqualTo(oldPath);
+    assertThat(file.getNewPath()).isEqualTo(newPath);
+    assertThat(file.getOldRevision()).isEqualTo(oldRevision);
+    assertThat(file.getNewRevision()).isEqualTo(newRevision);
+    assertThat(file.getType()).isEqualTo("copy");
+    assertThat(file.getLanguage()).isEqualTo(language);
+  }
+
   private DiffResult result(DiffFile... files) {
     DiffResult result = mock(DiffResult.class);
     when(result.iterator()).thenReturn(Arrays.asList(files).iterator());
@@ -184,6 +195,7 @@ class DiffResultToDiffResultDtoMapperTest {
     DiffFile file = mock(DiffFile.class);
     when(file.getNewPath()).thenReturn(path);
     when(file.getNewRevision()).thenReturn(revision);
+    when(file.getChangeType()).thenReturn(DiffFile.ChangeType.ADD);
     when(file.iterator()).thenReturn(Arrays.asList(hunks).iterator());
     return file;
   }
@@ -192,6 +204,7 @@ class DiffResultToDiffResultDtoMapperTest {
     DiffFile file = mock(DiffFile.class);
     when(file.getOldPath()).thenReturn(path);
     when(file.getOldRevision()).thenReturn(revision);
+    when(file.getChangeType()).thenReturn(DiffFile.ChangeType.DELETE);
     when(file.iterator()).thenReturn(Arrays.asList(hunks).iterator());
     return file;
   }
@@ -202,6 +215,7 @@ class DiffResultToDiffResultDtoMapperTest {
     when(file.getNewRevision()).thenReturn(newRevision);
     when(file.getOldPath()).thenReturn(path);
     when(file.getOldRevision()).thenReturn(oldRevision);
+    when(file.getChangeType()).thenReturn(DiffFile.ChangeType.MODIFY);
     when(file.iterator()).thenReturn(Arrays.asList(hunks).iterator());
     return file;
   }
@@ -212,6 +226,18 @@ class DiffResultToDiffResultDtoMapperTest {
     when(file.getNewRevision()).thenReturn(newRevision);
     when(file.getOldPath()).thenReturn(oldPath);
     when(file.getOldRevision()).thenReturn(oldRevision);
+    when(file.getChangeType()).thenReturn(DiffFile.ChangeType.RENAME);
+    when(file.iterator()).thenReturn(emptyIterator());
+    return file;
+  }
+
+  private DiffFile copiedFile(String newPath, String oldPath, String newRevision, String oldRevision) {
+    DiffFile file = mock(DiffFile.class);
+    when(file.getNewPath()).thenReturn(newPath);
+    when(file.getNewRevision()).thenReturn(newRevision);
+    when(file.getOldPath()).thenReturn(oldPath);
+    when(file.getOldRevision()).thenReturn(oldRevision);
+    when(file.getChangeType()).thenReturn(DiffFile.ChangeType.COPY);
     when(file.iterator()).thenReturn(emptyIterator());
     return file;
   }