autoland: refactor request validation (
bug 1368516) r?smacleod
Remove repeated error handling code by refactoring validation out into a
function.
MozReview-Commit-ID: B717LhJ5snK
--- a/autoland/autoland/autoland_rest.py
+++ b/autoland/autoland/autoland_rest.py
@@ -67,16 +67,34 @@ def check_pingback_url(pingback_url):
# Allow pingbacks to whitelisted hosts from config.json
for allowed_host in config.get('pingback_allow', []):
if url.hostname == allowed_host:
return True
return False
+def validate_request(request):
+ if request.json is None:
+ raise ValueError('Bad request: missing json')
+
+ for field in ['ldap_username', 'tree', 'rev', 'destination',
+ 'pingback_url']:
+ if field not in request.json:
+ raise ValueError('missing json field: %s' % field)
+
+ if not check_pingback_url(request.json['pingback_url']):
+ raise ValueError('bad pingback_url')
+
+ if not (request.json.get('trysyntax') or
+ request.json.get('commit_descriptions')):
+ raise ValueError('one of trysyntax or commit_descriptions must be '
+ 'specified.')
+
+
@app.route('/autoland', methods=['POST'])
def autoland():
"""
Autoland a patch from one tree to another.
Example request json:
{
@@ -102,54 +120,35 @@ def autoland():
auth_response = {'WWW-Authenticate': 'Basic realm="Login Required"'}
if not auth:
return Response('Login required', 401, auth_response)
if not check_auth(auth.username, auth.password):
logging.warn('Failed authentication for "%s" from %s' % (
auth.username, request.remote_addr))
return Response('Login required', 401, auth_response)
- if request.json is None:
- error = 'Bad request: missing json'
- return make_response(jsonify({'error': error}), 400)
-
- for field in ['ldap_username', 'tree', 'rev', 'destination',
- 'pingback_url']:
- if field not in request.json:
- error = 'Bad request: missing json field: %s' % field
- return make_response(jsonify({'error': error}), 400)
-
- if not check_pingback_url(request.json['pingback_url']):
- error = 'Bad request: bad pingback_url'
- return make_response(jsonify({'error': error}), 400)
-
- if not request.json.get('ldap_username'):
- error = 'Bad request: ldap_username must be specified'
- return make_response(jsonify({'error': error}), 400)
-
- if not (request.json.get('trysyntax') or
- request.json.get('commit_descriptions')):
- error = ('Bad request: one of trysyntax or commit_descriptions must '
- 'be specified.')
- return make_response(jsonify({'error': error}), 400)
+ try:
+ validate_request(request)
+ except ValueError as e:
+ return make_response(jsonify({'error': 'Bad request: %s' % e}), 400)
dbconn = get_dbconn()
cursor = dbconn.cursor()
query = """
SELECT created, request->>'ldap_username'
FROM Transplant
WHERE landed IS NULL
AND request->>'rev' = %s
AND request->>'destination' = %s
"""
cursor.execute(query, (request.json['rev'], request.json['destination']))
in_flight = cursor.fetchone()
if in_flight:
- error = ('Bad request: a request to land revision %s to %s is already '
+ error = ('Bad Request: a request to land revision %s to %s is already '
'in progress'
% (request.json['rev'], request.json['destination']))
app.logger.warn(
'%s from %s at %s' % (error, in_flight[0], in_flight[1]))
return make_response(jsonify({'error': error}), 400)
app.logger.info('received transplant request: %s' %
json.dumps(request.json))
--- a/autoland/tests/test-post-autoland-job.t
+++ b/autoland/tests/test-post-autoland-job.t
@@ -205,12 +205,12 @@ Post the same job twice. Start with sto
guarentee the first request is still in the queue when the second is submitted.
$ PID=`mozreview exec autoland ps x | grep autoland.py | grep -v grep | awk '{ print $1 }'`
$ mozreview exec autoland kill $PID
$ REV=`hg log -r . --template "{node|short}"`
$ ottoland post-autoland-job $AUTOLAND_URL test-repo $REV try http://localhost:9898 --trysyntax "stuff"
(200, u'{\n "request_id": 12\n}')
$ ottoland post-autoland-job $AUTOLAND_URL test-repo $REV try http://localhost:9898 --trysyntax "stuff"
- (400, u'{\n "error": "landing revision e7f4a0f07be3 to try is already in progress"\n}')
+ (400, u'{\n "error": "Bad Request: a request to land revision e7f4a0f07be3 to try is already in progress"\n}')
$ mozreview stop
stopped 9 containers