Changeset View
Standalone View
swh/deposit/tests/api/test_collection_list.py
- This file was added.
| # Copyright (C) 2017-2021 The Software Heritage developers | |||||
| # See the AUTHORS file at the top-level directory of this distribution | |||||
| # License: GNU General Public License version 3, or any later version | |||||
| # See top-level LICENSE file for more information | |||||
| from io import BytesIO | |||||
| from django.urls import reverse_lazy as reverse | |||||
| from requests.utils import parse_header_links | |||||
| from rest_framework import status | |||||
| from swh.deposit.config import ( | |||||
| COLLECTION_LIST, | |||||
| DEPOSIT_STATUS_DEPOSITED, | |||||
| DEPOSIT_STATUS_PARTIAL, | |||||
| ) | |||||
| from swh.deposit.models import DepositCollection | |||||
| from swh.deposit.parsers import parse_xml | |||||
anlambert: Deposit list api requires authentication | |||||
| def test_deposit_collection_list_is_auth_protected(anonymous_client): | |||||
| """Deposit list should require authentication | |||||
| """ | |||||
Done Inline ActionsIf the response data is JSON, as anonymous_client is a rest_framework.test.APIClient instance, you can use response.data here instead of response.content. anlambert: If the response data is JSON, as `anonymous_client` is a `rest_framework.test.APIClient`… | |||||
Done Inline Actionsas mentioned in the description, not in case of error, the answer is then xml but gotcha, i'll check the nominal case. I double checked to be sure ;) [31] > /home/tony/work/inria/repo/swh/swh-environment/swh-deposit/swh/deposit/tests/api/test_collection_list.py(26)test_deposit_collection_list_is_auth_protected()
-> assert b"protected by basic authentication" in response.content
5 frames hidden (try 'help hidden_frames')
(Pdb++) response.data
*** AttributeError: 'HttpResponse' object has no attribute 'data'
(Pdb++) ll
17 def test_deposit_collection_list_is_auth_protected(anonymous_client):
18 """Deposit list requires authentication
19
20 """
21 url = reverse(COLLECTION_LIST, args=("test",))
22 response = anonymous_client.get(url)
23 assert response.status_code == status.HTTP_401_UNAUTHORIZED
24 import pdb; pdb.set_trace()
25ardumont: as mentioned in the description, not in case of error, the answer is then xml but gotcha, i'll… | |||||
Not Done Inline ActionsIs it easy to turn the XML error response into a JSON one ? That's weird to have different response formats for an endpoint. anlambert: Is it easy to turn the XML error response into a JSON one ?
That's weird to have different… | |||||
Not Done Inline ActionsYou could for instance testing on the request content_type in make_error_response_from_dict and return a JSON response when requested. anlambert: You could for instance testing on the request `content_type` in [[ https://forge. | |||||
Done Inline ActionsAgreed on the inconsistency weirdness, see my other answer to val. I'm more of a mind to try and see if that other public endpoints can ardumont: Agreed on the inconsistency weirdness, see my other answer to val.
Yes, we can do as you… | |||||
Not Done Inline ActionsAck, I guess you will have to write a custom DRF renderer to output xml then. anlambert: Ack, I guess you will have to write a custom [[ https://www.django-rest-framework.org/api… | |||||
Done Inline Actions(1) I looked into it and found one [1] (albeit not packaged in debian). It does the job (i tried But then i recalled we did not use specific xml renderer so far. Plus, (1) clashes with the default error answers (which are rendered Thanks though. [1] https://jpadilla.github.io/django-rest-framework-xml/renderers/#setting-the-renderers ardumont: (1) I looked into it and found one [1] (albeit not packaged in debian). It does the job (i… | |||||
Done Inline Actionsif what I am saying is possible at all ¯\_(ツ)_/¯ ardumont: if what I am saying is possible at all ¯\_(ツ)_/¯ | |||||
Done Inline Actionsit is, i just got side-tracked by [1] not working [2] is the answer [1] {% for key, value in deposit.items() %} # d'oh[2] {% for key, value in deposit.items %}ardumont: it is, i just got side-tracked by [1] not working [2] is the answer
[1]
```
{% for key… | |||||
Not Done Inline ActionsYes, django template language has some limitations. You cannot call a function with arguments for instance. anlambert: Yes, django template language has some limitations. You cannot call a function with arguments… | |||||
| url = reverse(COLLECTION_LIST, args=("test",)) | |||||
| response = anonymous_client.get(url) | |||||
| assert response.status_code == status.HTTP_401_UNAUTHORIZED | |||||
| assert b"protected by basic authentication" in response.content | |||||
| def test_deposit_collection_list_collection_access_restricted_to_user_coll( | |||||
| deposit_another_collection, deposit_user, authenticated_client | |||||
| ): | |||||
| """Deposit list api should restrict access to user's collection | |||||
| """ | |||||
| collection_id = authenticated_client.deposit_client.collections[0] | |||||
| coll = DepositCollection.objects.get(pk=collection_id) | |||||
| # authenticated_client has access to the "coll" collection | |||||
| coll2 = deposit_another_collection | |||||
| assert coll.name != coll2.name | |||||
| # but does not have access to that coll2 collection | |||||
| url = reverse(COLLECTION_LIST, args=(coll2.name,)) | |||||
| response = authenticated_client.get(url) | |||||
| # so it gets rejected access to the listing of that coll2 collection | |||||
| assert response.status_code == status.HTTP_403_FORBIDDEN | |||||
| msg = f"{deposit_user.username} cannot access collection {coll2.name}" | |||||
| assert msg in response.content.decode("utf-8") | |||||
| def test_deposit_collection_list_nominal( | |||||
| partial_deposit, deposited_deposit, authenticated_client | |||||
| ): | |||||
| """Deposit list api should return the user deposits in a paginated way | |||||
| """ | |||||
| client_id = authenticated_client.deposit_client.id | |||||
| assert partial_deposit.client.id == client_id | |||||
| assert deposited_deposit.client.id == client_id | |||||
| # Both deposit were deposited by the authenticated client | |||||
| # so requesting the listing of the deposits, both should be listed | |||||
| deposit_id = str(partial_deposit.id) | |||||
| deposit_id2 = str(deposited_deposit.id) | |||||
| coll = partial_deposit.collection | |||||
| # requesting the listing of the deposit for the user's collection | |||||
| url = reverse(COLLECTION_LIST, args=(coll.name,)) | |||||
Done Inline ActionsAs authenticated_client is an instance of rest_framework.test.APIClient, you have directly access to the data using response.data. anlambert: As `authenticated_client` is an instance of `rest_framework.test.APIClient`, you have directly… | |||||
Done Inline Actionsright! ardumont: right! | |||||
| response = authenticated_client.get(f"{url}?page_size=1") | |||||
| assert response.status_code == status.HTTP_200_OK | |||||
| data = parse_xml(BytesIO(response.content)) | |||||
| assert ( | |||||
| data["swh:count"] == "2" | |||||
| ) # total result of 2 deposits if consuming all results | |||||
| header_link = parse_header_links(response._headers["Link"]) | |||||
| assert len(header_link) == 1 # only 1 next link | |||||
| expected_next = f"{url}?page=2&page_size=1" | |||||
| assert header_link[0]["url"].endswith(expected_next) | |||||
| assert header_link[0]["rel"] == "next" | |||||
| # only one deposit in the response | |||||
| deposit = data["swh:deposits"]["swh:deposit"] # dict as only 1 value (a-la js) | |||||
| assert isinstance(deposit, dict) | |||||
| assert deposit["swh:id"] == deposit_id | |||||
| assert deposit["swh:status"] == DEPOSIT_STATUS_PARTIAL | |||||
| # then 2nd page | |||||
| response2 = authenticated_client.get(expected_next) | |||||
| assert response2.status_code == status.HTTP_200_OK | |||||
| data2 = parse_xml(BytesIO(response2.content)) | |||||
| assert data2["swh:count"] == "2" # still total of 2 deposits across all results | |||||
| expected_previous = f"{url}?page_size=1" | |||||
| header_link2 = parse_header_links(response2._headers["Link"]) | |||||
| assert len(header_link2) == 1 # only 1 previous link | |||||
| assert header_link2[0]["url"].endswith(expected_previous) | |||||
| assert header_link2[0]["rel"] == "previous" | |||||
| # only 1 deposit in the response | |||||
| deposit2 = data2["swh:deposits"]["swh:deposit"] # dict as only 1 value (a-la js) | |||||
| assert isinstance(deposit2, dict) | |||||
| assert deposit2["swh:id"] == deposit_id2 | |||||
| assert deposit2["swh:status"] == DEPOSIT_STATUS_DEPOSITED | |||||
| # Retrieve every deposit in one query (no page_size parameter) | |||||
| response3 = authenticated_client.get(url) | |||||
| assert response3.status_code == status.HTTP_200_OK | |||||
| data3 = parse_xml(BytesIO(response3.content)) | |||||
| assert data3["swh:count"] == "2" # total result of 2 deposits across all results | |||||
| deposits3 = data3["swh:deposits"]["swh:deposit"] # list here | |||||
| assert isinstance(deposits3, list) | |||||
| assert len(deposits3) == 2 | |||||
| header_link3 = parse_header_links(response3._headers["Link"]) | |||||
| assert header_link3 == [] # no pagination as all results received in one round | |||||
| assert deposit in deposits3 | |||||
| assert deposit2 in deposits3 | |||||
Deposit list api requires authentication