From c3093a3b0dee06b1948ad21c49a36c6feb8366ae Mon Sep 17 00:00:00 2001 From: Rene Pfeuffer Date: Tue, 13 Jun 2023 16:16:15 +0200 Subject: [PATCH] Prevent repository loading in namespace mapper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Committed-by: Eduard Heimbuch Co-authored-by: René Pfeuffer --- gradle/changelog/namespace_mapper.yaml | 2 ++ .../NamespaceCollectionResource.java | 8 ++--- .../NamespaceCollectionToDtoMapper.java | 3 +- .../api/v2/resources/NamespaceResource.java | 11 +++---- .../NamespaceToNamespaceDtoMapper.java | 31 ++++++++++--------- .../resources/NamespaceRootResourceTest.java | 9 ++---- 6 files changed, 31 insertions(+), 33 deletions(-) create mode 100644 gradle/changelog/namespace_mapper.yaml diff --git a/gradle/changelog/namespace_mapper.yaml b/gradle/changelog/namespace_mapper.yaml new file mode 100644 index 0000000000..32c2b9546f --- /dev/null +++ b/gradle/changelog/namespace_mapper.yaml @@ -0,0 +1,2 @@ +- type: fixed + description: Performance (prevent reading of repositories in namespace mapper) diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/NamespaceCollectionResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/NamespaceCollectionResource.java index 6e76a7b951..10d33ab0fa 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/NamespaceCollectionResource.java +++ b/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()); } } diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/NamespaceCollectionToDtoMapper.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/NamespaceCollectionToDtoMapper.java index 64d0b4cb55..e9b584dc78 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/NamespaceCollectionToDtoMapper.java +++ b/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 namespaces) { + public HalRepresentation map(Collection namespaces) { Embedded namespaceDtos = embeddedBuilder() .with("namespaces", namespaces.stream().map(namespaceMapper::map).collect(toList())) .build(); diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/NamespaceResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/NamespaceResource.java index 1f0c828ca7..c369f00ee4 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/NamespaceResource.java +++ b/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; @Inject - public NamespaceResource(RepositoryManager manager, NamespaceToNamespaceDtoMapper namespaceMapper, Provider namespacePermissionResource) { + public NamespaceResource(NamespaceManager manager, NamespaceToNamespaceDtoMapper namespaceMapper, Provider 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))); } diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/NamespaceToNamespaceDtoMapper.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/NamespaceToNamespaceDtoMapper.java index f791d810f2..1a42551c00 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/NamespaceToNamespaceDtoMapper.java +++ b/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 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 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 diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/NamespaceRootResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/NamespaceRootResourceTest.java index 9423e179d0..52cd180e96 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/NamespaceRootResourceTest.java +++ b/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");