From e0ac203cecf194d5ff9c8ab26ad923ab3a7e73a0 Mon Sep 17 00:00:00 2001 From: Max Goodman Date: Fri, 5 Jul 2013 15:24:26 -0700 Subject: [PATCH] Add VValidatedJSON validator for checking schema'd json data. --- r2/r2/controllers/multi.py | 67 +++++++++++++++-------------- r2/r2/lib/validator/validator.py | 40 +++++++++++++++++ r2/r2/public/static/css/reddit.less | 4 ++ r2/r2/public/static/js/multi.js | 4 ++ 4 files changed, 82 insertions(+), 33 deletions(-) diff --git a/r2/r2/controllers/multi.py b/r2/r2/controllers/multi.py index 816070bd1..cf6975e80 100644 --- a/r2/r2/controllers/multi.py +++ b/r2/r2/controllers/multi.py @@ -38,11 +38,13 @@ from r2.models.subreddit import ( from r2.lib.db import tdb_cassandra from r2.lib.wrapped import Wrapped from r2.lib.validator import ( + nop, validate, VUser, VModhash, + VOneOf, VSRByName, - VJSON, + VValidatedJSON, VMarkdown, VMultiPath, VMultiByPath, @@ -53,6 +55,12 @@ from r2.lib.errors import errors, reddit_http_error, RedditError from r2.lib.base import abort +multi_json_spec = { + 'visibility': VOneOf('visibility', ('private', 'public')), + 'subreddits': nop('subreddits', docs={'subreddits': 'subreddit data'}), +} + + class MultiApiController(RedditController, OAuth2ResourceController): def pre(self): set_extension(request.environ, "json") @@ -97,40 +105,33 @@ class MultiApiController(RedditController, OAuth2ResourceController): fields='multipath') def _write_multi_data(self, multi, data): - if 'visibility' in data: - if data['visibility'] not in ('private', 'public'): - raise RedditError('INVALID_OPTION', code=400, fields="data") - multi.visibility = data['visibility'] + multi.visibility = data['visibility'] - if 'subreddits' in data: - multi.clear_srs() - srs = Subreddit._by_name(sr['name'] for sr in data['subreddits']) + # multi subreddits + multi.clear_srs() + srs = Subreddit._by_name(sr['name'] for sr in data['subreddits']) - for sr in srs.itervalues(): - if isinstance(sr, FakeSubreddit): - multi._revert() - raise RedditError('MULTI_SPECIAL_SUBREDDIT', - msg_params={'path': sr.path}, - code=400) - - sr_props = {} - for sr_data in data['subreddits']: - try: - sr = srs[sr_data['name']] - except KeyError: - raise RedditError('SUBREDDIT_NOEXIST', code=400) - else: - sr_props[sr] = sr_data - - try: - multi.add_srs(sr_props) - except TooManySubredditsError as e: + for sr in srs.itervalues(): + if isinstance(sr, FakeSubreddit): multi._revert() - raise RedditError('MULTI_TOO_MANY_SUBREDDITS', code=409) + raise RedditError('MULTI_SPECIAL_SUBREDDIT', + msg_params={'path': sr.path}, + code=400) - if 'description_md' in data: - md = VMarkdown('description_md').run(data['description_md']) - multi.description_md = md + sr_props = {} + for sr_data in data['subreddits']: + try: + sr = srs[sr_data['name']] + except KeyError: + raise RedditError('SUBREDDIT_NOEXIST', code=400) + else: + sr_props[sr] = sr_data + + try: + multi.add_srs(sr_props) + except TooManySubredditsError as e: + multi._revert() + raise RedditError('MULTI_TOO_MANY_SUBREDDITS', code=409) multi._commit() return multi @@ -141,7 +142,7 @@ class MultiApiController(RedditController, OAuth2ResourceController): VUser(), VModhash(), path_info=VMultiPath("multipath"), - data=VJSON("model"), + data=VValidatedJSON("model", multi_json_spec), ) def POST_multi(self, path_info, data): """Create a multi. Responds with 409 Conflict if it already exists.""" @@ -165,7 +166,7 @@ class MultiApiController(RedditController, OAuth2ResourceController): VUser(), VModhash(), path_info=VMultiPath("multipath"), - data=VJSON("model"), + data=VValidatedJSON("model", multi_json_spec), ) def PUT_multi(self, path_info, data): """Create or update a multi.""" diff --git a/r2/r2/lib/validator/validator.py b/r2/r2/lib/validator/validator.py index d3cba457f..40b76d8b8 100644 --- a/r2/r2/lib/validator/validator.py +++ b/r2/r2/lib/validator/validator.py @@ -2185,6 +2185,46 @@ class VJSON(Validator): } +class VValidatedJSON(VJSON): + def __init__(self, param, spec, **kw): + VJSON.__init__(self, param, **kw) + self.spec = spec + + def run(self, json_str): + data = VJSON.run(self, json_str) + if not data: + return + + # Note: this relies on the fact that all validator errors are dumped + # into a global (c.errors) and then checked by @validate. + validated_data = {} + for key, validator in self.spec.iteritems(): + validated_data[key] = validator.run(data[key]) + + return validated_data + + def param_docs(self): + spec_docs = {} + for validator in self.spec.itervalues(): + spec_docs.update(validator.param_docs()) + if validator.docs: + spec_docs.update(validator.docs) + + # generate markdown json schema docs + spec_lines = [] + spec_lines.append('{') + for key in sorted(spec_docs.keys()): + spec_lines.append(' "%s": <%s>' % (key, spec_docs[key])) + spec_lines.append('}') + spec_md = "json data:\n\n" + "\n".join( + ' ' + line for line in spec_lines + ) + + return { + self.param: spec_md, + } + + multi_name_rx = re.compile(r"\A[A-Za-z0-9][A-Za-z0-9_]{1,20}\Z") multi_name_chars_rx = re.compile(r"[^A-Za-z0-9_]") diff --git a/r2/r2/public/static/css/reddit.less b/r2/r2/public/static/css/reddit.less index 33ee6e329..8e5e0cf02 100755 --- a/r2/r2/public/static/css/reddit.less +++ b/r2/r2/public/static/css/reddit.less @@ -6593,6 +6593,10 @@ body:not(.gold) .allminus-link { width: 50%; } +.api-help .parameters td pre { + margin: .5em 0; +} + #classy-error { text-align: center; } diff --git a/r2/r2/public/static/js/multi.js b/r2/r2/public/static/js/multi.js index 8a8c83c8a..048f852ac 100644 --- a/r2/r2/public/static/js/multi.js +++ b/r2/r2/public/static/js/multi.js @@ -57,6 +57,10 @@ r.multi.MultiReddit = Backbone.Model.extend({ return r.utils.joinURLs('/api/multi', this.id) }, + defaults: { + visibility: 'private' + }, + initialize: function(attributes, options) { this.uncreated = options && !!options.isNew this.subreddits = new r.multi.MultiRedditList(this.get('subreddits'), {parse: true})