Merge with upstream

scm-webapp/src/main/java/sonia/scm/web/protocol/HttpProtocolServlet.java

@@ -16,7 +16,6 @@ import sonia.scm.repository.spi.HttpScmProtocol;
 import sonia.scm.web.UserAgent;
 import sonia.scm.web.UserAgentParser;
 
-import javax.inject.Provider;
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServlet;
 import javax.servlet.http.HttpServletRequest;
@@ -33,17 +32,15 @@ public class HttpProtocolServlet extends HttpServlet {
   public static final String PATTERN = PATH + "/*";
 
   private final RepositoryServiceFactory serviceFactory;
-
+  private final NamespaceAndNameFromPathExtractor pathExtractor;
-  private final Provider<HttpServletRequest> requestProvider;
-
   private final PushStateDispatcher dispatcher;
   private final UserAgentParser userAgentParser;
 
 
   @Inject
-  public HttpProtocolServlet(RepositoryServiceFactory serviceFactory, Provider<HttpServletRequest> requestProvider, PushStateDispatcher dispatcher, UserAgentParser userAgentParser) {
+  public HttpProtocolServlet(RepositoryServiceFactory serviceFactory, NamespaceAndNameFromPathExtractor pathExtractor, PushStateDispatcher dispatcher, UserAgentParser userAgentParser) {
     this.serviceFactory = serviceFactory;
-    this.requestProvider = requestProvider;
+    this.pathExtractor = pathExtractor;
     this.dispatcher = dispatcher;
     this.userAgentParser = userAgentParser;
   }
@@ -55,9 +52,8 @@ public class HttpProtocolServlet extends HttpServlet {
       log.trace("dispatch browser request for user agent {}", userAgent);
       dispatcher.dispatch(request, response, request.getRequestURI());
     } else {
-
       String pathInfo = request.getPathInfo();
-      Optional<NamespaceAndName> namespaceAndName = NamespaceAndNameFromPathExtractor.fromUri(pathInfo);
+      Optional<NamespaceAndName> namespaceAndName = pathExtractor.fromUri(pathInfo);
       if (namespaceAndName.isPresent()) {
         service(request, response, namespaceAndName.get());
       } else {
@@ -69,7 +65,7 @@ public class HttpProtocolServlet extends HttpServlet {
 
   private void service(HttpServletRequest req, HttpServletResponse resp, NamespaceAndName namespaceAndName) throws IOException, ServletException {
     try (RepositoryService repositoryService = serviceFactory.create(namespaceAndName)) {
-      requestProvider.get().setAttribute(DefaultRepositoryProvider.ATTRIBUTE_NAME, repositoryService.getRepository());
+      req.setAttribute(DefaultRepositoryProvider.ATTRIBUTE_NAME, repositoryService.getRepository());
       HttpScmProtocol protocol = repositoryService.getProtocol(HttpScmProtocol.class);
       protocol.serve(req, resp, getServletConfig());
     } catch (NotFoundException e) {

scm-webapp/src/main/java/sonia/scm/web/protocol/NamespaceAndNameFromPathExtractor.java

@@ -1,18 +1,31 @@
 package sonia.scm.web.protocol;
 
+import sonia.scm.Type;
 import sonia.scm.repository.NamespaceAndName;
+import sonia.scm.repository.RepositoryManager;
 import sonia.scm.util.HttpUtil;
 
+import javax.inject.Inject;
 import java.util.Optional;
+import java.util.Set;
+import java.util.stream.Collectors;
 
 import static java.util.Optional.empty;
 import static java.util.Optional.of;
 
 final class NamespaceAndNameFromPathExtractor {
 
-  private NamespaceAndNameFromPathExtractor() {}
+  private final Set<String> types;
 
-  static Optional<NamespaceAndName> fromUri(String uri) {
+  @Inject
+  public NamespaceAndNameFromPathExtractor(RepositoryManager repositoryManager) {
+    this.types = repositoryManager.getConfiguredTypes()
+      .stream()
+      .map(Type::getName)
+      .collect(Collectors.toSet());
+  }
+
+  Optional<NamespaceAndName> fromUri(String uri) {
     if (uri.startsWith(HttpUtil.SEPARATOR_PATH)) {
       uri = uri.substring(1);
     }
@@ -30,12 +43,13 @@ final class NamespaceAndNameFromPathExtractor {
     }
 
     String name = uri.substring(endOfNamespace + 1, nameIndex);
-
+    int nameDotIndex = name.lastIndexOf('.');
-    int nameDotIndex = name.indexOf('.');
     if (nameDotIndex >= 0) {
-      return of(new NamespaceAndName(namespace, name.substring(0, nameDotIndex)));
+      String suffix = name.substring(nameDotIndex + 1);
-    } else {
+      if (types.contains(suffix)) {
-      return of(new NamespaceAndName(namespace, name));
+        name = name.substring(0, nameDotIndex);
+      }
     }
+    return of(new NamespaceAndName(namespace, name));
   }
 }

scm-webapp/src/test/java/sonia/scm/web/protocol/HttpProtocolServletTest.java

@@ -1,114 +1,136 @@
 package sonia.scm.web.protocol;
 
-import org.junit.Before;
+import org.junit.jupiter.api.BeforeEach;
-import org.junit.Test;
+import org.junit.jupiter.api.Nested;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
 import org.mockito.InjectMocks;
 import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
 import sonia.scm.NotFoundException;
 import sonia.scm.PushStateDispatcher;
 import sonia.scm.repository.DefaultRepositoryProvider;
 import sonia.scm.repository.NamespaceAndName;
 import sonia.scm.repository.Repository;
+import sonia.scm.repository.RepositoryTestData;
 import sonia.scm.repository.api.RepositoryService;
 import sonia.scm.repository.api.RepositoryServiceFactory;
 import sonia.scm.repository.spi.HttpScmProtocol;
 import sonia.scm.web.UserAgent;
 import sonia.scm.web.UserAgentParser;
 
-import javax.inject.Provider;
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import java.io.IOException;
+import java.util.Optional;
 
-import static org.mockito.AdditionalMatchers.not;
-import static org.mockito.ArgumentMatchers.eq;
-import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
-import static org.mockito.MockitoAnnotations.initMocks;
-
-public class HttpProtocolServletTest {
 
+@ExtendWith(MockitoExtension.class)
+class HttpProtocolServletTest {
 
   @Mock
   private RepositoryServiceFactory serviceFactory;
+
   @Mock
-  private HttpServletRequest httpServletRequest;
+  private NamespaceAndNameFromPathExtractor extractor;
+
   @Mock
   private PushStateDispatcher dispatcher;
+
   @Mock
   private UserAgentParser userAgentParser;
-  @Mock
-  private Provider<HttpServletRequest> requestProvider;
 
   @InjectMocks
   private HttpProtocolServlet servlet;
 
   @Mock
   private RepositoryService repositoryService;
+
   @Mock
   private UserAgent userAgent;
 
   @Mock
   private HttpServletRequest request;
+
   @Mock
   private HttpServletResponse response;
+
   @Mock
   private HttpScmProtocol protocol;
 
-  @Before
+  @Nested
-  public void init() {
+  class Browser {
-    initMocks(this);
+
-    when(userAgentParser.parse(request)).thenReturn(userAgent);
+    @BeforeEach
-    when(userAgent.isBrowser()).thenReturn(false);
+    void prepareMocks() {
-    NamespaceAndName existingRepo = new NamespaceAndName("space", "repo");
+      when(userAgentParser.parse(request)).thenReturn(userAgent);
-    when(serviceFactory.create(not(eq(existingRepo)))).thenThrow(new NotFoundException("Test", "a"));
+      when(userAgent.isBrowser()).thenReturn(true);
-    when(serviceFactory.create(existingRepo)).thenReturn(repositoryService);
+      when(request.getRequestURI()).thenReturn("uri");
-    when(requestProvider.get()).thenReturn(httpServletRequest);
+    }
+
+    @Test
+    void shouldDispatchBrowserRequests() throws ServletException, IOException {
+      when(userAgent.isBrowser()).thenReturn(true);
+      when(request.getRequestURI()).thenReturn("uri");
+
+      servlet.service(request, response);
+
+      verify(dispatcher).dispatch(request, response, "uri");
+    }
+
   }
 
-  @Test
+  @Nested
-  public void shouldDispatchBrowserRequests() throws ServletException, IOException {
+  class ScmClient {
-    when(userAgent.isBrowser()).thenReturn(true);
-    when(request.getRequestURI()).thenReturn("uri");
 
-    servlet.service(request, response);
+    @BeforeEach
+    void prepareMocks() {
+      when(userAgentParser.parse(request)).thenReturn(userAgent);
+      when(userAgent.isBrowser()).thenReturn(false);
+    }
 
-    verify(dispatcher).dispatch(request, response, "uri");
+    @Test
-  }
+    void shouldHandleBadPaths() throws IOException, ServletException {
+      when(request.getPathInfo()).thenReturn("/illegal");
 
-  @Test
+      servlet.service(request, response);
-  public void shouldHandleBadPaths() throws IOException, ServletException {
-    when(request.getPathInfo()).thenReturn("/illegal");
 
-    servlet.service(request, response);
+      verify(response).setStatus(400);
+    }
 
-    verify(response).setStatus(400);
+    @Test
-  }
+    void shouldHandleNotExistingRepository() throws IOException, ServletException {
+      when(request.getPathInfo()).thenReturn("/not/exists");
 
-  @Test
+      NamespaceAndName repo = new NamespaceAndName("not", "exists");
-  public void shouldHandleNotExistingRepository() throws IOException, ServletException {
+      when(extractor.fromUri("/not/exists")).thenReturn(Optional.of(repo));
-    when(request.getPathInfo()).thenReturn("/not/exists");
+      when(serviceFactory.create(repo)).thenThrow(new NotFoundException("Test", "a"));
 
-    servlet.service(request, response);
+      servlet.service(request, response);
 
-    verify(response).setStatus(404);
+      verify(response).setStatus(404);
-  }
+    }
 
-  @Test
+    @Test
-  public void shouldDelegateToProvider() throws IOException, ServletException {
+    void shouldDelegateToProvider() throws IOException, ServletException {
-    when(request.getPathInfo()).thenReturn("/space/name");
+      NamespaceAndName repo = new NamespaceAndName("space", "name");
-    NamespaceAndName namespaceAndName = new NamespaceAndName("space", "name");
+      when(extractor.fromUri("/space/name")).thenReturn(Optional.of(repo));
-    doReturn(repositoryService).when(serviceFactory).create(namespaceAndName);
+      when(serviceFactory.create(repo)).thenReturn(repositoryService);
-    Repository repository = new Repository();
-    when(repositoryService.getRepository()).thenReturn(repository);
-    when(repositoryService.getProtocol(HttpScmProtocol.class)).thenReturn(protocol);
 
-    servlet.service(request, response);
+      when(request.getPathInfo()).thenReturn("/space/name");
+      Repository repository = RepositoryTestData.createHeartOfGold();
+      when(repositoryService.getRepository()).thenReturn(repository);
+      when(repositoryService.getProtocol(HttpScmProtocol.class)).thenReturn(protocol);
+
+      servlet.service(request, response);
+
+      verify(request).setAttribute(DefaultRepositoryProvider.ATTRIBUTE_NAME, repository);
+      verify(protocol).serve(request, response, null);
+      verify(repositoryService).close();
+    }
 
-    verify(httpServletRequest).setAttribute(DefaultRepositoryProvider.ATTRIBUTE_NAME, repository);
-    verify(protocol).serve(request, response, null);
-    verify(repositoryService).close();
   }
 }

scm-webapp/src/test/java/sonia/scm/web/protocol/NamespaceAndNameFromPathExtractorTest.java

@@ -1,17 +1,48 @@
 package sonia.scm.web.protocol;
 
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.DynamicNode;
 import org.junit.jupiter.api.DynamicTest;
+import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.TestFactory;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
 import sonia.scm.repository.NamespaceAndName;
+import sonia.scm.repository.RepositoryManager;
+import sonia.scm.repository.RepositoryType;
 
+import javax.inject.Inject;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
 import java.util.Optional;
 import java.util.stream.Stream;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.DynamicTest.dynamicTest;
+import static org.mockito.Mockito.when;
+
+@ExtendWith(MockitoExtension.class)
+class NamespaceAndNameFromPathExtractorTest {
+
+  @Mock
+  private RepositoryManager repositoryManager;
+
+  private NamespaceAndNameFromPathExtractor extractor;
+
+  @BeforeEach
+  void setUpObjectUnderTest() {
+    List<RepositoryType> types = Arrays.asList(
+      new RepositoryType("git", "Git", Collections.emptySet()),
+      new RepositoryType("hg", "Mercurial", Collections.emptySet()),
+      new RepositoryType("svn", "Subversion", Collections.emptySet())
+    );
+    when(repositoryManager.getConfiguredTypes()).thenReturn(types);
+    extractor = new NamespaceAndNameFromPathExtractor(repositoryManager);
+  }
 
-public class NamespaceAndNameFromPathExtractorTest {
   @TestFactory
   Stream<DynamicNode> shouldExtractCorrectNamespaceAndName() {
     return Stream.of(
@@ -26,21 +57,26 @@ public class NamespaceAndNameFromPathExtractorTest {
   }
 
   @TestFactory
-  Stream<DynamicNode> shouldHandleTrailingDotSomethings() {
+  Stream<DynamicNode> shouldHandleTypeSuffix() {
     return Stream.of(
       "/space/repo.git",
-      "/space/repo.and.more",
+      "/space/repo.hg",
-      "/space/repo."
+      "/space/repo.svn",
+      "/space/repo"
     ).map(this::createCorrectTest);
   }
 
   private DynamicTest createCorrectTest(String path) {
+    return createCorrectTest(path, new NamespaceAndName("space", "repo"));
+  }
+
+  private DynamicTest createCorrectTest(String path, NamespaceAndName expected) {
     return dynamicTest(
       "should extract correct namespace and name for path " + path,
       () -> {
-        Optional<NamespaceAndName> namespaceAndName = NamespaceAndNameFromPathExtractor.fromUri(path);
+        Optional<NamespaceAndName> namespaceAndName = extractor.fromUri(path);
 
-        assertThat(namespaceAndName.get()).isEqualTo(new NamespaceAndName("space", "repo"));
+        assertThat(namespaceAndName.get()).isEqualTo(expected);
       }
     );
   }
@@ -59,10 +95,26 @@ public class NamespaceAndNameFromPathExtractorTest {
     return dynamicTest(
       "should not fail for wrong path " + path,
       () -> {
-        Optional<NamespaceAndName> namespaceAndName = NamespaceAndNameFromPathExtractor.fromUri(path);
+        Optional<NamespaceAndName> namespaceAndName = extractor.fromUri(path);
 
         assertThat(namespaceAndName.isPresent()).isFalse();
       }
     );
   }
+
+  @TestFactory
+  Stream<DynamicNode> shouldHandleDots() {
+    return Stream.of(
+      "/space/repo.with.dots.git",
+      "/space/repo.with.dots.hg",
+      "/space/repo.with.dots.svn",
+      "/space/repo.with.dots"
+    ).map(path -> createCorrectTest(path, new NamespaceAndName("space", "repo.with.dots")));
+  }
+
+  @Test
+  void shouldNotFailOnEndingDot() {
+    Optional<NamespaceAndName> namespaceAndName = extractor.fromUri("/space/repo.");
+    assertThat(namespaceAndName).contains(new NamespaceAndName("space", "repo."));
+  }
 }