summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorTomas Klohna <tklohna@redhat.com>2019-01-10 13:47:04 +0100
committerTomas Klohna <tklohna@redhat.com>2019-01-14 08:49:11 +0000
commitb2f7c5be893de2bf8a20ca32e69f70243240c266 (patch)
tree2859119b8ab53c0bcf50618233f01c08a115863b
parent4e5968b0bf17236c30740ea76c9afb2acce595eb (diff)
Fix redirecting when given non-existent task instead of giving 404
-rw-r--r--IntegrationTests/src/bkr/inttest/server/selenium/test_task_by_name.py23
-rw-r--r--Server/bkr/server/tasks.py14
2 files changed, 34 insertions, 3 deletions
diff --git a/IntegrationTests/src/bkr/inttest/server/selenium/test_task_by_name.py b/IntegrationTests/src/bkr/inttest/server/selenium/test_task_by_name.py
index 6b7906a..5cc6e7c 100644
--- a/IntegrationTests/src/bkr/inttest/server/selenium/test_task_by_name.py
+++ b/IntegrationTests/src/bkr/inttest/server/selenium/test_task_by_name.py
@@ -9,6 +9,7 @@ from bkr.inttest import data_setup, get_server_base
from bkr.inttest.server.webdriver_utils import is_text_present
import unittest
from turbogears.database import session
+import requests
class TaskByName(WebDriverTestCase):
@@ -27,3 +28,25 @@ class TaskByName(WebDriverTestCase):
b.get(get_server_base() + 'tasks/%s' % task_id)
self.assert_('Task %s' % task_name in b.find_element_by_xpath('//body').text)
+
+ # https://bugzilla.redhat.com/show_bug.cgi?id=1614171
+ def test_task_404_by_name(self):
+ task_name_without_slash = 'no/such/name/exists/in/our/database'
+ task_name_with_slash = '/%s' % task_name_without_slash
+
+ # testing name 404 without slash
+ response = requests.get(get_server_base() + 'tasks/%s/' % task_name_without_slash)
+ self.assertEqual(response.status_code, 404)
+
+ # testing name 404 with starting slash
+ response = requests.get(get_server_base() + 'tasks/%s/' % task_name_with_slash)
+ self.assertEqual(response.status_code, 404)
+
+ # https://bugzilla.redhat.com/show_bug.cgi?id=1614171
+ # Searching for ID returns human friendly error as well as 404
+ def test_task_404_by_id(self):
+ task_id = 123456789099999
+
+ response = requests.get(get_server_base() + 'tasks/%s' % task_id)
+ self.assertEqual(response.status_code, 404)
+ self.assertEqual(response.text, 'No such task with ID: %s' % task_id)
diff --git a/Server/bkr/server/tasks.py b/Server/bkr/server/tasks.py
index 9bb84c1..400d50e 100644
--- a/Server/bkr/server/tasks.py
+++ b/Server/bkr/server/tasks.py
@@ -364,8 +364,7 @@ class Tasks(RPCRoot):
#This is the simplest way of dealing with it
redirect("/tasks/%s" % task.id)
except DatabaseLookupError as e:
- flash(unicode(e))
- redirect("/tasks")
+ raise cherrypy.HTTPError(status=404, message='%s' % e)
attributes = task.to_dict()
attributes['can_disable'] = bool(
@@ -448,11 +447,20 @@ def get_tasks():
})
return result
+# This route is used whenever user enters any integer past /tasks/.
+# Paths that don't match this template are rerouted by either default function or their specific function
+# eq. path .../tasks//custom/name/of/task - will NOT be picked up (processed by default function)
+# .../tasks/custom_name - will NOT be picked up
+# .../tasks/name/with/slash - will NOT be picked up
+# .../tasks/123456 - will be picked up
@app.route('/tasks/<int:task_id>', methods=['GET'])
def get_task(task_id):
# Dummy handler to fall back to CherryPy
# so that other methods such as PATCH/DELETE work.
- raise NotFound404('Fall back to CherryPy')
+ # ---
+ # Because Flask defined 404 has priority before CherryPy's 404,
+ # message defined in here will be presented to user when CherryPy's 404 is raised.
+ raise NotFound404('No such task with ID: %s' % task_id)
@app.route('/tasks/<int:task_id>', methods=['PATCH'])
@admin_auth_required