From f3e5d63bbc25a4a479563a03cf427883bff877d1 Mon Sep 17 00:00:00 2001 From: MJedr Date: Wed, 1 Mar 2023 10:22:36 +0100 Subject: [PATCH] headers: dont add link header to search & record response ref: cern-sis/issues-inspire#221 --- .github/workflows/build_and_test.yml | 4 +- invenio_records_rest/serializers/response.py | 6 -- setup.py | 4 ++ tests/test_permissions.py | 65 +++++++++++++++----- tests/test_serializer_response.py | 11 +++- tests/test_views_search.py | 32 +++------- 6 files changed, 74 insertions(+), 48 deletions(-) diff --git a/.github/workflows/build_and_test.yml b/.github/workflows/build_and_test.yml index 2fdb55c4..d27bb33f 100644 --- a/.github/workflows/build_and_test.yml +++ b/.github/workflows/build_and_test.yml @@ -14,10 +14,10 @@ env: jobs: Test: - runs-on: ubuntu-latest + runs-on: ubuntu-20.04 strategy: matrix: - python-version: [ 3.6, 3.7, 3.8, 3.9 ] + python-version: [ 3.6, 3.8 ] requirements-level: [min, pypi] services: postgres: diff --git a/invenio_records_rest/serializers/response.py b/invenio_records_rest/serializers/response.py index b7806971..9690acd1 100644 --- a/invenio_records_rest/serializers/response.py +++ b/invenio_records_rest/serializers/response.py @@ -37,9 +37,6 @@ def view(pid, record, code=200, headers=None, links_factory=None): if headers is not None: response.headers.extend(headers) - if links_factory is not None: - add_link_header(response, links_factory(pid)) - return response return view @@ -63,9 +60,6 @@ def view(pid_fetcher, search_result, code=200, headers=None, links=None, if headers is not None: response.headers.extend(headers) - if links is not None: - add_link_header(response, links) - return response return view diff --git a/setup.py b/setup.py index e53dee02..bd51948a 100644 --- a/setup.py +++ b/setup.py @@ -22,6 +22,9 @@ 'invenio-indexer>=1.0.0', 'invenio-config>=1.0.2', 'pytest-invenio>=1.2.1', + 'pydocstyle<=6.1.1', + 'jsonref==0.2', + 'jsonresolver==0.2.1' ] invenio_search_version = '1.2.3,<1.3.0' @@ -48,6 +51,7 @@ ], 'docs': [ 'Sphinx>=3', + 'MarkupSafe==2.0.1' ], 'dublincore': [ 'dcxml>=0.1.0', diff --git a/tests/test_permissions.py b/tests/test_permissions.py index cee89865..81344486 100644 --- a/tests/test_permissions.py +++ b/tests/test_permissions.py @@ -16,21 +16,26 @@ from helpers import record_url -def test_default_permissions(app, default_permissions, test_data, search_url, - test_records, indexed_records): +def test_default_permissions( + app, + default_permissions, + test_data, + search_url, + test_records, + indexed_records +): """Test default create permissions.""" - pid, record = test_records[0] + pid, _ = test_records[0] rec_url = record_url(pid) data = json.dumps(test_data[0]) - h = {'Content-Type': 'application/json'} - hp = {'Content-Type': 'application/json-patch+json'} + headers = {"Content-Type": "application/json"} + headers_with_json_patch = {"Content-Type": "application/json-patch+json"} with app.test_client() as client: - args = dict(data=data, headers=h) - pargs = dict(data=data, headers=hp) - qs = {'user': '1'} - uargs = dict(data=data, headers=h, query_string=qs) - upargs = dict(data=data, headers=hp, query_string=qs) + args = dict(data=data, headers=headers) + args_with_json_patch_headers = dict( + data=data, headers=headers_with_json_patch + ) assert client.get(search_url).status_code == 200 assert client.get(rec_url).status_code == 200 @@ -42,10 +47,40 @@ def test_default_permissions(app, default_permissions, test_data, search_url, assert 405 == client.post(rec_url, **args).status_code assert 401 == client.put(rec_url, **args).status_code - assert 401 == client.patch(rec_url, **pargs).status_code + assert 401 == client.patch( + rec_url, **args_with_json_patch_headers + ).status_code assert 401 == client.delete(rec_url).status_code - assert 403 == client.post(search_url, **uargs).status_code - assert 403 == client.put(rec_url, **uargs).status_code - assert 403 == client.patch(rec_url, **upargs).status_code - assert 403 == client.delete(rec_url, query_string=qs).status_code + +def test_default_permissions_with_logged_inuser( + app, + default_permissions, + test_data, + search_url, + test_records, + indexed_records +): + """Test default create permissions.""" + pid, _ = test_records[0] + rec_url = record_url(pid) + data = json.dumps(test_data[0]) + headers = {"Content-Type": "application/json"} + headers_with_json_patch = {"Content-Type": "application/json-patch+json"} + + with app.test_client() as client: + query_str = {"user": "1"} + args = dict(data=data, headers=headers, query_string=query_str) + args_with_json_patch_headers = dict( + data=data, headers=headers_with_json_patch, + query_string=query_str + ) + + assert 403 == client.post(search_url, **args).status_code + assert 403 == client.put(rec_url, **args).status_code + assert 403 == client.patch( + rec_url, **args_with_json_patch_headers + ).status_code + assert 403 == client.delete( + rec_url, query_string=query_str + ).status_code diff --git a/tests/test_serializer_response.py b/tests/test_serializer_response.py index d757fc4a..a3c2fde1 100644 --- a/tests/test_serializer_response.py +++ b/tests/test_serializer_response.py @@ -36,11 +36,17 @@ def test_record_responsify(app): pid = PersistentIdentifier(pid_type='rec', pid_value='1') rec = Record({'title': 'test'}) - resp = rec_serializer(pid, rec, headers=[('X-Test', 'test')]) + resp = rec_serializer( + pid, + rec, + headers=[('X-Test', 'test')], + links_factory=lambda x: str(x) + ) assert resp.status_code == 200 assert resp.content_type == 'application/x-custom' assert resp.get_data(as_text=True) == "1:test" assert resp.headers['X-Test'] == 'test' + assert not resp.headers.get('Link') resp = rec_serializer(pid, rec, code=201) assert resp.status_code == 201 @@ -62,6 +68,7 @@ def fetcher(): assert resp.get_data(as_text=True) == "5" resp = search_serializer( - fetcher, result, code=201, headers=[('X-Test', 'test')]) + fetcher, result, code=201, headers=[('X-Test', 'test')], links="test") assert resp.status_code == 201 assert resp.headers['X-Test'] == 'test' + assert not resp.headers.get("Link") diff --git a/tests/test_views_search.py b/tests/test_views_search.py index 50e8cecb..44cd03a8 100644 --- a/tests/test_views_search.py +++ b/tests/test_views_search.py @@ -134,28 +134,14 @@ def test_page_links(app, indexed_records, search_url): res = client.get(search_url, query_string=dict(size=1, page=1)) assert_hits_len(res, 1) - def parse_link_header(response): - """Parse the links from a REST response's HTTP header.""" - return { - k: v for (k, v) in - map(lambda s: re.findall(r'<(.*)>; rel="(.*)"', s)[0][::-1], - [x for x in res.headers['Link'].split(', ')]) - } - - links = parse_link_header(res) data = get_json(res)['links'] - assert 'self' in data \ - and 'self' in links \ - and data['self'] == links['self'] - assert 'next' in data \ - and 'next' in links \ - and data['next'] == links['next'] - assert 'prev' not in data \ - and 'prev' not in links + assert 'self' in data + assert 'next' in data + assert 'prev' not in data # Assert next URL before calling it - first_url = links['self'] - next_url = links['next'] + first_url = data['self'] + next_url = data['next'] parsed_url = parse_url(next_url) assert parsed_url['qs']['size'] == ['1'] assert parsed_url['qs']['page'] == ['2'] @@ -163,10 +149,10 @@ def parse_link_header(response): # Access next URL res = client.get(to_relative_url(next_url)) assert_hits_len(res, 1) - links = parse_link_header(res) - assert links['self'] == next_url - assert 'next' in links - assert 'prev' in links and links['prev'] == first_url + data = get_json(res)['links'] + assert data['self'] == next_url + assert 'next' in data + assert 'prev' in data and data['prev'] == first_url def test_query(app, indexed_records, search_url):