diff --git a/renku/service/controllers/datasets_create.py b/renku/service/controllers/datasets_create.py index 039f8d15a9..01eeed5117 100644 --- a/renku/service/controllers/datasets_create.py +++ b/renku/service/controllers/datasets_create.py @@ -21,6 +21,7 @@ from renku.service.config import CACHE_UPLOADS_PATH from renku.service.controllers.api.abstract import ServiceCtrl from renku.service.controllers.api.mixins import RenkuOpSyncMixin +from renku.service.controllers.utils.datasets import set_url_for_uploaded_images from renku.service.serializers.datasets import DatasetCreateRequest, DatasetCreateResponseRPC from renku.service.views import result_response @@ -45,16 +46,11 @@ def context(self): """Controller operation context.""" return self.ctx - def _handle_uploaded_images(self): - """Handles uploaded or relative dataset images.""" - for img in self.ctx.get("images", []): - if img.get("file_id"): - file = self.cache.get_file(self.user, img.pop("file_id")) - img["content_url"] = str(file.abs_path) - def renku_op(self): """Renku operation for the controller.""" - self._handle_uploaded_images() + images = self.ctx.get("images") + if images: + set_url_for_uploaded_images(images=images, cache=self.cache, user=self.user) user_cache_dir = CACHE_UPLOADS_PATH / self.user.user_id return ( diff --git a/renku/service/controllers/datasets_edit.py b/renku/service/controllers/datasets_edit.py index d2b4bd1574..02f4e54c15 100644 --- a/renku/service/controllers/datasets_edit.py +++ b/renku/service/controllers/datasets_edit.py @@ -21,6 +21,7 @@ from renku.service.config import CACHE_UPLOADS_PATH from renku.service.controllers.api.abstract import ServiceCtrl from renku.service.controllers.api.mixins import RenkuOpSyncMixin +from renku.service.controllers.utils.datasets import set_url_for_uploaded_images from renku.service.serializers.datasets import DatasetEditRequest, DatasetEditResponseRPC from renku.service.views import result_response @@ -47,6 +48,9 @@ def context(self): def renku_op(self): """Renku operation for the controller.""" + images = self.ctx.get("images") + if images: + set_url_for_uploaded_images(images=images, cache=self.cache, user=self.user) user_cache_dir = CACHE_UPLOADS_PATH / self.user.user_id result = ( diff --git a/renku/service/controllers/utils/__init__.py b/renku/service/controllers/utils/__init__.py new file mode 100644 index 0000000000..3b45c3097a --- /dev/null +++ b/renku/service/controllers/utils/__init__.py @@ -0,0 +1,18 @@ +# -*- coding: utf-8 -*- +# +# Copyright 2020 - Swiss Data Science Center (SDSC) +# A partnership between École Polytechnique Fédérale de Lausanne (EPFL) and +# Eidgenössische Technische Hochschule Zürich (ETHZ). +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +"""Renku service controller utils.""" diff --git a/renku/service/controllers/utils/datasets.py b/renku/service/controllers/utils/datasets.py new file mode 100644 index 0000000000..af8dbaeb13 --- /dev/null +++ b/renku/service/controllers/utils/datasets.py @@ -0,0 +1,26 @@ +# -*- coding: utf-8 -*- +# +# Copyright 2020 - Swiss Data Science Center (SDSC) +# A partnership between École Polytechnique Fédérale de Lausanne (EPFL) and +# Eidgenössische Technische Hochschule Zürich (ETHZ). +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +"""Utilities for renku service dataset controllers.""" + + +def set_url_for_uploaded_images(images, cache, user): + """Set proper url/path for uploaded or relative dataset images.""" + for img in images: + if img.get("file_id"): + file = cache.get_file(user, img.pop("file_id")) + img["content_url"] = str(file.abs_path) diff --git a/renku/service/serializers/datasets.py b/renku/service/serializers/datasets.py index 355b09a3c0..1968045e16 100644 --- a/renku/service/serializers/datasets.py +++ b/renku/service/serializers/datasets.py @@ -233,6 +233,7 @@ class DatasetImportResponseRPC(JsonRPCResponse): class DatasetEditRequest( AsyncSchema, CommitSchema, + DatasetDetailsRequest, DatasetNameSchema, DatasetRefSchema, LocalRepositorySchema, @@ -245,7 +246,6 @@ class DatasetEditRequest( description = fields.String(default=None) creators = fields.List(fields.Nested(DatasetCreators)) keywords = fields.List(fields.String()) - images = fields.List(fields.Nested(ImageObjectRequest)) class DatasetEditResponse(RenkuSyncSchema): diff --git a/tests/service/views/test_dataset_views.py b/tests/service/views/test_dataset_views.py index 5ce8092c1b..7b48801e30 100644 --- a/tests/service/views/test_dataset_views.py +++ b/tests/service/views/test_dataset_views.py @@ -45,6 +45,31 @@ def assert_rpc_response(response, with_key="result"): assert with_key in response_text +def upload_file(svc_client, headers, filename) -> str: + """Upload a file to the service cache.""" + content_type = headers.pop("Content-Type") + + response = svc_client.post( + "/cache.files_upload", + data=dict(file=(io.BytesIO(b"Test file content"), filename)), + query_string={"override_existing": True}, + headers=headers, + ) + + assert response + assert 200 == response.status_code + assert_rpc_response(response) + + assert 1 == len(response.json["result"]["files"]) + + file_id = response.json["result"]["files"][0]["file_id"] + assert isinstance(uuid.UUID(file_id), uuid.UUID) + + headers["Content-Type"] = content_type + + return file_id + + @pytest.mark.service @pytest.mark.integration @retry_failed @@ -337,41 +362,8 @@ def test_create_dataset_with_uploaded_images(svc_client_with_repo): """Create a new dataset with metadata.""" svc_client, headers, project_id, _ = svc_client_with_repo - content_type = headers.pop("Content-Type") - - response = svc_client.post( - "/cache.files_upload", - data=dict(file=(io.BytesIO(b"this is a test"), "image1.jpg")), - query_string={"override_existing": True}, - headers=headers, - ) - - assert response - assert 200 == response.status_code - assert_rpc_response(response) - - assert 1 == len(response.json["result"]["files"]) - - file_id1 = response.json["result"]["files"][0]["file_id"] - assert isinstance(uuid.UUID(file_id1), uuid.UUID) - - response = svc_client.post( - "/cache.files_upload", - data=dict(file=(io.BytesIO(b"this is another test"), "image2.png")), - query_string={"override_existing": True}, - headers=headers, - ) - - assert response - assert 200 == response.status_code - assert_rpc_response(response) - - assert 1 == len(response.json["result"]["files"]) - - file_id2 = response.json["result"]["files"][0]["file_id"] - assert isinstance(uuid.UUID(file_id2), uuid.UUID) - - headers["Content-Type"] = content_type + file_id1 = upload_file(svc_client, headers, "image1.jpg") + file_id2 = upload_file(svc_client, headers, "image2.png") payload = { "project_id": project_id, @@ -562,23 +554,8 @@ def test_add_file_view_with_no_identity(svc_client_with_repo): def test_add_file_view(svc_client_with_repo): """Check adding of uploaded file to dataset.""" svc_client, headers, project_id, _ = svc_client_with_repo - content_type = headers.pop("Content-Type") - response = svc_client.post( - "/cache.files_upload", - data=dict(file=(io.BytesIO(b"this is a test"), "datafile1.txt")), - query_string={"override_existing": True}, - headers=headers, - ) - - assert response - assert 200 == response.status_code - assert_rpc_response(response) - - assert 1 == len(response.json["result"]["files"]) - - file_id = response.json["result"]["files"][0]["file_id"] - assert isinstance(uuid.UUID(file_id), uuid.UUID) + file_id = upload_file(svc_client, headers, "datafile1.txt") payload = { "project_id": project_id, @@ -586,7 +563,6 @@ def test_add_file_view(svc_client_with_repo): "create_dataset": True, "files": [{"file_id": file_id}], } - headers["Content-Type"] = content_type response = svc_client.post("/datasets.add", data=json.dumps(payload), headers=headers) @@ -605,17 +581,8 @@ def test_add_file_view(svc_client_with_repo): def test_add_file_commit_msg(svc_client_with_repo): """Check adding of uploaded file to dataset with custom commit message.""" svc_client, headers, project_id, _ = svc_client_with_repo - content_type = headers.pop("Content-Type") - response = svc_client.post( - "/cache.files_upload", - data=dict(file=(io.BytesIO(b"this is a test"), "datafile1.txt")), - query_string={"override_existing": True}, - headers=headers, - ) - - file_id = response.json["result"]["files"][0]["file_id"] - assert isinstance(uuid.UUID(file_id), uuid.UUID) + file_id = upload_file(svc_client, headers, "datafile1.txt") payload = { "commit_message": "my awesome data file", @@ -624,7 +591,6 @@ def test_add_file_commit_msg(svc_client_with_repo): "create_dataset": True, "files": [{"file_id": file_id}], } - headers["Content-Type"] = content_type response = svc_client.post("/datasets.add", data=json.dumps(payload), headers=headers) assert response @@ -660,17 +626,8 @@ def test_remote_add_view(svc_client, it_remote_repo_url, identity_headers): def test_add_file_failure(svc_client_with_repo): """Check adding of uploaded file to dataset with non-existing file.""" svc_client, headers, project_id, _ = svc_client_with_repo - content_type = headers.pop("Content-Type") - - response = svc_client.post( - "/cache.files_upload", - data=dict(file=(io.BytesIO(b"this is a test"), "datafile1.txt")), - query_string={"override_existing": True}, - headers=headers, - ) - file_id = response.json["result"]["files"][0]["file_id"] - assert isinstance(uuid.UUID(file_id), uuid.UUID) + file_id = upload_file(svc_client, headers, "datafile1.txt") payload = { "commit_message": "my awesome data file", @@ -679,7 +636,6 @@ def test_add_file_failure(svc_client_with_repo): "create_dataset": True, "files": [{"file_id": file_id}, {"file_path": "my problem right here"}], } - headers["Content-Type"] = content_type response = svc_client.post("/datasets.add", data=json.dumps(payload), headers=headers) assert response @@ -931,30 +887,15 @@ def test_create_and_list_datasets_view(svc_client_with_repo): def test_list_dataset_files(svc_client_with_repo): """Check listing of dataset files""" svc_client, headers, project_id, _ = svc_client_with_repo - content_type = headers.pop("Content-Type") file_name = uuid.uuid4().hex - response = svc_client.post( - "/cache.files_upload", - data=dict(file=(io.BytesIO(b"this is a test"), file_name)), - query_string={"override_existing": True}, - headers=headers, - ) - - assert response - assert 200 == response.status_code - assert_rpc_response(response) - - assert 1 == len(response.json["result"]["files"]) - file_id = response.json["result"]["files"][0]["file_id"] - assert isinstance(uuid.UUID(file_id), uuid.UUID) + file_id = upload_file(svc_client, headers, file_name) payload = { "project_id": project_id, "name": "mydata", "files": [{"file_id": file_id}], } - headers["Content-Type"] = content_type response = svc_client.post("/datasets.add", data=json.dumps(payload), headers=headers) @@ -1483,6 +1424,8 @@ def test_edit_dataset_with_images(svc_client_with_repo): assert response assert_rpc_response(response) + file_id = upload_file(svc_client, headers, "image2.jpg") + # NOTE: test edit reordering and add edit_payload = { "project_id": project_id, @@ -1491,6 +1434,7 @@ def test_edit_dataset_with_images(svc_client_with_repo): {"content_url": "data/renku_logo.png", "position": 1}, {"content_url": "https://example.com/image1.jpg", "position": 2}, {"content_url": "https://example.com/other_image.jpg", "position": 3}, + {"file_id": file_id, "position": 4}, ], } response = svc_client.post("/datasets.edit", data=json.dumps(edit_payload), headers=headers) @@ -1502,15 +1446,15 @@ def test_edit_dataset_with_images(svc_client_with_repo): assert {"images"} == response.json["result"]["edited"].keys() images = response.json["result"]["edited"]["images"] - assert len(images) == 3 - img1 = next(img for img in images if img["position"] == 1) - img2 = next(img for img in images if img["position"] == 2) - img3 = next(img for img in images if img["position"] == 3) - - assert img1["content_url"].startswith(".renku/dataset_images/") - assert img1["content_url"].endswith("/1.png") - assert img2["content_url"] == "https://example.com/image1.jpg" - assert img3["content_url"] == "https://example.com/other_image.jpg" + assert len(images) == 4 + images.sort(key=lambda x: x["position"]) + + assert images[0]["content_url"].startswith(".renku/dataset_images/") + assert images[0]["content_url"].endswith("/1.png") + assert images[1]["content_url"] == "https://example.com/image1.jpg" + assert images[2]["content_url"] == "https://example.com/other_image.jpg" + assert images[3]["content_url"].startswith(".renku/dataset_images/") + assert images[3]["content_url"].endswith("/4.jpg") # NOTE: test edit with duplicate position edit_payload = {