Adds 'head' as revision for Subversion repositories

To still support the editor plugin, a new field in the
browse command results is needed to indicate, whether
such a result or rather the requested revision can be
modified by a new commit.

This is the case, when
- for Subversion repositories either the new 'head' or the
latest revision has been requested, or
- for Git and HG when a branch (or the default by specifying
no concrete revision) has been used.

gradle/changelog/svn_head_revision.yaml

@@ -0,0 +1,4 @@
+- type: changed
+  description: The code view for Subversion repositories now uses the alias 'head' by default for the latest revision
+- type: added
+  description: A 'is modifiable' flag for the browse command result.

scm-core/src/main/java/sonia/scm/repository/BrowserResult.java

@@ -28,6 +28,7 @@ public class BrowserResult implements Serializable {
   private String revision;
   private String requestedRevision;
   private FileObject file;
+  private boolean modifiable;
 
   public BrowserResult() {
   }
@@ -36,10 +37,19 @@ public class BrowserResult implements Serializable {
     this(revision, revision, file);
   }
 
+  public BrowserResult(String revision, FileObject file, boolean modifiable) {
+    this(revision, revision, file, modifiable);
+  }
+
   public BrowserResult(String revision, String requestedRevision, FileObject file) {
+    this(revision, requestedRevision, file, false);
+  }
+
+  public BrowserResult(String revision, String requestedRevision, FileObject file, boolean modifiable) {
     this.revision = revision;
     this.requestedRevision = requestedRevision;
     this.file = file;
+    this.modifiable = modifiable;
   }
 
   public String getRevision() {
@@ -53,4 +63,13 @@ public class BrowserResult implements Serializable {
   public FileObject getFile() {
     return file;
   }
+
+  /**
+   * If this is <code>true</code>, the requested revision represents a modifiable head of the repository or (if
+   * supported) a branch and therefore can be modified.
+   * @since 3.6.0
+   */
+  public boolean isModifiable() {
+    return modifiable;
+  }
 }

scm-it/src/test/java/sonia/scm/it/RepositoryAccessITCase.java

@@ -77,12 +77,11 @@ public class RepositoryAccessITCase {
   public void init() {
     TestData.createDefault();
     folder = tempFolder.getRoot();
-    String namespace = ADMIN_USERNAME;
     String repo = TestData.getDefaultRepoName(repositoryType);
     repositoryResponse =
       ScmRequests.start()
         .requestIndexResource(ADMIN_USERNAME, ADMIN_PASSWORD)
-        .requestRepository(namespace, repo)
+        .requestRepository(ADMIN_USERNAME, repo)
         .assertStatusCode(HttpStatus.SC_OK);
   }
 
@@ -256,6 +255,7 @@ public class RepositoryAccessITCase {
   }
 
   @Test
+  @SuppressWarnings("rawtypes")
   public void shouldFindChangesets() throws IOException {
     RepositoryClient repositoryClient = RepositoryUtil.createRepositoryClient(repositoryType, folder);
 
@@ -302,7 +302,6 @@ public class RepositoryAccessITCase {
   }
 
   @Test
-  @SuppressWarnings("unchecked")
   public void shouldFindAddedModifications() throws IOException {
     RepositoryClient repositoryClient = RepositoryUtil.createRepositoryClient(repositoryType, folder);
     String fileName = "a.txt";
@@ -324,7 +323,6 @@ public class RepositoryAccessITCase {
   }
 
   @Test
-  @SuppressWarnings("unchecked")
   public void shouldFindRemovedModifications() throws IOException {
     RepositoryClient repositoryClient = RepositoryUtil.createRepositoryClient(repositoryType, folder);
     String fileName = "a.txt";
@@ -348,7 +346,6 @@ public class RepositoryAccessITCase {
   }
 
   @Test
-  @SuppressWarnings("unchecked")
   public void shouldFindUpdateModifications() throws IOException {
     RepositoryClient repositoryClient = RepositoryUtil.createRepositoryClient(repositoryType, folder);
     String fileName = "a.txt";
@@ -372,16 +369,15 @@ public class RepositoryAccessITCase {
   }
 
   @Test
-  @SuppressWarnings("unchecked")
   public void shouldFindMultipleModifications() throws IOException {
     RepositoryClient repositoryClient = RepositoryUtil.createRepositoryClient(repositoryType, folder);
     RepositoryUtil.createAndCommitFile(repositoryClient, ADMIN_USERNAME, "b.txt", "b");
     RepositoryUtil.createAndCommitFile(repositoryClient, ADMIN_USERNAME, "c.txt", "c");
     RepositoryUtil.createAndCommitFile(repositoryClient, ADMIN_USERNAME, "d.txt", "d");
-    Map<String, String> addedFiles = new HashMap<String, String>() {{
+    Map<String, String> addedFiles = new HashMap<>() {{
       put("a.txt", "bla bla");
     }};
-    Map<String, String> modifiedFiles = new HashMap<String, String>() {{
+    Map<String, String> modifiedFiles = new HashMap<>() {{
       put("b.txt", "new content");
     }};
     ArrayList<String> removedFiles = Lists.newArrayList("c.txt", "d.txt");
@@ -406,19 +402,16 @@ public class RepositoryAccessITCase {
   }
 
   @Test
-  @SuppressWarnings("unchecked")
   public void svnShouldCreateOneModificationPerFolder() throws IOException {
     Assume.assumeThat(repositoryType, equalTo("svn"));
     RepositoryClient repositoryClient = RepositoryUtil.createRepositoryClient(repositoryType, folder);
     RepositoryUtil.createAndCommitFile(repositoryClient, ADMIN_USERNAME, "bbb/bb/b.txt", "b");
     RepositoryUtil.createAndCommitFile(repositoryClient, ADMIN_USERNAME, "ccc/cc/c.txt", "c");
     RepositoryUtil.createAndCommitFile(repositoryClient, ADMIN_USERNAME, "ddd/dd/d.txt", "d");
-    Map<String, String> addedFiles = new HashMap<String, String>()
-    {{
+    Map<String, String> addedFiles = new HashMap<>() {{
       put("aaa/aa/a.txt", "bla bla");
     }};
-    Map<String, String> modifiedFiles = new HashMap<String, String>()
-    {{
+    Map<String, String> modifiedFiles = new HashMap<>() {{
       put("bbb/bb/b.txt", "new content");
     }};
     ArrayList<String> removedFiles = Lists.newArrayList("ccc/cc/c.txt", "ddd/dd/d.txt");

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

@@ -23,6 +23,8 @@ import com.google.common.collect.Maps;
 import com.google.inject.assistedinject.Assisted;
 import jakarta.annotation.Nullable;
 import jakarta.inject.Inject;
+import org.eclipse.jgit.api.Git;
+import org.eclipse.jgit.api.errors.GitAPIException;
 import org.eclipse.jgit.attributes.Attributes;
 import org.eclipse.jgit.lfs.LfsPointer;
 import org.eclipse.jgit.lib.Constants;
@@ -72,10 +74,10 @@ import static sonia.scm.repository.spi.SyncAsyncExecutor.ExecutionType.ASYNCHRON
 public class GitBrowseCommand extends AbstractGitCommand
   implements BrowseCommand {
 
-  
+
   public static final String PATH_MODULES = ".gitmodules";
 
- 
+
   private static final Logger logger = LoggerFactory.getLogger(GitBrowseCommand.class);
 
   private final Map<ObjectId, Map<String, SubRepository>> subrepositoryCache = Maps.newHashMap();
@@ -119,11 +121,26 @@ public class GitBrowseCommand extends AbstractGitCommand
     revId = computeRevIdToBrowse();
 
     if (revId != null) {
-      browserResult = new BrowserResult(revId.getName(), request.getRevision(), getEntry());
-      return browserResult;
+      try {
+        boolean isBranch = Strings.isNullOrEmpty(request.getRevision()) || new Git(repo)
+          .branchList()
+          .call()
+          .stream()
+          .map(GitUtil::getBranch)
+          .anyMatch(branch -> branch.equals(request.getRevision()));
+        browserResult =
+          new BrowserResult(
+            revId.getName(),
+            request.getRevision() == null ? Constants.HEAD : request.getRevision(),
+            getEntry(),
+            isBranch);
+        return browserResult;
+      } catch (GitAPIException e) {
+        throw new IOException(e);
+      }
     } else {
       logger.warn("could not find head of repository {}, empty?", repository);
-      return new BrowserResult(Constants.HEAD, request.getRevision(), createEmptyRoot());
+      return new BrowserResult(Constants.HEAD, createEmptyRoot(), true);
     }
   }
 
@@ -406,7 +423,7 @@ public class GitBrowseCommand extends AbstractGitCommand
       // The increment is required to not close the repository if the getLatestCommit runs before the RepositoryService
       // is closed.
       repo.incrementOpen();
-      try (RevWalk walk = new RevWalk(repo)) {
+      try (repo; RevWalk walk = new RevWalk(repo)) {
         walk.setTreeFilter(AndTreeFilter.create(TreeFilter.ANY_DIFF, PathFilter.create(path)));
 
         RevCommit commit = walk.parseCommit(revId);
@@ -416,8 +433,6 @@ public class GitBrowseCommand extends AbstractGitCommand
       } catch (IOException ex) {
         logger.error("could not parse commit for file", ex);
         return empty();
-      } finally {
-        repo.close();
       }
     }
 

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

@@ -54,11 +54,27 @@ public class GitBrowseCommandTest extends AbstractGitCommandTestBase {
     BrowseCommandRequest request = new BrowseCommandRequest();
     request.setPath("a.txt");
     BrowserResult result = createCommand().getBrowserResult(request);
+    assertTrue(result.isModifiable());
+    assertEquals("HEAD", result.getRequestedRevision());
     FileObject fileObject = result.getFile();
     assertEquals("a.txt", fileObject.getName());
     assertFalse(fileObject.isTruncated());
   }
 
+  @Test
+  public void testDifferentBranch() throws IOException {
+    BrowseCommandRequest request = new BrowseCommandRequest();
+    request.setRevision("test-branch");
+
+    BrowserResult result = createCommand().getBrowserResult(request);
+
+    assertTrue(result.isModifiable());
+    Collection<FileObject> foList = result.getFile().getChildren();
+    assertThat(foList)
+      .extracting("name")
+      .containsExactly("c", "a.txt");
+  }
+
   @Test
   public void testDefaultDefaultBranch() throws IOException {
     // without default branch, the repository head should be used

scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/HgBrowseCommand.java

@@ -19,7 +19,9 @@ package sonia.scm.repository.spi;
 
 import com.google.inject.assistedinject.Assisted;
 import jakarta.inject.Inject;
+import org.javahg.Branch;
 import org.javahg.Changeset;
+import org.javahg.commands.BranchesCommand;
 import org.javahg.commands.LogCommand;
 import com.google.common.base.MoreObjects;
 import com.google.common.base.Strings;
@@ -82,7 +84,13 @@ public class HgBrowseCommand extends AbstractCommand implements BrowseCommand
 
     FileObject file = cmd.execute()
       .orElseThrow(() -> notFound(entity("File", request.getPath()).in("Revision", revision).in(getRepository())));
-    return new BrowserResult(c == null? "tip": c.getNode(), revision, file);
+    boolean requestedRevisionIsBranch = BranchesCommand
+      .on(getContext().open())
+      .execute()
+      .stream()
+      .map(Branch::getName)
+      .anyMatch(b -> b.equals(request.getRevision()));
+    return new BrowserResult(c == null ? "tip" : c.getNode(), revision, file, requestedRevisionIsBranch);
   }
 
   public interface Factory {

scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/spi/HgBrowseCommandTest.java

@@ -65,6 +65,16 @@ public class HgBrowseCommandTest extends AbstractHgCommandTestBase {
     assertEquals("c", c.getPath());
   }
 
+  @Test
+  public void testBrowseWithRevision() throws IOException {
+    BrowseCommandRequest request = new BrowseCommandRequest();
+    request.setRevision("2baab8e80280ef05a9aa76c49c76feca2872afb7");
+
+    BrowserResult result = new HgBrowseCommand(cmdContext).getBrowserResult(request);
+
+    assertFalse(result.isModifiable());
+  }
+
   @Test
   public void testBrowseShouldResolveBranchForRevision() throws IOException {
     String defaultBranchRevision = new LogCommand(cmdContext.open()).rev("default").single().getNode();
@@ -74,6 +84,7 @@ public class HgBrowseCommandTest extends AbstractHgCommandTestBase {
 
     BrowserResult result = new HgBrowseCommand(cmdContext).getBrowserResult(browseCommandRequest);
 
+    assertTrue(result.isModifiable());
     assertThat(result.getRevision()).isEqualTo(defaultBranchRevision);
   }
 

scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/repository/SvnUtil.java

@@ -77,13 +77,15 @@ public final class SvnUtil
 
 
   public static long parseRevision(String v, Repository repository) {
-    long result = -1l;
-
     if (!Strings.isNullOrEmpty(v))
     {
+      if ("head".equals(v))
+      {
+        return -1;
+      }
       try
       {
-        result = Long.parseLong(v);
+        return Long.parseLong(v);
       }
       catch (NumberFormatException ex)
       {
@@ -91,7 +93,7 @@ public final class SvnUtil
       }
     }
 
-    return result;
+    return -1;
   }
 
   public static Modifications createModifications(String startRevision, String endRevision, Collection<SVNLogEntry> entries) {
@@ -261,22 +263,23 @@ public final class SvnUtil
   }
 
   public static long getRevisionNumber(String revision, Repository repository) {
-    // REVIEW Bei SVN wird ohne Revision die -1 genommen, was zu einem Fehler führt
-    long revisionNumber = -1;
-
     if (Util.isNotEmpty(revision))
     {
+      if ("head".equals(revision))
+      {
+        return -1;
+      }
       try
       {
-        revisionNumber = Long.parseLong(revision);
+        return Long.parseLong(revision);
       }
       catch (NumberFormatException ex)
       {
         throw notFound(entity("Revision", revision).in(repository));
       }
+    } else {
+      return -1;
     }
-
-    return revisionNumber;
   }
 
 

scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/repository/spi/SvnBrowseCommand.java

@@ -45,7 +45,7 @@ import static sonia.scm.NotFoundException.notFound;
 public class SvnBrowseCommand extends AbstractSvnCommand
   implements BrowseCommand {
 
- 
+
   private static final Logger logger =
     LoggerFactory.getLogger(SvnBrowseCommand.class);
 
@@ -56,10 +56,10 @@ public class SvnBrowseCommand extends AbstractSvnCommand
   }
 
   @Override
-  @SuppressWarnings("unchecked")
   public BrowserResult getBrowserResult(BrowseCommandRequest request) {
     String path = Strings.nullToEmpty(request.getPath());
     long revisionNumber = SvnUtil.getRevisionNumber(request.getRevision(), repository);
+    boolean headRequest = revisionNumber < 0;
 
     if (logger.isDebugEnabled()) {
       logger.debug("browser repository {} in path \"{}\" at revision {}", repository, path, revisionNumber);
@@ -84,20 +84,19 @@ public class SvnBrowseCommand extends AbstractSvnCommand
         traverse(svnRepository, revisionNumber, request, root, createBasePath(path));
       }
 
-
-      result = new BrowserResult(String.valueOf(revisionNumber), root);
+      result = new BrowserResult(String.valueOf(revisionNumber), headRequest ? "head" : String.valueOf(revisionNumber), root, headRequest);
     } catch (SVNException ex) {
       if (FS_NO_SUCH_REVISION.equals(ex.getErrorMessage().getErrorCode())) {
         throw notFound(entity("Revision", Long.toString(revisionNumber)).in(this.repository));
       }
-      logger.error("could not open repository: " + repository.getNamespaceAndName(), ex);
+      logger.error("could not open repository: " + repository, ex);
     }
 
     return result;
   }
 
 
-  @SuppressWarnings("unchecked")
+  @SuppressWarnings({"unchecked", "rawtypes"})
   private void traverse(SVNRepository svnRepository, long revisionNumber, BrowseCommandRequest request,
                         FileObject parent, String basePath)
     throws SVNException {

scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/repository/spi/SvnLogCommand.java

@@ -53,11 +53,16 @@ public class SvnLogCommand extends AbstractSvnCommand implements LogCommand {
     }
 
     try {
-      long revisioNumber = parseRevision(revision, repository);
-      Preconditions.checkArgument(revisioNumber > 0, "revision must be greater than zero: %d", revisioNumber);
+      long revisionNumber;
+      if ("head".equals(revision)) {
+        revisionNumber = -1;
+      } else {
+        revisionNumber = parseRevision(revision, repository);
+        Preconditions.checkArgument(revisionNumber > 0, "revision must be greater than zero: %d", revisionNumber);
+      }
       SVNRepository repo = open();
-      Collection<SVNLogEntry> entries = repo.log(null, null, revisioNumber,
-        revisioNumber, true, true);
+      Collection<SVNLogEntry> entries = repo.log(null, null, revisionNumber,
+        revisionNumber, true, true);
 
       if (Util.isNotEmpty(entries)) {
         changeset = SvnUtil.createChangeset(entries.iterator().next());
@@ -70,7 +75,6 @@ public class SvnLogCommand extends AbstractSvnCommand implements LogCommand {
   }
 
   @Override
-  @SuppressWarnings("unchecked")
   public ChangesetPagingResult getChangesets(LogCommandRequest request) {
     if (LOG.isDebugEnabled()) {
       LOG.debug("fetch changesets for {}", request);

scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/repository/spi/SvnModifyCommand.java

@@ -108,7 +108,7 @@ public class SvnModifyCommand implements ModifyCommand {
       if (svnCommitInfo.toString().equals("EMPTY COMMIT")) {
         throw new NoChangesMadeException(repository);
       }
-      return String.valueOf(svnCommitInfo.getNewRevision());
+      return "head";
     } catch (SVNException e) {
       throw withPattern(SVN_ERROR_PATTERN).forMessage(repository, e.getErrorMessage().getRootErrorMessage().getFullMessage());
     }

scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/spi/SvnBrowseCommandTest.java

@@ -27,7 +27,6 @@ import org.tmatesoft.svn.core.wc.SVNClientManager;
 import org.tmatesoft.svn.core.wc.SVNRevision;
 import sonia.scm.repository.BrowserResult;
 import sonia.scm.repository.FileObject;
-import sonia.scm.repository.SubRepository;
 
 import java.io.File;
 import java.io.IOException;
@@ -38,7 +37,6 @@ import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 
 
@@ -82,11 +80,6 @@ public class SvnBrowseCommandTest extends AbstractSvnCommandTestBase {
     assertEquals("c", c.getPath());
   }
 
-  /**
-   * Method description
-   *
-   * @throws IOException
-   */
   @Test
   public void testBrowseSubDirectory() {
     BrowseCommandRequest request = new BrowseCommandRequest();
@@ -286,6 +279,16 @@ public class SvnBrowseCommandTest extends AbstractSvnCommandTestBase {
     assertTrue(containsSubRepository);
   }
 
+  @Test
+  public void shouldGetHeadRevision() {
+    BrowseCommandRequest request = new BrowseCommandRequest();
+    request.setRevision("head");
+    BrowserResult browserResult = createCommand().getBrowserResult(request);
+
+    assertThat(browserResult.getRequestedRevision()).isEqualTo("head");
+    assertThat(browserResult.getRevision()).isEqualTo("5");
+  }
+
   private SvnContext setProp(String propName, String propValue) throws SVNException, IOException {
     SvnContext context = createContext();
     SVNClientManager client = SVNClientManager.newInstance();
@@ -300,18 +303,14 @@ public class SvnBrowseCommandTest extends AbstractSvnCommandTestBase {
     return context;
   }
 
-
   private SvnBrowseCommand createCommand() {
     return new SvnBrowseCommand(createContext());
   }
 
-
-
   private FileObject getFileObject(Collection<FileObject> foList, String name) {
     return foList.stream()
       .filter(f -> name.equals(f.getName()))
       .findFirst()
       .orElseThrow(() -> new AssertionError("file " + name + " not found"));
   }
-
 }

scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/spi/SvnLogCommandTest.java

@@ -25,11 +25,14 @@ import sonia.scm.repository.Modifications;
 
 import java.util.stream.StreamSupport;
 
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
 
 
-public class SvnLogCommandTest extends AbstractSvnCommandTestBase
-{
+public class SvnLogCommandTest extends AbstractSvnCommandTestBase {
 
   @Test
   public void testGetAll() {
@@ -179,9 +182,13 @@ public class SvnLogCommandTest extends AbstractSvnCommandTestBase
     assertEquals("1", c2.getId());
   }
 
+  @Test
+  public void shouldGetCorrectHeadRevision() {
+    Changeset changeset = createCommand().getChangeset("head", new LogCommandRequest());
+    assertEquals("5", changeset.getId());
+  }
 
-  private SvnLogCommand createCommand()
-  {
+  private SvnLogCommand createCommand() {
     return new SvnLogCommand(createContext());
   }
 }

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

@@ -40,7 +40,7 @@ public abstract class BrowserResultToFileObjectDtoMapper extends BaseFileObjectD
 
   FileObjectDto map(BrowserResult browserResult, NamespaceAndName namespaceAndName, int offset) {
     FileObjectDto fileObjectDto = fileObjectToDto(browserResult.getFile(), namespaceAndName, browserResult, offset);
-    fileObjectDto.setRevision(browserResult.getRevision());
+    fileObjectDto.setRevision(browserResult.getRequestedRevision());
 
     return fileObjectDto;
   }
@@ -59,7 +59,7 @@ public abstract class BrowserResultToFileObjectDtoMapper extends BaseFileObjectD
       applyEnrichers(appender, browserResult, namespaceAndName);
     }
     // we call enrichers, which are responsible for all file object top level browse result and its children
-    applyEnrichers(appender, fileObject, namespaceAndName, browserResult, browserResult.getRevision());
+    applyEnrichers(appender, fileObject, namespaceAndName, browserResult, browserResult.getRequestedRevision());
   }
 
   Optional<Instant> mapOptionalInstant(OptionalLong optionalLong) {