From ef28350ce31279d9442e3e605b6b85d3d35c7ecb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 12 Sep 2018 11:43:29 +0200 Subject: [PATCH] Handle missing request scope for url computation --- .../InitializingHttpScmProtocolWrapper.java | 29 +++- .../test/java/sonia/scm}/MockProvider.java | 6 +- ...nitializingHttpScmProtocolWrapperTest.java | 143 ++++++++++++++++++ scm-webapp/pom.xml | 9 +- .../v2/resources/BranchRootResourceTest.java | 1 + .../resources/ChangesetRootResourceTest.java | 1 + .../v2/resources/GroupRootResourceTest.java | 1 + .../resources/PermissionRootResourceTest.java | 1 + .../resources/RepositoryRootResourceTest.java | 1 + .../RepositoryTypeRootResourceTest.java | 7 +- .../v2/resources/SourceRootResourceTest.java | 2 +- .../api/v2/resources/TagRootResourceTest.java | 1 + .../v2/resources/UserRootResourceTest.java | 1 + 13 files changed, 192 insertions(+), 11 deletions(-) rename {scm-webapp/src/test/java/sonia/scm/api/v2/resources => scm-core/src/test/java/sonia/scm}/MockProvider.java (80%) create mode 100644 scm-core/src/test/java/sonia/scm/repository/spi/InitializingHttpScmProtocolWrapperTest.java diff --git a/scm-core/src/main/java/sonia/scm/repository/spi/InitializingHttpScmProtocolWrapper.java b/scm-core/src/main/java/sonia/scm/repository/spi/InitializingHttpScmProtocolWrapper.java index 450f1d4495..7e7fb243cc 100644 --- a/scm-core/src/main/java/sonia/scm/repository/spi/InitializingHttpScmProtocolWrapper.java +++ b/scm-core/src/main/java/sonia/scm/repository/spi/InitializingHttpScmProtocolWrapper.java @@ -1,5 +1,7 @@ package sonia.scm.repository.spi; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import sonia.scm.api.v2.resources.ScmPathInfoStore; import sonia.scm.config.ScmConfiguration; import sonia.scm.repository.Repository; @@ -11,9 +13,15 @@ import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import java.io.IOException; +import java.util.Optional; + +import static java.util.Optional.empty; +import static java.util.Optional.of; public abstract class InitializingHttpScmProtocolWrapper { + private static final Logger logger = LoggerFactory.getLogger(InitializingHttpScmProtocolWrapper.class); + private final Provider delegateProvider; private final Provider permissionFilterProvider; private final Provider uriInfoStore; @@ -38,11 +46,24 @@ public abstract class InitializingHttpScmProtocolWrapper { } private String computeBasePath() { - if (uriInfoStore.get() != null && uriInfoStore.get().get() != null) { - return uriInfoStore.get().get().getApiRestUri().resolve("../..").toASCIIString(); - } else { - return scmConfiguration.getBaseUrl(); + return getPathFromScmPathInfoIfAvailable().orElse(getPathFromConfiguration()); + } + + private Optional getPathFromScmPathInfoIfAvailable() { + try { + ScmPathInfoStore scmPathInfoStore = uriInfoStore.get(); + if (scmPathInfoStore != null && scmPathInfoStore.get() != null) { + return of(scmPathInfoStore.get().getApiRestUri().resolve("../..").toASCIIString()); + } + } catch (Exception e) { + logger.debug("could not get ScmPathInfoStore from context", e); } + return empty(); + } + + private String getPathFromConfiguration() { + logger.debug("using base path from configuration: " + scmConfiguration.getBaseUrl()); + return scmConfiguration.getBaseUrl(); } private class ProtocolWrapper extends HttpScmProtocol { diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/MockProvider.java b/scm-core/src/test/java/sonia/scm/MockProvider.java similarity index 80% rename from scm-webapp/src/test/java/sonia/scm/api/v2/resources/MockProvider.java rename to scm-core/src/test/java/sonia/scm/MockProvider.java index bf84e4fe15..fae1b07586 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/MockProvider.java +++ b/scm-core/src/test/java/sonia/scm/MockProvider.java @@ -1,4 +1,4 @@ -package sonia.scm.api.v2.resources; +package sonia.scm; import javax.inject.Provider; @@ -8,11 +8,11 @@ import static org.mockito.Mockito.when; /** * A mockito implementation of CDI {@link javax.inject.Provider}. */ -class MockProvider { +public class MockProvider { private MockProvider() {} - static Provider of(I instance) { + public static Provider of(I instance) { @SuppressWarnings("unchecked") // Can't make mockito return typed provider Provider provider = mock(Provider.class); when(provider.get()).thenReturn(instance); diff --git a/scm-core/src/test/java/sonia/scm/repository/spi/InitializingHttpScmProtocolWrapperTest.java b/scm-core/src/test/java/sonia/scm/repository/spi/InitializingHttpScmProtocolWrapperTest.java new file mode 100644 index 0000000000..3b43e16e5b --- /dev/null +++ b/scm-core/src/test/java/sonia/scm/repository/spi/InitializingHttpScmProtocolWrapperTest.java @@ -0,0 +1,143 @@ +package sonia.scm.repository.spi; + +import com.google.inject.ProvisionException; +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mock; +import org.mockito.stubbing.Answer; +import org.mockito.stubbing.OngoingStubbing; +import sonia.scm.MockProvider; +import sonia.scm.api.v2.resources.ScmPathInfo; +import sonia.scm.api.v2.resources.ScmPathInfoStore; +import sonia.scm.config.ScmConfiguration; +import sonia.scm.repository.Repository; +import sonia.scm.web.filter.PermissionFilter; + +import javax.inject.Provider; +import javax.servlet.ServletConfig; +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.io.IOException; +import java.net.URI; + +import static org.junit.Assert.assertEquals; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.same; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import static org.mockito.MockitoAnnotations.initMocks; + +public class InitializingHttpScmProtocolWrapperTest { + + private static final Repository REPOSITORY = new Repository("", "git", "space", "name"); + + @Mock + private ScmProviderHttpServlet delegateServlet; + @Mock + private PermissionFilter permissionFilter; + @Mock + private ScmPathInfoStore pathInfoStore; + @Mock + private ScmConfiguration scmConfiguration; + private Provider pathInfoStoreProvider; + + @Mock + private HttpServletRequest request; + @Mock + private HttpServletResponse response; + @Mock + private ServletConfig servletConfig; + + private InitializingHttpScmProtocolWrapper wrapper; + + @Before + public void init() { + initMocks(this); + pathInfoStoreProvider = MockProvider.of(pathInfoStore); + wrapper = new InitializingHttpScmProtocolWrapper(MockProvider.of(this.delegateServlet), MockProvider.of(permissionFilter), pathInfoStoreProvider, scmConfiguration) {}; + when(scmConfiguration.getBaseUrl()).thenReturn("http://example.com/scm"); + } + + @Test + public void shouldUsePathFromPathInfo() { + mockSetPathInfo(); + + HttpScmProtocol httpScmProtocol = wrapper.get(REPOSITORY); + + assertEquals("http://example.com/scm/repo/space/name", httpScmProtocol.getUrl()); + } + + @Test + public void shouldUseConfigurationWhenPathInfoNotSet() { + HttpScmProtocol httpScmProtocol = wrapper.get(REPOSITORY); + + assertEquals("http://example.com/scm/repo/space/name", httpScmProtocol.getUrl()); + } + + @Test + public void shouldUseConfigurationWhenNotInRequestScope() { + when(pathInfoStoreProvider.get()).thenThrow(new ProvisionException("test")); + + HttpScmProtocol httpScmProtocol = wrapper.get(REPOSITORY); + + assertEquals("http://example.com/scm/repo/space/name", httpScmProtocol.getUrl()); + } + + @Test + public void shouldInitializeAndDelegateRequestThroughFilter() throws ServletException, IOException { + doAnswer(proceedInvocation()). + when(permissionFilter).executeIfPermitted(same(request), same(response), same(REPOSITORY), any()); + HttpScmProtocol httpScmProtocol = wrapper.get(REPOSITORY); + + httpScmProtocol.serve(request, response, servletConfig); + + verify(delegateServlet).init(servletConfig); + verify(delegateServlet).service(request, response, REPOSITORY); + } + + @Test + public void shouldNotDelegateRequestWhenFilterBlocks() throws ServletException, IOException { + doNothing(). + when(permissionFilter).executeIfPermitted(same(request), same(response), same(REPOSITORY), any()); + HttpScmProtocol httpScmProtocol = wrapper.get(REPOSITORY); + + httpScmProtocol.serve(request, response, servletConfig); + + verify(delegateServlet, never()).service(request, response, REPOSITORY); + } + + @Test + public void shouldInitializeOnlyOnce() throws ServletException, IOException { + doAnswer(proceedInvocation()). + when(permissionFilter).executeIfPermitted(same(request), same(response), same(REPOSITORY), any()); + HttpScmProtocol httpScmProtocol = wrapper.get(REPOSITORY); + + httpScmProtocol.serve(request, response, servletConfig); + httpScmProtocol.serve(request, response, servletConfig); + + verify(delegateServlet, times(1)).init(servletConfig); + verify(delegateServlet, times(2)).service(request, response, REPOSITORY); + } + + private Answer proceedInvocation() { + return invocation -> { + ((PermissionFilter.ContinuationServlet) invocation.getArgument(3)).doService(); + return null; + }; + } + + private OngoingStubbing mockSetPathInfo() { + return when(pathInfoStore.get()).thenReturn(new ScmPathInfo() { + @Override + public URI getApiRestUri() { + return URI.create("http://example.com/scm/api/rest/"); + } + }); + } + +} diff --git a/scm-webapp/pom.xml b/scm-webapp/pom.xml index fb5cc57e44..c7d4dbc45c 100644 --- a/scm-webapp/pom.xml +++ b/scm-webapp/pom.xml @@ -49,7 +49,14 @@ scm-core 2.0.0-SNAPSHOT - + + sonia.scm + scm-core + 2.0.0-SNAPSHOT + tests + test + + sonia.scm scm-dao-xml diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/BranchRootResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/BranchRootResourceTest.java index e23d3dc39b..f2563b3522 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/BranchRootResourceTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/BranchRootResourceTest.java @@ -17,6 +17,7 @@ import org.junit.runner.RunWith; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; +import sonia.scm.MockProvider; import sonia.scm.repository.Branch; import sonia.scm.repository.Branches; import sonia.scm.repository.Changeset; diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ChangesetRootResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ChangesetRootResourceTest.java index 40aa61852a..45ff631828 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ChangesetRootResourceTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ChangesetRootResourceTest.java @@ -18,6 +18,7 @@ import org.junit.runner.RunWith; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; +import sonia.scm.MockProvider; import sonia.scm.api.rest.AuthorizationExceptionMapper; import sonia.scm.repository.Changeset; import sonia.scm.repository.ChangesetPagingResult; diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/GroupRootResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/GroupRootResourceTest.java index 1e42016ace..9b1699691c 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/GroupRootResourceTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/GroupRootResourceTest.java @@ -12,6 +12,7 @@ import org.junit.Test; import org.mockito.ArgumentCaptor; import org.mockito.InjectMocks; import org.mockito.Mock; +import sonia.scm.MockProvider; import sonia.scm.PageResult; import sonia.scm.api.rest.JSONContextResolver; import sonia.scm.api.rest.ObjectMapperProvider; diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/PermissionRootResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/PermissionRootResourceTest.java index 8d05c1f455..f4c88b7be1 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/PermissionRootResourceTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/PermissionRootResourceTest.java @@ -27,6 +27,7 @@ import org.junit.jupiter.api.DynamicTest; import org.junit.jupiter.api.TestFactory; import org.mockito.InjectMocks; import org.mockito.Mock; +import sonia.scm.MockProvider; import sonia.scm.repository.NamespaceAndName; import sonia.scm.repository.Permission; import sonia.scm.repository.PermissionType; diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/RepositoryRootResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/RepositoryRootResourceTest.java index ca679764ad..68ea56299b 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/RepositoryRootResourceTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/RepositoryRootResourceTest.java @@ -12,6 +12,7 @@ import org.junit.Test; import org.mockito.ArgumentCaptor; import org.mockito.InjectMocks; import org.mockito.Mock; +import sonia.scm.MockProvider; import sonia.scm.PageResult; import sonia.scm.repository.NamespaceAndName; import sonia.scm.repository.Permission; diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/RepositoryTypeRootResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/RepositoryTypeRootResourceTest.java index 9adca13225..a0bb5ccf08 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/RepositoryTypeRootResourceTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/RepositoryTypeRootResourceTest.java @@ -12,6 +12,7 @@ import org.junit.runner.RunWith; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; +import sonia.scm.MockProvider; import sonia.scm.repository.RepositoryManager; import sonia.scm.repository.RepositoryType; import sonia.scm.web.VndMediaType; @@ -22,8 +23,10 @@ import java.util.List; import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND; import static javax.servlet.http.HttpServletResponse.SC_OK; -import static org.junit.Assert.*; -import static org.hamcrest.Matchers.*; +import static org.hamcrest.Matchers.equalToIgnoringCase; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.when; @RunWith(MockitoJUnitRunner.Silent.class) diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/SourceRootResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/SourceRootResourceTest.java index a59ed0c103..4bc11fad82 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/SourceRootResourceTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/SourceRootResourceTest.java @@ -1,7 +1,6 @@ package sonia.scm.api.v2.resources; import org.jboss.resteasy.core.Dispatcher; -import org.jboss.resteasy.mock.MockDispatcherFactory; import org.jboss.resteasy.mock.MockHttpRequest; import org.jboss.resteasy.mock.MockHttpResponse; import org.junit.Before; @@ -10,6 +9,7 @@ import org.junit.runner.RunWith; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; +import sonia.scm.MockProvider; import sonia.scm.repository.BrowserResult; import sonia.scm.repository.FileObject; import sonia.scm.repository.NamespaceAndName; diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/TagRootResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/TagRootResourceTest.java index 92d11b3895..eaa7deb578 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/TagRootResourceTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/TagRootResourceTest.java @@ -17,6 +17,7 @@ import org.junit.runner.RunWith; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; +import sonia.scm.MockProvider; import sonia.scm.api.rest.AuthorizationExceptionMapper; import sonia.scm.repository.NamespaceAndName; import sonia.scm.repository.Repository; diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/UserRootResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/UserRootResourceTest.java index 5004eb7665..e54c875373 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/UserRootResourceTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/UserRootResourceTest.java @@ -13,6 +13,7 @@ import org.junit.Test; import org.mockito.ArgumentCaptor; import org.mockito.InjectMocks; import org.mockito.Mock; +import sonia.scm.MockProvider; import sonia.scm.PageResult; import sonia.scm.user.User; import sonia.scm.user.UserManager;