Implement an Obj DELETE handler (skyportal#2047)

Author: kmshin1397

Diff for: config.yaml.defaults

@@ -285,6 +285,8 @@ misc:
   # consider a photometry point as a detection
   photometry_detection_threshold_nsigma: 3.0
 
+  allow_nonadmins_delete_objs: False
+
 weather:
   # time in seconds to wait before fetching weather for a given telescope
   refresh_time: 3600.0

Diff for: skyportal/app_server.py

@@ -32,6 +32,7 @@
     ObservingRunHandler,
     PhotometryHandler,
     BulkDeletePhotometryHandler,
+    ObjHandler,
     ObjPhotometryHandler,
     ObjClassificationHandler,
     ObjAnnotationHandler,
@@ -131,6 +132,7 @@ def log_request(self, handler):
     (r'/api/invitations(/.*)?', InvitationHandler),
     (r'/api/newsfeed', NewsFeedHandler),
     (r'/api/observing_run(/[0-9]+)?', ObservingRunHandler),
+    (r'/api/objs(/[0-9A-Za-z-_\.\+]+)', ObjHandler),
     (r'/api/photometry(/[0-9]+)?', PhotometryHandler),
     (r'/api/sharing', SharingHandler),
     (r'/api/photometry/bulk_delete/(.*)', BulkDeletePhotometryHandler),

Diff for: skyportal/handlers/api/__init__.py

@@ -32,6 +32,7 @@
 from .color_mag import ObjColorMagHandler
 from .public_group import PublicGroupHandler
 from .roles import RoleHandler, UserRoleHandler
+from .obj import ObjHandler
 from .sharing import SharingHandler
 from .source import (
     SourceHandler,

Diff for: skyportal/handlers/api/obj.py

@@ -0,0 +1,51 @@
+from baselayer.app.access import auth_or_token, AccessError
+from baselayer.app.env import load_env
+from ..base import BaseHandler
+from ...models import DBSession, Obj
+
+_, cfg = load_env()
+
+
+class ObjHandler(BaseHandler):
+    @auth_or_token  # ACLs will be checked below based on configs
+    def delete(self, obj_id):
+        """
+        ---
+        description: Delete an Obj
+        tags:
+          - objs
+        parameters:
+          - in: path
+            name: obj_id
+            required: true
+            schema:
+              type: string
+        responses:
+          200:
+            content:
+              application/json:
+                schema: Success
+          400:
+            content:
+              application/json:
+                schema: Error
+        """
+        try:
+            obj = Obj.get_if_accessible_by(
+                obj_id,
+                self.current_user,
+                mode='delete',
+                raise_if_none=True,
+            )
+            DBSession().delete(obj)
+            self.verify_and_commit()
+        except AccessError as e:
+            error_msg = (
+                "Insufficient permissions: Objs may only be deleted by system admins"
+            )
+            if cfg["misc.allow_nonadmins_delete_objs"]:
+                error_msg += " or by users who own all data associated with the Obj"
+
+            return self.error(f"{error_msg}. (Original exception: {e})")
+
+        return self.success()

Diff for: skyportal/models.py

@@ -637,11 +637,110 @@ def token_groups(self):
 Token.groups = token_groups
 
 
+def delete_obj_if_all_data_owned(cls, user_or_token):
+    allow_nonadmins = cfg["misc.allow_nonadmins_delete_objs"] or False
+
+    deletable_photometry = Photometry.query_records_accessible_by(
+        user_or_token, mode="delete"
+    ).subquery()
+    nondeletable_photometry = (
+        DBSession()
+        .query(Photometry.obj_id)
+        .join(
+            deletable_photometry,
+            deletable_photometry.c.id == Photometry.id,
+            isouter=True,
+        )
+        .filter(deletable_photometry.c.id.is_(None))
+        .distinct(Photometry.obj_id)
+        .subquery()
+    )
+
+    deletable_spectra = Spectrum.query_records_accessible_by(
+        user_or_token, mode="delete"
+    ).subquery()
+    nondeletable_spectra = (
+        DBSession()
+        .query(Spectrum.obj_id)
+        .join(
+            deletable_spectra,
+            deletable_spectra.c.id == Spectrum.id,
+            isouter=True,
+        )
+        .filter(deletable_spectra.c.id.is_(None))
+        .distinct(Spectrum.obj_id)
+        .subquery()
+    )
+
+    deletable_candidates = Candidate.query_records_accessible_by(
+        user_or_token, mode="delete"
+    ).subquery()
+    nondeletable_candidates = (
+        DBSession()
+        .query(Candidate.obj_id)
+        .join(
+            deletable_candidates,
+            deletable_candidates.c.id == Candidate.id,
+            isouter=True,
+        )
+        .filter(deletable_candidates.c.id.is_(None))
+        .distinct(Candidate.obj_id)
+        .subquery()
+    )
+
+    deletable_sources = Source.query_records_accessible_by(
+        user_or_token, mode="delete"
+    ).subquery()
+    nondeletable_sources = (
+        DBSession()
+        .query(Source.obj_id)
+        .join(
+            deletable_sources,
+            deletable_sources.c.id == Source.id,
+            isouter=True,
+        )
+        .filter(deletable_sources.c.id.is_(None))
+        .distinct(Source.obj_id)
+        .subquery()
+    )
+
+    return (
+        DBSession()
+        .query(cls)
+        .join(
+            nondeletable_photometry,
+            nondeletable_photometry.c.obj_id == cls.id,
+            isouter=True,
+        )
+        .filter(nondeletable_photometry.c.obj_id.is_(None))
+        .join(
+            nondeletable_spectra,
+            nondeletable_spectra.c.obj_id == cls.id,
+            isouter=True,
+        )
+        .filter(nondeletable_spectra.c.obj_id.is_(None))
+        .join(
+            nondeletable_candidates,
+            nondeletable_candidates.c.obj_id == cls.id,
+            isouter=True,
+        )
+        .filter(nondeletable_candidates.c.obj_id.is_(None))
+        .join(
+            nondeletable_sources,
+            nondeletable_sources.c.obj_id == cls.id,
+            isouter=True,
+        )
+        .filter(nondeletable_sources.c.obj_id.is_(None))
+        .filter(sa.literal(allow_nonadmins))
+    )
+
+
 class Obj(Base, ha.Point):
     """A record of an astronomical Object and its metadata, such as position,
     positional uncertainties, name, and redshift."""
 
     update = public
+    delete = restricted | CustomUserAccessControl(delete_obj_if_all_data_owned)
 
     id = sa.Column(sa.String, primary_key=True, doc="Name of the object.")
     # TODO should this column type be decimal? fixed-precison numeric
@@ -721,7 +820,7 @@ class Obj(Base, ha.Point):
     candidates = relationship(
         'Candidate',
         back_populates='obj',
-        cascade='save-update, merge, refresh-expire, expunge',
+        cascade='save-update, merge, refresh-expire, expunge, delete',
         passive_deletes=True,
         order_by="Candidate.passed_at",
         doc="Candidates associated with the object.",
@@ -739,7 +838,7 @@ class Obj(Base, ha.Point):
     comments_on_spectra = relationship(
         'CommentOnSpectrum',
         back_populates='obj',
-        cascade='save-update, merge, refresh-expire, expunge',
+        cascade='save-update, merge, refresh-expire, expunge, delete',
         passive_deletes=True,
         order_by="CommentOnSpectrum.created_at",
         doc="Comments posted about spectra belonging to the object.",
@@ -766,7 +865,7 @@ class Obj(Base, ha.Point):
     photometry = relationship(
         'Photometry',
         back_populates='obj',
-        cascade='save-update, merge, refresh-expire, expunge',
+        cascade='save-update, merge, refresh-expire, expunge, delete',
         single_parent=True,
         passive_deletes=True,
         order_by="Photometry.mjd",
@@ -799,19 +898,22 @@ class Obj(Base, ha.Point):
     followup_requests = relationship(
         'FollowupRequest',
         back_populates='obj',
+        cascade='delete',
         passive_deletes=True,
         doc="Robotic follow-up requests of the object.",
     )
     assignments = relationship(
         'ClassicalAssignment',
         back_populates='obj',
+        cascade='delete',
         passive_deletes=True,
         doc="Assignments of the object to classical observing runs.",
     )
 
     obj_notifications = relationship(
         "SourceNotification",
         back_populates="source",
+        cascade='delete',
         passive_deletes=True,
         doc="Notifications regarding the object sent out by users",
     )
@@ -1309,12 +1411,14 @@ def source_create_access_logic(cls, user_or_token):
 Obj.sources = relationship(
     Source,
     back_populates='obj',
+    cascade='delete',
     passive_deletes=True,
     doc="Instances in which a group saved this Obj.",
 )
 Obj.candidates = relationship(
     Candidate,
     back_populates='obj',
+    cascade='delete',
     passive_deletes=True,
     doc="Instances in which this Obj passed a group's filter.",
 )

Diff for: skyportal/tests/api/test_objs.py

@@ -0,0 +1,61 @@
+from skyportal.tests import api
+
+
+def test_delete_obj_non_admin(
+    manage_sources_token,
+    public_obj,
+    public_source_no_data,
+    upload_data_token,
+    ztf_camera,
+    public_group,
+):
+    # A manage_sources_token user cannot delete an obj from ObjFactory since things
+    # like Photometry and Comments are created with other users as authors/owners
+    status, _ = api("DELETE", f"objs/{public_obj.id}", token=manage_sources_token)
+    assert status == 400
+
+    # Now start with a fresh Obj with no associated data, and post photometry to it
+    status, data = api(
+        'POST',
+        'photometry',
+        data={
+            'obj_id': str(public_source_no_data.id),
+            'mjd': 58000.0,
+            'instrument_id': ztf_camera.id,
+            'flux': 12.24,
+            'fluxerr': 0.031,
+            'zp': 25.0,
+            'magsys': 'ab',
+            'filter': 'ztfi',
+            'group_ids': [public_group.id],
+        },
+        token=upload_data_token,
+    )
+    assert status == 200
+    assert data['status'] == 'success'
+    photometry_id = data['data']['ids'][0]
+
+    # Since the owner is the upload_data_token user, the manage_sources_token user
+    # won't be able to delete this Obj either
+    status, _ = api(
+        "DELETE", f"objs/{public_source_no_data.id}", token=manage_sources_token
+    )
+    assert status == 400
+
+    # Now delete the photometry blocking the delete
+    status, data = api('DELETE', f'photometry/{photometry_id}', token=upload_data_token)
+    assert status == 200
+
+    # Now the manage_source_token user should be able to delete the Obj,
+    # since they are a member of the group the associated Source is saved to,
+    # that is the only data referencing the `public_source_no_data` Obj, and
+    # cfg['misc.allow_nonadmins_delete_objs'] is True.
+    status, _ = api(
+        "DELETE", f"objs/{public_source_no_data.id}", token=manage_sources_token
+    )
+    assert status == 200
+
+
+def test_delete_obj_system_admin(public_obj, super_admin_token):
+    status, _ = api("DELETE", f"objs/{public_obj.id}", token=super_admin_token)
+    assert status == 200

Diff for: skyportal/tests/fixtures.py

@@ -9,6 +9,7 @@
 import factory
 import numpy as np
 from sqlalchemy import inspect
+from sqlalchemy.orm.exc import ObjectDeletedError
 
 from baselayer.app.config import load_config
 from baselayer.app.env import load_env
@@ -62,8 +63,17 @@ def is_already_deleted(instance, table):
     if instance in DBSession() and inspect(instance).detached:
         return True
 
-    if instance not in DBSession():
-        return DBSession().query(table).filter(table.id == instance.id).first() is None
+    if instance not in DBSession() or (
+        instance in DBSession() and inspect(instance).expired
+    ):
+        try:
+            return (
+                DBSession().query(table).filter(table.id == instance.id).first() is None
+            )
+        except ObjectDeletedError:
+            # If instance was deleted by the test, it would have taken place in
+            # another transaction and thus undiscovered until the exception here
+            return True
 
     # If the instance is in the session and has not been detached (deleted + committed)
     # then it still requires some teardown actions.

Diff for: test_config.yaml

@@ -14,6 +14,7 @@ server:
 misc:
   photometry_detection_threshold_nsigma: 3.0
   minutes_to_keep_candidate_query_cache: 0.033333 # 2 seconds
+  allow_nonadmins_delete_objs: True
 
 twilio:
   # Twilio Sendgrid API configs