Skip to content

Commit

Permalink
Merge pull request #861 from mansimarkaur/details_to_404 (Fixes #818)
Browse files Browse the repository at this point in the history
Added details attribute to 404 errors.
  • Loading branch information
leplatrem committed Oct 20, 2016
2 parents a889f53 + a47bba5 commit f7cd8d0
Show file tree
Hide file tree
Showing 11 changed files with 52 additions and 11 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.rst
Expand Up @@ -8,6 +8,7 @@ This document describes changes between each past release.

**New features**

- Added details attribute to 404 errors. (#818)
- Added a new built-in plugin ``kinto.plugins.admin`` to serve the kinto admin.
- Added a new ``parse_resource`` utility to ``kinto.core.utils``

Expand Down
1 change: 1 addition & 0 deletions CONTRIBUTORS.rst
Expand Up @@ -32,6 +32,7 @@ Contributors
* Julien Bouquillon <contact@revolunet.com>
* Lavish Aggarwal <lucky.lavish@gmail.com>
* Maksym Shalenyi <supamaxy@gmail.com>
* Mansimar Kaur <mansimarkaur.mks@gmail.com>
* Masataka Takeuchi <masataka.takeuchi@l-is-b.com>
* Mathieu Agopian <mathieu@agopian.info>
* Mathieu Leplatre <mathieu@mozilla.com>
Expand Down
5 changes: 5 additions & 0 deletions docs/api/index.rst
Expand Up @@ -12,6 +12,11 @@ Changelog
---------


1.13 (2016-10-14)
'''''''''''''''''

- ``details`` attribute present in response of 404 errors.

1.12 (2016-10-11)
'''''''''''''''''

Expand Down
8 changes: 6 additions & 2 deletions kinto/core/resource/__init__.py
Expand Up @@ -680,8 +680,12 @@ def _get_record_or_404(self, record_id):
try:
return self.model.get_record(record_id)
except storage_exceptions.RecordNotFoundError:
response = http_error(HTTPNotFound(),
errno=ERRORS.INVALID_RESOURCE_ID)
details = {
"id": record_id,
"resource_name": self.request.current_resource_name
}
response = http_error(HTTPNotFound(), errno=ERRORS.INVALID_RESOURCE_ID,
details=details)
raise response

def _add_timestamp_header(self, response, timestamp=None):
Expand Down
1 change: 0 additions & 1 deletion kinto/core/testing.py
Expand Up @@ -79,7 +79,6 @@ def assertFormattedError(self, response, code, errno, error,
self.assertEqual(response.json['code'], code)
self.assertEqual(response.json['errno'], errno)
self.assertEqual(response.json['error'], error)

if message is not None:
self.assertIn(message, response.json['message'])
else: # pragma: no cover
Expand Down
10 changes: 8 additions & 2 deletions kinto/views/__init__.py
Expand Up @@ -2,7 +2,8 @@
import string

from kinto.core.storage import generators, exceptions
from pyramid import httpexceptions
from pyramid.httpexceptions import HTTPNotFound
from kinto.core.errors import http_error, ERRORS


class NameGenerator(generators.Generator):
Expand All @@ -29,4 +30,9 @@ def object_exists_or_404(request, collection_id, object_id, parent_id=''):
object_id=object_id)
except exceptions.RecordNotFoundError:
# XXX: We gave up putting details about parent id here (See #53).
raise httpexceptions.HTTPNotFound()
details = {
"id": object_id,
"resource_name": collection_id
}
response = http_error(HTTPNotFound(), errno=ERRORS.MISSING_RESOURCE, details=details)
raise response
6 changes: 3 additions & 3 deletions tests/core/test_views_errors.py
Expand Up @@ -44,15 +44,15 @@ def test_404_is_valid_formatted_error(self):
self.assertFormattedError(response, 404, ERRORS.MISSING_RESOURCE, "Not Found",
"The resource you are looking for could not be found.")

def test_404_can_be_overridded(self):
def test_404_can_be_overridden(self):
custom_404 = http_error(httpexceptions.HTTPNotFound(),
errno=ERRORS.MISSING_RESOURCE,
message="Customized.")
with mock.patch('tests.core.testapp.views.Mushroom._extract_filters',
side_effect=custom_404):
response = self.app.get(self.sample_url, headers=self.headers, status=404)
self.assertFormattedError(
response, 404, ERRORS.MISSING_RESOURCE, "Not Found", "Customized.")
self.assertFormattedError(response, 404, ERRORS.MISSING_RESOURCE,
"Not Found", "Customized.")

def test_401_is_valid_formatted_error(self):
response = self.app.get(self.sample_url, status=401)
Expand Down
6 changes: 6 additions & 0 deletions tests/test_views_buckets.py
Expand Up @@ -220,6 +220,11 @@ def test_buckets_can_be_deleted_in_bulk(self):
self.app.get('/buckets/2', headers=self.headers, status=404)
self.app.get('/buckets/3', headers=self.headers, status=404)

def test_unknown_bucket_raises_404(self):
resp = self.app.get('/buckets/2', headers=self.headers, status=404)
self.assertEqual(resp.json['details']['id'], '2')
self.assertEqual(resp.json['details']['resource_name'], 'bucket')

def test_buckets_can_be_deleted(self):
self.app.get(self.bucket_url, headers=self.headers,
status=404)
Expand All @@ -228,6 +233,7 @@ def test_every_collections_are_deleted_too(self):
self.app.put_json(self.bucket_url, MINIMALIST_BUCKET,
headers=self.headers)
self.app.get(self.collection_url, headers=self.headers, status=404)

# Verify tombstones
resp = self.app.get('%s/collections?_since=0' % self.bucket_url,
headers=self.headers)
Expand Down
9 changes: 7 additions & 2 deletions tests/test_views_collections.py
Expand Up @@ -47,6 +47,12 @@ def test_unknown_bucket_raises_403(self):
other_bucket = self.collections_url.replace('beers', 'sodas')
self.app.get(other_bucket, headers=self.headers, status=403)

def test_unknown_collection_raises_404(self):
other_collection = self.collection_url.replace('barley', 'pills')
resp = self.app.get(other_collection, headers=self.headers, status=404)
self.assertEqual(resp.json['details']['id'], 'pills')
self.assertEqual(resp.json['details']['resource_name'], 'collection')

def test_collections_are_isolated_by_bucket(self):
other_bucket = self.collection_url.replace('beers', 'sodas')
self.app.put_json('/buckets/sodas',
Expand Down Expand Up @@ -117,8 +123,7 @@ def setUp(self):
self.app.delete(self.collection_url, headers=self.headers)

def test_collections_can_be_deleted(self):
self.app.get(self.collection_url, headers=self.headers,
status=404)
self.app.get(self.collection_url, headers=self.headers, status=404)

def test_collections_can_be_deleted_in_bulk(self):
alice_headers = get_user_headers('alice')
Expand Down
6 changes: 6 additions & 0 deletions tests/test_views_groups.py
Expand Up @@ -145,6 +145,12 @@ def test_groups_can_be_deleted(self):
self.app.get(self.group_url, headers=self.headers,
status=404)

def test_unknown_group_raises_404(self):
other_group = self.group_url.replace('moderators', 'blah')
resp = self.app.get(other_group, headers=self.headers, status=404)
self.assertEqual(resp.json['details']['id'], 'blah')
self.assertEqual(resp.json['details']['resource_name'], 'group')

def test_group_is_removed_from_users_principals_on_group_deletion(self):
self.app.put_json(self.group_url, MINIMALIST_GROUP,
headers=self.headers, status=201)
Expand Down
10 changes: 9 additions & 1 deletion tests/test_views_records.py
Expand Up @@ -38,7 +38,15 @@ def test_unknown_bucket_raises_403(self):

def test_unknown_collection_raises_404(self):
other_collection = self.collection_url.replace('barley', 'pills')
self.app.get(other_collection, headers=self.headers, status=404)
resp = self.app.get(other_collection, headers=self.headers, status=404)
self.assertEqual(resp.json['details']['id'], 'pills')
self.assertEqual(resp.json['details']['resource_name'], 'collection')

def test_unknown_record_raises_404(self):
other_record = self.record_url.replace(self.record['id'], self.record['id']+'blah')
response = self.app.get(other_record, headers=self.headers, status=404)
self.assertEqual(response.json['details']['id'], self.record['id']+'blah')
self.assertEqual(response.json['details']['resource_name'], 'record')

def test_unknown_collection_does_not_query_timestamp(self):
other_collection = self.collection_url.replace('barley', 'pills')
Expand Down

0 comments on commit f7cd8d0

Please sign in to comment.