From 1dc9477d03364aaab7d89f5579cd556d4144fb93 Mon Sep 17 00:00:00 2001
From: Erwan Rouchet <rouchet@teklia.com>
Date: Tue, 26 Oct 2021 17:20:45 +0200
Subject: [PATCH] Fix lots of potential stale reads in Element.add_parent and
.remove_child
---
arkindex/documents/models.py | 18 +++++++++---------
.../element_move_with_children.sql | 10 ----------
.../element_move_without_child.sql | 10 ----------
3 files changed, 9 insertions(+), 29 deletions(-)
diff --git a/arkindex/documents/models.py b/arkindex/documents/models.py
index 64084b1367..a1c0a4d28b 100644
--- a/arkindex/documents/models.py
+++ b/arkindex/documents/models.py
@@ -221,7 +221,7 @@ class Element(IndexableModel):
delete_element(self.id)
Element.objects.filter(id=self.id)._raw_delete(using='default')
- @transaction.atomic(using="default")
+ @transaction.atomic
def add_parent(self, parent, skip_children=False):
'''
Add an element as ancestor
@@ -230,12 +230,12 @@ class Element(IndexableModel):
# Load existing paths on current element, and the parent paths
# in a single query
- paths = ElementPath.objects.filter(element_id__in=(self.id, parent.id)).values('element_id', 'path')
+ paths = ElementPath.objects.using('default').filter(element_id__in=(self.id, parent.id)).values('element_id', 'path')
existing_paths = [p['path'] for p in paths if p['element_id'] == self.id]
parent_paths = [p['path'] for p in paths if p['element_id'] == parent.id]
# Get the next order for this type in the parent
- order = parent.get_next_order(self.type)
+ order = parent.get_next_order(self.type_id)
# Build all parent paths needed
if not parent_paths:
@@ -262,10 +262,10 @@ class Element(IndexableModel):
order,
))
- # Update children's paths in a non optimized way as this is pretty rare
+ # Update children's paths in a non optimized way
if skip_children is False:
to_delete = []
- children = ElementPath.objects.filter(path__contains=[self.id])
+ children = ElementPath.objects.using('default').filter(path__contains=[self.id])
for child in children:
base_path = child.path
@@ -299,7 +299,7 @@ class Element(IndexableModel):
Uses the primary database to avoid stale reads and duplicate orderings,
and path__overlap to let Postgres use the GIN index before filtering by last item.
"""
- assert isinstance(type, ElementType)
+ assert isinstance(type, (ElementType, uuid.UUID))
return ElementPath.objects \
.using('default') \
.filter(path__overlap=[self.id], path__last=self.id, element__type=type) \
@@ -313,16 +313,16 @@ class Element(IndexableModel):
assert isinstance(child, Element)
# Check child is a direct child of this element
- if not ElementPath.objects.filter(element=child, path__last=self.id).exists():
+ if not ElementPath.objects.using('default').filter(element=child, path__last=self.id).exists():
return
new_epaths = {}
# If child doesn't have any other parent, updates are needed
- if not ElementPath.objects.filter(element=child).exclude(path__last=self.id).exists():
+ if not ElementPath.objects.using('default').filter(element=child).exclude(path__last=self.id).exists():
# Get all the child's children's paths
- children_epaths = list(ElementPath.objects.filter(
+ children_epaths = list(ElementPath.objects.using('default').filter(
element__in=Element.objects.get_descending(child.id),
path__contains=[self.id]
))
diff --git a/arkindex/sql_validation/element_move_with_children.sql b/arkindex/sql_validation/element_move_with_children.sql
index 73df64a21f..3bc38631ae 100644
--- a/arkindex/sql_validation/element_move_with_children.sql
+++ b/arkindex/sql_validation/element_move_with_children.sql
@@ -65,16 +65,6 @@ FROM "documents_elementpath"
WHERE "documents_elementpath"."element_id" IN ('{source_id}'::uuid,
'{destination_id}'::uuid);
-SELECT "documents_elementtype"."id",
- "documents_elementtype"."corpus_id",
- "documents_elementtype"."slug",
- "documents_elementtype"."display_name",
- "documents_elementtype"."folder",
- "documents_elementtype"."indexable"
-FROM "documents_elementtype"
-WHERE "documents_elementtype"."id" = '{page_type_id}'::uuid
-LIMIT 21;
-
SELECT (MAX("documents_elementpath"."ordering") + 1) AS "max"
FROM "documents_elementpath"
INNER JOIN "documents_element" ON ("documents_elementpath"."element_id" = "documents_element"."id")
diff --git a/arkindex/sql_validation/element_move_without_child.sql b/arkindex/sql_validation/element_move_without_child.sql
index 4aa5c4e007..f9a87ec96e 100644
--- a/arkindex/sql_validation/element_move_without_child.sql
+++ b/arkindex/sql_validation/element_move_without_child.sql
@@ -56,16 +56,6 @@ FROM "documents_elementpath"
WHERE "documents_elementpath"."element_id" IN ('{source_id}'::uuid,
'{destination_id}'::uuid);
-SELECT "documents_elementtype"."id",
- "documents_elementtype"."corpus_id",
- "documents_elementtype"."slug",
- "documents_elementtype"."display_name",
- "documents_elementtype"."folder",
- "documents_elementtype"."indexable"
-FROM "documents_elementtype"
-WHERE "documents_elementtype"."id" = '{page_type_id}'::uuid
-LIMIT 21;
-
SELECT (MAX("documents_elementpath"."ordering") + 1) AS "max"
FROM "documents_elementpath"
INNER JOIN "documents_element" ON ("documents_elementpath"."element_id" = "documents_element"."id")
--
GitLab