Fix an issue with annotation permissions.

If a user or group has been removed from the system such that it still
exists in an access control list, when the access list is read, Girder
prunes the missing users and groups and writes back the access list.
However, for annotations, this could cause a save with an empty array of
elements, effectively removing the elements from the annotation.  This,
instead, updates the annotation's access list.

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

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')