From 9360feb96ddd54c619905a0122dbf1e57e29ddf4 Mon Sep 17 00:00:00 2001 From: David Manthey Date: Mon, 22 May 2023 17:00:03 -0400 Subject: [PATCH] Avoid using $unionWith as it isn't supported some places --- CHANGELOG.md | 3 + .../rest/annotation.py | 84 +++++++++++-------- .../test_annotation/test_annotations_rest.py | 65 ++++++++++++++ 3 files changed, 115 insertions(+), 37 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 59af0e533..da85b6565 100644 --- 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 index 98d25fe17..6483370e0 100644 --- 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 index 8adfbb262..ca6b26a78 100644 --- 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