Metadata fix (#4725)

* Add 'clean' method to MetadataMixin class

- Ensure that the "metadata" is a valid dict object

* Add "overwrite" option for set_metadata method

* Update unit tests

* full_clean -> clean

* Cleanup

* Fix for MetadataMixin

* Updates for unit tests

* Test

InvenTree/InvenTree/models.py

@@ -60,6 +60,27 @@ class MetadataMixin(models.Model):
         """Meta for MetadataMixin."""
         abstract = True
 
+    def save(self, *args, **kwargs):
+        """Save the model instance, and perform validation on the metadata field."""
+        self.validate_metadata()
+        super().save(*args, **kwargs)
+
+    def clean(self, *args, **kwargs):
+        """Perform model validation on the metadata field."""
+        super().clean()
+
+        self.validate_metadata()
+
+    def validate_metadata(self):
+        """Validate the metadata field."""
+
+        # Ensure that the 'metadata' field is a valid dict object
+        if self.metadata is None:
+            self.metadata = {}
+
+        if type(self.metadata) is not dict:
+            raise ValidationError({'metadata': _('Metadata must be a python dict object')})
+
     metadata = models.JSONField(
         blank=True, null=True,
         verbose_name=_('Plugin Metadata'),
@@ -80,16 +101,16 @@ class MetadataMixin(models.Model):
 
         return self.metadata.get(key, backup_value)
 
-    def set_metadata(self, key: str, data, commit: bool = True):
+    def set_metadata(self, key: str, data, commit: bool = True, overwrite: bool = False):
         """Save the provided metadata under the provided key.
 
         Args:
             key (str): Key for saving metadata
             data (Any): Data object to save - must be able to be rendered as a JSON string
             commit (bool, optional): If true, existing metadata with the provided key will be overwritten. If false, a merge will be attempted. Defaults to True.
+            overwrite (bool): If true, delete existing metadata before adding new value
         """
-        if self.metadata is None:
-            # Handle a null field value
+        if overwrite or self.metadata is None:
             self.metadata = {}
 
         self.metadata[key] = data

InvenTree/build/test_build.py

@@ -579,7 +579,7 @@ class BuildTest(BuildTestBase):
 
         for model in [Build, BuildItem]:
             p = model.objects.first()
-            self.assertIsNone(p.metadata)
+            self.assertEqual(len(p.metadata.keys()), 0)
 
             self.assertIsNone(p.get_metadata('test'))
             self.assertEqual(p.get_metadata('test', backup_value=123), 123)

InvenTree/company/tests.py

@@ -135,7 +135,7 @@ class CompanySimpleTest(TestCase):
     def test_metadata(self):
         """Unit tests for the metadata field."""
         p = Company.objects.first()
-        self.assertIsNone(p.metadata)
+        self.assertIn(p.metadata, [None, {}])
 
         self.assertIsNone(p.get_metadata('test'))
         self.assertEqual(p.get_metadata('test', backup_value=123), 123)
@@ -227,7 +227,7 @@ class ManufacturerPartSimpleTest(TestCase):
         """Unit tests for the metadata field."""
         for model in [ManufacturerPart, SupplierPart]:
             p = model.objects.first()
-            self.assertIsNone(p.metadata)
+            self.assertIn(p.metadata, [None, {}])
 
             self.assertIsNone(p.get_metadata('test'))
             self.assertEqual(p.get_metadata('test', backup_value=123), 123)

InvenTree/label/tests.py

@@ -135,7 +135,6 @@ class LabelTest(InvenTreeAPITestCase):
         """Unit tests for the metadata field."""
         for model in [StockItemLabel, StockLocationLabel, PartLabel]:
             p = model.objects.first()
-            self.assertIsNone(p.metadata)
 
             self.assertIsNone(p.get_metadata('test'))
             self.assertEqual(p.get_metadata('test', backup_value=123), 123)

InvenTree/order/test_sales_order.py

@@ -303,8 +303,6 @@ class SalesOrderTest(TestCase):
         for model in [SalesOrder, SalesOrderLineItem, SalesOrderExtraLine, SalesOrderShipment]:
             p = model.objects.first()
 
-            self.assertIsNone(p.metadata)
-
             self.assertIsNone(p.get_metadata('test'))
             self.assertEqual(p.get_metadata('test', backup_value=123), 123)
 

InvenTree/order/tests.py

@@ -390,7 +390,14 @@ class OrderTest(TestCase):
         """Unit tests for the metadata field."""
         for model in [PurchaseOrder, PurchaseOrderLineItem, PurchaseOrderExtraLine]:
             p = model.objects.first()
-            self.assertIsNone(p.metadata)
+
+            # Setting metadata to something *other* than a dict will fail
+            with self.assertRaises(django_exceptions.ValidationError):
+                p.metadata = 'test'
+                p.save()
+
+            # Reset metadata to known state
+            p.metadata = {}
 
             self.assertIsNone(p.get_metadata('test'))
             self.assertEqual(p.get_metadata('test', backup_value=123), 123)

InvenTree/part/test_bom_item.py

@@ -250,7 +250,6 @@ class BomItemTest(TestCase):
         """Unit tests for the metadata field."""
         for model in [BomItem]:
             p = model.objects.first()
-            self.assertIsNone(p.metadata)
 
             self.assertIsNone(p.get_metadata('test'))
             self.assertEqual(p.get_metadata('test', backup_value=123), 123)

InvenTree/part/test_param.py

@@ -47,7 +47,6 @@ class TestParams(TestCase):
         """Unit tests for the metadata field."""
         for model in [PartParameterTemplate]:
             p = model.objects.first()
-            self.assertIsNone(p.metadata)
 
             self.assertIsNone(p.get_metadata('test'))
             self.assertEqual(p.get_metadata('test', backup_value=123), 123)

InvenTree/part/test_part.py

@@ -277,7 +277,6 @@ class PartTest(TestCase):
         """Unit tests for the metadata field."""
         for model in [Part]:
             p = model.objects.first()
-            self.assertIsNone(p.metadata)
 
             self.assertIsNone(p.get_metadata('test'))
             self.assertEqual(p.get_metadata('test', backup_value=123), 123)

InvenTree/report/tests.py

@@ -299,7 +299,7 @@ class ReportTest(InvenTreeAPITestCase):
         if self.model is not None:
             p = self.model.objects.first()
 
-            self.assertIsNone(p.metadata)
+            self.assertEqual(p.metadata, {})
 
             self.assertIsNone(p.get_metadata('test'))
             self.assertEqual(p.get_metadata('test', backup_value=123), 123)

InvenTree/stock/tests.py

@@ -921,7 +921,6 @@ class StockTest(StockTestBase):
         """Unit tests for the metadata field."""
         for model in [StockItem, StockLocation]:
             p = model.objects.first()
-            self.assertIsNone(p.metadata)
 
             self.assertIsNone(p.get_metadata('test'))
             self.assertEqual(p.get_metadata('test', backup_value=123), 123)