Support recovery of deleted entities (fixes #88) (#156)

* Support recovery of deleted entities (fixes #88)

This commit adds an explicit way to restore deleted entities and an implicit way (i.e. an entity
gets un-deleted when changes are posted).

Co-authored-by: Gabriel Niebler <[email protected]>

backend/alembic/versions/5e46d4ff6f21_change_type.py

@@ -0,0 +1,24 @@
+"""ChangeType for restoring deleted items
+
+Revision ID: 5e46d4ff6f21
+Revises: 8a3f36ffa8df
+Create Date: 2023-09-25 15:28:13.752183
+
+"""
+from alembic import op
+import sqlalchemy as sa
+
+
+# revision identifiers, used by Alembic.
+revision = '5e46d4ff6f21'
+down_revision = '8a3f36ffa8df'
+branch_labels = None
+depends_on = None
+
+
+def upgrade():
+    op.execute("ALTER TYPE changetype ADD VALUE IF NOT EXISTS 'RESTORE'")
+
+
+def downgrade():
+    pass

backend/crud.py

@@ -88,10 +88,13 @@ def get_schema(db: Session, id_or_slug: Union[int, str]) -> Schema:
 
 def _check_bound_schema_id(db: Session, schema_id: int):
     try:
-        db.query(Schema.id).filter(Schema.id == schema_id, Schema.deleted == False).one()
+        schema = db.query(Schema).filter(Schema.id == schema_id).one()
     except NoResultFound:
         raise MissingSchemaException(obj_id=schema_id)
 
+    if schema.deleted:
+        raise SchemaIsDeletedException(obj_id=schema_id)
+
 
 def create_schema(db: Session, data: SchemaCreateSchema, commit: bool = True) -> Schema:
     try:
@@ -143,7 +146,7 @@ def create_schema(db: Session, data: SchemaCreateSchema, commit: bool = True) ->
 
 
 def delete_schema(db: Session, id_or_slug: Union[int, str], commit: bool = True) -> Schema:
-    q = select(Schema).where(Schema.deleted == False)
+    q = select(Schema)
     if isinstance(id_or_slug, int):
         q = q.where(Schema.id == id_or_slug)
     else:
@@ -152,6 +155,8 @@ def delete_schema(db: Session, id_or_slug: Union[int, str], commit: bool = True)
     schema = db.execute(q).scalar()
     if schema is None:
         raise MissingSchemaException(obj_id=id_or_slug)
+    if schema.deleted:
+        raise NoOpChangeException(f"Schema with id {schema.id} is already deleted")
 
     db.execute(update(Entity)
                .where(Entity.schema_id == schema.id, Entity.deleted == False)
@@ -645,10 +650,12 @@ def update_entity(db: Session, id_or_slug: Union[str, int], schema_id: int, data
     q = select(Entity).where(Entity.schema_id == schema_id)
     q = q.where(Entity.id == id_or_slug) if isinstance(id_or_slug, int) else q.where(Entity.slug == id_or_slug)
     e = db.execute(q).scalar()
-    if e is None or e.deleted:
+    if e is None:
         raise MissingEntityException(obj_id=id_or_slug)
     if e.schema.deleted:
         raise MissingSchemaException(obj_id=e.schema.id)
+    if e.deleted:
+        raise EntityIsDeletedException(obj_id=e.id)
 
     slug = data.pop('slug', e.slug)
     name = data.pop('name', e.name)
@@ -716,17 +723,38 @@ def update_entity(db: Session, id_or_slug: Union[str, int], schema_id: int, data
 
 
 def delete_entity(db: Session, id_or_slug: Union[int, str], schema_id: int, commit: bool = True) -> Entity:
-    q = select(Entity).where(Entity.deleted == False).where(Entity.schema_id == schema_id)
+    q = select(Entity).where(Entity.schema_id == schema_id)
     if isinstance(id_or_slug, int):
         q = q.where(Entity.id == id_or_slug)
     else:
         q = q.where(Entity.slug == id_or_slug)
     e = db.execute(q).scalar()
     if e is None:
         raise MissingEntityException(obj_id=id_or_slug)
+    if e.deleted:
+        raise NoOpChangeException(f"Entity with id {e.id} is already deleted")
     e.deleted = True
     if commit:
         db.commit()
     else:
         db.flush()
     return e
+
+
+def restore_entity(db: Session, id_or_slug: Union[int, str], schema_id: int, commit: bool = True) -> Entity:
+    q = select(Entity).where(Entity.schema_id == schema_id)
+    if isinstance(id_or_slug, int):
+        q = q.where(Entity.id == id_or_slug)
+    else:
+        q = q.where(Entity.slug == id_or_slug)
+    e = db.execute(q).scalar()
+    if e is None:
+        raise MissingEntityException(obj_id=id_or_slug)
+    if not e.deleted:
+        raise NoOpChangeException(f"Entity with id {e.id} is not deleted")
+    e.deleted = False
+    if commit:
+        db.commit()
+    else:
+        db.flush()
+    return e

backend/dynamic_routes.py

@@ -20,7 +20,7 @@
 
 from .traceability.entity import create_entity_create_request, create_entity_update_request, \
     create_entity_delete_request, apply_entity_create_request, apply_entity_update_request, \
-    apply_entity_delete_request
+    apply_entity_delete_request, create_entity_restore_request, apply_entity_restore_request
 
 
 factory = EntityModelFactory()
@@ -261,6 +261,7 @@ def route_update_entity(router: APIRouter, schema: Schema):
                 * there already exists an entity that has same value for unique field and this entity is not deleted
                 '''
             },
+            410: {"description": "Entity cannot be updated because it was deleted"},
             422: {
                 'description': '''Can be returned when:
 
@@ -288,6 +289,8 @@ def update_entity(id_or_slug: Union[int, str],
             return change_request
         except exceptions.NoOpChangeException as e:
             raise HTTPException(status.HTTP_208_ALREADY_REPORTED, str(e))
+        except exceptions.EntityIsDeletedException as e:
+            raise HTTPException(status.HTTP_410_GONE, str(e))
         except (exceptions.MissingEntityException, exceptions.MissingSchemaException) as e:
             raise HTTPException(status.HTTP_404_NOT_FOUND, str(e))
         except (exceptions.EntityExistsException, exceptions.UniqueValueException) as e:
@@ -314,25 +317,33 @@ def route_delete_entity(router: APIRouter, schema: Schema):
         responses={
             200: {"description": "Entity was deleted"},
             202: {"description": "Request to delete entity was stored"},
+            208: {"description": "Entity was not changed because request contained no changes"},
             404: {
                 'description': "entity with provided id/slug doesn't exist on current schema"
             }
         }
     )
     def delete_entity(id_or_slug: Union[int, str], response: Response,
+                      restore: bool = False,
                       db: Session = Depends(get_db),
                       user: User = Depends(req_permission)):
+        create_fun, apply_fun = create_entity_delete_request, apply_entity_delete_request
+        if restore:
+            create_fun, apply_fun = create_entity_restore_request, apply_entity_restore_request
+
         try:
-            change_request = create_entity_delete_request(
+            change_request = create_fun(
                 db=db, id_or_slug=id_or_slug, schema_id=schema.id, created_by=user, commit=False
             )
             if not schema.reviewable:
-                return apply_entity_delete_request(
+                return apply_fun(
                     db=db, change_request=change_request, reviewed_by=user, comment='Autosubmit'
                 )[1]
             db.commit()
             response.status_code = status.HTTP_202_ACCEPTED
             return change_request
+        except exceptions.NoOpChangeException as e:
+            raise HTTPException(status.HTTP_208_ALREADY_REPORTED, str(e))
         except exceptions.MissingEntityException as e:
             raise HTTPException(status.HTTP_404_NOT_FOUND, str(e))
 

backend/exceptions.py

@@ -27,22 +27,43 @@ def __str__(self) -> str:
         return f'Group with name {self.name} already exists'
 
 
-class MissingObjectException(Exception):
+class ObjectException(Exception):
     obj_type: str = "Object"
 
     def __init__(self, obj_id: Union[int, str], obj_type: Optional[str] = None):
         self.obj_id = obj_id
         self.obj_type = obj_type or self.obj_type
 
     def __str__(self) -> str:
-        return f"{self.obj_type} with id {self.obj_id} doesn't exist or was deleted"
+        raise NotImplementedError("Define in subclasses")
+
+
+class MissingObjectException(ObjectException):
+    def __str__(self) -> str:
+        return f"{self.obj_type} with id {self.obj_id} doesn't exist"
+
+
+class DeletedObjectException(ObjectException):
+    def __str__(self):
+        return f"{self.obj_type} with id {self.obj_id} is deleted"
+
 
 class MissingSchemaException(MissingObjectException):
     obj_type = 'Schema'
 
+
+class SchemaIsDeletedException(DeletedObjectException):
+    obj_type = 'Schema'
+
+
 class MissingEntityException(MissingObjectException):
     obj_type = 'Entity'
 
+
+class EntityIsDeletedException(DeletedObjectException):
+    obj_type = 'Entity'
+
+
 class MissingAttributeException(MissingObjectException):
     obj_type = 'Attribute'
 
@@ -65,6 +86,11 @@ def __str__(self) -> str:
         return f'There is no entity delete request with id {self.obj_id}'
 
 
+class MissingEntityRestoreRequestException(MissingObjectException):
+    def __str__(self) -> str:
+        return f'There is no entity restore request with id {self.obj_id}'
+
+
 class MissingEntityCreateRequestException(MissingObjectException):
     def __str__(self) -> str:
         return f'There is no entity create request with id {self.obj_id}'

backend/general_routes.py

@@ -86,7 +86,24 @@ def get_schemas(
 @router.post(
     '/schema',
     response_model=schemas.SchemaForListSchema,
-    tags=['General routes']
+    tags=['General routes'],
+    summary="Create a schema",
+    responses={
+            200: {"description": "Schema was created"},
+            404: {
+                'description': """Can be returned when:
+                
+                * A schema for foreign key binding does not exist
+                * No attribute exists for a given ID 
+                """
+            },
+            409: {"description": """Can be returned when:
+                
+                * Another schema uses the chosen slug already
+                * Duplicate use of attributes
+            """},
+            410: {"description": "The scheme to bind a foreign key to is deleted"}
+        }
 )
 def create_schema(data: schemas.SchemaCreateSchema, request: Request, db: Session = Depends(get_db),
                   user: User = Depends(authorized_user(RequirePermission(permission=PermissionType.CREATE_SCHEMA)))):
@@ -100,14 +117,14 @@ def create_schema(data: schemas.SchemaCreateSchema, request: Request, db: Sessio
         return schema
     except exceptions.SchemaExistsException as e:
         raise HTTPException(status.HTTP_409_CONFLICT, str(e))
-    except exceptions.MissingAttributeException as e:
+    except (exceptions.MissingAttributeException, exceptions.MissingSchemaException) as e:
         raise HTTPException(status.HTTP_404_NOT_FOUND, str(e))
     except exceptions.MultipleAttributeOccurencesException as e:
         raise HTTPException(status.HTTP_409_CONFLICT, str(e))
     except exceptions.NoSchemaToBindException as e:
         raise HTTPException(status.HTTP_422_UNPROCESSABLE_ENTITY, str(e))
-    except exceptions.MissingSchemaException as e:
-        raise HTTPException(status.HTTP_404_NOT_FOUND, str(e))
+    except exceptions.SchemaIsDeletedException as e:
+        raise HTTPException(status.HTTP_410_GONE, str(e))
 
 
 @router.get(
@@ -161,7 +178,13 @@ def update_schema(
     '/schema/{id_or_slug}',
     response_model=schemas.SchemaDetailSchema,
     response_model_exclude=['attributes', 'attr_defs'],
-    tags=['General routes']
+    tags=['General routes'],
+    summary="Delete schema",
+    responses={
+        200: {"description": "Schema was deleted"},
+        208: {"description": "Schema is already deleted"},
+        404: {"description": "Schema does not exist"},
+    }
 )
 def delete_schema(id_or_slug: Union[int, str], db: Session = Depends(get_db),
                   user: User = Depends(authorized_user(RequirePermission(permission=PermissionType.DELETE_SCHEMA)))):
@@ -173,6 +196,8 @@ def delete_schema(id_or_slug: Union[int, str], db: Session = Depends(get_db),
                                                 reviewed_by=user, comment='Autosubmit')
         db.commit()
         return schema
+    except exceptions.NoOpChangeException as e:
+        raise HTTPException(status.HTTP_208_ALREADY_REPORTED, str(e))
     except exceptions.MissingSchemaException as e:
         raise HTTPException(status.HTTP_404_NOT_FOUND, str(e))
 

backend/tests/test_crud_entity.py

@@ -637,6 +637,23 @@ def test_no_raise_on_non_unique_if_existing_is_deleted(self, dbsession):
         e = dbsession.execute(select(Entity).where(Entity.slug == 'Jack')).scalar()
         assert e.get('nickname', dbsession).value == 'jane'
 
+    def test_raise_on_update_deleted(self, dbsession):
+        """
+        Test that updating a deleted entity warns about the entity already being deleted
+        """
+        e = self.get_default_entity(dbsession)
+        delete_entity(dbsession, id_or_slug=e.id, schema_id=e.schema_id)
+
+        dbsession.refresh(e)
+        assert e.deleted is True
+
+        data = {'slug': e.slug}
+        with pytest.raises(EntityIsDeletedException):
+            update_entity(dbsession, id_or_slug=e.id, schema_id=e.schema_id, data=data)
+
+        dbsession.refresh(e)
+        assert e.deleted is True
+
 
 class TestEntityDelete(DefaultMixin):
     def asserts_after_entity_delete(self, db: Session):
@@ -665,5 +682,33 @@ def test_raise_on_already_deleted(self, dbsession):
         entity = self.get_default_entity(dbsession)
         entity.deleted = True
         dbsession.flush()
-        with pytest.raises(MissingEntityException):
+        with pytest.raises(NoOpChangeException):
             delete_entity(dbsession, id_or_slug=entity.id, schema_id=entity.schema_id)
+
+
+class TestRestoreEntity(DefaultMixin):
+    def asserts_after_entity_restore(self, db: Session):
+        entities = db.execute(select(Entity)).scalars().all()
+        assert len(entities) == 2
+        e = self.get_default_entity(db)
+        assert e.deleted is False
+
+    def test_restore_by_id(self, dbsession):
+        e = self.get_default_entity(dbsession)
+        e.deleted = True
+        dbsession.flush()
+        restore_entity(dbsession, id_or_slug=e.id, schema_id=e.schema_id)
+        self.asserts_after_entity_restore(db=dbsession)
+
+    def test_restore_by_slug(self, dbsession):
+        e = self.get_default_entity(dbsession)
+        e.deleted = True
+        dbsession.flush()
+        restore_entity(dbsession, id_or_slug=e.slug, schema_id=e.schema_id)
+        self.asserts_after_entity_restore(db=dbsession)
+
+    def test_restore_not_deleted(self, dbsession):
+        e = self.get_default_entity(dbsession)
+
+        with pytest.raises(NoOpChangeException):
+            restore_entity(dbsession, id_or_slug=e.slug, schema_id=e.schema_id)

backend/tests/test_crud_schema.py

@@ -7,7 +7,7 @@
     get_entities, update_entity, get_entity
 from ..exceptions import SchemaExistsException, MissingSchemaException, RequiredFieldException, \
     NoOpChangeException, ListedToUnlistedException, MultipleAttributeOccurencesException, \
-    InvalidAttributeChange
+    InvalidAttributeChange, SchemaIsDeletedException
 from ..models import Schema, AttributeDefinition, Attribute, AttrType, Entity
 from .. schemas import AttrDefSchema, SchemaCreateSchema, AttrTypeMapping, SchemaUpdateSchema
 
@@ -151,7 +151,7 @@ def test_raise_on_passed_deleted_schema_for_binding(self, dbsession):
         )
 
         sch = SchemaCreateSchema(name='Test', slug='test', attributes=[attr_def])
-        with pytest.raises(MissingSchemaException):
+        with pytest.raises(SchemaIsDeletedException):
             create_schema(dbsession, data=sch)
 
     def test_raise_on_multiple_attrs_with_same_name(self, dbsession):
@@ -656,7 +656,7 @@ def test_raise_on_already_deleted(self, dbsession):
         schema = self.get_default_schema(dbsession)
         schema.deleted = True
         dbsession.flush()
-        with pytest.raises(MissingSchemaException):
+        with pytest.raises(NoOpChangeException):
             delete_schema(dbsession, id_or_slug=schema.id)
 
     def test_raise_on_delete_nonexistent(self, dbsession):