From 9b2b559a8634e375b722175a797c388b25979804 Mon Sep 17 00:00:00 2001 From: Hugo Marcellin Date: Mon, 6 Oct 2025 14:09:44 +0200 Subject: [PATCH 01/11] Homogenize permissions checks --- .../directory/server/DirectoryService.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/gridsuite/directory/server/DirectoryService.java b/src/main/java/org/gridsuite/directory/server/DirectoryService.java index ba21acea..177cfcfd 100644 --- a/src/main/java/org/gridsuite/directory/server/DirectoryService.java +++ b/src/main/java/org/gridsuite/directory/server/DirectoryService.java @@ -270,17 +270,17 @@ public List getDirectoryElements(UUID directoryUuid, List descendents = repositoryService.findAllDescendants(directoryUuid).stream().toList(); return descendents .stream() - .filter(e -> types.isEmpty() || types.contains(e.getType())) + .filter(e -> (types.isEmpty() || types.contains(e.getType())) && hasReadPermissions(userId, List.of(e.getId()))) .map(ElementAttributes::toElementAttributes) .toList(); } else { - return getAllDirectoryElementsStream(directoryUuid, types).toList(); + return getAllDirectoryElementsStream(directoryUuid, types).filter(e -> hasReadPermissions(userId, List.of(e.getElementUuid()))).toList(); } } - private Stream getOnlyElementsStream(UUID directoryUuid, List types) { + private Stream getOnlyElementsStream(UUID directoryUuid, List types, String userId) { return getAllDirectoryElementsStream(directoryUuid, types) - .filter(elementAttributes -> !elementAttributes.getType().equals(DIRECTORY)); + .filter(elementAttributes -> !elementAttributes.getType().equals(DIRECTORY) && hasReadPermissions(userId, List.of(elementAttributes.getElementUuid()))); } private Stream getAllDirectoryElementsStream(UUID directoryUuid, List types) { @@ -313,7 +313,7 @@ public void updateElement(UUID elementUuid, ElementAttributes newElementAttribut DirectoryElementEntity directoryElement = getDirectoryElementEntity(elementUuid); if (!directoryElement.isAttributesUpdatable(newElementAttributes, userId) || !directoryElement.getName().equals(newElementAttributes.getElementName()) && - directoryHasElementOfNameAndType(directoryElement.getParentId(), newElementAttributes.getElementName(), directoryElement.getType())) { + directoryHasElementOfNameAndType(directoryElement.getParentId(), newElementAttributes.getElementName(), directoryElement.getType(), userId)) { throw new DirectoryException(NOT_ALLOWED); } @@ -350,7 +350,7 @@ private void moveElementDirectory(DirectoryElementEntity element, UUID newDirect List descendents = isDirectory ? repositoryService.findAllDescendants(element.getId()).stream().toList() : List.of(); // validate move elements - validateElementForMove(element, newDirectoryUuid, descendents.stream().map(DirectoryElementEntity::getId).collect(Collectors.toSet())); + validateElementForMove(element, newDirectoryUuid, descendents.stream().map(DirectoryElementEntity::getId).collect(Collectors.toSet()), userId); // we update the parent of the moving element updateElementParentDirectory(element, newDirectoryUuid); @@ -369,12 +369,12 @@ private void moveElementDirectory(DirectoryElementEntity element, UUID newDirect } - private void validateElementForMove(DirectoryElementEntity element, UUID newDirectoryUuid, Set descendentsUuids) { + private void validateElementForMove(DirectoryElementEntity element, UUID newDirectoryUuid, Set descendentsUuids, String userId) { if (newDirectoryUuid == element.getId() || descendentsUuids.contains(newDirectoryUuid)) { throw new DirectoryException(MOVE_IN_DESCENDANT_NOT_ALLOWED); } - if (directoryHasElementOfNameAndType(newDirectoryUuid, element.getName(), element.getType())) { + if (directoryHasElementOfNameAndType(newDirectoryUuid, element.getName(), element.getType(), userId)) { throw DirectoryException.createElementNameAlreadyExists(element.getName()); } } @@ -393,8 +393,8 @@ private void validateNewDirectory(UUID newDirectoryUuid) { } } - private boolean directoryHasElementOfNameAndType(UUID directoryUUID, String elementName, String elementType) { - return getOnlyElementsStream(directoryUUID, List.of(elementType)) + private boolean directoryHasElementOfNameAndType(UUID directoryUUID, String elementName, String elementType, String userId) { + return getOnlyElementsStream(directoryUUID, List.of(elementType), userId) .anyMatch( e -> e.getElementName().equals(elementName) ); From 53dc492bd1f6444433dc38a7c023a28dea830834 Mon Sep 17 00:00:00 2001 From: Hugo Marcellin Date: Mon, 6 Oct 2025 15:32:38 +0200 Subject: [PATCH 02/11] Fix test --- .../org/gridsuite/directory/server/DirectoryTest.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/test/java/org/gridsuite/directory/server/DirectoryTest.java b/src/test/java/org/gridsuite/directory/server/DirectoryTest.java index e8f87130..471af1b2 100644 --- a/src/test/java/org/gridsuite/directory/server/DirectoryTest.java +++ b/src/test/java/org/gridsuite/directory/server/DirectoryTest.java @@ -1121,18 +1121,18 @@ public void testGetElement() { @Test public void testGetElementWithNonAdminUser() { // Insert a root directory by the user1 - UUID rootDirUuid = insertAndCheckRootDirectory("rootDir1", "user1"); + UUID rootDirUuid = insertAndCheckRootDirectory("rootDir1", ADMIN_USER); // Insert an element of type TYPE_02 in the root directory by the user1 - ElementAttributes element1Attributes = toElementAttributes(TYPE_02_UUID, "elementName1", TYPE_02, "user1"); + ElementAttributes element1Attributes = toElementAttributes(TYPE_02_UUID, "elementName1", TYPE_02, ADMIN_USER); insertAndCheckSubElementInRootDir(rootDirUuid, element1Attributes); // Insert an element of type TYPE_03 in the root directory by the user1 - ElementAttributes element2Attributes = toElementAttributes(TYPE_03_UUID, "elementName2", TYPE_03, "user1"); + ElementAttributes element2Attributes = toElementAttributes(TYPE_03_UUID, "elementName2", TYPE_03, ADMIN_USER); insertAndCheckSubElementInRootDir(rootDirUuid, element2Attributes); // Insert an element of type TYPE_03 in the root directory by the user1 - ElementAttributes element3Attributes = toElementAttributes(UUID.randomUUID(), "elementName3", TYPE_03, "user1"); + ElementAttributes element3Attributes = toElementAttributes(UUID.randomUUID(), "elementName3", TYPE_03, ADMIN_USER); insertAndCheckSubElementInRootDir(rootDirUuid, element3Attributes); ElementAttributes rootDirectory = getElements(List.of(rootDirUuid), "user1", false, 200).get(0); From 543dac3fc89765826dfd249d4158e4cb9d2be4ee Mon Sep 17 00:00:00 2001 From: Hugo Marcellin Date: Mon, 6 Oct 2025 15:54:10 +0200 Subject: [PATCH 03/11] Set hasReadPermissions deeper --- .../gridsuite/directory/server/DirectoryService.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/gridsuite/directory/server/DirectoryService.java b/src/main/java/org/gridsuite/directory/server/DirectoryService.java index 177cfcfd..df69ccbd 100644 --- a/src/main/java/org/gridsuite/directory/server/DirectoryService.java +++ b/src/main/java/org/gridsuite/directory/server/DirectoryService.java @@ -274,21 +274,21 @@ public List getDirectoryElements(UUID directoryUuid, List hasReadPermissions(userId, List.of(e.getElementUuid()))).toList(); + return getAllDirectoryElementsStream(directoryUuid, types, userId).toList(); } } private Stream getOnlyElementsStream(UUID directoryUuid, List types, String userId) { - return getAllDirectoryElementsStream(directoryUuid, types) - .filter(elementAttributes -> !elementAttributes.getType().equals(DIRECTORY) && hasReadPermissions(userId, List.of(elementAttributes.getElementUuid()))); + return getAllDirectoryElementsStream(directoryUuid, types, userId) + .filter(elementAttributes -> !elementAttributes.getType().equals(DIRECTORY)); } - private Stream getAllDirectoryElementsStream(UUID directoryUuid, List types) { + private Stream getAllDirectoryElementsStream(UUID directoryUuid, List types, String userId) { List directoryElements = repositoryService.findAllByParentId(directoryUuid); Map subdirectoriesCountsMap = getSubDirectoriesCountsMap(types, directoryElements); return directoryElements .stream() - .filter(e -> e.getType().equals(DIRECTORY) || types.isEmpty() || types.contains(e.getType())) + .filter(e -> (e.getType().equals(DIRECTORY) || types.isEmpty() || types.contains(e.getType())) && hasReadPermissions(userId, List.of(e.getId()))) .map(e -> toElementAttributes(e, subdirectoriesCountsMap.getOrDefault(e.getId(), 0L))); } @@ -429,7 +429,7 @@ private void deleteElement(ElementAttributes elementAttributes, String userId) { } private void deleteSubElements(UUID elementUuid, String userId) { - getAllDirectoryElementsStream(elementUuid, List.of()).forEach(elementAttributes -> deleteElement(elementAttributes, userId)); + getAllDirectoryElementsStream(elementUuid, List.of(), userId).forEach(elementAttributes -> deleteElement(elementAttributes, userId)); } /** From b9ea629e7485121cc62b63371f78dbeb70ea0ef2 Mon Sep 17 00:00:00 2001 From: Hugo Marcellin Date: Tue, 7 Oct 2025 11:25:48 +0200 Subject: [PATCH 04/11] Add permission check to getSubDirectoriesCounts --- .../directory/server/DirectoryService.java | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/gridsuite/directory/server/DirectoryService.java b/src/main/java/org/gridsuite/directory/server/DirectoryService.java index df69ccbd..075b55c3 100644 --- a/src/main/java/org/gridsuite/directory/server/DirectoryService.java +++ b/src/main/java/org/gridsuite/directory/server/DirectoryService.java @@ -248,8 +248,8 @@ public void createElementInDirectoryPath(String directoryPath, ElementAttributes insertElement(elementAttributes, parentDirectoryUuid); } - private Map getSubDirectoriesCounts(List subDirectories, List types) { - List subdirectoriesCountsList = repositoryService.getSubDirectoriesCounts(subDirectories, types); + private Map getSubDirectoriesCounts(List subDirectories, List types, String userId) { + List subdirectoriesCountsList = repositoryService.getSubDirectoriesCounts(subDirectories, types).stream().filter(e -> hasReadPermissions(userId, List.of(e.getId()))).toList(); Map subdirectoriesCountsMap = new HashMap<>(); subdirectoriesCountsList.forEach(e -> subdirectoriesCountsMap.put(e.getId(), e.getCount())); return subdirectoriesCountsMap; @@ -285,7 +285,7 @@ private Stream getOnlyElementsStream(UUID directoryUuid, List private Stream getAllDirectoryElementsStream(UUID directoryUuid, List types, String userId) { List directoryElements = repositoryService.findAllByParentId(directoryUuid); - Map subdirectoriesCountsMap = getSubDirectoriesCountsMap(types, directoryElements); + Map subdirectoriesCountsMap = getSubDirectoriesCountsMap(types, directoryElements, userId); return directoryElements .stream() .filter(e -> (e.getType().equals(DIRECTORY) || types.isEmpty() || types.contains(e.getType())) && hasReadPermissions(userId, List.of(e.getId()))) @@ -299,14 +299,14 @@ public List getRootDirectories(List types, String use if (!roleService.isUserExploreAdmin()) { directoryElements = directoryElements.stream().filter(directoryElementEntity -> hasReadPermissions(userId, List.of(directoryElementEntity.getId()))).toList(); } - Map subdirectoriesCountsMap = getSubDirectoriesCountsMap(types, directoryElements); + Map subdirectoriesCountsMap = getSubDirectoriesCountsMap(types, directoryElements, userId); return directoryElements.stream() .map(e -> toElementAttributes(e, subdirectoriesCountsMap.getOrDefault(e.getId(), 0L))) .toList(); } - private Map getSubDirectoriesCountsMap(List types, List directoryElements) { - return getSubDirectoriesCounts(directoryElements.stream().map(DirectoryElementEntity::getId).toList(), types); + private Map getSubDirectoriesCountsMap(List types, List directoryElements, String userId) { + return getSubDirectoriesCounts(directoryElements.stream().map(DirectoryElementEntity::getId).toList(), types, userId); } public void updateElement(UUID elementUuid, ElementAttributes newElementAttributes, String userId) { @@ -502,15 +502,15 @@ public List getElements(List ids, boolean strictMode, L //if the user is not an admin we filter out elements he doesn't have the permission on if (!roleService.isUserExploreAdmin()) { elementEntities = elementEntities.stream().filter(directoryElementEntity -> - hasReadPermissions(userId, List.of(directoryElementEntity.getId())) - ).toList(); + hasReadPermissions(userId, List.of(directoryElementEntity.getId())) + ).toList(); } if (strictMode && elementEntities.size() != ids.stream().distinct().count()) { throw new DirectoryException(NOT_FOUND); } - Map subElementsCount = getSubDirectoriesCounts(elementEntities.stream().map(DirectoryElementEntity::getId).toList(), types); + Map subElementsCount = getSubDirectoriesCounts(elementEntities.stream().map(DirectoryElementEntity::getId).toList(), types, userId); return elementEntities.stream() .map(attribute -> toElementAttributes(attribute, subElementsCount.getOrDefault(attribute.getId(), 0L))) From 49b9bbb213dda27e4d7ebe49fb2882b71e02d463 Mon Sep 17 00:00:00 2001 From: Hugo Marcellin Date: Tue, 7 Oct 2025 12:53:06 +0200 Subject: [PATCH 05/11] Remove useless permission check to getSubDirectoriesCounts which is ineffective on count --- .../directory/server/DirectoryService.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/gridsuite/directory/server/DirectoryService.java b/src/main/java/org/gridsuite/directory/server/DirectoryService.java index 075b55c3..730cf58c 100644 --- a/src/main/java/org/gridsuite/directory/server/DirectoryService.java +++ b/src/main/java/org/gridsuite/directory/server/DirectoryService.java @@ -248,8 +248,8 @@ public void createElementInDirectoryPath(String directoryPath, ElementAttributes insertElement(elementAttributes, parentDirectoryUuid); } - private Map getSubDirectoriesCounts(List subDirectories, List types, String userId) { - List subdirectoriesCountsList = repositoryService.getSubDirectoriesCounts(subDirectories, types).stream().filter(e -> hasReadPermissions(userId, List.of(e.getId()))).toList(); + private Map getSubDirectoriesCounts(List subDirectories, List types) { + List subdirectoriesCountsList = repositoryService.getSubDirectoriesCounts(subDirectories, types); Map subdirectoriesCountsMap = new HashMap<>(); subdirectoriesCountsList.forEach(e -> subdirectoriesCountsMap.put(e.getId(), e.getCount())); return subdirectoriesCountsMap; @@ -285,7 +285,7 @@ private Stream getOnlyElementsStream(UUID directoryUuid, List private Stream getAllDirectoryElementsStream(UUID directoryUuid, List types, String userId) { List directoryElements = repositoryService.findAllByParentId(directoryUuid); - Map subdirectoriesCountsMap = getSubDirectoriesCountsMap(types, directoryElements, userId); + Map subdirectoriesCountsMap = getSubDirectoriesCountsMap(types, directoryElements); return directoryElements .stream() .filter(e -> (e.getType().equals(DIRECTORY) || types.isEmpty() || types.contains(e.getType())) && hasReadPermissions(userId, List.of(e.getId()))) @@ -299,14 +299,14 @@ public List getRootDirectories(List types, String use if (!roleService.isUserExploreAdmin()) { directoryElements = directoryElements.stream().filter(directoryElementEntity -> hasReadPermissions(userId, List.of(directoryElementEntity.getId()))).toList(); } - Map subdirectoriesCountsMap = getSubDirectoriesCountsMap(types, directoryElements, userId); + Map subdirectoriesCountsMap = getSubDirectoriesCountsMap(types, directoryElements); return directoryElements.stream() .map(e -> toElementAttributes(e, subdirectoriesCountsMap.getOrDefault(e.getId(), 0L))) .toList(); } - private Map getSubDirectoriesCountsMap(List types, List directoryElements, String userId) { - return getSubDirectoriesCounts(directoryElements.stream().map(DirectoryElementEntity::getId).toList(), types, userId); + private Map getSubDirectoriesCountsMap(List types, List directoryElements) { + return getSubDirectoriesCounts(directoryElements.stream().map(DirectoryElementEntity::getId).toList(), types); } public void updateElement(UUID elementUuid, ElementAttributes newElementAttributes, String userId) { @@ -510,7 +510,7 @@ public List getElements(List ids, boolean strictMode, L throw new DirectoryException(NOT_FOUND); } - Map subElementsCount = getSubDirectoriesCounts(elementEntities.stream().map(DirectoryElementEntity::getId).toList(), types, userId); + Map subElementsCount = getSubDirectoriesCounts(elementEntities.stream().map(DirectoryElementEntity::getId).toList(), types); return elementEntities.stream() .map(attribute -> toElementAttributes(attribute, subElementsCount.getOrDefault(attribute.getId(), 0L))) From 6b84bbb7b513db3a2a65037c975ee96255044b50 Mon Sep 17 00:00:00 2001 From: Hugo Marcellin Date: Tue, 7 Oct 2025 15:21:08 +0200 Subject: [PATCH 06/11] Rewrite getSubDirectoriesCounts --- .../directory/server/DirectoryService.java | 25 +++++++++++++------ .../DirectoryElementRepository.java | 12 +++------ .../services/DirectoryRepositoryService.java | 8 +++--- 3 files changed, 24 insertions(+), 21 deletions(-) diff --git a/src/main/java/org/gridsuite/directory/server/DirectoryService.java b/src/main/java/org/gridsuite/directory/server/DirectoryService.java index 730cf58c..46caffe3 100644 --- a/src/main/java/org/gridsuite/directory/server/DirectoryService.java +++ b/src/main/java/org/gridsuite/directory/server/DirectoryService.java @@ -248,10 +248,19 @@ public void createElementInDirectoryPath(String directoryPath, ElementAttributes insertElement(elementAttributes, parentDirectoryUuid); } - private Map getSubDirectoriesCounts(List subDirectories, List types) { - List subdirectoriesCountsList = repositoryService.getSubDirectoriesCounts(subDirectories, types); + private Map getSubDirectoriesCounts(List subDirectories, List types, String userId) { Map subdirectoriesCountsMap = new HashMap<>(); - subdirectoriesCountsList.forEach(e -> subdirectoriesCountsMap.put(e.getId(), e.getCount())); + Map> descendants = repositoryService.findAllByParentIdInAndTypeIn(subDirectories, types).stream() + .collect(Collectors.groupingBy(DirectoryElementEntity::getParentId)); + for (Map.Entry> entry : descendants.entrySet()) { + UUID parentId = entry.getKey(); + List childElements = entry.getValue(); + long readableCount = childElements.stream() + .filter(child -> hasReadPermissions(userId, List.of(child.getId()))) + .count(); + + subdirectoriesCountsMap.put(parentId, readableCount); + } return subdirectoriesCountsMap; } @@ -285,7 +294,7 @@ private Stream getOnlyElementsStream(UUID directoryUuid, List private Stream getAllDirectoryElementsStream(UUID directoryUuid, List types, String userId) { List directoryElements = repositoryService.findAllByParentId(directoryUuid); - Map subdirectoriesCountsMap = getSubDirectoriesCountsMap(types, directoryElements); + Map subdirectoriesCountsMap = getSubDirectoriesCountsMap(types, directoryElements, userId); return directoryElements .stream() .filter(e -> (e.getType().equals(DIRECTORY) || types.isEmpty() || types.contains(e.getType())) && hasReadPermissions(userId, List.of(e.getId()))) @@ -299,14 +308,14 @@ public List getRootDirectories(List types, String use if (!roleService.isUserExploreAdmin()) { directoryElements = directoryElements.stream().filter(directoryElementEntity -> hasReadPermissions(userId, List.of(directoryElementEntity.getId()))).toList(); } - Map subdirectoriesCountsMap = getSubDirectoriesCountsMap(types, directoryElements); + Map subdirectoriesCountsMap = getSubDirectoriesCountsMap(types, directoryElements, userId); return directoryElements.stream() .map(e -> toElementAttributes(e, subdirectoriesCountsMap.getOrDefault(e.getId(), 0L))) .toList(); } - private Map getSubDirectoriesCountsMap(List types, List directoryElements) { - return getSubDirectoriesCounts(directoryElements.stream().map(DirectoryElementEntity::getId).toList(), types); + private Map getSubDirectoriesCountsMap(List types, List directoryElements, String userId) { + return getSubDirectoriesCounts(directoryElements.stream().map(DirectoryElementEntity::getId).toList(), types, userId); } public void updateElement(UUID elementUuid, ElementAttributes newElementAttributes, String userId) { @@ -510,7 +519,7 @@ public List getElements(List ids, boolean strictMode, L throw new DirectoryException(NOT_FOUND); } - Map subElementsCount = getSubDirectoriesCounts(elementEntities.stream().map(DirectoryElementEntity::getId).toList(), types); + Map subElementsCount = getSubDirectoriesCounts(elementEntities.stream().map(DirectoryElementEntity::getId).toList(), types, userId); return elementEntities.stream() .map(attribute -> toElementAttributes(attribute, subElementsCount.getOrDefault(attribute.getId(), 0L))) diff --git a/src/main/java/org/gridsuite/directory/server/repository/DirectoryElementRepository.java b/src/main/java/org/gridsuite/directory/server/repository/DirectoryElementRepository.java index 90564827..b983713e 100644 --- a/src/main/java/org/gridsuite/directory/server/repository/DirectoryElementRepository.java +++ b/src/main/java/org/gridsuite/directory/server/repository/DirectoryElementRepository.java @@ -57,15 +57,6 @@ public interface DirectoryElementRepository extends JpaRepository getSubDirectoriesCounts(List subDirectories, List elementTypes); - @Transactional void deleteById(UUID id); @@ -101,4 +92,7 @@ interface SubDirectoryCount { "WHERE e.id IN (SELECT dh.element_id FROM DescendantHierarchy dh) AND e.id != :elementId" ) List findAllDescendants(@Param("elementId") UUID elementId); + + @Query("SELECT d FROM DirectoryElementEntity d WHERE d.parentId IN :parentIds AND (d.type = 'DIRECTORY' OR d.type IN :elementTypes)") + List findAllByParentIdsAndElementTypes(List parentIds, List elementTypes); } diff --git a/src/main/java/org/gridsuite/directory/server/services/DirectoryRepositoryService.java b/src/main/java/org/gridsuite/directory/server/services/DirectoryRepositoryService.java index cbfb71dc..f1f490e9 100644 --- a/src/main/java/org/gridsuite/directory/server/services/DirectoryRepositoryService.java +++ b/src/main/java/org/gridsuite/directory/server/services/DirectoryRepositoryService.java @@ -105,10 +105,6 @@ public UUID getParentUuid(UUID elementUuid) { .orElse(null); } - public List getSubDirectoriesCounts(List subDirectories, List elementTypes) { - return directoryElementRepository.getSubDirectoriesCounts(subDirectories, elementTypes); - } - public List findAllByIdIn(List uuids) { return directoryElementRepository.findAllByIdIn(uuids); } @@ -117,6 +113,10 @@ public List findAllByParentId(UUID parentId) { return directoryElementRepository.findAllByParentId(parentId); } + public List findAllByParentIdInAndTypeIn(List parentIds, List types) { + return directoryElementRepository.findAllByParentIdsAndElementTypes(parentIds, types); + } + public List findRootDirectories() { return directoryElementRepository.findRootDirectories(); } From e8d5488317938a85386f8cb5d05a8d57853a9f70 Mon Sep 17 00:00:00 2001 From: Hugo Marcellin Date: Tue, 7 Oct 2025 15:29:26 +0200 Subject: [PATCH 07/11] Simplify --- .../directory/server/DirectoryService.java | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/gridsuite/directory/server/DirectoryService.java b/src/main/java/org/gridsuite/directory/server/DirectoryService.java index 46caffe3..5320ddb5 100644 --- a/src/main/java/org/gridsuite/directory/server/DirectoryService.java +++ b/src/main/java/org/gridsuite/directory/server/DirectoryService.java @@ -249,19 +249,12 @@ public void createElementInDirectoryPath(String directoryPath, ElementAttributes } private Map getSubDirectoriesCounts(List subDirectories, List types, String userId) { - Map subdirectoriesCountsMap = new HashMap<>(); - Map> descendants = repositoryService.findAllByParentIdInAndTypeIn(subDirectories, types).stream() - .collect(Collectors.groupingBy(DirectoryElementEntity::getParentId)); - for (Map.Entry> entry : descendants.entrySet()) { - UUID parentId = entry.getKey(); - List childElements = entry.getValue(); - long readableCount = childElements.stream() - .filter(child -> hasReadPermissions(userId, List.of(child.getId()))) - .count(); - - subdirectoriesCountsMap.put(parentId, readableCount); - } - return subdirectoriesCountsMap; + return repositoryService.findAllByParentIdInAndTypeIn(subDirectories, types).stream() + .filter(child -> hasReadPermissions(userId, List.of(child.getId()))) + .collect(Collectors.groupingBy( + DirectoryElementEntity::getParentId, + Collectors.counting() + )); } public List getDirectoryElements(UUID directoryUuid, List types, Boolean recursive, String userId) { From b3bf66acc6616e8f0767d347bfd1ce4f197e0921 Mon Sep 17 00:00:00 2001 From: Hugo Marcellin Date: Tue, 7 Oct 2025 15:52:52 +0200 Subject: [PATCH 08/11] Reduce memory overhead by loading only element's id and their parent id --- .../org/gridsuite/directory/server/DirectoryService.java | 2 +- .../server/repository/DirectoryElementRepository.java | 9 +++++++-- .../server/services/DirectoryRepositoryService.java | 2 +- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/gridsuite/directory/server/DirectoryService.java b/src/main/java/org/gridsuite/directory/server/DirectoryService.java index 5320ddb5..d5f422f1 100644 --- a/src/main/java/org/gridsuite/directory/server/DirectoryService.java +++ b/src/main/java/org/gridsuite/directory/server/DirectoryService.java @@ -252,7 +252,7 @@ private Map getSubDirectoriesCounts(List subDirectories, List< return repositoryService.findAllByParentIdInAndTypeIn(subDirectories, types).stream() .filter(child -> hasReadPermissions(userId, List.of(child.getId()))) .collect(Collectors.groupingBy( - DirectoryElementEntity::getParentId, + DirectoryElementRepository.ElementParentage::getParentId, Collectors.counting() )); } diff --git a/src/main/java/org/gridsuite/directory/server/repository/DirectoryElementRepository.java b/src/main/java/org/gridsuite/directory/server/repository/DirectoryElementRepository.java index b983713e..c1d6f0e9 100644 --- a/src/main/java/org/gridsuite/directory/server/repository/DirectoryElementRepository.java +++ b/src/main/java/org/gridsuite/directory/server/repository/DirectoryElementRepository.java @@ -93,6 +93,11 @@ public interface DirectoryElementRepository extends JpaRepository findAllDescendants(@Param("elementId") UUID elementId); - @Query("SELECT d FROM DirectoryElementEntity d WHERE d.parentId IN :parentIds AND (d.type = 'DIRECTORY' OR d.type IN :elementTypes)") - List findAllByParentIdsAndElementTypes(List parentIds, List elementTypes); + interface ElementParentage { + UUID getId(); + UUID getParentId(); + } + + @Query("SELECT d.id AS id, d.parentId AS parentId FROM DirectoryElementEntity d WHERE d.parentId IN :parentIds AND (d.type = 'DIRECTORY' OR d.type IN :elementTypes)") + List findAllByParentIdsAndElementTypes(List parentIds, List elementTypes); } diff --git a/src/main/java/org/gridsuite/directory/server/services/DirectoryRepositoryService.java b/src/main/java/org/gridsuite/directory/server/services/DirectoryRepositoryService.java index f1f490e9..ea04ca2d 100644 --- a/src/main/java/org/gridsuite/directory/server/services/DirectoryRepositoryService.java +++ b/src/main/java/org/gridsuite/directory/server/services/DirectoryRepositoryService.java @@ -113,7 +113,7 @@ public List findAllByParentId(UUID parentId) { return directoryElementRepository.findAllByParentId(parentId); } - public List findAllByParentIdInAndTypeIn(List parentIds, List types) { + public List findAllByParentIdInAndTypeIn(List parentIds, List types) { return directoryElementRepository.findAllByParentIdsAndElementTypes(parentIds, types); } From 0af499b532c9e36f426bde382ee11888ec7c689b Mon Sep 17 00:00:00 2001 From: Hugo Marcellin Date: Tue, 7 Oct 2025 15:55:58 +0200 Subject: [PATCH 09/11] Checkstyle --- .../directory/server/repository/DirectoryElementRepository.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/org/gridsuite/directory/server/repository/DirectoryElementRepository.java b/src/main/java/org/gridsuite/directory/server/repository/DirectoryElementRepository.java index c1d6f0e9..23857e7b 100644 --- a/src/main/java/org/gridsuite/directory/server/repository/DirectoryElementRepository.java +++ b/src/main/java/org/gridsuite/directory/server/repository/DirectoryElementRepository.java @@ -95,6 +95,7 @@ public interface DirectoryElementRepository extends JpaRepository Date: Fri, 10 Oct 2025 09:02:23 +0200 Subject: [PATCH 10/11] add unit test for the issue Signed-off-by: David BRAQUART --- .../server/AccessRightsControlTest.java | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/src/test/java/org/gridsuite/directory/server/AccessRightsControlTest.java b/src/test/java/org/gridsuite/directory/server/AccessRightsControlTest.java index a5aaed1b..3f617d0a 100644 --- a/src/test/java/org/gridsuite/directory/server/AccessRightsControlTest.java +++ b/src/test/java/org/gridsuite/directory/server/AccessRightsControlTest.java @@ -80,6 +80,9 @@ public class AccessRightsControlTest { @Autowired private DirectoryElementRepository directoryElementRepository; + @Autowired + private DirectoryService directoryService; + @Autowired private PermissionRepository permissionRepository; @@ -318,6 +321,36 @@ public void testExistence() throws Exception { insertSubElement(dirUuid1, toElementAttributes(null, "elementName1", TYPE_01, ADMIN_USER), HttpStatus.CONFLICT); } + @Test + public void testDirectoryContentWithPermissions() throws Exception { + // user1 owns a dir with 2 sub-dirs + UUID rootUuid1 = insertRootDirectory("user1", "root1"); + UUID dirUuid1 = insertSubElement(rootUuid1, toElementAttributes(null, "dir1", DIRECTORY, "user1")); + UUID dirUuid2 = insertSubElement(rootUuid1, toElementAttributes(null, "dir2", DIRECTORY, "user1")); + + // both user1 ans user2 can see them + assertEquals(2, directoryService.getDirectoryElements(rootUuid1, List.of(DIRECTORY), false, "user1").size()); + assertEquals(2, directoryService.getDirectoryElements(rootUuid1, List.of(DIRECTORY), false, "user2").size()); + + // user1 restraints access to dir2 + List newPermissions = List.of( + new PermissionDTO(false, List.of(), PermissionType.READ), + new PermissionDTO(false, List.of(), PermissionType.WRITE) + ); + updateDirectoryPermissions("user1", dirUuid2, newPermissions).andExpect(status().isOk()); + + // only user1 still sees 2 sub-dirs ; users2 only 1 + assertEquals(2, directoryService.getDirectoryElements(rootUuid1, List.of(DIRECTORY), false, "user1").size()); + assertEquals(1, directoryService.getDirectoryElements(rootUuid1, List.of(DIRECTORY), false, "user2").size()); + + // user1 restraints access to dir1 + updateDirectoryPermissions("user1", dirUuid1, newPermissions).andExpect(status().isOk()); + + // only user1 still sees 2 sub-dirs ; users2 nothing + assertEquals(2, directoryService.getDirectoryElements(rootUuid1, List.of(DIRECTORY), false, "user1").size()); + assertEquals(0, directoryService.getDirectoryElements(rootUuid1, List.of(DIRECTORY), false, "user2").size()); + } + @Test public void testSetDirectoryPermissions() throws Exception { UUID rootDirectoryUuid = insertRootDirectory(ADMIN_USER, "root1"); From 32bea87dc8ece6dba3824ed567a6f09279265367 Mon Sep 17 00:00:00 2001 From: Hugo Marcellin Date: Fri, 10 Oct 2025 10:48:00 +0200 Subject: [PATCH 11/11] Check directories permission before retrieving their elements in getSubDirectoriesCounts --- .../java/org/gridsuite/directory/server/DirectoryService.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/gridsuite/directory/server/DirectoryService.java b/src/main/java/org/gridsuite/directory/server/DirectoryService.java index d5f422f1..e4265faf 100644 --- a/src/main/java/org/gridsuite/directory/server/DirectoryService.java +++ b/src/main/java/org/gridsuite/directory/server/DirectoryService.java @@ -249,7 +249,8 @@ public void createElementInDirectoryPath(String directoryPath, ElementAttributes } private Map getSubDirectoriesCounts(List subDirectories, List types, String userId) { - return repositoryService.findAllByParentIdInAndTypeIn(subDirectories, types).stream() + List readableSubDirectories = subDirectories.stream().filter(dirId -> hasReadPermissions(userId, List.of(dirId))).toList(); + return repositoryService.findAllByParentIdInAndTypeIn(readableSubDirectories, types).stream() .filter(child -> hasReadPermissions(userId, List.of(child.getId()))) .collect(Collectors.groupingBy( DirectoryElementRepository.ElementParentage::getParentId,