Differential D75

T138 - /testcases/{testcase_name}/results and /jobs/{job_id}/results should return 404
ClosedPublic
Actions

Authored by jskladan on Apr 29 2014, 9:25 AM.

Tags

None

Subscribers

Details

Reviewers

tflink
mkrizek

Maniphest Tasks

T138: Should /api/v1.0/testcases/{testcase_name}/results and /api/v1.0/jobs/{job_id}/results return 404?

Commits

rRSDB65cfdf9eb471: T138 - /testcases/{testcase_name}/results and /jobs/{job_id}/results should…

Summary

The API now checks whether the testcase/job exists, and throws 404 if it does not, instead of silently returning empty results set.

Test Plan

NaN

Diff Detail

Lint

Lint Skipped

Unit

Unit Tests Skipped

Looks good to me.

That code still seems a bit odd to me, but this patch would fix the 404 issue. Please merge and push to origin

Closed by commit rRSDB65cfdf9eb471 (authored by @jskladan).

Revision Contents

			Path
M			resultsdb/controllers/api_v1.py (22 lines)

Diff 224

resultsdb/controllers/api_v1.py

Show First 20 Lines • Show All 408 Lines • ▼ Show 20 Line(s)		375	def __get_results_parse_args():
409		409
410	if extra_data != {}:	410		if extra_data != {}:
411	retval['result_data'] = extra_data	411		retval['result_data'] = extra_data
412		412
413	return retval	413		return retval
414		414
415		415
416	@api.route('/v1.0/results', methods = ['GET'])	416		@api.route('/v1.0/results', methods = ['GET'])
417	@api.route('/v1.0/jobs/<job_id>/results', methods = ['GET'])
418	@api.route('/v1.0/testcases/<testcase_name>/results', methods = ['GET'])
419	def get_results(job_id = None, testcase_name = None):	417		def get_results(job_id = None, testcase_name = None):
420		418
421
422
423	p = __get_results_parse_args()	419		p = __get_results_parse_args()
424	if p['error'] is not None:	420		if p['error'] is not None:
425	return p['error']	421		return p['error']
426		422
427	args = p['args']	423		args = p['args']
428		424
429	j_id = job_id if job_id is not None else args['job_id']	425		j_id = job_id if job_id is not None else args['job_id']
430	t_nm = testcase_name if testcase_name is not None else args['testcase_name']	426		t_nm = testcase_name if testcase_name is not None else args['testcase_name']
Show All 9 Lines		429	q = select_results(
440	testcase_name = t_nm,	436		testcase_name = t_nm,
441	)	437		)
442	q = pagination(q, args['page'], args['limit'])	438		q = pagination(q, args['page'], args['limit'])
443		439
444	prev, next = prev_next_urls()	440		prev, next = prev_next_urls()
445		441
446	return jsonify(dict(href = request.url, prev = prev, next = next, data = [SERIALIZE(o) for o in q.all()]))	442		return jsonify(dict(href = request.url, prev = prev, next = next, data = [SERIALIZE(o) for o in q.all()]))
447		443
		444		@api.route('/v1.0/jobs/<job_id>/results', methods = ['GET'])
		445		@api.route('/v1.0/testcases/<testcase_name>/results', methods = ['GET'])
		446		def get_results_by_job_testcase(job_id = None, testcase_name = None):
		447		# check whether the job/testcase exists. If not, throw 404
		448		if job_id is not None:
		449		try:
		450		job = Job.query.filter_by(id = job_id).one()
		451		except orm_exc.NoResultFound:
		452		return jsonify({'message': "Job not found" }), 404
		453
		454		if testcase_name is not None:
		455		try:
		456		testcase = Testcase.query.filter_by(name = testcase_name).one()
		457		except orm_exc.NoResultFound:
		458		return jsonify({'message': "Testcase not found" }), 404
		459
		460		return get_results(job_id, testcase_name)
		461
448		462
449	@api.route('/v1.0/results/<result_id>', methods = ['GET'])	463		@api.route('/v1.0/results/<result_id>', methods = ['GET'])
450	def get_result(result_id):	464		def get_result(result_id):
451	try:	465		try:
452	result = Result.query.filter_by(id = result_id).one()	466		result = Result.query.filter_by(id = result_id).one()
453	except orm_exc.NoResultFound:	467		except orm_exc.NoResultFound:
454	return jsonify({'message': "Result not found" }), 404	468		return jsonify({'message': "Result not found" }), 404
455		469
▲ Show 20 Lines • Show All 158 Lines • Show Last 20 Lines