Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: propagate deletion of parent PIDs properly #1864

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

bug: propagate deletion of parent PIDs properly #1864

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion invenio_rdm_records/services/components/pids.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
from invenio_drafts_resources.services.records.uow import ParentRecordCommitOp
from invenio_records_resources.services.uow import TaskOp

from ..pids.tasks import register_or_update_pid
from ..pids.tasks import cleanup_parent_pids, register_or_update_pid


class PIDsComponent(ServiceComponent):
Expand Down Expand Up @@ -212,6 +212,11 @@ def delete_record(self, identity, data=None, record=None, uow=None):
self.service.pids.parent_pid_manager.discard_all(
parent_pids, soft_delete=True
)
else:
# We're sending a task in case there is a race condition with two
# versions being deleted at the same time to ensure that we have
# consistent database state
self.uow.register(TaskOp(cleanup_parent_pids, record["id"]))
carlinmack marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like that the if above is copy/pasted in the celery task.
Does it make sense to completely remove the if condition here and simply call:
self.uow.register(TaskOp(cleanup_parent_pids, record["id"]))?

This is a bit confusing.


# Async register/update tasks after transaction commit.
for scheme in parent_pids.keys():
Expand Down
15 changes: 15 additions & 0 deletions invenio_rdm_records/services/pids/tasks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

"""RDM PIDs Service tasks."""

from copy import deepcopy

from celery import shared_task
from invenio_access.permissions import system_identity

Expand All @@ -22,3 +24,16 @@ def register_or_update_pid(recid, scheme, parent=False):
scheme=scheme,
parent=parent,
)


@shared_task(ignore_result=True)
def cleanup_parent_pids(recid):
"""Clean up parent PIDs."""
record_cls = current_rdm_records.records_service
record = record_cls.pid.resolve(recid, with_deleted=True)
if record.deleted:
parent_pids = deepcopy(record.parent.get("pids", {}))
if record_cls.next_latest_published_record_by_parent(record.parent) is None:
record_cls.pids.parent_pid_manager.discard_all(
parent_pids, soft_delete=True
)
Loading