Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
48 changes: 32 additions & 16 deletions core/common/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -775,6 +775,7 @@ def persist_new(cls, obj, created_by, **kwargs):

@classmethod
def persist_new_version(cls, obj, user=None, **kwargs):
"""Persist a repository version and schedule its children snapshot."""
errors = {}

obj.is_active = True
Expand All @@ -787,24 +788,39 @@ def persist_new_version(cls, obj, user=None, **kwargs):
if not head:
errors[repo_resource_name.lower()] = 'Version Head not found.'
return errors
obj.update_version_data(head)
obj.save(**kwargs)

if obj.id:
obj.sibling_versions.update(is_latest_version=False)

is_test_mode = get(settings, 'TEST_MODE', False)
if is_test_mode or sync:
seed_children_to_new_version(obj.resource_type.lower(), obj.id, not is_test_mode, sync)
else:
from core.tasks.models import Task
task = Task.new(queue='default', user=user, name=seed_children_to_new_version.__name__)
seed_children_to_new_version.apply_async(
(obj.resource_type.lower(), obj.id, True, sync),
task_id=task.id,
queue='default',
persist_args=True
)
with transaction.atomic():
# Serialize version creation with destructive child operations on the HEAD repository.
head = cls.objects.select_for_update().get(id=head.id)
obj.update_version_data(head)
obj.save(**kwargs)

if obj.id:
obj.sibling_versions.update(is_latest_version=False)

task_args = (obj.resource_type.lower(), obj.id, not is_test_mode, sync)
if is_test_mode or sync:
seed_children_to_new_version(*task_args)
else:
from core.tasks.models import Task
task = Task.new(
queue='default',
user=user,
name=seed_children_to_new_version.__name__,
args=task_args,
)

def enqueue_seed_task():
"""Queue snapshot seeding only after its repository version is committed."""
seed_children_to_new_version.apply_async(
task_args,
task_id=task.id,
queue='default',
persist_args=True
)

transaction.on_commit(enqueue_seed_task)

return errors

Expand Down
3 changes: 3 additions & 0 deletions core/concepts/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@
CONCEPT_WAS_UNRETIRED = 'Concept was un-retired'
CONCEPT_IS_ALREADY_RETIRED = 'Concept is already retired'
CONCEPT_IS_ALREADY_NOT_RETIRED = 'Concept is already not retired'
CONCEPT_HARD_DELETE_REQUIRES_HEAD_ONLY = (
'Concept cannot be hard deleted because it belongs to a source version.'
)
ALREADY_EXISTS = "Concept ID must be unique within a source."
PERSIST_CLONE_SPECIFY_USER_ERROR = "Must specify which user is attempting to create a new version."
PERSIST_CLONE_ERROR = 'An error occurred while saving new version.'
Expand Down
39 changes: 39 additions & 0 deletions core/concepts/models.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from celery.states import SUCCESS
from django.conf import settings
from django.contrib.postgres.indexes import HashIndex
from django.core.exceptions import ValidationError
Expand All @@ -20,6 +21,7 @@
MAX_NAMES_LIMIT, MAX_DESCRIPTIONS_LIMIT
from core.concepts.mixins import ConceptValidationMixin
from core.services.storages.postgres import PostgresQL
from core.tasks.models import Task


class AbstractLocalizedText(ChecksumModel):
Expand Down Expand Up @@ -897,6 +899,43 @@ def is_existing_in_parent(self):
def latest_source_version(self):
return self.sources.exclude(version=HEAD).order_by('-created_at').first()

def belongs_to_non_head_source_version(self):
"""Return whether any version of this source-scoped concept was snapshotted."""
versioned_object_id = self.versioned_object_id or self.id
concept_ids = Concept.objects.filter(
parent_id=self.parent_id,
versioned_object_id=versioned_object_id,
).values_list('id', flat=True)
return Concept.sources.through.objects.filter(
concept_id__in=concept_ids,
).exclude(source__version=HEAD).exists()

def has_pending_source_version_seed(self):
"""Return whether an unfinished source snapshot may include this concept."""
versioned_object_id = self.versioned_object_id or self.id
concept_created_at = Concept.objects.filter(id=versioned_object_id).values_list(
'created_at', flat=True
).first()
if not concept_created_at:
return False

source = self.parent
source_version_ids = source.__class__.objects.filter(
mnemonic=source.mnemonic,
organization_id=source.organization_id,
user_id=source.user_id,
created_at__gte=concept_created_at,
).exclude(version=HEAD).values_list('id', flat=True)

for source_version_id in source_version_ids:
seed_task = Task.find(
name__iendswith='seed_children_to_new_version',
args__contains=['source', source_version_id],
)
if seed_task and seed_task.state != SUCCESS:
return True
return False

def get_source_version_before_creation(self):
return self.sources.exclude(version=HEAD).filter(
created_at__lte=self.created_at).order_by('-created_at').first()
Expand Down
72 changes: 56 additions & 16 deletions core/concepts/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,10 @@
drop_version, get_falsy_values)
from core.common.views import SourceChildCommonBaseView, SourceChildExtrasView, \
SourceChildExtraRetrieveUpdateDestroyView, BaseAPIView
from core.concepts.constants import PARENT_VERSION_NOT_LATEST_CANNOT_UPDATE_CONCEPT
from core.concepts.constants import (
CONCEPT_HARD_DELETE_REQUIRES_HEAD_ONLY,
PARENT_VERSION_NOT_LATEST_CANNOT_UPDATE_CONCEPT,
)
from core.concepts.documents import ConceptDocument
from core.concepts.models import Concept, ConceptName
from core.concepts.permissions import CanViewParentDictionary, CanEditParentDictionary
Expand Down Expand Up @@ -344,7 +347,10 @@ def get_permissions(self):
if self.request.method in ['GET']:
return [CanViewParentDictionary(), ]

if self.request.method == 'DELETE' and self.is_hard_delete_requested():
if (
self.request.method == 'DELETE' and self.is_hard_delete_requested() and
(self.is_async_requested() or self.is_db_delete_requested())
):
return [IsAdminUser(), ]

return [CanEditParentDictionary(), ]
Expand Down Expand Up @@ -383,30 +389,64 @@ def update(self, request, *args, **kwargs):
def is_db_delete_requested(self):
return self.request.query_params.get('db', None) in TRUTHY

def _db_hard_delete(self):
parent_filters = Concept.get_parent_and_owner_filters_from_kwargs(self.kwargs)
concepts = Concept.objects.filter(mnemonic=self.kwargs['concept'], **parent_filters)
concept = concepts.filter(id=F('versioned_object_id')).first()
parent = concept.parent
result = concepts.delete()
parent.update_concepts_count()
return Response(result, status=status.HTTP_204_NO_CONTENT)

def _hard_delete(self, request, concept):
if self.is_async_requested():
task = Task.new(queue='default', user=request.user, name=delete_concept.__name__)
delete_concept.apply_async((concept.id,), queue=task.queue, task_id=task.id)
return Response(status=status.HTTP_204_NO_CONTENT)

parent = concept.parent
with transaction.atomic():
# Source version creation locks the same HEAD row before registering its seed task.
parent.__class__.objects.select_for_update().get(id=concept.parent_id)
locked_concepts = list(Concept.objects.select_for_update().filter(
parent_id=concept.parent_id,
versioned_object_id=concept.versioned_object_id,
))
Comment on lines +411 to +414

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Block deletion while source-version seeding is pending

When a source version has just been created through the normal asynchronous path, persist_new_version returns before seed_children_to_new_version calls Source.seed_concepts, so the new version temporarily has no concept associations. During that window this lock and the subsequent belongs_to_non_head_source_version() check see no non-HEAD membership and allow an editor to delete the concept; the seeding task then either omits the deleted concept or fails if it already selected the concept ID before the cascade. Source.seed_concepts in core/sources/models.py does not acquire these row locks, so locking only the concept rows here does not synchronize the operations and can corrupt a draft or released snapshot.

Useful? React with 👍 / 👎.

versioned_concept = next(
(candidate for candidate in locked_concepts if candidate.id == concept.id),
None,
)
if not versioned_concept:
raise Http404()

is_admin = IsAdminUser().has_permission(request, self)
if not is_admin and (
versioned_concept.belongs_to_non_head_source_version() or
versioned_concept.has_pending_source_version_seed()
):
return Response(
{'detail': CONCEPT_HARD_DELETE_REQUIRES_HEAD_ONLY},
status=status.HTTP_409_CONFLICT,
)

# Versions reference the versioned concept with on_delete=CASCADE.
# Deleting the root removes every HEAD-only version and its related rows.
versioned_concept.delete()
parent.update_concepts_count()
return Response(status=status.HTTP_204_NO_CONTENT)

def destroy(self, request, *args, **kwargs):
if self.is_container_version_specified():
return Response(status=status.HTTP_405_METHOD_NOT_ALLOWED)
is_hard_delete_requested = self.is_hard_delete_requested()
if self.is_db_delete_requested() and is_hard_delete_requested:
parent_filters = Concept.get_parent_and_owner_filters_from_kwargs(self.kwargs)
concepts = Concept.objects.filter(mnemonic=self.kwargs['concept'], **parent_filters)
concept = concepts.filter(id=F('versioned_object_id')).first()
parent = concept.parent
result = concepts.delete()
parent.update_concepts_count()
return Response(result, status=status.HTTP_204_NO_CONTENT)
return self._db_hard_delete()

concept = self.get_object()
parent = concept.parent

if is_hard_delete_requested:
if self.is_async_requested():
task = Task.new(queue='default', user=request.user, name=delete_concept.__name__)
delete_concept.apply_async((concept.id,), queue=task.queue, task_id=task.id)
return Response(status=status.HTTP_204_NO_CONTENT)
concept.delete()
parent.update_concepts_count()
return Response(status=status.HTTP_204_NO_CONTENT)
return self._hard_delete(request, concept)

comment = request.data.get('update_comment', None) or request.data.get('comment', None)
reason = request.data.get('retire_reason', None)
Expand Down
Loading