🐛 Fixes on studies dispatcher: file-picker download, extension mapping and study naming (#4390)

Author: pcrespov

packages/postgres-database/src/simcore_postgres_database/models/services_consume_filetypes.py

@@ -30,7 +30,7 @@
         "service_version",
         sa.String,
         nullable=False,
-        doc="Version part of a $key:$version service resource name",
+        doc="Defines the minimum version (included) of this version from which this information applies",
     ),
     sa.Column(
         "service_display_name",

services/static-webserver/client/source/class/osparc/file/FilePicker.js

@@ -237,7 +237,7 @@ qx.Class.define("osparc.file.FilePicker", {
       } else if (osparc.file.FilePicker.isOutputDownloadLink(node.getOutputs())) {
         const outFileValue = osparc.file.FilePicker.getOutput(node.getOutputs());
         if (osparc.utils.Utils.isObject(outFileValue) && "downloadLink" in outFileValue) {
-          osparc.utils.Utils.downloadLink(outFileValue["downloadLink"], "GET", null, progressCb, loadedCb);
+          osparc.utils.Utils.downloadLink(outFileValue["downloadLink"], "GET", outFileValue["label"], progressCb, loadedCb);
         }
       }
     },

services/web/server/src/simcore_service_webserver/projects/_nodes_previews.py

@@ -3,6 +3,7 @@
 import urllib.parse
 
 from aiohttp import web
+from aiohttp.client import ClientError
 from models_library.projects_nodes import Node, NodeID
 from models_library.projects_nodes_io import SimCoreFileLink
 from pydantic import (
@@ -79,9 +80,9 @@ async def fake_screenshots_factory(
                     file_url=file_url,
                 )
             )
-        except (KeyError, ValidationError) as err:
-            _logger.debug(
-                "Failed to create link from file-picker %s: %s",
+        except (KeyError, ValidationError, ClientError) as err:
+            _logger.warning(
+                "Skipping fake node. Unable to create link from file-picker %s: %s",
                 node.json(indent=1),
                 err,
             )

services/web/server/src/simcore_service_webserver/storage/api.py

@@ -3,6 +3,7 @@
 """
 import asyncio
 import logging
+import urllib.parse
 from typing import Any, AsyncGenerator
 
 from aiohttp import ClientError, ClientSession, ClientTimeout, web
@@ -194,9 +195,14 @@ async def get_download_link(
     """
     session, api_endpoint = _get_storage_client(app)
     url = (
-        api_endpoint / f"locations/{filelink.store}/files" / filelink.path
+        api_endpoint
+        / f"locations/{filelink.store}/files"
+        / urllib.parse.quote(filelink.path, safe="")
     ).with_query(user_id=user_id)
+
     async with session.get(f"{url}") as response:
         response.raise_for_status()
-        download = PresignedLink.parse_obj(await response.json())
+        download: PresignedLink = (
+            Envelope[PresignedLink].parse_obj(await response.json()).data
+        )
         return parse_obj_as(HttpUrl, download.link)

services/web/server/src/simcore_service_webserver/studies_dispatcher/_core.py

@@ -3,13 +3,16 @@
 from collections import deque
 from functools import lru_cache
 
+import sqlalchemy as sa
 from aiohttp import web
+from models_library.services import ServiceVersion
 from models_library.utils.pydantic_tools_extension import parse_obj_or_none
-from pydantic import ByteSize, ValidationError
+from pydantic import ByteSize, ValidationError, parse_obj_as
 from servicelib.logging_utils import log_decorator
 from simcore_postgres_database.models.services_consume_filetypes import (
     services_consume_filetypes,
 )
+from sqlalchemy.dialects.postgresql import ARRAY, INTEGER
 
 from .._constants import APP_DB_ENGINE_KEY
 from ._errors import FileToLarge, IncompatibleService
@@ -39,21 +42,20 @@ async def list_viewers_info(
     consumers: deque = deque()
 
     async with app[APP_DB_ENGINE_KEY].acquire() as conn:
-
         # FIXME: ADD CONDITION: service MUST be shared with EVERYBODY!
-        stmt = services_consume_filetypes.select()
+        query = services_consume_filetypes.select()
         if file_type:
-            stmt = stmt.where(services_consume_filetypes.c.filetype == file_type)
+            query = query.where(services_consume_filetypes.c.filetype == file_type)
 
-        stmt = stmt.order_by("filetype", "preference_order")
+        query = query.order_by("filetype", "preference_order")
 
         if file_type and only_default:
-            stmt = stmt.limit(1)
+            query = query.limit(1)
 
-        _logger.debug("Listing viewers:\n%s", stmt)
+        _logger.debug("Listing viewers:\n%s", query)
 
         listed_filetype = set()
-        async for row in await conn.execute(stmt):
+        async for row in await conn.execute(query):
             try:
                 # TODO: filter in database (see test_list_default_compatible_services )
                 if only_default:
@@ -109,20 +111,35 @@ async def validate_requested_viewer(
 
     """
 
+    def _version(column_or_value):
+        # converts version value string to array[integer] that can be compared
+        return sa.func.string_to_array(column_or_value, ".").cast(ARRAY(INTEGER))
+
     if not service_key and not service_version:
         return await get_default_viewer(app, file_type, file_size)
 
     if service_key and service_version:
         async with app[APP_DB_ENGINE_KEY].acquire() as conn:
-            stmt = services_consume_filetypes.select().where(
-                (services_consume_filetypes.c.filetype == file_type)
-                & (services_consume_filetypes.c.service_key == service_key)
-                & (services_consume_filetypes.c.service_version == service_version)
+            query = (
+                services_consume_filetypes.select()
+                .where(
+                    (services_consume_filetypes.c.filetype == file_type)
+                    & (services_consume_filetypes.c.service_key == service_key)
+                    & (
+                        _version(services_consume_filetypes.c.service_version)
+                        <= _version(service_version)
+                    )
+                )
+                .order_by(_version(services_consume_filetypes.c.service_version).desc())
+                .limit(1)
             )
-            result = await conn.execute(stmt)
+
+            result = await conn.execute(query)
             row = await result.first()
             if row:
-                return ViewerInfo.create_from_db(row)
+                view = ViewerInfo.create_from_db(row)
+                view.version = parse_obj_as(ServiceVersion, service_version)
+                return view
 
     raise IncompatibleService(file_type=file_type)
 

services/web/server/src/simcore_service_webserver/studies_dispatcher/_models.py

@@ -70,4 +70,5 @@ class FileParams(BaseModel):
 
     @property
     def footprint(self) -> str:
-        return f"{self.file_name}.{self.file_type}:{self.file_size}"
+        """Identifier used to create UUID"""
+        return f"{self.file_name}:{self.file_type}:{self.file_size}"

services/web/server/src/simcore_service_webserver/studies_dispatcher/_projects.py

@@ -340,8 +340,8 @@ async def get_or_create_project_with_file(
         # project
         project = _create_project(
             project_id=project_uid,
-            name=f"File picker {file_params.file_name}.{file_params.file_type}",
-            description=f"Autogenerated study with a file-picker {file_params.footprint}",
+            name=f"File {file_params.file_name[-20:]}",
+            description=f"Autogenerated study with a file-picker for {file_params.file_name} [{file_params.file_type} file]",
             thumbnail=project_thumbnail,
             owner=user,
             workbench={

services/web/server/src/simcore_service_webserver/studies_dispatcher/_redirects_handlers.py

@@ -28,7 +28,7 @@
     get_or_create_project_with_file_and_service,
     get_or_create_project_with_service,
 )
-from ._users import UserInfo, ensure_authentication, get_or_create_user
+from ._users import UserInfo, ensure_authentication, get_or_create_guest_user
 from .settings import get_plugin_settings
 
 _logger = logging.getLogger(__name__)
@@ -239,8 +239,8 @@ async def get_redirection_to_viewer(request: web.Request):
         )
 
         # Retrieve user or create a temporary guest
-        user = await get_or_create_user(
-            request, allow_anonymous_users=viewer.is_guest_allowed
+        user = await get_or_create_guest_user(
+            request, allow_anonymous_or_guest_users=viewer.is_guest_allowed
         )
 
         # Generate one project per user + download_link + viewer
@@ -269,8 +269,8 @@ async def get_redirection_to_viewer(request: web.Request):
             service_version=service_params_.viewer_version,
         )
 
-        user = await get_or_create_user(
-            request, allow_anonymous_users=valid_service.is_public
+        user = await get_or_create_guest_user(
+            request, allow_anonymous_or_guest_users=valid_service.is_public
         )
 
         project_id, viewer_id = await get_or_create_project_with_service(
@@ -301,7 +301,9 @@ async def get_redirection_to_viewer(request: web.Request):
         # - Anonymous user rights associated with services, not files
         # - Front-end viewer for anonymous users cannot render a single file-picker. SEE https://github.com/ITISFoundation/osparc-simcore/issues/4342
         # - Risk of anonymous users to polute platform with data
-        user = await get_or_create_user(request, allow_anonymous_users=False)
+        user = await get_or_create_guest_user(
+            request, allow_anonymous_or_guest_users=False
+        )
 
         project_id, file_picker_id = await get_or_create_project_with_file(
             request.app,

services/web/server/src/simcore_service_webserver/studies_dispatcher/_users.py

@@ -50,7 +50,7 @@ async def _get_authorized_user(request: web.Request) -> dict:
     return {}
 
 
-async def _create_temporary_user(request: web.Request):
+async def _create_temporary_guest_user(request: web.Request):
     db: AsyncpgStorage = get_plugin_storage(request.app)
     redis_locks_client: aioredis.Redis = get_redis_lock_manager_client(request.app)
     settings: StudiesDispatcherSettings = get_plugin_settings(app=request.app)
@@ -115,15 +115,23 @@ async def _create_temporary_user(request: web.Request):
 
 
 @log_decorator(_logger, level=logging.DEBUG)
-async def get_or_create_user(
-    request: web.Request, *, allow_anonymous_users: bool
+async def get_or_create_guest_user(
+    request: web.Request, *, allow_anonymous_or_guest_users: bool
 ) -> UserInfo:
     """
+    A user w/o authentication is denoted ANONYMOUS. If allow_anonymous_or_guest_users=True, then
+    these users can be automatically promoted to GUEST. For that, a temporary guest account
+    is created and associated to this user.
+
+    GUEST users are therefore a special user that is un-identified to us (no email/name, etc)
+
+    NOTE that if allow_anonymous_or_guest_users=False, GUEST users are NOT allowed in the system either.
+
     Arguments:
-        allow_anonymous_users -- if True, it will create a temporary GUEST account
+        allow_anonymous_or_guest_users -- if True, it will create a temporary GUEST account
 
     Raises:
-        web.HTTPUnauthorized
+        web.HTTPUnauthorized if ANONYMOUS users are not allowed (either w/o auth or as GUEST)
 
     """
     user = None
@@ -134,12 +142,12 @@ async def get_or_create_user(
         # NOTE: covers valid cookie with unauthorized user (e.g. expired guest/banned)
         user = await _get_authorized_user(request)
 
-    if not user and allow_anonymous_users:
+    if not user and allow_anonymous_or_guest_users:
         _logger.debug("Anonymous user is accepted as guest...")
-        user = await _create_temporary_user(request)
+        user = await _create_temporary_guest_user(request)
         is_anonymous_user = True
 
-    if not allow_anonymous_users and (not user or user.get("role") == GUEST):
+    if not allow_anonymous_or_guest_users and (not user or user.get("role") == GUEST):
         # NOTE: if allow_anonymous_users=False then GUEST users are NOT allowed!
         raise web.HTTPUnauthorized(reason=MSG_GUESTS_NOT_ALLOWED)