From 0a98e8aa67b32d7b1a1d757dbcdeeafbc1704d8b Mon Sep 17 00:00:00 2001 From: Gunnar Velle Date: Tue, 16 Apr 2024 12:12:22 +0200 Subject: [PATCH 1/2] Remove duplicate repository method --- .../NodeConnectionRepository.java | 33 ++++--------- .../java/no/ndla/taxonomy/rest/v1/Nodes.java | 13 ++--- .../no/ndla/taxonomy/rest/v1/Subjects.java | 14 ++---- .../java/no/ndla/taxonomy/rest/v1/Topics.java | 13 ++--- .../no/ndla/taxonomy/service/NodeService.java | 47 +++++++++---------- 5 files changed, 42 insertions(+), 78 deletions(-) diff --git a/src/main/java/no/ndla/taxonomy/repositories/NodeConnectionRepository.java b/src/main/java/no/ndla/taxonomy/repositories/NodeConnectionRepository.java index b0411ae1..2324d075 100644 --- a/src/main/java/no/ndla/taxonomy/repositories/NodeConnectionRepository.java +++ b/src/main/java/no/ndla/taxonomy/repositories/NodeConnectionRepository.java @@ -31,33 +31,18 @@ public interface NodeConnectionRepository extends TaxonomyRepository getResourceBy(Set nodeIds, Set resourceTypePublicIds, URI relevancePublicId); - - @Query( - """ - SELECT DISTINCT nc FROM NodeConnection nc - JOIN FETCH nc.child r - LEFT JOIN FETCH nc.parent n - LEFT JOIN FETCH r.resourceResourceTypes rrt - LEFT JOIN FETCH nc.relevance rel - LEFT JOIN FETCH rrt.resourceType rt - WHERE n.publicId IN :nodeIds - AND r.nodeType = 'RESOURCE' - """) - List getByResourceIds(Collection nodeIds); - - @Query("SELECT nc FROM NodeConnection nc JOIN FETCH nc.parent JOIN FETCH nc.child") - List findAllIncludingParentAndChild(); + List getResourceBy( + Set nodeIds, Optional> resourceTypeIds, Optional relevanceId); @Query( "SELECT nc FROM NodeConnection nc JOIN FETCH nc.parent JOIN FETCH nc.child c WHERE c.nodeType = :childNodeType") diff --git a/src/main/java/no/ndla/taxonomy/rest/v1/Nodes.java b/src/main/java/no/ndla/taxonomy/rest/v1/Nodes.java index 91900c21..becb9785 100644 --- a/src/main/java/no/ndla/taxonomy/rest/v1/Nodes.java +++ b/src/main/java/no/ndla/taxonomy/rest/v1/Nodes.java @@ -377,20 +377,13 @@ public List getResources( "Select by resource type id(s). If not specified, resources of all types will be returned. " + "Multiple ids may be separated with comma or the parameter may be repeated for each id.") @RequestParam(value = "type", required = false) - URI[] resourceTypeIds, + Optional> resourceTypeIds, @Parameter(description = "Select by relevance. If not specified, all resources will be returned.") @RequestParam(value = "relevance", required = false) - URI relevance) { - final Set resourceTypeIdSet; - - if (resourceTypeIds == null) { - resourceTypeIdSet = Set.of(); - } else { - resourceTypeIdSet = new HashSet<>(Arrays.asList(resourceTypeIds)); - } + Optional relevance) { return nodeService.getResourcesByNodeId( - nodeId, resourceTypeIdSet, relevance, language, recursive, includeContexts, filterProgrammes); + nodeId, resourceTypeIds, relevance, language, recursive, includeContexts, filterProgrammes); } @GetMapping("{id}/full") diff --git a/src/main/java/no/ndla/taxonomy/rest/v1/Subjects.java b/src/main/java/no/ndla/taxonomy/rest/v1/Subjects.java index 9172a440..68a46a58 100644 --- a/src/main/java/no/ndla/taxonomy/rest/v1/Subjects.java +++ b/src/main/java/no/ndla/taxonomy/rest/v1/Subjects.java @@ -326,17 +326,13 @@ public List getSubjectResources( description = "Filter by resource type id(s). If not specified, resources of all types will be returned." + "Multiple ids may be separated with comma or the parameter may be repeated for each id.") - @RequestParam(value = "type", required = false, defaultValue = "") - URI[] resourceTypeIds, + @RequestParam(value = "type", required = false) + Optional> resourceTypeIds, @Parameter(description = "Select by relevance. If not specified, all resources will be returned.") - @RequestParam(value = "relevance", required = false, defaultValue = "") - URI relevance) { - final Set resourceTypeIdSet = resourceTypeIds != null ? Set.of(resourceTypeIds) : Set.of(); - - // If null is sent to query it will be ignored, otherwise it will filter by relevance - final var relevanceArgument = relevance == null || relevance.toString().equals("") ? null : relevance; + @RequestParam(value = "relevance", required = false) + Optional relevance) { return nodeService.getResourcesByNodeId( - subjectId, resourceTypeIdSet, relevanceArgument, language, true, Optional.of(false), false); + subjectId, resourceTypeIds, relevance, language, true, Optional.of(false), false); } } diff --git a/src/main/java/no/ndla/taxonomy/rest/v1/Topics.java b/src/main/java/no/ndla/taxonomy/rest/v1/Topics.java index 3274f1b7..f0992eb0 100644 --- a/src/main/java/no/ndla/taxonomy/rest/v1/Topics.java +++ b/src/main/java/no/ndla/taxonomy/rest/v1/Topics.java @@ -235,20 +235,13 @@ public List getTopicResources( "Select by resource type id(s). If not specified, resources of all types will be returned." + "Multiple ids may be separated with comma or the parameter may be repeated for each id.") @RequestParam(value = "type", required = false) - URI[] resourceTypeIds, + Optional> resourceTypeIds, @Parameter(description = "Select by relevance. If not specified, all resources will be returned.") @RequestParam(value = "relevance", required = false) - URI relevance) { - final Set resourceTypeIdSet; - - if (resourceTypeIds == null) { - resourceTypeIdSet = Set.of(); - } else { - resourceTypeIdSet = new HashSet<>(Arrays.asList(resourceTypeIds)); - } + Optional relevance) { return nodeService.getResourcesByNodeId( - topicId, resourceTypeIdSet, relevance, language, recursive, Optional.of(false), false); + topicId, resourceTypeIds, relevance, language, recursive, Optional.of(false), false); } @Deprecated diff --git a/src/main/java/no/ndla/taxonomy/service/NodeService.java b/src/main/java/no/ndla/taxonomy/service/NodeService.java index a741209b..fb51f86d 100644 --- a/src/main/java/no/ndla/taxonomy/service/NodeService.java +++ b/src/main/java/no/ndla/taxonomy/service/NodeService.java @@ -200,8 +200,8 @@ public Node getNode(URI publicId) { public List getResourcesByNodeId( URI nodePublicId, - Set resourceTypeIds, - URI relevancePublicId, + Optional> resourceTypeIds, + Optional relevanceId, Optional languageCode, boolean recursive, Optional includeContexts, @@ -238,7 +238,7 @@ public List getResourcesByNodeId( node, topicIdsToSearchFor, resourceTypeIds, - relevancePublicId, + relevanceId, resourcesToSort, languageCode, includeContexts, @@ -248,35 +248,32 @@ public List getResourcesByNodeId( private List filterNodeResourcesByIdsAndReturn( Node root, Set nodeIds, - Set resourceTypeIds, - URI relevance, + Optional> resourceTypeIds, + Optional relevanceId, Set sortableListToAddTo, Optional languageCode, Optional includeContexts, boolean filterProgrammes) { final List nodeResources; - if (!resourceTypeIds.isEmpty()) { - nodeResources = nodeConnectionRepository.getResourceBy(nodeIds, resourceTypeIds, relevance); - } else { - var nodeResourcesStream = nodeConnectionRepository.getByResourceIds(nodeIds).stream(); - if (relevance != null) { - final var isRequestingCore = "urn:relevance:core".equals(relevance.toString()); - nodeResourcesStream = nodeResourcesStream.filter(nodeResource -> { - final var resource = nodeResource.getChild().orElse(null); - if (resource == null) { - return false; - } - final var rel = nodeResource.getRelevance().orElse(null); - if (rel != null) { - return rel.getPublicId().equals(relevance); - } else { - return isRequestingCore; - } - }); - } - nodeResources = nodeResourcesStream.collect(toList()); + var nodeResourcesStream = + nodeConnectionRepository.getResourceBy(nodeIds, resourceTypeIds, relevanceId).stream(); + if (relevanceId.isPresent()) { + final var isRequestingCore = "urn:relevance:core".equals(relevanceId.toString()); + nodeResourcesStream = nodeResourcesStream.filter(nodeResource -> { + final var resource = nodeResource.getChild().orElse(null); + if (resource == null) { + return false; + } + final var rel = nodeResource.getRelevance().orElse(null); + if (rel != null) { + return rel.getPublicId().equals(relevanceId.get()); + } else { + return isRequestingCore; + } + }); } + nodeResources = nodeResourcesStream.toList(); nodeResources.forEach(nodeResource -> sortableListToAddTo.add(new ResourceTreeSortable(nodeResource))); From 1942bdf9136c2e1d92d8e9eeeffe53294dfc83c9 Mon Sep 17 00:00:00 2001 From: Gunnar Velle Date: Wed, 17 Apr 2024 09:10:37 +0200 Subject: [PATCH 2/2] Do not fetch all the data at once to offload db cpu --- .../repositories/NodeConnectionRepository.java | 10 +++++----- .../java/no/ndla/taxonomy/service/NodeService.java | 4 +--- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/main/java/no/ndla/taxonomy/repositories/NodeConnectionRepository.java b/src/main/java/no/ndla/taxonomy/repositories/NodeConnectionRepository.java index 2324d075..bde2a4fa 100644 --- a/src/main/java/no/ndla/taxonomy/repositories/NodeConnectionRepository.java +++ b/src/main/java/no/ndla/taxonomy/repositories/NodeConnectionRepository.java @@ -31,11 +31,11 @@ public interface NodeConnectionRepository extends TaxonomyRepository filterNodeResourcesByIdsAndReturn( includeContexts, filterProgrammes); }) - .collect(toList()); + .toList(); } @Override