Skip to content

Commit

Permalink
Update the workspace model PK to be a uuid and drop the explicit uuid…
Browse files Browse the repository at this point in the history
… column

This would start a new convention with the v2 APIs and models in order to have
consistent, clear naming, particularly when it comes to FK references.

We currently have the `uuid` field as the self-referencial FK column on the `Workspace`
model. More details around the impetus for changing the naming around IDs can be
found in RedHatInsights#1257.

These changes offer an alternate approach, since we have no data in stage/production,
where we no longer use the `uuid` as the `lookup_field` in Django, but rather
use a `uuid` as the `id` format. The rationale for not doing this, and having an
explicit `uuid` was primarily for having sequential integers as the PK/FK relations.
However, UUID7 is a time-ordered UUID, eliminating index issues and solving the
need for having distributed ID values across our services.

We're using `uuid-utils` [1] which is a compliant implementation using Rust's
UUID library. There's also an open proposal [2,3] to add it to Python's standard
library.

This updates the model, view and serializer. In order to move the `id` from int
to uuid, we need two migrations:
- one to move the current `id` column, and the `parent` column (because of the FK ref)
  as well as making the current `uuid` column the PK
- a second to then rename the `uuid` column to `id` and add the `parent` FK ref/column back

[1] https://github.com/aminalaee/uuid-utils
[2] python/cpython#89083
[3] https://discuss.python.org/t/add-uuid7-in-uuid-module-in-standard-library/44390
  • Loading branch information
coderbydesign committed Oct 25, 2024
1 parent 592b9c2 commit 0d95c01
Show file tree
Hide file tree
Showing 8 changed files with 440 additions and 297 deletions.
1 change: 1 addition & 0 deletions Pipfile
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ joserfc = "==0.8.0"
jinja2 = "==3.1.4"
stompest = "==2.3.0"
xmltodict = "==0.13.0"
uuid-utils = "==0.9.0"

[dev-packages]
astroid = "==2.2.5"
Expand Down
621 changes: 352 additions & 269 deletions Pipfile.lock

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# Generated by Django 4.2.16 on 2024-10-24 20:26

from django.db import migrations, models
import uuid_utils.compat


class Migration(migrations.Migration):

dependencies = [
("management", "0056_alter_tenantmapping_tenant"),
]

operations = [
migrations.RemoveField(
model_name="workspace",
name="id",
),
migrations.RemoveField(
model_name="workspace",
name="parent",
),
migrations.AlterField(
model_name="workspace",
name="uuid",
field=models.UUIDField(
default=uuid_utils.compat.uuid7,
editable=False,
primary_key=True,
serialize=False,
unique=True,
),
),
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# Generated by Django 4.2.16 on 2024-10-24 20:27

from django.db import migrations, models
import django.db.models.deletion


class Migration(migrations.Migration):

dependencies = [
("management", "0057_remove_workspace_id_remove_workspace_parent_and_more"),
]

operations = [
migrations.RenameField(
model_name="workspace",
old_name="uuid",
new_name="id",
),
migrations.AddField(
model_name="workspace",
name="parent",
field=models.ForeignKey(
blank=True,
null=True,
on_delete=django.db.models.deletion.PROTECT,
related_name="children",
to="management.workspace",
),
),
]
26 changes: 11 additions & 15 deletions rbac/management/workspace/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@
# along with this program. If not, see <https://www.gnu.org/licenses/>.
#
"""Model for workspace management."""
from uuid import uuid4

import uuid_utils.compat as uuid
from django.db import models
from django.db.models import Q, UniqueConstraint
from django.utils import timezone
Expand All @@ -33,11 +32,9 @@ class Types(models.TextChoices):
DEFAULT = "default"
ROOT = "root"

id = models.UUIDField(primary_key=True, default=uuid.uuid7, editable=False, unique=True, null=False)
name = models.CharField(max_length=255)
uuid = models.UUIDField(default=uuid4, editable=False, unique=True, null=False)
parent = models.ForeignKey(
"self", to_field="uuid", on_delete=models.PROTECT, related_name="children", null=True, blank=True
)
parent = models.ForeignKey("self", on_delete=models.PROTECT, related_name="children", null=True, blank=True)
description = models.CharField(max_length=255, null=True, blank=True, editable=True)
type = models.CharField(choices=Types.choices, default=Types.STANDARD, null=False)
created = models.DateTimeField(default=timezone.now)
Expand All @@ -60,26 +57,25 @@ def save(self, *args, **kwargs):

def ancestors(self):
"""Return a list of ancestors for a Workspace instance."""
ancestor_ids = [a.uuid for a in self._ancestry_queryset() if a.uuid != self.uuid]
ancestors = Workspace.objects.filter(uuid__in=ancestor_ids)
ancestor_ids = [a.id for a in self._ancestry_queryset() if a.id != self.id]
ancestors = Workspace.objects.filter(id__in=ancestor_ids)
return ancestors

def _ancestry_queryset(self):
"""Return a raw queryset on the workspace model for ancestors."""
return Workspace.objects.raw(
"""
WITH RECURSIVE ancestors AS
(SELECT uuid,
(SELECT id,
parent_id
FROM management_workspace
WHERE uuid = %s
UNION SELECT w.uuid,
WHERE id = %s
UNION SELECT w.id,
w.parent_id
FROM management_workspace w
JOIN ancestors a ON w.uuid = a.parent_id)
SELECT uuid AS uuid,
uuid AS id
JOIN ancestors a ON w.id = a.parent_id)
SELECT id
FROM ancestors
""",
[self.uuid],
[self.id],
)
10 changes: 5 additions & 5 deletions rbac/management/workspace/serializer.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@
class WorkspaceSerializer(serializers.ModelSerializer):
"""Serializer for the Workspace model."""

id = serializers.UUIDField(read_only=True, required=False)
name = serializers.CharField(required=False, max_length=255)
uuid = serializers.UUIDField(read_only=True, required=False)
description = serializers.CharField(allow_null=True, required=False, max_length=255)
parent_id = serializers.UUIDField(allow_null=True, required=False)
created = serializers.DateTimeField(read_only=True)
Expand All @@ -38,7 +38,7 @@ class Meta:
model = Workspace
fields = (
"name",
"uuid",
"id",
"parent_id",
"description",
"created",
Expand All @@ -57,15 +57,15 @@ def create(self, validated_data):
class WorkspaceAncestrySerializer(serializers.ModelSerializer):
"""Serializer for the Workspace ancestry."""

id = serializers.UUIDField(read_only=True, required=False)
name = serializers.CharField(required=False, max_length=255)
uuid = serializers.UUIDField(read_only=True, required=False)
parent_id = serializers.UUIDField(allow_null=True, required=False)

class Meta:
"""Metadata for the serializer."""

model = Workspace
fields = ("name", "uuid", "parent_id")
fields = ("name", "id", "parent_id")


class WorkspaceWithAncestrySerializer(WorkspaceSerializer):
Expand All @@ -81,5 +81,5 @@ class Meta:

def get_ancestry(self, obj):
"""Serialize the workspace's ancestors."""
ancestors = obj.ancestors().only("name", "uuid", "parent_id")
ancestors = obj.ancestors().only("name", "id", "parent_id")
return WorkspaceAncestrySerializer(ancestors, many=True).data
7 changes: 3 additions & 4 deletions rbac/management/workspace/view.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ class WorkspaceViewSet(BaseV2ViewSet):

permission_classes = (WorkspaceAccessPermission,)
queryset = Workspace.objects.annotate()
lookup_field = "uuid"
serializer_class = WorkspaceSerializer
ordering_fields = ("name",)
ordering = ("name",)
Expand Down Expand Up @@ -115,8 +114,8 @@ def update_validation(self, request):
"""Validate a workspace for update."""
instance = self.get_object()
parent_id = request.data.get("parent_id")
if str(instance.uuid) == parent_id:
message = "Parent ID and UUID can't be same"
if str(instance.id) == parent_id:
message = "Parent ID and ID can't be same"
error = {"workspace": [_(message)]}
raise serializers.ValidationError(error)

Expand All @@ -142,7 +141,7 @@ def validate_workspace(self, request, action="create"):
raise serializers.ValidationError(error)
if parent_id:
validate_uuid(parent_id)
if not Workspace.objects.filter(uuid=parent_id, tenant=tenant).exists():
if not Workspace.objects.filter(id=parent_id, tenant=tenant).exists():
message = f"Parent workspace '{parent_id}' doesn't exist in tenant"
error = {"workspace": [message]}
raise serializers.ValidationError(error)
9 changes: 5 additions & 4 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ click==8.1.7; python_version >= '3.7'
click-didyoumean==0.3.1; python_full_version >= '3.6.2'
click-plugins==1.1.1
click-repl==0.3.0; python_version >= '3.6'
cryptography==43.0.1; python_version >= '3.7'
cryptography==43.0.3; python_version >= '3.7'
django==4.2.16; python_version >= '3.8'
django-cors-headers==3.13.0; python_version >= '3.7'
django-environ==0.10.0; python_version >= '3.5' and python_version < '4'
Expand All @@ -37,11 +37,11 @@ jmespath==1.0.1; python_version >= '3.7'
joserfc==0.8.0; python_version >= '3.8'
kafka-python==2.0.2
kombu==5.4.2; python_version >= '3.8'
markupsafe==3.0.1; python_version >= '3.9'
markupsafe==3.0.2; python_version >= '3.9'
packaging==24.1; python_version >= '3.8'
prometheus-client==0.15.0; python_version >= '3.6'
prompt-toolkit==3.0.48; python_full_version >= '3.7.0'
protobuf==5.29.0rc1; python_version >= '3.8'
protobuf==5.29.0rc2; python_version >= '3.8'
protoc-gen-validate==1.0.4; python_version >= '3.6'
psycopg2==2.9.5; python_version >= '3.6'
psycopg2-binary==2.9.5; python_version >= '3.6'
Expand All @@ -53,14 +53,15 @@ relations-grpc-clients-python-kessel-project==0.3.2; python_version >= '3.9'
requests==2.32.3; python_version >= '3.8'
s3transfer==0.6.2; python_version >= '3.7'
sentry-sdk==2.8.0; python_version >= '3.6'
setuptools==75.1.0; python_version >= '3.8'
setuptools==75.2.0; python_version >= '3.8'
six==1.16.0; python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2, 3.3'
sqlparse==0.5.0; python_version >= '3.8'
stompest==2.3.0
typing-extensions==4.12.2; python_version < '3.11'
tzdata==2022.2; python_version >= '2'
unicodecsv==0.14.1
urllib3==1.26.19; python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2, 3.3, 3.4, 3.5'
uuid-utils==0.9.0; python_version >= '3.8'
validate-email==1.3
vine==5.1.0; python_version >= '3.6'
watchtower==3.0.0; python_version >= '3.6'
Expand Down

0 comments on commit 0d95c01

Please sign in to comment.