adds new constructor to InitializingHttpScmProtocolWrapper which uses the new RootURL api

2026-05-05 21:36:04 +02:00 · 2020-08-03 13:57:07 +02:00
parent 5bf4917914
commit b981d62905
2 changed files with 161 additions and 90 deletions
--- a/scm-core/src/main/java/sonia/scm/repository/spi/InitializingHttpScmProtocolWrapper.java
+++ b/scm-core/src/main/java/sonia/scm/repository/spi/InitializingHttpScmProtocolWrapper.java
@@ -21,10 +21,11 @@
 * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 * SOFTWARE.
 */
-    
+
 package sonia.scm.repository.spi;

 import lombok.extern.slf4j.Slf4j;
+import sonia.scm.RootURL;
 import sonia.scm.api.v2.resources.ScmPathInfoStore;
 import sonia.scm.config.ScmConfiguration;
 import sonia.scm.repository.Repository;
@@ -37,6 +38,7 @@ import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import java.io.IOException;
 import java.util.Optional;
+import java.util.function.Supplier;

 import static java.util.Optional.empty;
 import static java.util.Optional.of;
@@ -45,16 +47,36 @@ import static java.util.Optional.of;
 public abstract class InitializingHttpScmProtocolWrapper implements ScmProtocolProvider<HttpScmProtocol> {

  private final Provider<? extends ScmProviderHttpServlet> delegateProvider;
-  private final Provider<ScmPathInfoStore> pathInfoStore;
-  private final ScmConfiguration scmConfiguration;
+  private final Supplier<String> basePathSupplier;

  private volatile boolean isInitialized = false;

-
+  /**
+   * Constructs a new {@link InitializingHttpScmProtocolWrapper}.
+   *
+   * @param delegateProvider injection provider for the servlet delegate
+   * @param pathInfoStore url info store
+   * @param scmConfiguration scm-manager main configuration
+   *
+   * @deprecated use {@link InitializingHttpScmProtocolWrapper(Provider, RootURL)} instead.
+   */
+  @Deprecated
  protected InitializingHttpScmProtocolWrapper(Provider<? extends ScmProviderHttpServlet> delegateProvider, Provider<ScmPathInfoStore> pathInfoStore, ScmConfiguration scmConfiguration) {
    this.delegateProvider = delegateProvider;
-    this.pathInfoStore = pathInfoStore;
-    this.scmConfiguration = scmConfiguration;
+    this.basePathSupplier = new LegacySupplier(pathInfoStore, scmConfiguration);
+  }
+
+  /**
+   * Constructs a new {@link InitializingHttpScmProtocolWrapper}.
+   *
+   * @param delegateProvider injection provider for the servlet delegate
+   * @param rootURL root url
+   *
+   * @since 2.3.1
+   */
+  public InitializingHttpScmProtocolWrapper(Provider<? extends ScmProviderHttpServlet> delegateProvider, RootURL rootURL) {
+    this.delegateProvider = delegateProvider;
+    this.basePathSupplier = rootURL::getAsString;
  }

  protected void initializeServlet(ServletConfig config, ScmProviderHttpServlet httpServlet) throws ServletException {
@@ -64,30 +86,45 @@ public abstract class InitializingHttpScmProtocolWrapper implements ScmProtocolP
  @Override
  public HttpScmProtocol get(Repository repository) {
    if (!repository.getType().equals(getType())) {
-      throw new IllegalArgumentException(String.format("cannot handle repository with type %s with protocol for type %s", repository.getType(), getType()));
+      throw new IllegalArgumentException(
+        String.format("cannot handle repository with type %s with protocol for type %s", repository.getType(), getType())
+      );
    }
-    return new ProtocolWrapper(repository, computeBasePath());
+    return new ProtocolWrapper(repository, basePathSupplier.get());
  }

-  private String computeBasePath() {
-    return getPathFromScmPathInfoIfAvailable().orElse(getPathFromConfiguration());
-  }
+  private static class LegacySupplier implements Supplier<String> {

-  private Optional<String> getPathFromScmPathInfoIfAvailable() {
-    try {
-      ScmPathInfoStore scmPathInfoStore = pathInfoStore.get();
-      if (scmPathInfoStore != null && scmPathInfoStore.get() != null) {
-        return of(scmPathInfoStore.get().getRootUri().toASCIIString());
+    private final Provider<ScmPathInfoStore> pathInfoStore;
+    private final ScmConfiguration scmConfiguration;
+
+    private LegacySupplier(Provider<ScmPathInfoStore> pathInfoStore, ScmConfiguration scmConfiguration) {
+      this.pathInfoStore = pathInfoStore;
+      this.scmConfiguration = scmConfiguration;
+    }
+
+    @Override
+    public String get() {
+      return getPathFromScmPathInfoIfAvailable().orElse(getPathFromConfiguration());
+    }
+
+    private Optional<String> getPathFromScmPathInfoIfAvailable() {
+      try {
+        ScmPathInfoStore scmPathInfoStore = pathInfoStore.get();
+        if (scmPathInfoStore != null && scmPathInfoStore.get() != null) {
+          return of(scmPathInfoStore.get().getRootUri().toASCIIString());
+        }
+      } catch (Exception e) {
+        log.debug("could not get ScmPathInfoStore from context", e);
      }
-    } catch (Exception e) {
-      log.debug("could not get ScmPathInfoStore from context", e);
+      return empty();
+    }
+
+    private String getPathFromConfiguration() {
+      log.debug("using base path from configuration: {}", scmConfiguration.getBaseUrl());
+      return scmConfiguration.getBaseUrl();
    }
-    return empty();
-  }

-  private String getPathFromConfiguration() {
-    log.debug("using base path from configuration: {}", scmConfiguration.getBaseUrl());
-    return scmConfiguration.getBaseUrl();
  }

  private class ProtocolWrapper extends HttpScmProtocol {
--- a/scm-core/src/test/java/sonia/scm/repository/spi/InitializingHttpScmProtocolWrapperTest.java
+++ b/scm-core/src/test/java/sonia/scm/repository/spi/InitializingHttpScmProtocolWrapperTest.java
@@ -27,11 +27,13 @@ package sonia.scm.repository.spi;
 import com.google.inject.ProvisionException;
 import com.google.inject.util.Providers;
 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.Mock;
 import org.mockito.junit.jupiter.MockitoExtension;
 import org.mockito.stubbing.OngoingStubbing;
+import sonia.scm.RootURL;
 import sonia.scm.api.v2.resources.ScmPathInfo;
 import sonia.scm.api.v2.resources.ScmPathInfoStore;
 import sonia.scm.config.ScmConfiguration;
@@ -56,93 +58,125 @@ class InitializingHttpScmProtocolWrapperTest {

  @Mock
  private ScmProviderHttpServlet delegateServlet;
-  @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;

-  @BeforeEach
-  void init() {
-    pathInfoStoreProvider = mock(Provider.class);
-    lenient().when(pathInfoStoreProvider.get()).thenReturn(pathInfoStore);

-    wrapper = new InitializingHttpScmProtocolWrapper(Providers.of(this.delegateServlet), pathInfoStoreProvider, scmConfiguration) {
-      @Override
-      public String getType() {
-        return "git";
-      }
-    };
-    lenient().when(scmConfiguration.getBaseUrl()).thenReturn("http://example.com/scm");
+  @Nested
+  class WithRootURL {
+
+    @Mock
+    private RootURL rootURL;
+
+    @BeforeEach
+    void init() {
+      wrapper = new InitializingHttpScmProtocolWrapper(Providers.of(delegateServlet), rootURL) {
+        @Override
+        public String getType() {
+          return "git";
+        }
+      };
+      when(rootURL.getAsString()).thenReturn("https://hitchhiker.com/scm");
+    }
+
+    @Test
+    void shouldReturnUrlFromRootURL() {
+      HttpScmProtocol httpScmProtocol = wrapper.get(REPOSITORY);
+
+      assertEquals("https://hitchhiker.com/scm/repo/space/name", httpScmProtocol.getUrl());
+    }
+
  }

-  @Test
-  void shouldUsePathFromPathInfo() {
-    mockSetPathInfo();
+  @Nested
+  class WithPathInfoStore {

-    HttpScmProtocol httpScmProtocol = wrapper.get(REPOSITORY);
+    @Mock
+    private ScmPathInfoStore pathInfoStore;
+    @Mock
+    private ScmConfiguration scmConfiguration;

-    assertEquals("http://example.com/scm/repo/space/name", httpScmProtocol.getUrl());
-  }
+    private Provider<ScmPathInfoStore> pathInfoStoreProvider;

-  @Test
-  void shouldUseConfigurationWhenPathInfoNotSet() {
-    HttpScmProtocol httpScmProtocol = wrapper.get(REPOSITORY);
+    @Mock
+    private HttpServletRequest request;
+    @Mock
+    private HttpServletResponse response;
+    @Mock
+    private ServletConfig servletConfig;

-    assertEquals("http://example.com/scm/repo/space/name", httpScmProtocol.getUrl());
-  }
+    @BeforeEach
+    void init() {
+      pathInfoStoreProvider = mock(Provider.class);
+      lenient().when(pathInfoStoreProvider.get()).thenReturn(pathInfoStore);

-  @Test
-  void shouldUseConfigurationWhenNotInRequestScope() {
-    when(pathInfoStoreProvider.get()).thenThrow(new ProvisionException("test"));
+      wrapper = new InitializingHttpScmProtocolWrapper(Providers.of(delegateServlet), pathInfoStoreProvider, scmConfiguration) {
+        @Override
+        public String getType() {
+          return "git";
+        }
+      };
+      lenient().when(scmConfiguration.getBaseUrl()).thenReturn("http://example.com/scm");
+    }

-    HttpScmProtocol httpScmProtocol = wrapper.get(REPOSITORY);
+    @Test
+    void shouldUsePathFromPathInfo() {
+      mockSetPathInfo();

-    assertEquals("http://example.com/scm/repo/space/name", httpScmProtocol.getUrl());
-  }
+      HttpScmProtocol httpScmProtocol = wrapper.get(REPOSITORY);

-  @Test
-  void shouldInitializeAndDelegateRequestThroughFilter() throws ServletException, IOException {
-    HttpScmProtocol httpScmProtocol = wrapper.get(REPOSITORY);
+      assertEquals("http://example.com/scm/repo/space/name", httpScmProtocol.getUrl());
+    }

-    httpScmProtocol.serve(request, response, servletConfig);
+    @Test
+    void shouldUseConfigurationWhenPathInfoNotSet() {
+      HttpScmProtocol httpScmProtocol = wrapper.get(REPOSITORY);

-    verify(delegateServlet).init(servletConfig);
-    verify(delegateServlet).service(request, response, REPOSITORY);
-  }
+      assertEquals("http://example.com/scm/repo/space/name", httpScmProtocol.getUrl());
+    }

-  @Test
-  void shouldInitializeOnlyOnce() throws ServletException, IOException {
-    HttpScmProtocol httpScmProtocol = wrapper.get(REPOSITORY);
+    @Test
+    void shouldUseConfigurationWhenNotInRequestScope() {
+      when(pathInfoStoreProvider.get()).thenThrow(new ProvisionException("test"));

-    httpScmProtocol.serve(request, response, servletConfig);
-    httpScmProtocol.serve(request, response, servletConfig);
+      HttpScmProtocol httpScmProtocol = wrapper.get(REPOSITORY);

-    verify(delegateServlet, times(1)).init(servletConfig);
-    verify(delegateServlet, times(2)).service(request, response, REPOSITORY);
-  }
+      assertEquals("http://example.com/scm/repo/space/name", httpScmProtocol.getUrl());
+    }

-  @Test
-  void shouldFailForIllegalScmType() {
-    Repository repository = new Repository("", "other", "space", "name");
-    assertThrows(
-      IllegalArgumentException.class,
-      () -> wrapper.get(repository)
-    );
-  }
+    @Test
+    void shouldInitializeAndDelegateRequestThroughFilter() throws ServletException, IOException {
+      HttpScmProtocol httpScmProtocol = wrapper.get(REPOSITORY);
+
+      httpScmProtocol.serve(request, response, servletConfig);
+
+      verify(delegateServlet).init(servletConfig);
+      verify(delegateServlet).service(request, response, REPOSITORY);
+    }
+
+    @Test
+    void shouldInitializeOnlyOnce() throws ServletException, IOException {
+      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);
+    }
+
+    @Test
+    void shouldFailForIllegalScmType() {
+      Repository repository = new Repository("", "other", "space", "name");
+      assertThrows(
+        IllegalArgumentException.class,
+        () -> wrapper.get(repository)
+      );
+    }
+
+    private OngoingStubbing<ScmPathInfo> mockSetPathInfo() {
+      return when(pathInfoStore.get()).thenReturn(() -> URI.create("http://example.com/scm/api/"));
+    }

-  private OngoingStubbing<ScmPathInfo> mockSetPathInfo() {
-    return when(pathInfoStore.get()).thenReturn(() -> URI.create("http://example.com/scm/api/"));
  }

 }