services: edit workflow and soft deletion of draft

invenio_drafts_resources/services/records/service.py

@@ -93,38 +93,44 @@ def create(self, identity, data, links_config=None):
            self.draft_cls, identity, data, links_config=links_config
         )
 
-    def _edit_or_create(self, id_):
-        """Edit or create a draft based on the pid."""
-        try:
-            # Draft exists
-            return self.draft_cls.pid.resolve(id_, registered_only=False)
-        except NoResultFound:
-            # Draft does not exists - create a new one
-            record = self.record_cls.pid.resolve(id_)  # Needs Record as getter
-            draft = self.draft_cls.create(
-                {}, id_=record.id, fork_version_id=record.revision_id,
-                pid=record.pid, conceptpid=record.conceptpid
-            )
-
-            # FIXME: Is this enough to copy over the data?
-            draft.update(**record)
-
-        return draft
-
     def edit(self, id_, identity, links_config=None):
         """Create a new revision or a draft for an existing record.
 
         Note: Because of the workflow, this method does not accept data.
         :param id_: record PID value.
         """
         self.require_permission(identity, "create")
+        # Draft exists - return it
+        try:
+            draft = self.draft_cls.pid.resolve(id_, registered_only=False)
+            return self.result_item(
+                self, identity, draft, links_config=links_config)
+        except NoResultFound:
+            pass
 
-        # Get draft
-        # TODO: Workflow does not seem correct:
-        # 1 if it exists, do nothing and redirect (components) shouldn't run)
-        # 2 if it doesn't exist return
-
-        draft = self._edit_or_create(id_)
+        # Draft does not exists - create a new draft or get a soft deleted
+        # draft.
+        record = self.record_cls.pid.resolve(id_)
+        try:
+            # We soft-delete a draft once it has been published, in order to
+            # keep the version_id counter around for optimistic concurrency
+            # control (both for ES indexing and for REST API clients)
+            draft = self.draft_cls.get_record(record.id, with_deleted=True)
+            if draft.is_deleted:
+                draft.undelete()
+                draft.update(**record)
+                draft.pid = record.pid
+                draft.conceptpid = record.conceptpid
+                draft.fork_version_id = record.revision_id
+        except NoResultFound:
+            # If a draft was ever force deleted, then we will create the draft.
+            # This is a very exceptional case as normally, when we edit a
+            # record then the soft-deleted draft exists and we are in above
+            # case.
+            draft = self.draft_cls.create(
+                record, id_=record.id, fork_version_id=record.revision_id,
+                pid=record.pid, conceptpid=record.conceptpid
+            )
 
         # Run components
         for component in self.components:
@@ -133,7 +139,6 @@ def edit(self, id_, identity, links_config=None):
 
         draft.commit()
         db.session.commit()
-
         self.indexer.index(draft)
 
         return self.result_item(
@@ -165,9 +170,7 @@ def publish(self, id_, identity, links_config=None):
         record.commit()
         db.session.commit()
 
-        # Remove draft. Hard delete to remove the uuid (pk) from the table.
-        # TODO: Question is this what we wanted?
-        draft.delete(force=True)
+        draft.delete()
         db.session.commit()  # Persist DB
         self.indexer.delete(draft)
         self.indexer.index(record)
@@ -215,7 +218,13 @@ def delete_draft(self, id_, identity, revision_id=None):
             if hasattr(component, 'delete_draft'):
                 component.delete(identity, record=draft)
 
-        draft.delete()
+        try:
+            record = self.record_cls.pid.resolve(id_, registered_only=False)
+        except NoResultFound:  # FIXME: Should be at resolver level?
+            record = None
+        # If it is tight to a record soft remove, else wipe fully
+        force = True if record else False
+        draft.delete(force=force)
         db.session.commit()
         self.indexer.delete(draft)
 

setup.py

@@ -16,14 +16,8 @@
 history = open("CHANGES.rst").read()
 
 tests_require = [
-    "pytest-invenio>=1.3.4",
+    "pytest-invenio>=1.4.0",
     "invenio-app>=1.3.0",
-    # TODO: Remove all lines below with pytest-invenio v1.4.0:
-    "pytest-cov>=2.10.1",
-    "pytest-isort>=1.2.0",
-    "pytest-pycodestyle>=2.2.0",
-    "pytest-pydocstyle>=2.2.0",
-    "pytest>=6,<7",
 ]
 
 # Should follow inveniosoftware/invenio versions

tests/resources/test_record_resource.py

@@ -197,10 +197,6 @@ def test_create_publish_new_revision(client, input_data,
     """Test draft creation of an existing record and publish it."""
     recid = _create_and_publish(client, input_data)
 
-    # # FIXME: Allow ES to clean deleted documents.
-    # # Flush is not the same. Default collection time is 1 minute.
-    # time.sleep(70)
-
     # Create new draft of said record
     orig_title = input_data["metadata"]["title"]
     input_data["metadata"]["title"] = "Edited title"
@@ -210,7 +206,7 @@ def test_create_publish_new_revision(client, input_data,
     )
 
     assert response.status_code == 201
-    assert response.json['revision_id'] == 1
+    assert response.json['revision_id'] == 4
     _assert_single_item_response(response)
 
     # Update that new draft
@@ -250,6 +246,42 @@ def test_create_publish_new_revision(client, input_data,
         input_data["metadata"]["title"]
 
 
+def test_mutiple_edit(client, identity_simple, input_data):
+    """Test the revision_id when editing record multiple times.
+
+    This tests the `edit` service method.
+    """
+    # Needs `app` context because of invenio_access/permissions.py#166
+    recid = _create_and_publish(client, input_data)
+
+    # Create new draft of said record
+    response = client.post(
+        "/mocks/{}/draft".format(recid), headers=HEADERS)
+
+    assert response.status_code == 201
+    assert response.json['revision_id'] == 4
+
+    # Request a second edit. Get the same draft (revision_id)
+    response = client.post(
+        "/mocks/{}/draft".format(recid), headers=HEADERS)
+
+    assert response.status_code == 201
+    assert response.json['revision_id'] == 4
+
+    # Publish it to check the increment in version_id
+    response = client.post(
+        "/mocks/{}/draft/actions/publish".format(recid), headers=HEADERS)
+
+    assert response.status_code == 202
+
+    # Edit again
+    response = client.post(
+        "/mocks/{}/draft".format(recid), headers=HEADERS)
+
+    assert response.status_code == 201
+    assert response.json['revision_id'] == 7
+
+
 def test_create_publish_new_version(client, input_data,
                                     identity_simple):
     """Creates a new version of a record.

tests/services/test_record_service.py

@@ -161,6 +161,8 @@ def test_create_publish_new_revision(app, service, identity_simple,
     draft = service.edit(recid, identity_simple)
     assert draft.id == recid
     assert draft._record.fork_version_id == record._record.revision_id
+    # create, soft-delete, undelete, update
+    assert draft._record.revision_id == 4
 
     # Update the content
     orig_title = input_data['metadata']['title']
@@ -185,6 +187,35 @@ def test_create_publish_new_revision(app, service, identity_simple,
     assert record["metadata"]['title'] == edited_title
 
 
+def test_mutiple_edit(app, service, identity_simple, input_data):
+    """Test the revision_id when editing record multiple times..
+
+    This tests the `edit` service method.
+    """
+    # Needs `app` context because of invenio_access/permissions.py#166
+    record = _create_and_publish(service, input_data, identity_simple)
+    recid = record.id
+
+    # Create new draft of said record
+    draft = service.edit(recid, identity_simple)
+    assert draft.id == recid
+    assert draft._record.fork_version_id == record._record.revision_id
+    assert draft._record.revision_id == 4
+
+    draft = service.edit(recid, identity_simple)
+    assert draft.id == recid
+    assert draft._record.fork_version_id == record._record.revision_id
+    assert draft._record.revision_id == 4
+
+    # Publish it to check the increment in version_id
+    record = service.publish(recid, identity_simple)
+
+    draft = service.edit(recid, identity_simple)
+    assert draft.id == recid
+    assert draft._record.fork_version_id == record._record.revision_id
+    assert draft._record.revision_id == 7  # soft-delete, undelete, update
+
+
 def test_create_publish_new_version(app, service, identity_simple,
                                     input_data):
     """Test creating a new revision of a record.