Review changes

uc-cdis · Jul 2, 2024 · 391865b · 391865b
1 parent 71ec4e7
commit 391865b
Show file tree

Hide file tree

Showing 3 changed files with 75 additions and 36 deletions.
diff --git a/fence/blueprints/data/indexd.py b/fence/blueprints/data/indexd.py
@@ -260,9 +260,8 @@ def __init__(
         self.authz = authz
 
         self.guid = guid
-        self.guid = self.index_document[
-            "did"
-        ]  # .index_document is a cached property with code below, it creates/retrieves the actual record and this line updates the stored GUID to the returned record
+        # .index_document is a cached property with code below, it creates/retrieves the actual record and this line updates the stored GUID to the returned record
+        self.guid = self.index_document["did"]
 
     @cached_property
     def index_document(self):

diff --git a/openapis/swagger.yaml b/openapis/swagger.yaml
@@ -566,7 +566,7 @@ paths:
         "file_upload" on "/data_file" resource. When "authz" is *not* provided, this
         endpoint will check for that permission for your user.
 
-        Accepts a "guid" field in the request body. If "guid" is provided, it checks indexd for an existing record. If not found, a new blank record will be created.
+        Accepts a "guid" field in the request body. If "guid" is provided, it checks indexd for an existing record. If not found, it raises a 404.
       security:
         - OAuth2:
           - user
@@ -592,6 +592,8 @@ paths:
                   url:
                     type: string
                     description: the presigned URL usable for data upload
+        404:
+          description: Record with <guid> not found.
   '/data/upload/{file_id}':
     get:
       tags:
@@ -671,7 +673,7 @@ paths:
         the GUID for this new record and an uploadId for multipart upload presigned url.
 
 
-        Accepts a "guid" field in the request body. If "guid" is provided, it checks indexd for an existing record. If not found, a new blank record will be created.
+        Accepts a "guid" field in the request body. If "guid" is provided, it checks indexd for an existing record. If not found, it raises a 404.
       security:
         - OAuth2:
           - user
@@ -697,6 +699,8 @@ paths:
                   uploadId:
                     type: string
                     description: the uploadId for multipart upload presigned URL usable for data upload
+        404:
+          description: Record with <guid> not found.
 
   '/multipart/upload':
     post:

diff --git a/tests/data/test_data.py b/tests/data/test_data.py
@@ -1730,9 +1730,8 @@ def json(self):
         assert "uploadId" in response.json
 
 
-@pytest.mark.parametrize("indexd_200_response", [True, False])
 def test_initialize_multipart_upload_with_guid_in_request(
-    app, client, auth_client, encoded_creds_jwt, user_client, indexd_200_response
+    app, client, auth_client, encoded_creds_jwt, user_client
 ):
     """
     Test /data/multipart/init with guid parameter in request data
@@ -1756,28 +1755,71 @@ def json(self):
     fence.blueprints.data.indexd.BlankIndex.init_multipart_upload = MagicMock()
     with data_requests_mocker as data_requests, arborist_requests_mocker as arborist_requests:
         did = str(uuid.uuid4())
-        if indexd_200_response:
-            data_requests.get.return_value = MockResponse(
-                {
-                    "did": did,
-                    "baseid": "",
-                    "rev": "",
-                    "size": 10,
-                    "file_name": "file1",
-                    "urls": ["s3://bucket1/key"],
-                    "hashes": {},
-                    "metadata": {},
-                    "authz": ["/open"],
-                    "acl": ["*"],
-                    "form": "",
-                    "created_date": "",
-                    "updated_date": "",
-                }
-            )
-            data_requests.get.return_value.status_code = 200
-        else:
-            data_requests.get.return_value = MockResponse("no record found")
-            data_requests.get.return_value.status_code = 404
+        data_requests.get.return_value = MockResponse(
+            {
+                "did": did,
+                "baseid": "",
+                "rev": "",
+                "size": 10,
+                "file_name": "file1",
+                "urls": ["s3://bucket1/key"],
+                "hashes": {},
+                "metadata": {},
+                "authz": ["/open"],
+                "acl": ["*"],
+                "form": "",
+                "created_date": "",
+                "updated_date": "",
+            }
+        )
+        data_requests.get.return_value.status_code = 200
+
+        arborist_requests.return_value = MockResponse({"auth": True})
+        arborist_requests.return_value.status_code = 200
+        fence.blueprints.data.indexd.BlankIndex.init_multipart_upload.return_value = (
+            "test_uploadId"
+        )
+        headers = {
+            "Authorization": "Bearer " + encoded_creds_jwt.jwt,
+            "Content-Type": "application/json",
+        }
+        file_name = "asdf"
+        data = json.dumps({"file_name": file_name, "guid": did})
+        response = client.post("/data/multipart/init", headers=headers, data=data)
+
+        assert response.status_code == 201, response
+        assert "guid" in response.json
+        assert did == response.json.get("guid")
+        assert "uploadId" in response.json
+
+
+def test_initialize_multipart_upload_with_non_existent_guid_in_request(
+    app, client, auth_client, encoded_creds_jwt, user_client
+):
+    """
+    Test /data/multipart/init with guid parameter in request data but no guid exist in indexd
+    """
+
+    class MockResponse(object):
+        def __init__(self, data, status_code=200):
+            self.data = data
+            self.status_code = status_code
+
+        def json(self):
+            return self.data
+
+    data_requests_mocker = mock.patch(
+        "fence.blueprints.data.indexd.requests", new_callable=mock.Mock
+    )
+    arborist_requests_mocker = mock.patch(
+        "gen3authz.client.arborist.client.httpx.Client.request", new_callable=mock.Mock
+    )
+
+    fence.blueprints.data.indexd.BlankIndex.init_multipart_upload = MagicMock()
+    with data_requests_mocker as data_requests, arborist_requests_mocker as arborist_requests:
+        did = str(uuid.uuid4())
+        data_requests.get.return_value = MockResponse("no record found")
+        data_requests.get.return_value.status_code = 404
         arborist_requests.return_value = MockResponse({"auth": True})
         arborist_requests.return_value.status_code = 200
         fence.blueprints.data.indexd.BlankIndex.init_multipart_upload.return_value = (
@@ -1791,13 +1833,7 @@ def json(self):
         data = json.dumps({"file_name": file_name, "guid": did})
         response = client.post("/data/multipart/init", headers=headers, data=data)
 
-        if indexd_200_response:
-            assert response.status_code == 201, response
-            assert "guid" in response.json
-            assert did == response.json.get("guid")
-            assert "uploadId" in response.json
-        else:
-            assert response.status_code == 404, response
+        assert response.status_code == 404, response
 
 
 def test_multipart_upload_presigned_url(