diff --git a/girder_annotation/girder_large_image_annotation/models/annotation.py b/girder_annotation/girder_large_image_annotation/models/annotation.py index 24cbc8d09..2bb6c61d0 100644 --- a/girder_annotation/girder_large_image_annotation/models/annotation.py +++ b/girder_annotation/girder_large_image_annotation/models/annotation.py @@ -490,12 +490,12 @@ def _copyAnnotationsFromOtherItem(self, srcItemId, destItem): continue annotation['itemId'] = destItemId del annotation['_id'] - # Remove existing permissionsi, then give it the same permissions + # Remove existing permissions, then give it the same permissions # as the item's folder. annotation.pop('access', None) self.copyAccessPolicies(destItem, annotation, save=False) self.setPublic(annotation, folder.get('public'), save=False) - Annotation().save(annotation) + self.save(annotation) count += 1 logger.info('Copied %d annotations from %s to %s ', count, srcItemId, destItemId) @@ -646,7 +646,8 @@ def remove(self, annotation, *args, **kwargs): # just mark the annotations as inactive result = self.update({'_id': annotation['_id']}, {'$set': {'_active': False}}) else: - delete_one = self.collection.delete_one + with self._writeLock: + delete_one = self.collection.delete_one def deleteElements(query, *args, **kwargs): ret = delete_one(query, *args, **kwargs) @@ -1001,3 +1002,18 @@ def injectAnnotationGroupSet(self, annotation): } self.collection.update_one(query, update) return annotation + + def setAccessList(self, doc, access, save=False, **kwargs): + """ + The super class's setAccessList function can save a document. However, + annotations which have not loaded elements lose their elements when + this occurs, because the validation step of the save function adds an + empty element list. By using an update instead of a save, this + prevents the problem. + """ + update = save and '_id' in doc + save = save and '_id' not in doc + doc = super(Annotation, self).setAccessList(doc, access, save=save, **kwargs) + if update: + self.update({'_id': doc['_id']}, {'$set': {'access': doc['access']}}) + return doc diff --git a/girder_annotation/test_annotation/test_annotations.py b/girder_annotation/test_annotation/test_annotations.py index 8d27c8f83..76e4a718b 100644 --- a/girder_annotation/test_annotation/test_annotations.py +++ b/girder_annotation/test_annotation/test_annotations.py @@ -11,6 +11,7 @@ from girder.constants import AccessType from girder.exceptions import AccessException, ValidationException from girder.models.item import Item +from girder.models.group import Group from girder.models.setting import Setting from girder_large_image_annotation.models import annotation @@ -338,6 +339,27 @@ def testRevertVersion(self, admin): assert len(Annotation().revertVersion( annot['_id'], user=admin)['annotation']['elements']) == 1 + def testPermissions(self, admin): + publicFolder = utilities.namedFolder(admin, 'Public') + item = Item().createItem('sample', admin, publicFolder) + annot = Annotation().createAnnotation(item, admin, sampleAnnotation) + group = Group().createGroup('Delete Me', admin) + annot['access']['groups'].append({'id': str(group['_id']), 'level': 0, 'flags': []}) + Annotation().save(annot) + annot = Annotation().load(annot['_id'], getElements=False, force=True) + acl = Annotation().getFullAccessList(annot) + assert len(annot['access']['groups']) == 1 + assert len(acl['groups']) == 1 + # If you remove the group using the remove method, the acls will be + # pruned. If you use removeWithQuery, they won't be, and getting the + # access list will cause the access list to be resaved. + Group().removeWithQuery({'_id': group['_id']}) + acl = Annotation().getFullAccessList(annot) + assert len(acl['groups']) == 0 + assert len(annot['access']['groups']) == 0 + check = Annotation().load(annot['_id'], force=True) + assert len(check['annotation']['elements']) > 0 + @pytest.mark.usefixtures('unbindLargeImage', 'unbindAnnotation') @pytest.mark.plugin('large_image_annotation')