diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a6c1bcf..85b97ee3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,22 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Added +- **Users in several NDI Cloud organizations** (mirroring VH-Lab/NDI-matlab#1037). + Login no longer keeps only the first organization: it caches the user's + whole list -- id, name and upload permission -- as JSON in + `NDI_CLOUD_ORGANIZATIONS`, and `logout` clears it. New + `ndi.cloud.internal.getOrganizations`, `setOrganizations`, + `normalizeOrganizations`, `chooseOrganization` and `userIsAdmin`, and + `ndi.cloud.ui.dialog.organizationLabels` / `selectOrganization`. + `chooseOrganization` uses a named organization, else the only one that + qualifies, else asks (on a terminal, or through a supplied chooser), else + raises `CloudOrganizationError` listing the organizations by name and id. + `createDataset` and `uploadDataset` take `organization_name` / + `organization_id`; `listDatasets()` with no organization lists every + organization's datasets, tagged with `organizationId` and + `organizationName`. File uploads now go to the organization that owns the + dataset rather than the login's first organization. + - **A double-click installer for end users** in `install/`: `Install NDI.command` (macOS), `Install NDI.cmd` (Windows), and the scripts behind them, `install-ndi.sh` (Linux and macOS) and `install-ndi.ps1` (Windows), which @@ -123,6 +139,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Changed +- **No single NDI Cloud organization any more** (VH-Lab/NDI-matlab#1037). + `CloudConfig.org_id` and `NDI_CLOUD_ORGANIZATION_ID` are gone, and + `ndi.cloud.auth.getActiveToken` returns the token only. The second output of + `ndi.cloud.auth.authenticate` is deprecated: it is the user's only + organization, or `""` when there are several. Code that read + `client.config.org_id` should call `ndi.cloud.internal.chooseOrganization`. + - **`napariViewGEF` and `napariViewLightsheet` are safe to start from MATLAB by themselves.** They now go through `_ndi_launch`, a stdlib-only module beside the package: started with `NDI_FROM_MATLAB=1` (or with MATLAB's diff --git a/src/ndi/cloud/api/datasets.py b/src/ndi/cloud/api/datasets.py index 7c45c655..f70a7270 100644 --- a/src/ndi/cloud/api/datasets.py +++ b/src/ndi/cloud/api/datasets.py @@ -22,20 +22,6 @@ _Client = Annotated[CloudClient | None, SkipValidation()] -def _resolve_org_id(org_id: str | None, client: CloudClient) -> str: - """Return *org_id* if given, otherwise pull it from client config.""" - if org_id: - return org_id - resolved = getattr(client, "config", None) - if resolved and getattr(resolved, "org_id", ""): - return resolved.org_id - raise ValueError( - "org_id is required but was not provided and could not be " - "resolved from the client config. Either pass org_id explicitly " - "or set NDI_CLOUD_ORGANIZATION_ID in the environment." - ) - - @_auto_client @validate_call(config=VALIDATE_CONFIG) def getDataset(dataset_id: CloudId, *, client: _Client = None) -> dict[str, Any]: @@ -49,17 +35,33 @@ def createDataset( name: str = "", description: str = "", *, + organization_name: str = "", client: _Client = None, **kwargs: Any, ) -> dict[str, Any]: """POST /organizations/{organizationId}/datasets - If *org_id* is omitted it is resolved from the client's config - (populated automatically during login), matching MATLAB behaviour. + MATLAB equivalent: ndi.cloud.api.datasets.createDataset + + Creates a new dataset in one of the organizations the current user + belongs to. Name the organization with *org_id* or *organization_name* + (mutually exclusive). If neither is given and the user may upload to + exactly one organization, that one is used; otherwise the user is asked + to choose (when standard input is a terminal) or a + :class:`~ndi.cloud.exceptions.CloudOrganizationError` lists the + organizations. See :func:`ndi.cloud.internal.chooseOrganization`. """ - org_id = _resolve_org_id(org_id, client) if not name: raise ValueError("name is required") + from ..internal import chooseOrganization + + org_id, _ = chooseOrganization( + organization_name, + org_id or "", + require_upload=True, + prompt_string="Create the new dataset in which organization?", + client=client, + ) body: dict[str, Any] = {"name": name} if description: body["description"] = description @@ -112,6 +114,9 @@ def deleteDataset( ) +_MAX_PAGES = 1000 + + @_auto_client def listDatasets( org_id: str | None = None, @@ -122,39 +127,77 @@ def listDatasets( ) -> dict[str, Any]: """GET /organizations/{organizationId}/datasets?page=&pageSize= - If *org_id* is omitted it is resolved from the client's config - (populated automatically during login), matching MATLAB behaviour. - """ - org_id = _resolve_org_id(org_id, client) - return client.get( - "/organizations/{organizationId}/datasets", - params={"page": page, "pageSize": page_size}, - organizationId=org_id, - ) + MATLAB equivalent: ndi.cloud.api.datasets.listDatasets + With *org_id*, retrieves one page of the datasets within that + organization. -_MAX_PAGES = 1000 - - -@_auto_client -def listAllDatasets(org_id: str | None = None, *, client: _Client = None) -> APIResponse: - """Auto-paginate through all datasets for an organisation. - - If *org_id* is omitted it is resolved from the client's config - (populated automatically during login), matching MATLAB behaviour. + Without *org_id*, retrieves ALL datasets in ALL of the organizations the + current user belongs to (:func:`ndi.cloud.internal.getOrganizations`). + NDI Cloud only lists datasets per organization, so this queries each + organization in turn and fetches every page; *page* and *page_size* are + ignored in this mode. Each returned dataset is tagged with the keys + ``organizationId`` and ``organizationName`` of the organization it was + listed under, and the answer's ``page`` is 1 while ``pageSize`` and + ``totalNumber`` are the number of datasets returned. """ - org_id = _resolve_org_id(org_id, client) + if org_id: + return client.get( + "/organizations/{organizationId}/datasets", + params={"page": page, "pageSize": page_size}, + organizationId=org_id, + ) + + from ..internal import getOrganizations + + datasets: list[dict[str, Any]] = [] + for organization in getOrganizations(client=client): + for dataset in _listOrganizationDatasets(organization["id"], client): + tagged = dict(dataset) + tagged.setdefault("organizationId", organization["id"]) + tagged["organizationName"] = organization["name"] + datasets.append(tagged) + answer = { + "totalNumber": len(datasets), + "page": 1, + "pageSize": len(datasets), + "datasets": datasets, + } + return APIResponse(answer, success=True, status_code=200, url="") + + +def _listOrganizationDatasets(org_id: str, client: CloudClient) -> list[dict[str, Any]]: + """Every dataset of one organization, fetching page after page.""" all_datasets: list[dict[str, Any]] = [] page = 1 while page <= _MAX_PAGES: result = listDatasets(org_id, page=page, client=client) - datasets = result.get("datasets", []) + datasets = result.get("datasets", []) or [] all_datasets.extend(datasets) total = result.get("totalNumber", 0) if len(all_datasets) >= total or not datasets: break page += 1 - return APIResponse(all_datasets, success=True, status_code=200, url="") + return all_datasets + + +@_auto_client +def listAllDatasets(org_id: str | None = None, *, client: _Client = None) -> APIResponse: + """Auto-paginate through all datasets for an organisation. + + Without *org_id*, lists every dataset in every organization the user + belongs to, tagged as :func:`listDatasets` does. + """ + if not org_id: + return APIResponse( + listDatasets(client=client).get("datasets", []), + success=True, + status_code=200, + url="", + ) + return APIResponse( + _listOrganizationDatasets(org_id, client), success=True, status_code=200, url="" + ) @_auto_client diff --git a/src/ndi/cloud/api/ndi_matlab_python_bridge.yaml b/src/ndi/cloud/api/ndi_matlab_python_bridge.yaml index c66182ff..91dfe0fa 100644 --- a/src/ndi/cloud/api/ndi_matlab_python_bridge.yaml +++ b/src/ndi/cloud/api/ndi_matlab_python_bridge.yaml @@ -21,11 +21,6 @@ infrastructure: Python-only Pydantic validators (CloudId, NonEmptyStr, PageNumber, PageSize, Scope, FilePath). MATLAB uses arguments blocks for validation. - - name: _resolve_org_id - type: internal_helper - python_path: "ndi/cloud/api/datasets.py" - decision_log: "Python-only helper to resolve org_id from client config." - - name: _coerce_search_structure type: internal_helper python_path: "ndi/cloud/api/documents.py" @@ -71,7 +66,7 @@ functions: - name: createDataset matlab_path: "+ndi/+cloud/+api/+datasets/createDataset.m" - matlab_last_sync_hash: "9b75c0fe" + matlab_last_sync_hash: "293ec411" python_path: "ndi/cloud/api/datasets.py" input_arguments: - name: org_id @@ -86,13 +81,22 @@ functions: type_matlab: "char" type_python: "str" default: "''" + - name: organization_name + type_matlab: "char" + type_python: "str" + default: "''" - name: client type_python: "_Client" default: "None" output_arguments: - name: dataset type_python: "dict[str, Any]" - decision_log: "Exact match." + decision_log: > + 2026-10-18: synchronized with NDI-matlab 293ec411 (VH-Lab/NDI-matlab#1037). The organization is + named with org_id (MATLAB's organizationID) or organization_name, else + chosen by chooseOrganization with RequireUpload -- never taken from the + client config, which no longer holds one. The Python-only + _resolve_org_id helper that read it is gone. - name: updateDataset matlab_path: "+ndi/+cloud/+api/+datasets/updateDataset.m" @@ -134,7 +138,7 @@ functions: - name: listDatasets matlab_path: "+ndi/+cloud/+api/+datasets/listDatasets.m" - matlab_last_sync_hash: "587ebc54" + matlab_last_sync_hash: "293ec411" python_path: "ndi/cloud/api/datasets.py" input_arguments: - name: org_id @@ -155,7 +159,12 @@ functions: output_arguments: - name: result type_python: "dict[str, Any]" - decision_log: "Exact match." + decision_log: > + 2026-10-18: synchronized with NDI-matlab 293ec411 (VH-Lab/NDI-matlab#1037). With org_id, one page + of that organization, as before. Without, every page of every + organization the user belongs to, each dataset tagged with + organizationId and organizationName (NDI Cloud only lists per + organization, so this loops client-side, as in MATLAB). - name: listAllDatasets matlab_path: "N/A" @@ -172,7 +181,8 @@ functions: type_python: "APIResponse" decision_log: > Python-only auto-paginator that wraps listDatasets. - No direct MATLAB equivalent. + No direct MATLAB equivalent. Without org_id it lists every + organization the user belongs to, as listDatasets does. - name: getPublished matlab_path: "+ndi/+cloud/+api/+datasets/getPublished.m" diff --git a/src/ndi/cloud/auth.py b/src/ndi/cloud/auth.py index c5fc8564..e019b391 100644 --- a/src/ndi/cloud/auth.py +++ b/src/ndi/cloud/auth.py @@ -111,7 +111,13 @@ def credentialReport() -> str: lines.append(f"token : {state}, exp {expires:%Y-%m-%d %H:%M UTC}") except CloudAuthError as exc: lines.append(f"token : unreadable ({exc})") - lines.append(f"organization : {config.org_id or '(none)'}") + from .internal import _cachedOrganizations + + organizations = _cachedOrganizations() + lines.append( + "organizations : " + + (", ".join(o["name"] or o["id"] for o in organizations) or "(none cached)") + ) lines.append(f"username in env: {'yes' if os.environ.get('NDI_CLOUD_USERNAME') else 'no'}") email, password = _profile_credentials() if email and password: @@ -123,18 +129,18 @@ def credentialReport() -> str: # the only line that can disagree with the ones above -- a credential # that is present but wrong fails here and nowhere else. try: - _token, org = authenticate(config) + authenticate(config) except CloudAuthError as exc: lines.append(f"authenticate : FAILED -- {exc}") else: - lines.append(f"authenticate : ok (organization {org})") + lines.append("authenticate : ok") return "\n".join(lines) def _noCredentialsMessage(config: CloudConfig) -> str: """Say WHICH step of :func:`authenticate` came up empty. - Three quite different situations end at the same raise, and the reader + Two quite different situations end at the same raise, and the reader of the message is usually looking at a failed file fetch rather than at a login: a token that expired while the process ran, a token that was never set, or credentials that exist but did not resolve. Naming the @@ -155,27 +161,20 @@ def _noCredentialsMessage(config: CloudConfig) -> str: if not config.token: return f"Not logged in to NDI Cloud: no token, no username/password, no profile. {fix}" - if isTokenExpired(config.token): - when = "" - try: - when = f" (it expired at {getTokenExpiration(config.token):%Y-%m-%d %H:%M UTC})" - except CloudAuthError: - when = " (its expiry could not be read, which counts as expired)" - # The token is held in os.environ, so a login in another shell does - # NOT reach a process that is already running -- which is exactly - # the shape of this failure when a viewer has been open a while. - return ( - f"The NDI Cloud token has expired{when}, and there are no " - f"credentials to renew it with. A login in another shell will " - f"not reach this process: log in and start it again. " - f"{fix}" - ) - - # A token that is neither absent nor expired: step 1 wanted an org id - # too, and did not get one. + # A token is present, so it has expired: an unexpired one is accepted at + # step 1 of authenticate and never reaches this message. + try: + when = f" (it expired at {getTokenExpiration(config.token):%Y-%m-%d %H:%M UTC})" + except CloudAuthError: + when = " (its expiry could not be read, which counts as expired)" + # The token is held in os.environ, so a login in another shell does + # NOT reach a process that is already running -- which is exactly + # the shape of this failure when a viewer has been open a while. return ( - "The NDI Cloud token is present and unexpired but carries no " - f"organization id, so it cannot be used. {fix}" + f"The NDI Cloud token has expired{when}, and there are no " + f"credentials to renew it with. A login in another shell will " + f"not reach this process: log in and start it again. " + f"{fix}" ) @@ -270,14 +269,16 @@ def verifyToken(token: str) -> bool: return not isTokenExpired(token) -def getActiveToken(config: CloudConfig | None = None) -> tuple[str, str]: - """Return ``(token, org_id)`` from *config* or environment. +def getActiveToken(config: CloudConfig | None = None) -> str: + """Return the active NDI Cloud token from *config* or environment. + + MATLAB equivalent: ndi.cloud.internal.getActiveToken + + The organizations the user belongs to are not part of the token; see + :func:`ndi.cloud.internal.getOrganizations`. Raises: - CloudAuthError: If no valid token is available, or if the - organization id is missing. Mirrors the MATLAB requirement - that the organization id be populated before the token can - be used for cached auth. + CloudAuthError: If no valid token is available. """ if config is None: config = CloudConfig.from_env() @@ -288,56 +289,7 @@ def getActiveToken(config: CloudConfig | None = None) -> tuple[str, str]: if isTokenExpired(config.token): raise CloudAuthError("Token is expired") - if not config.org_id: - raise CloudAuthError( - "Token is present but NDI_CLOUD_ORGANIZATION_ID is empty; " - "cached auth requires an organization id." - ) - - return config.token, config.org_id - - -# --------------------------------------------------------------------------- -# Organization-id extraction (handles struct / list / dict shapes) -# --------------------------------------------------------------------------- - - -def _extract_first_organization_id(user: dict) -> str: - """Extract the first organization id from a login response's user. - - Mirrors MATLAB ``extractFirstOrganizationId``: accepts dict, list of - dicts, or a single dict; warns when multiple organizations are - present (we pick the first; explicit selection is not yet - implemented). - """ - if not isinstance(user, dict) or "organizations" not in user: - raise CloudAuthError("Login response did not include an organizations field.") - - orgs = user["organizations"] - org_id = "" - n_orgs = 0 - - if isinstance(orgs, dict) and "id" in orgs: - org_id = orgs.get("id", "") - n_orgs = 1 - elif isinstance(orgs, list) and orgs: - first = orgs[0] - if isinstance(first, dict) and "id" in first: - org_id = first.get("id", "") - n_orgs = len(orgs) - - if not org_id: - raise CloudAuthError("Could not extract an organization id from the login response.") - - if n_orgs > 1: - logger.warning( - "Login response contained %d organizations; using the first (%r). " - "Selection among multiple organizations is not yet supported.", - n_orgs, - org_id, - ) - - return str(org_id) + return config.token # --------------------------------------------------------------------------- @@ -360,7 +312,9 @@ def login( config: Optional base config. Defaults to ``CloudConfig.from_env()``. Returns: - Updated :class:`CloudConfig` with token and org_id populated. + Updated :class:`CloudConfig` with its token populated. The + organizations the user belongs to are cached + (:func:`ndi.cloud.internal.setOrganizations`). Raises: CloudAuthError: On failed login. @@ -403,15 +357,16 @@ def login( data = resp.json() token = data.get("token", "") user = data.get("user", {}) or {} - org_id = _extract_first_organization_id(user) - # Store in environment for other code to pick up + # Store in environment for other code to pick up. A user may belong to + # several organizations, so all of them are cached rather than one + # chosen here; see ndi.cloud.internal.chooseOrganization. + from .internal import setOrganizations + os.environ["NDI_CLOUD_TOKEN"] = token - if org_id: - os.environ["NDI_CLOUD_ORGANIZATION_ID"] = org_id + setOrganizations(user.get("organizations") if isinstance(user, dict) else None) config.token = token - config.org_id = org_id return config @@ -447,7 +402,6 @@ def logout(config: CloudConfig | None = None) -> None: _clear_env_tokens() config.token = "" - config.org_id = "" def _profile_credentials() -> tuple[str, str]: @@ -478,7 +432,7 @@ def _profile_credentials() -> tuple[str, str]: def authenticate(config: CloudConfig | None = None) -> tuple[str, str]: - """Return an active token and organization ID, attempting login if needed. + """Return an active token, attempting login if needed. Priority (matching MATLAB ``authenticate.m``): 1. Existing valid token in config/env (local JWT exp pre-check). @@ -490,26 +444,39 @@ def authenticate(config: CloudConfig | None = None) -> tuple[str, str]: Returns: A ``(token, organization_id)`` tuple, matching the two-output - convention of the MATLAB ``authenticate.m`` function. + convention of the MATLAB ``authenticate.m`` function. The second + output is deprecated, as in MATLAB: a user may belong to several + organizations, so there is no single "logged-in" organization. It + is the id of the user's only organization, or ``""`` when the user + belongs to several (or none is cached). New code should call + :func:`ndi.cloud.internal.chooseOrganization` or + :func:`ndi.cloud.internal.getOrganizations`. Raises: CloudAuthError: If authentication fails. + + Python-specific Notes: + MATLAB computes the deprecated second output only when asked for, + and then lets chooseOrganization decide (a dialog when interaction + is enabled). A Python tuple is always fully built, so it never asks + and never makes a network call for it: it reads the cache, and + reports "" rather than guessing among several organizations. """ if config is None: config = CloudConfig.from_env() - # 1. Already have a non-expired token AND an org id? Use it. - # Mirrors MATLAB isAuthenticated() which requires both token and - # organization_id to be present before short-circuiting. - if config.token and config.org_id and not isTokenExpired(config.token): - return config.token, config.org_id + # 1. Already have a non-expired token? Use it. Mirrors MATLAB + # isAuthenticated(), which checks the token only: the organizations + # are a list, cached at login, not part of being logged in. + if config.token and not isTokenExpired(config.token): + return config.token, _soleOrganizationId() # 2. Try env-var credentials email = config.username or os.environ.get("NDI_CLOUD_USERNAME", "") password = config.password or os.environ.get("NDI_CLOUD_PASSWORD", "") if email and password: updated = login(email, password, config) - return updated.token, updated.org_id + return updated.token, _soleOrganizationId() # 3. Fall back to the saved cloud profile. # @@ -530,7 +497,7 @@ def authenticate(config: CloudConfig | None = None) -> tuple[str, str]: email, password = _profile_credentials() if email and password: updated = login(email, password, config) - return updated.token, updated.org_id + return updated.token, _soleOrganizationId() # WHICH OF THE THREE FAILED IS THE WHOLE DIAGNOSIS, and the old message # ran them together. "No valid token and no credentials available" is @@ -880,7 +847,22 @@ def resendConfirmation( # --------------------------------------------------------------------------- +def _soleOrganizationId() -> str: + """The id of the user's only cached organization, else "". No network.""" + from .internal import _cachedOrganizations + + organizations = _cachedOrganizations() + return organizations[0]["id"] if len(organizations) == 1 else "" + + def _clear_env_tokens() -> None: - """Remove cloud-related env vars.""" - for var in ("NDI_CLOUD_TOKEN", "NDI_CLOUD_ORGANIZATION_ID"): - os.environ.pop(var, None) + """Remove the cloud token and the cached organization list. + + ``NDI_CLOUD_ORGANIZATION_ID`` is no longer written; it is cleared too so + a value left by an older version cannot linger. + """ + from .internal import setOrganizations + + os.environ.pop("NDI_CLOUD_TOKEN", None) + os.environ.pop("NDI_CLOUD_ORGANIZATION_ID", None) + setOrganizations(None) diff --git a/src/ndi/cloud/client.py b/src/ndi/cloud/client.py index 98c2c954..ea9cfcbf 100644 --- a/src/ndi/cloud/client.py +++ b/src/ndi/cloud/client.py @@ -503,7 +503,7 @@ def from_env(cls) -> CloudClient: from .auth import authenticate config = CloudConfig.from_env() - config.token, config.org_id = authenticate(config) + config.token, _ = authenticate(config) client = cls(config) # We just obtained this token ourselves, so a mid-run 401/403 # (typical after the token's TTL expires) can be answered by @@ -528,10 +528,8 @@ def _reauthenticate(self) -> None: from .auth import authenticate fresh_config = CloudConfig.from_env() - token, org_id = authenticate(fresh_config) + token, _ = authenticate(fresh_config) self.config.token = token - if org_id: - self.config.org_id = org_id def __repr__(self) -> str: return f"CloudClient(api_url={self.config.api_url!r})" diff --git a/src/ndi/cloud/config.py b/src/ndi/cloud/config.py index 9fed1b83..7f6dd4fa 100644 --- a/src/ndi/cloud/config.py +++ b/src/ndi/cloud/config.py @@ -6,7 +6,8 @@ Environment variables: NDI_CLOUD_TOKEN — JWT bearer token - NDI_CLOUD_ORGANIZATION_ID — Organisation ID + NDI_CLOUD_ORGANIZATIONS — The user's organizations, cached at login as + JSON (see ndi.cloud.internal.getOrganizations) NDI_CLOUD_URL — Full API base URL override CLOUD_API_ENVIRONMENT — 'prod' (default) or 'dev' NDI_CLOUD_UPLOAD_NO_ZIP — 'true' to skip ZIP on upload @@ -57,12 +58,15 @@ class CloudConfig: config = CloudConfig.from_env() # or config = CloudConfig(api_url='https://api.ndi-cloud.com/v1', - token='eyJ...', org_id='org-123') + token='eyJ...') + + There is no organization here: a user may belong to several, so the + list is cached at login and an operation that needs one asks + :func:`ndi.cloud.internal.chooseOrganization`. """ api_url: str = "" token: str = "" - org_id: str = "" upload_no_zip: bool = False username: str = "" password: str = "" @@ -91,7 +95,6 @@ def from_env(cls) -> CloudConfig: return cls( api_url=api_url, token=os.environ.get("NDI_CLOUD_TOKEN", ""), - org_id=os.environ.get("NDI_CLOUD_ORGANIZATION_ID", ""), upload_no_zip=upload_no_zip, username=os.environ.get("NDI_CLOUD_USERNAME", ""), password=os.environ.get("NDI_CLOUD_PASSWORD", ""), @@ -105,6 +108,4 @@ def is_authenticated(self) -> bool: def __repr__(self) -> str: masked = (self.token[:8] + "...") if len(self.token) > 8 else "***" - return ( - f"CloudConfig(api_url={self.api_url!r}, " f"token={masked!r}, org_id={self.org_id!r})" - ) + return f"CloudConfig(api_url={self.api_url!r}, token={masked!r})" diff --git a/src/ndi/cloud/exceptions.py b/src/ndi/cloud/exceptions.py index e53867ff..dc245016 100644 --- a/src/ndi/cloud/exceptions.py +++ b/src/ndi/cloud/exceptions.py @@ -44,3 +44,19 @@ class CloudSyncError(CloudError): class CloudUploadError(CloudError): """Upload operation failure.""" + + +class CloudOrganizationError(CloudError): + """No usable NDI Cloud organization could be decided on. + + Raised by :func:`ndi.cloud.internal.chooseOrganization` and its callers. + + Attributes: + identifier: The MATLAB error identifier for the same failure, e.g. + ``"NDI:Cloud:OrganizationRequired"``, so a caller can tell the + cases apart the way MATLAB code reads ``ME.identifier``. + """ + + def __init__(self, identifier: str, message: str): + super().__init__(message) + self.identifier = identifier diff --git a/src/ndi/cloud/internal.py b/src/ndi/cloud/internal.py index 8bc43897..0c81a1c2 100644 --- a/src/ndi/cloud/internal.py +++ b/src/ndi/cloud/internal.py @@ -7,10 +7,16 @@ from __future__ import annotations +import json import logging +import os +import sys +from collections.abc import Callable from pathlib import Path from typing import TYPE_CHECKING, Any +from .exceptions import CloudOrganizationError + logger = logging.getLogger(__name__) if TYPE_CHECKING: @@ -484,3 +490,405 @@ def formatApiError(response: Any) -> str: if body_part: return body_part return "unknown error" + + +# --------------------------------------------------------------------------- +# Organizations +# +# MATLAB equivalents: +ndi/+cloud/+internal/normalizeOrganizations.m, +# setOrganizations.m, getOrganizations.m, chooseOrganization.m, +# userIsAdmin.m +# +# An NDI Cloud user has one token but may belong to several organizations, +# so there is no single "logged-in organization". The list is cached at +# login; any operation that needs one organization asks chooseOrganization. +# --------------------------------------------------------------------------- + +#: Environment variable holding the cached organization list as JSON. Same +#: name as NDI-matlab uses, alongside NDI_CLOUD_TOKEN. +ORGANIZATIONS_ENV_VAR = "NDI_CLOUD_ORGANIZATIONS" + + +def normalizeOrganizations(raw: Any) -> list[dict[str, Any]]: + """Normalize an NDI Cloud organizations list. + + MATLAB equivalent: ndi.cloud.internal.normalizeOrganizations + + Converts the ``organizations`` field of an NDI Cloud login or + ``/users/me`` response (or a previously normalized list, or its cached + JSON) into a list of dicts with the keys: + + id - (str) the organization id + name - (str) the organization name ('' if none) + canUploadDataset - (bool) whether datasets may be created in it + + RAW may be a list of dicts, a single dict, or empty. Entries without an + id are dropped. An absent canUploadDataset is treated as True so that an + older API response never blocks an upload on the client side; the server + still enforces the permission. + + Raises: + CloudOrganizationError: ``NDI:Cloud:InvalidOrganizations`` if RAW is + neither a list nor a dict. + """ + if not raw: + return [] + if isinstance(raw, dict): + items: list[Any] = [raw] + elif isinstance(raw, (list, tuple)): + items = list(raw) + else: + raise CloudOrganizationError( + "NDI:Cloud:InvalidOrganizations", + "Organizations must be a list of dicts or a dict.", + ) + + organizations: list[dict[str, Any]] = [] + for org in items: + if not isinstance(org, dict): + continue + org_id = org.get("id") + if org_id is None or str(org_id) == "": + continue + name = org.get("name") + can_upload = org.get("canUploadDataset") + organizations.append( + { + "id": str(org_id), + "name": "" if name is None else str(name), + "canUploadDataset": True if can_upload is None else bool(can_upload), + } + ) + return organizations + + +def setOrganizations(organizations: Any = None) -> None: + """Cache the logged-in user's NDI Cloud organizations. + + MATLAB equivalent: ndi.cloud.internal.setOrganizations + + Stores the list of organizations the current NDI Cloud user belongs to, + so callers can offer a choice by name without another round trip to the + server. ORGANIZATIONS is the ``organizations`` field of a login or + ``/users/me`` response, or a list as returned by :func:`getOrganizations`. + + The list is stored as JSON in the ``NDI_CLOUD_ORGANIZATIONS`` environment + variable, alongside ``NDI_CLOUD_TOKEN``, for the life of the process. + + ``setOrganizations(None)`` (or an empty list) clears the cache. This is + what :func:`ndi.cloud.auth.logout` does. + """ + normalized = normalizeOrganizations(organizations) + if normalized: + os.environ[ORGANIZATIONS_ENV_VAR] = json.dumps(normalized) + else: + os.environ.pop(ORGANIZATIONS_ENV_VAR, None) + + +def _cachedOrganizations() -> list[dict[str, Any]]: + """The cached organization list, or [] if absent or unreadable. No network.""" + cached = os.environ.get(ORGANIZATIONS_ENV_VAR, "") + if not cached: + return [] + try: + return normalizeOrganizations(json.loads(cached)) + except (ValueError, CloudOrganizationError): + return [] + + +def _unwrap(answer: Any) -> Any: + """The payload of an API answer, whether or not it is an APIResponse.""" + return getattr(answer, "data", answer) + + +def getOrganizations( + *, + refresh: bool = False, + client: CloudClient | None = None, +) -> list[dict[str, Any]]: + """The organizations the logged-in NDI Cloud user belongs to. + + MATLAB equivalent: ndi.cloud.internal.getOrganizations + + Returns a list of dicts with keys ``id``, ``name`` and + ``canUploadDataset`` (see :func:`normalizeOrganizations`), one per + organization the current user belongs to, in the order the server + returned them. + + The list is read from the cache written at login (:func:`setOrganizations`). + If the cache is empty or unreadable, the list is fetched from + :func:`ndi.cloud.api.users.me` (which authenticates as needed) and cached. + + Args: + refresh: Always fetch a fresh list from the server and update the + cache; use this if the user may have been added to an + organization since login. + client: Authenticated cloud client (auto-created if omitted). + """ + if not refresh: + cached = _cachedOrganizations() + if cached: + return cached + + from .api import users as users_api + + info = _unwrap(users_api.me(client=client)) + raw = info.get("organizations") if isinstance(info, dict) else None + organizations = normalizeOrganizations(raw) + setOrganizations(organizations) + return organizations + + +def _haveActiveToken() -> bool: + """True if this process holds an unexpired NDI Cloud token. No network.""" + from .auth import isTokenExpired + + token = os.environ.get("NDI_CLOUD_TOKEN", "") + return bool(token) and not isTokenExpired(token) + + +def userIsAdmin(*, client: CloudClient | None = None) -> bool: + """Whether the logged-in NDI Cloud user is an administrator. + + MATLAB equivalent: ndi.cloud.internal.userIsAdmin + + Returns True if a user is logged in (there is an active token, or a + CLIENT was given) and :func:`ndi.cloud.api.users.me` reports + ``isAdmin``. Returns False when no one is logged in or the lookup + fails; this never logs in. + + NDI Cloud lets administrators create datasets in any organization they + belong to, regardless of that organization's canUploadDataset flag. + """ + if client is None and not _haveActiveToken(): + return False + try: + from .api import users as users_api + + info = _unwrap(users_api.me(client=client)) + except Exception: # noqa: BLE001 - "could not tell" is not an admin + return False + return isinstance(info, dict) and bool(info.get("isAdmin")) + + +def _stdinIsInteractive() -> bool: + """True when a person can answer a prompt on standard input.""" + try: + return sys.stdin is not None and sys.stdin.isatty() + except (AttributeError, ValueError): + return False + + +def _consoleChoose(labels: list[str], prompt: str) -> int | None: + """Ask on the console for one of LABELS; its 0-based index, or None. + + The list is numbered from 1, as a person counts. An empty answer, or the + end of input, cancels. + """ + print(prompt) + for number, label in enumerate(labels, start=1): + print(f" {number}. {label}") + while True: + try: + answer = input(f"Enter 1-{len(labels)} (empty to cancel): ").strip() + except EOFError: + return None + if not answer: + return None + if answer.isdigit() and 1 <= int(answer) <= len(labels): + return int(answer) - 1 + print(f"Please enter a number from 1 to {len(labels)}.") + + +def _describeOrganization(organization: dict[str, Any]) -> str: + if not organization["name"]: + return f"(no name) ({organization['id']})" + return f'"{organization["name"]}" ({organization["id"]})' + + +def _availableList(organizations: list[dict[str, Any]]) -> str: + return ", ".join(_describeOrganization(o) for o in organizations) + + +def chooseOrganization( + organization_name: str = "", + organization_id: str = "", + *, + require_upload: bool = False, + interactive: bool | None = None, + always_ask: bool = False, + prompt_string: str = "Select an organization:", + chooser: Callable[[list[str], str], int | None] | None = None, + client: CloudClient | None = None, +) -> tuple[str, str]: + """Decide which of the user's NDI Cloud organizations to act on. + + MATLAB equivalent: ndi.cloud.internal.chooseOrganization + + NDI Cloud users may belong to several organizations, so any operation + that needs one (for example, creating a dataset) must decide which. This + function makes that decision from the user's organization list + (:func:`getOrganizations`), in this order: + + 1. If ORGANIZATION_NAME or ORGANIZATION_ID is given, it is matched + against the user's organizations and returned. When logged in, an + unknown value triggers one refresh of the list from the server + before failing, in case the user was added to an organization + after login. + 2. Otherwise, if exactly one organization qualifies, it is returned + (unless ALWAYS_ASK is true). + 3. Otherwise, if INTERACTIVE is true, the user picks one by name. + 4. Otherwise an error is raised that lists the available organizations + by name and id, so the caller can pass one explicitly. + + When REQUIRE_UPLOAD is true, only organizations in which the user may + create datasets qualify: those with canUploadDataset set, or any of the + user's organizations if the user is an NDI Cloud administrator (as on + the server). The administrator check (:func:`userIsAdmin`) is only made + when canUploadDataset would exclude an organization. + + Args: + organization_name: Name of the organization to use. Matched + case-sensitively. Mutually exclusive with ORGANIZATION_ID. + organization_id: Id of the organization to use. + require_upload: Only consider organizations that allow dataset + upload. + interactive: Allow asking the user when a choice is needed. + Default (None): true when standard input is a terminal. + always_ask: Ask even when only one organization qualifies. Ignored + when not interactive. + prompt_string: Prompt shown when asking. + chooser: Called as ``chooser(labels, prompt_string)`` to ask; returns + the 0-based index of the chosen label, or None to cancel. + Default: a numbered prompt on the console. + client: Authenticated cloud client (auto-created if omitted). + + Returns: + Tuple of ``(organization_id, organization_name)``; the name is '' + if the organization has none. + + Raises: + CloudOrganizationError: with the MATLAB identifier of the failure -- + ``NDI:Cloud:OrganizationArguments``, ``NDI:Cloud:NoOrganizations``, + ``NDI:Cloud:UnknownOrganization``, + ``NDI:Cloud:AmbiguousOrganization``, + ``NDI:Cloud:OrganizationCannotUpload``, + ``NDI:Cloud:NoUploadableOrganization``, + ``NDI:Cloud:OrganizationRequired``, or + ``NDI:CloudDialog:UserCanceled`` when the user cancels. + + Python-specific Notes: + MATLAB asks in a dialog when running in the desktop. Python asks on + the console when standard input is a terminal, or through CHOOSER + (the Qt navigator passes its own list dialog). + """ + if interactive is None: + interactive = _stdinIsInteractive() + + if organization_name and organization_id: + raise CloudOrganizationError( + "NDI:Cloud:OrganizationArguments", + "organization_name and organization_id are mutually exclusive.", + ) + + organizations = getOrganizations(client=client) + if not organizations: + raise CloudOrganizationError( + "NDI:Cloud:NoOrganizations", + "Your NDI Cloud account does not belong to any organizations.", + ) + + # 1. An explicit request. + if organization_name or organization_id: + + def find(orgs: list[dict[str, Any]]) -> list[int]: + key, wanted = ( + ("name", organization_name) if organization_name else ("id", organization_id) + ) + return [i for i, o in enumerate(orgs) if o[key] == wanted] + + matches = find(organizations) + if not matches and (client is not None or _haveActiveToken()): + organizations = getOrganizations(refresh=True, client=client) + matches = find(organizations) + if not matches: + requested = ( + f'name "{organization_name}"' if organization_name else f'id "{organization_id}"' + ) + raise CloudOrganizationError( + "NDI:Cloud:UnknownOrganization", + f"Organization {requested} is not one of the organizations you belong " + f"to. Available: {_availableList(organizations)}.", + ) + if len(matches) > 1: + raise CloudOrganizationError( + "NDI:Cloud:AmbiguousOrganization", + f'Organization name "{organization_name}" matches {len(matches)} ' + "organizations you belong to; pass organization_id instead. Matches: " + f"{_availableList([organizations[i] for i in matches])}.", + ) + chosen = organizations[matches[0]] + if require_upload and not chosen["canUploadDataset"] and not userIsAdmin(client=client): + raise CloudOrganizationError( + "NDI:Cloud:OrganizationCannotUpload", + f"Organization {_describeOrganization(chosen)} does not allow dataset upload.", + ) + return chosen["id"], chosen["name"] + + # 2. Only one candidate. + candidates = organizations + if ( + require_upload + and not all(o["canUploadDataset"] for o in organizations) + and not userIsAdmin(client=client) + ): + candidates = [o for o in organizations if o["canUploadDataset"]] + if not candidates: + raise CloudOrganizationError( + "NDI:Cloud:NoUploadableOrganization", + "None of your organizations allows dataset upload. Your organizations: " + f"{_availableList(organizations)}.", + ) + + if len(candidates) == 1 and not (always_ask and interactive): + return candidates[0]["id"], candidates[0]["name"] + + # 3. Ask the user. + if interactive: + from .ui.dialog import organizationLabels + + labels = organizationLabels([c["name"] for c in candidates], [c["id"] for c in candidates]) + index = (chooser or _consoleChoose)(labels, prompt_string) + if index is None or not 0 <= index < len(candidates): + raise CloudOrganizationError( + "NDI:CloudDialog:UserCanceled", + "Operation canceled during selection of an organization.", + ) + return candidates[index]["id"], candidates[index]["name"] + + # 4. No way to decide. + raise CloudOrganizationError( + "NDI:Cloud:OrganizationRequired", + f"You belong to {len(candidates)} organizations; specify one with " + f"organization_name or organization_id. Available: {_availableList(candidates)}.", + ) + + +def _datasetOrganizationId(cloud_dataset_id: str, *, client: CloudClient | None = None) -> str: + """The id of the organization that owns CLOUD_DATASET_ID. + + MATLAB's file-upload calls read this from the dataset record + (``dsetinfo.organizationId`` in GetFileUploadURL / GetFileCollectionUploadURL) + rather than from the logged-in user, because a user in several + organizations has no single organization to fall back on. + """ + from .api import datasets as datasets_api + + info = _unwrap(datasets_api.getDataset(cloud_dataset_id, client=client)) + org_id = info.get("organizationId", "") if isinstance(info, dict) else "" + if not org_id: + raise CloudOrganizationError( + "NDI:Cloud:NoOrganizations", + f"Could not determine the organization of cloud dataset {cloud_dataset_id}.", + ) + return str(org_id) diff --git a/src/ndi/cloud/ndi_matlab_python_bridge.yaml b/src/ndi/cloud/ndi_matlab_python_bridge.yaml index b4b2703b..c35d774d 100644 --- a/src/ndi/cloud/ndi_matlab_python_bridge.yaml +++ b/src/ndi/cloud/ndi_matlab_python_bridge.yaml @@ -90,7 +90,7 @@ functions: # --- auth.py functions --- - name: authenticate matlab_path: "+ndi/+cloud/authenticate.m" - matlab_last_sync_hash: "2566fe4d" + matlab_last_sync_hash: "293ec411" python_path: "ndi/cloud/auth.py" input_arguments: - name: config @@ -103,18 +103,20 @@ functions: - name: organizationID type_python: "str" decision_log: > - Synchronized with MATLAB main as of 2026-05-11 (matlab HEAD - 2566fe4d). Now requires both token AND organization_id to be - populated before treating a cached login as valid (mirrors - commit 5c4352eeac). The local JWT exp pre-check is delegated to - isTokenExpired (extracted in commit 9562eb28e7). Organization-id - extraction (auth._extract_first_organization_id) accepts dict, - list-of-dicts, or single dict, and warns when multiple - organizations are present. + 2026-10-18: synchronized with NDI-matlab 293ec411 (VH-Lab/NDI-matlab#1037). A user may belong + to several organizations, so login no longer picks the first: it caches + the whole list (ndi.cloud.internal.setOrganizations) and a cached login + is valid on the token alone. auth._extract_first_organization_id is + gone. The second output is deprecated on both sides. MATLAB computes it + only when asked for and then lets chooseOrganization decide (a dialog + when interaction is enabled); a Python tuple is always fully built, so + Python never asks and never makes a network call for it -- it returns + the id of the user's only cached organization, or "" when there are + several. No caller in this repo reads it any more. - name: testLogin matlab_path: "+ndi/+cloud/testLogin.m" - matlab_last_sync_hash: "2566fe4d" + matlab_last_sync_hash: "293ec411" python_path: "ndi/cloud/auth.py" input_arguments: - name: user_name @@ -133,6 +135,10 @@ functions: - name: is_good type_python: "bool" decision_log: > + 2026-10-18: NDI-matlab 293ec411 (VH-Lab/NDI-matlab#1037) changed only the Verbose token-state + report, which now prints the cached organization list instead of + NDI_CLOUD_ORGANIZATION_ID. Python's verbose output never printed the + organization, so there is nothing to follow. Ported from MATLAB main as of 2026-05-11 (matlab HEAD 2566fe4d). Three-attempt cascade: (1) probe NDI_CLOUD_TOKEN via direct GET /users/me bypassing wrappers (to avoid silent re-auth swapping @@ -166,14 +172,18 @@ functions: - name: logout matlab_path: "+ndi/+cloud/logout.m" - matlab_last_sync_hash: "23a1f8d8" + matlab_last_sync_hash: "293ec411" python_path: "ndi/cloud/auth.py" input_arguments: - name: config type_python: "CloudConfig | None" default: "None" output_arguments: [] - decision_log: "Exact match." + decision_log: > + 2026-10-18: synchronized with NDI-matlab 293ec411 (VH-Lab/NDI-matlab#1037): clears the token and + the cached organization list (NDI_CLOUD_ORGANIZATIONS) instead of + NDI_CLOUD_ORGANIZATION_ID. Python also clears a leftover + NDI_CLOUD_ORGANIZATION_ID from an older version. - name: logout matlab_path: "+ndi/+cloud/+api/+auth/logout.m" @@ -367,7 +377,7 @@ functions: - name: getActiveToken matlab_path: "+ndi/+cloud/+internal/getActiveToken.m" - matlab_last_sync_hash: "2566fe4d" + matlab_last_sync_hash: "293ec411" python_path: "ndi/cloud/auth.py" input_arguments: - name: config @@ -376,13 +386,11 @@ functions: output_arguments: - name: token type_python: "str" - - name: org_id - type_python: "str" decision_log: > - Updated 2026-05-11 to require a non-empty organization id - (matches MATLAB commit 5c4352eeac which made org id required for - cached auth). MATLAB places this in +internal; Python places it - in auth.py. Name exact match. + 2026-10-18: synchronized with NDI-matlab 293ec411 (VH-Lab/NDI-matlab#1037): returns the token + only. The organizations are a list, not part of the token; see + getOrganizations. MATLAB places this in +internal; Python places it in + auth.py. Name exact match. # --- orchestration.py functions --- - name: downloadDataset @@ -417,7 +425,7 @@ functions: - name: uploadDataset matlab_path: "+ndi/+cloud/uploadDataset.m" - matlab_last_sync_hash: "7180cc8" + matlab_last_sync_hash: "293ec411" python_path: "ndi/cloud/orchestration.py" input_arguments: - name: dataset @@ -447,6 +455,17 @@ functions: type_matlab: "double (via SyncOptions)" type_python: "float | None" default: "None" + - name: organization_name + type_matlab: "char" + type_python: "str" + default: "''" + - name: organization_id + type_matlab: "char" + type_python: "str" + default: "''" + - name: organization_chooser + type_python: "Callable[[list[str], str], int | None] | None" + default: "None" - name: client type_python: "CloudClient | None" default: "None" @@ -458,6 +477,20 @@ functions: - name: message type_python: "str" decision_log: > + 2026-10-18: NDI-matlab 293ec411 (VH-Lab/NDI-matlab#1037): a NEW remote dataset's organization + comes from chooseOrganization (organization_name / organization_id, else + the only uploadable organization, else ask, else an error listing them), + replacing both the first-organization default and the env-var swap + around resolveOrganizationId. Ported, with organization_name and + organization_id (MATLAB's organizationName / organizationID). Python's + file uploads now go to the organization that owns the dataset -- the + chosen one for a new dataset, the dataset record's organizationId for an + existing one -- as MATLAB's GetFileUploadURL reads it; they used the + login's first organization before, which was wrong for an existing + dataset in another organization. organization_chooser is Python-only: + MATLAB asks in a desktop dialog, and this lets the Qt navigator supply + its list dialog (the default asks on a terminal, if there is one). + 2026-10-17: NDI-matlab 7180cc8 (VH-Lab/NDI-matlab#1033) adds SyncOptions.FileBatchSizeLimit, passed through to the file upload. Python's uploadDataset takes no SyncOptions, so file_upload_strategy and @@ -984,7 +1017,7 @@ functions: - name: uploadFilesForDatasetDocuments matlab_path: "+ndi/+cloud/+sync/+internal/uploadFilesForDatasetDocuments.m" - matlab_last_sync_hash: "7180cc8" + matlab_last_sync_hash: "293ec411" python_path: "ndi/cloud/upload.py" input_arguments: - name: org_id @@ -1018,6 +1051,12 @@ functions: - name: result type_python: "dict[str, Any]" decision_log: > + 2026-10-18: NDI-matlab 293ec411 (VH-Lab/NDI-matlab#1037) only rewrote a Verbose error message + (it now names the user's organizations instead of + NDI_CLOUD_ORGANIZATION_ID). Python's org_id may now be "" to look the + organization up from the dataset record, which is what MATLAB's + file-upload calls do; callers pass "" or the dataset's own organization + instead of the login's first. MATLAB places this in +sync/+internal. Python places it in upload.py. Exact name match. 2026-10-17: examined against NDI-matlab 7180cc8 (VH-Lab/NDI-matlab#1033). The batch path is now ported: @@ -1790,21 +1829,16 @@ porting_deferred: with requests.ndi_session which handles auth headers internally. - name: resolveOrganizationId - matlab_path: "+ndi/+cloud/+internal/resolveOrganizationId.m" - matlab_last_sync_hash: "f847d7c4" - status: porting_deferred + matlab_path: "N/A" + matlab_last_sync_hash: "293ec411" + status: retired decision_log: > - MATLAB helper introduced with NDI-matlab#979: given - organizationName or organizationID, resolves against - ndi.cloud.api.users.me and returns the id, verifying membership. - Wired into ndi.cloud.uploadDataset as the 'organizationName' / - 'organizationID' options so a user who belongs to more than one - NDI Cloud organization can route the upload explicitly. Python's - uploadDataset does not currently expose those options, so the - resolver has no Python caller yet -- porting_deferred rather - than matlab_only because Python COULD do this once the upload - surface grows the same options. See NDI-python cloud - orchestration when that lands. + Retired on both sides. NDI-matlab 293ec411 (VH-Lab/NDI-matlab#1037) deleted + +ndi/+cloud/+internal/resolveOrganizationId.m and folded it into + chooseOrganization, which resolves an organization name or id the same + way and also decides when none is given. Python never ported the + resolver; chooseOrganization is ported instead. matlab_path is "N/A" + per the stale-path rule. - name: createCloudMetadataStruct matlab_path: "+ndi/+cloud/+utility/createCloudMetadataStruct.m" @@ -1829,9 +1863,12 @@ porting_deferred: - name: uilogin matlab_path: "+ndi/+cloud/uilogin.m" - matlab_last_sync_hash: "80092b7c" + matlab_last_sync_hash: "293ec411" status: matlab_only decision_log: > + 2026-10-18: NDI-matlab 293ec411 (VH-Lab/NDI-matlab#1037) made uilogin return the token only + and stop requiring an organization id to count as logged in. Still a + MATLAB GUI wrapper with nothing for Python to follow. MATLAB GUI login. No Python equivalent; testLogin() in Python simply returns False when env credentials are absent (since a Python CLI session has no UI). The MATLAB profileEditor GUI is a @@ -1854,7 +1891,7 @@ porting_deferred: - name: selectCloudDataset matlab_path: "+ndi/+cloud/+ui/+dialog/selectCloudDataset.m" - matlab_last_sync_hash: "b7102250" + matlab_last_sync_hash: "293ec411" status: ported_differently python_path: "ndi/gui/nav/datasets_pane.py" decision_log: > @@ -1865,17 +1902,25 @@ porting_deferred: open_cloud_dataset(is_public) is the menu action that uses it. Same public/private split, same endpoints, different entry point. - One difference worth recording: MATLAB's listDatasets() takes no - argument, while Python authenticates first and passes the resulting - organization id. Python also DROPS records whose id cannot be - extracted rather than offering a row that nothing can open. + 2026-10-18: synchronized with NDI-matlab 293ec411 (VH-Lab/NDI-matlab#1037). Private lists the + datasets in every organization the user belongs to (listDatasets with + no organization), and labels are prefixed with "[Org Name]" when there + is more than one -- as in MATLAB, which also fixed its private branch + reading the response wrapper instead of answer.datasets. Python still + DROPS records whose id cannot be extracted rather than offering a row + that nothing can open. - name: LoginDialog matlab_path: "+ndi/+cloud/+ui/+dialog/LoginDialog.mlapp" - matlab_last_sync_hash: "b7102250" + matlab_last_sync_hash: "9a3c972d" status: ported_differently python_path: "ndi/gui/profile_editor.py" decision_log: > + 2026-10-18: NDI-matlab 9a3c972d (VH-Lab/NDI-matlab#1037) changed what the dialog stores + after logging in: the token plus every organization + (setOrganizations), instead of the first organization's id. Python has + no login dialog; its logins go through ndi.cloud.auth.login, which + makes the same change, so profile_editor.py has nothing to follow. MATLAB App Designer credential prompt, the UI behind uilogin (recorded separately as not_applicable). Python collects and stores credentials through ndi.gui.profile_editor.ProfileEditor -- add_profile() and @@ -1884,31 +1929,176 @@ porting_deferred: - name: selectOrganization matlab_path: "+ndi/+cloud/+ui/+dialog/selectOrganization.m" - matlab_last_sync_hash: "56e2669c" - status: porting_deferred + matlab_last_sync_hash: "293ec411" + python_path: "ndi/cloud/ui/dialog.py" + input_arguments: + - name: auto_select_single + type_matlab: "logical" + type_python: "bool" + default: "True" + - name: require_upload + type_matlab: "logical" + type_python: "bool" + default: "False" + - name: prompt_string + type_matlab: "string" + type_python: "str" + default: "'Select an organization:'" + - name: chooser + type_python: "Callable[[list[str], str], int | None] | None" + default: "None" + - name: client + type_python: "CloudClient | None" + default: "None" + output_arguments: + - name: organizationID + type_python: "str" + - name: organizationName + type_python: "str" decision_log: > - The interactive counterpart to the "use the first organization" - behavior that Python already has in - ndi.cloud.auth._extract_first_organization_id. A user who belongs to - more than one organization cannot currently choose which one to act - on: Python takes the first and logs a warning saying selection is - unsupported. That warning is the honest marker of this gap, and this - entry is where the gap is recorded. Porting it means porting - organizationLabels below with it. + Ported 2026-10-18 against NDI-matlab 293ec411 (VH-Lab/NDI-matlab#1037), where it became the + always-interactive form of chooseOrganization (SetActive removed: there + is no active organization any more; RequireUpload added). MATLAB's + Parent (a figure to center the dialog over) has no meaning without a + dialog; Python asks on the console unless a chooser is given, which is + how a Python GUI supplies its own list dialog. - name: organizationLabels matlab_path: "+ndi/+cloud/+ui/+dialog/organizationLabels.m" matlab_last_sync_hash: "56e2669c" - status: porting_deferred + python_path: "ndi/cloud/ui/dialog.py" + input_arguments: + - name: organizationName + type_matlab: "cell" + type_python: "list" + - name: organizationID + type_matlab: "cell" + type_python: "list" + output_arguments: + - name: labels + type_python: "list[str]" decision_log: > - Pure helper for selectOrganization: turns parallel name/id lists into - unique display labels, falling back to the id when a name is empty and - appending the id in parentheses when two organizations share a name. - No UI and no MATLAB-only dependency, so it ports directly and is - exactly testable -- but it has no caller in Python until the picker - above exists, which is why it is deliberately deferred rather than - ported on its own. + Ported 2026-10-18 with selectOrganization / chooseOrganization, its + callers (VH-Lab/NDI-matlab#1037). Pure helper: the name, or the id when + the name is empty, with the id appended in parentheses when two + organizations share a label. Argument names kept as in MATLAB. + + # --- organizations (+internal), ported with VH-Lab/NDI-matlab#1037 --- + + - name: normalizeOrganizations + matlab_path: "+ndi/+cloud/+internal/normalizeOrganizations.m" + matlab_last_sync_hash: "293ec411" + python_path: "ndi/cloud/internal.py" + input_arguments: + - name: raw + type_matlab: "struct array | cell" + type_python: "Any" + output_arguments: + - name: organizations + type_matlab: "1xN struct (id, name, canUploadDataset)" + type_python: "list[dict[str, Any]]" + decision_log: > + MATLAB returns a struct array; Python a list of dicts with the same + three keys. Raises CloudOrganizationError with MATLAB's identifier + (NDI:Cloud:InvalidOrganizations). + + - name: setOrganizations + matlab_path: "+ndi/+cloud/+internal/setOrganizations.m" + matlab_last_sync_hash: "293ec411" + python_path: "ndi/cloud/internal.py" + input_arguments: + - name: organizations + type_python: "Any" + default: "None" + output_arguments: [] + decision_log: > + Same cache, same environment variable (NDI_CLOUD_ORGANIZATIONS, JSON). + Clearing pops the variable rather than setting it to ''. + + - name: getOrganizations + matlab_path: "+ndi/+cloud/+internal/getOrganizations.m" + matlab_last_sync_hash: "293ec411" + python_path: "ndi/cloud/internal.py" + input_arguments: + - name: refresh + type_matlab: "logical" + type_python: "bool" + default: "False" + - name: client + type_python: "CloudClient | None" + default: "None" + output_arguments: + - name: organizations + type_python: "list[dict[str, Any]]" + decision_log: > + MATLAB's Refresh option is refresh. Falls back to users.me when the + cache is empty, as in MATLAB. + + - name: chooseOrganization + matlab_path: "+ndi/+cloud/+internal/chooseOrganization.m" + matlab_last_sync_hash: "c24d338d" + python_path: "ndi/cloud/internal.py" + input_arguments: + - name: organization_name + type_matlab: "char" + type_python: "str" + default: "''" + - name: organization_id + type_matlab: "char" + type_python: "str" + default: "''" + - name: require_upload + type_matlab: "logical" + type_python: "bool" + default: "False" + - name: interactive + type_matlab: "logical (default: usejava('desktop'))" + type_python: "bool | None" + default: "None" + - name: always_ask + type_matlab: "logical" + type_python: "bool" + default: "False" + - name: prompt_string + type_matlab: "string" + type_python: "str" + default: "'Select an organization:'" + - name: chooser + type_python: "Callable[[list[str], str], int | None] | None" + default: "None" + - name: client + type_python: "CloudClient | None" + default: "None" + output_arguments: + - name: organizationID + type_python: "str" + - name: organizationName + type_python: "str" + decision_log: > + Same decision order and the same errors, raised as + CloudOrganizationError carrying MATLAB's identifier. Includes NDI-matlab + c24d338d: an administrator may upload to any of their organizations, + checked only when canUploadDataset would exclude one. Interactive + defaults to "standard input is a terminal" where MATLAB uses "running in + the desktop"; Python asks on the console (counting from 1) or through + chooser, where MATLAB opens ndi.util.ListDialog. MATLAB's Parent + option is not ported (no dialog). + + - name: userIsAdmin + matlab_path: "+ndi/+cloud/+internal/userIsAdmin.m" + matlab_last_sync_hash: "c24d338d" + python_path: "ndi/cloud/internal.py" + input_arguments: + - name: client + type_python: "CloudClient | None" + default: "None" + output_arguments: + - name: tf + type_python: "bool" + decision_log: > + False when not logged in or the lookup fails, never prompting, as in + MATLAB. A given client counts as logged in. - name: DatasetUploadMonitor matlab_path: "+ndi/+cloud/+ui/DatasetUploadMonitor.mlapp" matlab_last_sync_hash: "b7102250" diff --git a/src/ndi/cloud/orchestration.py b/src/ndi/cloud/orchestration.py index d6d93d00..d49149d4 100644 --- a/src/ndi/cloud/orchestration.py +++ b/src/ndi/cloud/orchestration.py @@ -13,6 +13,7 @@ import logging import re import warnings +from collections.abc import Callable from pathlib import Path from typing import TYPE_CHECKING, Any, NamedTuple @@ -436,6 +437,9 @@ def uploadDataset( *, file_upload_strategy: str = "batch", file_batch_size_limit: float | None = None, + organization_name: str = "", + organization_id: str = "", + organization_chooser: Callable[[list[str], str], int | None] | None = None, client: CloudClient | None = None, ) -> tuple[bool, str, str]: """Upload a local dataset to NDI Cloud. @@ -454,15 +458,44 @@ def uploadDataset( file_batch_size_limit: Maximum size in bytes of each zip batch; ``None`` uses the preference ``Cloud.Upload.Max_File_Batch_Size``. MATLAB: SyncOptions ``FileBatchSizeLimit``. + organization_name: The NAME of the NDI Cloud organization a NEW + remote dataset should be created under. The current user must + belong to it and be allowed to upload to it. Names are matched + case-sensitively. Mutually exclusive with *organization_id*. + When neither is given and the user can upload to exactly one + organization, that one is used; otherwise the user is asked to + choose (see *organization_chooser*) or the upload fails with a + message listing the organizations. See + :func:`ndi.cloud.internal.chooseOrganization`. Ignored when the + dataset already exists on NDI Cloud. + organization_id: The ID of the NDI Cloud organization to use. + Mutually exclusive with *organization_name*. + organization_chooser: How to ask the user, when an organization must + be chosen: called as ``chooser(labels, prompt)`` and returning the + 0-based index of the chosen label, or None to cancel. Default: + a numbered console prompt when standard input is a terminal, + otherwise no asking. client: Authenticated cloud client (auto-created if omitted). Returns: Tuple of ``(success, cloud_dataset_id, message)``. + + Python-specific Notes: + MATLAB asks in a dialog in the desktop. *organization_chooser* lets + a Python GUI (the Qt navigator) supply its own list dialog instead. """ from .api import datasets as ds_api - from .internal import createRemoteDatasetDoc, getCloudDatasetIdForLocalDataset + from .exceptions import CloudError + from .internal import ( + chooseOrganization, + createRemoteDatasetDoc, + getCloudDatasetIdForLocalDataset, + ) from .upload import uploadDocumentCollection, uploadFilesForDatasetDocuments + if organization_name and organization_id: + return False, "", "organization_name and organization_id are mutually exclusive." + # MATLAB refuses to upload a dataset that is not fully ingested, and it is # the FIRST thing uploadDataset.m does (line 53). This had no counterpart # here for a simpler reason than oversight: ndi_dataset had no isIngested() @@ -481,15 +514,32 @@ def uploadDataset( "Dataset is not fully ingested. All sessions must be ingested " "before uploading.", ) - # Resolve or create remote dataset + # Resolve or create remote dataset. org_id stays "" for an existing one: + # its files go to the organization that owns it, which the file upload + # reads from the dataset record when it needs it. cloud_id = "" + org_id = "" if not upload_as_new: cloud_id, _ = getCloudDatasetIdForLocalDataset(dataset, client=client) if not cloud_id: + # Decide which organization the new dataset belongs to. A user may + # belong to several, so this is asked rather than assumed. + try: + org_id, _ = chooseOrganization( + organization_name, + organization_id, + require_upload=True, + interactive=True if organization_chooser else None, + prompt_string="Upload the dataset to which organization?", + chooser=organization_chooser, + client=client, + ) + except CloudError as exc: + return False, "", str(exc) + # Create new remote dataset name = remote_name or getattr(dataset, "name", "Unnamed ndi_dataset") - org_id = client.config.org_id try: result = ds_api.createDataset(org_id, name, client=client) cloud_id = result.get("id", result.get("_id", "")) @@ -544,7 +594,7 @@ def uploadDataset( # Upload files if sync_files: file_report = uploadFilesForDatasetDocuments( - client.config.org_id, + org_id, cloud_id, doc_jsons, dataset=dataset, diff --git a/src/ndi/cloud/sync/operations.py b/src/ndi/cloud/sync/operations.py index 9e18fc8d..ec4015a3 100644 --- a/src/ndi/cloud/sync/operations.py +++ b/src/ndi/cloud/sync/operations.py @@ -355,7 +355,7 @@ def _upload_binaries( try: report = uploadFilesForDatasetDocuments( - getattr(getattr(client, "config", None), "org_id", ""), + "", # looked up from the dataset record cloud_dataset_id, documents, dataset=dataset, diff --git a/src/ndi/cloud/ui/__init__.py b/src/ndi/cloud/ui/__init__.py new file mode 100644 index 00000000..db094132 --- /dev/null +++ b/src/ndi/cloud/ui/__init__.py @@ -0,0 +1,5 @@ +""" +ndi.cloud.ui - User-interaction helpers for NDI Cloud. + +MATLAB equivalent: +ndi/+cloud/+ui/ +""" diff --git a/src/ndi/cloud/ui/dialog.py b/src/ndi/cloud/ui/dialog.py new file mode 100644 index 00000000..ff50e04c --- /dev/null +++ b/src/ndi/cloud/ui/dialog.py @@ -0,0 +1,105 @@ +""" +ndi.cloud.ui.dialog - Choosing among NDI Cloud items by name. + +MATLAB equivalents: +ndi/+cloud/+ui/+dialog/organizationLabels.m, + +ndi/+cloud/+ui/+dialog/selectOrganization.m +""" + +from __future__ import annotations + +from collections.abc import Callable +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from ..client import CloudClient + + +def organizationLabels(organizationName: list, organizationID: list) -> list[str]: + """Build unique display labels for a list of organizations. + + MATLAB equivalent: ndi.cloud.ui.dialog.organizationLabels + + Given parallel lists of organization names and ids, returns a list of + display labels suitable for a name-based picker. + + The label for each organization is its name. Two rules keep the labels + usable as unique keys in a list dialog: + + * An organization with no name falls back to its id. + * When a name is shared by more than one organization (or a name + collides with another organization's id/label), the organization id + is appended in parentheses so every label is distinct. + + Args: + organizationName: Organization names, parallel to ORGANIZATIONID. May + be shorter than ORGANIZATIONID, in which case the missing names + are treated as empty. + organizationID: Organization ids. + + Returns: + One label per organization, in the input order, all distinct. + """ + ids = [str(i) for i in organizationID] + names = [ + str(organizationName[i]) if i < len(organizationName) and organizationName[i] else "" + for i in range(len(ids)) + ] + base = [name or ident for name, ident in zip(names, ids, strict=True)] + return [ + f"{label} ({ident})" if base.count(label) > 1 and ident else label + for label, ident in zip(base, ids, strict=True) + ] + + +def selectOrganization( + *, + auto_select_single: bool = True, + require_upload: bool = False, + prompt_string: str = "Select an organization:", + chooser: Callable[[list[str], str], int | None] | None = None, + client: CloudClient | None = None, +) -> tuple[str, str]: + """Choose one of the current user's NDI Cloud organizations by name. + + MATLAB equivalent: ndi.cloud.ui.dialog.selectOrganization + + Lists, by name, the organizations that the currently authenticated NDI + Cloud user belongs to, and returns the chosen organization's id and name. + The list comes from :func:`ndi.cloud.internal.getOrganizations` (cached at + login). + + This is the always-interactive form of + :func:`ndi.cloud.internal.chooseOrganization`. + + Args: + auto_select_single: When True (default) and only one organization + qualifies, it is returned without asking. False always asks. + require_upload: Only list organizations in which the user may create + datasets. + prompt_string: Prompt shown above the list. + chooser: Called as ``chooser(labels, prompt_string)``; returns the + 0-based index of the chosen label, or None to cancel. Default: a + numbered prompt on the console. + client: Authenticated cloud client (auto-created if omitted). + + Returns: + Tuple of ``(organization_id, organization_name)``. + + Raises: + CloudOrganizationError: ``NDI:CloudDialog:UserCanceled`` if the user + cancels, or any error of :func:`~ndi.cloud.internal.chooseOrganization`. + + Python-specific Notes: + MATLAB opens a modal dialog. Python asks on the console unless a + CHOOSER is given (the Qt navigator passes its list dialog). + """ + from ..internal import chooseOrganization + + return chooseOrganization( + interactive=True, + always_ask=not auto_select_single, + require_upload=require_upload, + prompt_string=prompt_string, + chooser=chooser, + client=client, + ) diff --git a/src/ndi/cloud/upload.py b/src/ndi/cloud/upload.py index 67927bde..b13c4eff 100644 --- a/src/ndi/cloud/upload.py +++ b/src/ndi/cloud/upload.py @@ -162,7 +162,8 @@ def zipForUpload( file_manifest: Entries with ``file_uid``, ``file_path``, and optionally ``document_id`` and ``bytes`` (looked up on disk when absent). dataset_id: Cloud dataset ID. - org_id: Organisation ID. Defaults to ``client.config.org_id``. + org_id: Organisation ID. Defaults to the organization that owns + *dataset_id*, read from the dataset record. size_limit: Maximum batch size in bytes. ``None`` (default) reads the preference ``Cloud.Upload.Max_File_Batch_Size``. number_retries: Attempts at getting the upload URL and at the PUT. @@ -188,7 +189,6 @@ def zipForUpload( size_limit = float(preferences.get("Cloud.Upload.Max_File_Batch_Size")) if size_limit <= 0: raise ValueError(f"size_limit must be positive (got {size_limit!r})") - org_id = org_id or getattr(getattr(client, "config", None), "org_id", "") report: dict[str, Any] = { "uploaded": 0, @@ -197,6 +197,10 @@ def zipForUpload( "failed_document_ids": [], "batches": 0, } + if file_manifest and not org_id: + from .internal import _datasetOrganizationId + + org_id = _datasetOrganizationId(dataset_id, client=client) def _fail(batch: list[dict[str, Any]], message: str) -> None: report["failed"] += len(batch) @@ -332,7 +336,9 @@ def uploadFilesForDatasetDocuments( """Upload associated binary files for a list of documents. Args: - org_id: Organisation ID. + org_id: Organisation ID -- the organization that owns *dataset_id*. + ``""`` looks it up from the dataset record, as MATLAB's + file-upload calls do. dataset_id: Cloud dataset ID. documents: List of document property dicts. dataset: The local ``ndi.dataset`` that produced *documents*. When @@ -408,6 +414,10 @@ def uploadFilesForDatasetDocuments( doc_id = document_id(doc) for file_uid, file_path in file_uploads_for_document(doc, dataset=dataset): try: + if not org_id: + from .internal import _datasetOrganizationId + + org_id = _datasetOrganizationId(dataset_id, client=client) url = files_api.getFileUploadURL(org_id, dataset_id, file_uid, client=client) files_api.putFiles(url, file_path) report["uploaded"] += 1 @@ -610,8 +620,12 @@ def uploadSingleFile( import uuid from .api import files as files_api + from .internal import _datasetOrganizationId try: + # The dataset's own organization, as MATLAB's GetFileUploadURL reads + # it: a user in several organizations has no single one to assume. + org_id = _datasetOrganizationId(dataset_id, client=client) if use_bulk_upload: zip_name = f"{dataset_id}.{uuid.uuid4().hex}.zip" zip_path = Path(tempfile.gettempdir()) / zip_name @@ -619,7 +633,7 @@ def uploadSingleFile( with zipfile.ZipFile(zip_path, "w", zipfile.ZIP_DEFLATED) as zf: zf.write(file_path, os.path.basename(file_path)) info = files_api.getFileCollectionUploadURL( - client.config.org_id, + org_id, dataset_id, client=client, ) @@ -636,7 +650,7 @@ def uploadSingleFile( checksum_crc32 = crc32(file_path) url = files_api.getFileUploadURL( - client.config.org_id, + org_id, dataset_id, file_uid, checksum_crc32=checksum_crc32, diff --git a/src/ndi/gui/nav/datasets_cloud.py b/src/ndi/gui/nav/datasets_cloud.py index b8689ac1..cd6abbaa 100644 --- a/src/ndi/gui/nav/datasets_cloud.py +++ b/src/ndi/gui/nav/datasets_cloud.py @@ -37,6 +37,7 @@ from __future__ import annotations +from collections.abc import Callable from typing import Any, NamedTuple from .datasets_text import cloud_check_message, sync_result_message @@ -173,19 +174,30 @@ def check_cloud_status(dataset: Any) -> CloudActionResult: ) -def upload_dataset(dataset: Any) -> CloudActionResult: +def upload_dataset( + dataset: Any, + organization_chooser: Callable[[list[str], str], int | None] | None = None, +) -> CloudActionResult: """Upload DATASET's documents and files to NDI Cloud. Creates the remote dataset on a first upload and otherwise sends whatever is missing. A successful upload is what LINKS the dataset to the cloud, so the new state is known without another query -- which is why success reports ``"incloud"`` directly. + + A first upload must decide which of the user's organizations the new + dataset belongs to. ORGANIZATION_CHOOSER is how the user is asked when + they belong to more than one (see + :func:`ndi.cloud.orchestration.uploadDataset`); the navigator passes its + list dialog, as MATLAB's navigator shows ndi.util.ListDialog. """ title = "Upload to Cloud" from ...cloud.orchestration import uploadDataset try: - success, _cloud_id, message = uploadDataset(dataset, verbose=False) + success, _cloud_id, message = uploadDataset( + dataset, verbose=False, organization_chooser=organization_chooser + ) except Exception as exc: # noqa: BLE001 return CloudActionResult(False, title, str(exc), "error", None) diff --git a/src/ndi/gui/nav/datasets_pane.py b/src/ndi/gui/nav/datasets_pane.py index 2c78adfc..1e6b0e64 100644 --- a/src/ndi/gui/nav/datasets_pane.py +++ b/src/ndi/gui/nav/datasets_pane.py @@ -116,8 +116,9 @@ def _launcher(app_class: str): def fetch_cloud_datasets(is_public: bool) -> tuple[list[str], list[str]]: """The NDI Cloud dataset labels and their ids. - Public reads the published catalogue; private authenticates first to get - the organization id, then lists that organization's datasets. + Public reads the published catalogue; private lists the datasets in + every organization the user belongs to, and prefixes each label with its + organization's name when the user belongs to more than one. Entries whose id could not be extracted are DROPPED rather than listed with an empty id: a row the user can select but nothing can open is worse @@ -129,18 +130,22 @@ def fetch_cloud_datasets(is_public: bool) -> tuple[list[str], list[str]]: if is_public: answer = datasets_api.getPublished() else: - from ...cloud.auth import authenticate + answer = datasets_api.listDatasets() - _, org_id = authenticate() - answer = datasets_api.listDatasets(org_id) + records = normalize_cloud_list(_unwrap(answer)) + org_names = [ + str(record.get("organizationName") or "") if isinstance(record, dict) else "" + for record in records + ] + show_org = not is_public and len(set(org_names)) > 1 labels: list[str] = [] ids: list[str] = [] - for record in normalize_cloud_list(_unwrap(answer)): + for record, org_name in zip(records, org_names, strict=True): identifier, label = cloud_dataset_id_label(record) if identifier: ids.append(identifier) - labels.append(label) + labels.append(f"[{org_name}] {label}" if show_org and org_name else label) return labels, ids @@ -727,7 +732,12 @@ def _run_cloud(self, node: Any, action: str, dataset: Any) -> Any: if action == "check_status": result = datasets_cloud.check_cloud_status(dataset) elif action == "upload": - result = datasets_cloud.upload_dataset(dataset) + result = datasets_cloud.upload_dataset( + dataset, + organization_chooser=lambda labels, prompt: self._ask_choice( + labels, prompt, "Select organization" + ), + ) elif action == "check_new_remote": result = datasets_cloud.check_for_new(dataset, "remote") else: diff --git a/src/ndi/gui/ndi_matlab_python_bridge.yaml b/src/ndi/gui/ndi_matlab_python_bridge.yaml index 427b39b4..ea18e2d8 100644 --- a/src/ndi/gui/ndi_matlab_python_bridge.yaml +++ b/src/ndi/gui/ndi_matlab_python_bridge.yaml @@ -754,19 +754,28 @@ ported_late: - name: datasetsPane matlab_path: "+ndi/+gui/+nav/datasetsPane.m" - matlab_last_sync_hash: "958ff7c5" + matlab_last_sync_hash: "293ec411" python_path: "ndi/gui/nav/datasets_pane.py" decision_log: > The navigator's elastic pane, ported across #103, #109, #110 and #111 in four layers (pure, model, cloud actions, widgets) which is why Python spreads it over datasets_pane.py, datasets_model.py, datasets_cloud.py and datasets_text.py rather than one module. + 2026-10-18: synchronized with NDI-matlab 293ec411 (VH-Lab/NDI-matlab#1037): the private cloud + list covers every organization the user belongs to, labelled + "[Org Name]" when there is more than one. Python's "Upload to Cloud" + also passes its list dialog to uploadDataset, so a user in several + organizations is asked which one, as MATLAB's navigator asks. - name: cloudPane matlab_path: "+ndi/+gui/+nav/cloudPane.m" - matlab_last_sync_hash: "92d7d903" + matlab_last_sync_hash: "293ec411" python_path: "ndi/gui/nav/cloud_pane.py" - decision_log: "Cloud status and actions pane. Ported in #112." + decision_log: > + Cloud status and actions pane. Ported in #112. NDI-matlab 293ec411 (VH-Lab/NDI-matlab#1037) + changed only a comment in refreshLogin (logout now clears the cached + organization list, not NDI_CLOUD_ORGANIZATION_ID); Python's docstring + already says "the locally stored credentials", so nothing to follow. - name: progressPane matlab_path: "+ndi/+gui/+nav/progressPane.m" diff --git a/tests/_cloud_organizations.py b/tests/_cloud_organizations.py new file mode 100644 index 00000000..1be6a177 --- /dev/null +++ b/tests/_cloud_organizations.py @@ -0,0 +1,62 @@ +"""Shared test setup for code that needs the user's NDI Cloud organizations. + +A user may belong to several organizations, so nothing holds "the" +organization: the list is cached at login (``NDI_CLOUD_ORGANIZATIONS``), an +operation that creates a dataset asks +:func:`ndi.cloud.internal.chooseOrganization`, and file uploads use the +organization that owns the dataset, read from its record. Offline tests set +those up here instead of a client config. +""" + +from __future__ import annotations + +import json + +import ndi.cloud.internal + + +def use_organizations(monkeypatch, *organizations, dataset_org_id=None): + """Cache ORGANIZATIONS as the logged-in user's, with no token to refresh with. + + Each organization is a dict with ``id`` and optionally ``name`` and + ``canUploadDataset``. Clearing the token keeps chooseOrganization and + userIsAdmin off the network. File uploads are told every dataset belongs + to DATASET_ORG_ID (default: the first organization's id). + """ + monkeypatch.delenv("NDI_CLOUD_TOKEN", raising=False) + monkeypatch.setenv("NDI_CLOUD_ORGANIZATIONS", json.dumps(list(organizations))) + owner = dataset_org_id if dataset_org_id is not None else organizations[0]["id"] + monkeypatch.setattr( + ndi.cloud.internal, + "_datasetOrganizationId", + lambda dataset_id, client=None: owner, + ) + + +def testOrganizationID(client=None) -> str: + """The NDI Cloud organization live tests create datasets in. + + MATLAB equivalent: ndi.unittest.cloud.testOrganizationID + + The id of the first organization the test account belongs to that allows + dataset upload (any of its organizations, if the account is an NDI Cloud + administrator, as the server allows). Test accounts may belong to several + organizations, and a test cannot answer the organization prompt, so + tests pass this id explicitly. + """ + from ndi.cloud.internal import getOrganizations, userIsAdmin + + organizations = getOrganizations(client=client) + uploadable = organizations + if not userIsAdmin(client=client): + uploadable = [o for o in organizations if o["canUploadDataset"]] + if not uploadable: + raise RuntimeError( + "The NDI Cloud test account does not belong to any organization that " + "allows dataset upload." + ) + return uploadable[0]["id"] + + +# Collected by name otherwise: it is a helper, not a test. +testOrganizationID.__test__ = False diff --git a/tests/manual_cloud_download_check.py b/tests/manual_cloud_download_check.py index 6dd01744..a63e64ca 100644 --- a/tests/manual_cloud_download_check.py +++ b/tests/manual_cloud_download_check.py @@ -101,7 +101,10 @@ def run_test(username: str, password: str) -> dict: config = login(username, password) client = CloudClient(config) print(f" Authenticated as: {username}") - print(f" Org ID: {config.org_id}") + from ndi.cloud.internal import _cachedOrganizations + + names = ", ".join(o["name"] or o["id"] for o in _cachedOrganizations()) + print(f" Organizations: {names or '(none)'}") results["auth"] = True except Exception as exc: print(f" FAILED to authenticate: {exc}") diff --git a/tests/test_cloud_api_datasets.py b/tests/test_cloud_api_datasets.py index 89dcc759..416bda13 100644 --- a/tests/test_cloud_api_datasets.py +++ b/tests/test_cloud_api_datasets.py @@ -1,16 +1,40 @@ -"""Unit tests for ndi.cloud.api.datasets — no network required.""" +"""Unit tests for ndi.cloud.api.datasets — no network required. + +A user may belong to several NDI Cloud organizations, so there is no +organization on the client: the user's organizations are cached at login +(``NDI_CLOUD_ORGANIZATIONS``) and each call names one or lets +:func:`ndi.cloud.internal.chooseOrganization` decide. The cache is set +directly here, and the token cleared, so nothing reaches the network. +""" from __future__ import annotations +import json from unittest.mock import MagicMock import pytest +from ndi.cloud.exceptions import CloudOrganizationError + +ALPHA = {"id": "org-abc", "name": "Alpha Lab", "canUploadDataset": True} +BETA = {"id": "org-def", "name": "Beta Institute", "canUploadDataset": True} +ARCHIVE = {"id": "org-ghi", "name": "Archive", "canUploadDataset": False} + + +@pytest.fixture +def organizations(monkeypatch): + """Set the cached organization list; no token, so never a refresh or admin lookup.""" + monkeypatch.delenv("NDI_CLOUD_TOKEN", raising=False) + + def _set(*orgs): + monkeypatch.setenv("NDI_CLOUD_ORGANIZATIONS", json.dumps(list(orgs))) + + return _set + -def _make_client(org_id: str = "org-123") -> MagicMock: - """Return a mock CloudClient with a config carrying *org_id*.""" +def _make_client() -> MagicMock: + """Return a mock CloudClient whose list call answers one dataset.""" client = MagicMock() - client.config.org_id = org_id client.config.api_url = "https://api.ndi-cloud.com/v1" client.get.return_value = { "datasets": [{"id": "ds-1", "name": "Test"}], @@ -20,67 +44,117 @@ def _make_client(org_id: str = "org-123") -> MagicMock: class TestListDatasets: - """listDatasets should work with or without an explicit org_id.""" + """With an organization, one page of it; without, every organization.""" - def test_without_org_id_uses_client_config(self): - """Calling listDatasets() with no org_id should resolve it from client config.""" + def test_with_explicit_org_id(self, organizations): from ndi.cloud.api.datasets import listDatasets - client = _make_client(org_id="org-abc") - result = listDatasets(client=client) + organizations(ALPHA, BETA) + client = _make_client() + result = listDatasets("org-explicit", client=client) client.get.assert_called_once() - call_kwargs = client.get.call_args - assert call_kwargs.kwargs["organizationId"] == "org-abc" + assert client.get.call_args.kwargs["organizationId"] == "org-explicit" assert result["datasets"][0]["name"] == "Test" - def test_with_explicit_org_id(self): - """Passing org_id explicitly should use that value.""" + def test_without_org_id_lists_every_organization(self, organizations): from ndi.cloud.api.datasets import listDatasets - client = _make_client(org_id="org-abc") - listDatasets("org-explicit", client=client) + organizations(ALPHA, BETA) + client = _make_client() + pages = { + "org-abc": {"datasets": [{"id": "ds-a", "name": "A"}], "totalNumber": 1}, + "org-def": {"datasets": [{"id": "ds-b", "name": "B"}], "totalNumber": 1}, + } + client.get.side_effect = lambda path, **kw: pages[kw["organizationId"]] + + result = listDatasets(client=client) - call_kwargs = client.get.call_args - assert call_kwargs.kwargs["organizationId"] == "org-explicit" + assert [d["id"] for d in result["datasets"]] == ["ds-a", "ds-b"] + assert [d["organizationName"] for d in result["datasets"]] == [ + "Alpha Lab", + "Beta Institute", + ] + assert [d["organizationId"] for d in result["datasets"]] == ["org-abc", "org-def"] + assert result["totalNumber"] == 2 + assert result["page"] == 1 - def test_no_org_id_and_no_config_raises(self): - """If org_id is omitted and client config has none, raise ValueError.""" + def test_without_org_id_fetches_every_page(self, organizations): from ndi.cloud.api.datasets import listDatasets - client = _make_client(org_id="") - with pytest.raises(ValueError, match="org_id is required"): - listDatasets(client=client) + organizations(ALPHA) + client = _make_client() + client.get.side_effect = lambda path, params, **kw: { + "datasets": [{"id": f"ds-{params['page']}"}], + "totalNumber": 3, + } + + result = listDatasets(client=client) + + assert [d["id"] for d in result["datasets"]] == ["ds-1", "ds-2", "ds-3"] class TestListAllDatasets: - """listAllDatasets should work without an explicit org_id.""" + def test_without_org_id_lists_every_organization(self, organizations): + from ndi.cloud.api.datasets import listAllDatasets + + organizations(ALPHA, BETA) + result = listAllDatasets(client=_make_client()) + + assert len(result.data) == 2 - def test_without_org_id(self): + def test_with_org_id(self, organizations): from ndi.cloud.api.datasets import listAllDatasets - client = _make_client(org_id="org-abc") - result = listAllDatasets(client=client) + organizations(ALPHA, BETA) + client = _make_client() + result = listAllDatasets("org-abc", client=client) assert len(result.data) == 1 + assert client.get.call_args.kwargs["organizationId"] == "org-abc" class TestCreateDataset: - """createDataset should work without an explicit org_id.""" + """createDataset chooses the organization rather than assuming one.""" - def test_without_org_id(self): + def test_the_only_organization_is_used(self, organizations): from ndi.cloud.api.datasets import createDataset - client = _make_client(org_id="org-abc") + organizations(ALPHA) + client = _make_client() client.post.return_value = {"id": "ds-new", "name": "NewDS"} createDataset(name="NewDS", client=client) - call_kwargs = client.post.call_args - assert call_kwargs.kwargs["organizationId"] == "org-abc" + assert client.post.call_args.kwargs["organizationId"] == "org-abc" + + def test_an_organization_can_be_named(self, organizations): + from ndi.cloud.api.datasets import createDataset + + organizations(ALPHA, BETA) + client = _make_client() + createDataset(name="NewDS", organization_name="Beta Institute", client=client) - def test_without_org_id_and_no_config_raises(self): + assert client.post.call_args.kwargs["organizationId"] == "org-def" + + def test_several_organizations_need_a_choice(self, organizations): from ndi.cloud.api.datasets import createDataset - client = _make_client(org_id="") - with pytest.raises(ValueError, match="org_id is required"): + organizations(ALPHA, BETA) + client = _make_client() + with pytest.raises(CloudOrganizationError) as excinfo: createDataset(name="NewDS", client=client) + + assert excinfo.value.identifier == "NDI:Cloud:OrganizationRequired" + assert "Alpha Lab" in str(excinfo.value) + client.post.assert_not_called() + + def test_an_organization_without_upload_is_refused(self, organizations): + from ndi.cloud.api.datasets import createDataset + + organizations(ALPHA, ARCHIVE) + client = _make_client() + with pytest.raises(CloudOrganizationError) as excinfo: + createDataset("org-ghi", name="NewDS", client=client) + + assert excinfo.value.identifier == "NDI:Cloud:OrganizationCannotUpload" + client.post.assert_not_called() diff --git a/tests/test_cloud_api_documents.py b/tests/test_cloud_api_documents.py index 2c5d1c8f..7739037b 100644 --- a/tests/test_cloud_api_documents.py +++ b/tests/test_cloud_api_documents.py @@ -10,7 +10,6 @@ def _make_client() -> MagicMock: """Return a mock CloudClient.""" client = MagicMock() - client.config.org_id = "org-123" client.config.api_url = "https://api.ndi-cloud.com/v1" return client diff --git a/tests/test_cloud_api_files_crc32.py b/tests/test_cloud_api_files_crc32.py index 111497cf..5059add9 100644 --- a/tests/test_cloud_api_files_crc32.py +++ b/tests/test_cloud_api_files_crc32.py @@ -252,13 +252,16 @@ def _run(self, tmp_path, **kwargs): p = tmp_path / "f.bin" p.write_bytes(b"hello") client = MagicMock() - client.config.org_id = "org1" + # The upload URL is asked for in the dataset's own organization, + # read from the dataset record. + client.get.return_value = {"organizationId": "org1"} with ( patch.object(files_api, "getFileUploadURL", return_value="https://s3/x") as get_url, patch.object(files_api, "putFiles", return_value=True) as put, ): ok, err = uploadSingleFile(DATASET, "uid1", str(p), client=client, **kwargs) assert ok, err + assert get_url.call_args.args[0] == "org1" return get_url, put def test_sends_the_checksum_by_default(self, tmp_path): diff --git a/tests/test_cloud_auth_profile_fallback.py b/tests/test_cloud_auth_profile_fallback.py index 34e9c8a3..4f886ad0 100644 --- a/tests/test_cloud_auth_profile_fallback.py +++ b/tests/test_cloud_auth_profile_fallback.py @@ -32,11 +32,12 @@ from ndi.cloud import auth from ndi.cloud.config import CloudConfig from ndi.cloud.exceptions import CloudAuthError +from ndi.cloud.internal import setOrganizations from ndi.cloud.profile import ProfileEntry _ENV_KEYS = ( "NDI_CLOUD_TOKEN", - "NDI_CLOUD_ORGANIZATION_ID", + "NDI_CLOUD_ORGANIZATIONS", "NDI_CLOUD_USERNAME", "NDI_CLOUD_PASSWORD", "NDI_CLOUD_URL", @@ -73,7 +74,8 @@ def fake(email=None, password=None, config=None): calls.append((email, password, config.api_url if config else None)) config = config or CloudConfig() config.token = token - config.org_id = org + # A real login caches the user's organizations; one here. + setOrganizations([{"id": org, "name": "Org"}]) return config return fake, calls @@ -185,7 +187,7 @@ def test_a_config_built_after_login_still_points_at_dev(self): def test_authenticate_short_circuits_without_losing_the_dev_url(self): os.environ["NDI_CLOUD_TOKEN"] = _unexpired_token() - os.environ["NDI_CLOUD_ORGANIZATION_ID"] = "org-1" + setOrganizations([{"id": "org-1", "name": "Org"}]) with mock.patch("ndi.cloud.config._profile_stage", return_value="dev"): config = CloudConfig.from_env() _, org = auth.authenticate(config) @@ -275,40 +277,45 @@ def _raised(self, config): def test_an_expired_token_is_named_as_expired(self): """The one that reads as missing data if it is not said out loud: the tile is there, the token lapsed.""" - message = self._raised(CloudConfig(token=_expired_token(), org_id="org-1")) + message = self._raised(CloudConfig(token=_expired_token())) self.assertIn("expired", message.lower()) self.assertNotIn("Not logged in", message) def test_an_expired_token_says_when(self): - message = self._raised(CloudConfig(token=_expired_token(), org_id="org-1")) + message = self._raised(CloudConfig(token=_expired_token())) self.assertIn("UTC", message) def test_an_expired_token_says_another_shell_will_not_do(self): """The token lives in os.environ, so logging in elsewhere does not reach a process that is already running. Without that sentence the obvious remedy looks like it did not work.""" - message = self._raised(CloudConfig(token=_expired_token(), org_id="org-1")) + message = self._raised(CloudConfig(token=_expired_token())) self.assertIn("another shell", message) self.assertIn("start it again", message) def test_no_token_at_all_says_so_instead(self): - message = self._raised(CloudConfig(token="", org_id="")) + message = self._raised(CloudConfig(token="")) self.assertIn("Not logged in", message) self.assertNotIn("expired", message.lower().split("only until")[0]) - def test_a_token_with_no_organization_is_its_own_case(self): - """Step 1 wants both. A token that passes the expiry check and is - still refused is otherwise indistinguishable from a bad password.""" - message = self._raised(CloudConfig(token=_unexpired_token(), org_id="")) - self.assertIn("organization id", message) + def test_an_unexpired_token_needs_no_organization(self): + """A user may belong to several organizations, so being logged in + is the token alone: step 1 accepts it with no organization cached, + and the deprecated second output is empty rather than a guess.""" + token = _unexpired_token() + self.assertEqual(auth.authenticate(CloudConfig(token=token)), (token, "")) + + def test_the_second_output_is_empty_for_several_organizations(self): + token = _unexpired_token() + setOrganizations([{"id": "org-1", "name": "A"}, {"id": "org-2", "name": "B"}]) + self.assertEqual(auth.authenticate(CloudConfig(token=token)), (token, "")) def test_every_case_says_how_to_stop_it_happening_again(self): - """Credentials it can reach are the fix for all three: with them, + """Credentials it can reach are the fix for both: with them, authenticate renews the token itself and step 1 stops mattering.""" for config in ( - CloudConfig(token="", org_id=""), - CloudConfig(token=_expired_token(), org_id="org-1"), - CloudConfig(token=_unexpired_token(), org_id=""), + CloudConfig(token=""), + CloudConfig(token=_expired_token()), ): message = self._raised(config) self.assertIn("NDI_CLOUD_USERNAME", message) diff --git a/tests/test_cloud_file_series_round_trip.py b/tests/test_cloud_file_series_round_trip.py index 099d24f4..fe971784 100644 --- a/tests/test_cloud_file_series_round_trip.py +++ b/tests/test_cloud_file_series_round_trip.py @@ -46,6 +46,8 @@ import pytest +from tests._cloud_organizations import testOrganizationID + # --------------------------------------------------------------------------- # Skip entire module if no credentials # --------------------------------------------------------------------------- @@ -80,8 +82,9 @@ def _member_content(i: int) -> bytes: def cloud_client(): """Authenticate once for the whole module and return an authenticated client. - Login populates ``NDI_CLOUD_ORGANIZATION_ID`` on the process, so any - subsequent call that resolves the org id from the environment sees it. + Login caches the account's organizations on the process; the upload + names one explicitly (testOrganizationID) rather than relying on there + being only one. """ from ndi.cloud.auth import login from ndi.cloud.client import CloudClient @@ -163,12 +166,22 @@ def uploaded_series(tmp_path_factory, cloud_client): len(member_uids) == MEMBER_COUNT ), f"the local manifest names {len(member_uids)} members; expected {MEMBER_COUNT}" + # An account that may upload to none of its organizations (and is not an + # administrator) cannot create the dataset this test needs. The server + # used to say so as an HTTP 400 from createDataset, skipped below; the + # organization is now resolved first, so the same account is skipped here. + try: + organization_id = testOrganizationID(cloud_client) + except RuntimeError as exc: + pytest.skip(f"cannot create cloud dataset for this test: {exc}") + unique_name = DATASET_NAME_PREFIX + uuid.uuid4().hex ok, cloud_id, message = uploadDataset( local, remote_name=unique_name, sync_files=True, verbose=False, + organization_id=organization_id, client=cloud_client, ) # The CI test accounts share a quota, and the ingest lane may already diff --git a/tests/test_cloud_file_tier.py b/tests/test_cloud_file_tier.py index 74f6c9b4..2ca3ddc7 100644 --- a/tests/test_cloud_file_tier.py +++ b/tests/test_cloud_file_tier.py @@ -27,7 +27,6 @@ def _make_client() -> MagicMock: """Return a mock CloudClient.""" client = MagicMock() - client.config.org_id = "org-123" client.config.api_url = "https://api.ndi-cloud.com/v1" return client diff --git a/tests/test_cloud_hello_matlab_offline.py b/tests/test_cloud_hello_matlab_offline.py index 971b4872..50de11c9 100644 --- a/tests/test_cloud_hello_matlab_offline.py +++ b/tests/test_cloud_hello_matlab_offline.py @@ -44,7 +44,7 @@ class FakeClient: """Stands in for an authenticated CloudClient.""" - config = types.SimpleNamespace(org_id="org", api_url="https://example.invalid") + config = types.SimpleNamespace(api_url="https://example.invalid") def session_doc(*, session_status="RUNNING", stage_status="RUNNING", message="", instance=""): diff --git a/tests/test_cloud_live.py b/tests/test_cloud_live.py index c8ec5597..8841248d 100644 --- a/tests/test_cloud_live.py +++ b/tests/test_cloud_live.py @@ -27,6 +27,8 @@ import pytest +from tests._cloud_organizations import testOrganizationID + # --------------------------------------------------------------------------- # Skip entire module if no credentials # --------------------------------------------------------------------------- @@ -156,7 +158,9 @@ def can_write(client, cloud_config): try: result = _retry_on_server_error( - lambda: createDataset(cloud_config.org_id, "NDI_PYTEST_WRITE_CHECK", client=client) + lambda: createDataset( + testOrganizationID(client), "NDI_PYTEST_WRITE_CHECK", client=client + ) ) ds_id = result.get("_id", result.get("id", "")) if ds_id: @@ -179,7 +183,7 @@ def fresh_dataset(client, cloud_config, can_write): from ndi.cloud.api.datasets import createDataset, deleteDataset from ndi.cloud.exceptions import CloudAPIError - org_id = cloud_config.org_id + org_id = testOrganizationID(client) try: result = _retry_on_server_error( lambda: createDataset(org_id, "NDI_PYTEST_TEMP_DATASET", client=client) @@ -214,7 +218,7 @@ def _cleanup_stale_pytest_datasets(client, cloud_config): from ndi.cloud.api.datasets import deleteDataset, listDatasets try: - result = listDatasets(cloud_config.org_id, client=client) + result = listDatasets(testOrganizationID(client), client=client) datasets = result.get("datasets", []) stale = [ ds @@ -333,10 +337,14 @@ def test_jwt_has_expected_claims(self, cloud_config): assert "exp" in payload assert any(k in payload for k in ("sub", "email", "userId", "id")) - def test_config_has_org_id(self, cloud_config): - """Login should populate org_id.""" - assert cloud_config.org_id - assert len(cloud_config.org_id) > 10 + def test_login_caches_the_organizations(self, cloud_config): + """Login should cache every organization the user belongs to, with names.""" + from ndi.cloud.internal import _cachedOrganizations + + organizations = _cachedOrganizations() + assert organizations + assert all(len(o["id"]) > 10 for o in organizations) + assert all(isinstance(o["name"], str) for o in organizations) def test_decodeJwt_structure(self, cloud_config): """decodeJwt should return a dict with expected keys.""" @@ -400,7 +408,7 @@ def test_create_and_deleteDataset(self, client, cloud_config, can_write): ) from ndi.cloud.exceptions import CloudAPIError as _APIError - org_id = cloud_config.org_id + org_id = testOrganizationID(client) try: result = _retry_on_server_error( lambda: createDataset(org_id, "NDI_PYTEST_CREATE_DELETE", client=client) @@ -445,7 +453,7 @@ def test_list_datasets(self, client, cloud_config, fresh_dataset): """Created dataset should appear in the org's dataset list.""" from ndi.cloud.api.datasets import listDatasets - result = listDatasets(cloud_config.org_id, client=client) + result = listDatasets(testOrganizationID(client), client=client) datasets = result.get("datasets", []) ids = {d.get("_id", d.get("id", "")) for d in datasets} assert fresh_dataset in ids @@ -696,7 +704,7 @@ def test_getFileUploadURL(self, client, cloud_config, fresh_dataset): from ndi.cloud.api.files import getFileUploadURL url = getFileUploadURL( - cloud_config.org_id, + testOrganizationID(client), fresh_dataset, "pytest-test-file-uid", client=client, @@ -719,7 +727,9 @@ def test_upload_and_download_file(self, client, cloud_config, fresh_dataset): test_content = b"Hello from NDI pytest! This is test file content." # Get upload URL - upload_url = getFileUploadURL(cloud_config.org_id, fresh_dataset, file_uid, client=client) + upload_url = getFileUploadURL( + testOrganizationID(client), fresh_dataset, file_uid, client=client + ) assert upload_url # Upload @@ -764,7 +774,9 @@ def test_listFiles(self, client, fresh_dataset, cloud_config): from ndi.cloud.api.files import getFileUploadURL, listFiles, listFilesAll file_uid = "pytest-list-test-file" - upload_url = getFileUploadURL(cloud_config.org_id, fresh_dataset, file_uid, client=client) + upload_url = getFileUploadURL( + testOrganizationID(client), fresh_dataset, file_uid, client=client + ) requests.put( upload_url, data=b"list test data", @@ -792,7 +804,9 @@ def test_file_details_has_download_url(self, client, fresh_dataset, cloud_config from ndi.cloud.api.files import getFileDetails, getFileUploadURL file_uid = "pytest-details-test-file" - upload_url = getFileUploadURL(cloud_config.org_id, fresh_dataset, file_uid, client=client) + upload_url = getFileUploadURL( + testOrganizationID(client), fresh_dataset, file_uid, client=client + ) requests.put( upload_url, data=b"details test", @@ -847,7 +861,11 @@ def test_upload_with_checksum_round_trip(self, client, cloud_config, fresh_datas missing_uid = "pytest-crc32-never-uploaded" url = getFileUploadURL( - cloud_config.org_id, fresh_dataset, file_uid, checksum_crc32=local_crc, client=client + testOrganizationID(client), + fresh_dataset, + file_uid, + checksum_crc32=local_crc, + client=client, ) assert putFiles(url, str(path), checksum_crc32=local_crc) @@ -880,7 +898,7 @@ def test_upload_with_wrong_checksum_is_rejected( assert crc32(str(path)) != wrong url = getFileUploadURL( - cloud_config.org_id, + testOrganizationID(client), fresh_dataset, "pytest-crc32-wrong", checksum_crc32=wrong, @@ -1077,7 +1095,7 @@ def test_deferred_delete_and_undelete(self, client, cloud_config, can_write): from ndi.cloud.api.documents import addDocument, listDatasetDocumentsAll from ndi.cloud.exceptions import CloudAPIError as _APIError - org_id = cloud_config.org_id + org_id = testOrganizationID(client) try: result = _retry_on_server_error( lambda: createDataset(org_id, "NDI_PYTEST_SOFT_DELETE", client=client) @@ -1150,7 +1168,7 @@ def test_immediate_delete_cannot_undelete(self, client, cloud_config, can_write) from ndi.cloud.api.documents import addDocument from ndi.cloud.exceptions import CloudAPIError as _APIError - org_id = cloud_config.org_id + org_id = testOrganizationID(client) try: result = _retry_on_server_error( lambda: createDataset(org_id, "NDI_PYTEST_HARD_DELETE", client=client) @@ -1212,7 +1230,7 @@ def test_deleteDataset_returns_message(self, client, cloud_config, can_write): from ndi.cloud.api.datasets import createDataset, deleteDataset from ndi.cloud.exceptions import CloudAPIError as _APIError - org_id = cloud_config.org_id + org_id = testOrganizationID(client) try: result = _retry_on_server_error( lambda: createDataset(org_id, "NDI_PYTEST_DEL_MSG", client=client) diff --git a/tests/test_cloud_organizations.py b/tests/test_cloud_organizations.py new file mode 100644 index 00000000..7e63d943 --- /dev/null +++ b/tests/test_cloud_organizations.py @@ -0,0 +1,242 @@ +"""Offline tests for the NDI Cloud organization helpers. + +MATLAB counterpart: tests/+ndi/+unittest/OrganizationsTest.m + +Exercises ndi.cloud.internal.normalizeOrganizations, setOrganizations, +getOrganizations (cache path), chooseOrganization and +ndi.cloud.ui.dialog.organizationLabels / selectOrganization, plus the login +and logout ends of the cache. The token is cleared for every test so nothing +reaches the network. +""" + +from __future__ import annotations + +import json +import os +from unittest import mock + +import pytest + +from ndi.cloud import auth, internal +from ndi.cloud.config import CloudConfig +from ndi.cloud.exceptions import CloudOrganizationError +from ndi.cloud.internal import ( + chooseOrganization, + getOrganizations, + normalizeOrganizations, + setOrganizations, +) +from ndi.cloud.ui.dialog import organizationLabels, selectOrganization + +THREE = [ + {"id": "org-1", "name": "Alpha Lab", "canUploadDataset": True}, + {"id": "org-2", "name": "Beta Institute", "canUploadDataset": True}, + {"id": "org-3", "name": "Gamma Archive", "canUploadDataset": False}, +] + + +@pytest.fixture(autouse=True) +def isolated_env(monkeypatch): + for name in ("NDI_CLOUD_TOKEN", "NDI_CLOUD_ORGANIZATIONS", "NDI_CLOUD_ORGANIZATION_ID"): + monkeypatch.delenv(name, raising=False) + + +def _identifier(call) -> str: + with pytest.raises(CloudOrganizationError) as excinfo: + call() + return excinfo.value.identifier + + +class TestNormalize: + def test_a_list_of_dicts(self): + orgs = normalizeOrganizations(THREE) + assert [o["id"] for o in orgs] == ["org-1", "org-2", "org-3"] + assert [o["canUploadDataset"] for o in orgs] == [True, True, False] + + def test_missing_fields_and_entries_without_an_id(self): + orgs = normalizeOrganizations([{"id": 7, "name": None}, {"name": "dropped"}, "junk"]) + assert orgs == [{"id": "7", "name": "", "canUploadDataset": True}] + + def test_a_single_dict_and_empty(self): + assert normalizeOrganizations({"id": "org-1"})[0]["id"] == "org-1" + assert normalizeOrganizations(None) == [] + assert normalizeOrganizations([]) == [] + + def test_anything_else_is_refused(self): + assert _identifier(lambda: normalizeOrganizations(42)) == "NDI:Cloud:InvalidOrganizations" + + +class TestCache: + def test_round_trip(self): + setOrganizations(THREE) + assert getOrganizations() == normalizeOrganizations(THREE) + + def test_empty_clears(self): + setOrganizations(THREE) + setOrganizations(None) + assert "NDI_CLOUD_ORGANIZATIONS" not in os.environ + + def test_an_empty_cache_is_fetched_and_kept(self): + with mock.patch("ndi.cloud.api.users.me", return_value={"organizations": THREE[:1]}): + assert [o["id"] for o in getOrganizations()] == ["org-1"] + assert [o["id"] for o in getOrganizations()] == ["org-1"] # from the cache now + + +class TestChoose: + def test_the_only_organization(self): + setOrganizations(THREE[:1]) + assert chooseOrganization(interactive=False) == ("org-1", "Alpha Lab") + + def test_by_name_and_by_id(self): + setOrganizations(THREE) + assert chooseOrganization("Beta Institute", interactive=False) == ( + "org-2", + "Beta Institute", + ) + assert chooseOrganization(organization_id="org-3", interactive=False)[0] == "org-3" + + def test_several_need_a_choice_when_no_one_can_be_asked(self): + setOrganizations(THREE) + assert ( + _identifier(lambda: chooseOrganization(interactive=False)) + == "NDI:Cloud:OrganizationRequired" + ) + + def test_require_upload_narrows_the_candidates(self): + orgs = [dict(o) for o in THREE] + orgs[1]["canUploadDataset"] = False + setOrganizations(orgs) + assert chooseOrganization(require_upload=True, interactive=False)[0] == "org-1" + + def test_require_upload_refuses_a_read_only_organization(self): + setOrganizations(THREE) + assert ( + _identifier( + lambda: chooseOrganization("Gamma Archive", require_upload=True, interactive=False) + ) + == "NDI:Cloud:OrganizationCannotUpload" + ) + + def test_an_administrator_may_upload_anywhere(self): + setOrganizations(THREE[2:]) + with mock.patch.object(internal, "userIsAdmin", return_value=True): + assert chooseOrganization(require_upload=True, interactive=False)[0] == "org-3" + + def test_no_uploadable_organization(self): + setOrganizations(THREE[2:]) + assert ( + _identifier(lambda: chooseOrganization(require_upload=True, interactive=False)) + == "NDI:Cloud:NoUploadableOrganization" + ) + + def test_unknown_and_ambiguous(self): + setOrganizations(THREE) + assert ( + _identifier(lambda: chooseOrganization("Nope", interactive=False)) + == "NDI:Cloud:UnknownOrganization" + ) + twins = [dict(o) for o in THREE] + twins[1]["name"] = "Alpha Lab" + setOrganizations(twins) + assert ( + _identifier(lambda: chooseOrganization("Alpha Lab", interactive=False)) + == "NDI:Cloud:AmbiguousOrganization" + ) + + def test_name_and_id_are_mutually_exclusive(self): + setOrganizations(THREE) + assert ( + _identifier(lambda: chooseOrganization("Alpha Lab", "org-1", interactive=False)) + == "NDI:Cloud:OrganizationArguments" + ) + + def test_no_organizations_at_all(self): + with mock.patch("ndi.cloud.api.users.me", return_value={"organizations": []}): + assert ( + _identifier(lambda: chooseOrganization(interactive=False)) + == "NDI:Cloud:NoOrganizations" + ) + + +class TestAsking: + def test_the_chooser_sees_names_and_its_index_picks(self): + setOrganizations(THREE) + seen = {} + + def chooser(labels, prompt): + seen["labels"], seen["prompt"] = labels, prompt + return 1 + + assert chooseOrganization(interactive=True, chooser=chooser)[0] == "org-2" + assert seen["labels"] == ["Alpha Lab", "Beta Institute", "Gamma Archive"] + assert seen["prompt"] == "Select an organization:" + + def test_cancelling_is_its_own_error(self): + setOrganizations(THREE) + assert ( + _identifier(lambda: chooseOrganization(interactive=True, chooser=lambda *_: None)) + == "NDI:CloudDialog:UserCanceled" + ) + + def test_the_console_counts_from_one(self, monkeypatch, capsys): + setOrganizations(THREE) + answers = iter(["9", "3"]) + monkeypatch.setattr("builtins.input", lambda _prompt: next(answers)) + assert chooseOrganization(interactive=True)[0] == "org-3" + assert " 1. Alpha Lab" in capsys.readouterr().out + + def test_select_organization_always_asks(self): + setOrganizations(THREE[:1]) + asked = [] + selectOrganization( + auto_select_single=False, chooser=lambda labels, p: asked.append(labels) or 0 + ) + assert asked == [["Alpha Lab"]] + + +class TestLabels: + def test_names_ids_and_duplicates(self): + assert organizationLabels(["Alpha", "Beta"], ["o1", "o2"]) == ["Alpha", "Beta"] + assert organizationLabels(["", "Beta"], ["o1", "o2"]) == ["o1", "Beta"] + assert organizationLabels(["Same", "Same"], ["o1", "o2"]) == [ + "Same (o1)", + "Same (o2)", + ] + assert organizationLabels(["Only"], ["o1", "o2"]) == ["Only", "o2"] + + +class TestLoginAndLogout: + def test_login_caches_every_organization(self): + response = mock.Mock(status_code=200) + response.json.return_value = {"token": "tok", "user": {"organizations": THREE}} + with mock.patch("requests.post", return_value=response): + config = auth.login("a@example.com", "pw", CloudConfig(api_url="https://x.invalid")) + assert config.token == "tok" + assert [o["id"] for o in json.loads(os.environ["NDI_CLOUD_ORGANIZATIONS"])] == [ + "org-1", + "org-2", + "org-3", + ] + + def test_logout_clears_the_cache(self): + setOrganizations(THREE) + with mock.patch("requests.post"): + auth.logout(CloudConfig(api_url="https://x.invalid", token="tok")) + assert internal._cachedOrganizations() == [] + + +class TestDatasetOrganization: + def test_read_from_the_dataset_record(self): + client = mock.MagicMock() + client.get.return_value = {"organizationId": "org-2"} + assert internal._datasetOrganizationId("65a1b2c3d4e5f60718293a4b", client=client) == "org-2" + + def test_a_record_without_one_is_an_error(self): + client = mock.MagicMock() + client.get.return_value = {} + assert ( + _identifier( + lambda: internal._datasetOrganizationId("65a1b2c3d4e5f60718293a4b", client=client) + ) + == "NDI:Cloud:NoOrganizations" + ) diff --git a/tests/test_cloud_secrets_hygiene.py b/tests/test_cloud_secrets_hygiene.py index 9b1a8d10..cf9150a9 100644 --- a/tests/test_cloud_secrets_hygiene.py +++ b/tests/test_cloud_secrets_hygiene.py @@ -173,7 +173,7 @@ def test_login_401_message_does_not_echo_body(self, monkeypatch): from ndi.cloud.auth import CloudConfig _login_401_with_body(monkeypatch, self._BODY_WITH_SECRETS) - cfg = CloudConfig(api_url="https://example.invalid", token="", org_id="") + cfg = CloudConfig(api_url="https://example.invalid", token="") with pytest.raises(auth.CloudAuthError) as exc_info: auth.login("user@example.com", "hunter2", config=cfg) self._assert_body_absent_and_status_present(exc_info.value) @@ -183,7 +183,7 @@ def test_change_password_401_message_does_not_echo_body(self, monkeypatch): from ndi.cloud.auth import CloudConfig _login_401_with_body(monkeypatch, self._BODY_WITH_SECRETS) - cfg = CloudConfig(api_url="https://example.invalid", token="tok", org_id="") + cfg = CloudConfig(api_url="https://example.invalid", token="tok") with pytest.raises(auth.CloudAuthError) as exc_info: auth.changePassword("old", "new", config=cfg) self._assert_body_absent_and_status_present(exc_info.value) @@ -193,7 +193,7 @@ def test_reset_password_401_message_does_not_echo_body(self, monkeypatch): from ndi.cloud.auth import CloudConfig _login_401_with_body(monkeypatch, self._BODY_WITH_SECRETS) - cfg = CloudConfig(api_url="https://example.invalid", token="", org_id="") + cfg = CloudConfig(api_url="https://example.invalid", token="") with pytest.raises(auth.CloudAuthError) as exc_info: auth.resetPassword("user@example.com", config=cfg) self._assert_body_absent_and_status_present(exc_info.value) @@ -203,7 +203,7 @@ def test_verify_user_401_message_does_not_echo_body(self, monkeypatch): from ndi.cloud.auth import CloudConfig _login_401_with_body(monkeypatch, self._BODY_WITH_SECRETS) - cfg = CloudConfig(api_url="https://example.invalid", token="", org_id="") + cfg = CloudConfig(api_url="https://example.invalid", token="") with pytest.raises(auth.CloudAuthError) as exc_info: auth.verifyUser("user@example.com", "code123", config=cfg) self._assert_body_absent_and_status_present(exc_info.value) @@ -213,7 +213,7 @@ def test_resend_confirmation_401_message_does_not_echo_body(self, monkeypatch): from ndi.cloud.auth import CloudConfig _login_401_with_body(monkeypatch, self._BODY_WITH_SECRETS) - cfg = CloudConfig(api_url="https://example.invalid", token="", org_id="") + cfg = CloudConfig(api_url="https://example.invalid", token="") with pytest.raises(auth.CloudAuthError) as exc_info: auth.resendConfirmation("user@example.com", config=cfg) self._assert_body_absent_and_status_present(exc_info.value) @@ -226,7 +226,7 @@ def test_body_is_still_available_at_debug_level(self, monkeypatch, caplog): from ndi.cloud.auth import CloudConfig _login_401_with_body(monkeypatch, self._BODY_WITH_SECRETS) - cfg = CloudConfig(api_url="https://example.invalid", token="", org_id="") + cfg = CloudConfig(api_url="https://example.invalid", token="") with caplog.at_level(logging.DEBUG, logger="ndi.cloud.auth"): with pytest.raises(auth.CloudAuthError): diff --git a/tests/test_cloud_sync.py b/tests/test_cloud_sync.py index 6976887a..24fe667e 100644 --- a/tests/test_cloud_sync.py +++ b/tests/test_cloud_sync.py @@ -143,7 +143,7 @@ def test_from_env_creates_client(self, mock_config_from_env, mock_authenticate): mock_config = MagicMock() mock_config.api_url = "https://api.example.com" mock_config_from_env.return_value = mock_config - mock_authenticate.return_value = ("fake-token", "fake-org-id") + mock_authenticate.return_value = ("fake-token", "") client = CloudClient.from_env() @@ -151,7 +151,6 @@ def test_from_env_creates_client(self, mock_config_from_env, mock_authenticate): assert client.config is mock_config mock_authenticate.assert_called_once_with(mock_config) assert mock_config.token == "fake-token" - assert mock_config.org_id == "fake-org-id" @patch("ndi.cloud.auth.authenticate") @patch("ndi.cloud.config.CloudConfig.from_env") diff --git a/tests/test_cloud_tests_name_organization.py b/tests/test_cloud_tests_name_organization.py new file mode 100644 index 00000000..b6d06ca8 --- /dev/null +++ b/tests/test_cloud_tests_name_organization.py @@ -0,0 +1,72 @@ +"""Tests that create cloud datasets must name the organization. + +MATLAB counterpart: tests/+ndi/+unittest/CloudTestsNameOrganizationTest.m + +NDI Cloud users may belong to several organizations. When a call to +``createDataset``, ``uploadDataset`` or ``newDataset`` does not name one, +:func:`ndi.cloud.internal.chooseOrganization` asks on a terminal and raises +otherwise (a test cannot answer). A test that relies on that only works for +accounts with a single organization -- which is what CI runs with, so CI +cannot catch it. Every such call in tests/ must therefore pass +``organization_id`` / ``org_id`` (normally ``testOrganizationID(client)``) or +``organization_name``. + +This scans the test sources statically; it needs no network or credentials. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +TESTS_ROOT = Path(__file__).resolve().parent + +#: Files whose calls deliberately name no organization, with why. +EXEMPT = { + "test_cloud_api_datasets.py": "tests chooseOrganization's own defaults, offline", + "test_cloud_upload_orchestration.py": "offline; caches exactly one organization", + "test_dataset_ingestion.py": "offline; caches exactly one organization", + "test_cloud_tests_name_organization.py": "this guard", +} + +_CALL = re.compile(r"\b(createDataset|uploadDataset|newDataset)\(") +_NAMED = re.compile(r"\b(organization_name|organization_id|org_id)\b|testOrganizationID") + + +def _dataset_creating_calls(source: str) -> list[str]: + """Full text of each call in SOURCE, ignoring comment lines and definitions.""" + text = "\n".join(line for line in source.splitlines() if not line.lstrip().startswith("#")) + calls = [] + for match in _CALL.finditer(text): + if text[max(0, match.start() - 4) : match.start()] == "def ": + continue + depth, i = 1, match.end() + while depth and i < len(text): + depth += {"(": 1, ")": -1}.get(text[i], 0) + i += 1 + calls.append(text[match.start() : i]) + return calls + + +def test_dataset_creating_calls_name_an_organization(): + offenders = [] + for path in sorted(TESTS_ROOT.rglob("*.py")): + if path.name in EXEMPT: + continue + for call in _dataset_creating_calls(path.read_text(encoding="utf-8")): + if not _NAMED.search(call): + offenders.append(f"{path.relative_to(TESTS_ROOT)}: {' '.join(call.split())}") + assert not offenders, ( + "These calls create a cloud dataset without naming an organization; pass " + "organization_id=testOrganizationID(client):\n" + "\n".join(offenders) + ) + + +def test_the_scan_finds_a_call_that_names_none(): + """The guard would catch one: an unnamed call is found, a named one is not.""" + unnamed = 'ok = uploadDataset(\n local,\n remote_name="x",\n)' + named = "uploadDataset(local, organization_id=testOrganizationID(client))" + calls = _dataset_creating_calls(unnamed + "\n" + named) + assert len(calls) == 2 + assert not _NAMED.search(calls[0]) + assert _NAMED.search(calls[1]) diff --git a/tests/test_cloud_upload_file_batches.py b/tests/test_cloud_upload_file_batches.py index 5d0c7b1c..069e4514 100644 --- a/tests/test_cloud_upload_file_batches.py +++ b/tests/test_cloud_upload_file_batches.py @@ -21,6 +21,7 @@ import pytest +import ndi.cloud.internal import ndi.cloud.sync.operations as ops import ndi.cloud.upload as upload import ndi.preferences @@ -30,12 +31,8 @@ CLOUD_ID = "65a1b2c3d4e5f60718293a4b" -class _Config: - org_id = "org-1" - - class _Client: - config = _Config() + """A stand-in client; the dataset's organization is looked up, not configured.""" def _manifest(tmp_path: Path, sizes: dict[str, int]) -> list[dict]: @@ -96,6 +93,13 @@ def _wait(job_id, timeout=60.0, client=None, **kwargs): state = self._job_states.pop(0) if self._job_states else "complete" return {"state": state} + # The organization comes from the dataset record, as in MATLAB's + # GetFileCollectionUploadURL -- not from the logged-in user. + def _dataset_org(dataset_id, client=None): + assert dataset_id == CLOUD_ID + return "org-1" + + monkeypatch.setattr(ndi.cloud.internal, "_datasetOrganizationId", _dataset_org) monkeypatch.setattr(files_api, "getFileCollectionUploadURL", _url) monkeypatch.setattr(files_api, "putFiles", _put) monkeypatch.setattr(files_api, "waitForBulkUpload", _wait) diff --git a/tests/test_cloud_upload_orchestration.py b/tests/test_cloud_upload_orchestration.py index 99bffe92..376f1ee4 100644 --- a/tests/test_cloud_upload_orchestration.py +++ b/tests/test_cloud_upload_orchestration.py @@ -43,10 +43,11 @@ from ndi.dataset import ndi_dataset_dir from ndi.document import ndi_document from ndi.query import ndi_query +from tests._cloud_organizations import use_organizations class FakeClient: - config = types.SimpleNamespace(org_id="org") + config = types.SimpleNamespace() @pytest.fixture @@ -80,6 +81,7 @@ def upload_docs(cloud_id, doc_jsons, client=None, **kw): # helpers are imported inside the function bodies, so a name bound on the # orchestration module is never consulted. monkeypatch.setattr(upload_mod, "uploadDocumentCollection", upload_docs) + use_organizations(monkeypatch, {"id": "org", "name": "Org"}) return box diff --git a/tests/test_dataset_ingestion.py b/tests/test_dataset_ingestion.py index 87c10813..eff2f301 100644 --- a/tests/test_dataset_ingestion.py +++ b/tests/test_dataset_ingestion.py @@ -36,6 +36,7 @@ from ndi.document import ndi_document from ndi.query import ndi_query from ndi.session.dir import ndi_session_dir +from tests._cloud_organizations import use_organizations FILE_SLOT = "filename1.ext" PAYLOAD = "hello world" @@ -386,7 +387,7 @@ def test_an_unmeasurable_session_does_not_block_the_copy(self, linked, monkeypat # ====================================================================== class FakeClient: class config: - org_id = "org" + pass class TestUploadDatasetRefusesUningestedDatasets: @@ -409,6 +410,7 @@ def upload_docs(cloud_id, doc_jsons, client=None, **kw): monkeypatch.setattr(ds_api, "createDataset", create_dataset) monkeypatch.setattr(upload_mod, "uploadDocumentCollection", upload_docs) + use_organizations(monkeypatch, {"id": "org", "name": "Org"}) return box def test_an_uningested_dataset_is_refused(self, linked, captured, monkeypatch): diff --git a/tests/test_gui_nav_datasets_cloud.py b/tests/test_gui_nav_datasets_cloud.py index 3cf72332..d512dc8b 100644 --- a/tests/test_gui_nav_datasets_cloud.py +++ b/tests/test_gui_nav_datasets_cloud.py @@ -64,7 +64,9 @@ def test_success_links_the_dataset_without_another_query(self, monkeypatch): """A successful upload is what links it, so the state is known.""" import ndi.cloud.orchestration as orch - monkeypatch.setattr(orch, "uploadDataset", lambda ds, verbose=False: (True, "cid", "")) + monkeypatch.setattr( + orch, "uploadDataset", lambda ds, verbose=False, **kw: (True, "cid", "") + ) r = dc.upload_dataset(FakeDataset()) assert r.ok and r.state == "incloud" @@ -72,7 +74,7 @@ def test_a_reported_failure_is_not_a_success(self, monkeypatch): import ndi.cloud.orchestration as orch monkeypatch.setattr( - orch, "uploadDataset", lambda ds, verbose=False: (False, "", "quota exceeded") + orch, "uploadDataset", lambda ds, verbose=False, **kw: (False, "", "quota exceeded") ) r = dc.upload_dataset(FakeDataset()) assert not r.ok @@ -82,7 +84,7 @@ def test_a_reported_failure_is_not_a_success(self, monkeypatch): def test_a_raised_error_is_reported_not_propagated(self, monkeypatch): import ndi.cloud.orchestration as orch - def boom(ds, verbose=False): + def boom(ds, verbose=False, **kw): raise RuntimeError("network down") monkeypatch.setattr(orch, "uploadDataset", boom) diff --git a/tests/test_gui_nav_datasets_pane.py b/tests/test_gui_nav_datasets_pane.py index ee3cee83..fde57575 100644 --- a/tests/test_gui_nav_datasets_pane.py +++ b/tests/test_gui_nav_datasets_pane.py @@ -966,21 +966,41 @@ def test_labels_and_ids_stay_index_aligned(self, monkeypatch): assert labels[1] == "i2" assert ids == ["i1", "i2"] - def test_private_authenticates_first(self, monkeypatch): + def test_private_lists_every_organization(self, monkeypatch): + """No organization is passed: listDatasets() covers all of them.""" import ndi.cloud.api.datasets as api - import ndi.cloud.auth as auth from ndi.gui.nav.datasets_pane import fetch_cloud_datasets seen = {} - monkeypatch.setattr(auth, "authenticate", lambda *a, **k: ("tok", "org-7")) - def fake_list(org_id, *a, **k): - seen["org"] = org_id - return {"datasets": []} + def fake_list(*a, **k): + seen["args"] = a + return {"datasets": [{"id": "i1", "name": "One", "organizationName": "Alpha"}]} monkeypatch.setattr(api, "listDatasets", fake_list) - fetch_cloud_datasets(False) - assert seen["org"] == "org-7" + labels, ids = fetch_cloud_datasets(False) + assert seen["args"] == () + # One organization: its name is not repeated on every row. + assert labels == ["One (i1)"] + assert ids == ["i1"] + + def test_private_labels_name_the_organization_when_there_are_several(self, monkeypatch): + import ndi.cloud.api.datasets as api + from ndi.gui.nav.datasets_pane import fetch_cloud_datasets + + monkeypatch.setattr( + api, + "listDatasets", + lambda *a, **k: { + "datasets": [ + {"id": "i1", "name": "One", "organizationName": "Alpha"}, + {"id": "i2", "name": "Two", "organizationName": "Beta"}, + ] + }, + ) + labels, ids = fetch_cloud_datasets(False) + assert labels == ["[Alpha] One (i1)", "[Beta] Two (i2)"] + assert ids == ["i1", "i2"] def test_an_empty_catalogue_is_two_empty_lists(self, monkeypatch): import ndi.cloud.api.datasets as api diff --git a/tests/test_phase2_gaps.py b/tests/test_phase2_gaps.py index eab60d5a..ec321927 100644 --- a/tests/test_phase2_gaps.py +++ b/tests/test_phase2_gaps.py @@ -438,14 +438,17 @@ def test_direct_upload_success(self): mock_files.getFileUploadURL.return_value = "https://s3.example.com/upload" mock_files.putFiles.return_value = None - with patch.dict("sys.modules", {"ndi.cloud.api.files": mock_files}): - with patch("ndi.cloud.api.files", mock_files): - # The file is never read here (the upload API is mocked), so - # skip the CRC-32 that uploadSingleFile computes by default; - # tests/test_cloud_api_files_crc32.py covers that path. - success, err = uploadSingleFile( - "ds-123", "file-uid-1", "/tmp/test.dat", checksum=False, client=client - ) + with ( + patch.dict("sys.modules", {"ndi.cloud.api.files": mock_files}), + patch("ndi.cloud.api.files", mock_files), + patch("ndi.cloud.internal._datasetOrganizationId", return_value="org-1"), + ): + # The file is never read here (the upload API is mocked), so + # skip the CRC-32 that uploadSingleFile computes by default; + # tests/test_cloud_api_files_crc32.py covers that path. + success, err = uploadSingleFile( + "ds-123", "file-uid-1", "/tmp/test.dat", checksum=False, client=client + ) assert success is True assert err == "" @@ -458,13 +461,16 @@ def test_upload_failure(self): mock_files = MagicMock() mock_files.getFileUploadURL.side_effect = Exception("Network error") - with patch.dict("sys.modules", {"ndi.cloud.api.files": mock_files}): - with patch("ndi.cloud.api.files", mock_files): - # As above: the file does not exist, so skip the default CRC-32 - # and let the mocked network error be what fails. - success, err = uploadSingleFile( - "ds-123", "file-uid-1", "/tmp/test.dat", checksum=False, client=client - ) + with ( + patch.dict("sys.modules", {"ndi.cloud.api.files": mock_files}), + patch("ndi.cloud.api.files", mock_files), + patch("ndi.cloud.internal._datasetOrganizationId", return_value="org-1"), + ): + # As above: the file does not exist, so skip the default CRC-32 + # and let the mocked network error be what fails. + success, err = uploadSingleFile( + "ds-123", "file-uid-1", "/tmp/test.dat", checksum=False, client=client + ) assert success is False assert "Network error" in err diff --git a/tests/test_sync_binaries_and_deltas.py b/tests/test_sync_binaries_and_deltas.py index 1e70651e..5c71301f 100644 --- a/tests/test_sync_binaries_and_deltas.py +++ b/tests/test_sync_binaries_and_deltas.py @@ -25,6 +25,7 @@ import ndi.cloud.upload as upload from ndi.cloud.sync.index import SyncIndex from ndi.cloud.sync.mode import SyncOptions +from tests._cloud_organizations import use_organizations from .conftest import FakeDataset, make_document @@ -37,7 +38,7 @@ def _no_bulk_settling(monkeypatch): class _Config: - org_id = "org-1" + """No organization here: file uploads use the dataset's own.""" class _Client: @@ -132,6 +133,7 @@ def uploads(self, monkeypatch): from ndi.cloud.api import files as files_api sent: list[str] = [] + use_organizations(monkeypatch, {"id": "org-1", "name": "Org"}) monkeypatch.setattr(docs_api, "addDocument", lambda *a, **k: {"ok": True}) monkeypatch.setattr(internal, "listRemoteDocumentIds", lambda cid, client=None: {}) monkeypatch.setattr( diff --git a/tests/test_sync_binaries_gate_the_index.py b/tests/test_sync_binaries_gate_the_index.py index e50bd09e..11449101 100644 --- a/tests/test_sync_binaries_gate_the_index.py +++ b/tests/test_sync_binaries_gate_the_index.py @@ -37,6 +37,7 @@ import ndi.cloud.upload as upload from ndi.cloud.sync.index import SyncIndex from ndi.cloud.sync.mode import SyncOptions +from tests._cloud_organizations import use_organizations from .conftest import FakeDataset, make_document @@ -44,7 +45,7 @@ class _Config: - org_id = "org-1" + """No organization here: file uploads use the dataset's own.""" class _Client: @@ -98,6 +99,7 @@ def _binaries(monkeypatch, failing): from ndi.cloud.api import files as files_api failing_uids = {f"uid-{d}" for d in failing} + use_organizations(monkeypatch, {"id": "org-1", "name": "Org"}) def _url(org, ds, uid, client=None): if uid in failing_uids: