Skip to content

Commit c159f0a

Browse files
ambrussimonambrussimon
committed
Add gear config to rules and improve rule input validation
1 parent c4bba02 commit c159f0a

File tree

10 files changed

+117
-155
lines changed

10 files changed

+117
-155
lines changed

‎api/config.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ def apply_env_variables(config):
171171
'project.json',
172172
'project-template.json',
173173
'project-update.json',
174-
'rule-add.json',
174+
'rule-new.json',
175175
'rule-update.json',
176176
'session.json',
177177
'session-update.json',

‎api/jobs/handlers.py‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -175,13 +175,14 @@ def post(self, cid):
175175

176176
doc = self.request.json
177177

178-
validate_data(doc, 'rule-add.json', 'input', 'POST', optional=True)
178+
validate_data(doc, 'rule-new.json', 'input', 'POST', optional=True)
179179
try:
180-
get_gear_by_name(doc['alg'])
180+
gear = get_gear_by_name(doc['alg'])
181181
except APINotFoundException:
182182
self.abort(400, 'Cannot find gear for alg {}, alg not valid'.format(doc['alg']))
183183

184184
doc['project_id'] = cid
185+
doc.setdefault('config', gear['gear']['config'])
185186

186187
result = config.db.project_rules.insert_one(doc)
187188
return { '_id': result.inserted_id }

‎raml/resources/projects.raml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,9 @@ post:
8181
get:
8282
responses:
8383
200:
84+
body:
85+
application/json:
86+
schema: !include ../schemas/output/rule-list.json
8487
/analyses:
8588
type: analyses-list
8689
/{AnalysisId}:

‎raml/schemas/definitions/rule.json‎

Lines changed: 59 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,63 @@
11
{
22
"$schema": "http://json-schema.org/draft-04/schema#",
3-
"type": "object",
4-
"properties": {
5-
"alg": {
6-
"type": "string"
7-
},
8-
"all":{
9-
"type":"array",
10-
"items":{
11-
"type":"array",
12-
"items":{
13-
"type":"string"
14-
}
3+
"definitions": {
4+
"_id": { "type": "string" },
5+
"project_id": { "type": "string" },
6+
"alg": { "type": "string" },
7+
"name": { "type": "string" },
8+
9+
"config": {
10+
"type": "object",
11+
"additionalProperties": { "$ref": "http://json-schema.org/draft-04/schema" }
12+
},
13+
14+
"rule-items": {
15+
"type": "array",
16+
"items": {
17+
"type": "object",
18+
"properties": {
19+
"type": {
20+
"type": "string",
21+
"enum": [
22+
"file.type",
23+
"file.name",
24+
"file.measurements",
25+
"container.has-type",
26+
"container.has-measurement"
27+
]
28+
},
29+
"value": { "type": "string" }
30+
},
31+
"required": [ "type", "value" ],
32+
"additionalProperties": false
33+
}
34+
},
35+
36+
37+
"rule-input": {
38+
"type": "object",
39+
"properties": {
40+
"_id": { "$ref": "#/definitions/_id" },
41+
"project_id": { "$ref": "#/definitions/project_id" },
42+
"alg": { "$ref": "#/definitions/alg" },
43+
"name": { "$ref": "#/definitions/name" },
44+
"config": { "$ref": "#/definitions/config" },
45+
"any": { "$ref": "#/definitions/rule-items" },
46+
"all": { "$ref": "#/definitions/rule-items" }
47+
},
48+
"additionalProperties": false
49+
},
50+
51+
"rule-output": {
52+
"type": "object",
53+
"properties": {
54+
"_id": { "$ref": "#/definitions/_id" },
55+
"alg": { "$ref": "#/definitions/alg" },
56+
"name": { "$ref": "#/definitions/name" },
57+
"config": { "$ref": "#/definitions/config" },
58+
"any": { "$ref": "#/definitions/rule-items" },
59+
"all": { "$ref": "#/definitions/rule-items" }
60+
}
1561
}
16-
}
17-
},
18-
"required": [
19-
"alg",
20-
"all"
21-
]
62+
}
2263
}

‎raml/schemas/input/rule-add.json‎

Lines changed: 0 additions & 62 deletions
This file was deleted.

‎raml/schemas/input/rule-new.json‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"$schema": "http://json-schema.org/draft-04/schema#",
3+
"title": "Rule",
4+
"type": "object",
5+
"allOf": [{"$ref": "../definitions/rule.json#/definitions/rule-input"}],
6+
"required": ["alg", "name", "any", "all"]
7+
}

‎raml/schemas/input/rule-update.json‎

Lines changed: 2 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -1,62 +1,6 @@
11
{
22
"$schema": "http://json-schema.org/draft-04/schema#",
3-
"title": "rule",
3+
"title": "Rule",
44
"type": "object",
5-
"properties": {
6-
"_id": {
7-
"type": "string"
8-
},
9-
"project_id": {
10-
"type": "string"
11-
},
12-
"alg": {
13-
"type": "string"
14-
},
15-
"name": {
16-
"type": "string"
17-
},
18-
"any": {
19-
"type": "array",
20-
"items": {
21-
"type": "object",
22-
"properties": {
23-
"type": {
24-
"type": "string",
25-
"enum": [
26-
"file.type",
27-
"file.name",
28-
"file.measurements",
29-
"container.has-type",
30-
"container.has-measurement"
31-
]
32-
},
33-
"value": {
34-
"type": "string"
35-
}
36-
}
37-
}
38-
},
39-
"all": {
40-
"type": "array",
41-
"items": {
42-
"type": "object",
43-
"properties": {
44-
"type": {
45-
"type": "string",
46-
"enum": [
47-
"file.type",
48-
"file.name",
49-
"file.measurements",
50-
"container.has-type",
51-
"container.has-measurement"
52-
]
53-
},
54-
"value": {
55-
"type": "string"
56-
}
57-
}
58-
}
59-
}
60-
},
61-
"additionalProperties": false
5+
"allOf": [{"$ref": "../definitions/rule.json#/definitions/rule-input"}]
626
}

‎raml/schemas/output/rule-list.json‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"$schema": "http://json-schema.org/draft-04/schema#",
3+
"type": "array",
4+
"items": { "$ref":"../definitions/rule.json#/definitions/rule-output" }
5+
}

‎raml/schemas/output/rules-list.json‎

Lines changed: 0 additions & 7 deletions
This file was deleted.

‎test/integration_tests/python/test_rules.py‎

Lines changed: 37 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -207,11 +207,12 @@ def test_site_rules_copied_to_new_projects(randstr, data_builder, file_form, as_
207207
data_builder.delete_group(group, recursive=True)
208208

209209

210-
def test_rules(randstr, data_builder, file_form, as_root, as_admin, with_user, api_db):
210+
def test_project_rules(randstr, data_builder, file_form, as_root, as_admin, with_user, api_db):
211211
# create versioned gear to cover code selecting latest gear
212+
# add gear config to latest gear to check rules inheriting it
212213
gear_name = randstr()
213214
gear_1 = data_builder.create_gear(gear={'name': gear_name, 'version': '0.0.1'})
214-
gear_2 = data_builder.create_gear(gear={'name': gear_name, 'version': '0.0.2'})
215+
gear_2 = data_builder.create_gear(gear={'name': gear_name, 'version': '0.0.2', 'config': {'param': {'type': 'string'}}})
215216
project = data_builder.create_project()
216217

217218
bad_payload = {'test': 'rules'}
@@ -254,15 +255,39 @@ def test_rules(randstr, data_builder, file_form, as_root, as_admin, with_user, a
254255
'alg': 'non-existent-gear-name',
255256
'name': 'csv-job-trigger-rule',
256257
'any': [],
257-
'all': [
258-
{'type': 'file.type', 'value': 'tabular data'},
259-
]
258+
'all': [],
260259
}
261260

261+
# try to add project rule w/ invalid rule-item (invalid type)
262+
# NOTE this is a legacy rule
263+
rule_json['all'] = [{'type': 'invalid', 'value': 'test'}]
264+
r = as_admin.post('/projects/' + project + '/rules', json=rule_json)
265+
assert r.status_code == 400
266+
assert "'invalid' is not one of" in r.json()['message']
267+
268+
# try to add project rule w/ invalid rule-item (missing value)
269+
# NOTE this is a legacy rule
270+
rule_json['all'] = [{'type': 'file.name'}]
271+
r = as_admin.post('/projects/' + project + '/rules', json=rule_json)
272+
assert r.status_code == 400
273+
assert "'value' is a required property" in r.json()['message']
274+
275+
# set valid rule-item
276+
rule_json['all'] = [{'type': 'file.type', 'value': 'tabular data'}]
277+
278+
# try to add project rule w/ invalid gear config
279+
# NOTE this is a legacy rule
280+
rule_json['config'] = {'foo': 'bar'}
281+
r = as_admin.post('/projects/' + project + '/rules', json=rule_json)
282+
assert r.status_code == 400
283+
assert "'bar' is not of type u'object'" in r.json()['message']
284+
del rule_json['config']
285+
262286
# try to add project rule w/ non-existent gear
263287
# NOTE this is a legacy rule
264288
r = as_admin.post('/projects/' + project + '/rules', json=rule_json)
265289
assert r.status_code == 400
290+
assert "Cannot find gear" in r.json()['message']
266291

267292
# add project rule w/ proper gear alg
268293
# 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
271296
assert r.ok
272297
rule = r.json()['_id']
273298

274-
# get project rules (verify rule was added)
299+
# get project rules (verify rule was added and uses default gear config)
275300
r = as_admin.get('/projects/' + project + '/rules')
276301
assert r.ok
277302
assert r.json()[0]['alg'] == gear_name
303+
assert r.json()[0]['config'] == {'param': {'type': 'string'}}
278304

279305
# try to get single project rule using non-existent rule id
280306
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
292318
r = with_user.session.put('/projects/' + project + '/rules/' + rule, json={'alg': gear_name})
293319
assert r.status_code == 403
294320

295-
# try to update rule to with invalid gear alg
321+
# try to update rule with invalid gear alg
296322
r = as_admin.put('/projects/' + project + '/rules/' + rule, json={'alg': 'not-a-real-gear'})
297323
assert r.status_code == 400
298324

325+
# try to update rule with invalid gear config
326+
r = as_admin.put('/projects/' + project + '/rules/' + rule, json={'config': {'foo': 'bar'}})
327+
assert r.status_code == 400
328+
299329
# update name of rule
300330
rule_name = 'improved-csv-trigger-rule'
301331
r = as_admin.put('/projects/' + project + '/rules/' + rule, json={'name': rule_name})

0 commit comments

Comments
 (0)