swisscom / cleanerversion

CleanerVersion adds a versioning/historizing layer to your relational DB which implements a "Slowly Changing Dimensions Type 2" behavior
Apache License 2.0
136 stars 53 forks source link

The VERSION_UNIQUE does not seem to work #162

Open simkimsia opened 5 years ago

simkimsia commented 5 years ago

I have the following

class VersionedWBSElement(Versionable):
    # wbs number is supposed to be unique
    wbs_number = models.CharField(max_length=100)
    wbs_description = models.TextField()

    VERSION_UNIQUE = [['wbs_number']]

I can still create duplicate with the same wbs_number despite the VERSION_UNIQUE

My workaround is install django-partial-index as a package and then

from partial_index import PQ, PartialIndex, ValidatePartialUniqueMixin

class VersionedWBSElement(Versionable, ValidatePartialUniqueMixin):
    # wbs number is supposed to be unique
    wbs_number = models.CharField(max_length=100)
    wbs_description = models.TextField()

     class Meta:
          indexes = [
              PartialIndex(fields=['wbs_number'], unique=True, where=PQ(version_end_date__isnull=True))
          ]

And because I use DRF, my serializers.py looks like this:

from django.core.exceptions import ValidationError
from rest_framework import serializers
from dynamic_rest.serializers import DynamicModelSerializer

class VersionedWBSElementSerializer(DynamicModelSerializer):
    wbs_number = serializers.CharField()

    def validate_wbs_number(self, wbs_number):
        try:
            VersionedWBSElement(wbs_number=wbs_number).validate_unique()
       ## I added try-except in order to overwrite the error message
        except ValidationError as e:
            raise serializers.ValidationError("This WBS number already exists.")
        return wbs_number
maennel commented 5 years ago

Hi @simkimsia, sorry for the delay in replying to all your PRs and questions. Your engagement with Cleanerversion is deeply appreciated! Regarding the VERSION_UNIQUE statement, I'm afraid we didn't make it clear enough it will have an effect with PostgreSQL only:

If you are using a PostgreSQL data store, you may have skipped, that VERSION_UNIQUE indexes have to be created explicitely as in https://github.com/swisscom/cleanerversion/blob/master/versions_tests/apps.py#L20

Otherwise, the VERSION_UNIQUE keyword will not have any impact.

I hope this helps.

simkimsia commented 5 years ago

@maennel thanks for responding :)

I used to use django-simplehistory and also softdeletable in django-modelutils but I decided to experiment with cleanerversions and I am beginning to like it more :)

By the way, when you say the indexes have to be created explicitly, what exactly do you mean? I saw the tests code, but I am still confused.

maennel commented 5 years ago

Glad you like it! By "explicitely" I mean that you have to take care of it yourself in your code and CleanerVersion doesn't do it for you.

So, you'd have to create an object inheriting from AppConfig and override the ready() method, where you can make a call to the CleanerVersion-provided methods create_current_version_unique_indexes and create_current_version_unique_identity_indexes, similar to what we've done for the versions_tests app in our test-setup (https://github.com/swisscom/cleanerversion/blob/master/versions_tests/apps.py#L28).

That app config then gets added to INSTALLED_APPS (see https://github.com/swisscom/cleanerversion/blob/master/cleanerversion/settings/base.py#L79) so Django will take care of calling the ready method and adapting your PostgreSQL indexes.

simkimsia commented 5 years ago

@maennel I am still not too sure but I think I will give this a try as well. Thanks!