Fix recursive iteration

Due to the api of TreeWalk we have no real change to iterate this
recursively, so we get back to good old loops.

CHANGELOG.md

@@ -5,6 +5,9 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## unreleased
+- Fix recursive browse command for git ([#1361](https://github.com/scm-manager/scm-manager/pull/1361))
+
 ## [2.6.1] - 2020-09-30
 ### Fixed
 - Not found error when using browse command in empty hg repository ([#1355](https://github.com/scm-manager/scm-manager/pull/1355))

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

@@ -24,8 +24,6 @@
 
 package sonia.scm.repository.spi;
 
-//~--- non-JDK imports --------------------------------------------------------
-
 import com.google.common.base.Stopwatch;
 import com.google.common.base.Strings;
 import com.google.common.collect.Lists;
@@ -66,17 +64,15 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+import java.util.Stack;
 import java.util.function.Consumer;
 
-import static java.util.Collections.emptyList;
 import static java.util.Optional.empty;
 import static java.util.Optional.of;
 import static sonia.scm.ContextEntry.ContextBuilder.entity;
 import static sonia.scm.NotFoundException.notFound;
 import static sonia.scm.repository.spi.SyncAsyncExecutor.ExecutionType.ASYNCHRONOUS;
 
-//~--- JDK imports ------------------------------------------------------------
-
 /**
  *
  * @author Sebastian Sdorra
@@ -251,7 +247,7 @@ public class GitBrowseCommand extends AbstractGitCommand
 
   private void findChildren(FileObject parent, TreeWalk treeWalk) throws IOException {
     TreeEntry entry = new TreeEntry();
-    createTree(parent.getPath(), entry, treeWalk);
+    createTree(entry, treeWalk);
     convertToFileObject(parent, entry.getChildren());
   }
 
@@ -282,25 +278,22 @@ public class GitBrowseCommand extends AbstractGitCommand
     parent.setTruncated(hasNext);
   }
 
-  private Optional<TreeEntry> createTree(String path, TreeEntry parent, TreeWalk treeWalk) throws IOException {
+  private void createTree(TreeEntry parent, TreeWalk treeWalk) throws IOException {
-    List<TreeEntry> entries = new ArrayList<>();
+    Stack<TreeEntry> parents = new Stack<>();
+    parents.push(parent);
     while (treeWalk.next()) {
-      TreeEntry treeEntry = new TreeEntry(repo, treeWalk);
+      final String currentPath = treeWalk.getPathString();
-      if (!treeEntry.getPathString().startsWith(path)) {
+      while (!currentPath.startsWith(parents.peek().pathString)) {
-        parent.setChildren(entries);
+        parents.pop();
-        return of(treeEntry);
       }
-
+      TreeEntry currentParent = parents.peek();
-      entries.add(treeEntry);
+      TreeEntry treeEntry = new TreeEntry(repo, treeWalk);
-
+      currentParent.addChild(treeEntry);
       if (request.isRecursive() && treeEntry.getType() == TreeType.DIRECTORY) {
         treeWalk.enterSubtree();
-        Optional<TreeEntry> surplus = createTree(treeEntry.getNameString(), treeEntry, treeWalk);
+        parents.push(treeEntry);
-        surplus.ifPresent(entries::add);
       }
     }
-    parent.setChildren(entries);
-    return empty();
   }
 
   private FileObject findFirstMatch(TreeWalk treeWalk) throws IOException {
@@ -465,7 +458,9 @@ public class GitBrowseCommand extends AbstractGitCommand
     private final ObjectId objectId;
     private final TreeType type;
     private final SubRepository subRepository;
-    private List<TreeEntry> children = emptyList();
+    private final List<TreeEntry> children = new ArrayList<>();
+
+    private boolean sorted = true;
 
     TreeEntry() {
       pathString = "";
@@ -513,12 +508,16 @@ public class GitBrowseCommand extends AbstractGitCommand
     }
 
     List<TreeEntry> getChildren() {
+      if (!sorted) {
+        sort(children, entry -> entry.type != TreeType.FILE, TreeEntry::getNameString);
+        sorted = true;
+      }
       return children;
     }
 
-    void setChildren(List<TreeEntry> children) {
+    private void addChild(TreeEntry treeEntry) {
-      sort(children, entry -> entry.type != TreeType.FILE, TreeEntry::getNameString);
+      sorted = false;
-      this.children = children;
+      children.add(treeEntry);
     }
   }
 }

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

@@ -346,6 +346,50 @@ public class GitBrowseCommandTest extends AbstractGitCommandTestBase {
       .containsExactly("e.txt");
   }
 
+  @Test
+  public void testRecursionWithDeepPaths() throws IOException {
+    BrowseCommandRequest request = new BrowseCommandRequest();
+    request.setRevision("deep-folders");
+    request.setRecursive(true);
+
+    FileObject root = createCommand().getBrowserResult(request).getFile();
+
+    assertThat(root.getChildren())
+      .extracting("name")
+      .containsExactly("c", "a.txt", "b.txt", "f.txt");
+    FileObject c = findFile(root.getChildren(), "c");
+
+    assertThat(c.getChildren())
+      .extracting("name")
+      .containsExactly("1", "4", "d.txt", "e.txt");
+
+    FileObject f_1 = findFile(c.getChildren(), "1");
+    assertThat(f_1.getChildren())
+      .extracting("name")
+      .containsExactly("2");
+    FileObject f_12 = findFile(f_1.getChildren(), "2");
+    assertThat(f_12.getChildren())
+      .extracting("name")
+      .containsExactly("3");
+    FileObject f_123 = findFile(f_12.getChildren(), "3");
+    assertThat(f_123.getChildren())
+      .extracting("name")
+      .containsExactly("123.txt");
+
+    FileObject f_4 = findFile(c.getChildren(), "4");
+    assertThat(f_4.getChildren())
+      .extracting("name")
+      .containsExactly("5", "6");
+    FileObject f_45 = findFile(f_4.getChildren(), "5");
+    assertThat(f_45.getChildren())
+      .extracting("name")
+      .containsExactly("45-1.txt", "45-2.txt");
+    FileObject f_46 = findFile(f_4.getChildren(), "6");
+    assertThat(f_46.getChildren())
+      .extracting("name")
+      .containsExactly("46-1.txt", "46-2.txt");
+  }
+
   private FileObject findFile(Collection<FileObject> foList, String name) {
     return foList.stream()
       .filter(f -> name.equals(f.getName()))