Skip to content

Commit

Permalink
Merge pull request #544 from bento-platform/fix/chord/patch-dataset
Browse files Browse the repository at this point in the history
fix(chord): fix bad logic for checking if dataset project changed
  • Loading branch information
davidlougheed authored Sep 27, 2024
2 parents 06699e2 + 4606a8b commit 9c666ba
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 16 deletions.
11 changes: 11 additions & 0 deletions chord_metadata_service/authz/tests/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,17 @@ def one_no_authz_put(self, url: str, *args, **kwargs):
"""
return self._one_authz_put(False, url, *args, **kwargs)

def _one_authz_patch(self, authz_res: bool, url: str, *args, **kwargs):
with aioresponses() as m:
mock_authz_eval_one_result(m, authz_res)
return self.client.patch(url, *args, content_type="application/json", **kwargs)

def one_authz_patch(self, url: str, *args, **kwargs):
"""
Mocks a single True response from the authorization service and executes a JSON PATCH request.
"""
return self._one_authz_patch(True, url, *args, **kwargs)

def _one_authz_delete(self, authz_res: bool, url: str, *args, **kwargs):
with aioresponses() as m:
mock_authz_eval_one_result(m, authz_res)
Expand Down
2 changes: 1 addition & 1 deletion chord_metadata_service/chord/api_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ async def update(self, request, *args, **kwargs):
return forbidden(request) # side effect: sets authz done flag

# Do not allow datasets to change project
if request.data["project"] != dataset_project_id:
if "project" in request.data and request.data["project"] != dataset_project_id:
return bad_request(request, "Dataset project ID cannot change")

authz.mark_authz_done(request)
Expand Down
41 changes: 26 additions & 15 deletions chord_metadata_service/chord/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -236,9 +236,17 @@ def setUp(self):

def test_update_dataset(self):
r = self.one_authz_put(f"/api/datasets/{self.dataset.identifier}", data=json.dumps(self.valid_update))
assert r.status_code == status.HTTP_200_OK
self.assertEqual(r.status_code, status.HTTP_200_OK)
self.dataset.refresh_from_db()
self.assertEqual(self.dataset.title, self.valid_update["title"])

def test_update_dataset_partial(self):
r = self.one_authz_patch(
f"/api/datasets/{self.dataset.identifier}", data=json.dumps({"title": self.valid_update["title"]})
)
self.assertEqual(r.status_code, status.HTTP_200_OK)
self.dataset.refresh_from_db()
assert self.dataset.title == self.valid_update["title"]
self.assertEqual(self.dataset.title, self.valid_update["title"])

def test_update_dataset_changed_project(self):
r = self.one_authz_put(
Expand All @@ -248,50 +256,53 @@ def test_update_dataset_changed_project(self):
"project": str(self.project_2.identifier),
})
)
assert r.status_code == status.HTTP_400_BAD_REQUEST
self.assertEqual(r.status_code, status.HTTP_400_BAD_REQUEST)
res = r.json()
assert res["message"] == "Bad Request"
assert res["errors"][0]["message"] == "Dataset project ID cannot change"
self.assertEqual(res["message"], "Bad Request")
self.assertEqual(res["errors"][0]["message"], "Dataset project ID cannot change")

def test_update_dataset_bad_dats_json(self):
r = self.one_authz_put(
f"/api/datasets/{self.dataset.identifier}",
data=json.dumps({**self.valid_update, "dats_file": "asdf"}), # asdf is not JSON
)
assert r.status_code == status.HTTP_400_BAD_REQUEST
self.assertEqual(r.status_code, status.HTTP_400_BAD_REQUEST)
res = r.json()
assert res["message"] == "Bad Request"
assert res["errors"][0]["message"] == (
"Submitted dataset.dats_file data is not a valid JSON string. Make sure the string value is JSON "
"compatible, or submit dats_file as a JSON object."
self.assertEqual(res["message"], "Bad Request")
self.assertEqual(
res["errors"][0]["message"],
(
"Submitted dataset.dats_file data is not a valid JSON string. Make sure the string value is JSON "
"compatible, or submit dats_file as a JSON object."
)
)

def test_update_dataset_forbidden(self):
r = self.one_no_authz_put(f"/api/datasets/{self.dataset.identifier}", data=json.dumps(self.valid_update))
assert r.status_code == status.HTTP_403_FORBIDDEN
self.assertEqual(r.status_code, status.HTTP_403_FORBIDDEN)

def test_update_dataset_not_found(self):
r = self.one_authz_put(f"/api/datasets/{uuid.uuid4()}", data=json.dumps(self.valid_update))
assert r.status_code == status.HTTP_404_NOT_FOUND
self.assertEqual(r.status_code, status.HTTP_404_NOT_FOUND)


class DeleteDatasetTest(AuthzAPITestCase, ProjectTestCase):

def test_delete_dataset(self):
r = self.one_authz_delete(f"/api/datasets/{self.dataset.identifier}")
assert r.status_code == status.HTTP_204_NO_CONTENT
self.assertEqual(r.status_code, status.HTTP_204_NO_CONTENT)

with self.assertRaises(Dataset.DoesNotExist): # must not exist in DB anymore
self.dataset.refresh_from_db()

def test_delete_dataset_forbidden(self):
r = self.one_no_authz_delete(f"/api/datasets/{self.dataset.identifier}")
assert r.status_code == status.HTTP_403_FORBIDDEN
self.assertEqual(r.status_code, status.HTTP_403_FORBIDDEN)
self.dataset.refresh_from_db() # must not raise DoesNotExist

def test_delete_dataset_not_found(self):
r = self.client.delete(f"/api/datasets/{uuid.uuid4()}")
assert r.status_code == status.HTTP_404_NOT_FOUND
self.assertEqual(r.status_code, status.HTTP_404_NOT_FOUND)


class CreateProjectJsonSchema(AuthzAPITestCaseWithProjectJSON):
Expand Down

0 comments on commit 9c666ba

Please sign in to comment.