Collection: also save the collection UID on the model itself.

This enables us to have db-constraints for making sure that UIDs are
unique, as well as being more efficient for lookups (which are very
common).

The UID should always be the same as the main_item.uid, though that's
easily enforced as neither of them is allowed to change.
This commit is contained in:
Tom Hacohen 2020-12-14 13:30:30 +02:00
parent 057b908565
commit baa42d040d
7 changed files with 93 additions and 30 deletions

View File

@ -0,0 +1,19 @@
# Generated by Django 3.1.1 on 2020-12-14 11:21
import django.core.validators
from django.db import migrations, models
class Migration(migrations.Migration):
dependencies = [
('django_etebase', '0032_auto_20201013_1409'),
]
operations = [
migrations.AddField(
model_name='collection',
name='uid',
field=models.CharField(db_index=True, max_length=43, null=True, validators=[django.core.validators.RegexValidator(message='Not a valid UID', regex='^[a-zA-Z0-9\\-_]{20,}$')]),
),
]

View File

@ -0,0 +1,22 @@
# Generated by Django 3.1.1 on 2020-12-14 11:24
from django.db import migrations
def update_collection_uid(apps, schema_editor):
Collection = apps.get_model("django_etebase", "Collection")
for collection in Collection.objects.all():
collection.uid = collection.main_item.uid
collection.save()
class Migration(migrations.Migration):
dependencies = [
("django_etebase", "0033_collection_uid"),
]
operations = [
migrations.RunPython(update_collection_uid),
]

View File

@ -0,0 +1,19 @@
# Generated by Django 3.1.1 on 2020-12-14 11:26
import django.core.validators
from django.db import migrations, models
class Migration(migrations.Migration):
dependencies = [
('django_etebase', '0034_auto_20201214_1124'),
]
operations = [
migrations.AlterField(
model_name='collection',
name='uid',
field=models.CharField(db_index=True, max_length=43, validators=[django.core.validators.RegexValidator(message='Not a valid UID', regex='^[a-zA-Z0-9\\-_]{20,}$')]),
),
]

View File

@ -0,0 +1,19 @@
# Generated by Django 3.1.1 on 2020-12-14 11:28
import django.core.validators
from django.db import migrations, models
class Migration(migrations.Migration):
dependencies = [
('django_etebase', '0035_auto_20201214_1126'),
]
operations = [
migrations.AlterField(
model_name='collection',
name='uid',
field=models.CharField(db_index=True, max_length=43, unique=True, validators=[django.core.validators.RegexValidator(message='Not a valid UID', regex='^[a-zA-Z0-9\\-_]{20,}$')]),
),
]

View File

@ -43,6 +43,8 @@ class CollectionType(models.Model):
class Collection(models.Model): class Collection(models.Model):
main_item = models.OneToOneField("CollectionItem", related_name="parent", null=True, on_delete=models.SET_NULL) main_item = models.OneToOneField("CollectionItem", related_name="parent", null=True, on_delete=models.SET_NULL)
# The same as main_item.uid, we just also save it here so we have DB constraints for uniqueness (and efficiency)
uid = models.CharField(db_index=True, unique=True, blank=False, max_length=43, validators=[UidValidator])
owner = models.ForeignKey(settings.AUTH_USER_MODEL, on_delete=models.CASCADE) owner = models.ForeignKey(settings.AUTH_USER_MODEL, on_delete=models.CASCADE)
stoken_annotation = stoken_annotation_builder(["items__revisions__stoken", "members__stoken"]) stoken_annotation = stoken_annotation_builder(["items__revisions__stoken", "members__stoken"])
@ -50,10 +52,6 @@ class Collection(models.Model):
def __str__(self): def __str__(self):
return self.uid return self.uid
@cached_property
def uid(self):
return self.main_item.uid
@property @property
def content(self): def content(self):
return self.main_item.content return self.main_item.content
@ -76,20 +74,6 @@ class Collection(models.Model):
return Stoken.objects.get(id=stoken_id).uid return Stoken.objects.get(id=stoken_id).uid
def validate_unique(self, exclude=None):
super().validate_unique(exclude=exclude)
if exclude is None or "main_item" in exclude:
return
if (
self.__class__.objects.filter(owner=self.owner, main_item__uid=self.main_item.uid)
.exclude(id=self.id)
.exists()
):
raise EtebaseValidationError(
"unique_uid", "Collection with this uid already exists", status_code=status.HTTP_409_CONFLICT
)
class CollectionItem(models.Model): class CollectionItem(models.Model):
uid = models.CharField(db_index=True, blank=False, max_length=43, validators=[UidValidator]) uid = models.CharField(db_index=True, blank=False, max_length=43, validators=[UidValidator])

View File

@ -313,13 +313,13 @@ class CollectionSerializer(BetterErrorsMixin, serializers.ModelSerializer):
user = validated_data.get("owner") user = validated_data.get("owner")
main_item_data = validated_data.pop("main_item") main_item_data = validated_data.pop("main_item")
uid = main_item_data.get("uid")
etag = main_item_data.pop("etag") etag = main_item_data.pop("etag")
revision_data = main_item_data.pop("content") revision_data = main_item_data.pop("content")
instance = self.__class__.Meta.model(**validated_data) instance = self.__class__.Meta.model(uid=uid, **validated_data)
with transaction.atomic(): with transaction.atomic():
_ = self.__class__.Meta.model.objects.select_for_update().filter(owner=user)
if etag is not None: if etag is not None:
raise EtebaseValidationError("bad_etag", "etag is not null") raise EtebaseValidationError("bad_etag", "etag is not null")

View File

@ -174,7 +174,7 @@ class CollectionViewSet(BaseViewSet):
permission_classes = BaseViewSet.permission_classes + (permissions.IsCollectionAdminOrReadOnly,) permission_classes = BaseViewSet.permission_classes + (permissions.IsCollectionAdminOrReadOnly,)
queryset = Collection.objects.all() queryset = Collection.objects.all()
serializer_class = CollectionSerializer serializer_class = CollectionSerializer
lookup_field = "main_item__uid" lookup_field = "uid"
lookup_url_kwarg = "uid" lookup_url_kwarg = "uid"
stoken_annotation = Collection.stoken_annotation stoken_annotation = Collection.stoken_annotation
@ -246,7 +246,7 @@ class CollectionViewSet(BaseViewSet):
# can point to the most recent collection change rather than most recent removed membership. # can point to the most recent collection change rather than most recent removed membership.
remed_qs = remed_qs.filter(stoken__id__lte=new_stoken_obj.id) remed_qs = remed_qs.filter(stoken__id__lte=new_stoken_obj.id)
remed = remed_qs.values_list("collection__main_item__uid", flat=True) remed = remed_qs.values_list("collection__uid", flat=True)
if len(remed) > 0: if len(remed) > 0:
ret["removedMemberships"] = [{"uid": x} for x in remed] ret["removedMemberships"] = [{"uid": x} for x in remed]
@ -264,7 +264,7 @@ class CollectionItemViewSet(BaseViewSet):
def get_queryset(self): def get_queryset(self):
collection_uid = self.kwargs["collection_uid"] collection_uid = self.kwargs["collection_uid"]
try: try:
collection = self.get_collection_queryset(Collection.objects).get(main_item__uid=collection_uid) collection = self.get_collection_queryset(Collection.objects).get(uid=collection_uid)
except Collection.DoesNotExist: except Collection.DoesNotExist:
raise Http404("Collection does not exist") raise Http404("Collection does not exist")
# XXX Potentially add this for performance: .prefetch_related('revisions__chunks') # XXX Potentially add this for performance: .prefetch_related('revisions__chunks')
@ -312,7 +312,7 @@ class CollectionItemViewSet(BaseViewSet):
@action_decorator(detail=True, methods=["GET"]) @action_decorator(detail=True, methods=["GET"])
def revision(self, request, collection_uid=None, uid=None, *args, **kwargs): def revision(self, request, collection_uid=None, uid=None, *args, **kwargs):
col = get_object_or_404(self.get_collection_queryset(Collection.objects), main_item__uid=collection_uid) col = get_object_or_404(self.get_collection_queryset(Collection.objects), uid=collection_uid)
item = get_object_or_404(col.items, uid=uid) item = get_object_or_404(col.items, uid=uid)
limit = int(request.GET.get("limit", 50)) limit = int(request.GET.get("limit", 50))
@ -386,7 +386,7 @@ class CollectionItemViewSet(BaseViewSet):
with transaction.atomic(): # We need this for locking on the collection object with transaction.atomic(): # We need this for locking on the collection object
collection_object = get_object_or_404( collection_object = get_object_or_404(
self.get_collection_queryset(Collection.objects).select_for_update(), # Lock writes on the collection self.get_collection_queryset(Collection.objects).select_for_update(), # Lock writes on the collection
main_item__uid=collection_uid, uid=collection_uid,
) )
if stoken is not None and stoken != collection_object.stoken: if stoken is not None and stoken != collection_object.stoken:
@ -435,7 +435,7 @@ class CollectionItemChunkViewSet(viewsets.ViewSet):
return queryset.filter(members__user=user) return queryset.filter(members__user=user)
def update(self, request, *args, collection_uid=None, collection_item_uid=None, uid=None, **kwargs): def update(self, request, *args, collection_uid=None, collection_item_uid=None, uid=None, **kwargs):
col = get_object_or_404(self.get_collection_queryset(), main_item__uid=collection_uid) col = get_object_or_404(self.get_collection_queryset(), uid=collection_uid)
# IGNORED FOR NOW: col_it = get_object_or_404(col.items, uid=collection_item_uid) # IGNORED FOR NOW: col_it = get_object_or_404(col.items, uid=collection_item_uid)
data = { data = {
@ -459,7 +459,7 @@ class CollectionItemChunkViewSet(viewsets.ViewSet):
import os import os
from django.views.static import serve from django.views.static import serve
col = get_object_or_404(self.get_collection_queryset(), main_item__uid=collection_uid) col = get_object_or_404(self.get_collection_queryset(), uid=collection_uid)
# IGNORED FOR NOW: col_it = get_object_or_404(col.items, uid=collection_item_uid) # IGNORED FOR NOW: col_it = get_object_or_404(col.items, uid=collection_item_uid)
chunk = get_object_or_404(col.chunks, uid=uid) chunk = get_object_or_404(col.chunks, uid=uid)
@ -487,7 +487,7 @@ class CollectionMemberViewSet(BaseViewSet):
def get_queryset(self, queryset=None): def get_queryset(self, queryset=None):
collection_uid = self.kwargs["collection_uid"] collection_uid = self.kwargs["collection_uid"]
try: try:
collection = self.get_collection_queryset(Collection.objects).get(main_item__uid=collection_uid) collection = self.get_collection_queryset(Collection.objects).get(uid=collection_uid)
except Collection.DoesNotExist: except Collection.DoesNotExist:
raise Http404("Collection does not exist") raise Http404("Collection does not exist")
@ -525,7 +525,7 @@ class CollectionMemberViewSet(BaseViewSet):
@action_decorator(detail=False, methods=["POST"], permission_classes=our_base_permission_classes) @action_decorator(detail=False, methods=["POST"], permission_classes=our_base_permission_classes)
def leave(self, request, collection_uid=None, *args, **kwargs): def leave(self, request, collection_uid=None, *args, **kwargs):
collection_uid = self.kwargs["collection_uid"] collection_uid = self.kwargs["collection_uid"]
col = get_object_or_404(self.get_collection_queryset(Collection.objects), main_item__uid=collection_uid) col = get_object_or_404(self.get_collection_queryset(Collection.objects), uid=collection_uid)
member = col.members.get(user=request.user) member = col.members.get(user=request.user)
self.perform_destroy(member) self.perform_destroy(member)
@ -584,7 +584,7 @@ class InvitationOutgoingViewSet(InvitationBaseViewSet):
collection_uid = serializer.validated_data.get("collection", {}).get("uid") collection_uid = serializer.validated_data.get("collection", {}).get("uid")
try: try:
collection = self.get_collection_queryset(Collection.objects).get(main_item__uid=collection_uid) collection = self.get_collection_queryset(Collection.objects).get(uid=collection_uid)
except Collection.DoesNotExist: except Collection.DoesNotExist:
raise Http404("Collection does not exist") raise Http404("Collection does not exist")