🐛 Fix webserver storage OpenAPI schema (#4426)

Co-authored-by: Andrei Neagu <[email protected]>
ITISFoundation · Jun 27, 2023 · 9dc5a02 · 9dc5a02
1 parent 03dc50a
commit 9dc5a02
Show file tree

Hide file tree

Showing 6 changed files with 170 additions and 81 deletions.
diff --git a/api/specs/webserver/openapi-storage.yaml b/api/specs/webserver/openapi-storage.yaml
@@ -144,11 +144,7 @@ paths:
       - required: true
         schema:
           title: File Id
-          anyOf:
-          - pattern: ^(api|([0-9a-fA-F]{8}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{12}))\/([0-9a-fA-F]{8}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{12})\/(.+)$
-            type: string
-          - pattern: ^N:package:[0-9a-fA-F]{8}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{12}$
-            type: string
+          type: string
         name: file_id
         in: path
       responses:
@@ -178,11 +174,7 @@ paths:
       - required: true
         schema:
           title: File Id
-          anyOf:
-          - pattern: ^(api|([0-9a-fA-F]{8}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{12}))\/([0-9a-fA-F]{8}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{12})\/(.+)$
-            type: string
-          - pattern: ^N:package:[0-9a-fA-F]{8}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{12}$
-            type: string
+          type: string
         name: file_id
         in: path
       - required: false
@@ -216,11 +208,7 @@ paths:
       - required: true
         schema:
           title: File Id
-          anyOf:
-          - pattern: ^(api|([0-9a-fA-F]{8}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{12}))\/([0-9a-fA-F]{8}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{12})\/(.+)$
-            type: string
-          - pattern: ^N:package:[0-9a-fA-F]{8}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{12}$
-            type: string
+          type: string
         name: file_id
         in: path
       - required: true
@@ -269,11 +257,7 @@ paths:
       - required: true
         schema:
           title: File Id
-          anyOf:
-          - pattern: ^(api|([0-9a-fA-F]{8}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{12}))\/([0-9a-fA-F]{8}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{12})\/(.+)$
-            type: string
-          - pattern: ^N:package:[0-9a-fA-F]{8}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{12}$
-            type: string
+          type: string
         name: file_id
         in: path
       responses:
@@ -297,11 +281,7 @@ paths:
       - required: true
         schema:
           title: File Id
-          anyOf:
-          - pattern: ^(api|([0-9a-fA-F]{8}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{12}))\/([0-9a-fA-F]{8}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{12})\/(.+)$
-            type: string
-          - pattern: ^N:package:[0-9a-fA-F]{8}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{12}$
-            type: string
+          type: string
         name: file_id
         in: path
       responses:
@@ -324,11 +304,7 @@ paths:
       - required: true
         schema:
           title: File Id
-          anyOf:
-          - pattern: ^(api|([0-9a-fA-F]{8}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{12}))\/([0-9a-fA-F]{8}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{12})\/(.+)$
-            type: string
-          - pattern: ^N:package:[0-9a-fA-F]{8}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{12}$
-            type: string
+          type: string
         name: file_id
         in: path
       requestBody:
@@ -361,11 +337,7 @@ paths:
       - required: true
         schema:
           title: File Id
-          anyOf:
-          - pattern: ^(api|([0-9a-fA-F]{8}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{12}))\/([0-9a-fA-F]{8}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{12})\/(.+)$
-            type: string
-          - pattern: ^N:package:[0-9a-fA-F]{8}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{12}$
-            type: string
+          type: string
         name: file_id
         in: path
       - required: true

diff --git a/api/specs/webserver/scripts/openapi_storage.py b/api/specs/webserver/scripts/openapi_storage.py
@@ -8,6 +8,7 @@
 
 
 from enum import Enum
+from typing import TypeAlias
 
 from fastapi import FastAPI, status
 from models_library.api_schemas_storage import (
@@ -21,7 +22,7 @@
     TableSynchronisation,
 )
 from models_library.generics import Envelope
-from models_library.projects_nodes_io import LocationID, StorageFileID
+from models_library.projects_nodes_io import LocationID
 from pydantic import AnyUrl, ByteSize
 from simcore_service_webserver.storage.schemas import DatasetMetaData, FileMetaData
 
@@ -32,6 +33,13 @@
 ]
 
 
+# NOTE: storage generates URLs that contain double encoded
+# slashes, and when applying validation via `StorageFileID`
+# it raises an error. Before `StorageFileID`, `str` was the
+# type used in the OpenAPI specs.
+StorageFileIDStr: TypeAlias = str
+
+
 @app.get(
     "/storage/locations",
     response_model=list[DatasetMetaData],
@@ -96,7 +104,7 @@ async def get_files_metadata_dataset(location_id: LocationID, dataset_id: str):
     summary="Get File Metadata",
     operation_id="get_file_metadata",
 )
-async def get_file_metadata(location_id: LocationID, file_id: StorageFileID):
+async def get_file_metadata(location_id: LocationID, file_id: StorageFileIDStr):
     ...
 
 
@@ -109,7 +117,7 @@ async def get_file_metadata(location_id: LocationID, file_id: StorageFileID):
 )
 async def download_file(
     location_id: LocationID,
-    file_id: StorageFileID,
+    file_id: StorageFileIDStr,
     link_type: LinkType = LinkType.PRESIGNED,
 ):
     """Returns a presigned link"""
@@ -124,7 +132,7 @@ async def download_file(
 )
 async def upload_file(
     location_id: LocationID,
-    file_id: StorageFileID,
+    file_id: StorageFileIDStr,
     file_size: ByteSize | None,
     link_type: LinkType = LinkType.PRESIGNED,
     is_directory: bool = False,
@@ -139,7 +147,7 @@ async def upload_file(
     operation_id="delete_file",
     summary="Deletes File",
 )
-async def delete_file(location_id: LocationID, file_id: StorageFileID):
+async def delete_file(location_id: LocationID, file_id: StorageFileIDStr):
     ...
 
 
@@ -149,7 +157,7 @@ async def delete_file(location_id: LocationID, file_id: StorageFileID):
     tags=TAGS,
     operation_id="abort_upload_file",
 )
-async def abort_upload_file(location_id: LocationID, file_id: StorageFileID):
+async def abort_upload_file(location_id: LocationID, file_id: StorageFileIDStr):
     """Asks the server to abort the upload and revert to the last valid version if any"""
 
 
@@ -163,7 +171,7 @@ async def abort_upload_file(location_id: LocationID, file_id: StorageFileID):
 async def complete_upload_file(
     body_item: Envelope[FileUploadCompletionBody],
     location_id: LocationID,
-    file_id: StorageFileID,
+    file_id: StorageFileIDStr,
 ):
     """Asks the server to complete the upload"""
 
@@ -176,7 +184,7 @@ async def complete_upload_file(
     operation_id="is_completed_upload_file",
 )
 async def is_completed_upload_file(
-    location_id: LocationID, file_id: StorageFileID, future_id: str
+    location_id: LocationID, file_id: StorageFileIDStr, future_id: str
 ):
     """Returns state of upload completion"""
 

diff --git a/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml b/services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml
@@ -2026,11 +2026,7 @@ paths:
         - required: true
           schema:
             title: File Id
-            anyOf:
-              - pattern: '^(api|([0-9a-fA-F]{8}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{12}))\/([0-9a-fA-F]{8}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{12})\/(.+)$'
-                type: string
-              - pattern: '^N:package:[0-9a-fA-F]{8}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{12}$'
-                type: string
+            type: string
           name: file_id
           in: path
         - required: false
@@ -2084,11 +2080,7 @@ paths:
         - required: true
           schema:
             title: File Id
-            anyOf:
-              - pattern: '^(api|([0-9a-fA-F]{8}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{12}))\/([0-9a-fA-F]{8}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{12})\/(.+)$'
-                type: string
-              - pattern: '^N:package:[0-9a-fA-F]{8}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{12}$'
-                type: string
+            type: string
           name: file_id
           in: path
         - required: true
@@ -2187,11 +2179,7 @@ paths:
         - required: true
           schema:
             title: File Id
-            anyOf:
-              - pattern: '^(api|([0-9a-fA-F]{8}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{12}))\/([0-9a-fA-F]{8}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{12})\/(.+)$'
-                type: string
-              - pattern: '^N:package:[0-9a-fA-F]{8}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{12}$'
-                type: string
+            type: string
           name: file_id
           in: path
       responses:
@@ -2214,11 +2202,7 @@ paths:
         - required: true
           schema:
             title: File Id
-            anyOf:
-              - pattern: '^(api|([0-9a-fA-F]{8}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{12}))\/([0-9a-fA-F]{8}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{12})\/(.+)$'
-                type: string
-              - pattern: '^N:package:[0-9a-fA-F]{8}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{12}$'
-                type: string
+            type: string
           name: file_id
           in: path
       requestBody:
@@ -2301,11 +2285,7 @@ paths:
         - required: true
           schema:
             title: File Id
-            anyOf:
-              - pattern: '^(api|([0-9a-fA-F]{8}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{12}))\/([0-9a-fA-F]{8}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{12})\/(.+)$'
-                type: string
-              - pattern: '^N:package:[0-9a-fA-F]{8}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{12}$'
-                type: string
+            type: string
           name: file_id
           in: path
       responses:
@@ -2328,11 +2308,7 @@ paths:
         - required: true
           schema:
             title: File Id
-            anyOf:
-              - pattern: '^(api|([0-9a-fA-F]{8}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{12}))\/([0-9a-fA-F]{8}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{12})\/(.+)$'
-                type: string
-              - pattern: '^N:package:[0-9a-fA-F]{8}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{12}$'
-                type: string
+            type: string
           name: file_id
           in: path
         - required: true
@@ -2383,11 +2359,7 @@ paths:
         - required: true
           schema:
             title: File Id
-            anyOf:
-              - pattern: '^(api|([0-9a-fA-F]{8}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{12}))\/([0-9a-fA-F]{8}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{12})\/(.+)$'
-                type: string
-              - pattern: '^N:package:[0-9a-fA-F]{8}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{4}-?[0-9a-fA-F]{12}$'
-                type: string
+            type: string
           name: file_id
           in: path
       responses:

diff --git a/services/web/server/src/simcore_service_webserver/storage/_handlers.py b/services/web/server/src/simcore_service_webserver/storage/_handlers.py
@@ -192,7 +192,7 @@ async def complete_upload_file(request: web.Request) -> web.Response:
 
 
 @login_required
-@permission_required("storages.files.*")
+@permission_required("storage.files.*")
 async def abort_upload_file(request: web.Request) -> web.Response:
     payload, status = await _request_storage(request, "POST")
     return create_data_response(payload, status=status)

diff --git a/services/web/server/src/simcore_service_webserver/storage/_routes.py b/services/web/server/src/simcore_service_webserver/storage/_routes.py
@@ -56,14 +56,14 @@ def create(specs: openapi.Spec) -> list[web.RouteDef]:
     operation_id = specs.paths[path].operations["get"].operation_id
     routes.append(web.get(BASEPATH + path, handle, name=operation_id))
 
+    _FILE_PATH = "/storage/locations/{location_id}/files/{file_id}"
     path, handle = (
-        "/storage/locations/{location_id}/files/{file_id}/metadata",
+        f"{_FILE_PATH}/metadata",
         _handlers.get_file_metadata,
     )
     operation_id = specs.paths[path].operations["get"].operation_id
     routes.append(web.get(BASEPATH + path, handle, name=operation_id))
 
-    _FILE_PATH = "/storage/locations/{location_id}/files/{file_id}"
     path, handle = (
         _FILE_PATH,
         _handlers.download_file,