From 951a186f144d98e8de549ab9cee7ac529e35fd22 Mon Sep 17 00:00:00 2001
From: Samuel Gaist <samuel.gaist@idiap.ch>
Date: Thu, 9 Apr 2020 17:52:13 +0200
Subject: [PATCH] [common][serializers] Refactor creation serializer and a
version as a mandatory field
The original code would automatically generate a number based on
the previous version if any or set it to one. This has the drawback
of generating cryptic errors if for example someone sends an asset
with a version number higher than the one available on the platform
without providing historical data.
To avoid that, version is now a mandatory field. When doing local
development, it's already assigned. This allows for better handling
of posted data.
The refactored code makes only use of the database rather than mixing
prefix and database usage.
---
beat/web/common/serializers.py | 181 ++++++++++++++-------------------
1 file changed, 76 insertions(+), 105 deletions(-)
diff --git a/beat/web/common/serializers.py b/beat/web/common/serializers.py
index a404df168..05344e4da 100644
--- a/beat/web/common/serializers.py
+++ b/beat/web/common/serializers.py
@@ -25,11 +25,13 @@
# #
###############################################################################
-from django.conf import settings
from django.contrib.auth.models import User
from django.utils import six
+from django.db.models import CharField, Value as V
+from django.db.models.functions import Concat
+
from rest_framework import serializers
from ..team.models import Team
@@ -295,41 +297,12 @@ class ContributionSerializer(VersionableSerializer):
# ----------------------------------------------------------
-class MapDot(dict):
- def __init__(self, *args, **kwargs):
- super(MapDot, self).__init__(*args, **kwargs)
- for arg in args:
- if isinstance(arg, dict):
- for k, v in arg.items():
- self[k] = v
-
- if kwargs:
- for k, v in kwargs.items():
- self[k] = v
-
- def __getattr__(self, attr):
- return self.get(attr)
-
- def __setattr__(self, key, value):
- self.__setitem__(key, value)
-
- def __setitem__(self, key, value):
- super(MapDot, self).__setitem__(key, value)
- self.__dict__.update({key: value})
-
- def __delattr__(self, item):
- self.__delitem__(item)
-
- def __delitem__(self, key):
- super(MapDot, self).__delitem__(key)
- del self.__dict__[key]
-
-
class ContributionCreationSerializer(serializers.ModelSerializer):
declaration = JSONSerializerField(required=False)
description = serializers.CharField(required=False, allow_blank=True)
fork_of = serializers.JSONField(required=False)
previous_version = serializers.CharField(required=False)
+ version = serializers.IntegerField(min_value=1)
class Meta:
fields = [
@@ -339,9 +312,24 @@ class ContributionCreationSerializer(serializers.ModelSerializer):
"declaration",
"previous_version",
"fork_of",
+ "version",
]
beat_core_class = None
+ def validate_fork_of(self, fork_of):
+ if "previous_version" in self.initial_data:
+ raise serializers.ValidationError(
+ "fork_of and previous_version cannot appear together"
+ )
+ return fork_of
+
+ def validate_previous_version(self, previous_version):
+ if "fork_of" in self.initial_data:
+ raise serializers.ValidationError(
+ "previous_version and fork_of cannot appear together"
+ )
+ return previous_version
+
def validate_description(self, description):
if description.find("\\") >= 0: # was escaped, unescape
description = description.decode("string_escape")
@@ -351,102 +339,85 @@ class ContributionCreationSerializer(serializers.ModelSerializer):
user = self.context.get("user")
name = self.Meta.model.sanitize_name(data["name"])
data["name"] = name
+ version = data.get("version")
- if "previous_version" in data:
- if self.Meta.beat_core_class is not None:
- previous_version_asset = self.Meta.beat_core_class.Storage(
- settings.PREFIX, data["previous_version"]
+ # If version is not one then it's necessarily a new version
+ # forks start at one
+ if version > 1 and "previous_version" not in data:
+ raise serializers.ValidationError(
+ "{} {} version {} incomplete history data posted".format(
+ self.Meta.model.__name__.lower(), name, version
)
- if previous_version_asset.username is None:
- previous_version_asset.username = user.username
- else:
- previous_version_asset = MapDot()
- previous_version_asset["username"] = user.username
- previous_version_asset["name"] = name
- previous_version_asset["version"] = data["previous_version"]
- data["data"] = json.dumps(data["data"])
-
- else:
- previous_version_asset = None
+ )
- if "fork_of" in data:
- if self.Meta.beat_core_class is not None:
- fork_of_asset = self.Meta.beat_core_class.Storage(
- settings.PREFIX, data["fork_of"]
+ if self.Meta.model.objects.filter(
+ author__username__iexact=user, name=name, version=version
+ ).exists():
+ raise serializers.ValidationError(
+ "{} {} version {} already exists on this account".format(
+ self.Meta.model.__name__.lower(), name, version
)
- if fork_of_asset.username is None:
- fork_of_asset.username = user.username
- else:
- fork_of_asset = MapDot()
- fork_elem = data["fork_of"]
- fork_of_asset["username"] = fork_elem["username"]
- fork_of_asset["name"] = fork_elem["name"]
- fork_of_asset["version"] = fork_elem["version"]
- data["data"] = json.dumps(data["data"])
+ )
- else:
- fork_of_asset = None
+ previous_version = data.get("previous_version")
+ fork_of = data.get("fork_of")
- # Retrieve the previous version (if applicable)
- if previous_version_asset is not None:
+ if previous_version is not None:
try:
- previous_version = self.Meta.model.objects.get(
- author__username__iexact=previous_version_asset.username,
- name=previous_version_asset.name,
- version=previous_version_asset.version,
- )
+ previous_object = self.Meta.model.objects.annotate(
+ fullname=Concat(
+ "author__username",
+ V("/"),
+ "name",
+ V("/"),
+ "version",
+ output_field=CharField(),
+ )
+ ).get(fullname=previous_version)
except self.Meta.model.DoesNotExist:
raise serializers.ValidationError(
"{} '{}' not found".format(
- self.Meta.model.__name__, previous_version_asset.fullname
+ self.Meta.model.__name__, previous_version
)
)
-
- has_access, _, _ = previous_version.accessibility_for(user)
- if not has_access:
+ accessibility_infos = previous_object.accessibility_for(user)
+ if not accessibility_infos.has_access:
raise serializers.ValidationError("No access allowed")
- data["previous_version"] = previous_version
- # Retrieve the forked algorithm (if applicable)
- if fork_of_asset is not None:
- try:
- fork_of = self.Meta.model.objects.get(
- author__username__iexact=fork_of_asset.username,
- name=fork_of_asset.name,
- version=fork_of_asset.version,
- )
- except self.Meta.model.DoesNotExist:
+ if version - previous_object.version != 1:
raise serializers.ValidationError(
- "{} '{}' not found".format(
- self.Meta.model.__name__, fork_of_asset.fullname
+ "The requested version ({}) for this {} does not match"
+ "the standard increment with {}".format(
+ version, self.Meta.model.__name__, previous_object.version
)
)
+ data["previous_version"] = previous_object
- has_access, _, _ = fork_of.accessibility_for(user)
- if not has_access:
- raise serializers.ValidationError("No access allowed")
- data["fork_of"] = fork_of
-
- # Determine the version number
- last_version = None
-
- if previous_version_asset is not None:
- if (previous_version_asset.username == user.username) and (
- previous_version_asset.name == name
- ):
- last_version = self.Meta.model.objects.filter(
- author=user, name=name
- ).order_by("-version")[0]
+ elif fork_of is not None:
+ if version > 1:
+ raise serializers.ValidationError("A fork starts at 1")
- if last_version is None:
- if self.Meta.model.objects.filter(author=user, name=name).count() > 0:
+ try:
+ forked_of_object = self.Meta.model.objects.annotate(
+ fullname=Concat(
+ "author__username",
+ V("/"),
+ "name",
+ V("/"),
+ "version",
+ output_field=CharField(),
+ )
+ ).get(fullname=fork_of)
+ except self.Meta.model.DoesNotExist:
raise serializers.ValidationError(
- "This {} name already exists on this account".format(
- self.Meta.model.__name__.lower()
+ "{} '{}' fork origin not found".format(
+ self.Meta.model.__name__, fork_of
)
)
-
- data["version"] = last_version.version + 1 if last_version is not None else 1
+ accessibility_infos = forked_of_object.accessibility_for(user)
+ if not accessibility_infos.has_access:
+ raise serializers.ValidationError("No access allowed")
+ data["fork_of"] = forked_of_object
return data
--
GitLab