Merge pull request #1167 from girder/avoid-unionwith

Avoid using $unionWith as it isn't supported some places
girder · May 23, 2023 · 3304446 · 3304446
2 parents 40a5490 + 9360feb
commit 3304446
Show file tree

Hide file tree

Showing 3 changed files with 115 additions and 37 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,10 +9,13 @@
 ### Changes
 - Rename tiff exceptions to be better follow python standards ([#1162](../../pull/1162))
 - Require a newer version of girder ([#1163](../../pull/1163))
+- Add manual mongo index names where index names might get too long ([#1166](../../pull/1166))
+- Avoid using $unionWith for annotation searches as it isn't supported everywhere ([#1167](../../pull/1167))
 
 ### Bug Fixes
 - The deepzoom tile source misreported the format of its tile output ([#1158](../../pull/1158))
 - Guard against errors in a log message ([#1164](../../pull/1164))
+- Fix thumbnail query typo ([#1165](../../pull/1165))
 
 ## 1.20.6
 

diff --git a/girder_annotation/girder_large_image_annotation/rest/annotation.py b/girder_annotation/girder_large_image_annotation/rest/annotation.py
@@ -582,20 +582,7 @@ def deleteItemAnnotations(self, item):
 
     def getFolderAnnotations(self, id, recurse, user, limit=False, offset=False, sort=False,
                              sortDir=False, count=False):
-        recursivePipeline = [
-            {'$graphLookup': {
-                'from': 'folder',
-                'startWith': '$_id',
-                'connectFromField': '_id',
-                'connectToField': 'parentId',
-                'as': '__children'
-            }},
-            {'$unwind': {'path': '$__children'}},
-            {'$replaceRoot': {'newRoot': '$__children'}},
-            {'$unionWith': {
-                'coll': 'folder',
-                'pipeline': [{'$match': {'_id': ObjectId(id)}}]
-            }}] if recurse else []
+
         accessPipeline = [
             {'$match': {
                 '$or': [
@@ -612,36 +599,59 @@ def getFolderAnnotations(self, id, recurse, user, limit=False, offset=False, sor
                 ]
             }}
         ] if not user['admin'] else []
-        pipeline = [
-            {'$match': {'_id': 'none'}},
-            {'$unionWith': {
-                'coll': 'folder',
-                'pipeline': [{'$match': {'_id': ObjectId(id)}}] +
-                recursivePipeline +
-                [{'$lookup': {
-                    'from': 'item',
-                    'localField': '_id',
-                    'foreignField': 'folderId',
-                    'as': '__items'
-                }}, {'$lookup': {
-                    'from': 'annotation',
-                    'localField': '__items._id',
-                    'foreignField': 'itemId',
-                    'as': '__annotations'
-                }}, {'$unwind': '$__annotations'},
-                    {'$replaceRoot': {'newRoot': '$__annotations'}},
-                    {'$match': {'_active': {'$ne': False}}}
-                ] + accessPipeline
+        recursivePipeline = [
+            {'$match': {'_id': ObjectId(id)}},
+            {'$facet': {
+                'documents1': [{'$match': {'_id': ObjectId(id)}}],
+                'documents2': [
+                    {'$graphLookup': {
+                        'from': 'folder',
+                        'startWith': '$_id',
+                        'connectFromField': '_id',
+                        'connectToField': 'parentId',
+                        'as': '__children'
+                    }},
+                    {'$unwind': {'path': '$__children'}},
+                    {'$replaceRoot': {'newRoot': '$__children'}}
+                ]
             }},
-        ]
+            {'$project': {'__children': {'$concatArrays': [
+                '$documents1', '$documents2'
+            ]}}},
+            {'$unwind': {'path': '$__children'}},
+            {'$replaceRoot': {'newRoot': '$__children'}}
+        ] if recurse else [{'$match': {'_id': ObjectId(id)}}]
+
+        # We are only finding anntoations that we can change the permissions
+        # on.  If we wanted to expose annotations based on a permissions level,
+        # we need to add a folder access pipeline immediately after the
+        # recursivePipleine that for write and above would include the
+        # ANNOTATION_ACCSESS_FLAG
+        pipeline = recursivePipeline + [
+            {'$lookup': {
+                'from': 'item',
+                'localField': '_id',
+                'foreignField': 'folderId',
+                'as': '__items'
+            }},
+            {'$lookup': {
+                'from': 'annotation',
+                'localField': '__items._id',
+                'foreignField': 'itemId',
+                'as': '__annotations'
+            }},
+            {'$unwind': '$__annotations'},
+            {'$replaceRoot': {'newRoot': '$__annotations'}},
+            {'$match': {'_active': {'$ne': False}}}
+        ] + accessPipeline
+
         if count:
             pipeline += [{'$count': 'count'}]
         else:
             pipeline = pipeline + [{'$sort': {sort: sortDir}}] if sort else pipeline
             pipeline = pipeline + [{'$skip': offset}] if offset else pipeline
             pipeline = pipeline + [{'$limit': limit}] if limit else pipeline
-
-        return Annotation().collection.aggregate(pipeline)
+        return Folder().collection.aggregate(pipeline)
 
     @autoDescribeRoute(
         Description('Check if the user owns any annotations for the items in a folder')

diff --git a/girder_annotation/test_annotation/test_annotations_rest.py b/girder_annotation/test_annotation/test_annotations_rest.py
@@ -16,6 +16,8 @@
     from girder_large_image_annotation.models.annotation import Annotation
 
     from girder.constants import AccessType
+    from girder.models.collection import Collection
+    from girder.models.folder import Folder
     from girder.models.item import Item
     from girder.models.setting import Setting
 except ImportError:
@@ -808,3 +810,66 @@ def testMetadataSearch(server, admin, fsAssetstore):
         params={'q': 'key:key2 value', 'mode': 'li_annotation_metadata', 'types': '["item"]'})
     assert utilities.respStatus(resp) == 200
     assert len(resp.json['item']) == 0
+
+
+@pytest.mark.usefixtures('unbindLargeImage', 'unbindAnnotation')
+@pytest.mark.plugin('large_image_annotation')
+def testFolderEndpoints(server, admin, user):
+    collection = Collection().createCollection(
+        'collection A', user)
+    colFolderA = Folder().createFolder(
+        collection, 'folder A', parentType='collection',
+        creator=user)
+    colFolderB = Folder().createFolder(
+        colFolderA, 'folder B', creator=user)
+    colFolderC = Folder().createFolder(
+        colFolderA, 'folder C', creator=admin, public=False)
+    colFolderC = Folder().setAccessList(colFolderC, access={'users': [], 'groups': []}, save=True)
+    itemA1 = Item().createItem('sample A1', user, colFolderA)
+    itemA2 = Item().createItem('sample A1', user, colFolderA)
+    itemB1 = Item().createItem('sample B1', user, colFolderB)
+    itemB2 = Item().createItem('sample B1', user, colFolderB)
+    itemC1 = Item().createItem('sample C1', admin, colFolderC)
+    itemC2 = Item().createItem('sample C1', admin, colFolderC)
+    Annotation().createAnnotation(itemA1, user, sampleAnnotation)
+    ann = Annotation().createAnnotation(itemA1, admin, sampleAnnotation, public=False)
+    Annotation().setAccessList(ann, access={'users': [], 'groups': []}, save=True)
+    Annotation().createAnnotation(itemA2, user, sampleAnnotation)
+    Annotation().createAnnotation(itemB1, user, sampleAnnotation)
+    ann = Annotation().createAnnotation(itemB1, admin, sampleAnnotation, public=False)
+    Annotation().setAccessList(ann, access={'users': [], 'groups': []}, save=True)
+    Annotation().createAnnotation(itemB2, user, sampleAnnotation)
+    Annotation().createAnnotation(itemC1, user, sampleAnnotation)
+    ann = Annotation().createAnnotation(itemC1, admin, sampleAnnotation, public=False)
+    Annotation().setAccessList(ann, access={'users': [], 'groups': []}, save=True)
+    Annotation().createAnnotation(itemC2, user, sampleAnnotation)
+
+    resp = server.request(
+        path='/annotation/folder/' + str(colFolderA['_id']), user=admin,
+        params={'recurse': False})
+    assert utilities.respStatus(resp) == 200
+    assert len(resp.json) == 3
+
+    resp = server.request(
+        path='/annotation/folder/' + str(colFolderA['_id']), user=admin,
+        params={'recurse': True})
+    assert utilities.respStatus(resp) == 200
+    assert len(resp.json) == 9
+
+    resp = server.request(
+        path='/annotation/folder/' + str(colFolderA['_id']), user=user,
+        params={'recurse': False})
+    assert utilities.respStatus(resp) == 200
+    assert len(resp.json) == 2
+
+    resp = server.request(
+        path='/annotation/folder/' + str(colFolderA['_id']), user=user,
+        params={'recurse': True})
+    assert utilities.respStatus(resp) == 200
+    assert len(resp.json) == 6
+
+    resp = server.request(
+        path='/annotation/folder/' + str(colFolderC['_id']), user=user,
+        params={'recurse': True})
+    assert utilities.respStatus(resp) == 200
+    assert len(resp.json) == 2