Prevent repository loading in namespace mapper

Committed-by: Eduard Heimbuch <eduard.heimbuch@cloudogu.com>
Co-authored-by: René Pfeuffer <rene.pfeuffer@cloudogu.com>

gradle/changelog/namespace_mapper.yaml

@@ -0,0 +1,2 @@
+- type: fixed
+  description: Performance (prevent reading of repositories in namespace mapper)

scm-webapp/src/main/java/sonia/scm/api/v2/resources/NamespaceCollectionResource.java

@@ -29,7 +29,7 @@ import io.swagger.v3.oas.annotations.Operation;
 import io.swagger.v3.oas.annotations.media.Content;
 import io.swagger.v3.oas.annotations.media.Schema;
 import io.swagger.v3.oas.annotations.responses.ApiResponse;
-import sonia.scm.repository.RepositoryManager;
+import sonia.scm.repository.NamespaceManager;
 import sonia.scm.web.VndMediaType;
 
 import javax.inject.Inject;
@@ -39,11 +39,11 @@ import javax.ws.rs.Produces;
 
 public class NamespaceCollectionResource {
 
-  private final RepositoryManager manager;
+  private final NamespaceManager manager;
   private final NamespaceCollectionToDtoMapper repositoryCollectionToDtoMapper;
 
   @Inject
-  public NamespaceCollectionResource(RepositoryManager manager, NamespaceCollectionToDtoMapper repositoryCollectionToDtoMapper) {
+  public NamespaceCollectionResource(NamespaceManager manager, NamespaceCollectionToDtoMapper repositoryCollectionToDtoMapper) {
     this.manager = manager;
     this.repositoryCollectionToDtoMapper = repositoryCollectionToDtoMapper;
   }
@@ -72,6 +72,6 @@ public class NamespaceCollectionResource {
       schema = @Schema(implementation = ErrorDto.class)
     ))
   public HalRepresentation getAll() {
-    return repositoryCollectionToDtoMapper.map(manager.getAllNamespaces());
+    return repositoryCollectionToDtoMapper.map(manager.getAll());
   }
 }

scm-webapp/src/main/java/sonia/scm/api/v2/resources/NamespaceCollectionToDtoMapper.java

@@ -26,6 +26,7 @@ package sonia.scm.api.v2.resources;
 
 import de.otto.edison.hal.Embedded;
 import de.otto.edison.hal.HalRepresentation;
+import sonia.scm.repository.Namespace;
 
 import javax.inject.Inject;
 import java.util.Collection;
@@ -45,7 +46,7 @@ public class NamespaceCollectionToDtoMapper {
     this.links = links;
   }
 
-  public HalRepresentation map(Collection<String> namespaces) {
+  public HalRepresentation map(Collection<Namespace> namespaces) {
     Embedded namespaceDtos = embeddedBuilder()
       .with("namespaces", namespaces.stream().map(namespaceMapper::map).collect(toList()))
       .build();

scm-webapp/src/main/java/sonia/scm/api/v2/resources/NamespaceResource.java

@@ -28,7 +28,7 @@ import io.swagger.v3.oas.annotations.Operation;
 import io.swagger.v3.oas.annotations.media.Content;
 import io.swagger.v3.oas.annotations.media.Schema;
 import io.swagger.v3.oas.annotations.responses.ApiResponse;
-import sonia.scm.repository.RepositoryManager;
+import sonia.scm.repository.NamespaceManager;
 import sonia.scm.web.VndMediaType;
 
 import javax.inject.Inject;
@@ -43,12 +43,12 @@ import static sonia.scm.NotFoundException.notFound;
 
 public class NamespaceResource {
 
-  private final RepositoryManager manager;
+  private final NamespaceManager manager;
   private final NamespaceToNamespaceDtoMapper namespaceMapper;
   private final Provider<NamespacePermissionResource> namespacePermissionResource;
 
   @Inject
-  public NamespaceResource(RepositoryManager manager, NamespaceToNamespaceDtoMapper namespaceMapper, Provider<NamespacePermissionResource> namespacePermissionResource) {
+  public NamespaceResource(NamespaceManager manager, NamespaceToNamespaceDtoMapper namespaceMapper, Provider<NamespacePermissionResource> namespacePermissionResource) {
     this.manager = manager;
     this.namespaceMapper = namespaceMapper;
     this.namespacePermissionResource = namespacePermissionResource;
@@ -92,11 +92,8 @@ public class NamespaceResource {
     )
   )
   public NamespaceDto get(@PathParam("namespace") String namespace) {
-    return manager.getAllNamespaces()
-      .stream()
-      .filter(n -> n.equals(namespace))
+    return manager.get(namespace)
       .map(namespaceMapper::map)
-      .findFirst()
       .orElseThrow(() -> notFound(entity("Namespace", namespace)));
   }
 

scm-webapp/src/main/java/sonia/scm/api/v2/resources/NamespaceToNamespaceDtoMapper.java

@@ -28,7 +28,6 @@ import com.google.common.annotations.VisibleForTesting;
 import de.otto.edison.hal.Embedded;
 import de.otto.edison.hal.Link;
 import de.otto.edison.hal.Links;
-import org.mapstruct.InjectionStrategy;
 import org.mapstruct.Mapper;
 import org.mapstruct.Mapping;
 import org.mapstruct.ObjectFactory;
@@ -41,13 +40,14 @@ import sonia.scm.web.EdisonHalAppender;
 
 import javax.inject.Inject;
 import java.util.List;
-import java.util.Optional;
 import java.util.stream.Collectors;
 
 import static de.otto.edison.hal.Embedded.embeddedBuilder;
 import static de.otto.edison.hal.Link.link;
 import static de.otto.edison.hal.Link.linkBuilder;
 import static de.otto.edison.hal.Links.linkingTo;
+import static sonia.scm.ContextEntry.ContextBuilder.entity;
+import static sonia.scm.NotFoundException.notFound;
 
 // Mapstruct does not support parameterized (i.e. non-default) constructors. Thus, we need to use field injection.
 @SuppressWarnings("squid:S3306")
@@ -61,29 +61,30 @@ public abstract class NamespaceToNamespaceDtoMapper extends BaseMapper<Namespace
   @Inject
   protected NamespaceManager namespaceManager;
 
+  public NamespaceDto map(String namespace) {
+    return map(namespaceManager.get(namespace).orElseThrow(() -> notFound(entity(Namespace.class, namespace))));
+  }
+
   @Mapping(target = "attributes", ignore = true) // We do not map HAL attributes
-  public abstract NamespaceDto map(String namespace);
+  public abstract NamespaceDto map(Namespace namespace);
 
   @ObjectFactory
-  NamespaceDto createDto(String namespace) {
+  NamespaceDto createDto(Namespace namespace) {
     Links.Builder linkingTo = linkingTo();
     linkingTo
-      .self(links.namespace().self(namespace))
-      .single(link("repositories", links.repositoryCollection().forNamespace(namespace)));
+      .self(links.namespace().self(namespace.getNamespace()))
+      .single(link("repositories", links.repositoryCollection().forNamespace(namespace.getNamespace())));
 
     if (NamespacePermissions.permissionRead().isPermitted()) {
       linkingTo
-        .single(link("permissions", links.namespacePermission().all(namespace)));
-    }
-    linkingTo.array(searchLinks(namespace));
-    linkingTo.single(link("searchableTypes", links.searchableTypes().searchableTypesForNamespace(namespace)));
-    Optional<Namespace> optionalNamespace = namespaceManager.get(namespace);
-    if (optionalNamespace.isPresent()) {
-      Embedded.Builder embeddedBuilder = embeddedBuilder();
-      applyEnrichers(new EdisonHalAppender(linkingTo, embeddedBuilder), optionalNamespace.get(), namespace);
+        .single(link("permissions", links.namespacePermission().all(namespace.getNamespace())));
     }
+    linkingTo.array(searchLinks(namespace.getNamespace()));
+    linkingTo.single(link("searchableTypes", links.searchableTypes().searchableTypesForNamespace(namespace.getNamespace())));
+    Embedded.Builder embeddedBuilder = embeddedBuilder();
+    applyEnrichers(new EdisonHalAppender(linkingTo, embeddedBuilder), namespace, namespace.getNamespace());
 
-    return new NamespaceDto(namespace, linkingTo.build());
+    return new NamespaceDto(namespace.getNamespace(), linkingTo.build());
   }
 
   @VisibleForTesting

scm-webapp/src/test/java/sonia/scm/api/v2/resources/NamespaceRootResourceTest.java

@@ -39,7 +39,6 @@ import org.mockito.Mock;
 import org.mockito.junit.jupiter.MockitoExtension;
 import sonia.scm.repository.Namespace;
 import sonia.scm.repository.NamespaceManager;
-import sonia.scm.repository.RepositoryManager;
 import sonia.scm.repository.RepositoryPermission;
 import sonia.scm.search.SearchEngine;
 import sonia.scm.search.SearchableType;
@@ -66,8 +65,6 @@ import static org.mockito.Mockito.when;
 @ExtendWith(MockitoExtension.class)
 class NamespaceRootResourceTest {
 
-  @Mock
-  RepositoryManager repositoryManager;
   @Mock
   NamespaceManager namespaceManager;
   @Mock
@@ -106,15 +103,15 @@ class NamespaceRootResourceTest {
     RepositoryPermissionCollectionToDtoMapper repositoryPermissionCollectionToDtoMapper = new RepositoryPermissionCollectionToDtoMapper(repositoryPermissionToRepositoryPermissionDtoMapper, links);
     RepositoryPermissionDtoToRepositoryPermissionMapperImpl dtoToModelMapper = new RepositoryPermissionDtoToRepositoryPermissionMapperImpl();
 
-    NamespaceCollectionResource namespaceCollectionResource = new NamespaceCollectionResource(repositoryManager, namespaceCollectionToDtoMapper);
+    NamespaceCollectionResource namespaceCollectionResource = new NamespaceCollectionResource(namespaceManager, namespaceCollectionToDtoMapper);
     NamespacePermissionResource namespacePermissionResource = new NamespacePermissionResource(dtoToModelMapper, repositoryPermissionToRepositoryPermissionDtoMapper, repositoryPermissionCollectionToDtoMapper, links, namespaceManager);
-    NamespaceResource namespaceResource = new NamespaceResource(repositoryManager, namespaceMapper, of(namespacePermissionResource));
+    NamespaceResource namespaceResource = new NamespaceResource(namespaceManager, namespaceMapper, of(namespacePermissionResource));
     dispatcher.addSingletonResource(new NamespaceRootResource(of(namespaceCollectionResource), of(namespaceResource)));
   }
 
   @BeforeEach
   void mockExistingNamespaces() {
-    lenient().when(repositoryManager.getAllNamespaces()).thenReturn(asList("hitchhiker", "space"));
+    lenient().when(namespaceManager.getAll()).thenReturn(asList(new Namespace("hitchhiker"), new Namespace("space")));
     Namespace hitchhikerNamespace = new Namespace("hitchhiker");
     hitchhikerNamespace.setPermissions(singleton(new RepositoryPermission("humans", "READ", true)));
     Namespace spaceNamespace = new Namespace("space");