Merge pull request #246 from NDLANO/simplify-code

Simplify fetching child resources
NDLANO · Apr 17, 2024 · 74f87b4 · 74f87b4
2 parents 1e081b2 + 1942bdf
commit 74f87b4
Show file tree

Hide file tree

Showing 5 changed files with 45 additions and 83 deletions.
diff --git 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<NodeConnect
     @Query(
             """
             SELECT DISTINCT nc FROM NodeConnection nc
-            LEFT JOIN FETCH nc.child child
-            LEFT JOIN FETCH nc.parent n
-            LEFT JOIN FETCH child.resourceResourceTypes rrt
-            LEFT JOIN FETCH nc.relevance rel
-            LEFT JOIN FETCH rrt.resourceType rt
-            WHERE n.publicId IN :nodeIds
-            AND ((:#{#resourceTypePublicIds == null} = true) OR rt.publicId IN :resourceTypePublicIds)
-            AND (:relevancePublicId IS NULL OR rel.publicId = :relevancePublicId)
-            AND child.nodeType = 'RESOURCE'
-            """)
-    List<NodeConnection> getResourceBy(Set<URI> nodeIds, Set<URI> 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'
+            LEFT JOIN nc.child c
+            LEFT JOIN nc.parent p
+            LEFT JOIN c.resourceResourceTypes rrt
+            LEFT JOIN nc.relevance rel
+            LEFT JOIN rrt.resourceType rt
+            WHERE p.publicId IN :nodeIds
+            AND ((:#{#resourceTypeIds == null} = true) OR rt.publicId IN :resourceTypeIds)
+            AND (:relevanceId IS NULL OR rel.publicId = :relevanceId)
+            AND c.nodeType = 'RESOURCE'
             """)
-    List<NodeConnection> getByResourceIds(Collection<URI> nodeIds);
-
-    @Query("SELECT nc FROM NodeConnection nc JOIN FETCH nc.parent JOIN FETCH nc.child")
-    List<NodeConnection> findAllIncludingParentAndChild();
+    List<NodeConnection> getResourceBy(
+            Set<URI> nodeIds, Optional<List<URI>> resourceTypeIds, Optional<URI> 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
@@ -377,20 +377,13 @@ public List<NodeChildDTO> 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<List<URI>> resourceTypeIds,
             @Parameter(description = "Select by relevance. If not specified, all resources will be returned.")
                     @RequestParam(value = "relevance", required = false)
-                    URI relevance) {
-        final Set<URI> resourceTypeIdSet;
-
-        if (resourceTypeIds == null) {
-            resourceTypeIdSet = Set.of();
-        } else {
-            resourceTypeIdSet = new HashSet<>(Arrays.asList(resourceTypeIds));
-        }
+                    Optional<URI> 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
@@ -326,17 +326,13 @@ public List<NodeChildDTO> 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<List<URI>> resourceTypeIds,
             @Parameter(description = "Select by relevance. If not specified, all resources will be returned.")
-                    @RequestParam(value = "relevance", required = false, defaultValue = "")
-                    URI relevance) {
-        final Set<URI> 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<URI> 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
@@ -235,20 +235,13 @@ public List<NodeChildDTO> 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<List<URI>> resourceTypeIds,
             @Parameter(description = "Select by relevance. If not specified, all resources will be returned.")
                     @RequestParam(value = "relevance", required = false)
-                    URI relevance) {
-        final Set<URI> resourceTypeIdSet;
-
-        if (resourceTypeIds == null) {
-            resourceTypeIdSet = Set.of();
-        } else {
-            resourceTypeIdSet = new HashSet<>(Arrays.asList(resourceTypeIds));
-        }
+                    Optional<URI> 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
@@ -7,8 +7,6 @@
 
 package no.ndla.taxonomy.service;
 
-import static java.util.stream.Collectors.toList;
-
 import java.net.URI;
 import java.util.*;
 import java.util.concurrent.ExecutorService;
@@ -200,8 +198,8 @@ public Node getNode(URI publicId) {
 
     public List<NodeChildDTO> getResourcesByNodeId(
             URI nodePublicId,
-            Set<URI> resourceTypeIds,
-            URI relevancePublicId,
+            Optional<List<URI>> resourceTypeIds,
+            Optional<URI> relevanceId,
             Optional<String> languageCode,
             boolean recursive,
             Optional<Boolean> includeContexts,
@@ -238,7 +236,7 @@ public List<NodeChildDTO> getResourcesByNodeId(
                 node,
                 topicIdsToSearchFor,
                 resourceTypeIds,
-                relevancePublicId,
+                relevanceId,
                 resourcesToSort,
                 languageCode,
                 includeContexts,
@@ -248,35 +246,32 @@ public List<NodeChildDTO> getResourcesByNodeId(
     private List<NodeChildDTO> filterNodeResourcesByIdsAndReturn(
             Node root,
             Set<URI> nodeIds,
-            Set<URI> resourceTypeIds,
-            URI relevance,
+            Optional<List<URI>> resourceTypeIds,
+            Optional<URI> relevanceId,
             Set<ResourceTreeSortable> sortableListToAddTo,
             Optional<String> languageCode,
             Optional<Boolean> includeContexts,
             boolean filterProgrammes) {
         final List<NodeConnection> 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)));
 
@@ -298,7 +293,7 @@ private List<NodeChildDTO> filterNodeResourcesByIdsAndReturn(
                             includeContexts,
                             filterProgrammes);
                 })
-                .collect(toList());
+                .toList();
     }
 
     @Override