Merge pull request #1694 from girder/speed-up-recursive-items

Speed up recursive item lists
girder · Oct 18, 2024 · 311540d · 311540d
2 parents 8faa513 + 824a403
commit 311540d
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 13 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,11 @@
 # Change Log
 
+## 1.30.2
+
+### Improvements
+
+- Speed up recursive item lists ([#1694](../../pull/1694))
+
 ## 1.30.1
 
 ### Improvements

diff --git a/girder/girder_large_image/rest/__init__.py b/girder/girder_large_image/rest/__init__.py
@@ -75,10 +75,10 @@ def _groupingPipeline(initialPipeline, cbase, grouping, sort=None):
         {'keys': ['meta.dicom.PatientID'], 'counts': {
         'meta.dicom.StudyInstanceUID': 'meta._count.studycount',
         'meta.dicom.SeriesInstanceUID': 'meta._count.seriescount'}}
-    :param sort: an optional lost of (key, direction) tuples
+    :param sort: an optional list of (key, direction) tuples
     """
     for gidx, gr in enumerate(grouping['keys']):
-        grsort = [(gr, 1)] + (sort or [])
+        grsort = [(gr, 1)] + (sort or []) + [('_id', 1)]
         initialPipeline.extend([{
             '$match': {gr: {'$exists': True}},
         }, {
@@ -130,6 +130,7 @@ def _itemFindRecursive(  # noqa
     from bson.objectid import ObjectId
 
     if folderId:
+        user = self.getCurrentUser()
         if recurse:
             pipeline = [
                 {'$match': {'_id': ObjectId(folderId)}},
@@ -141,6 +142,7 @@ def _itemFindRecursive(  # noqa
                     'as': '_folder',
                     'startWith': '$_id',
                 }},
+                {'$match': Folder().permissionClauses(user, AccessType.READ, '_folder.')},
                 {'$group': {'_id': '$_folder._id'}},
             ]
             children = [ObjectId(folderId)] + next(Folder().collection.aggregate(pipeline))['_id']
@@ -155,22 +157,13 @@ def _itemFindRecursive(  # noqa
             if name:
                 filters['name'] = name
             filters['folderId'] = {'$in': children}
-            user = self.getCurrentUser()
             if isinstance(sort, list):
                 sort.append(('parentId', 1))
 
             # This is taken from girder.utility.acl_mixin.findWithPermissions,
             # except it adds a grouping stage
             initialPipeline = [
                 {'$match': filters},
-                {'$lookup': {
-                    'from': 'folder',
-                    'localField': Item().resourceParent,
-                    'foreignField': '_id',
-                    'as': '__parent',
-                }},
-                {'$match': Item().permissionClauses(user, AccessType.READ, '__parent.')},
-                {'$project': {'__parent': False}},
             ]
             if group is not None:
                 if not isinstance(group, list):
@@ -200,6 +193,7 @@ def _itemFindRecursive(  # noqa
             if offset:
                 fullPipeline.append({'$skip': offset})
 
+            print(fullPipeline)
             logger.debug('Find item pipeline %r', fullPipeline)
 
             options = {
@@ -220,8 +214,6 @@ def count():
             result.count = count
             result.fromAggregate = True
             return result
-
-            return Item().findWithPermissions(filters, offset, limit, sort=sort, user=user)
     return origItemFind(folderId, text, name, limit, offset, sort, filters)