Merge branch 'develop' into bugfix/rest-download

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

@@ -55,16 +55,18 @@ import sonia.scm.store.BlobStore;
 import sonia.scm.util.Util;
 import sonia.scm.web.lfs.LfsBlobStoreFactory;
 
+import javax.annotation.Nullable;
 import javax.inject.Inject;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
+import java.util.ArrayDeque;
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.Deque;
 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.Optional.empty;
@@ -141,12 +143,12 @@ public class GitBrowseCommand extends AbstractGitCommand
     if (Util.isEmpty(request.getRevision())) {
       return getDefaultBranch(repo);
     } else {
-      ObjectId revId = GitUtil.getRevisionId(repo, request.getRevision());
-      if (revId == null) {
+      ObjectId revisionId = GitUtil.getRevisionId(repo, request.getRevision());
+      if (revisionId == null) {
         logger.error("could not find revision {}", request.getRevision());
         throw notFound(entity("Revision", request.getRevision()).in(this.repository));
       }
-      return revId;
+      return revisionId;
     }
   }
 
@@ -212,7 +214,9 @@ public class GitBrowseCommand extends AbstractGitCommand
 
   private FileObject getEntry() throws IOException {
     try (RevWalk revWalk = new RevWalk(repo); TreeWalk treeWalk = new TreeWalk(repo)) {
-      logger.debug("load repository browser for revision {}", revId.name());
+      if (logger.isDebugEnabled()) { // method call in logger call
+        logger.debug("load repository browser for revision {}", revId.name());
+      }
 
       if (!isRootRequest()) {
         treeWalk.setFilter(PathFilter.create(request.getPath()));
@@ -275,7 +279,7 @@ public class GitBrowseCommand extends AbstractGitCommand
   }
 
   private void createTree(TreeEntry parent, TreeWalk treeWalk) throws IOException {
-    Stack<TreeEntry> parents = new Stack<>();
+    Deque<TreeEntry> parents = new ArrayDeque<>();
     parents.push(parent);
     while (treeWalk.next()) {
       final String currentPath = treeWalk.getPathString();
@@ -283,11 +287,15 @@ public class GitBrowseCommand extends AbstractGitCommand
         parents.pop();
       }
       TreeEntry currentParent = parents.peek();
-      TreeEntry treeEntry = new TreeEntry(repo, treeWalk);
-      currentParent.addChild(treeEntry);
-      if (request.isRecursive() && treeEntry.getType() == TreeType.DIRECTORY) {
-        treeWalk.enterSubtree();
-        parents.push(treeEntry);
+      TreeEntry treeEntry = createTreeEntry(repo, treeWalk);
+      if (treeEntry != null) {
+        currentParent.addChild(treeEntry);
+        if (request.isRecursive() && treeEntry.getType() == TreeType.DIRECTORY) {
+          treeWalk.enterSubtree();
+          parents.push(treeEntry);
+        }
+      } else {
+        logger.warn("failed to find tree entry for {}", currentPath);
       }
     }
   }
@@ -304,7 +312,12 @@ public class GitBrowseCommand extends AbstractGitCommand
         currentDepth++;
 
         if (currentDepth >= limit) {
-          return createFileObject(new TreeEntry(repo, treeWalk));
+          TreeEntry treeEntry = createTreeEntry(repo, treeWalk);
+          if (treeEntry != null) {
+            return createFileObject(treeEntry);
+          } else {
+            logger.warn("could not find tree entry at {}", name);
+          }
         } else {
           treeWalk.enterSubtree();
         }
@@ -328,8 +341,12 @@ public class GitBrowseCommand extends AbstractGitCommand
     }
   }
 
-  private SubRepository getSubRepository(String path)
-    throws IOException {
+  @Nullable
+  private SubRepository getSubRepository(String path) throws IOException {
+    if (request.isDisableSubRepositoryDetection()) {
+      return null;
+    }
+
     Map<String, SubRepository> subRepositories = subrepositoryCache.get(revId);
 
     if (subRepositories == null) {
@@ -447,6 +464,23 @@ public class GitBrowseCommand extends AbstractGitCommand
     FILE, DIRECTORY, SUB_REPOSITORY
   }
 
+  @Nullable
+  TreeEntry createTreeEntry(org.eclipse.jgit.lib.Repository repo, TreeWalk treeWalk) throws IOException {
+    String pathString = treeWalk.getPathString();
+    ObjectId objectId = treeWalk.getObjectId(0);
+    SubRepository subRepository = getSubRepository(pathString);
+    if (subRepository != null) {
+      return new TreeEntry(pathString, treeWalk.getNameString(), objectId, subRepository);
+    } else if (repo.getObjectDatabase().has(objectId)) {
+      TreeType type = TreeType.FILE;
+      if (repo.open(objectId).getType() == Constants.OBJ_TREE) {
+        type = TreeType.DIRECTORY;
+      }
+      return new TreeEntry(pathString, treeWalk.getNameString(), objectId, type);
+    }
+    return null;
+  }
+
   private class TreeEntry {
 
     private final String pathString;
@@ -466,21 +500,20 @@ public class GitBrowseCommand extends AbstractGitCommand
       type = TreeType.DIRECTORY;
     }
 
-    TreeEntry(org.eclipse.jgit.lib.Repository repo, TreeWalk treeWalk) throws IOException {
-      this.pathString = treeWalk.getPathString();
-      this.nameString = treeWalk.getNameString();
-      this.objectId = treeWalk.getObjectId(0);
+    TreeEntry(String pathString, String nameString, ObjectId objectId, SubRepository subRepository) {
+      this.pathString = pathString;
+      this.nameString = nameString;
+      this.objectId = objectId;
+      this.type = TreeType.SUB_REPOSITORY;
+      this.subRepository = subRepository;
+    }
 
-      if (!request.isDisableSubRepositoryDetection() && GitBrowseCommand.this.getSubRepository(pathString) != null) {
-        subRepository = GitBrowseCommand.this.getSubRepository(pathString);
-        type = TreeType.SUB_REPOSITORY;
-      } else if (repo.open(objectId).getType() == Constants.OBJ_TREE) {
-        subRepository = null;
-        type = TreeType.DIRECTORY;
-      } else {
-        subRepository = null;
-        type = TreeType.FILE;
-      }
+    TreeEntry(String pathString, String nameString, ObjectId objectId, TreeType type) {
+      this.pathString = pathString;
+      this.nameString = nameString;
+      this.objectId = objectId;
+      this.type = type;
+      this.subRepository = null;
     }
 
     String getPathString() {

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

@@ -0,0 +1,129 @@
+/*
+ * MIT License
+ *
+ * Copyright (c) 2020-present Cloudogu GmbH and Contributors
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in all
+ * copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+package sonia.scm.repository.spi;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnitRunner;
+import sonia.scm.repository.BrowserResult;
+import sonia.scm.repository.FileObject;
+import sonia.scm.web.lfs.LfsBlobStoreFactory;
+
+import javax.annotation.Nonnull;
+import java.io.IOException;
+import java.util.Collection;
+import java.util.List;
+import java.util.stream.Collectors;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static sonia.scm.repository.spi.SyncAsyncExecutors.synchronousExecutor;
+
+@RunWith(MockitoJUnitRunner.class)
+public class GitBrowseCommand_BrokenSubmoduleTest extends AbstractGitCommandTestBase {
+
+  @Mock
+  private LfsBlobStoreFactory lfsBlobStoreFactory;
+
+  private GitBrowseCommand command;
+
+  @Before
+  public void createCommand() {
+    command = new GitBrowseCommand(createContext(), lfsBlobStoreFactory, synchronousExecutor());
+  }
+
+  @Test
+  public void testBrowse() throws IOException {
+    BrowserResult result = command.getBrowserResult(new BrowseCommandRequest());
+    Collection<FileObject> children = result.getFile().getChildren();
+
+    List<String> subrepos = subRepositoriesOnly(children);
+    assertThat(subrepos).containsExactly(
+      "anonymous-access",
+      "hasselhoffme",
+      "recipes",
+      "scm-redmine-plugin"
+    );
+
+    List<String> directories = directoriesOnly(children);
+    assertThat(directories).containsExactly(
+      "dir",
+      "plugins"
+      );
+
+    List<String> files = filesOnly(children);
+    assertThat(files)
+      .containsExactly(
+        ".gitmodules",
+        "README.md",
+        "test.txt"
+      );
+  }
+
+  @Test
+  public void testBrowseRecursive() throws IOException {
+    BrowseCommandRequest request = new BrowseCommandRequest();
+    request.setRecursive(true);
+    BrowserResult result = command.getBrowserResult(request);
+    Collection<FileObject> children = result.getFile().getChildren();
+    FileObject fileObject = children.stream().filter(f -> "plugins".equals(f.getPath())).findFirst().get();
+    assertThat(fileObject.getChildren()).hasSize(3);
+    List<String> subrepos = subRepositoriesOnly(fileObject.getChildren());
+    assertThat(subrepos)
+      .containsExactly(
+        "plugins/scm-branchwp-plugin",
+        "plugins/scm-jira-plugin",
+        "plugins/statistic-plugin"
+      );
+  }
+
+  @Nonnull
+  private List<String> filesOnly(Collection<FileObject> children) {
+    return children.stream().filter(f -> !f.isDirectory()).map(FileObject::getPath).collect(Collectors.toList());
+  }
+
+  @Nonnull
+  private List<String> directoriesOnly(Collection<FileObject> children) {
+    return children.stream()
+      .filter(FileObject::isDirectory)
+      .filter(f -> f.getSubRepository() == null)
+      .map(FileObject::getPath)
+      .collect(Collectors.toList());
+  }
+
+  @Nonnull
+  private List<String> subRepositoriesOnly(Collection<FileObject> children) {
+    return children.stream()
+      .filter(f -> f.getSubRepository() != null)
+      .map(FileObject::getPath)
+      .collect(Collectors.toList());
+  }
+
+  @Override
+  protected String getZippedRepositoryResource() {
+    return "sonia/scm/repository/spi/scm-git-broken-submodule-repo.zip";
+  }
+}