Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid using $unionWith as it isn't supported some places #1167

Merged
merged 1 commit into from
May 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
84 changes: 47 additions & 37 deletions girder_annotation/girder_large_image_annotation/rest/annotation.py
Original file line number Diff line number Diff line change
Expand Up @@ -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': [
Expand All @@ -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')
Expand Down
65 changes: 65 additions & 0 deletions girder_annotation/test_annotation/test_annotations_rest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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