diff --git a/scm-webapp/src/main/java/sonia/scm/web/protocol/HttpProtocolServlet.java b/scm-webapp/src/main/java/sonia/scm/web/protocol/HttpProtocolServlet.java index 250846a8cc..623be728c1 100644 --- a/scm-webapp/src/main/java/sonia/scm/web/protocol/HttpProtocolServlet.java +++ b/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 Provider requestProvider; - + private final NamespaceAndNameFromPathExtractor pathExtractor; private final PushStateDispatcher dispatcher; private final UserAgentParser userAgentParser; @Inject - public HttpProtocolServlet(RepositoryServiceFactory serviceFactory, Provider 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 = NamespaceAndNameFromPathExtractor.fromUri(pathInfo); + Optional 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) { diff --git a/scm-webapp/src/main/java/sonia/scm/web/protocol/NamespaceAndNameFromPathExtractor.java b/scm-webapp/src/main/java/sonia/scm/web/protocol/NamespaceAndNameFromPathExtractor.java index 22e2433561..2c1eeb1b90 100644 --- a/scm-webapp/src/main/java/sonia/scm/web/protocol/NamespaceAndNameFromPathExtractor.java +++ b/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 types; - static Optional fromUri(String uri) { + @Inject + public NamespaceAndNameFromPathExtractor(RepositoryManager repositoryManager) { + this.types = repositoryManager.getConfiguredTypes() + .stream() + .map(Type::getName) + .collect(Collectors.toSet()); + } + + Optional 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.indexOf('.'); + int nameDotIndex = name.lastIndexOf('.'); if (nameDotIndex >= 0) { - return of(new NamespaceAndName(namespace, name.substring(0, nameDotIndex))); - } else { - return of(new NamespaceAndName(namespace, name)); + String suffix = name.substring(nameDotIndex + 1); + if (types.contains(suffix)) { + name = name.substring(0, nameDotIndex); + } } + return of(new NamespaceAndName(namespace, name)); } } diff --git a/scm-webapp/src/test/java/sonia/scm/web/protocol/HttpProtocolServletTest.java b/scm-webapp/src/test/java/sonia/scm/web/protocol/HttpProtocolServletTest.java index 1bd6358c95..59b29c8eda 100644 --- a/scm-webapp/src/test/java/sonia/scm/web/protocol/HttpProtocolServletTest.java +++ b/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.Test; +import org.junit.jupiter.api.BeforeEach; +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 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 - public void init() { - initMocks(this); - when(userAgentParser.parse(request)).thenReturn(userAgent); - when(userAgent.isBrowser()).thenReturn(false); - NamespaceAndName existingRepo = new NamespaceAndName("space", "repo"); - when(serviceFactory.create(not(eq(existingRepo)))).thenThrow(new NotFoundException("Test", "a")); - when(serviceFactory.create(existingRepo)).thenReturn(repositoryService); - when(requestProvider.get()).thenReturn(httpServletRequest); + @Nested + class Browser { + + @BeforeEach + void prepareMocks() { + when(userAgentParser.parse(request)).thenReturn(userAgent); + when(userAgent.isBrowser()).thenReturn(true); + when(request.getRequestURI()).thenReturn("uri"); + } + + @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 - public void shouldDispatchBrowserRequests() throws ServletException, IOException { - when(userAgent.isBrowser()).thenReturn(true); - when(request.getRequestURI()).thenReturn("uri"); + @Nested + class ScmClient { - 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 - public void shouldHandleBadPaths() throws IOException, ServletException { - when(request.getPathInfo()).thenReturn("/illegal"); + servlet.service(request, response); - 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 - public void shouldHandleNotExistingRepository() throws IOException, ServletException { - when(request.getPathInfo()).thenReturn("/not/exists"); + NamespaceAndName repo = new NamespaceAndName("not", "exists"); + when(extractor.fromUri("/not/exists")).thenReturn(Optional.of(repo)); + 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 - public void shouldDelegateToProvider() throws IOException, ServletException { - when(request.getPathInfo()).thenReturn("/space/name"); - NamespaceAndName namespaceAndName = new NamespaceAndName("space", "name"); - doReturn(repositoryService).when(serviceFactory).create(namespaceAndName); - Repository repository = new Repository(); - when(repositoryService.getRepository()).thenReturn(repository); - when(repositoryService.getProtocol(HttpScmProtocol.class)).thenReturn(protocol); + @Test + void shouldDelegateToProvider() throws IOException, ServletException { + NamespaceAndName repo = new NamespaceAndName("space", "name"); + when(extractor.fromUri("/space/name")).thenReturn(Optional.of(repo)); + when(serviceFactory.create(repo)).thenReturn(repositoryService); - 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(); } } diff --git a/scm-webapp/src/test/java/sonia/scm/web/protocol/NamespaceAndNameFromPathExtractorTest.java b/scm-webapp/src/test/java/sonia/scm/web/protocol/NamespaceAndNameFromPathExtractorTest.java index 0998010069..5ac8f37c01 100644 --- a/scm-webapp/src/test/java/sonia/scm/web/protocol/NamespaceAndNameFromPathExtractorTest.java +++ b/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 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 shouldExtractCorrectNamespaceAndName() { return Stream.of( @@ -26,21 +57,26 @@ public class NamespaceAndNameFromPathExtractorTest { } @TestFactory - Stream shouldHandleTrailingDotSomethings() { + Stream shouldHandleTypeSuffix() { return Stream.of( "/space/repo.git", - "/space/repo.and.more", - "/space/repo." + "/space/repo.hg", + "/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 = NamespaceAndNameFromPathExtractor.fromUri(path); + Optional 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 = NamespaceAndNameFromPathExtractor.fromUri(path); + Optional namespaceAndName = extractor.fromUri(path); assertThat(namespaceAndName.isPresent()).isFalse(); } ); } + + @TestFactory + Stream 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 = extractor.fromUri("/space/repo."); + assertThat(namespaceAndName).contains(new NamespaceAndName("space", "repo.")); + } }