TMSS-272: processed review comments

LCS/PyCommon/test/postgres.py

@@ -39,7 +39,7 @@ class PostgresTestDatabaseInstance():
     Best used in a 'with'-context so the server is destroyed automagically.
     Derive your own sub-class and implement apply_database_schema with your own sql schema to setup your type of database.
     '''
-    _named_lock = NamedAtomicLock('PostgresTestDatabaseInstance', maxLockAge=60)
+    _named_lock = NamedAtomicLock('PostgresTestDatabaseInstance')
 
     def __init__(self, user: str = 'test_user', preferred_port: int=5444) -> None:
         self._postgresql = None

LCS/PyCommon/test/t_json_utils.py

@@ -23,7 +23,8 @@ logging.basicConfig(format='%(asctime)s %(process)s %(threadName)s %(levelname)s
 
 import unittest
 import threading
-from lofar.common.json_utils import *
+import json
+from lofar.common.json_utils import get_default_json_object_for_schema, replace_host_in_urls, resolved_refs
 
 class TestJSONUtils(unittest.TestCase):
     def test_empty_schema_yields_empty_object(self):
@@ -94,8 +95,14 @@ class TestJSONUtils(unittest.TestCase):
                                "type": "string",
                                "minLength": 2 },
                                 "email": {
-                                    "$ref": base_url + "/base_schema.json" + "#/definitions/email"  }
-                            } }
+                                    "$ref": base_url + "/base_schema.json" + "#/definitions/email"
+                                },
+                           "other_emails": {
+                               "type": "array",
+                               "items": {
+                                    "$ref": base_url + "/base_schema.json" + "#/definitions/email"
+                                }
+                           } } }
 
         class TestRequestHandler(http.server.BaseHTTPRequestHandler):
             '''helper class to serve the schemas via http. Needed for resolving the $ref URLs'''
@@ -146,7 +153,13 @@ class TestJSONUtils(unittest.TestCase):
                           "type": "string",
                           "minLength": 2 },
                       "email": {
-                          "$ref": base_host + path + "/base_schema.json" + "/#definitions/email"  }
+                          "$ref": base_host + path + "/base_schema.json" + "#/definitions/email"  },
+                       "other_emails": {
+                           "type": "array",
+                           "items": {
+                                "$ref": base_host + path + "/base_schema.json" + "#/definitions/email"
+                            }
+                       }
                   } }
 
         new_base_host = 'http://127.0.0.1'
@@ -156,7 +169,8 @@ class TestJSONUtils(unittest.TestCase):
         print('url_fixed_schema: ', json.dumps(url_fixed_schema, indent=2))
 
         self.assertEqual(new_base_host+path+"/user_schema.json", url_fixed_schema['$id'])
-        self.assertEqual(new_base_host+path+"/base_schema.json" + "/#definitions/email", url_fixed_schema['properties']['email']['$ref'])
+        self.assertEqual(new_base_host+path+"/base_schema.json" + "#/definitions/email", url_fixed_schema['properties']['email']['$ref'])
+        self.assertEqual(new_base_host+path+"/base_schema.json" + "#/definitions/email", url_fixed_schema['properties']['other_emails']['items']['$ref'])
         self.assertEqual("http://json-schema.org/draft-06/schema#", url_fixed_schema['$schema'])
         self.assertEqual(json.dumps(schema, indent=2).replace(base_host, new_base_host), json.dumps(url_fixed_schema, indent=2))
 

SAS/TMSS/src/tmss/settings.py

@@ -70,7 +70,7 @@ LOGGING = {
         },
         'django.server': {
             'handlers': ['django.server'],
-            'level': 'DEBUG',
+            'level': 'INFO',
             'propagate': False,
         },
         'django_auth_ldap': {

SAS/TMSS/src/tmss/tmssapp/models/specification.py

@@ -301,7 +301,7 @@ class Template(NamedCommon):
     def save(self, force_insert=False, force_update=False, using=None, update_fields=None):
         self.auto_set_version_number()
         self.validate_and_annotate_schema()
-        super().save(force_insert, force_update, using, update_fields)
+        super().save(force_insert or self.pk is None, force_update, using, update_fields)
 
 
 # concrete models

SAS/TMSS/test/ldap_test_service.py

@@ -27,7 +27,7 @@ class TestLDAPServer():
     ''' A helper class which instantiates a running LDAP server (not interfering with any other test/production LDAP servers)
     Best used in a 'with'-context so the server is stoped automagically.
     '''
-    _named_lock = NamedAtomicLock('TestLDAPServer', maxLockAge=60)
+    _named_lock = NamedAtomicLock('TestLDAPServer')
 
     def __init__(self, user: str = 'test', password: str = 'test') -> None:
         self._tmp_creds = TemporaryCredentials(user=user, password=password)

SAS/TMSS/test/t_tmssapp_specification_django_API.py

@@ -40,7 +40,7 @@ from lofar.sas.tmss.test.tmss_test_data_django_models import *
 
 from django.db.utils import IntegrityError
 from django.core.exceptions import ValidationError
-
+from lofar.sas.tmss.tmss.exceptions import SchemaValidationException
 
 class GeneratorTemplateTest(unittest.TestCase):
     def test_GeneratorTemplate_gets_created_with_correct_creation_timestamp(self):
@@ -131,24 +131,83 @@ class TaskTemplateTest(unittest.TestCase):
         self.assertLess(before, entry.updated_at)
         self.assertGreater(after, entry.updated_at)
 
+    def test_TaskTemplate_incorrect_schema_raises(self):
+        with self.assertRaises(SchemaValidationException):
+            models.TaskTemplate.objects.create(**TaskTemplate_test_data(schema=""))
+
+        with self.assertRaises(SchemaValidationException) as context:
+            models.TaskTemplate.objects.create(**TaskTemplate_test_data(schema={}))
+        self.assertTrue(True)
+
+        with self.assertRaises(SchemaValidationException) as context:
+            schema = minimal_json_schema()
+            del schema['$schema']
+            models.TaskTemplate.objects.create(**TaskTemplate_test_data(schema=schema))
+        self.assertTrue("Missing required properties" in str(context.exception))
+
+        with self.assertRaises(SchemaValidationException) as context:
+            models.TaskTemplate.objects.create(**TaskTemplate_test_data(schema= minimal_json_schema(id="my id with no url")))
+        self.assertTrue("should contain a valid URL" in str(context.exception))
+
+    def test_TaskTemplate_annotated_schema(self):
+        schema = minimal_json_schema()
+        data = TaskTemplate_test_data(schema=schema, name="foo", description="bar")
+        template = models.TaskTemplate.objects.create(**data)
+        self.assertEqual("foo", template.name)
+        self.assertEqual("foo", template.schema['title'])
+        self.assertEqual("bar", template.description)
+        self.assertEqual("bar", template.schema['description'])
+
+
     def test_TaskTemplate_name_version_unique(self):
-        test_data = TaskTemplate_test_data(name=str(uuid.uuid4()))
+        name = str(uuid.uuid4())
+        self.assertEqual(0, models.TaskTemplate.objects.filter(name=name).count())
+        test_data = TaskTemplate_test_data(name=name)
         # save data twice
         entry1 = models.TaskTemplate.objects.create(**test_data)
         entry2 = models.TaskTemplate.objects.create(**test_data)
+        self.assertEqual(2, models.TaskTemplate.objects.filter(name=name).count())
 
         self.assertEqual(1, entry1.version)
         self.assertEqual(2, entry2.version) #version is autoincremented
 
         # try to modify version... should be allowed, cause the template is not used, but should raise IntegrityError (unique constraint)
+        self.assertFalse(entry2.is_used)
         with self.assertRaises(IntegrityError):
             entry2.version = 1
             entry2.save()
         entry2.refresh_from_db()
 
+        # versions still the same?
         self.assertEqual(1, entry1.version)
         self.assertEqual(2, entry2.version)
 
+        # let's use the template in a task
+        models.TaskDraft.objects.create(**TaskDraft_test_data(specifications_template=entry2))
+        self.assertTrue(entry2.is_used)
+
+        # there should still be only 2 templates with this name
+        self.assertEqual(2, models.TaskTemplate.objects.filter(name=name).count())
+
+        # now (try to) modify the template
+        org_pk = entry2.pk
+        org_schema = dict(entry2.schema)
+        new_schema = minimal_json_schema(properties={"new_prop":{"type":"string"}})
+        entry2.schema = new_schema
+        entry2.save()
+        #this should now be a NEW instance
+        self.assertNotEqual(org_pk, entry2.pk)
+        self.assertEqual(3, models.TaskTemplate.objects.filter(name=name).count())
+
+        # lets request the "old" entry2 via name and version, so we can check if it is unchanged
+        entry2 = models.TaskTemplate.objects.get(name=name, version=2)
+        self.assertEqual(org_schema, entry2.schema)
+
+        # instead there should be a new version of the template with the new schema
+        entry3 = models.TaskTemplate.objects.get(name=name, version=3)
+        self.assertEqual(3, entry3.version)
+        self.assertEqual(new_schema, entry3.schema)
+
 
 class TaskRelationSelectionTemplateTest(unittest.TestCase):
     def test_TaskRelationSelectionTemplate_gets_created_with_correct_creation_timestamp(self):

SAS/TMSS/test/tmss_test_data_django_models.py

@@ -72,12 +72,15 @@ def SchedulingUnitObservingStrategyTemplate_test_data(name="my_SchedulingUnitObs
             "scheduling_unit_template": scheduling_unit_template,
             "tags": ["TMSS", "TESTING"]}
 
-def TaskTemplate_test_data(name="my TaskTemplate") -> dict:
+def TaskTemplate_test_data(name="my TaskTemplate", description:str=None, schema:dict=None) -> dict:
+    if schema is None:
+        schema = minimal_json_schema(properties={"mykey": {}})
+
     return {"type": models.TaskType.objects.get(value='observation'),
             "validation_code_js":"",
             "name": name,
-            "description": 'My TaskTemplate description',
-            "schema": minimal_json_schema(properties={"mykey": {}}),
+            "description": description or "<no description>",
+            "schema": schema,
             "tags": ["TMSS", "TESTING"]}
 
 def TaskRelationSelectionTemplate_test_data(name="my_TaskRelationSelectionTemplate") -> dict: