Add gear config to rules and improve rule input validation

scitran · Oct 20, 2017 · b6e2319 · b6e2319
1 parent 7e40036
commit b6e2319
Show file tree

Hide file tree

Showing 10 changed files with 117 additions and 155 deletions.
diff --git a/api/config.py b/api/config.py
@@ -170,7 +170,7 @@ def apply_env_variables(config):
     'project.json',
     'project-template.json',
     'project-update.json',
-    'rule-add.json',
+    'rule-new.json',
     'rule-update.json',
     'session.json',
     'session-update.json',

diff --git a/api/jobs/handlers.py b/api/jobs/handlers.py
@@ -175,13 +175,14 @@ def post(self, cid):
 
         doc = self.request.json
 
-        validate_data(doc, 'rule-add.json', 'input', 'POST', optional=True)
+        validate_data(doc, 'rule-new.json', 'input', 'POST', optional=True)
         try:
-            get_gear_by_name(doc['alg'])
+            gear = get_gear_by_name(doc['alg'])
         except APINotFoundException:
             self.abort(400, 'Cannot find gear for alg {}, alg not valid'.format(doc['alg']))
 
         doc['project_id'] = cid
+        doc.setdefault('config', gear['gear']['config'])
 
         result = config.db.project_rules.insert_one(doc)
         return { '_id': result.inserted_id }

diff --git a/raml/resources/projects.raml b/raml/resources/projects.raml
@@ -81,6 +81,9 @@ post:
     get:
       responses:
         200:
+          body:
+            application/json:
+              schema: !include ../schemas/output/rule-list.json
   /analyses:
     type: analyses-list
     /{AnalysisId}:

diff --git a/raml/schemas/definitions/rule.json b/raml/schemas/definitions/rule.json
@@ -1,22 +1,63 @@
 {
     "$schema": "http://json-schema.org/draft-04/schema#",
-    "type": "object",
-    "properties": {
-      "alg": {
-        "type": "string"
-      },
-      "all":{
-        "type":"array",
-        "items":{
-          "type":"array",
-          "items":{
-            "type":"string"
-          }
+    "definitions": {
+        "_id":         { "type": "string" },
+        "project_id":  { "type": "string" },
+        "alg":         { "type": "string" },
+        "name":        { "type": "string" },
+
+        "config": {
+            "type": "object",
+            "additionalProperties": { "$ref": "http://json-schema.org/draft-04/schema" }
+        },
+
+        "rule-items": {
+            "type": "array",
+            "items": {
+                "type": "object",
+                "properties": {
+                    "type": {
+                        "type": "string",
+                        "enum": [
+                            "file.type",
+                            "file.name",
+                            "file.measurements",
+                            "container.has-type",
+                            "container.has-measurement"
+                        ]
+                    },
+                    "value": { "type": "string" }
+                },
+                "required": [ "type", "value" ],
+                "additionalProperties": false
+            }
+        },
+
+
+        "rule-input": {
+            "type": "object",
+            "properties": {
+                "_id":        { "$ref": "#/definitions/_id" },
+                "project_id": { "$ref": "#/definitions/project_id" },
+                "alg":        { "$ref": "#/definitions/alg" },
+                "name":       { "$ref": "#/definitions/name" },
+                "config":     { "$ref": "#/definitions/config" },
+                "any":        { "$ref": "#/definitions/rule-items" },
+                "all":        { "$ref": "#/definitions/rule-items" }
+            },
+            "additionalProperties": false
+        },
+
+        "rule-output": {
+            "type": "object",
+            "properties": {
+                "_id":        { "$ref": "#/definitions/_id" },
+                "alg":        { "$ref": "#/definitions/alg" },
+                "name":       { "$ref": "#/definitions/name" },
+                "config":     { "$ref": "#/definitions/config" },
+                "any":        { "$ref": "#/definitions/rule-items" },
+                "all":        { "$ref": "#/definitions/rule-items" }
+            }
         }
-      }
-    },
-    "required": [
-      "alg",
-      "all"
-    ]
+    }
 }
diff --git a/raml/schemas/input/rule-add.json b/raml/schemas/input/rule-add.json
diff --git a/raml/schemas/input/rule-new.json b/raml/schemas/input/rule-new.json
@@ -0,0 +1,7 @@
+{
+    "$schema": "http://json-schema.org/draft-04/schema#",
+    "title": "Rule",
+    "type": "object",
+    "allOf": [{"$ref": "../definitions/rule.json#/definitions/rule-input"}],
+    "required": ["alg", "name", "any", "all"]
+}
diff --git a/raml/schemas/input/rule-update.json b/raml/schemas/input/rule-update.json
@@ -1,62 +1,6 @@
 {
     "$schema": "http://json-schema.org/draft-04/schema#",
-    "title": "rule",
+    "title": "Rule",
     "type": "object",
-    "properties": {
-        "_id": {
-            "type": "string"
-        },
-        "project_id": {
-            "type": "string"
-        },
-        "alg": {
-            "type": "string"
-        },
-        "name": {
-            "type": "string"
-        },
-        "any": {
-            "type": "array",
-            "items": {
-                "type": "object",
-                "properties": {
-                    "type": {
-                        "type": "string",
-                        "enum": [
-                            "file.type",
-                            "file.name",
-                            "file.measurements",
-                            "container.has-type",
-                            "container.has-measurement"
-                        ]
-                    },
-                    "value": {
-                        "type": "string"
-                    }
-                }
-            }
-        },
-        "all": {
-            "type": "array",
-            "items": {
-                "type": "object",
-                "properties": {
-                    "type": {
-                        "type": "string",
-                        "enum": [
-                            "file.type",
-                            "file.name",
-                            "file.measurements",
-                            "container.has-type",
-                            "container.has-measurement"
-                        ]
-                    },
-                    "value": {
-                        "type": "string"
-                    }
-                }
-            }
-        }
-    },
-    "additionalProperties": false
+    "allOf": [{"$ref": "../definitions/rule.json#/definitions/rule-input"}]
 }
diff --git a/raml/schemas/output/rule-list.json b/raml/schemas/output/rule-list.json
@@ -0,0 +1,5 @@
+{
+    "$schema": "http://json-schema.org/draft-04/schema#",
+    "type": "array",
+    "items": { "$ref":"../definitions/rule.json#/definitions/rule-output" }
+}
diff --git a/raml/schemas/output/rules-list.json b/raml/schemas/output/rules-list.json
diff --git a/test/integration_tests/python/test_rules.py b/test/integration_tests/python/test_rules.py
@@ -207,11 +207,12 @@ def test_site_rules_copied_to_new_projects(randstr, data_builder, file_form, as_
     data_builder.delete_group(group, recursive=True)
 
 
-def test_rules(randstr, data_builder, file_form, as_root, as_admin, with_user, api_db):
+def test_project_rules(randstr, data_builder, file_form, as_root, as_admin, with_user, api_db):
     # create versioned gear to cover code selecting latest gear
+    # add gear config to latest gear to check rules inheriting it
     gear_name = randstr()
     gear_1 = data_builder.create_gear(gear={'name': gear_name, 'version': '0.0.1'})
-    gear_2 = data_builder.create_gear(gear={'name': gear_name, 'version': '0.0.2'})
+    gear_2 = data_builder.create_gear(gear={'name': gear_name, 'version': '0.0.2', 'config': {'param': {'type': 'string'}}})
     project = data_builder.create_project()
 
     bad_payload = {'test': 'rules'}
@@ -254,15 +255,39 @@ def test_rules(randstr, data_builder, file_form, as_root, as_admin, with_user, a
         'alg': 'non-existent-gear-name',
         'name': 'csv-job-trigger-rule',
         'any': [],
-        'all': [
-            {'type': 'file.type', 'value': 'tabular data'},
-        ]
+        'all': [],
     }
 
+    # try to add project rule w/ invalid rule-item (invalid type)
+    # NOTE this is a legacy rule
+    rule_json['all'] = [{'type': 'invalid', 'value': 'test'}]
+    r = as_admin.post('/projects/' + project + '/rules', json=rule_json)
+    assert r.status_code == 400
+    assert "'invalid' is not one of" in r.json()['message']
+
+    # try to add project rule w/ invalid rule-item (missing value)
+    # NOTE this is a legacy rule
+    rule_json['all'] = [{'type': 'file.name'}]
+    r = as_admin.post('/projects/' + project + '/rules', json=rule_json)
+    assert r.status_code == 400
+    assert "'value' is a required property" in r.json()['message']
+
+    # set valid rule-item
+    rule_json['all'] = [{'type': 'file.type', 'value': 'tabular data'}]
+
+    # try to add project rule w/ invalid gear config
+    # NOTE this is a legacy rule
+    rule_json['config'] = {'foo': 'bar'}
+    r = as_admin.post('/projects/' + project + '/rules', json=rule_json)
+    assert r.status_code == 400
+    assert "'bar' is not of type u'object'" in r.json()['message']
+    del rule_json['config']
+
     # try to add project rule w/ non-existent gear
     # NOTE this is a legacy rule
     r = as_admin.post('/projects/' + project + '/rules', json=rule_json)
     assert r.status_code == 400
+    assert "Cannot find gear" in r.json()['message']
 
     # add project rule w/ proper gear alg
     # NOTE this is a legacy rule
@@ -271,10 +296,11 @@ def test_rules(randstr, data_builder, file_form, as_root, as_admin, with_user, a
     assert r.ok
     rule = r.json()['_id']
 
-    # get project rules (verify rule was added)
+    # get project rules (verify rule was added and uses default gear config)
     r = as_admin.get('/projects/' + project + '/rules')
     assert r.ok
     assert r.json()[0]['alg'] == gear_name
+    assert r.json()[0]['config'] == {'param': {'type': 'string'}}
 
     # try to get single project rule using non-existent rule id
     r = as_admin.get('/projects/' + project + '/rules/000000000000000000000000')
@@ -292,10 +318,14 @@ def test_rules(randstr, data_builder, file_form, as_root, as_admin, with_user, a
     r = with_user.session.put('/projects/' + project + '/rules/' + rule, json={'alg': gear_name})
     assert r.status_code == 403
 
-    # try to update rule to with invalid gear alg
+    # try to update rule with invalid gear alg
     r = as_admin.put('/projects/' + project + '/rules/' + rule, json={'alg': 'not-a-real-gear'})
     assert r.status_code == 400
 
+    # try to update rule with invalid gear config
+    r = as_admin.put('/projects/' + project + '/rules/' + rule, json={'config': {'foo': 'bar'}})
+    assert r.status_code == 400
+
     # update name of rule
     rule_name = 'improved-csv-trigger-rule'
     r = as_admin.put('/projects/' + project + '/rules/' + rule, json={'name': rule_name})