Skip to content

Commit 3b1b62e

Browse files
authored
Version: use field validator instead of custom field for slug (#11957)
Instead of having a custom field with some extra complexity, we can achieve the same result by using a more standard django pattern, populating the field if isn't defined and using a validator. This will help to re-use this code in #11930.
1 parent f5ee80c commit 3b1b62e

File tree

6 files changed

+141
-180
lines changed

6 files changed

+141
-180
lines changed

readthedocs/builds/migrations/0001_initial.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
import taggit.managers
22
from django.db import migrations, models
3+
from django_safemigrate import Safe
34

45
import readthedocs.builds.version_slug
56

67

78
class Migration(migrations.Migration):
9+
safe = Safe.always
810
dependencies = [
911
("projects", "0001_initial"),
1012
("taggit", "0001_initial"),
@@ -146,7 +148,6 @@ class Migration(migrations.Migration):
146148
(
147149
"slug",
148150
readthedocs.builds.version_slug.VersionSlugField(
149-
populate_from=b"verbose_name",
150151
max_length=255,
151152
verbose_name="Slug",
152153
db_index=True,

readthedocs/builds/migrations/0005_remove-version-alias.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
# Generated by Django 1.9.13 on 2018-10-17 04:20
22
from django.db import migrations, models
3+
from django_safemigrate import Safe
34

45
import readthedocs.builds.version_slug
56

67

78
class Migration(migrations.Migration):
9+
safe = Safe.always
810
dependencies = [
911
("builds", "0004_add-apiversion-proxy-model"),
1012
]
@@ -77,7 +79,6 @@ class Migration(migrations.Migration):
7779
field=readthedocs.builds.version_slug.VersionSlugField(
7880
db_index=True,
7981
max_length=255,
80-
populate_from="verbose_name",
8182
verbose_name="Slug",
8283
),
8384
),
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
# Generated by Django 4.2.17 on 2025-01-27 18:32
2+
3+
import django.core.validators
4+
from django.db import migrations, models
5+
from django_safemigrate import Safe
6+
7+
8+
class Migration(migrations.Migration):
9+
10+
safe = Safe.after_deploy
11+
12+
dependencies = [
13+
("builds", "0059_add_version_date_index"),
14+
]
15+
16+
operations = [
17+
migrations.AlterField(
18+
model_name="version",
19+
name="slug",
20+
field=models.CharField(
21+
db_index=True,
22+
help_text="A unique identifier used in the URL and links for this version. It can contain lowercase letters, numbers, dots, dashes or underscores. It must start with a lowercase letter or a number.",
23+
max_length=255,
24+
validators=[
25+
django.core.validators.RegexValidator(
26+
message="Enter a valid slug consisting of lowercase letters, numbers, dots, dashes or underscores. It must start with a letter or a number.",
27+
regex="^(?:[a-z0-9a-z][-._a-z0-9a-z]*?)$",
28+
)
29+
],
30+
verbose_name="Slug",
31+
),
32+
),
33+
]

readthedocs/builds/models.py

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,10 @@
5454
get_gitlab_username_repo,
5555
get_vcs_url,
5656
)
57-
from readthedocs.builds.version_slug import VersionSlugField
57+
from readthedocs.builds.version_slug import (
58+
generate_unique_version_slug,
59+
version_slug_validator,
60+
)
5861
from readthedocs.core.utils import extract_valid_attributes_for_model, trigger_build
5962
from readthedocs.notifications.models import Notification
6063
from readthedocs.projects.constants import (
@@ -118,10 +121,14 @@ class Version(TimeStampedModel):
118121
#: in the URL to identify this version in a project. It's also used in the
119122
#: filesystem to determine how the paths for this version are called. It
120123
#: must not be used for any other identifying purposes.
121-
slug = VersionSlugField(
124+
slug = models.CharField(
122125
_("Slug"),
123126
max_length=255,
124-
populate_from="verbose_name",
127+
validators=[version_slug_validator],
128+
db_index=True,
129+
help_text=_(
130+
"A unique identifier used in the URL and links for this version. It can contain lowercase letters, numbers, dots, dashes or underscores. It must start with a lowercase letter or a number."
131+
),
125132
)
126133

127134
# TODO: this field (`supported`) could be removed. It's returned only on
@@ -415,6 +422,11 @@ def clean_resources(self):
415422
self.built = False
416423
self.save()
417424

425+
def save(self, *args, **kwargs):
426+
if not self.slug:
427+
self.slug = generate_unique_version_slug(self.verbose_name, self)
428+
super().save(*args, **kwargs)
429+
418430
def post_save(self, was_active=False):
419431
"""
420432
Run extra steps after updating a version.

readthedocs/builds/version_slug.py

Lines changed: 84 additions & 169 deletions
Original file line numberDiff line numberDiff line change
@@ -22,24 +22,14 @@
2222
import string
2323
from operator import truediv
2424

25+
from django.core.validators import RegexValidator
2526
from django.db import models
26-
from django.utils.encoding import force_str
27+
from django.utils.translation import gettext_lazy as _
2728
from slugify import slugify as unicode_slugify
2829

2930

30-
def get_fields_with_model(cls):
31-
"""
32-
Replace deprecated function of the same name in Model._meta.
33-
34-
This replaces deprecated function (as of Django 1.10) in Model._meta as
35-
prescrived in the Django docs.
36-
https://docs.djangoproject.com/en/1.11/ref/models/meta/#migrating-from-the-old-api
37-
"""
38-
return [
39-
(f, f.model if f.model != cls else None)
40-
for f in cls._meta.get_fields()
41-
if not f.is_relation or f.one_to_one or (f.many_to_one and f.related_model)
42-
]
31+
class VersionSlugField(models.CharField):
32+
"""Just for backwards compatibility with old migrations."""
4333

4434

4535
# Regex breakdown:
@@ -50,164 +40,89 @@ def get_fields_with_model(cls):
5040
# regexes.
5141
VERSION_SLUG_REGEX = "(?:[a-z0-9A-Z][-._a-z0-9A-Z]*?)"
5242

43+
version_slug_validator = RegexValidator(
44+
# NOTE: we use the lower case version of the regex here,
45+
# since slugs are always lower case,
46+
# maybe we can change the VERSION_SLUG_REGEX itself,
47+
# but that may be a breaking change somewhere else...
48+
regex=f"^{VERSION_SLUG_REGEX.lower()}$",
49+
message=_(
50+
"Enter a valid slug consisting of lowercase letters, numbers, dots, dashes or underscores. It must start with a letter or a number."
51+
),
52+
)
53+
54+
55+
def generate_unique_version_slug(source, version):
56+
slug = generate_version_slug(source) or "unknown"
57+
queryset = version.project.versions.all()
58+
if version.pk:
59+
queryset = queryset.exclude(pk=version.pk)
60+
base_slug = slug
61+
iteration = 0
62+
while queryset.filter(slug=slug).exists():
63+
suffix = _uniquifying_suffix(iteration)
64+
slug = f"{base_slug}_{suffix}"
65+
iteration += 1
66+
return slug
67+
68+
69+
def generate_version_slug(source):
70+
normalized = _normalize(source)
71+
ok_chars = "-._" # dash, dot, underscore
72+
slugified = unicode_slugify(
73+
normalized,
74+
only_ascii=True,
75+
spaces=False,
76+
lower=True,
77+
ok=ok_chars,
78+
space_replacement="-",
79+
)
80+
# Remove first character wile it's an invalid character for the beginning of the slug.
81+
slugified = slugified.lstrip(ok_chars)
82+
return slugified
83+
84+
85+
def _normalize(value):
86+
"""
87+
Normalize some invalid characters (/, %, !, ?) to become a dash (``-``).
5388
54-
class VersionSlugField(models.CharField):
89+
.. note::
5590
91+
We replace these characters to a dash to keep compatibility with the
92+
old behavior and also because it makes this more readable.
93+
94+
For example, ``release/1.0`` will become ``release-1.0``.
5695
"""
57-
Inspired by ``django_extensions.db.fields.AutoSlugField``.
96+
return re.sub("[/%!?]", "-", value)
97+
5898

59-
Uses ``unicode-slugify`` to generate the slug.
99+
def _uniquifying_suffix(iteration):
60100
"""
101+
Create a unique suffix.
61102
62-
ok_chars = "-._" # dash, dot, underscore
63-
test_pattern = re.compile("^{pattern}$".format(pattern=VERSION_SLUG_REGEX))
64-
fallback_slug = "unknown"
65-
66-
def __init__(self, *args, **kwargs):
67-
kwargs.setdefault("db_index", True)
68-
69-
populate_from = kwargs.pop("populate_from", None)
70-
if populate_from is None:
71-
raise ValueError("missing 'populate_from' argument")
72-
73-
self._populate_from = populate_from
74-
super().__init__(*args, **kwargs)
75-
76-
def get_queryset(self, model_cls, slug_field):
77-
for field, model in get_fields_with_model(model_cls):
78-
if model and field == slug_field:
79-
return model._default_manager.all()
80-
return model_cls._default_manager.all()
81-
82-
def _normalize(self, content):
83-
"""
84-
Normalize some invalid characters (/, %, !, ?) to become a dash (``-``).
85-
86-
.. note::
87-
88-
We replace these characters to a dash to keep compatibility with the
89-
old behavior and also because it makes this more readable.
90-
91-
For example, ``release/1.0`` will become ``release-1.0``.
92-
"""
93-
return re.sub("[/%!?]", "-", content)
94-
95-
def slugify(self, content):
96-
"""
97-
Make ``content`` a valid slug.
98-
99-
It uses ``unicode-slugify`` behind the scenes which works properly with
100-
Unicode characters.
101-
"""
102-
if not content:
103-
return ""
104-
105-
normalized = self._normalize(content)
106-
slugified = unicode_slugify(
107-
normalized,
108-
only_ascii=True,
109-
spaces=False,
110-
lower=True,
111-
ok=self.ok_chars,
112-
space_replacement="-",
113-
)
114-
115-
# Remove first character wile it's an invalid character for the
116-
# beginning of the slug
117-
slugified = slugified.lstrip(self.ok_chars)
118-
119-
if not slugified:
120-
return self.fallback_slug
121-
return slugified
122-
123-
def uniquifying_suffix(self, iteration):
124-
"""
125-
Create a unique suffix.
126-
127-
This creates a suffix based on the number given as ``iteration``. It
128-
will return a value encoded as lowercase ascii letter. So we have an
129-
alphabet of 26 letters. The returned suffix will be for example ``_yh``
130-
where ``yh`` is the encoding of ``iteration``. The length of it will be
131-
``math.log(iteration, 26)``.
132-
133-
Examples::
134-
135-
uniquifying_suffix(0) == '_a'
136-
uniquifying_suffix(25) == '_z'
137-
uniquifying_suffix(26) == '_ba'
138-
uniquifying_suffix(52) == '_ca'
139-
"""
140-
alphabet = string.ascii_lowercase
141-
length = len(alphabet)
142-
if iteration == 0:
143-
power = 0
144-
else:
145-
power = int(math.log(iteration, length))
146-
current = iteration
147-
suffix = ""
148-
for exp in reversed(list(range(0, power + 1))):
149-
digit = int(truediv(current, length**exp))
150-
suffix += alphabet[digit]
151-
current = current % length**exp
152-
return "_{suffix}".format(suffix=suffix)
153-
154-
def create_slug(self, model_instance):
155-
"""Generate a unique slug for a model instance."""
156-
157-
# get fields to populate from and slug field to set
158-
slug_field = model_instance._meta.get_field(self.attname)
159-
160-
slug = self.slugify(getattr(model_instance, self._populate_from))
161-
count = 0
162-
163-
# strip slug depending on max_length attribute of the slug field
164-
# and clean-up
165-
slug_len = slug_field.max_length
166-
if slug_len:
167-
slug = slug[:slug_len]
168-
original_slug = slug
169-
170-
# exclude the current model instance from the queryset used in finding
171-
# the next valid slug
172-
queryset = self.get_queryset(model_instance.__class__, slug_field)
173-
if model_instance.pk:
174-
queryset = queryset.exclude(pk=model_instance.pk)
175-
176-
# form a kwarg dict used to implement any unique_together constraints
177-
kwargs = {}
178-
for params in model_instance._meta.unique_together:
179-
if self.attname in params:
180-
for param in params:
181-
kwargs[param] = getattr(model_instance, param, None)
182-
kwargs[self.attname] = slug
183-
184-
# increases the number while searching for the next valid slug
185-
# depending on the given slug, clean-up
186-
while not slug or queryset.filter(**kwargs).exists():
187-
slug = original_slug
188-
end = self.uniquifying_suffix(count)
189-
end_len = len(end)
190-
if slug_len and len(slug) + end_len > slug_len:
191-
slug = slug[: slug_len - end_len]
192-
slug = slug + end
193-
kwargs[self.attname] = slug
194-
count += 1
195-
196-
is_slug_valid = self.test_pattern.match(slug)
197-
if not is_slug_valid:
198-
# pylint: disable=broad-exception-raised
199-
raise Exception("Invalid generated slug: {slug}".format(slug=slug))
200-
return slug
201-
202-
def pre_save(self, model_instance, add):
203-
value = getattr(model_instance, self.attname)
204-
# We only create a new slug if none was set yet.
205-
if not value and add:
206-
value = force_str(self.create_slug(model_instance))
207-
setattr(model_instance, self.attname, value)
208-
return value
209-
210-
def deconstruct(self):
211-
name, path, args, kwargs = super().deconstruct()
212-
kwargs["populate_from"] = self._populate_from
213-
return name, path, args, kwargs
103+
This creates a suffix based on the number given as ``iteration``. It
104+
will return a value encoded as lowercase ascii letter. So we have an
105+
alphabet of 26 letters. The returned suffix will be for example ``yh``
106+
where ``yh`` is the encoding of ``iteration``. The length of it will be
107+
``math.log(iteration, 26)``.
108+
109+
Examples::
110+
111+
uniquifying_suffix(0) == 'a'
112+
uniquifying_suffix(25) == 'z'
113+
uniquifying_suffix(26) == 'ba'
114+
uniquifying_suffix(52) == 'ca'
115+
"""
116+
alphabet = string.ascii_lowercase
117+
length = len(alphabet)
118+
if iteration == 0:
119+
power = 0
120+
else:
121+
power = int(math.log(iteration, length))
122+
current = iteration
123+
suffix = ""
124+
for exp in reversed(list(range(0, power + 1))):
125+
digit = int(truediv(current, length**exp))
126+
suffix += alphabet[digit]
127+
current = current % length**exp
128+
return suffix

0 commit comments

Comments
 (0)