Add config to rules

Author: ambrussimon

api/jobs/gears.py

@@ -5,6 +5,7 @@
 from __future__ import absolute_import
 
 import bson.objectid
+import copy
 import datetime
 from jsonschema import Draft4Validator, ValidationError
 import gears as gear_tools
@@ -96,13 +97,13 @@ def suggest_for_files(gear, files):
     return suggested_files
 
 def validate_gear_config(gear, config_):
-    if len(gear.get('manifest', {}).get('config', {})) > 0:
-        invocation = gear_tools.derive_invocation_schema(gear['manifest'])
+    if len(gear.get('gear', {}).get('config', {})) > 0:
+        invocation = gear_tools.derive_invocation_schema(gear['gear'])
         ci = gear_tools.isolate_config_invocation(invocation)
         validator = Draft4Validator(ci)
 
         try:
-            validator.validate(config_)
+            validator.validate(fill_gear_default_values(gear, config_))
         except ValidationError as err:
             key = None
             if len(err.relative_path) > 0:
@@ -120,8 +121,7 @@ def fill_gear_default_values(gear, config_):
     Given a gear and a config map, fill any missing keys using defaults from the gear's config
     """
 
-    if config_ is None:
-        config_ = {}
+    config_ = copy.deepcopy(config_) or {}
 
     for k,v in gear['gear'].get('config', {}).iteritems():
         if 'default' in v:

api/jobs/handlers.py

@@ -147,7 +147,7 @@ def post(self, cid):
 
         validate_data(payload, 'rule-new.json', 'input', 'POST', optional=True)
         validate_regexes(payload)
-        get_gear(payload['gear_id'])
+        validate_gear_config(get_gear(payload['gear_id']), payload.get('config'))
 
         payload['project_id'] = cid
 
@@ -197,8 +197,9 @@ def put(self, cid, rid):
         updates = self.request.json
         validate_data(updates, 'rule-update.json', 'input', 'POST', optional=True)
         validate_regexes(updates)
-        if updates.get('gear_id'):
-            get_gear(updates['gear_id'])
+        gear_id = updates.get('gear_id', doc['gear_id'])
+        config_ = updates.get('config', doc.get('config'))
+        validate_gear_config(get_gear(gear_id), config_)
 
         doc.update(updates)
         config.db.project_rules.replace_one({'_id': bson.ObjectId(rid)}, doc)

api/jobs/queue.py

@@ -157,7 +157,7 @@ def enqueue_job(job_map, origin, perm_check_uid=None):
         if gear.get('gear', {}).get('custom', {}).get('flywheel', {}).get('invalid', False):
             raise InputValidationException('Gear marked as invalid, will not run!')
 
-        config_ = fill_gear_default_values(gear, job_map.get('config', {}))
+        config_ = job_map.get('config', {})
         validate_gear_config(gear, config_)
 
         # Translate maps to FileReferences
@@ -194,7 +194,7 @@ def enqueue_job(job_map, origin, perm_check_uid=None):
 
         # Config options are stored on the job object under the "config" key
         config_ = {
-            'config': config_,
+            'config': fill_gear_default_values(gear, config_),
             'inputs': { },
             'destination': {
                 'type': destination.type,

api/jobs/rules.py

@@ -203,6 +203,9 @@ def create_potential_jobs(db, container, container_type, file_):
 
                 job = Job(str(gear['_id']), inputs, tags=['auto', gear_tag])
 
+            if 'config' in rule:
+                job.config = rule['config']
+
             potential_jobs.append({
                 'job': job,
                 'rule': rule

swagger/schemas/definitions/rule.json

@@ -31,6 +31,7 @@
                 "project_id": { "type": "string" },
                 "gear_id":    { "type": "string" },
                 "name":       { "type": "string" },
+                "config":     { "type": "object" },
                 "any":        { "$ref": "#/definitions/rule-items" },
                 "all":        { "$ref": "#/definitions/rule-items" },
                 "disabled":   { "type": "boolean" }
@@ -45,6 +46,7 @@
                 "_id":      { "type": "string" },
                 "gear_id":  { "type": "string" },
                 "name":     { "type": "string" },
+                "config":   { "type": "object" },
                 "any":      { "$ref": "#/definitions/rule-items" },
                 "all":      { "$ref": "#/definitions/rule-items" },
                 "disabled": { "type": "boolean" }

tests/integration_tests/python/test_rules.py

@@ -222,9 +222,10 @@ 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
-    gear = data_builder.create_gear(gear={'version': '0.0.1'})
+    gear_config = {'param': {'type': 'string', 'pattern': '^default|custom$', 'default': 'default'}}
+    gear = data_builder.create_gear(gear={'version': '0.0.1', 'config': gear_config})
     project = data_builder.create_project()
 
     bad_payload = {'test': 'rules'}
@@ -267,19 +268,42 @@ def test_rules(randstr, data_builder, file_form, as_root, as_admin, with_user, a
         'gear_id': '000000000000000000000000',
         '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/ non-existent gear
     # NOTE this is a legacy rule
     r = as_admin.post('/projects/' + project + '/rules', json=rule_json)
     assert r.status_code == 404
 
-    # add project rule w/ proper gear id
+    # try to add project rule w/ invalid config
     # NOTE this is a legacy rule
     rule_json['gear_id'] = gear
+    rule_json['config'] = {'param': 'invalid'}
+    r = as_admin.post('/projects/' + project + '/rules', json=rule_json)
+    assert r.status_code == 422
+    assert r.json()['reason'] == 'config did not match manifest'
+    del rule_json['config']
+
+    # add project rule w/ proper gear id
+    # NOTE this is a legacy rule
     r = as_admin.post('/projects/' + project + '/rules', json=rule_json)
     assert r.ok
     rule = r.json()['_id']
@@ -305,10 +329,15 @@ 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={'gear_id': gear})
     assert r.status_code == 403
 
-    # try to update rule to with invalid gear id
+    # try to update rule with invalid gear id
     r = as_admin.put('/projects/' + project + '/rules/' + rule, json={'gear_id': '000000000000000000000000'})
     assert r.status_code == 404
 
+    # try to update rule with invalid gear config
+    r = as_admin.put('/projects/' + project + '/rules/' + rule, json={'config': {'param': 'invalid'}})
+    assert r.status_code == 422
+    assert r.json()['reason'] == 'config did not match manifest'
+
     # update name of rule
     rule_name = 'improved-csv-trigger-rule'
     r = as_admin.put('/projects/' + project + '/rules/' + rule, json={'name': rule_name})
@@ -323,11 +352,25 @@ def test_rules(randstr, data_builder, file_form, as_root, as_admin, with_user, a
     r = as_admin.post('/projects/' + project + '/files', files=file_form('test2.csv'))
     assert r.ok
 
-    # test that job was created via rule
+    # test that job was created via rule and uses gear default config
     gear_jobs = [job for job in api_db.jobs.find({'gear_id': gear})]
     assert len(gear_jobs) == 1
     assert len(gear_jobs[0]['inputs']) == 1
     assert gear_jobs[0]['inputs'][0]['name'] == 'test2.csv'
+    assert gear_jobs[0]['config']['config'] == {'param': 'default'}
+
+    # update rule to have a custom config
+    r = as_admin.put('/projects/' + project + '/rules/' + rule, json={'config': {'param': 'custom'}})
+    assert r.ok
+
+    # upload another file that matches rule
+    r = as_admin.post('/projects/' + project + '/files', files=file_form('test3.csv'))
+    assert r.ok
+
+    # test that job was created via rule and custom config
+    gear_jobs = [job for job in api_db.jobs.find({'gear_id': gear})]
+    assert len(gear_jobs) == 2
+    assert gear_jobs[1]['config']['config'] == {'param': 'custom'}
 
     # try to delete rule of non-existent project
     r = as_admin.delete('/projects/000000000000000000000000/rules/000000000000000000000000')
@@ -368,7 +411,7 @@ def test_rules(randstr, data_builder, file_form, as_root, as_admin, with_user, a
 
     # test that job was not created via rule
     gear_jobs = [job for job in api_db.jobs.find({'gear_id': gear})]
-    assert len(gear_jobs) == 1 # still 1 from before
+    assert len(gear_jobs) == 2 # still 2 from before
 
     # update test2.csv's metadata to include a valid measurement to spawn job
     metadata = {
@@ -396,9 +439,9 @@ def test_rules(randstr, data_builder, file_form, as_root, as_admin, with_user, a
 
     # test that only one job was created via rule
     gear_jobs = [job for job in api_db.jobs.find({'gear_id': gear})]
-    assert len(gear_jobs) == 2
-    assert len(gear_jobs[1]['inputs']) == 1
-    assert gear_jobs[1]['inputs'][0]['name'] == 'test3.txt'
+    assert len(gear_jobs) == 3
+    assert len(gear_jobs[2]['inputs']) == 1
+    assert gear_jobs[2]['inputs'][0]['name'] == 'test3.txt'
 
     # delete rule
     r = as_admin.delete('/projects/' + project + '/rules/' + rule2)
@@ -423,7 +466,7 @@ def test_rules(randstr, data_builder, file_form, as_root, as_admin, with_user, a
 
     # test that job was created via regex rule
     gear_jobs = [job for job in api_db.jobs.find({'gear_id': gear})]
-    assert len(gear_jobs) == 3
+    assert len(gear_jobs) == 4
 
     # delete rule
     r = as_admin.delete('/projects/' + project + '/rules/' + rule3)