sts-sponsors team mailing list archive

[Alberto Donato <mp+438007@xxxxxxxxxxxxxxxxxx>]

Alberto Donato has proposed merging ~ack/maas:custom-fields-cleanups into maas:master.

Commit message:
drop legacy model/form fields

 * replace EditableBinaryField with BinaryField(editable=True)
 * drop StrippedCharField form field
 * drop UnstrippedCharField form field



Requested reviews:
  MAAS Maintainers (maas-maintainers)

For more details, see:
https://code.launchpad.net/~ack/maas/+git/maas/+merge/438007
-- 
Your team MAAS Committers is subscribed to branch maas:master.

diff --git a/src/maasserver/fields.py b/src/maasserver/fields.py
index bc8277d..c2b238c 100644
--- a/src/maasserver/fields.py
+++ b/src/maasserver/fields.py
@@ -5,7 +5,6 @@
 
 __all__ = [
     "CIDRField",
-    "EditableBinaryField",
     "Field",
     "HostListFormField",
     "IPListFormField",
@@ -24,7 +23,6 @@ from django.core.exceptions import ObjectDoesNotExist, ValidationError
 from django.core.validators import RegexValidator, URLValidator
 from django.db import connections
 from django.db.models import (
-    BinaryField,
     CharField,
     Field,
     GenericIPAddressField,
@@ -59,34 +57,6 @@ MAC_ERROR_MSG = "'%(value)s' is not a valid MAC address."
 mac_validator = RegexValidator(regex=MAC_RE, message=MAC_ERROR_MSG)
 
 
-class StrippedCharField(forms.CharField):
-    """A CharField that will strip surrounding whitespace before validation."""
-
-    def clean(self, value):
-        value = self.to_python(value).strip()
-        return super().clean(value)
-
-
-class UnstrippedCharField(forms.CharField):
-    """A version of forms.CharField that never strips the whitespace.
-
-    Django 1.9 has introduced a strip argument that controls stripping of
-    whitespace *and* which defaults to True, thus breaking compatibility with
-    1.8 and earlier.
-    """
-
-    def __init__(self, *args, **kwargs):
-        # Instead of relying on a version check, we check for CharField
-        # constructor having a strip kwarg instead.
-        parent_init = super().__init__
-        if "strip" in parent_init.__code__.co_varnames:
-            parent_init(*args, strip=False, **kwargs)
-        else:
-            # In Django versions that do not support strip, False was the
-            # default.
-            parent_init(*args, **kwargs)
-
-
 class MACAddressFormField(forms.CharField):
     """Form field type: MAC address."""
 
@@ -128,23 +98,6 @@ class XMLField(Field):
         return "xml"
 
 
-class EditableBinaryField(BinaryField):
-    """An editable binary field.
-
-    An editable version of Django's BinaryField.
-    """
-
-    def __init__(self, *args, **kwargs):
-        super().__init__(*args, **kwargs)
-        self.editable = True
-
-    def deconstruct(self):
-        # Override deconstruct not to fail on the removal of the 'editable'
-        # field: the Django migration module assumes the field has its default
-        # value (False).
-        return Field.deconstruct(self)
-
-
 class LargeObjectFile:
     """Large object file.
 
diff --git a/src/maasserver/forms/__init__.py b/src/maasserver/forms/__init__.py
index 4fc8031..09ce9b8 100644
--- a/src/maasserver/forms/__init__.py
+++ b/src/maasserver/forms/__init__.py
@@ -111,11 +111,7 @@ from maasserver.enum import (
     NODE_STATUS,
     NODE_TYPE,
 )
-from maasserver.fields import (
-    LargeObjectFile,
-    MACAddressFormField,
-    UnstrippedCharField,
-)
+from maasserver.fields import LargeObjectFile, MACAddressFormField
 from maasserver.forms.settings import (
     CONFIG_ITEMS_KEYS,
     get_config_field,
@@ -1293,11 +1289,7 @@ class KeyForm(MAASModelForm):
 
 
 class SSHKeyForm(MAASModelForm):
-    key = UnstrippedCharField(
-        label="Public key",
-        widget=forms.Textarea(attrs={"rows": "5", "cols": "30"}),
-        required=True,
-    )
+    key = forms.CharField(label="Public key", strip=False)
 
     class Meta:
         model = SSHKey
@@ -1324,11 +1316,7 @@ class SSHKeyForm(MAASModelForm):
 
 
 class SSLKeyForm(KeyForm):
-    key = UnstrippedCharField(
-        label="SSL key",
-        widget=forms.Textarea(attrs={"rows": "15", "cols": "30"}),
-        required=True,
-    )
+    key = forms.CharField(label="SSL key", strip=False)
 
     class Meta:
         model = SSLKey
diff --git a/src/maasserver/forms/filesystem.py b/src/maasserver/forms/filesystem.py
index 3d3d7ba..0e4c38b 100644
--- a/src/maasserver/forms/filesystem.py
+++ b/src/maasserver/forms/filesystem.py
@@ -11,10 +11,9 @@ __all__ = [
 
 from typing import Optional
 
-from django.forms import ChoiceField, Form
+from django.forms import CharField, ChoiceField, Form
 
 from maasserver.enum import FILESYSTEM_FORMAT_TYPE_CHOICES
-from maasserver.fields import StrippedCharField
 from maasserver.forms import AbsolutePathField
 from maasserver.models import Filesystem, Node
 
@@ -31,7 +30,7 @@ class MountFilesystemForm(Form):
         if self.filesystem is not None:
             if self.filesystem.uses_mount_point:
                 self.fields["mount_point"] = AbsolutePathField(required=True)
-            self.fields["mount_options"] = StrippedCharField(required=False)
+            self.fields["mount_options"] = CharField(required=False)
 
     def clean(self):
         cleaned_data = super().clean()
@@ -61,7 +60,7 @@ class MountNonStorageFilesystemForm(Form):
     """Form used to create and mount a non-storage filesystem."""
 
     mount_point = AbsolutePathField(required=True)
-    mount_options = StrippedCharField(required=False)
+    mount_options = CharField(required=False)
     fstype = ChoiceField(
         required=True,
         choices=[
diff --git a/src/maasserver/migrations/fields.py b/src/maasserver/migrations/fields.py
index 517b927..2ccad90 100644
--- a/src/maasserver/migrations/fields.py
+++ b/src/maasserver/migrations/fields.py
@@ -5,7 +5,21 @@
 from copy import deepcopy
 import json
 
-from django.db.models import Field
+from django.db.models import BinaryField, Field
+
+
+class EditableBinaryField(BinaryField):
+    """An editable binary field."""
+
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        self.editable = True
+
+    def deconstruct(self):
+        # Override deconstruct not to fail on the removal of the 'editable'
+        # field: the Django migration module assumes the field has its default
+        # value (False).
+        return Field.deconstruct(self)
 
 
 class JSONObjectField(Field):
diff --git a/src/maasserver/migrations/maasserver/0001_initial.py b/src/maasserver/migrations/maasserver/0001_initial.py
index 2fadac9..ddff643 100644
--- a/src/maasserver/migrations/maasserver/0001_initial.py
+++ b/src/maasserver/migrations/maasserver/0001_initial.py
@@ -231,7 +231,7 @@ class Migration(migrations.Migration):
                 ),
                 (
                     "keyring_data",
-                    maasserver.fields.EditableBinaryField(
+                    maasserver.migrations.fields.EditableBinaryField(
                         help_text="The GPG keyring for this BootSource, as a binary blob.",
                         blank=True,
                     ),
diff --git a/src/maasserver/migrations/maasserver/0294_keyring_data_binary_field.py b/src/maasserver/migrations/maasserver/0294_keyring_data_binary_field.py
new file mode 100644
index 0000000..9018667
--- /dev/null
+++ b/src/maasserver/migrations/maasserver/0294_keyring_data_binary_field.py
@@ -0,0 +1,21 @@
+# Generated by Django 3.2.12 on 2023-02-28 11:36
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("maasserver", "0293_drop_verbose_regex_validator"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="bootsource",
+            name="keyring_data",
+            field=models.BinaryField(
+                blank=True,
+                editable=True,
+                help_text="The GPG keyring for this BootSource, as a binary blob.",
+            ),
+        ),
+    ]
diff --git a/src/maasserver/models/bootsource.py b/src/maasserver/models/bootsource.py
index 926f483..d38335d 100644
--- a/src/maasserver/models/bootsource.py
+++ b/src/maasserver/models/bootsource.py
@@ -5,9 +5,8 @@
 
 
 from django.core.exceptions import ValidationError
-from django.db.models import CharField, URLField
+from django.db.models import BinaryField, CharField, URLField
 
-from maasserver.fields import EditableBinaryField
 from maasserver.models.cleansave import CleanSave
 from maasserver.models.timestampedmodel import TimestampedModel
 
@@ -25,9 +24,10 @@ class BootSource(CleanSave, TimestampedModel):
         help_text="The path to the keyring file for this BootSource.",
     )
 
-    keyring_data = EditableBinaryField(
+    keyring_data = BinaryField(
         blank=True,
         help_text="The GPG keyring for this BootSource, as a binary blob.",
+        editable=True,
     )
 
     def clean(self, *args, **kwargs):
diff --git a/src/maasserver/tests/test_fields.py b/src/maasserver/tests/test_fields.py
index ea6ce96..24014da 100644
--- a/src/maasserver/tests/test_fields.py
+++ b/src/maasserver/tests/test_fields.py
@@ -6,13 +6,11 @@ import re
 
 from django.core.exceptions import ValidationError
 from django.db import connection, DatabaseError
-from django.db.models import BinaryField
 from psycopg2 import OperationalError
 from testtools import ExpectedException
 
 from maasserver.enum import INTERFACE_TYPE
 from maasserver.fields import (
-    EditableBinaryField,
     HostListFormField,
     IPListFormField,
     LargeObjectField,
@@ -109,14 +107,6 @@ class TestXMLField(MAASLegacyServerTestCase):
         self.assertIsNone(test_instance.value)
 
 
-class TestEditableBinaryField(MAASServerTestCase):
-    def test_is_BinaryField(self):
-        self.assertIsInstance(EditableBinaryField(), BinaryField)
-
-    def test_is_editable(self):
-        self.assertTrue(EditableBinaryField().editable)
-
-
 class TestLargeObjectField(MAASLegacyServerTestCase):
     apps = ["maasserver.tests"]