diff --git a/fileglancer/database.py b/fileglancer/database.py index cfef60e69..3b1d9670c 100644 --- a/fileglancer/database.py +++ b/fileglancer/database.py @@ -597,9 +597,13 @@ def get_cloud_buckets(session: Session, username: str) -> List[CloudBucketDB]: return session.query(CloudBucketDB).filter_by(username=username).order_by(CloudBucketDB.name).all() -def bucket_url_roots(session: Session, username: str) -> List[Tuple[str, str]]: - """(fsp_name, public base URL) of each of the user's buckets, for ngstate.bucket_source.""" - return [(b.name, s3_public_url(b.bucket, b.endpoint_url, b.prefix)) for b in get_cloud_buckets(session, username)] +def bucket_url_roots(session: Session, username: str) -> List[Tuple[str, str, str]]: + """(fsp_name, public base URL, path it serves) of each of the user's buckets + and each externally mapped path, for ngstate.bucket_source.""" + return ([(b.name, s3_public_url(b.bucket, b.endpoint_url, b.prefix), '') + for b in get_cloud_buckets(session, username)] + + [(e.fsp_name, e.external_url, e.relative_path or '') + for e in get_external_buckets(session) if e.external_url]) def get_cloud_bucket(session: Session, username: str, name: str) -> Optional[CloudBucketDB]: @@ -1328,6 +1332,24 @@ def _path_aliases(path: str) -> List[str]: return ["", "."] if path == "" else [path] +def get_views_for_dataset(session: Session, owner: str, fsp_name: str, path: str) -> List[ViewDB]: + """The owner's Views with a working layer on this dataset, matched by + (fsp_name, path) so layers without a Data Link (cloud buckets, externally + mapped paths) count too. IN-subquery for the reason in get_views_for_data_link.""" + layer_view_ids = ( + session.query(ViewLayerDB.view_id) + .filter(ViewLayerDB.broken.is_(False), + ViewLayerDB.fsp_name == fsp_name, + ViewLayerDB.path.in_(_path_aliases(path))) + ) + return ( + session.query(ViewDB) + .filter(ViewDB.id.in_(layer_view_ids), ViewDB.owner == owner) + .order_by(ViewDB.created_at.desc()) + .all() + ) + + def get_relinkable_views(session: Session, owner: str, fsp_name: str, path: str) -> List[ViewDB]: """The owner's Views with a broken layer on this dataset that can be relinked (its dead Data Link key and prefix are known).""" diff --git a/fileglancer/ngstate.py b/fileglancer/ngstate.py index cf74e0634..a6e2ad5f5 100644 --- a/fileglancer/ngstate.py +++ b/fileglancer/ngstate.py @@ -46,16 +46,19 @@ def layer_sharing_key(layer: dict, proxy_url: str) -> Optional[str]: return None -def bucket_source(layer: dict, buckets: Iterable[Tuple[str, str]]) -> Optional[Tuple[str, str]]: +def bucket_source(layer: dict, buckets: Iterable[Tuple[str, str, str]]) -> Optional[Tuple[str, str]]: """(fsp_name, path) of the first source in `layer` served straight from one - of `buckets`, given as (fsp_name, public base URL) pairs; None if none is. - A bucket layer has no Data Link, so its source URL is the only identity.""" - roots = sorted(((name, root.rstrip('/')) for name, root in buckets), key=lambda r: -len(r[1])) + of `buckets`, given as (fsp_name, public base URL, path the URL serves) + triples; None if none is. A bucket layer has no Data Link, so its source + URL is the only identity.""" + roots = sorted(((name, root.rstrip('/'), prefix.strip('/')) for name, root, prefix in buckets), + key=lambda r: -len(r[1])) for url in _iter_source_urls(layer.get('source')): base = re.split(r'[|?#]', _SCHEME_PREFIX.sub('', url), maxsplit=1)[0] - for name, root in roots: + for name, root, prefix in roots: if base == root or base.startswith(root + '/'): - return name, unquote(base[len(root):]).strip('/') + rest = unquote(base[len(root):]).strip('/') + return name, '/'.join(p for p in (prefix, rest) if p) return None diff --git a/fileglancer/server.py b/fileglancer/server.py index 702db69b4..b58ad5100 100644 --- a/fileglancer/server.py +++ b/fileglancer/server.py @@ -1756,6 +1756,16 @@ async def list_views_endpoint(username: str = Depends(get_current_user)): views = db.get_views(session, username) return ViewResponse(views=[View.model_validate(v) for v in views]) + @app.get("/api/neuroglancer/views/for-dataset", response_model=ViewResponse, + description="The caller's Views with a working layer on this dataset") + async def views_for_dataset_endpoint(fsp_name: str = Query(..., description="File share path name"), + path: str = Query(..., description="Path relative to the file share path"), + username: str = Depends(get_current_user)): + path = _normalize_proxied_path(path) + with db.get_db_session(settings.db_url) as session: + views = db.get_views_for_dataset(session, username, fsp_name, path) + return ViewResponse(views=[View.model_validate(v) for v in views]) + @app.get("/api/neuroglancer/views/relinkable", response_model=ViewSummaryResponse, description="The caller's broken Views that a Data Link to this dataset would repair") async def relinkable_views_endpoint(fsp_name: str = Query(..., description="File share path name"), diff --git a/frontend/src/__tests__/componentTests/AppearsInViews.test.tsx b/frontend/src/__tests__/componentTests/AppearsInViews.test.tsx index 3470f6da7..b28a8f28e 100644 --- a/frontend/src/__tests__/componentTests/AppearsInViews.test.tsx +++ b/frontend/src/__tests__/componentTests/AppearsInViews.test.tsx @@ -2,15 +2,25 @@ import { describe, it, expect, vi } from 'vitest'; import { render, screen } from '@testing-library/react'; import { MemoryRouter } from 'react-router'; -const { useViewsForDataLinkQuery } = vi.hoisted(() => ({ - useViewsForDataLinkQuery: vi.fn() +const { useViewsForDataLinkQuery, useViewsForDatasetQuery } = vi.hoisted( + () => ({ + useViewsForDataLinkQuery: vi.fn(), + useViewsForDatasetQuery: vi.fn() + }) +); +vi.mock('@/queries/viewQueries', () => ({ + useViewsForDataLinkQuery, + useViewsForDatasetQuery })); -vi.mock('@/queries/viewQueries', () => ({ useViewsForDataLinkQuery })); + +// What a disabled TanStack query returns +const disabled = { data: undefined, isPending: true, isError: false }; import AppearsInViews from '@/components/ui/PropertiesDrawer/AppearsInViews'; describe('AppearsInViews', () => { it('lists the dependent Views with a count', () => { + useViewsForDatasetQuery.mockReturnValue(disabled); useViewsForDataLinkQuery.mockReturnValue({ data: [ { short_key: 'v1', name: 'Alpha', read_key: 'rk1' }, @@ -35,7 +45,31 @@ describe('AppearsInViews', () => { ); }); + it('lists the Views of a dataset without a Data Link', () => { + useViewsForDataLinkQuery.mockReturnValue(disabled); + useViewsForDatasetQuery.mockReturnValue({ + data: [{ short_key: 'v1', name: 'Alpha', read_key: 'rk1' }], + isPending: false, + isError: false + }); + render( + + + + ); + expect(useViewsForDatasetQuery).toHaveBeenCalledWith( + 'idr', + 'zarr/img.zarr' + ); + expect(screen.getByText(/appears in 1 view$/i)).toBeInTheDocument(); + expect(screen.getByRole('link', { name: 'Alpha' })).toHaveAttribute( + 'href', + '/view/rk1' + ); + }); + it('renders nothing when there are no dependent Views', () => { + useViewsForDatasetQuery.mockReturnValue(disabled); useViewsForDataLinkQuery.mockReturnValue({ data: [], isPending: false, diff --git a/frontend/src/__tests__/componentTests/CreateViewButton.test.tsx b/frontend/src/__tests__/componentTests/CreateViewButton.test.tsx index 3b5b1cc0e..aa3411e0b 100644 --- a/frontend/src/__tests__/componentTests/CreateViewButton.test.tsx +++ b/frontend/src/__tests__/componentTests/CreateViewButton.test.tsx @@ -8,7 +8,8 @@ let automatic = true; vi.mock('@/hooks/useCartCheckout', () => ({ useCartCheckout: () => ({ checkout, - isBucketDataset: (ds: { fsp_name: string }) => ds.fsp_name === 'bucket' + isServedWithoutLink: async (ds: { fsp_name: string }) => + ds.fsp_name === 'bucket' }) })); vi.mock('@/contexts/PreferencesContext', () => ({ @@ -56,7 +57,7 @@ describe('CreateViewButton', () => { ).not.toBeInTheDocument(); await user.click(screen.getByRole('button', { name: /^create$/i })); - expect(checkout).toHaveBeenCalledWith(datasets, 'V'); + expect(checkout).toHaveBeenCalledWith(datasets, 'V', undefined); }); it('checks out with the edited name, not defaultName', async () => { @@ -69,7 +70,11 @@ describe('CreateViewButton', () => { await user.type(input, 'My Renamed View'); await user.click(screen.getByRole('button', { name: /^create$/i })); - expect(checkout).toHaveBeenCalledWith(datasets, 'My Renamed View'); + expect(checkout).toHaveBeenCalledWith( + datasets, + 'My Renamed View', + undefined + ); }); it('disables the create button when the name is empty', async () => { @@ -99,6 +104,6 @@ describe('CreateViewButton', () => { await user.type(input, 'Linked View'); await user.click(screen.getByRole('button', { name: /continue/i })); - expect(checkout).toHaveBeenCalledWith(datasets, 'Linked View'); + expect(checkout).toHaveBeenCalledWith(datasets, 'Linked View', undefined); }); }); diff --git a/frontend/src/__tests__/componentTests/SidebarFileBrowser.test.tsx b/frontend/src/__tests__/componentTests/SidebarFileBrowser.test.tsx index e51a0577f..33e63bf91 100644 --- a/frontend/src/__tests__/componentTests/SidebarFileBrowser.test.tsx +++ b/frontend/src/__tests__/componentTests/SidebarFileBrowser.test.tsx @@ -170,7 +170,7 @@ describe('SidebarFileBrowser', () => { false, expect.any(Function) ); - expect(addToView).toHaveBeenCalledWith(items); + expect(addToView).toHaveBeenCalledWith(items, { urlPrefixes: undefined }); expect( screen.getByRole('checkbox', { name: 'Select a.zarr' }) ).not.toBeChecked(); diff --git a/frontend/src/__tests__/componentTests/ViewerSidebar.test.tsx b/frontend/src/__tests__/componentTests/ViewerSidebar.test.tsx index 8da725299..63fe38c78 100644 --- a/frontend/src/__tests__/componentTests/ViewerSidebar.test.tsx +++ b/frontend/src/__tests__/componentTests/ViewerSidebar.test.tsx @@ -1,6 +1,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { act, render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; import type { ReactNode } from 'react'; import { makeFakeBridge } from '@/__tests__/mocks/fakeNeuroglancer'; @@ -98,14 +99,16 @@ function setup() { selectedLayer: { layer: 'a GFP', visible: true } }); render( - + + + ); return fake; } diff --git a/frontend/src/__tests__/componentTests/useCartCheckout.test.tsx b/frontend/src/__tests__/componentTests/useCartCheckout.test.tsx index b91891d76..c7831bf10 100644 --- a/frontend/src/__tests__/componentTests/useCartCheckout.test.tsx +++ b/frontend/src/__tests__/componentTests/useCartCheckout.test.tsx @@ -6,6 +6,8 @@ import type { ReactNode } from 'react'; const createProxied = vi.fn(); const createViewAsync = vi.fn(); const buildViewState = vi.hoisted(() => vi.fn()); +const fetchExternalBucket = vi.hoisted(() => vi.fn()); +let subpathMode = 'name'; vi.mock('@/queries/proxiedPathQueries', () => ({ useAllProxiedPathsQuery: () => ({ @@ -19,7 +21,14 @@ vi.mock('@/contexts/ViewsContext', () => ({ }) })); vi.mock('@/contexts/PreferencesContext', () => ({ - usePreferencesContext: () => ({ disableHeuristicalLayerTypeDetection: false }) + usePreferencesContext: () => ({ + disableHeuristicalLayerTypeDetection: false, + dataLinkSubpathMode: subpathMode + }) +})); +vi.mock('@/queries/externalBucketQueries', async importOriginal => ({ + ...(await importOriginal()), + fetchExternalBucket })); vi.mock('@/utils/viewCheckout', () => ({ buildViewState @@ -28,7 +37,12 @@ vi.mock('@/contexts/ZonesAndFspMapContext', () => ({ useZoneAndFspMapContext: () => ({ zonesAndFspQuery: { data: { - fsp_f: { name: 'f', protocol: 'file', mount_path: '/f' }, + fsp_f: { + name: 'f', + protocol: 'file', + mount_path: '/f', + linux_path: '/groups/f' + }, fsp_bucket: { name: 'bucket', protocol: 's3', @@ -49,6 +63,8 @@ function wrapper({ children }: { children: ReactNode }) { } beforeEach(() => { + subpathMode = 'name'; + fetchExternalBucket.mockReset().mockResolvedValue(null); createProxied.mockReset().mockResolvedValue({ fsp_name: 'f', path: '/b', @@ -117,8 +133,83 @@ describe('useCartCheckout', () => { }) ]); expect(createProxied).not.toHaveBeenCalled(); - expect(result.current.isBucketDataset({ fsp_name: 'bucket' })).toBe(true); - expect(result.current.isBucketDataset({ fsp_name: 'f' })).toBe(false); + expect( + await result.current.isServedWithoutLink({ fsp_name: 'bucket', path: '' }) + ).toBe(true); + expect( + await result.current.isServedWithoutLink({ fsp_name: 'f', path: '/b' }) + ).toBe(false); + }); + + it('names new Data Links by the subpath-mode preference, or the given custom prefix', async () => { + subpathMode = 'full_path'; + const { result } = renderHook(() => useCartCheckout(), { wrapper }); + await result.current.resolveCartDatasets([ + { fsp_name: 'f', path: 'x/b.zarr', label: 'B' } + ]); + expect(createProxied).toHaveBeenLastCalledWith({ + fsp_name: 'f', + path: 'x/b.zarr', + url_prefix: 'groups/f/x/b.zarr' + }); + + await result.current.resolveCartDatasets( + [{ fsp_name: 'f', path: 'x/c.zarr', label: 'C' }], + { 'f::x/c.zarr': 'my-name' } + ); + expect(createProxied).toHaveBeenLastCalledWith( + expect.objectContaining({ path: 'x/c.zarr', url_prefix: 'my-name' }) + ); + }); + + it('uses an external bucket URL instead of a Data Link', async () => { + fetchExternalBucket.mockResolvedValue({ + full_path: '/groups/f/pub', + external_url: 'https://s3.example.org/pub', + fsp_name: 'f', + relative_path: 'pub' + }); + const { result } = renderHook(() => useCartCheckout(), { wrapper }); + const resolved = await result.current.resolveCartDatasets([ + { fsp_name: 'f', path: 'pub/s.zarr', label: 'S' } + ]); + expect(resolved).toEqual([ + expect.objectContaining({ + url: 'https://s3.example.org/pub/s.zarr', + sharing_key: null + }) + ]); + expect(createProxied).not.toHaveBeenCalled(); + expect( + await result.current.isServedWithoutLink({ + fsp_name: 'f', + path: 'pub/s.zarr' + }) + ).toBe(true); + }); + + it('falls back to a Data Link when the external bucket lookup fails', async () => { + fetchExternalBucket.mockRejectedValue(new Error('boom')); + const { result } = renderHook(() => useCartCheckout(), { wrapper }); + const resolved = await result.current.resolveCartDatasets([ + { fsp_name: 'f', path: '/b', label: 'B' } + ]); + expect(resolved).toEqual([expect.objectContaining({ sharing_key: 'kb' })]); + expect(createProxied).toHaveBeenCalled(); + expect( + await result.current.isServedWithoutLink({ fsp_name: 'f', path: '/b' }) + ).toBe(false); + }); + + it('rejects an invalid custom prefix before creating the link', async () => { + const { result } = renderHook(() => useCartCheckout(), { wrapper }); + await expect( + result.current.resolveCartDatasets( + [{ fsp_name: 'f', path: 'x/c.zarr', label: 'C' }], + { 'f::x/c.zarr': 'a//b' } + ) + ).rejects.toThrow(/consecutive slashes/); + expect(createProxied).not.toHaveBeenCalled(); }); it('resolves Data Links without building or creating a View', async () => { diff --git a/frontend/src/__tests__/componentTests/useCreateViewFlow.test.tsx b/frontend/src/__tests__/componentTests/useCreateViewFlow.test.tsx index 4d2207e59..8cfc210e2 100644 --- a/frontend/src/__tests__/componentTests/useCreateViewFlow.test.tsx +++ b/frontend/src/__tests__/componentTests/useCreateViewFlow.test.tsx @@ -8,6 +8,7 @@ const resolveCartDatasets = vi.fn(); const buildViewState = vi.fn(); const navigate = vi.fn(); let automatic = true; +let subpathMode = 'full_path'; vi.mock('@/utils/viewCheckout', () => ({ buildViewState: (...args: unknown[]) => buildViewState(...args) })); @@ -15,13 +16,14 @@ vi.mock('@/hooks/useCartCheckout', () => ({ useCartCheckout: () => ({ checkout, resolveCartDatasets, - isBucketDataset: (ds: { fsp_name: string }) => ds.fsp_name === 'bucket' + isServedWithoutLink: async (ds: { fsp_name: string }) => + ds.fsp_name === 'bucket' }) })); vi.mock('@/contexts/PreferencesContext', () => ({ usePreferencesContext: () => ({ areDataLinksAutomatic: automatic, - dataLinkSubpathMode: 'full_path', + dataLinkSubpathMode: subpathMode, toggleAutomaticDataLinks: vi.fn() }) })); @@ -62,6 +64,7 @@ beforeEach(() => { .mockReset() .mockResolvedValue({ short_key: 'v1', read_key: 'rk1', name: 'A' }); automatic = true; + subpathMode = 'full_path'; navigate.mockReset(); resolveCartDatasets.mockReset().mockResolvedValue([{ url: 'u' }]); buildViewState @@ -86,7 +89,7 @@ describe('useCreateViewFlow draft', () => { }); expect(screen.queryByRole('textbox')).not.toBeInTheDocument(); - expect(window.open).toHaveBeenCalledWith('', '_blank'); + await waitFor(() => expect(window.open).toHaveBeenCalledWith('', '_blank')); await waitFor(() => expect(tab.location.href).not.toBe('')); const url = tab.location.href; const id = new URL(url, 'http://x').searchParams.get('draft')!; @@ -157,6 +160,22 @@ describe('useCreateViewFlow draft', () => { await waitFor(() => expect(tab.location.href).not.toBe('')); }); + it('opens the tab on the click and closes it again when consent is needed', async () => { + automatic = false; + const apiRef: { current: FlowApi | null } = { current: null }; + render(); + + act(() => { + apiRef.current!.startDraftView(datasets, 'A'); + }); + + // Synchronously, inside the click's user activation. + expect(window.open).toHaveBeenCalledWith('', '_blank'); + await screen.findByText(/are you sure you want to create a data link/i); + expect(tab.close).toHaveBeenCalled(); + expect(tab.location.href).toBe(''); + }); + it('toasts and stays put when the draft cannot be built', async () => { buildViewState.mockRejectedValue(new Error('No layers')); const apiRef: { current: FlowApi | null } = { current: null }; @@ -194,7 +213,7 @@ describe('useCreateViewFlow', () => { const user = userEvent.setup(); await user.click(screen.getByRole('button', { name: /^create$/i })); - expect(checkout).toHaveBeenCalledWith(datasets, 'A'); + expect(checkout).toHaveBeenCalledWith(datasets, 'A', undefined); expect(onCreated).toHaveBeenCalledWith( expect.objectContaining({ read_key: 'rk1' }) ); @@ -238,7 +257,57 @@ describe('useCreateViewFlow', () => { const user = userEvent.setup(); await user.click(screen.getByRole('button', { name: /continue/i })); - expect(checkout).toHaveBeenCalledWith(datasets, 'A'); + expect(checkout).toHaveBeenCalledWith(datasets, 'A', undefined); + }); + + it('asks for a name per new data link in custom naming mode and checks out with them', async () => { + subpathMode = 'custom'; + const apiRef: { current: FlowApi | null } = { current: null }; + render(); + + act(() => { + apiRef.current!.startCreateView( + [{ fsp_name: 'f', path: 'x/a.zarr', label: 'a.zarr' }], + 'A' + ); + }); + + const input = await screen.findByLabelText('Data link name for a.zarr'); + expect(input).toHaveValue('a.zarr'); + const user = userEvent.setup(); + await user.clear(input); + expect(screen.getByRole('button', { name: /continue/i })).toBeDisabled(); + await user.type(input, 'my-link'); + await user.click(screen.getByRole('button', { name: /continue/i })); + expect(checkout).toHaveBeenCalledWith( + [{ fsp_name: 'f', path: 'x/a.zarr', label: 'a.zarr' }], + 'A', + { 'f::x/a.zarr': 'my-link' } + ); + }); + + it('only asks for names, not consent, when links are automatic in custom mode', async () => { + subpathMode = 'custom'; + const apiRef: { current: FlowApi | null } = { current: null }; + render(); + + act(() => { + apiRef.current!.startCreateView( + [{ fsp_name: 'f', path: '.', label: 'root' }], + 'A' + ); + }); + + expect(await screen.findByText('Set data link name')).toBeInTheDocument(); + expect( + screen.queryByText(/are you sure you want to create a data link/i) + ).toBeNull(); + expect(screen.queryByLabelText(/automatically create/i)).toBeNull(); + // FSP root: the default name is the FSP, not "." + expect(screen.getByLabelText('Data link name for root')).toHaveValue('f'); + expect( + screen.getByRole('button', { name: /continue/i }) + ).not.toBeDisabled(); }); it('asks no data-link consent for a bucket dataset, which needs no link', async () => { @@ -271,9 +340,9 @@ describe('useCreateViewFlow', () => { }); const user = userEvent.setup(); - await user.click(screen.getByRole('button', { name: /^create$/i })); + await user.click(await screen.findByRole('button', { name: /^create$/i })); - expect(checkout).toHaveBeenCalledWith(datasets, 'A'); + expect(checkout).toHaveBeenCalledWith(datasets, 'A', undefined); expect(toast.error).toHaveBeenCalledWith('checkout failed'); expect( screen.getByRole('textbox', { name: /view name/i }) @@ -306,7 +375,7 @@ describe('useCreateViewFlow', () => { ).toBeInTheDocument(); expect(add).not.toHaveBeenCalled(); await userEvent.click(screen.getByRole('button', { name: 'Continue' })); - await waitFor(() => expect(add).toHaveBeenCalledWith(datasets)); + await waitFor(() => expect(add).toHaveBeenCalledWith(datasets, undefined)); await waitFor(() => expect(screen.queryByText('Add to this View')).toBeNull() ); @@ -324,7 +393,7 @@ describe('useCreateViewFlow', () => { await screen.findByText(/differ from the first layer/i) ).toBeInTheDocument(); await userEvent.click(screen.getByRole('button', { name: 'Add' })); - await waitFor(() => expect(add).toHaveBeenCalledWith(datasets)); + await waitFor(() => expect(add).toHaveBeenCalledWith(datasets, undefined)); }); it('shows Data Link consent and the mismatch warning together', async () => { diff --git a/frontend/src/__tests__/unitTests/useViewSources.test.tsx b/frontend/src/__tests__/unitTests/useViewSources.test.tsx index add5a6c89..014956e33 100644 --- a/frontend/src/__tests__/unitTests/useViewSources.test.tsx +++ b/frontend/src/__tests__/unitTests/useViewSources.test.tsx @@ -1,5 +1,7 @@ import { describe, it, expect, vi, afterEach } from 'vitest'; -import { act, renderHook } from '@testing-library/react'; +import { act, renderHook, waitFor } from '@testing-library/react'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import type { ReactNode } from 'react'; import { makeFakeBridge } from '@/__tests__/mocks/fakeNeuroglancer'; import { @@ -12,6 +14,11 @@ import type { ViewLayer } from '@/queries/viewQueries'; import type { ResolvedCheckoutDataset } from '@/utils/viewCheckout'; const BUCKET_URL = 'https://idr.s3.amazonaws.com'; +const fetchExternalBucket = vi.hoisted(() => vi.fn()); +vi.mock('@/queries/externalBucketQueries', async importOriginal => ({ + ...(await importOriginal()), + fetchExternalBucket +})); vi.mock('@/queries/zoneAndFileSharePathMapQuery', () => ({ default: () => ({ data: { @@ -146,6 +153,7 @@ describe('groupLayersBySource', () => { describe('useViewSources', () => { afterEach(() => { vi.useRealTimers(); + fetchExternalBucket.mockReset(); }); const setup = (opts: { @@ -156,13 +164,22 @@ describe('useViewSources', () => { removedSources?: string[]; }) => { const fake = makeFakeBridge({ layers: opts.layers }); - const hook = renderHook(() => - useViewSources(fake.bridge, { - viewLayers: opts.viewLayers ?? [], - added: opts.added ?? [], - pendingSources: opts.pendingSources ?? [], - removedSources: opts.removedSources ?? [] - }) + const client = new QueryClient({ + defaultOptions: { queries: { retry: false } } + }); + const hook = renderHook( + () => + useViewSources(fake.bridge, { + viewLayers: opts.viewLayers ?? [], + added: opts.added ?? [], + pendingSources: opts.pendingSources ?? [], + removedSources: opts.removedSources ?? [] + }), + { + wrapper: ({ children }: { children: ReactNode }) => ( + {children} + ) + } ); return { fake, ...hook }; }; @@ -208,6 +225,27 @@ describe('useViewSources', () => { ]); }); + it('maps saved layers of an externally mapped path by their external URL', async () => { + fetchExternalBucket.mockResolvedValue({ + full_path: '/mnt/f/pub', + external_url: 'https://s3.example.org/pub/', + fsp_name: 'f', + relative_path: 'pub' + }); + const { result } = setup({ + layers: [ + { name: 'img', source: 'https://s3.example.org/pub/a.zarr/|zarr3:' } + ], + viewLayers: [row(null, 'pub/a.zarr', { data_link_id: null })] + }); + await waitFor(() => + expect(result.current.map(s => [s.fsp_name, s.path])).toEqual([ + ['f', 'pub/a.zarr'] + ]) + ); + expect(fetchExternalBucket).toHaveBeenCalledWith('f', expect.anything()); + }); + it('follows layers removed in Neuroglancer', () => { vi.useFakeTimers(); const { fake, result } = setup({ diff --git a/frontend/src/components/ui/Dialogs/DataLink.tsx b/frontend/src/components/ui/Dialogs/DataLink.tsx index 98bb37763..7e0aa5a33 100644 --- a/frontend/src/components/ui/Dialogs/DataLink.tsx +++ b/frontend/src/components/ui/Dialogs/DataLink.tsx @@ -123,17 +123,19 @@ function BtnContainer({ children }: { readonly children: ReactNode }) { return
{children}
; } -function CustomSubpathInput({ +export function CustomSubpathInput({ value, error, - onChange + onChange, + label = 'Data link name' }: { readonly value: string; readonly error: string | null; readonly onChange: (value: string) => void; + readonly label?: string; }) { return ( - + onChange(e.target.value)} diff --git a/frontend/src/components/ui/PropertiesDrawer/AppearsInViews.tsx b/frontend/src/components/ui/PropertiesDrawer/AppearsInViews.tsx index a7e65a3d6..90ef5b694 100644 --- a/frontend/src/components/ui/PropertiesDrawer/AppearsInViews.tsx +++ b/frontend/src/components/ui/PropertiesDrawer/AppearsInViews.tsx @@ -1,14 +1,33 @@ import { Typography } from '@material-tailwind/react'; import { Link } from 'react-router'; -import { useViewsForDataLinkQuery } from '@/queries/viewQueries'; +import { + useViewsForDataLinkQuery, + useViewsForDatasetQuery +} from '@/queries/viewQueries'; -interface AppearsInViewsProps { - readonly sharingKey: string; -} +// A Data Link's list, or a dataset's (fspName + path) when it has no Data Link +type AppearsInViewsProps = + | { + readonly sharingKey: string; + readonly fspName?: never; + readonly path?: never; + } + | { + readonly sharingKey?: never; + readonly fspName: string; + readonly path: string; + }; -export default function AppearsInViews({ sharingKey }: AppearsInViewsProps) { - const viewsQuery = useViewsForDataLinkQuery(sharingKey); +export default function AppearsInViews({ + sharingKey, + fspName, + path +}: AppearsInViewsProps) { + const dataLinkQuery = useViewsForDataLinkQuery(sharingKey); + const datasetQuery = useViewsForDatasetQuery(fspName, path); + // The unused query is disabled, and a disabled query stays pending + const viewsQuery = sharingKey ? dataLinkQuery : datasetQuery; // Stay quiet while loading / on error / when unused — this is a // supplementary read-only hint, not a primary control. diff --git a/frontend/src/components/ui/PropertiesDrawer/PropertiesDrawer.tsx b/frontend/src/components/ui/PropertiesDrawer/PropertiesDrawer.tsx index d28f7eb65..a7b5f1919 100644 --- a/frontend/src/components/ui/PropertiesDrawer/PropertiesDrawer.tsx +++ b/frontend/src/components/ui/PropertiesDrawer/PropertiesDrawer.tsx @@ -20,6 +20,7 @@ import DataLinkUsageDialog from '@/components/ui/Dialogs/dataLinkUsage/DataLinkU import TextDialogBtn from '@/components/ui/buttons/DialogTextBtn'; import FgSwitch from '@/components/ui/widgets/FgSwitch'; import { getPreferredPathForDisplay } from '@/utils'; +import { normalizeFspRootPath } from '@/utils/pathHandling'; import { copyToClipboard } from '@/utils/copyText'; import { isReadOnlyFsp } from '@/utils/cloudStorage'; import { useFileBrowserContext } from '@/contexts/FileBrowserContext'; @@ -417,6 +418,13 @@ export default function PropertiesDrawer({ + ) : currentFsp && + fileBrowserState.dataLinkPath !== null && + (readOnly || externalDataUrlQuery.data) ? ( + ) : null} diff --git a/frontend/src/components/ui/Views/SidebarFileBrowser.tsx b/frontend/src/components/ui/Views/SidebarFileBrowser.tsx index 231be7044..7d5185e37 100644 --- a/frontend/src/components/ui/Views/SidebarFileBrowser.tsx +++ b/frontend/src/components/ui/Views/SidebarFileBrowser.tsx @@ -8,12 +8,16 @@ import { useCreateViewFlow } from '@/hooks/useCreateViewFlow'; import useFileSelector from '@/hooks/useFileSelector'; import type { CartItem } from '@/contexts/CartContext'; import type { DatasetRef } from '@/hooks/useCartDimensionCheck'; +import type { UrlPrefixes } from '@/hooks/useCartCheckout'; import type { FileOrFolder } from '@/shared.types'; type SidebarFileBrowserProps = { /** The datasets in the viewer now, in layer order: the dimension reference. */ readonly viewSources: DatasetRef[]; - readonly addToView: (items: CartItem[]) => Promise; + readonly addToView: ( + items: CartItem[], + options?: { urlPrefixes?: UrlPrefixes } + ) => Promise; }; /** @@ -83,8 +87,8 @@ export default function SidebarFileBrowser({ }); const add = () => - startAddToView(items, hasMismatch, async datasets => { - await addToView(datasets); + startAddToView(items, hasMismatch, async (datasets, urlPrefixes) => { + await addToView(datasets, { urlPrefixes }); setChecked(new Set()); }); diff --git a/frontend/src/hooks/useAddToView.ts b/frontend/src/hooks/useAddToView.ts index 2f4e0977f..64a26acf3 100644 --- a/frontend/src/hooks/useAddToView.ts +++ b/frontend/src/hooks/useAddToView.ts @@ -13,6 +13,7 @@ import { } from '@/utils/viewCheckout'; import { datasetKey } from '@/utils/pathHandling'; import type { CartItem } from '@/contexts/CartContext'; +import type { UrlPrefixes } from '@/hooks/useCartCheckout'; import type { NeuroglancerBridge } from '@/hooks/useNeuroglancerViewer'; import type { ViewLayer } from '@/queries/viewQueries'; import type { NgLayer, ResolvedCheckoutDataset } from '@/utils/viewCheckout'; @@ -38,7 +39,10 @@ export function useAddToView( readonly onAddSources: (sharingKeys: string[]) => void; readonly onAdded: (datasets: ResolvedCheckoutDataset[]) => void; } -): (items: CartItem[], options?: { shown?: boolean }) => Promise { +): ( + items: CartItem[], + options?: { shown?: boolean; urlPrefixes?: UrlPrefixes } +) => Promise { const { resolveCartDatasets } = useCartCheckout(); const { disableHeuristicalLayerTypeDetection } = usePreferencesContext(); // The sidebar unmounts when NG reloads (the bridge leaves `ready`). An add @@ -52,11 +56,14 @@ export function useAddToView( }, []); return useCallback( - async (items: CartItem[], options?: { shown?: boolean }) => { + async ( + items: CartItem[], + options?: { shown?: boolean; urlPrefixes?: UrlPrefixes } + ) => { // Snapshot before resolving: creating a Data Link relinks this View's // broken layers on that dataset server-side and refetches the rows. const broken = viewLayers.filter(isRelinkableLayer); - const resolved = await resolveCartDatasets(items); + const resolved = await resolveCartDatasets(items, options?.urlPrefixes); const { ng_state, layers } = await buildViewState( resolved, !disableHeuristicalLayerTypeDetection diff --git a/frontend/src/hooks/useCartCheckout.ts b/frontend/src/hooks/useCartCheckout.ts index 67b4b6533..53a993ff2 100644 --- a/frontend/src/hooks/useCartCheckout.ts +++ b/frontend/src/hooks/useCartCheckout.ts @@ -1,9 +1,15 @@ import { useCallback } from 'react'; +import { useQueryClient } from '@tanstack/react-query'; import { useAllProxiedPathsQuery, useCreateProxiedPathMutation } from '@/queries/proxiedPathQueries'; +import { + externalBucketQueryKeys, + fetchExternalBucket, + transformBucketToDataUrl +} from '@/queries/externalBucketQueries'; import { usePreferencesContext } from '@/contexts/PreferencesContext'; import { useViewsContext } from '@/contexts/ViewsContext'; import { useZoneAndFspMapContext } from '@/contexts/ZonesAndFspMapContext'; @@ -15,6 +21,8 @@ import { normalizeFspRootPath } from '@/utils/pathHandling'; import { isReadOnlyFsp } from '@/utils/cloudStorage'; +import { defaultUrlPrefix, validateUrlPrefix } from '@/hooks/useDataToolLinks'; +import { default as log } from '@/logger'; import { makeMapKey } from '@/utils'; import type { CartItem } from '@/queries/preferencesQueries'; import type { View } from '@/queries/viewQueries'; @@ -26,6 +34,9 @@ type ResolvedLink = Pick & { sharing_key: string | null; }; +/** Custom url_prefix per new Data Link, keyed by datasetKey. */ +export type UrlPrefixes = Record; + // A dataset can hold both a base (channel-less) CartItem and channel-specific // CartItems at once. Drop the base entry when channel entries exist for the // same dataset, so buildViewState doesn't run the dataset twice (once for @@ -53,28 +64,63 @@ export function useCartCheckout() { const createProxiedPath = useCreateProxiedPathMutation(); const { createViewMutation } = useViewsContext(); const { zonesAndFspQuery } = useZoneAndFspMapContext(); - const { disableHeuristicalLayerTypeDetection } = usePreferencesContext(); + const { disableHeuristicalLayerTypeDetection, dataLinkSubpathMode } = + usePreferencesContext(); + const queryClient = useQueryClient(); + + const fspFor = useCallback( + (ds: Pick): FileSharePath | undefined => + zonesAndFspQuery.data?.[makeMapKey('fsp', ds.fsp_name)] as + | FileSharePath + | undefined, + [zonesAndFspQuery.data] + ); const bucketFor = useCallback( (ds: Pick): FileSharePath | null => { - const fsp = zonesAndFspQuery.data?.[makeMapKey('fsp', ds.fsp_name)] as - | FileSharePath - | undefined; + const fsp = fspFor(ds); return isReadOnlyFsp(fsp) ? fsp! : null; }, - [zonesAndFspQuery.data] + [fspFor] + ); + + /** The URL an external bucket mapping serves this dataset at, if any. */ + const externalUrlFor = useCallback( + async (ds: Pick): Promise => { + try { + const bucket = await queryClient.fetchQuery({ + queryKey: externalBucketQueryKeys.byFsp(ds.fsp_name), + queryFn: ({ signal }) => fetchExternalBucket(ds.fsp_name, signal), + staleTime: 5 * 60 * 1000 + }); + return transformBucketToDataUrl(bucket, ds.path, ds.fsp_name); + } catch (error) { + // Like the file browser, a failed lookup means "no mapping": fall + // back to a Data Link rather than failing the whole checkout. + log.warn(`External bucket lookup failed for ${ds.fsp_name}`, error); + return null; + } + }, + [queryClient] ); - /** A public bucket's data needs no Data Link; it is served at its own URL. */ - const isBucketDataset = useCallback( - (ds: Pick): boolean => bucketFor(ds) !== null, - [bucketFor] + /** + * Public buckets and externally mapped paths need no Data Link; they are + * served at their own URL. + */ + const isServedWithoutLink = useCallback( + async (ds: Pick): Promise => + bucketFor(ds) !== null || (await externalUrlFor(ds)) !== null, + [bucketFor, externalUrlFor] ); // Finds or creates one Data Link per dataset. Shared by checkout and // "Add to this View". const resolveCartDatasets = useCallback( - async (rawDatasets: CartItem[]): Promise => { + async ( + rawDatasets: CartItem[], + urlPrefixes?: UrlPrefixes + ): Promise => { const datasets = dropShadowedBaseEntries(rawDatasets); const existing = new Map( (allProxiedPathsQuery.data ?? []).map(p => [ @@ -91,17 +137,40 @@ export function useCartCheckout() { continue; } const bucket = bucketFor(ds); - const link: ResolvedLink = bucket - ? { - url: bucketDatasetUrl(bucket, ds.path), - sharing_key: null, - url_prefix: '' - } - : (existing.get(key) ?? - (await createProxiedPath.mutateAsync({ - fsp_name: ds.fsp_name, - path: normalizeFspRootPath(ds.path) - }))); + // Like the file browser's data tools, an external URL wins over a + // Data Link. + const ownUrl = bucket + ? bucketDatasetUrl(bucket, ds.path) + : await externalUrlFor(ds); + if (ownUrl) { + linkByKey.set(key, { + url: ownUrl, + sharing_key: null, + url_prefix: '' + }); + continue; + } + const path = normalizeFspRootPath(ds.path); + let link = existing.get(key); + if (!link) { + const url_prefix = + urlPrefixes?.[key] ?? + defaultUrlPrefix( + dataLinkSubpathMode, + path, + ds.fsp_name, + fspFor(ds)?.linux_path + ); + const prefixError = validateUrlPrefix(url_prefix); + if (prefixError) { + throw new Error(prefixError); + } + link = await createProxiedPath.mutateAsync({ + fsp_name: ds.fsp_name, + path, + url_prefix + }); + } linkByKey.set(key, link); } @@ -124,12 +193,23 @@ export function useCartCheckout() { }; }); }, - [allProxiedPathsQuery.data, createProxiedPath, bucketFor] + [ + allProxiedPathsQuery.data, + createProxiedPath, + bucketFor, + externalUrlFor, + fspFor, + dataLinkSubpathMode + ] ); const checkout = useCallback( - async (rawDatasets: CartItem[], name: string): Promise => { - const resolved = await resolveCartDatasets(rawDatasets); + async ( + rawDatasets: CartItem[], + name: string, + urlPrefixes?: UrlPrefixes + ): Promise => { + const resolved = await resolveCartDatasets(rawDatasets, urlPrefixes); const { ng_state, layers } = await buildViewState( resolved, !disableHeuristicalLayerTypeDetection @@ -143,5 +223,5 @@ export function useCartCheckout() { ] ); - return { checkout, resolveCartDatasets, isBucketDataset }; + return { checkout, resolveCartDatasets, isServedWithoutLink }; } diff --git a/frontend/src/hooks/useCreateViewFlow.tsx b/frontend/src/hooks/useCreateViewFlow.tsx index f7c325e0e..7c8ece801 100644 --- a/frontend/src/hooks/useCreateViewFlow.tsx +++ b/frontend/src/hooks/useCreateViewFlow.tsx @@ -5,10 +5,12 @@ import { useNavigate } from 'react-router'; import type { ReactNode } from 'react'; import { useCartCheckout } from '@/hooks/useCartCheckout'; +import type { UrlPrefixes } from '@/hooks/useCartCheckout'; +import { defaultUrlPrefix, validateUrlPrefix } from '@/hooks/useDataToolLinks'; import { useCartDimensionCheck } from '@/hooks/useCartDimensionCheck'; import { usePreferencesContext } from '@/contexts/PreferencesContext'; import { useAllProxiedPathsQuery } from '@/queries/proxiedPathQueries'; -import { datasetKey } from '@/utils/pathHandling'; +import { datasetKey, normalizeFspRootPath } from '@/utils/pathHandling'; import { buildUrl } from '@/utils'; import { buildViewState } from '@/utils/viewCheckout'; import type { ResolvedCheckoutDataset } from '@/utils/viewCheckout'; @@ -16,6 +18,7 @@ import FgButton from '@/components/designSystem/atoms/FgButton'; import FgInput from '@/components/designSystem/atoms/formElements/FgInput'; import FgSwitch from '@/components/designSystem/atoms/formElements/FgSwitch'; import FgDialog from '@/components/ui/Dialogs/FgDialog'; +import { CustomSubpathInput } from '@/components/ui/Dialogs/DataLink'; import type { CartItem } from '@/queries/preferencesQueries'; import type { View, ViewCreateRequest } from '@/queries/viewQueries'; @@ -65,7 +68,8 @@ export function clearDraft(id: string | null) { type PendingRequest = { datasets: CartItem[]; - newLinkCount: number; + /** One dataset per Data Link checkout will create. */ + newLinks: CartItem[]; needsLinkConsent: boolean; } & ( | { mode: 'create'; onCreated?: (view: View) => void } @@ -73,14 +77,15 @@ type PendingRequest = { | { mode: 'draft'; name: string } | { mode: 'add'; - add: (datasets: CartItem[]) => Promise; + add: (datasets: CartItem[], urlPrefixes?: UrlPrefixes) => Promise; // Checked against the View's own datasets by the caller. hasMismatch: boolean; } ); export function useCreateViewFlow() { - const { checkout, resolveCartDatasets, isBucketDataset } = useCartCheckout(); + const { checkout, resolveCartDatasets, isServedWithoutLink } = + useCartCheckout(); const { areDataLinksAutomatic, dataLinkSubpathMode, @@ -92,21 +97,52 @@ export function useCreateViewFlow() { const [pending, setPending] = useState(false); const [name, setName] = useState(''); - const [request, setRequest] = useState(null); + const [request, setRequestState] = useState(null); + // Data link names typed in the dialog, used when naming mode is 'custom'. + const [urlPrefixes, setUrlPrefixes] = useState({}); const nameInputRef = useRef(null); const cartCheck = useCartDimensionCheck(request?.datasets ?? []); const hasMismatch = request?.mode === 'add' ? request.hasMismatch : cartCheck.hasMismatch; - const openDraft = async (datasets: CartItem[], name: string) => { - // Open the tab now, while the click's user activation is still fresh: - // building the state renders a thumbnail per dataset, which can outlast - // it and get the popup blocked. Falls back to this tab if blocked anyway. - // No 'noopener': it makes window.open return null, which reads as blocked. - const tab = window.open('', '_blank'); + const askNames = dataLinkSubpathMode === 'custom'; + const setRequest = (next: PendingRequest | null) => { + setRequestState(next); + setUrlPrefixes( + Object.fromEntries( + (next?.newLinks ?? []).map(ds => [ + datasetKey(ds.fsp_name, ds.path), + defaultUrlPrefix( + 'custom', + normalizeFspRootPath(ds.path), + ds.fsp_name, + null + ) + ]) + ) + ); + }; + const prefixErrors = askNames + ? Object.values(urlPrefixes).map(validateUrlPrefix).filter(Boolean) + : []; + const chosenPrefixes = askNames ? urlPrefixes : undefined; + + // Open the tab while the click's user activation is still fresh: the + // consent check can fetch, and building the state renders a thumbnail per + // dataset, either of which can outlast it and get the popup blocked. Falls + // back to this tab if blocked anyway. No 'noopener': it makes window.open + // return null, which reads as blocked. + const openTab = () => window.open('', '_blank'); + + const openDraft = async ( + datasets: CartItem[], + name: string, + prefixes?: UrlPrefixes, + tab: Window | null = openTab() + ) => { try { - const resolved = await resolveCartDatasets(datasets); + const resolved = await resolveCartDatasets(datasets, prefixes); const { ng_state, layers } = await buildViewState( resolved, !disableHeuristicalLayerTypeDetection @@ -130,11 +166,11 @@ export function useCreateViewFlow() { setPending(true); try { if (request.mode === 'add') { - await request.add(request.datasets); + await request.add(request.datasets, chosenPrefixes); } else if (request.mode === 'draft') { - await openDraft(request.datasets, request.name); + await openDraft(request.datasets, request.name, chosenPrefixes); } else { - const view = await checkout(request.datasets, name); + const view = await checkout(request.datasets, name, chosenPrefixes); toast.success(`Created View "${name}"`); if (request.onCreated) { request.onCreated(view); @@ -150,85 +186,108 @@ export function useCreateViewFlow() { } }; - const linkConsent = (datasets: CartItem[]) => { + const linkConsent = async (datasets: CartItem[]) => { const existingKeys = new Set( (allProxiedPathsQuery.data ?? []).map(p => datasetKey(p.fsp_name, p.path)) ); - // Public buckets need no Data Link, so they never count - const newLinkCount = new Set( - datasets - .filter(ds => !isBucketDataset(ds)) - .map(ds => datasetKey(ds.fsp_name, ds.path)) - .filter(key => !existingKeys.has(key)) - ).size; + const newLinksByKey = new Map(); + for (const ds of datasets) { + const key = datasetKey(ds.fsp_name, ds.path); + // Buckets and externally mapped paths need no Data Link, so they never count + if ( + !existingKeys.has(key) && + !newLinksByKey.has(key) && + !(await isServedWithoutLink(ds)) + ) { + newLinksByKey.set(key, ds); + } + } + const newLinks = [...newLinksByKey.values()]; const autoLinksCoverThis = areDataLinksAutomatic && dataLinkSubpathMode !== 'custom'; return { - newLinkCount, - needsLinkConsent: !autoLinksCoverThis && newLinkCount > 0 + newLinks, + needsLinkConsent: !autoLinksCoverThis && newLinks.length > 0 }; }; - const startCreateView = ( + // Checking for external URLs can fetch, so the start* entry points are + // async; `pending` covers the check and whatever `then` awaits. + const withConsent = ( datasets: CartItem[], - defaultName: string, - onCreated?: (view: View) => void + then: ( + consent: Awaited> + ) => void | Promise, + failMessage: string, + onAbort?: () => void ) => { if (datasets.length === 0) { toast.error('Nothing to add'); + onAbort?.(); return; } - setName(defaultName); - setRequest({ - datasets, - ...linkConsent(datasets), - mode: 'create', - onCreated - }); + setPending(true); + linkConsent(datasets) + .then(then) + .catch(error => { + onAbort?.(); + toast.error(error instanceof Error ? error.message : failMessage); + }) + .finally(() => setPending(false)); }; + const startCreateView = ( + datasets: CartItem[], + defaultName: string, + onCreated?: (view: View) => void + ) => + withConsent( + datasets, + consent => { + setName(defaultName); + setRequest({ datasets, ...consent, mode: 'create', onCreated }); + }, + 'Checkout failed' + ); + /** Opens a View in the viewer without saving it; consent dialog only if needed. */ const startDraftView = (datasets: CartItem[], name: string) => { - if (datasets.length === 0) { - toast.error('Nothing to add'); - return; - } - const consent = linkConsent(datasets); - if (!consent.needsLinkConsent) { - setPending(true); - openDraft(datasets, name) - .catch(error => - toast.error(error instanceof Error ? error.message : 'Open failed') - ) - .finally(() => setPending(false)); - return; - } - setRequest({ datasets, ...consent, mode: 'draft', name }); + const tab = openTab(); + withConsent( + datasets, + consent => { + if (!consent.needsLinkConsent) { + return openDraft(datasets, name, undefined, tab); + } + // The dialog's Continue click opens a fresh tab. + tab?.close(); + setRequest({ datasets, ...consent, mode: 'draft', name }); + }, + 'Open failed', + () => tab?.close() + ); }; const startAddToView = ( datasets: CartItem[], hasMismatch: boolean, - add: (datasets: CartItem[]) => Promise - ) => { - if (datasets.length === 0) { - toast.error('Nothing to add'); - return; - } - const consent = linkConsent(datasets); - if (!consent.needsLinkConsent && !hasMismatch) { - // Nothing to ask, and unlike Create View there's no name to pick. - setPending(true); - add(datasets) - .catch(error => - toast.error(error instanceof Error ? error.message : 'Add failed') - ) - .finally(() => setPending(false)); - return; - } - setRequest({ datasets, ...consent, mode: 'add', add, hasMismatch }); - }; + add: (datasets: CartItem[], urlPrefixes?: UrlPrefixes) => Promise + ) => + withConsent( + datasets, + consent => { + if (!consent.needsLinkConsent && !hasMismatch) { + // Nothing to ask, and unlike Create View there's no name to pick. + return add(datasets); + } + setRequest({ datasets, ...consent, mode: 'add', add, hasMismatch }); + }, + 'Add failed' + ); + // Automatic links in custom naming mode: consent was already given, only + // the names are missing (mirrors DataLinkDialog's isAutoCustom branch). + const autoCustom = areDataLinksAutomatic && askNames; const open = request !== null; const creating = request?.mode === 'create'; const drafting = request?.mode === 'draft'; @@ -255,27 +314,47 @@ export function useCreateViewFlow() { {request.needsLinkConsent ? ( <> - Are you sure you want to create a data link? + {autoCustom + ? `Set data link name${request.newLinks.length === 1 ? '' : 's'}` + : 'Are you sure you want to create a data link?'} - This will create {request.newLinkCount} data link - {request.newLinkCount === 1 ? '' : 's'} + This will create {request.newLinks.length} data link + {request.newLinks.length === 1 ? '' : 's'} {creating || drafting ? ' and 1 View' : ''}. If you share the data link(s) with internal collaborators, they will be able to view these data. -
- - Don't ask me this again: - - { - void toggleAutomaticDataLinks(); - }} - /> -
+ {autoCustom ? null : ( +
+ + Don't ask me this again: + + { + void toggleAutomaticDataLinks(); + }} + /> +
+ )} + {askNames + ? request.newLinks.map(ds => { + const key = datasetKey(ds.fsp_name, ds.path); + return ( + + setUrlPrefixes(prev => ({ ...prev, [key]: value })) + } + value={urlPrefixes[key] ?? ''} + /> + ); + }) + : null} ) : null} @@ -294,7 +373,11 @@ export function useCreateViewFlow() {
0 + } loading={pending} loadingText={creating ? 'Creating...' : 'Adding...'} onClick={() => void runRequest()} diff --git a/frontend/src/hooks/useViewSources.ts b/frontend/src/hooks/useViewSources.ts index c6c740b51..8e7e324ef 100644 --- a/frontend/src/hooks/useViewSources.ts +++ b/frontend/src/hooks/useViewSources.ts @@ -1,4 +1,5 @@ -import { useEffect, useMemo, useState } from 'react'; +import { useCallback, useEffect, useMemo, useState } from 'react'; +import { useQueries } from '@tanstack/react-query'; import { dataLinkSegment, @@ -9,8 +10,14 @@ import { bucketDatasetUrl, datasetKey } from '@/utils/pathHandling'; import { isReadOnlyFsp } from '@/utils/cloudStorage'; import { makeMapKey } from '@/utils'; import useZoneAndFileSharePathMapQuery from '@/queries/zoneAndFileSharePathMapQuery'; +import { + externalBucketQueryKeys, + fetchExternalBucket, + transformBucketToDataUrl +} from '@/queries/externalBucketQueries'; import type { NeuroglancerBridge } from '@/hooks/useNeuroglancerViewer'; import type { ViewLayer } from '@/queries/viewQueries'; +import type { ExternalBucket } from '@/queries/externalBucketQueries'; import type { FileSharePath } from '@/shared.types'; import type { NgLayer, ResolvedCheckoutDataset } from '@/utils/viewCheckout'; @@ -214,9 +221,49 @@ export function useViewSources( }; }, [status, getState, subscribe]); - // A saved bucket layer has no Data Link; its segment is the dataset's - // public URL, built the same way checkout built the layer's source. + // A saved layer without a Data Link is in a public bucket or an externally + // mapped path; its segment is the dataset's public URL, built the same way + // checkout built the layer's source. const fspMap = useZoneAndFileSharePathMapQuery().data; + const fspOf = useCallback( + (fspName: string) => + fspMap?.[makeMapKey('fsp', fspName)] as FileSharePath | undefined, + [fspMap] + ); + const externalFspNames = [ + ...new Set( + viewLayers + .filter( + l => + l.sharing_key === null && + l.fsp_name !== null && + !isReadOnlyFsp(fspOf(l.fsp_name)) + ) + .map(l => l.fsp_name!) + ) + ].sort(); + // A stable combine keeps the Map, and so the rows below, from changing on + // every render. + const externalKey = externalFspNames.join('\n'); + const combineBuckets = useCallback( + (results: { data?: ExternalBucket | null }[]) => + new Map( + (externalKey ? externalKey.split('\n') : []).map((name, i) => [ + name, + results[i]?.data ?? null + ]) + ), + [externalKey] + ); + const externalBuckets = useQueries({ + queries: externalFspNames.map(fspName => ({ + queryKey: externalBucketQueryKeys.byFsp(fspName), + queryFn: ({ signal }: { signal: AbortSignal }) => + fetchExternalBucket(fspName, signal), + staleTime: 5 * 60 * 1000 + })), + combine: combineBuckets + }); const { known, unsupported } = useMemo(() => { const saved: KnownDataset[] = []; const unsupported: KnownDataset[] = []; @@ -228,10 +275,14 @@ export function useViewSources( if (l.sharing_key !== null && l.url_prefix !== null) { segment = dataLinkSegment(l.sharing_key, l.url_prefix); } else if (l.sharing_key === null && !l.broken) { - const fsp = fspMap?.[makeMapKey('fsp', l.fsp_name)] as - | FileSharePath - | undefined; - segment = isReadOnlyFsp(fsp) ? bucketDatasetUrl(fsp!, l.path) : null; + const fsp = fspOf(l.fsp_name); + segment = isReadOnlyFsp(fsp) + ? bucketDatasetUrl(fsp!, l.path) + : transformBucketToDataUrl( + externalBuckets.get(l.fsp_name) ?? null, + l.path, + l.fsp_name + ); } if (segment === null) { continue; @@ -264,7 +315,14 @@ export function useViewSources( known: [...saved, ...linked], unsupported: [...unsupported, ...pending] }; - }, [viewLayers, added, pendingSources, removedSources, fspMap]); + }, [ + viewLayers, + added, + pendingSources, + removedSources, + fspOf, + externalBuckets + ]); return useMemo( () => groupLayersBySource(layers, known, unsupported), diff --git a/frontend/src/omezarr-helper.ts b/frontend/src/omezarr-helper.ts index ecb03d9e8..3293f816b 100644 --- a/frontend/src/omezarr-helper.ts +++ b/frontend/src/omezarr-helper.ts @@ -797,9 +797,11 @@ async function getZarrArray( dataUrl: string, zarrVersion: 2 | 3 ): Promise> { + // 'same-origin', not 'include': public buckets answer CORS with + // Allow-Origin '*', which browsers reject for credentialed requests. const store = new zarr.FetchStore(dataUrl, { overrides: { - credentials: 'include' + credentials: 'same-origin' } }); return await omezarr.getArray(store, '/', zarrVersion); @@ -811,7 +813,7 @@ async function getZarrArray( async function getOmeZarrMetadata(dataUrl: string): Promise { const store = new zarr.FetchStore(dataUrl, { overrides: { - credentials: 'include' + credentials: 'same-origin' } }); const { arr, shapes, multiscale, omero, scales, zarr_version } = @@ -946,7 +948,7 @@ async function getOmeZarrThumbnail( ): Promise { const store = new zarr.FetchStore(dataUrl, { overrides: { - credentials: 'include', + credentials: 'same-origin', signal } }); diff --git a/frontend/src/queries/externalBucketQueries.ts b/frontend/src/queries/externalBucketQueries.ts index 10b1f5abe..2dc2e687d 100644 --- a/frontend/src/queries/externalBucketQueries.ts +++ b/frontend/src/queries/externalBucketQueries.ts @@ -21,7 +21,7 @@ export const externalBucketQueryKeys = { * Fetches an external bucket by FSP name * Returns null if no external bucket exists (404) */ -const fetchExternalBucket = async ( +export const fetchExternalBucket = async ( fspName: string, signal?: AbortSignal ): Promise => { diff --git a/frontend/src/queries/viewQueries.ts b/frontend/src/queries/viewQueries.ts index 0589f9194..4c1f9a834 100644 --- a/frontend/src/queries/viewQueries.ts +++ b/frontend/src/queries/viewQueries.ts @@ -96,6 +96,9 @@ export const viewQueryKeys = { forDataLink: (sharingKey: string) => ['views', 'forDataLink', sharingKey] as const, forDataLinkAll: () => ['views', 'forDataLink'] as const, + forDataset: (fspName: string, path: string) => + ['views', 'forDataset', fspName, path] as const, + forDatasetAll: () => ['views', 'forDataset'] as const, relinkable: (fspName: string, path: string) => ['views', 'relinkable', fspName, path] as const, state: (readKey: string) => ['views', 'state', readKey] as const @@ -211,6 +214,32 @@ export function useViewsForDataLinkQuery( }); } +/** + * Query hook for the Views with a working layer on a dataset, matched by + * (fspName, path), so datasets without a Data Link are covered. Disabled + * until both are provided. + */ +export function useViewsForDatasetQuery( + fspName?: string, + path?: string +): UseQueryResult { + return useQuery({ + queryKey: viewQueryKeys.forDataset(fspName ?? '', path ?? ''), + queryFn: async ({ signal }) => { + const url = buildUrl('/api/neuroglancer/views/for-dataset', null, { + fsp_name: fspName!, + path: path! + }); + const data = (await sendRequestAndThrowForNotOk(url, 'GET', undefined, { + signal + })) as ViewsResponse; + return data.views ?? []; + }, + // path '' is the FSP root, so test for undefined, not falsiness + enabled: fspName !== undefined && path !== undefined + }); +} + export function useRelinkableViewsQuery( fspName?: string, path?: string @@ -347,6 +376,9 @@ export function useUpdateViewMutation(): UseMutationResult< queryClient.invalidateQueries({ queryKey: viewQueryKeys.forDataLinkAll() }); + queryClient.invalidateQueries({ + queryKey: viewQueryKeys.forDatasetAll() + }); // A name-only rename must not touch the state query: the embedded // viewer's active viewQueryKeys.state(readKey) query would refetch, // changing the iframe src and reloading Neuroglancer (losing the diff --git a/tests/test_database.py b/tests/test_database.py index b7c5bd527..82aa2cfe0 100644 --- a/tests/test_database.py +++ b/tests/test_database.py @@ -1153,3 +1153,42 @@ def test_reconcile_keeps_bucket_layers_as_dataset_rows(db_session): rows = sorted(v.layers, key=lambda l: l.layer_index) assert [(r.fsp_name, r.path, r.channel, r.broken) for r in rows] == [ (None, None, None, False), ("idr", "zarr/img.zarr", "DAPI", False)] + + +def test_reconcile_keeps_externally_mapped_layers_as_dataset_rows(db_session): + db_session.add(ExternalBucketDB(full_path="/nrs/lab/data_external", external_url="https://s3.example.org/lab/", + fsp_name="nrs_lab", relative_path="data_external")) + db_session.commit() + src = "https://s3.example.org/lab/a/img.zarr/|zarr3:" + state = {"layers": [{"name": "img", "source": src}]} + v = create_view(db_session, "u", "e", state, + [{"layer_index": 0, "channel": None, "opts": None, + "fsp_name": "nrs_lab", "path": "data_external/a/img.zarr"}], "read") + + update_view(db_session, v, ng_state=state, proxy_url=PROXY) + assert [(r.fsp_name, r.path, r.broken) for r in v.layers] == [("nrs_lab", "data_external/a/img.zarr", False)] + + +def test_get_views_for_dataset_matches_layers_without_data_links(db_session): + def layer(i, fsp_name, path, channel=None): + return {"layer_index": i, "channel": channel, "opts": None, "fsp_name": fsp_name, "path": path} + + bucket = create_view(db_session, "u", "bucket", {"layers": []}, [layer(0, "idr", "zarr/img.zarr")], "read") + external = create_view(db_session, "u", "ext", {"layers": []}, + [layer(0, "nrs_lab", "data_external/a/img.zarr")], "read") + channels = create_view(db_session, "u", "channels", {"layers": []}, + [layer(0, "idr", "zarr/img.zarr", "DAPI"), layer(1, "idr", "zarr/img.zarr", "GFP")], "read") + create_view(db_session, "other", "theirs", {"layers": []}, [layer(0, "idr", "zarr/img.zarr")], "read") + + found = get_views_for_dataset(db_session, "u", "idr", "zarr/img.zarr") + assert sorted(v.short_key for v in found) == sorted([bucket.short_key, channels.short_key]) + assert [v.short_key for v in get_views_for_dataset(db_session, "u", "nrs_lab", "data_external/a/img.zarr")] == [external.short_key] + + +def test_get_views_for_dataset_skips_broken_layers_and_matches_legacy_root(db_session): + v = create_view(db_session, "u", "root", {"layers": []}, + [{"layer_index": 0, "channel": None, "opts": None, "fsp_name": "nrs", "path": "."}], "read") + assert [x.short_key for x in get_views_for_dataset(db_session, "u", "nrs", "")] == [v.short_key] + v.layers[0].broken = True + db_session.commit() + assert get_views_for_dataset(db_session, "u", "nrs", "") == [] diff --git a/tests/test_ngstate.py b/tests/test_ngstate.py index 442b53f0c..e9bf160d1 100644 --- a/tests/test_ngstate.py +++ b/tests/test_ngstate.py @@ -75,7 +75,8 @@ def test_default_url_prefix(): def test_bucket_source_matches_public_url_and_unquotes_path(): from fileglancer.ngstate import bucket_source - roots = [("idr", "https://idr.s3.amazonaws.com"), ("deep", "https://s3.example.org/b/pre%20fix")] + roots = [("idr", "https://idr.s3.amazonaws.com", ""), ("deep", "https://s3.example.org/b/pre%20fix", ""), + ("nrs", "https://s3.example.org/pub/", "data_external")] assert bucket_source({"source": "zarr://https://idr.s3.amazonaws.com/zarr/v0.4/my%20img.zarr|zarr2:"}, roots) == ( "idr", "zarr/v0.4/my img.zarr") # the longest root wins; the bucket root itself is the empty path @@ -84,3 +85,7 @@ def test_bucket_source_matches_public_url_and_unquotes_path(): assert bucket_source({"source": "https://idr.s3.amazonaws.com.evil/x"}, roots) is None assert bucket_source({"source": "s3://idr/x.zarr"}, roots) is None assert bucket_source({"type": "annotation"}, roots) is None + # an externally mapped path: the URL's path sits under the mapped folder + assert bucket_source({"source": "https://s3.example.org/pub/a/b.zarr/|zarr3:"}, roots) == ( + "nrs", "data_external/a/b.zarr") + assert bucket_source({"source": "https://s3.example.org/pub"}, roots) == ("nrs", "data_external")