diff --git a/.env.example b/.env.example index 03b6ae69..6a85f21f 100644 --- a/.env.example +++ b/.env.example @@ -40,4 +40,14 @@ STACKABLE_COCKPIT_OIDC_CLIENT_SECRET=your-client-secret # Feature flags # STACKABLE_COCKPIT_COMPLETION_ENABLED=false # Disable SQL editor code completion (default: true) # STACKABLE_COCKPIT_STORAGE_BROWSER_ENABLED=true # Enable S3/HDFS file browser (default: false) +# STACKABLE_COCKPIT_ARCHIVE_PREVIEW_MAX_MB=100 # Max decompressed size for archive preview in megabytes (default: 100) +# STACKABLE_COCKPIT_PARQUET_PREVIEW_DISALLOWED_COMPRESSION_TYPES=gzip-no_offset # Comma-separated list of compression types to disallow from parquet data preview (default: "gzip-no_offset") +# PUBLIC_STACKABLE_COCKPIT_STORAGE_AUTO_CONNECT_TIMEOUT_MS=15000 # Auto-connect to last used storage after this many milliseconds (default: 15000) +# PUBLIC_STACKABLE_COCKPIT_STORAGE_RESTORE_TABS=true # File browser: continue where you left off — restore open tabs from the last session (default: false) +# PUBLIC_STACKABLE_COCKPIT_MAX_EDITABLE_FILE_SIZE=1048576 # Max file size (in bytes) of editable text files in the storage browser (default: 5 MiB) # PUBLIC_STACKABLE_COCKPIT_UPLOAD_CONCURRENCY=3 # Maximum number of concurrent file uploads (default: 3) +# PUBLIC_STACKABLE_COCKPIT_INFINITE_SCROLL_ENABLED=true # Enable infinite scroll in the storage browser (default: true) +# PUBLIC_STACKABLE_COCKPIT_STORAGE_CUT_COPY_ENABLED=true # Enables cut/copy/paste functionality in the storage browser (default: false) +# PUBLIC_STACKABLE_COCKPIT_STORAGE_PASTE_ENABLED=true # Enables paste functionality in the storage browser (default: false) +# PUBLIC_STACKABLE_COCKPIT_STORAGE_RENAME_ENABLED=true # Enables rename functionality in the storage browser (default: false) +# PUBLIC_STACKABLE_COCKPIT_STORAGE_MOVE_ENABLED=true # Enables move functionality in the storage browser (default: false) diff --git a/.env.test b/.env.test index c1b50cfe..766f4835 100644 --- a/.env.test +++ b/.env.test @@ -11,10 +11,21 @@ STACKABLE_COCKPIT_IMAGE_PREVIEW_BYTES=5242880 STACKABLE_COCKPIT_PDF_PREVIEW_BYTES=26214400 STACKABLE_COCKPIT_FILE_PREVIEW_ROWS=250 STACKABLE_COCKPIT_FILE_PREVIEW_COLUMNS=50 +STACKABLE_COCKPIT_PARQUET_PREVIEW_DISALLOWED_COMPRESSION_TYPES=gzip-no_offset PUBLIC_STACKABLE_COCKPIT_STORAGE_AUTO_CONNECT=true +PUBLIC_STACKABLE_COCKPIT_STORAGE_RESTORE_TABS=true PUBLIC_STACKABLE_COCKPIT_PAGE_SIZES=25,50,100 PUBLIC_STACKABLE_COCKPIT_DEFAULT_PAGE_SIZE=25 PUBLIC_STACKABLE_COCKPIT_MAX_RECENT_FILES=15 PUBLIC_STACKABLE_COCKPIT_UPLOAD_CONCURRENCY=3 +PUBLIC_STACKABLE_COCKPIT_INFINITE_SCROLL_ENABLED=true +PUBLIC_STACKABLE_COCKPIT_STORAGE_CUT_COPY_ENABLED=true +PUBLIC_STACKABLE_COCKPIT_STORAGE_PASTE_ENABLED=true +PUBLIC_STACKABLE_COCKPIT_STORAGE_RENAME_ENABLED=true +PUBLIC_STACKABLE_COCKPIT_STORAGE_MOVE_ENABLED=true GARAGE_ADMIN_URL=http://localhost:30902 GARAGE_ADMIN_TOKEN=stackable-cockpit-e2e-admin-token +BETTER_AUTH_URL=http://localhost:4173 +BETTER_AUTH_SECRET=stackable-cockpit-e2e-better-auth-secret +STORAGE_ENCRYPTION_KEY=a781775ee543107bfa97191691bb23c982dd9d9550542df27194ebc09d617dfd +PORT=4173 diff --git a/.github/workflows/pr_checks.yaml b/.github/workflows/pr_checks.yaml index f1b7216b..399cd25d 100644 --- a/.github/workflows/pr_checks.yaml +++ b/.github/workflows/pr_checks.yaml @@ -59,7 +59,7 @@ jobs: run: python -m pip install pre-commit - name: Run pre-commit - run: pre-commit run --all-files --show-diff-on-failure --color=always + run: pre-commit run --all-files --hook-stage manual --show-diff-on-failure --color=always unit-tests: name: Unit Tests @@ -85,10 +85,9 @@ jobs: - name: Run unit tests run: npm run test:unit - e2e-with-garage: - name: E2E Tests with Garage S3 + e2e-tests: + name: E2E Tests with Garage S3 and PostgreSQL runs-on: ubuntu-latest - steps: - name: Checkout uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 @@ -107,35 +106,13 @@ jobs: - name: Install Playwright browsers run: npx playwright install --with-deps chromium firefox - - name: Start Garage S3 and initialize bucket - run: | - # GitHub Actions services now support entrypoint + command, but we still - # start Garage manually here because service containers are created before - # actions/checkout runs. The upstream Garage image contains only the - # /garage binary (no /bin/sh), so the service cannot wait for the checked- - # out e2e/garage.toml to appear before it starts. - docker run -d --name garage \ - -p 3900:3900 \ - -p 3902:3902 \ - -e GARAGE_CONFIG_FILE=/workspace/dev/garage/garage.toml \ - -v "$PWD:/workspace" \ - --entrypoint /garage \ - oci.stackable.tech/stackable/dxflrs/garage:v2.3.0 \ - server --single-node - - # Wait for the S3 API to become available - for _ in $(seq 1 30); do - if curl -s http://localhost:3900 >/dev/null 2>&1; then - break - fi - sleep 1 - done - - S3_SECRET_ACCESS_KEY="e2e-test-secret-key-for-ci" \ - GARAGE_ADMIN_TOKEN='stackable-cockpit-e2e-admin-token' \ - S3_ENDPOINT='http://localhost:3900' \ - S3_CONFIG_PATH="$PWD/s3-config.json" \ - ./e2e/init-garage-s3.sh - - - name: Run E2E tests with Garage - run: npm run test:e2e:garage + - name: Run E2E tests with Garage and PostgreSQL + run: npm run test:e2e + + - name: Upload Playwright traces + if: always() + uses: actions/upload-artifact@v7 + with: + name: playwright-traces + path: e2e/test-results/ + retention-days: 7 diff --git a/.gitignore b/.gitignore index 871677f2..aa194c48 100644 --- a/.gitignore +++ b/.gitignore @@ -40,6 +40,7 @@ coverage/* # Playwright e2e/test-results e2e/.auth +.playwright/ # Paraglide src/lib/paraglide project.inlang/cache/ diff --git a/.nvmrc b/.nvmrc new file mode 100644 index 00000000..32f8c50d --- /dev/null +++ b/.nvmrc @@ -0,0 +1 @@ +24.13.1 diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 239c27d2..1f71bd65 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -43,27 +43,45 @@ repos: - repo: local hooks: + # ── Fix variant (runs locally on commit — auto-fixes where possible) ── + - id: npm-lint-fix + name: npm lint with --fix (prettier + eslint) + language: system + entry: npm run lint:fix + stages: [pre-commit, pre-merge-commit] + pass_filenames: false + files: \.(svelte|ts|js|css|html|json|md)$ + + # ── Check-only variant (runs in CI — no auto-fix) ── - id: npm-lint - name: npm lint (prettier + eslint) + name: npm lint check only (prettier + eslint) language: system entry: npm run lint - stages: [pre-commit, pre-merge-commit] + stages: [manual] pass_filenames: false files: \.(svelte|ts|js|css|html|json|md)$ + # ── Shared hooks (same behaviour locally and in CI) ── - id: npm-check name: npm check (svelte-check) language: system entry: npm run check - stages: [pre-commit, pre-merge-commit] + stages: [pre-commit, pre-merge-commit, manual] pass_filenames: false files: \.(svelte|ts|js)$ + - id: npm-arch-test + name: npm arch test (fitness functions) + language: system + entry: npm run test:arch + stages: [pre-commit, pre-merge-commit, manual] + pass_filenames: false + - id: helm-lint name: helm lint language: system entry: helm lint deploy/helm/cockpit - stages: [pre-commit, pre-merge-commit] + stages: [pre-commit, pre-merge-commit, manual] pass_filenames: false files: ^deploy/helm/ @@ -76,6 +94,6 @@ repos: --set auth.oidc.discoveryUrl=https://stub --set auth.oidc.clientId=stub --set auth.oidc.clientSecret.secretKeyRef.name=stub > /dev/null' - stages: [pre-commit, pre-merge-commit] + stages: [pre-commit, pre-merge-commit, manual] pass_filenames: false files: ^deploy/helm/ diff --git a/.prettierignore b/.prettierignore index 6dbc7f7c..f2eaf48d 100644 --- a/.prettierignore +++ b/.prettierignore @@ -33,3 +33,4 @@ coverage/ # Helm templates deploy/helm/ dev/garage/ +dev/postgresql/ diff --git a/AGENTS.md b/AGENTS.md index c6b6ddbc..bc575200 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -189,10 +189,10 @@ The application uses **Paraglide-JS v2** for type-safe, compiler-based internati ### Node.js Version -The required Node.js version is pinned in `.node-version`. Use `nvm` to install and activate it before running any `npm` commands — `npm` will hard-fail with engine errors otherwise (`.npmrc` sets `engine-strict=true`). +The required Node.js version is pinned in `.node-version` for CI and `.nvmrc` for NVM. Keep both files synchronised. Use `nvm` to install and activate it before running any `npm` commands — `npm` will hard-fail with engine errors otherwise (`.npmrc` sets `engine-strict=true`). ```bash -nvm install # installs the version from .node-version +nvm install # installs the version from .nvmrc nvm use # activates it in the current shell ``` @@ -225,6 +225,37 @@ pre-commit run --all-files # Run all checks: lint (prettier + eslint), type ch # markdownlint, yamllint, shellcheck, actionlint, hadolint, helm lint ``` +### Architecture Fitness Functions + +```bash +npm run test:arch # Run all architecture fitness function tests (~5 s) +npm run test:arch:report # Same, plus generate HTML dependency/metrics reports in /reports/ +``` + +Architecture tests live in `src/architecture/*.spec.ts` and use [ArchUnitTS](https://github.com/LukasNiessen/ArchUnitTS) together with plain Node.js `fs` checks. They run in CI and must remain **green at all times**. + +**Run `npm run test:arch` whenever you:** + +- Add a new file to `src/lib/server/` (verify it doesn't break client-boundary rules) +- Add a new Svelte component (PascalCase naming, no raw ``, no hardcoded colours) +- Add or remove message keys in `messages/en.json` (both locale files must stay in sync) +- Refactor the Trino sub-layer (circular-dependency rules) + +**What the fitness functions enforce:** + +| Category | What is checked | +| ------------------------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Server / Client Boundary | `src/lib/client`, `stores`, `storage`, `editor`, `types` must not import `src/lib/server/**` | +| No Circular Dependencies | `src/lib/**` (excluding Trino, see TECH_DEBT.md) and `src/routes/**` must be cycle-free | +| Naming Conventions | Stores → `*.svelte.ts`; lib components → PascalCase `.svelte`; server files → no `.svelte.ts` extension | +| Code Size Limits | `.ts` files < 2 400 LOC; `.svelte` files < 1 100 LOC; test files < 1 000 LOC | +| UI Pattern Enforcement | No hardcoded Tailwind colours; no raw ``; no native date inputs; `` must have `alt`; no `
` (known violations listed in TECH_DEBT.md) | +| Server Logging | Server files must use pino logger, not `console.*` | +| i18n Compliance | `messages/en.json` and `messages/de.json` must have the same keys; no static `aria-label="English text"` (known violations listed in TECH_DEBT.md) | + +**Extending the fitness functions:** +When you add a new architectural rule (e.g., a new layer, a new naming convention), add a new `.spec.ts` file in `src/architecture/` following the existing patterns. Use archunit for TypeScript dependency/cycle rules and plain Node.js `fs` for content checks on Svelte files. + ### E2E Testing ```bash diff --git a/TECH_DEBT.md b/TECH_DEBT.md index 63591e87..f85c17cc 100644 --- a/TECH_DEBT.md +++ b/TECH_DEBT.md @@ -30,19 +30,19 @@ When `tls.verification` is set to `'None'`, the S3 client is created with `rejec --- -### S3 connection credentials stored in localStorage +### ~~S3 connection credentials stored in localStorage~~ — RESOLVED -**File:** `src/lib/storage/connection-storage.ts`, `src/lib/components/storage/StorageConnectForm.svelte` - -S3 connection credentials (access key ID and secret access key) are persisted in plaintext `localStorage` so the browser can auto-reconnect after a page reload or server restart. `localStorage` is accessible to any JavaScript running on the page and is visible in browser DevTools, making it vulnerable to XSS. Acceptable for the current early stage where the alternative is users having to re-enter credentials after every server restart. Long-term fix: persist encrypted credentials server-side, tied to the authenticated session; send only a session token to the client. +Previously, S3 connection credentials were persisted in plaintext `localStorage`. This has been replaced by a server-side encrypted credential store backed by PostgreSQL. Credentials are encrypted with AES-256-GCM using a per-deployment application key (`STORAGE_ENCRYPTION_KEY`). The client now sends only a connection UUID (`x-storage-connection-id` header); the server decrypts and creates the S3 client. See `src/lib/server/storage/encryption.ts` and `src/lib/server/storage/connection.ts`. --- -### Download endpoint buffers entire object in browser memory +### Download token store is in-memory only + +**File:** `src/lib/server/storage/download-tokens.ts` -**File:** `src/lib/storage/download.ts` +The `downloadObject` function no longer buffers objects in browser memory. It now uses a token exchange flow: a HEAD pre-flight validates access, the client exchanges the connection header for a short-lived token via `POST /api/storage/download/token`, then navigates to the download URL with that token. The browser streams the object directly to disk — no JavaScript-side buffering. -`downloadObject` fetches the full S3 object body via the `/storage/api/download` endpoint, buffers it as a `Blob` in browser memory, then triggers a programmatic anchor click. This is simpler than streaming directly to disk but means the entire object must fit in browser memory before the save dialog appears. Acceptable for the current object sizes; for very large files (multiple GiB) this will cause memory pressure. Long-term fix: use the [File System Access API](https://developer.mozilla.org/en-US/docs/Web/API/File_System_Access_API) `createWritable()` to stream bytes directly to disk without buffering, with a fallback to the current Blob approach for Firefox (which does not support `showSaveFilePicker`). +The token store is an in-memory `Map`. Tokens expire after 60 seconds and are single-use. In a multi-process or serverless deployment, the in-memory store would not be shared across instances. The long-term fix is to use a shared store (Redis or signed JWTs that need no server-side state). --- @@ -136,9 +136,77 @@ The dev server accepts requests from any host. This enables DNS rebinding attack --- +### Archive browsing downloads entire file before parsing + +**File:** `src/lib/server/storage/archive.ts` + +Archive browsing (ZIP, TAR.GZ, RAR, 7z) downloads the entire archive from S3 to a temporary file on the server before listing or extracting entries. The feasibility of partial/random-access reading depends entirely on the archive format. + +#### ZIP (fixable with S3 Range requests) + +ZIP stores a **Central Directory** (full file listing) at the end of the file, preceded by an **EOCD** (End of Central Directory) record. This allows true random access: + +1. **HEAD** the S3 object to get `Content-Length` +2. **Range request for the last ~100 bytes** → parse EOCD to locate the Central Directory +3. **Range request for the Central Directory** → get full file listing (typically <50 KB) +4. **To extract one file**: single Range request to that file's compressed data offset + +Total data transferred for listing: 2 Range requests, often under 50 KB regardless of archive size. Extraction costs only what the user opens — no temp file I/O needed for ZIPs at all. + +Available libraries with S3 Range support: + +- **`unzipper`** (`ZJONSSON/node-unzipper`) — `Open.s3()` for AWS SDK v2; `Open.custom()` with S3 Range workaround for v3 (see [issue #241](https://github.com/ZJONSSON/node-unzipper/issues/241)). Most mature option. +- **`s3-range-zip`** (`numtel/s3-range-zip`) — Purpose-built for S3 Range-based ZIP reading with `@aws-sdk/client-s3`. Smaller and simpler. +- **`unzipit`** (`greggman/unzipit`) — `HTTPRangeReader` requires presigned S3 URLs; better suited for browser use. + +**Recommendation**: Replace `adm-zip` with `unzipper.Open.custom()` using the AWS SDK v3 workaround. This eliminates download latency for ZIPs entirely (the most common archive format) and naturally supports nested archives (inner ZIP's central directory read the same way). Implementation steps: + +1. Install `unzipper` npm package, remove `adm-zip` +2. Implement S3 custom reader using `HeadObjectCommand` + `GetObjectCommand` with `Range` header +3. Replace `listZip()` with central directory parsing (no extraction needed for listing) +4. Replace `extractFromZip()` with offset-based single-file extraction +5. Keep `extractFromZip` as fallback name but implement via `unzipper` +6. Remove ZIP from temp-file caching (TAR.GZ/RAR/7z still need it) +7. Remove `adm-zip` dependency + +#### TAR.GZ (fundamentally limited — format constraint) + +TAR has no central index (sequential tape format). Gzip is a streaming compressor. Together they force full sequential decompression from the start. Three tiers of mitigation exist: + +- **Stream + skip** (Node.js `node-tar` + `zlib.createGunzip()`): decompress everything but avoid writing to disk. This is what the current approach effectively does after download. Bandwidth + CPU cost same as full download. +- **Index-on-first-pass** (e.g. Rust [`iluvatar`](https://docs.rs/crate/iluvatar/latest)): records decompressor checkpoints on first pass, subsequent extractions restore nearest checkpoint and seek forward. No Node.js equivalent exists. +- **Specialised format** ([`estargz`](https://github.com/containerd/stargz-snapshotter), [`tarzan`](https://github.com/astraw/tarzan-rs)): archives must be created in these formats; existing `.tar.gz` files cannot be retrofitted. + +**Recommendation**: Keep current full-download + temp cache. This is unavoidable for arbitrary `.tar.gz` files. + +#### RAR & 7z (limited — solid compression blocks) + +Both use solid compression where a single file's data may be interleaved across a compressed block. Without format-level block-to-file index (which neither exposes easily), extracting one file requires decompressing the entire solid block. + +**Recommendation**: Keep full-download + temp cache. + +#### Summary + +| Format | Partial access | Approach | Bandwidth per listing | +| ------ | -------------- | ----------------------- | ---------------------- | +| ZIP | ✅ Yes | S3 Range via `unzipper` | ~50 KB (2 Range calls) | +| TAR.GZ | ❌ No | Full download + cache | Full archive | +| RAR | ❌ No | Full download + cache | Full archive | +| 7z | ❌ No | Full download + cache | Full archive | + +--- + +### RAR and 7z archive support requires system binaries + +**File:** `src/lib/server/storage/archive.ts` + +RAR and 7z archive parsing shells out to `unrar` and `7zz`/`7z` system binaries respectively. These may not be installed in the production container image. If absent, the user sees a clear error message telling them to install the binary. ZIP and TAR.GZ work without system dependencies (pure JS). The Dockerfile should be updated to include `unrar` and `p7zip` (or similar) packages when RAR/7z support is needed in production. + +--- + ### No server-side file size limit on uploads (v0) -**File:** `src/routes/(app)/storage/api/upload/+server.ts` +**File:** `src/routes/(app)/api/storage/upload/+server.ts` The upload endpoint imposes no maximum file size. S3's 5 TB single-object limit applies as a natural backstop. For v0 this is acceptable; large uploads will consume server-side streaming resources proportionally but do not buffer the body in memory (the stream is piped directly to the `@aws-sdk/lib-storage` Upload). Add a configurable `MAX_UPLOAD_BYTES` guard in a future iteration once typical object sizes are known. @@ -149,3 +217,77 @@ The upload endpoint imposes no maximum file size. S3's 5 TB single-object limit **File:** `src/routes/healthz/+server.ts`, `deploy/helm/cockpit/values.yaml` Both `livenessProbe` and `readinessProbe` point at `/healthz`, which always returns 200. There is currently nothing meaningful to gate readiness on (better-auth uses an in-memory session store, OIDC discovery is fetched lazily on first auth call), so a separate `/readyz` would just be a placeholder. Once one of these lands — a real session store / DB, eager OIDC discovery, or a startup-time cache warm — split into `/healthz` (liveness, trivial) and `/readyz` (readiness, checking the new dependency), and update the helm probes accordingly. + +--- + +## Architecture (Tracked by fitness functions in src/architecture/\*.spec.ts) + +### Circular imports in the Trino server layer + +**Files:** `src/lib/server/trino/client.ts` ↔ `src/lib/server/trino/user-clients.ts` and `src/lib/server/trino/queries.ts` ↔ `src/lib/server/trino/result-collector.ts` + +Two pairs of circular imports exist in the Trino sub-layer. They cause unpredictable module initialisation order and block reliable tree-shaking. The fitness function `KNOWN VIOLATION — Trino layer has circular imports` documents this and will fail the moment the cycle count drops to zero (prompting removal of the exception). + +**Fix:** Extract shared interfaces (`TrinoQuery`, `QueryState`, etc.) into `src/lib/server/trino/types.ts`. Neither `client.ts` nor `user-clients.ts` should import from each other — both should import types from the new file. + +--- + +### `state.svelte.ts` is too large (2 331 LOC) + +**File:** `src/lib/storage/state.svelte.ts` + +This single file contains the entire client-side storage state machine, all reactive derived values, and all event handlers. It is the primary obstacle to the 600-line LOC fitness function threshold. The fitness function currently allows up to 2 400 lines. + +**Fix:** Split into focused modules: + +- `state/selection.svelte.ts` — selected items, range-selection logic +- `state/upload.svelte.ts` — upload job tracking +- `state/navigation.svelte.ts` — current path, history +- `state/operations.svelte.ts` — copy/move/delete operation state +- `state/index.svelte.ts` — re-exports and glue + +--- + +### `PreviewModal.svelte` is too large (1 065 LOC) + +**File:** `src/lib/components/storage/modals/PreviewModal.svelte` + +This Svelte component mixes routing logic, multiple preview renderers, and toolbar state in one file. The fitness function currently allows up to 1 100 lines. + +**Fix:** Extract preview renderers into separate child components (`CsvRenderer.svelte`, `ParquetRenderer.svelte`, `TextRenderer.svelte`, etc.) and pass the selected object as a prop. + +--- + +### Pre-existing `
` patterns (BITV 2.0 violation) + +**Files:** + +- `src/lib/components/catalog/CatalogTree.svelte` — tree node rows +- `src/lib/components/storage/explorer/OperationsButton.svelte` — operation rows +- `src/lib/components/storage/explorer/TabBar.svelte` — tab items +- `src/lib/components/storage/shared/FloatingMenu.svelte` — portal overlay +- `src/lib/components/trino/StatementResult.svelte` — result rows + +Using `
` instead of ` +
+ + + {#if dropdownOpen} + + {/if} +
+{/if} diff --git a/src/lib/components/storage/explorer/SelectionToolbar.svelte b/src/lib/components/storage/explorer/SelectionToolbar.svelte index 91dd742f..693598e3 100644 --- a/src/lib/components/storage/explorer/SelectionToolbar.svelte +++ b/src/lib/components/storage/explorer/SelectionToolbar.svelte @@ -1,6 +1,7 @@ -{#if breadcrumbCtx} - {@const breadcrumbIsPinned = storage.bookmarks.isPinned( - breadcrumbCtx.bucket, - breadcrumbCtx.prefix - )} - {@const BreadcrumbPinIcon = breadcrumbIsPinned ? IconPushPin : IconPushPinOutline} - -
e.key === 'Escape' && closeBreadcrumbCtx()} - >
- -{/if} + {#snippet pinButton(bucket: string, prefix: string)} {@const pinned = storage.bookmarks.isPinned(bucket, prefix)} @@ -102,34 +260,38 @@ motion-safe:transition-[width] motion-safe:duration-150 motion-safe:group-hover:delay-700 " > - + + {/snippet} @@ -141,19 +303,25 @@ flex min-w-0 flex-1 items-center gap-1 text-sm " > - {#if breadcrumbParts.length === 0} + {#if breadcrumbParts.length === 0 && !storage.archive.isInArchive} openBreadcrumbCtx(e, storage.bucket, '')} + ondragover={(e) => handleBreadcrumbDragOver(e, '')} + ondragleave={handleBreadcrumbDragLeave} + ondrop={(e) => handleBreadcrumbDrop(e, '')} > - {@render pinButton(storage.bucket, '')} @@ -165,10 +333,16 @@ text-base-content/70 hover:bg-base-200 hover:text-base-content flex shrink-0 items-center gap-1.5 rounded-sm px-1.5 py-0.5 transition-colors hover:cursor-pointer + {dropTargetPrefix === '' + ? 'bg-primary/20 outline-primary/50 outline -outline-offset-2' + : ''} " title={storage.bucket} - onclick={() => storage.navigate('')} + onclick={() => navigateS3('')} oncontextmenu={(e) => openBreadcrumbCtx(e, storage.bucket, '')} + ondragover={(e) => handleBreadcrumbDragOver(e, '')} + ondragleave={handleBreadcrumbDragLeave} + ondrop={(e) => handleBreadcrumbDrop(e, '')} > {/if} {#if collapsedParts.length > 0} -
diff --git a/src/lib/components/storage/explorer/TabBar.svelte b/src/lib/components/storage/explorer/TabBar.svelte new file mode 100644 index 00000000..60b74c87 --- /dev/null +++ b/src/lib/components/storage/explorer/TabBar.svelte @@ -0,0 +1,402 @@ + + + + +{#if tabsState.hasTabs} +
+ +
+ {#each tabsState.tabs as tab, idx (tab.id)} + {@const isActive = tab.id === tabsState.activeTabId} + {#if renamingId === tab.id} +
+ + +
+ {:else} + + {/if} + {/each} + + + + + +
+ + + {#if isOverflowLeft} + + {/if} + + + {#if isOverflowRight} + + {/if} +
+{/if} + + diff --git a/src/lib/components/storage/explorer/TabBar.svelte.spec.ts b/src/lib/components/storage/explorer/TabBar.svelte.spec.ts new file mode 100644 index 00000000..7324aad1 --- /dev/null +++ b/src/lib/components/storage/explorer/TabBar.svelte.spec.ts @@ -0,0 +1,211 @@ +import { page, userEvent } from 'vitest/browser'; +import { describe, expect, it, vi, beforeEach } from 'vitest'; +import { render } from 'vitest-browser-svelte'; +import TabBarWrapper from './__tests__/TabBarWrapper.svelte'; +import { TabsState } from '$lib/storage/tabs.svelte.js'; +import { StorageState } from '$lib/storage/state.svelte.js'; + +// ── Helpers ────────────────────────────────────────────────────────────────── + +function makeStorage(bucket = 'test-bucket', prefix = ''): StorageState { + const state = new StorageState({ connected: true }); + state.bucket = bucket; + state.prefix = prefix; + state.objects = { objects: [], hasNextPage: false, currentPage: 1, pageSize: 25 }; + return state; +} + +function makeTabsState(storage: StorageState, tabCount = 2): TabsState { + const ts = new TabsState(storage, { persistEnabled: false }); + ts.ensureInitialTab(); + for (let i = 1; i < tabCount; i++) { + ts.addTab(); + } + return ts; +} + +function renderTabBar(storage: StorageState, tabsState: TabsState) { + return render(TabBarWrapper, { storage, tabsState }); +} + +// ── Tests ───────────────────────────────────────────────────────────────────── + +describe('TabBar', () => { + beforeEach(() => { + localStorage.clear(); + }); + + it('renders a tab for each entry in tabsState.tabs', async () => { + const storage = makeStorage('bucket-a'); + const tabsState = makeTabsState(storage, 2); + renderTabBar(storage, tabsState); + + const tabs = page.getByRole('tab'); + // 2 tabs rendered + await expect.element(tabs.nth(0)).toBeInTheDocument(); + await expect.element(tabs.nth(1)).toBeInTheDocument(); + }); + + it('marks the active tab with aria-selected="true"', async () => { + const storage = makeStorage('bucket-a'); + const tabsState = makeTabsState(storage, 2); + renderTabBar(storage, tabsState); + + const activeTab = page.getByRole('tab', { selected: true }); + await expect.element(activeTab).toBeInTheDocument(); + }); + + it('has a tablist with an accessible label', async () => { + const storage = makeStorage(); + const tabsState = makeTabsState(storage, 2); + renderTabBar(storage, tabsState); + + await expect.element(page.getByRole('tablist')).toBeInTheDocument(); + }); + + it('renders a "New Tab" plus button', async () => { + const storage = makeStorage(); + const tabsState = makeTabsState(storage, 2); + renderTabBar(storage, tabsState); + + await expect.element(page.getByRole('button', { name: 'New Tab' })).toBeInTheDocument(); + }); + + it('calls addTab when the plus button is clicked', async () => { + const storage = makeStorage(); + const tabsState = makeTabsState(storage, 2); + const spy = vi.spyOn(tabsState, 'addTab'); + renderTabBar(storage, tabsState); + + await page.getByRole('button', { name: 'New Tab' }).click(); + + expect(spy).toHaveBeenCalled(); + }); + + it('calls switchTo with the correct id when a non-active tab is clicked', async () => { + const storage = makeStorage('bucket', ''); + const tabsState = makeTabsState(storage, 2); + // After makeTabsState(2): tabs[0] is the initial tab (not active), tabs[1] is active + const spy = vi.spyOn(tabsState, 'switchTo'); + renderTabBar(storage, tabsState); + + // Click the first tab (not the active one) + await page.getByRole('tab').nth(0).click(); + + expect(spy).toHaveBeenCalledWith(tabsState.tabs[0].id); + }); + + it('shows close buttons when multiple tabs exist', async () => { + const storage = makeStorage(); + const tabsState = makeTabsState(storage, 2); + renderTabBar(storage, tabsState); + + // Close tab buttons have aria-label="Close tab" + const closeButtons = page.getByRole('button', { name: 'Close tab' }); + await expect.element(closeButtons.first()).toBeInTheDocument(); + }); + + it('calls closeTab when a close button is clicked', async () => { + const storage = makeStorage(); + const tabsState = makeTabsState(storage, 2); + const spy = vi.spyOn(tabsState, 'closeTab'); + renderTabBar(storage, tabsState); + + await page.getByRole('button', { name: 'Close tab' }).first().click(); + + expect(spy).toHaveBeenCalled(); + }); + + it('opens a context menu on right-click of a tab', async () => { + const storage = makeStorage(); + const tabsState = makeTabsState(storage, 2); + renderTabBar(storage, tabsState); + + await page.getByRole('tab').first().click({ button: 'right' }); + + await expect.element(page.getByRole('menu')).toBeInTheDocument(); + }); + + it('context menu contains Rename tab and Close tab options', async () => { + const storage = makeStorage(); + const tabsState = makeTabsState(storage, 2); + renderTabBar(storage, tabsState); + + await page.getByRole('tab').first().click({ button: 'right' }); + + await expect.element(page.getByRole('menuitem', { name: 'Rename tab' })).toBeInTheDocument(); + await expect.element(page.getByRole('menuitem', { name: 'Close tab' })).toBeInTheDocument(); + }); + + it('calls renameTab via context menu rename action', async () => { + const storage = makeStorage(); + const tabsState = makeTabsState(storage, 2); + const spy = vi.spyOn(tabsState, 'renameTab'); + renderTabBar(storage, tabsState); + + const firstTabId = tabsState.tabs[0].id; + await page.getByRole('tab').first().click({ button: 'right' }); + await page.getByRole('menuitem', { name: 'Rename tab' }).click(); + + // The rename input should appear (context menu closes and rename mode starts) + await expect.element(page.getByRole('textbox', { name: 'Rename tab' })).toBeInTheDocument(); + + await page.getByRole('textbox', { name: 'Rename tab' }).fill('Renamed'); + await userEvent.keyboard('{Enter}'); + + expect(spy).toHaveBeenCalledWith(firstTabId, 'Renamed'); + }); + + it('shows rename input on double-click of a tab', async () => { + const storage = makeStorage(); + const tabsState = makeTabsState(storage, 2); + renderTabBar(storage, tabsState); + + await page.getByRole('tab').first().dblClick(); + + await expect.element(page.getByRole('textbox', { name: 'Rename tab' })).toBeInTheDocument(); + }); + + it('commits rename on Enter key', async () => { + const storage = makeStorage(); + const tabsState = makeTabsState(storage, 2); + const spy = vi.spyOn(tabsState, 'renameTab'); + renderTabBar(storage, tabsState); + + const firstTabId = tabsState.tabs[0].id; + await page.getByRole('tab').first().dblClick(); + + await page.getByRole('textbox', { name: 'Rename tab' }).fill('My New Name'); + await userEvent.keyboard('{Enter}'); + + expect(spy).toHaveBeenCalledWith(firstTabId, 'My New Name'); + }); + + it('cancels rename on Escape key without calling renameTab', async () => { + const storage = makeStorage(); + const tabsState = makeTabsState(storage, 2); + const spy = vi.spyOn(tabsState, 'renameTab'); + renderTabBar(storage, tabsState); + + await page.getByRole('tab').first().dblClick(); + + await page.getByRole('textbox', { name: 'Rename tab' }).fill('Something'); + await userEvent.keyboard('{Escape}'); + + expect(spy).not.toHaveBeenCalled(); + await expect.element(page.getByRole('textbox', { name: 'Rename tab' })).not.toBeInTheDocument(); + }); + + it('calls closeTab via context menu close action', async () => { + const storage = makeStorage(); + const tabsState = makeTabsState(storage, 2); + const spy = vi.spyOn(tabsState, 'closeTab'); + const firstTabId = tabsState.tabs[0].id; + renderTabBar(storage, tabsState); + + await page.getByRole('tab').first().click({ button: 'right' }); + await page.getByRole('menuitem', { name: 'Close tab' }).click(); + + expect(spy).toHaveBeenCalledWith(firstTabId); + }); +}); diff --git a/src/lib/components/storage/explorer/__tests__/ContextMenu.svelte.spec.ts b/src/lib/components/storage/explorer/__tests__/ContextMenu.svelte.spec.ts index 2e86509d..6f823e49 100644 --- a/src/lib/components/storage/explorer/__tests__/ContextMenu.svelte.spec.ts +++ b/src/lib/components/storage/explorer/__tests__/ContextMenu.svelte.spec.ts @@ -113,7 +113,7 @@ describe('ContextMenu', () => { selectedKeys: ['test.txt'] }); const spy = vi.spyOn(state, 'closeContextMenu'); - render(ContextMenuWrapper, { state }); + render(ContextMenuWrapper, { state, title: 'Menu' }); await page.getByRole('menuitem', { name: 'Close' }).click(); expect(spy).toHaveBeenCalled(); diff --git a/src/lib/components/storage/explorer/__tests__/ContextMenuWrapper.svelte b/src/lib/components/storage/explorer/__tests__/ContextMenuWrapper.svelte index 8307de87..5defe339 100644 --- a/src/lib/components/storage/explorer/__tests__/ContextMenuWrapper.svelte +++ b/src/lib/components/storage/explorer/__tests__/ContextMenuWrapper.svelte @@ -6,10 +6,11 @@ interface Props { state: StorageState; + title?: string; } - let { state }: Props = $props(); + let { state, title }: Props = $props(); untrack(() => setStorageState(state)); - + diff --git a/src/lib/components/storage/explorer/__tests__/FileExplorerWrapper.svelte b/src/lib/components/storage/explorer/__tests__/FileExplorerWrapper.svelte index 7106df4f..2c577d6b 100644 --- a/src/lib/components/storage/explorer/__tests__/FileExplorerWrapper.svelte +++ b/src/lib/components/storage/explorer/__tests__/FileExplorerWrapper.svelte @@ -1,7 +1,8 @@ diff --git a/src/lib/components/storage/explorer/__tests__/FileRow.svelte.spec.ts b/src/lib/components/storage/explorer/__tests__/FileRow.svelte.spec.ts index 24f0be8c..d0d07a27 100644 --- a/src/lib/components/storage/explorer/__tests__/FileRow.svelte.spec.ts +++ b/src/lib/components/storage/explorer/__tests__/FileRow.svelte.spec.ts @@ -74,7 +74,7 @@ describe('FileRow', () => { const state = createState([file]); render(FileRowWrapper, { state, file }); - await expect.element(page.getByText('unknown-file')).toBeInTheDocument(); + await expect.element(page.getByText('unknown-file').first()).toBeInTheDocument(); }); it('should show checkbox when showCheckboxes is true', async () => { @@ -124,13 +124,62 @@ describe('FileRow', () => { expect(spy).toHaveBeenCalledWith('click.txt', false); }); + async function dblClickRow(): Promise { + const row = page.getByRole('row').element(); + row.dispatchEvent(new MouseEvent('dblclick', { bubbles: true })); + } + it('should call executeAction preview on double click', async () => { const file = makeFile({ key: 'dbl.txt', contentType: 'text/plain' }); const state = createState([file]); const spy = vi.spyOn(state, 'executeAction'); render(FileRowWrapper, { state, file }); - await page.getByRole('row').dblClick(); + await dblClickRow(); + expect(spy).toHaveBeenCalledWith('preview'); + }); + + it('should call enterArchive on double click for zip files', async () => { + const file = makeFile({ key: 'archive.zip', contentType: 'application/zip' }); + const state = createState([file]); + const spy = vi.spyOn(state.archive, 'enterArchive'); + render(FileRowWrapper, { state, file }); + + await dblClickRow(); + expect(spy).toHaveBeenCalledWith('archive.zip'); + }); + + it('should call enterArchive on double click for tar.gz files', async () => { + const file = makeFile({ key: 'bundle.tar.gz', contentType: 'application/gzip' }); + const state = createState([file]); + const spy = vi.spyOn(state.archive, 'enterArchive'); + render(FileRowWrapper, { state, file }); + + await dblClickRow(); + expect(spy).toHaveBeenCalledWith('bundle.tar.gz'); + }); + + it('should call enterArchive on double click for nested archive inside an archive', async () => { + const file = makeFile({ key: 'nested.zip', contentType: undefined }); + const state = createState([file]); + state.archive.archiveKey = 'outer.zip'; + state.archive.archivePrefix = ''; + const spy = vi.spyOn(state.archive, 'enterArchive'); + render(FileRowWrapper, { state, file }); + + await dblClickRow(); + expect(spy).toHaveBeenCalledWith('nested.zip'); + }); + + it('should call executeAction preview on double click for non-archive file inside archive', async () => { + const file = makeFile({ key: 'readme.txt', contentType: 'text/plain' }); + const state = createState([file]); + state.archive.archiveKey = 'outer.zip'; + state.archive.archivePrefix = ''; + const spy = vi.spyOn(state, 'executeAction'); + render(FileRowWrapper, { state, file }); + + await dblClickRow(); expect(spy).toHaveBeenCalledWith('preview'); }); diff --git a/src/lib/components/storage/explorer/__tests__/StorageBreadcrumb.svelte.spec.ts b/src/lib/components/storage/explorer/__tests__/StorageBreadcrumb.svelte.spec.ts index 3bbffaaa..c557470a 100644 --- a/src/lib/components/storage/explorer/__tests__/StorageBreadcrumb.svelte.spec.ts +++ b/src/lib/components/storage/explorer/__tests__/StorageBreadcrumb.svelte.spec.ts @@ -191,16 +191,14 @@ describe('StorageBreadcrumb', () => { }); describe('pin/unpin', () => { - it('should show pin option in more options menu', async () => { + it('should show a pin control for the current location', async () => { const state = createState(); render(StorageBreadcrumbWrapper, { state }); - // The more options button exists - const moreBtn = page.getByRole('button', { name: /more options/i }); - await expect.element(moreBtn).toBeInTheDocument(); + await expect.element(page.getByRole('button', { name: 'Pin' })).toBeInTheDocument(); }); - it('should show unpin when current location is pinned', async () => { + it('should show unpin control when current location is pinned', async () => { const state = createState({ bucket: 'test-bucket', prefix: 'data/', @@ -208,35 +206,20 @@ describe('StorageBreadcrumb', () => { }); render(StorageBreadcrumbWrapper, { state }); - // The more menu should contain unpin - const menuItems = page.getByRole('menuitem'); - await expect.element(menuItems.first()).toBeInTheDocument(); + await expect.element(page.getByRole('button', { name: 'Unpin' })).toBeInTheDocument(); }); - it('should call pin via more options menu when not pinned', async () => { + it('should pin the current location', async () => { const state = createState({ bucket: 'test-bucket', prefix: 'data/' }); const spy = vi.spyOn(state.bookmarks, 'pin'); render(StorageBreadcrumbWrapper, { state }); - // Open the dropdown by focusing/clicking the trigger - const moreBtn = page.getByRole('button', { name: /more options/i }); - await moreBtn.click(); - - // DaisyUI dropdown uses focus to show content; click the menuitem - const menuItems = page.getByRole('menuitem'); - // Force the click by using element() - const el = menuItems.first(); - await el.click(); - - // If DaisyUI dropdown prevents click, try direct dispatch - if (!spy.mock.calls.length) { - const domEl = (await el.element()) as HTMLElement; - domEl.click(); - } + const pinButton = page.getByRole('button', { name: 'Pin' }).last(); + ((await pinButton.element()) as HTMLElement).click(); expect(spy).toHaveBeenCalledWith('test-bucket', 'data/'); }); - it('should call unpin via more options menu when pinned', async () => { + it('should unpin the current location', async () => { const state = createState({ bucket: 'test-bucket', prefix: 'data/', @@ -245,12 +228,8 @@ describe('StorageBreadcrumb', () => { const spy = vi.spyOn(state.bookmarks, 'unpin'); render(StorageBreadcrumbWrapper, { state }); - // Open the dropdown first - const moreBtn = page.getByRole('button', { name: /more options/i }); - await moreBtn.click(); - - const menuItem = page.getByRole('menuitem'); - await menuItem.first().click(); + const unpinButton = page.getByRole('button', { name: 'Unpin' }); + ((await unpinButton.element()) as HTMLElement).click(); expect(spy).toHaveBeenCalledWith('test-bucket', 'data/'); }); }); @@ -350,7 +329,7 @@ describe('StorageBreadcrumb', () => { const bucketEl = nav.getByText('test-bucket'); await bucketEl.click({ button: 'right' }); - const menuItem = page.getByRole('menuitem'); + const menuItem = page.getByRole('menuitem', { name: /pin/i }); await menuItem.first().click(); expect(spy).toHaveBeenCalledWith('test-bucket', ''); }); @@ -367,7 +346,7 @@ describe('StorageBreadcrumb', () => { const bucketEl = nav.getByText('test-bucket'); await bucketEl.click({ button: 'right' }); - const menuItem = page.getByRole('menuitem'); + const menuItem = page.getByRole('menuitem', { name: /unpin/i }); await menuItem.first().click(); expect(spy).toHaveBeenCalledWith('test-bucket', ''); }); @@ -391,9 +370,8 @@ describe('StorageBreadcrumb', () => { const backdrop = menuUl.previousElementSibling as HTMLElement; backdrop.dispatchEvent(new MouseEvent('mousedown', { bubbles: true })); - // Context menu should be closed - menuitem from ctx menu should be gone - // The only remaining menuitem should be from the more options dropdown - await expect.element(page.getByRole('menuitem')).toBeInTheDocument(); // the one in more options + // Context menu should be closed. + await expect.element(page.getByRole('menuitem').first()).not.toBeInTheDocument(); }); it('should open context menu on right-click of bucket with prefix', async () => { @@ -455,4 +433,68 @@ describe('StorageBreadcrumb', () => { await expect.element(moreBtn).not.toBeInTheDocument(); }); }); + + describe('archive mode', () => { + it('should show archive name when inside an archive', async () => { + const state = createState(); + state.archive.archiveKey = 'data.zip'; + render(StorageBreadcrumbWrapper, { state }); + + const nav = page.getByRole('navigation', { name: 'breadcrumb' }); + await expect.element(nav.getByText('data.zip')).toBeInTheDocument(); + }); + + it('should show internal path parts when navigating within archive', async () => { + const state = createState(); + state.archive.archiveKey = 'data.zip'; + state.archive.archivePrefix = 'music/videos/'; + render(StorageBreadcrumbWrapper, { state }); + + const nav = page.getByRole('navigation', { name: 'breadcrumb' }); + await expect + .element(nav.getByRole('button', { name: 'music', exact: true })) + .toBeInTheDocument(); + await expect.element(nav.getByText('videos')).toHaveAttribute('aria-current', 'page'); + }); + + it('should show nested archive entry when browsing nested archive', async () => { + const state = createState(); + state.archive.archiveKey = 'outer.zip'; + state.archive._restoreFullState({ + archiveKey: 'outer.zip', + archivePrefix: 'subdir/', + archiveNestedPath: 'inner.tar', + previousS3Prefix: '', + archiveLoading: false, + archiveTooLarge: false + }); + state.archive.archivePrefix = 'subdir/'; + render(StorageBreadcrumbWrapper, { state }); + + const nav = page.getByRole('navigation', { name: 'breadcrumb' }); + await expect.element(nav.getByText('outer.zip')).toBeInTheDocument(); + await expect.element(nav.getByText('inner.tar')).toBeInTheDocument(); + await expect.element(nav.getByText('subdir')).toBeInTheDocument(); + }); + + it('should call navigateInArchive when clicking breadcrumb folder inside archive', async () => { + const state = createState(); + state.archive.archiveKey = 'data.zip'; + state.archive.archivePrefix = 'music/videos/'; + const spy = vi.spyOn(state.archive, 'navigateInArchive'); + render(StorageBreadcrumbWrapper, { state }); + + const nav = page.getByRole('navigation', { name: 'breadcrumb' }); + await nav.getByRole('button', { name: 'music', exact: true }).click(); + expect(spy).toHaveBeenCalledWith('music/'); + }); + + it('should hide upload button when in archive mode', async () => { + const state = createState(); + state.archive.archiveKey = 'data.zip'; + render(StorageBreadcrumbWrapper, { state }); + + await expect.element(page.getByRole('button', { name: /upload/i })).not.toBeInTheDocument(); + }); + }); }); diff --git a/src/lib/components/storage/explorer/__tests__/StorageBreadcrumbWrapper.svelte b/src/lib/components/storage/explorer/__tests__/StorageBreadcrumbWrapper.svelte index 073599f2..3c73ae8f 100644 --- a/src/lib/components/storage/explorer/__tests__/StorageBreadcrumbWrapper.svelte +++ b/src/lib/components/storage/explorer/__tests__/StorageBreadcrumbWrapper.svelte @@ -1,15 +1,21 @@ diff --git a/src/lib/components/storage/explorer/__tests__/TabBarWrapper.svelte b/src/lib/components/storage/explorer/__tests__/TabBarWrapper.svelte new file mode 100644 index 00000000..ffebff1c --- /dev/null +++ b/src/lib/components/storage/explorer/__tests__/TabBarWrapper.svelte @@ -0,0 +1,21 @@ + + + diff --git a/src/lib/components/storage/explorer/file-icon/FileIconAndName.svelte b/src/lib/components/storage/explorer/file-icon/FileIconAndName.svelte new file mode 100644 index 00000000..c510f0e5 --- /dev/null +++ b/src/lib/components/storage/explorer/file-icon/FileIconAndName.svelte @@ -0,0 +1,38 @@ + + +
+ {@render renderIcon(config)} + {keyToName(file.key)} + {#if badge} + + {badge} + + {/if} +
+ +{#snippet renderIcon(cfg: IconConfig)} +