Skip to content

🐛 Fix webserver storage OpenAPI schema #4426

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jun 27, 2023
Merged

🐛 Fix webserver storage OpenAPI schema #4426

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6aa9640
updated specs
Jun 27, 2023
667fe8c
spec generation
Jun 27, 2023
b1fb3b0
fix issue with permissions
Jun 27, 2023
7de9d54
refactor
Jun 27, 2023
0784607
added regression test
Jun 27, 2023
ebbc7e5
Merge remote-tracking branch 'upstream/master' into pr-osparc-fix-sto…
Jun 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 7 additions & 35 deletions api/specs/webserver/openapi-storage.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand All @@ -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:
Expand All @@ -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:
Expand Down Expand Up @@ -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
Expand Down
24 changes: 16 additions & 8 deletions api/specs/webserver/scripts/openapi_storage.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@


from enum import Enum
from typing import TypeAlias

from fastapi import FastAPI, status
from models_library.api_schemas_storage import (
Expand All @@ -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

Expand All @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand that StorageFileID (which is itself a str) is too restrictive, right?

  • Why do we define this id?
  • If this is not the ID I want for the API, shouldn't you call it AnyFileIDStr instead of StorageFileIDStr?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you look at the tests, the incoming string is very closely related to this StorageFileID type. Though, the one incoming has the character "/" url encoded twice.
So I wanted to give an idea that the type should be something related to the original StorageFileID. Also I did not want to break how everything is working at the moment, since this double encoded slash url is generated by the backend, there must be a reason for it.



@app.get(
"/storage/locations",
response_model=list[DatasetMetaData],
Expand Down Expand Up @@ -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):
...


Expand All @@ -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"""
Expand All @@ -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,
Expand All @@ -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):
...


Expand All @@ -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"""


Expand All @@ -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"""

Expand All @@ -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"""

Expand Down
42 changes: 7 additions & 35 deletions services/web/server/src/simcore_service_webserver/api/v0/openapi.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand All @@ -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:
Expand Down Expand Up @@ -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:
Expand All @@ -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
Expand Down Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion services/web/server/src/simcore_service_webserver/storage/_handlers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions services/web/server/src/simcore_service_webserver/storage/_routes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading