Handle missing request scope for url computation

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<? extends ScmProviderHttpServlet> delegateProvider;
   private final Provider<? extends PermissionFilter> permissionFilterProvider;
   private final Provider<ScmPathInfoStore> uriInfoStore;
@@ -38,11 +46,24 @@ public abstract class InitializingHttpScmProtocolWrapper {
   }
 
   private String computeBasePath() {
-    if (uriInfoStore.get() != null && uriInfoStore.get().get() != null) {
+    return getPathFromScmPathInfoIfAvailable().orElse(getPathFromConfiguration());
-      return uriInfoStore.get().get().getApiRestUri().resolve("../..").toASCIIString();
+  }
-    } else {
+
-      return scmConfiguration.getBaseUrl();
+  private Optional<String> 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 {

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 <I> Provider<I> of(I instance) {
+  public static <I> Provider<I> of(I instance) {
     @SuppressWarnings("unchecked") // Can't make mockito return typed provider
     Provider<I> provider = mock(Provider.class);
     when(provider.get()).thenReturn(instance);

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<ScmPathInfoStore> 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<ScmPathInfo> mockSetPathInfo() {
+    return when(pathInfoStore.get()).thenReturn(new ScmPathInfo() {
+      @Override
+      public URI getApiRestUri() {
+        return URI.create("http://example.com/scm/api/rest/");
+      }
+    });
+  }
+
+}

scm-webapp/pom.xml

@@ -49,7 +49,14 @@
       <artifactId>scm-core</artifactId>
       <version>2.0.0-SNAPSHOT</version>
     </dependency>
-    
+    <dependency>
+      <groupId>sonia.scm</groupId>
+      <artifactId>scm-core</artifactId>
+      <version>2.0.0-SNAPSHOT</version>
+      <classifier>tests</classifier>
+      <scope>test</scope>
+    </dependency>
+
     <dependency>
       <groupId>sonia.scm</groupId>
       <artifactId>scm-dao-xml</artifactId>

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;

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;

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;

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;

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;

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.equalToIgnoringCase;
-import static org.hamcrest.Matchers.*;
+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)

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;

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;

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;