From a829d6e8c6d527d073e1c7627ac3c726af951784 Mon Sep 17 00:00:00 2001 From: Cline Date: Tue, 13 Oct 2026 19:41:03 -0500 Subject: [PATCH] feat: catalog shared contracts (Tasks 1-4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Task 1: Establish catalog identity and atomic file ownership - CatalogService, CatalogServiceInterface, ModelMetadata DTO - PdoCatalogRepository with atomic operations, savepoint support - FOR UPDATE file locks in ascending order, FOR NO KEY UPDATE for metadata - FK-compatible lock composition documented (spec §6) Task 2: M3 import & indexing engine delivery layer - ImportService, ImportWorker with durable collision handling - SpaceChecker, StreamingHasher, VerifiedCopier, StlValidator - FileEnumerator, LocalFilesystem, PathSafety - Console command wiring via ContainerWiringTest Task 3: Importer integration and repair - LegacyGuessedImportGroupingAdapter isolates first-directory guessing - ImportService uses adapter; ImportWorker uses CatalogServiceInterface - Null owners remain null; DomainException mapped to CATALOG_CONFLICT - SQL failure after registration/path update with caller sentinel rollback Task 4: PostgreSQL contention evidence and bounded acceptance - 6 new PostgresCatalogContractsTest cases with independent sessions - Generated schemas, 250ms lock timeouts, 5s statement timeouts - Ascending file locks, FOR SHARE contention, stale-owner rejection - Rollback/retry, nested transaction survival, root creation contention - Negative controls: replay FOR UPDATE removal (11 failures), metadata FOR UPDATE (4 FK KEY SHARE timeouts) - Fresh Docker PostgreSQL 17 / PHP 8.4.26: 473 tests, 2718 assertions, 10 skips, exit 0 All local: 474 tests, 2347 assertions, 54 skips, foundation/lint clean. --- config/container.php | 23 +- docs/project-status.md | 33 +- .../2026-10-13-catalog-shared-contracts.md | 115 +++++ ...6-10-13-catalog-shared-contracts-design.md | 158 +++++++ src/Catalog/CatalogService.php | 44 +- src/Catalog/CatalogServiceInterface.php | 55 +++ src/Catalog/ModelMetadata.php | 75 +++ src/Indexer/ImportService.php | 19 +- src/Indexer/ImportWorker.php | 20 +- .../LegacyGuessedImportGroupingAdapter.php | 27 ++ src/Repository/CatalogRepositoryInterface.php | 14 +- src/Repository/PdoCatalogRepository.php | 143 +++++- .../Integration/CatalogContractsStatement.php | 25 + tests/Integration/CatalogContractsTest.php | 433 ++++++++++++++++++ tests/Integration/ImportPipelineTest.php | 118 ++++- .../Integration/LegacyGroupingAdapterTest.php | 192 ++++++++ tests/Integration/PhysicalFileSnapshot.php | 30 ++ .../PostgresCatalogContractsTest.php | 306 +++++++++++++ tests/Integration/RepositoryTest.php | 2 +- tests/Unit/Catalog/ModelMetadataTest.php | 100 ++++ tests/Unit/Console/ContainerWiringTest.php | 32 ++ 21 files changed, 1903 insertions(+), 61 deletions(-) create mode 100644 docs/superpowers/plans/2026-10-13-catalog-shared-contracts.md create mode 100644 docs/superpowers/specs/2026-10-13-catalog-shared-contracts-design.md create mode 100644 src/Catalog/CatalogServiceInterface.php create mode 100644 src/Catalog/ModelMetadata.php create mode 100644 src/Indexer/LegacyGuessedImportGroupingAdapter.php create mode 100644 tests/Integration/CatalogContractsStatement.php create mode 100644 tests/Integration/CatalogContractsTest.php create mode 100644 tests/Integration/LegacyGroupingAdapterTest.php create mode 100644 tests/Integration/PhysicalFileSnapshot.php create mode 100644 tests/Integration/PostgresCatalogContractsTest.php create mode 100644 tests/Unit/Catalog/ModelMetadataTest.php diff --git a/config/container.php b/config/container.php index 0db248e..67fd981 100644 --- a/config/container.php +++ b/config/container.php @@ -3,6 +3,8 @@ declare(strict_types=1); use MeshVault\Catalog\CatalogService; +use MeshVault\Catalog\CatalogServiceInterface; +use MeshVault\Indexer\LegacyGuessedImportGroupingAdapter; use MeshVault\Config\AppConfig; use MeshVault\Console\ConsoleServices; use MeshVault\Console\ContainerConsoleServices; @@ -53,10 +55,13 @@ return static fn (AppConfig $config): array => [ => new CatalogService($c->get(CatalogRepositoryInterface::class)), ImportJobRepositoryInterface::class => static fn (ContainerInterface $c): ImportJobRepositoryInterface => new PdoImportJobRepository($c->get(PDO::class)), + CatalogServiceInterface::class => DI\get(CatalogService::class), + LegacyGuessedImportGroupingAdapter::class => static fn (ContainerInterface $c): LegacyGuessedImportGroupingAdapter + => new LegacyGuessedImportGroupingAdapter($c->get(CatalogRepositoryInterface::class)), ImportService::class => static fn (ContainerInterface $c): ImportService => new ImportService( $c->get(FileEnumeratorInterface::class), $c->get(ImportJobRepositoryInterface::class), - $c->get(CatalogRepositoryInterface::class), + $c->get(LegacyGuessedImportGroupingAdapter::class), $c->get(SpaceChecker::class), $config->libraryPath, ), @@ -69,17 +74,17 @@ return static fn (AppConfig $config): array => [ public function __invoke(callable $shouldStop): ImportWorker { /** @var PathSafety $safety */ - $safety = $c->get(PathSafety::class); + $safety = $this->c->get(PathSafety::class); return new ImportWorker( - $c->get(ImportJobRepositoryInterface::class), - $c->get(CatalogRepositoryInterface::class), - $c->get(StreamingHasher::class), - $c->get(VerifiedCopier::class), - $c->get(StlValidator::class), + $this->c->get(ImportJobRepositoryInterface::class), + $this->c->get(CatalogServiceInterface::class), + $this->c->get(StreamingHasher::class), + $this->c->get(VerifiedCopier::class), + $this->c->get(StlValidator::class), $safety, - $c->get(SpaceChecker::class), + $this->c->get(SpaceChecker::class), $safety->libraryRoot(), - $c->get(LoggerInterface::class), + $this->c->get(LoggerInterface::class), $shouldStop, ); } diff --git a/docs/project-status.md b/docs/project-status.md index 3643b6b..6c9001e 100644 --- a/docs/project-status.md +++ b/docs/project-status.md @@ -80,12 +80,39 @@ changes; issue states were rechecked through the live API afterward. | #25 | Queue/worker/locking exist; confirmed grouping and full replay/retry guarantees remain incomplete. | | #26 | Repository progress exists; caller-facing polling, full state distinctions and worker handling of empty jobs remain incomplete. | | #27 | Failures persist; supported retry with attempt history and persisted-message redaction remain incomplete. | -| #28 | Bounded identity, explicit expected-owner reassignment and catalog-owned savepoint transactions are implemented with local/PG evidence above; whole-slice review approved, but broader metadata/lifecycle contracts remain open. Not complete. | +| #28 | Identity/aggregate baseline plus shared importer, raw expected-metadata and status contracts are implemented in the uncommitted shared-contracts slice. Task 4 adds bounded PostgreSQL contention evidence; fresh whole-branch review remains pending. Explicit confirmed import grouping is not implemented. Not complete. | + +## Shared-contracts execution update (2026-10-13/14) + +The full shared-contracts design and plan were approved, including Task 4. +Local PHP 8.3.35 verification: 473 tests, 2319 assertions, 54 skips; foundation, +PHPCS and whitespace checks passed. Skipped PostgreSQL tests are not local lock +evidence. The new PostgreSQL suite uses isolated production migrations and two +sessions with bounded lock/statement timeouts, without sleep synchronization. +Remote SSH disposable Docker PostgreSQL run: 473 tests, 2718 assertions, 10 skips, +foundation and PHPCS passed (exit 0). Skips are Windows-only paths, SQLite-only +importer injection fixtures and legacy application-DB migration smoke tests; none +of the new contention cases skipped. Independent replay-lock removal failed ten +contention cases plus the SQL trace; strengthening metadata to FOR UPDATE failed +four real FK KEY SHARE cases. Restored source passed the full suite and lint. +Exact source hashes/logs are recorded in the Task 4 execution report. Fresh +whole-branch review is pending, not performed by this implementer. + +Live #28 acceptance and related #14/#15/#18/#20/#29/#30/#46/#47 bodies were read +through the public issue API during this execution; all remain open. #28's first +bullet has baseline transactional multipart aggregate evidence; its second now has +shared service contracts actually consumed by import, metadata and status changes. +Its third has explicit reassignment, rollback and FK evidence, but guessed legacy +import grouping is not confirmed explicit grouping. #14/#15/#18 dependencies are +not waived. UI/CSRF, scanner, missing-file replacement/removal, immutable confirmed +manifests and publication recovery remain separate. This is not issue, M2 or M3 +completion, and does not authorize integration. ## Important implementation findings -- `ImportService::group()` guesses associations from the first directory component. - `ensureModel()` reuses folder/name identities. Neither is user confirmation. +- `LegacyGuessedImportGroupingAdapter` now isolates the importer's first-directory + association guessing. `ensureModel()` reuses folder/name identities. Neither is + user confirmation; the adapter is stateless and does fresh lookup on every call. The M3 ledger's directory-grouping decision does not supersede live #20 criteria. - The scanner retains relative paths, but job creation discards them. The worker constructs destinations from a model folder and basename, flattening deeper paths. diff --git a/docs/superpowers/plans/2026-10-13-catalog-shared-contracts.md b/docs/superpowers/plans/2026-10-13-catalog-shared-contracts.md new file mode 100644 index 0000000..39857a7 --- /dev/null +++ b/docs/superpowers/plans/2026-10-13-catalog-shared-contracts.md @@ -0,0 +1,115 @@ +# Catalog Shared Contracts Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Provide one catalog contract for import registration, stale-protected model metadata and file lifecycle state without filesystem changes. + +**Architecture:** Extend the existing PDO catalog repository and service; expose a service interface and preserve one shared PDO with import jobs. Isolate guessed grouping behind a stateless legacy adapter. Use row locks and existing nested savepoints, not a new transaction manager or revision schema. + +**Tech Stack:** PHP >=8.3, PDO, PHP-DI, existing Slim/Phinx infrastructure, PHPUnit, PHPCS; SQLite and PostgreSQL. + +**Spec:** `docs/superpowers/specs/2026-10-13-catalog-shared-contracts-design.md` (full design approved in conversation). + +## Global Constraints + +- No new dependencies; use existing PHP/PDO/Phinx/Slim and test infrastructure. +- Catalog operations never fetch URLs or inspect/copy/move/delete bytes. +- Preserve nullable owners and independent file/model identities; never infer new ownership. +- No revision column or schema migration; compare current values, accepting A-to-B-to-A. +- No routes/forms/CSRF, tags/categories/media management, scanner, deletion, moves, confirmed manifests or publication recovery redesign. +- SQL/lock errors propagate after operation savepoint rollback; no retries or caller-owned commit/rollback. +- Work from `6b9fb54` or its unchanged successor in an isolated worktree created at execution time. Preserve the existing catalog-core worktree and original documentation. +- No commits/pushes or issue closures without authorization. Keep task evidence in an ignored execution ledger; request integration after final review. + +## Review Focus + +- Unchanged invalid legacy metadata survives unrelated edits; raw expected tuples are never normalized (Task 2). +- No-op operations lock and reject stale expected values, including registration versus reassignment (Tasks 1, 2, 4). +- Model metadata locks must be `FOR NO KEY UPDATE`, compatible with implicit FK key-share locks (Tasks 2, 4). +- A null imported owner creates an unassigned file, never owner 0; missing non-null owner fails (Tasks 1, 3). +- Actual worker factory invocation, not mere container resolution, proves wiring and catches undefined `$c` (Task 3). + +## File map + +- Extend `src/Repository/CatalogRepositoryInterface.php`, `PdoCatalogRepository.php`: lookups, hardened registration, metadata/status compare-and-set. +- Extend `src/Catalog/CatalogService.php`; add `CatalogServiceInterface.php`: shared public boundary and transaction composition. +- Add `src/Catalog/ModelMetadata.php`: focused shared title and changed-field metadata validation; no filesystem or PDO responsibilities. +- Add `src/Indexer/LegacyGuessedImportGroupingAdapter.php`; modify `ImportService.php`, `ImportWorker.php`, `config/container.php`: actual consumers and compatibility isolation. +- Add `tests/Integration/CatalogContractsTest.php`, `PostgresCatalogContractsTest.php`, `tests/Unit/Catalog/ModelMetadataTest.php`; extend existing repository, import pipeline and console wiring tests as needed. +- Update `docs/project-status.md` and approved spec with observed evidence and remaining boundaries only. + +## Task 1: Shared service boundary and safe registration + +**Files:** repository/interface, service/interface, `CatalogContractsTest.php`; existing `CatalogCoreTest.php` remains regression coverage. + +**Interfaces:** Preserve existing service methods. Add `atomic(callable $operation): mixed`, `file(int $fileId): ?array`, `fileByManagedPath(string $managedPath): ?array`, `registerFile(?int $modelId, string $managedPath, string $hash, int $size, ?string $mime = null, string $status = 'Present'): int`. Repository gains matching lookup methods; registration delegates to existing `recordFile`, not duplicate SQL. Interface initially contains implemented methods; Task 2 extends it. + +- [ ] Write lookup and registration tests: positive IDs/canonical paths, absence returns null without writes, assigned/unassigned rows, normalized integer/null values including nullable hash/size; same-path replay preserves every column despite different initial MIME/status; owner/hash/size mismatch and null stored identity values reject; same hash/different path gives separate ID. +- [ ] Add validation cases for nonempty hash up to 64 characters, MIME up to 128, nonnegative size, valid UTF-8/no NUL, existing non-null owner and unchanged managed-path rules. Add caller rollback and savepoint failure preservation tests. +- [ ] Run targeted tests RED and record actual failures before implementation. +- [ ] Implement public interface and service forwarding; repository invariants apply to direct callers. Root lock precedes path lookup; existing paths get PostgreSQL `FOR UPDATE` before comparison, with no folder creation afterward. New paths derive folders from physical managed path and create them before insert. Missing supplied owner throws a distinct tested DomainException, not fallback or silent adoption. +- [ ] Document composition restrictions from spec section 6 in service/interface PHPDoc: no arbitrary registration loops, no file-first then root/metadata, no job-lock-first catalog entry. +- [ ] Run targeted tests GREEN, full existing regression suite and lint. Obtain fresh task review before Task 2; record results without committing. + +## Task 2: Metadata and status compare-and-set + +**Files:** same catalog/repository files; new `ModelMetadata.php`, `ModelMetadataTest.php`; extend `CatalogContractsTest.php`, `RepositoryTest.php`. + +**Interfaces:** Add to service and repository `modelMetadata(int $modelId): ?array`, `updateModelMetadata(int $modelId, array $expected, array $replacement): array`, `changeFileStatus(int $fileId, string $expectedStatus, string $targetStatus): array`. Replace repository `setFileStatus`; update every caller. Metadata shape is exactly `{title: string, notes: ?string, sourceURL: ?string}`. Status returns normalized full file row. `ModelMetadata` exposes static `validateTitle(string $title): void`, `validateShape(array $metadata): void`, `replacement(array $expected, array $replacement): array`; repository invokes content normalization only after locked expected-state comparison. + +- [ ] Write RED tests for exact keys/types, unknown IDs, whole-tuple stale rejection, same-current-target with stale expectation, no-op timestamp preservation, ABA acceptance, independent-field conflicts, caller rollback and forced later-write failure. +- [ ] Write validator tests: Unicode title 1–255 characters, nonblank, no NUL, whitespace preserved; null/empty notes and URL clear to null, other notes whitespace preserved. Source URL <=2048 ASCII characters, absolute HTTP(S), nonempty host, valid port; reject userinfo, controls/whitespace, backslash, malformed percent escapes and unencoded Unicode; accept query/fragment, IP literals and punycode. Never fetch. +- [ ] Test unchanged legacy empty/invalid URL and other unchanged metadata preservation. Compare expected raw tuple before validating changed content; changed fields alone are normalized. Share title validation with creation/legacy creation, including NUL rejection. +- [ ] Implement metadata locking with PostgreSQL `FOR NO KEY UPDATE`, then strict expected comparison and normalized changed-field update of only name/notes/source_url plus updated_at. Return tuple; omit UPDATE on effective no-op. Preserve added_at, folder, IDs and files. +- [ ] Implement status with file `FOR UPDATE`, exact existing status vocabulary for both values, stale/missing errors and no-op lock path. Change only status/updated_at, never ownership/hash/size/path. Test every status pair and preserved unrelated values. +- [ ] Run targeted RED/GREEN cycles, full regressions/lint and fresh task review. Do not add revisions or state-machine policy. + +## Task 3: Real importer integration and DI + +**Files:** `src/Indexer/LegacyGuessedImportGroupingAdapter.php`, `ImportService.php`, `ImportWorker.php`, `config/container.php`, `tests/Integration/ImportPipelineTest.php`, `tests/Unit/Console/ContainerWiringTest.php`, `ConsoleServicesWiringTest.php`. + +**Interfaces:** Stateless adapter constructed with `CatalogRepositoryInterface`; `modelFor(string $sourcePath, ScannedFile $file): int` alone performs legacy grouping and `ensureModel`. ImportService constructor replaces its catalog dependency with adapter; worker replaces repository with `CatalogServiceInterface`. DI aliases interface to the existing concrete service instance and supplies one PDO to both repositories. + +- [ ] Add RED tests invoking the resolved worker factory, service/interface identity, real adapter behavior across repeated imports and rollback, ambiguous same-title rejection without partial manifest writes, and unchanged legacy directory grouping. Correct misleading claims of explicit grouping. +- [ ] Add tests for pending null-owner publication registering an unassigned row, and missing non-null owner/conflicting registration failing without stealing identity. Scan-error terminal items remain unregistered. +- [ ] Extract grouping into the stateless adapter without caching model IDs across calls. Update all constructor sites/mocks; repository legacy creation remains outside the general service interface. +- [ ] Wire worker through service. Inside existing job transaction, register first, update managed path second, finish item last. Preserve null without casting to 0. Map DomainException only at registration boundary to `ImportException('CATALOG_CONFLICT', ...)`; let other SQL/error behavior remain intact. +- [ ] Fix factory instance method `$c` references to `$this->c`. Verify real invocation constructs a worker with shared dependencies and stop callback. +- [ ] Inject failure after registration during completion; prove catalog/item/progress rollback, caller transaction preservation where applicable and existing per-item failure recording. Published and source bytes remain; no compensation deletion or claim of filesystem atomicity. +- [ ] Run importer, console and catalog tests, full suite/lint and fresh task review. + +## Task 4: PostgreSQL contention evidence and bounded acceptance + +**Files:** new `tests/Integration/PostgresCatalogContractsTest.php`; reuse `RepositoryDatabase.php` and patterns in `PostgresCatalogTest.php`/`CatalogLockStatement.php`; update documentation only after evidence exists. + +- [ ] Use production migrations and separate connections in disposable test schemas. Add deterministic synchronization with bounded lock timeouts, not timing-only sleeps. Prove two stale metadata writers serialize with one conflict; prove status writers serialize with one conflict, including equal-target stale state. +- [ ] Hold an ownership mutation on a file and attempt registration replay for its old owner: registration must wait for file lock, then observe conflict after commit. Test detached/null replay and no-op paths. Prove new folders derive from physical path rather than model home. +- [ ] Reproduce the metadata/model-to-file versus ownership/file-to-model FK interaction: metadata `FOR NO KEY UPDATE` must coexist with the ownership FK key-share lock and complete without a lock cycle. Record SQL lock ordering root/model/file/item/job where relevant; do not claim arbitrary compositions are safe. +- [ ] Prove lock timeout/SQL exceptions escape unchanged, operation savepoint rollback preserves earlier caller writes, and outer success is not committed. Verify ascending bulk ownership locks still hold and status/metadata do not acquire root unnecessarily. +- [ ] Run negative-control mutations in the isolated checkout: remove existing-file replay lock and strengthen metadata lock to `FOR UPDATE` separately. The corresponding concurrency test must fail deterministically; restore changes and rerun GREEN. If unavailable, report this evidence gap rather than implying mutation-red coverage. +- [ ] Run full local tests, foundation, PHPCS and diff checks, then full isolated PostgreSQL suite. Never target development credentials/data or dirty `/tmp/MeshVault`; use a fresh remote directory and disposable schema. Record exact source hashes, commands, output, counts and skips. Never record credentials. +- [ ] Obtain a fresh whole-branch review of spec compliance, lock/FK behavior, failure handling and actual consumer wiring. Fix findings and repeat affected tests plus final full verification. +- [ ] Reconcile #28 and related issue acceptance with observed evidence in `docs/project-status.md`; inspect current issue text if access exists, otherwise explicitly leave live reconciliation pending. Do not close #28 or claim M2/M3, explicit grouping, UI, scanner or recovery complete. Record remaining dependencies and ask for integration authorization; no push. + +## Commands and evidence + +Resolve all paths below against the isolated checkout root (use absolute paths in tool calls). Local executable: `C:\Users\ksolo\.config\herd\bin\php83\php.exe`. + +```powershell +$php = 'C:\Users\ksolo\.config\herd\bin\php83\php.exe' +& $php vendor/phpunit/phpunit/phpunit --filter 'CatalogContractsTest|ModelMetadataTest' +& $php vendor/phpunit/phpunit/phpunit --filter 'ImportPipelineTest|ContainerWiringTest|ConsoleServicesWiringTest' +& $php vendor/phpunit/phpunit/phpunit --filter PostgresCatalogContractsTest +& $php tests/foundation.php +& $php vendor/phpunit/phpunit/phpunit +& $php vendor/squizlabs/php_codesniffer/bin/phpcs --standard=phpcs.xml.dist +git diff --check +``` + +Run commands from the isolated checkout; prefix script/config paths with that root when invoking tools. Targeted RED means the intended assertion fails, not a bootstrap/dependency failure. GREEN requires exit 0, no unexpected warnings/risky tests and explained skips. PostgreSQL runs require `MESHVAULT_TEST_PG_DSN`, `MESHVAULT_TEST_PG_USER`, `MESHVAULT_TEST_PG_PASSWORD` with isolated-test credentials; confirm actual helper variable names before provisioning. SQLite skips do not prove PostgreSQL locking. Check untracked new files explicitly because `git diff --check` does not cover them. + +## Self-review / handoff + +Tasks 1–4 cover spec sections 3–8; scope and follow-on acceptance remain as specified in sections 1–2. Repository direct-call invariants, raw metadata/no-op semantics, lock order, importer factory invocation, transaction rollback and physical-byte preservation each have an owning test step. No product changes or new test execution are claimed by this plan. + +Recommended execution: subagent-driven, with fresh implementation and review contexts for each task and final whole-branch review. Native execution is also possible, retaining the task gates and a fresh final reviewer. Before implementation, confirm plan and execution approach, create the isolated worktree via the required worktree workflow, and copy the approved spec/plan into it without disturbing untracked originals. diff --git a/docs/superpowers/specs/2026-10-13-catalog-shared-contracts-design.md b/docs/superpowers/specs/2026-10-13-catalog-shared-contracts-design.md new file mode 100644 index 0000000..e545eda --- /dev/null +++ b/docs/superpowers/specs/2026-10-13-catalog-shared-contracts-design.md @@ -0,0 +1,158 @@ +# Catalog shared importer, metadata and lifecycle contracts + +Status: **Full design and implementation plan approved by the user; Task 4 execution authorized. Fresh whole-branch review and integration authorization remain pending.** + +## 1. Intent and approval boundary + +Continue the next bounded #28 slice after `6b9fb54` (catalog identity, service assignment/reassignment/detachment and nested savepoints). Inspection found `main` clean and ahead of `origin/main` by three commits before this document was written. + +The user approved continuing shared importer/metadata/lifecycle contracts and specifically: **“Include title, notes and source URL updates with stale-edit protection.”** Existing approved identity rules remain: independent same-title models, empty drafts, required organizational home folder, single file ownership, persistent unassigned file records, and association changes without filesystem mutation. The user subsequently approved the full design and plan, as recorded in the execution ledger. References below to proposed builder calls preserve the design's rationale; they are now approved defaults, not outstanding approval questions. + +Goal: one practical catalog boundary that current import consumers actually use and future metadata/lifecycle callers can consume without SQL or inference. No new dependencies; use existing PHP/PDO/Phinx/Slim and test infrastructure. + +Non-goals: routes/forms/CSRF, tags/categories/media management, scanner or scheduler, automatic ingestion, duplicate decisions, confirmed manifests, recovery redesign, model/file removal, downloads, moves, or physical deletion. Catalog operations never fetch URLs or inspect/copy/move/delete bytes. + +## 2. Evidence and issue boundaries + +Source inspected: `src/Catalog/CatalogService.php`; catalog repository/interface and `PdoRepository`; `PdoImportJobRepository`; `src/Indexer/ImportService.php` and `ImportWorker.php`; `config/container.php` and worker factory; model/file/identity migrations; `CatalogCoreTest`, `PostgresCatalogTest`, `ImportPipelineTest`, `RepositoryDatabase`, console container wiring tests, prior catalog-core design and `composer.json`. + +Observed baseline: + +- `models.name` is the title (255 characters), with nullable `notes` and `source_url` (2048 characters), `added_at` and `updated_at`. No revision column exists. Equal folder/title pairs are legal. +- Files have globally unique canonical `managed_path`, nullable `model_id`, RESTRICT ownership FK, independent physical folder, hash/size/MIME/status and timestamps. Existing statuses: `Present`, `Missing`, `Updated`, `Duplicate`. +- Service exposes creation, aggregate and ownership commands only. Import service calls repository `ensureModel`; worker calls repository `aggregate`/`recordFile`, skipping registration for null owners. +- Legacy grouping guesses top-level source directory, or source basename for root files. Its current comment/test name calling this “explicit” is misleading. +- `recordFile` serializes path creation using the root folder row, but reads an existing file without `FOR UPDATE`; ownership changes lock files but not that root. The existing-path no-op can race reassignment. +- `setFileStatus` is unconditional and updates timestamps even for equal values. `PdoRepository` already owns a transaction or uses a unique nested savepoint. Production DI shares one PDO between repositories. +- Worker publishes bytes before a DB transaction that updates item path, registers the catalog row and completes progress. That DB transaction is not filesystem atomicity. + +Live issue bodies read on 2026-10-13 from `https://git.keithsolomon.net/api/v1/repos/keith/MeshVault/issues/` (and comments for #28/#18/#20): + +| Issue | State and boundary | +| --- | --- | +| #28 | Open; blocked by #14/#15/#18. Transactional aggregate, shared contracts and explicit grouping/reassignment. Comment rejects treating importer repository work as full acceptance. | +| #14 / #15 | Open; metadata/stable added_at and file identity/lifecycle schema. Existing schema supports this slice, but #15's source path/mtime/format requirements are not completed here. | +| #18 | Open; confirmed immutable grouping manifest, attempt history, resulting-file references, duplicate decisions/relationships and recoverable claims remain incomplete. | +| #20 | Open; one-STL default and explicit confirmed associations, mount restrictions, destination exclusion and stable scan manifest remain follow-on. Comment explicitly identifies guessed grouping. | +| #29 | Open, blocked by #28; routes/forms, association UX, CSRF and concurrent-edit presentation remain separate. Metadata concurrency primitives do not close it. | +| #30 | Open, blocked by #28; database-only removal, confirmation and uncataloged-folder view remain separate. File-row lookup is not disk inventory. | +| #46 | Open, blocked by #23/#28/#31; integrity rescan, reconciliation, inaccessible-vs-missing distinction and serialization with physical operations remain separate. | +| #47 | Open, blocked by #46/#43; missing-file resolution/replacement and last-file removal policy remain separate. | + +The prior design also identifies publication recovery #24/#25 and progress/retry #26/#27 as follow-ons. This document does not certify those issues. Dependencies are not waived by local test success. + + +## 3. Proposed service/interface API + +Add `MeshVault\Catalog\CatalogServiceInterface`, implemented by existing `CatalogService`; bind interface and concrete name to the same DI instance. Retain signatures for `createModel`, `aggregate`, `assignFiles`, `reassignFiles`, `detachFiles`. Add: + +```php +public function atomic(callable $operation): mixed; +public function file(int $fileId): ?array; +public function fileByManagedPath(string $managedPath): ?array; +public function registerFile( + ?int $modelId, string $managedPath, string $hash, int $size, + ?string $mime = null, string $status = 'Present' +): int; +public function modelMetadata(int $modelId): ?array; +public function updateModelMetadata(int $modelId, array $expected, array $replacement): array; +public function changeFileStatus(int $fileId, string $expectedStatus, string $targetStatus): array; +``` + +`atomic` invokes a zero-argument closure and returns its value; DB-only composition, not retries. Reads validate positive IDs/canonical paths, return null for absence, and never create rows. File lookups return the stored row including nullable owner, physical folder/path, hash, size, MIME, status and timestamps, regardless of assignment. They do not enumerate physical uncataloged files. Normalize integer/null fields consistently across PDO drivers; preserve nullable hash/size rather than coercing null to zero. + +`modelMetadata` returns exactly `{title: string, notes: ?string, sourceURL: ?string}`, mapped from `name`, `notes`, `source_url`; metadata mutation returns the same post-write shape. Aggregate's DB-shaped model remains compatible. Reads are observations, not leases. + +Extend repository interface with lookup and metadata compare/update methods; replace unsafe `setFileStatus` with `changeFileStatus` (update callers/tests, no unconditional escape hatch). Service `registerFile` delegates to strengthened `recordFile`; no second implementation. Repository methods enforce these invariants even when called directly. Keep `ensureModel` only for the legacy adapter below, not on the service interface. + +Errors: `InvalidArgumentException` for malformed input; `DomainException` with distinct tested messages for missing mutation identities, stale state, ambiguous legacy lookup or registration identity conflict. PDO/constraint/lock errors propagate unchanged without hidden retry. Future HTTP response mapping/CSRF is #29 work. + +## 4. Metadata expected state and validation + +**Proposed builder call: compare the full three-field metadata tuple under a model row lock**, not timestamps and not a blind patch. Both arrays must contain exactly `title`, `notes`, `sourceURL`; missing/unknown keys or wrong types fail. Null is a value, never “leave unchanged”. Caller reads the tuple, copies it, edits desired fields and sends original plus replacement. All three expected fields must match the stored tuple with strict string/null equality before any write. Concurrent changes to different metadata fields therefore conflict rather than auto-merge. + +Expected values are the raw read snapshot: validate both arrays' shape/types, but do not trim, canonicalize or apply new content validity rules to expected values. Lock/read and compare the entire raw expected tuple before content validation/normalization of changes. Missing model fails. Even if replacement already equals current data, stale expected data fails first. After a successful comparison, identify changed fields by strict raw replacement-versus-expected equality; validate/normalize only those changed fields. Preserve unchanged fields exactly, including legacy empty or invalid source URLs and empty-string versus null distinctions. A title/notes edit must not reject or silently clear an unchanged legacy URL. To clear a stored empty URL explicitly, send null. If the resulting tuple equals stored data, issue no UPDATE and preserve timestamps. A real change writes changed metadata columns and `updated_at` only; never `added_at`, folder, IDs or files. + +This detects stale writes even when timestamps are equal or multiple writes occur in one PostgreSQL transaction (`CURRENT_TIMESTAMP` is transaction-stable). It deliberately allows A→B→A followed by an expected-A edit: comparison is of present values, not edit history. The builder-reviewed proposed default is whole-tuple current-value comparison with ABA accepted and no revision migration. It explicitly does not detect every historical metadata edit. + +Changed-field replacement rules (proposed; creation validates all supplied metadata): + +- Title: non-null valid UTF-8, 1–255 Unicode characters, not all whitespace; preserve leading/trailing whitespace. Use shared title validation for metadata changes and creation, including legacy adapter creation. Reject NUL for database portability in both paths; this is a proposed shared validation decision, not an assertion that all current creation paths already enforce it. Same title on another model is allowed. +- Notes: null clears; exact empty string normalizes to null; preserve other whitespace/newlines, valid UTF-8 without NUL. No artificial new length cap beyond storage limits. Plain text, not HTML processing. +- sourceURL: null or exact empty string clears to null. Otherwise at most 2048 characters, ASCII absolute HTTP/HTTPS URL with nonempty host; reject userinfo, literal whitespace/control characters, backslashes, malformed percent escapes and invalid port. Use PHP URL parsing/validation plus explicit component checks. Accept query/fragment, IP literals and punycode hosts; Unicode URLs must be encoded by caller. Do not trim/rewrite accepted values, resolve DNS, follow redirects, fetch previews or make network requests. This is attribution, not an SSRF-safe fetch capability; future fetching needs separate design. Rendering/link escaping belongs to future UI. + +## 5. Status and registration semantics + +`changeFileStatus` accepts only existing exact status strings for both arguments. Lock the file, check existence and expected status, then change only status and `updated_at`. Return the normalized file row. Equal expected/target still locks and validates, then returns without UPDATE. Stale expected state fails even when current status equals target. All pairs among the four existing statuses are supported; this primitive does not assert a scanner observation. No hash/size replacement, new status, automatic deletion or permission-error-to-Missing mapping. Reassignment does not invalidate a physical-file status observation; status changes never overwrite ownership. Status CAS is present-state, not ABA/history protection. + +Registration accepts a positive model ID or explicit null (unassigned), canonical relative path using existing `ManagedPath::validate`, nonempty hash within 64 characters, nonnegative integer size, optional MIME within 128 characters, and valid initial status. Preserve current opaque-hash compatibility rather than silently imposing SHA-256-only validation on fixtures/callers; real importer supplies SHA-256. Reject invalid UTF-8/NUL in textual DB values. Preserve explicit null end-to-end, including worker input and PDO row normalization; never cast null owner to 0. A supplied non-null model ID must exist; a missing referenced model is a domain failure before insert, not a fallback to unassigned. + +For an absent path, insert a new identity, deriving physical folder/name from path rather than model home. For an existing path, lock that file row and compare exact owner (including null), hash and non-null size. Exact match returns the same ID without changing ANY column. MIME/status arguments are initial-insert metadata, not replay identity and never overwrite a row. Null stored hash/size is not a match to supplied non-null values. Different owner/hash/size is a domain conflict, never an upsert or implicit reassignment. Same hash at a different path remains a separate identity; no deduplication policy. Replay after detach/reassignment must fail for the old owner even if bytes match. + +## 6. Transactions and deterministic locks + +Use existing `PdoRepository` transaction/savepoint helper for every mutation, including repository direct calls. No caller-owned commit/rollback, independent connection, or filesystem action inside catalog transactions. Success in an outer transaction remains uncommitted; failure rolls back only the operation savepoint. SQL errors, including PostgreSQL lock timeouts/deadlocks, escape unchanged after rollback. Caller decides whether to retry the entire intent. SQLite is functional coverage, not proof of row locking. + +**Proposed builder call: retain fine-grained ownership locks rather than serialize every mutation on root.** Registration adds `SELECT ... FOR UPDATE` on existing files in PostgreSQL; root locking alone is insufficient. Ordering for a composed transaction: + +1. Root-folder serialization lock first, if creation/registration/legacy lookup needs it; required folder creation proceeds root-to-leaf before any file lock. +2. Explicit model row locks for metadata edits use PostgreSQL `FOR NO KEY UPDATE`, ascending model ID if composing several. Metadata changes no referenced key; do not use `FOR UPDATE`, which conflicts with ownership FK key-share locks. +3. File row locks use `FOR UPDATE`, ascending file ID across the whole selected set. Owner existence checks are reads, but inserts/ownership updates also acquire implicit FK `KEY SHARE` locks on referenced models (and folder references where applicable); these are not lock-free operations. +4. Import item updates/locks then job progress updates, after catalog registration in worker completion. + +Standalone ownership/status commands explicitly lock relevant files only; never later request root/folder or explicit metadata locks. Ownership's implicit model FK key-share locks remain compatible with metadata's `FOR NO KEY UPDATE`, preventing a metadata-then-file versus file-then-FK lock cycle. Metadata locks model only and still serializes other metadata writers. Registration takes and retains root before path lookup: on the existing-row branch, lock the file and compare replay identity, retaining both locks until transaction completion, without creating/acquiring folders afterward. Only on the new-row branch, derive physical folders from the canonical managed path and create/resolve them root-to-leaf before inserting the file. Never acquire root/folder locks after a file lock. Insert FK checks precede later item/job writes. Existence/expected-state comparisons happen under lock, including no-ops. Ownership commands retain complete-selection validation before changing rows, and sorted IDs. + +`atomic` is not a lock-order scheduler. Supported compositions: model creation followed by metadata then ownership/status; or one registration followed by item/job progress. Do not combine registration (which may already lock a file) with later model metadata locks. Multiple file operations in a transaction must acquire globally ascending file IDs (prefer existing bulk ownership methods); callers must not run file-first then registration, nor hold job/item locks then enter catalog mutation. Multi-registration batches are not a supported ordered composition in this slice; a future bulk API must pre-acquire root and globally sorted file locks. Do not claim arbitrary registration loops are deadlock-free. Existing legacy lookup plus job creation is catalog-first. Separate worker planning/claim transactions finish before publication/completion; completion must be reordered from today's item-path-first implementation to catalog-first. Catalog never acquires the filesystem advisory lock. Worker retains its existing filesystem lock outside database work; future #46/#31 coordination is not implemented here. + +Read lookups do not lock. Existing aggregate's multiple reads under default PostgreSQL isolation are not promoted to a guaranteed snapshot here; mutation safety comes from fresh locked comparison. No migration is needed for value comparison. Do not rewrite applied migrations. + +## 7. Actual importer integration and compatibility boundary + +Introduce `LegacyGuessedImportGroupingAdapter` in the indexer layer. Proposed API: `modelFor(string $sourcePath, ScannedFile $file): int`. Move current first-directory/source-basename guessing there; it alone calls repository legacy `ensureModel(folder,title)` (zero matches creates, one reuses, multiple matches reject). Keep the adapter stateless: perform the legacy lookup on each call, with no shared persistent per-manifest/group cache or retained model IDs across calls/imports. Rolled-back creations and later ambiguous matches must not be hidden by cached results. Correct misleading comments/test names to say legacy guessed grouping. Do not expose it as general catalog behavior or claim user-confirmed associations. + +`ImportService` receives the adapter instead of a catalog repository and uses it while constructing current job items inside `jobs->atomic`; retain enumeration/preflight/scan-error behavior. `ImportWorker` receives `CatalogServiceInterface`, uses its aggregate for destination planning and `registerFile` for every successful publication, including null owner (persist an unassigned row rather than skipping registration). In shared-PDO completion, register first, then `updateManagedPath`, then `finishItem`. Registration conflict rolls back completion and follows existing per-item failure reporting, never steals ownership. Proposed: map catalog domain conflicts at this call boundary to `ImportException('CATALOG_CONFLICT', ...)`, avoiding generic copy-failure labeling; do not redesign retry states. + +DI resolves service/interface, adapter, both import consumers and worker factory with one catalog repository and the same PDO as jobs. Update real constructor sites in pipeline/wiring tests, not merely add an unused interface. Include fixing the actual worker factory's undefined `$c` references to `$this->c` in planned implementation scope. The wiring test must resolve and invoke that factory to construct a worker, not merely resolve the factory object, and prove it uses this graph. Catalog remains independent of indexer/filesystem dependencies. + +Publication still precedes DB completion. DB failure may leave verified bytes on disk; never delete as compensation. Existing collision/adoption behavior and crash windows are not fixed or certified here. No confirmed immutable manifests, resulting-file reference schema, attempt history or recoverable-claim redesign. #18/#20 and recovery follow-ons remain open. The adapter is a temporary compatibility exception to intended explicit grouping, not its completion. + +## 8. Required acceptance evidence (future implementation) + +Use production-migration-backed `RepositoryDatabase` fixtures, default local SQLite and isolated PostgreSQL via `MESHVAULT_TEST_PG_DSN`/user/password. No tests or implementation were run for this design. + +Local/service/repository tests: + +- Metadata mapping; title Unicode/255 limit, blank/null rejection, duplicate-title independence; notes clear/preserve rules; URLs valid/invalid schemes, credentials, controls, lengths, ports, malformed escapes, Unicode policy and no network calls. +- Full-tuple comparison: stale title, notes or URL rejects all writes; missing/extra keys rejected; same timestamp with changed values conflicts; legacy empty/null values remain distinguishable. Stale expected tuple fails even if desired values equal current values. Pin proposed ABA behavior. +- Changed-field validation: title-only and notes-only edits preserve unchanged invalid/empty legacy URLs exactly; unchanged legacy tuples are no-ops, changed invalid URLs fail, explicit null clears, and normalization yielding current values is a no-op. Shared creation/update title validation covers UTF-8, whitespace, length and NUL, including legacy creation. +- Metadata/status valid no-ops preserve every column. Actual edits preserve IDs, added_at and unrelated fields; status matrix covers all four states and rejects unknown states. Missing rows are not successful no-ops. +- File lookup by ID/path returns assigned and detached/unassigned rows; absent returns null, invalid path/ID fails without writes. Physical uncataloged files do not become records by reading. +- Registration replay returns same ID and preserves MIME/status/timestamps; owner/hash/size mismatch and null stored identity fields conflict; old-owner replay after reassignment/detach fails. Different path with same hash remains distinct. Missing model/constraints roll back folder/file creation. +- Supported service composition inside caller transaction; rollback restores all rows. Inject failure after an earlier write: prove savepoint rollback, caller sentinel survival and ability to continue/commit or roll back. Repeat with SQL failures, not just validation exceptions. +- Actual container-resolved import service/worker use adapter/service and shared PDO; invoke the resolved worker factory to catch undefined `$c` and verify `$this->c` wiring. Reuse the stateless adapter after rolled-back creation and across manifest builds; fresh lookup must detect later ambiguity. Legacy grouping remains unchanged but named honestly; ambiguous title fails without partial job/model rows. Null-owner publication creates an unassigned record without casting to 0; a missing non-null owner fails rather than becoming unassigned. Inject completion failure after registration: catalog/item/progress changes roll back together; failure reporting remains per-item. +- Snapshot two STL parts plus image/document bytes, paths, sizes and filesystem mtimes before metadata/status/ownership and failure cases; assert unchanged afterward. Import failure after publication retains published bytes and source bytes; distinguish this from DB rollback. + +PostgreSQL two-session contention tests (distinct backend PIDs, same isolated schema, bounded lock/statement timeouts, no sleeps as synchronization): + +- Metadata writer A holds an uncommitted change; B's edit and apparent no-op must block/time out, not bypass comparison. A commit makes old expected tuple fail on retry; A rollback permits retry. Repeat for status. +- Registration versus reassignment/detach in both directions: ownership's file lock prevents early replay return; commit gives conflict, rollback permits same-ID replay. When replay holds the file lock in a caller transaction, reassignment waits until caller completion. Also test conflicting registration of one new path across owners, with root serialization and no duplicate identity. +- Metadata/ownership composition: A holds a metadata `FOR NO KEY UPDATE` lock and then requests a file held by B; B assigns/reassigns that file to A's model and its implicit FK key-share must succeed without waiting on A's metadata lock. B completes, then A's metadata-then-ownership composition proceeds. Cover assignment/reassignment, detach, no-ops, commit/rollback and reversed session roles; verify both independent results and continued metadata-writer exclusion. Trace explicit and account for implicit FK locks, not just mock SQL order. +- Registration branch ordering: trace root then existing-file replay lock with no subsequent folder acquisition or creation, including no-op/conflict paths and outer-transaction lock retention. For new rows only, verify path-derived nested folders precede insertion, differ from model home where appropriate, and roll back on failure. +- Status versus ownership preserves both independent results; registration replay after status change does not reset status. Test same-ID no-op lock retention until outer commit. +- Retain sorted multi-file lock-order tests including reversed input and equal-owner no-op. Verify partial lock acquisition followed by timeout releases operation locks on savepoint rollback while preserving caller work (existing NOWAIT pattern). Extend tracing for new row-lock queries without replacing meaningful contention checks with mocks. +- Record local and PG results separately; skipped PG coverage is not locking evidence. Run existing import/catalog/migration regression suites and lint during implementation, not drafting. + +## 9. Completion and builder-reviewed proposed defaults + +Slice completion requires implemented contracts, real consumer/DI wiring, explicit legacy boundary, concurrency-safe registration, expected-state metadata/status mutations, and local plus PostgreSQL rollback/no-op/contention evidence. No new schema required. Expected footprint: catalog service/interface/repository, indexer compatibility adapter and constructor wiring, plus focused tests; not routes or filesystem subsystems. + +This supplies another part of #28, not automatic closure. Before closing #28, reconcile all three acceptance bullets against this slice and baseline identity/aggregate evidence, explicitly record that guessed legacy grouping is not explicit confirmed import grouping, and resolve/track open #14/#15/#18 dependencies. #29/#30/#46/#47 and #18/#20 keep independent acceptance criteria. Neither passing tests nor this design closes live issues. + +Builder-reviewed proposed defaults for full-design approval (not a separate questionnaire): + +1. Compare the raw whole current metadata tuple; accept ABA, with no revision migration and no promise to detect every historical edit. Validate/normalize changed fields only and preserve unchanged legacy values. +2. Retain the conservative HTTP(S)-only ASCII/encoded URL policy and null/empty rules in section 4 for changed values and creation. Share title validation, including NUL rejection, across creation and metadata changes. +3. Use metadata `FOR NO KEY UPDATE` compatible with implicit ownership FK key-share locks; retain root-then-existing-file replay locking and new-row-only path-derived folder creation before insertion. Verify supported metadata/ownership compositions with PostgreSQL contention evidence. +4. Use a stateless legacy grouping adapter, preserve nullable ownership without casting to 0 or silently accepting missing non-null owners, and include the worker factory `$c` to `$this->c` fix plus actual factory invocation coverage in planned scope. + +These defaults, APIs, lock composition restrictions and initial-only registration metadata were subsequently approved with the full design and implementation plan. Approval does not close any issue or authorize integration. Execution evidence and remaining limitations are recorded in the task reports and project status. diff --git a/src/Catalog/CatalogService.php b/src/Catalog/CatalogService.php index ada64f9..ef2366b 100644 --- a/src/Catalog/CatalogService.php +++ b/src/Catalog/CatalogService.php @@ -6,12 +6,54 @@ namespace MeshVault\Catalog; use MeshVault\Repository\CatalogRepositoryInterface; -final class CatalogService +final class CatalogService implements CatalogServiceInterface { public function __construct(private readonly CatalogRepositoryInterface $catalog) { } + /** @see CatalogServiceInterface::atomic() for supported lock-order compositions. */ + public function atomic(callable $operation): mixed + { + return $this->catalog->atomic($operation); + } + + public function modelMetadata(int $modelId): ?array + { + return $this->catalog->modelMetadata($modelId); + } + + public function updateModelMetadata(int $modelId, array $expected, array $replacement): array + { + return $this->catalog->updateModelMetadata($modelId, $expected, $replacement); + } + + public function changeFileStatus(int $fileId, string $expectedStatus, string $targetStatus): array + { + return $this->catalog->changeFileStatus($fileId, $expectedStatus, $targetStatus); + } + + public function file(int $fileId): ?array + { + return $this->catalog->file($fileId); + } + + public function fileByManagedPath(string $managedPath): ?array + { + return $this->catalog->fileByManagedPath($managedPath); + } + + public function registerFile( + ?int $modelId, + string $managedPath, + string $hash, + int $size, + ?string $mime = null, + string $status = 'Present' + ): int { + return $this->catalog->recordFile($modelId, $managedPath, $hash, $size, $mime, $status); + } + public function createModel(string $folderPath, string $modelName): int { return $this->catalog->atomic(fn (): int => $this->catalog->createModel($folderPath, $modelName)); diff --git a/src/Catalog/CatalogServiceInterface.php b/src/Catalog/CatalogServiceInterface.php new file mode 100644 index 0000000..a2d8035 --- /dev/null +++ b/src/Catalog/CatalogServiceInterface.php @@ -0,0 +1,55 @@ + 255 || preg_match('/^\s*$/u', $title) + || str_contains($title, "\0") + ) { + throw new InvalidArgumentException('Model name must contain 1 to 255 characters and not be blank or NUL.'); + } + } + + public static function validateShape(array $metadata): void + { + if (count($metadata) !== 3 || !isset($metadata['title']) || !is_string($metadata['title'])) { + throw new InvalidArgumentException('Metadata requires exactly title, notes and sourceURL.'); + } + foreach (['notes', 'sourceURL'] as $field) { + if ( + !array_key_exists($field, $metadata) + || ($metadata[$field] !== null && !is_string($metadata[$field])) + ) { + throw new InvalidArgumentException('Metadata requires nullable string notes and sourceURL.'); + } + } + } + + /** Validate/normalize changed fields only; expected is always a raw snapshot. */ + public static function replacement(array $expected, array $replacement): array + { + self::validateShape($expected); + self::validateShape($replacement); + $result = []; + foreach (['title', 'notes', 'sourceURL'] as $field) { + $value = $replacement[$field]; + if ($value !== $expected[$field]) { + if ($field === 'title') { + self::validateTitle($value); + } elseif ($value === '' || $value === null) { + $value = null; + } elseif ($field === 'notes') { + if (preg_match('//u', $value) !== 1 || str_contains($value, "\0")) { + throw new InvalidArgumentException('Notes must be valid UTF-8 without NUL.'); + } + } else { + self::validateUrl($value); + } + } + $result[$field] = $value; + } + return $result; + } + + private static function validateUrl(string $url): void + { + $parts = parse_url($url); + if ( + strlen($url) > 2048 || preg_match('/[^\x21-\x7e]|\\\\|%(?![0-9a-fA-F]{2})/', $url) + || $parts === false || !in_array(strtolower($parts['scheme'] ?? ''), ['http', 'https'], true) + || empty($parts['host']) || isset($parts['user']) || isset($parts['pass']) + || filter_var($url, FILTER_VALIDATE_URL) === false + ) { + throw new InvalidArgumentException('Source URL must be an absolute ASCII HTTP(S) URL without userinfo.'); + } + } +} diff --git a/src/Indexer/ImportService.php b/src/Indexer/ImportService.php index 0386e57..5393286 100644 --- a/src/Indexer/ImportService.php +++ b/src/Indexer/ImportService.php @@ -5,14 +5,13 @@ declare(strict_types=1); namespace MeshVault\Indexer; use MeshVault\Indexer\Contracts\FileEnumeratorInterface; -use MeshVault\Repository\CatalogRepositoryInterface; use MeshVault\Repository\ImportJobRepositoryInterface; use Throwable; /** * Turns a source tree into a durable import job manifest. * - * Grouping policy (explicit, never inferred from file contents): each file is + * Legacy guessed grouping (not user-confirmed associations): each file is * assigned to the model named after its first directory component below the * source root; files sitting directly in the source root join a model named * after the source directory itself. Every model lives in a folder of the same @@ -24,7 +23,7 @@ final readonly class ImportService public function __construct( private FileEnumeratorInterface $enumerator, private ImportJobRepositoryInterface $jobs, - private CatalogRepositoryInterface $catalog, + private LegacyGuessedImportGroupingAdapter $groupingAdapter, private SpaceChecker $space, private string $libraryRoot, ) { @@ -59,15 +58,12 @@ final readonly class ImportService /** @return list> */ private function manifest(string $sourcePath, ScanResult $scan): array { - $models = []; $items = []; foreach ($scan->files as $file) { - $group = $this->group($sourcePath, $file); - $models[$group] ??= $this->catalog->ensureModel('/' . $group, $group); $items[] = [ 'source_path' => $file->sourcePath, 'size' => $file->size, - 'model_id' => $models[$group], + 'model_id' => $this->groupingAdapter->modelFor($sourcePath, $file), ]; } // Scan failures become terminal items so the job reports them durably. @@ -82,13 +78,4 @@ final readonly class ImportService } return $items; } - - private function group(string $sourcePath, ScannedFile $file): string - { - $parent = trim(str_replace('\\', '/', dirname($file->relativePath)), '/'); - if ($parent === '' || $parent === '.') { - return basename(rtrim(str_replace('\\', '/', $sourcePath), '/')); - } - return explode('/', $parent)[0]; - } } diff --git a/src/Indexer/ImportWorker.php b/src/Indexer/ImportWorker.php index 7c0c01d..d3c8542 100644 --- a/src/Indexer/ImportWorker.php +++ b/src/Indexer/ImportWorker.php @@ -7,10 +7,11 @@ namespace MeshVault\Indexer; use MeshVault\Exception\PathSafetyException; use MeshVault\Indexer\Filesystem\PathSafety; use MeshVault\Indexer\SpaceChecker; -use MeshVault\Repository\CatalogRepositoryInterface; +use MeshVault\Catalog\CatalogServiceInterface; use MeshVault\Repository\ImportJobRepositoryInterface; use Psr\Log\LoggerInterface; use Throwable; +use DomainException; /** * Serializes every managed-library mutation behind one advisory lock file and @@ -32,7 +33,7 @@ final readonly class ImportWorker /** @param null|callable(): bool $shouldStop Consulted before each item. */ public function __construct( private ImportJobRepositoryInterface $jobs, - private CatalogRepositoryInterface $catalog, + private CatalogServiceInterface $catalog, private StreamingHasher $hasher, private VerifiedCopier $copier, private StlValidator $validator, @@ -112,20 +113,21 @@ final readonly class ImportWorker $published = $this->publish($plan, $destination); $relative = $this->relative($published); // Catalog and progress commit together, so a crash here simply replays. - $this->jobs->atomic(function () use ($item, $itemId, $plan, $published, $relative): void { - // Update managed_path to the actual published path (with collision suffix if any) - // so import_items.managed_path always points at the published file. - $this->jobs->updateManagedPath($itemId, $relative); - if ($item['model_id'] !== null) { - $this->catalog->recordFile( - (int) $item['model_id'], + $this->catalog->atomic(function () use ($item, $itemId, $plan, $relative): void { + // Catalog locks precede item/job locks on the shared PDO connection. + try { + $this->catalog->registerFile( + $item['model_id'] === null ? null : (int) $item['model_id'], $relative, $plan['hash'], $plan['size'], null, 'Present' ); + } catch (DomainException $error) { + throw new ImportException('CATALOG_CONFLICT', $error->getMessage(), $error); } + $this->jobs->updateManagedPath($itemId, $relative); $this->jobs->finishItem($itemId, 'Completed'); }); } catch (Throwable $error) { diff --git a/src/Indexer/LegacyGuessedImportGroupingAdapter.php b/src/Indexer/LegacyGuessedImportGroupingAdapter.php new file mode 100644 index 0000000..4df2537 --- /dev/null +++ b/src/Indexer/LegacyGuessedImportGroupingAdapter.php @@ -0,0 +1,27 @@ +relativePath)), '/'); + $group = $parent === '' || $parent === '.' + ? basename(rtrim(str_replace('\\', '/', $sourcePath), '/')) + : explode('/', $parent)[0]; + // No retained IDs: rollback and later ambiguity must be visible on every call. + return $this->catalog->ensureModel('/' . $group, $group); + } +} diff --git a/src/Repository/CatalogRepositoryInterface.php b/src/Repository/CatalogRepositoryInterface.php index ec9653b..c077d80 100644 --- a/src/Repository/CatalogRepositoryInterface.php +++ b/src/Repository/CatalogRepositoryInterface.php @@ -15,6 +15,11 @@ interface CatalogRepositoryInterface /** Legacy lookup: create on zero matches, reuse one, reject ambiguity. */ public function ensureModel(string $folderPath, string $modelName): int; + /** Read a normalized stored row without acquiring locks or creating rows. */ + public function file(int $fileId): ?array; + + public function fileByManagedPath(string $managedPath): ?array; + public function recordFile( ?int $modelId, string $managedPath, @@ -35,5 +40,12 @@ interface CatalogRepositoryInterface */ public function changeFileOwner(array $fileIds, ?int $expectedOwnerId, ?int $targetOwnerId): void; - public function setFileStatus(int $fileId, string $status): void; + /** Raw observation; absent models return null. */ + public function modelMetadata(int $modelId): ?array; + + /** Full raw named-tuple CAS; validate changed content only after locked comparison. */ + public function updateModelMetadata(int $modelId, array $expected, array $replacement): array; + + /** Locked status CAS, including no-ops; returns the normalized complete file row. */ + public function changeFileStatus(int $fileId, string $expectedStatus, string $targetStatus): array; } diff --git a/src/Repository/PdoCatalogRepository.php b/src/Repository/PdoCatalogRepository.php index a99e9d7..8a4543b 100644 --- a/src/Repository/PdoCatalogRepository.php +++ b/src/Repository/PdoCatalogRepository.php @@ -7,6 +7,7 @@ namespace MeshVault\Repository; use DomainException; use InvalidArgumentException; use MeshVault\Catalog\ManagedPath; +use MeshVault\Catalog\ModelMetadata; use PDO; final class PdoCatalogRepository extends PdoRepository implements CatalogRepositoryInterface @@ -28,10 +29,7 @@ final class PdoCatalogRepository extends PdoRepository implements CatalogReposit private function createOrFindModel(string $folderPath, string $modelName, bool $legacy): int { - $length = preg_match_all('/./us', $modelName); - if ($length === false || $length > 255 || preg_match('/^\s*$/u', $modelName)) { - throw new InvalidArgumentException('Model name must contain 1 to 255 characters and not be blank.'); - } + ModelMetadata::validateTitle($modelName); if ($folderPath === '') { throw new InvalidArgumentException('A home folder is required.'); } @@ -63,6 +61,40 @@ final class PdoCatalogRepository extends PdoRepository implements CatalogReposit $this->execute("UPDATE folders SET name = name WHERE path = '/'"); } + public function file(int $fileId): ?array + { + if ($fileId <= 0) { + throw new InvalidArgumentException('File ID must be positive.'); + } + return $this->normalizeFile($this->row('SELECT * FROM files WHERE id = ?', [$fileId])); + } + + public function fileByManagedPath(string $managedPath): ?array + { + ManagedPath::validate($managedPath); + $this->validateText($managedPath); + return $this->normalizeFile($this->row('SELECT * FROM files WHERE managed_path = ?', [$managedPath])); + } + + private function normalizeFile(?array $file): ?array + { + if ($file === null) { + return null; + } + foreach (['id', 'model_id', 'folder_id', 'size'] as $field) { + $file[$field] = $file[$field] === null ? null : (int) $file[$field]; + } + return $file; + } + + private function validateText(string $value, ?int $maximum = null): void + { + $length = preg_match_all('/./us', $value); + if ($length === false || str_contains($value, "\0") || ($maximum !== null && $length > $maximum)) { + throw new InvalidArgumentException('File text must be valid UTF-8 without NUL and within length limits.'); + } + } + public function recordFile( ?int $modelId, string $managedPath, @@ -73,6 +105,11 @@ final class PdoCatalogRepository extends PdoRepository implements CatalogReposit ): int { $this->validateStatus($status); ManagedPath::validate($managedPath); + $this->validateText($managedPath); + $this->validateText($hash, 64); + if ($mime !== null) { + $this->validateText($mime, 128); + } if ($modelId !== null && $modelId <= 0) { throw new InvalidArgumentException('Model ID must be positive or null.'); } @@ -82,18 +119,22 @@ final class PdoCatalogRepository extends PdoRepository implements CatalogReposit return $this->transaction(function () use ($modelId, $managedPath, $hash, $size, $mime, $status): int { // Serialize catalog writers on the root row, including paths assigned to different models. $this->lockRoot(); - $existing = $this->row('SELECT * FROM files WHERE managed_path = ?', [$managedPath]); + $lock = $this->pdo->getAttribute(PDO::ATTR_DRIVER_NAME) === 'pgsql' ? ' FOR UPDATE' : ''; + $existing = $this->row('SELECT * FROM files WHERE managed_path = ?' . $lock, [$managedPath]); if ($existing !== null) { $owner = $existing['model_id'] === null ? null : (int) $existing['model_id']; if ( $owner !== $modelId || $existing['hash'] !== $hash - || (int) $existing['size'] !== $size + || $existing['size'] === null || (int) $existing['size'] !== $size ) { throw new DomainException('Managed path already records a different file.'); } return (int) $existing['id']; } - $folder = $this->ensureFolder(dirname(str_replace('\\', '/', $managedPath))); + if ($modelId !== null && $this->row('SELECT id FROM models WHERE id = ?', [$modelId]) === null) { + throw new DomainException('Unknown model.'); + } + $folder = $this->ensureFolder(dirname($managedPath)); $file = $this->row( 'INSERT INTO files (model_id, folder_id, name, managed_path, hash, size, mime_type, status)' . ' VALUES (?, ?, ?, ?, ?, ?, ?, ?) RETURNING id', @@ -164,17 +205,89 @@ final class PdoCatalogRepository extends PdoRepository implements CatalogReposit }); } - public function setFileStatus(int $fileId, string $status): void + public function modelMetadata(int $modelId): ?array { - $this->validateStatus($status); - if ( + $this->validateModelId($modelId); + return $this->readMetadata($modelId); + } + + private function validateModelId(int $modelId): void + { + if ($modelId <= 0) { + throw new InvalidArgumentException('Model ID must be positive.'); + } + } + + private function readMetadata(int $modelId, string $lock = ''): ?array + { + $row = $this->row('SELECT name, notes, source_url FROM models WHERE id = ?' . $lock, [$modelId]); + return $row === null ? null : [ + 'title' => $row['name'], 'notes' => $row['notes'], 'sourceURL' => $row['source_url'], + ]; + } + + public function updateModelMetadata(int $modelId, array $expected, array $replacement): array + { + $this->validateModelId($modelId); + ModelMetadata::validateShape($expected); + ModelMetadata::validateShape($replacement); + return $this->transaction(function () use ($modelId, $expected, $replacement): array { + // Compatible with ownership's implicit FK KEY SHARE; never strengthen to FOR UPDATE. + $lock = $this->pdo->getAttribute(PDO::ATTR_DRIVER_NAME) === 'pgsql' ? ' FOR NO KEY UPDATE' : ''; + $current = $this->readMetadata($modelId, $lock); + if ($current === null) { + throw new DomainException('Unknown model.'); + } + foreach ($current as $field => $value) { + if ($expected[$field] !== $value) { + throw new DomainException('Model metadata has changed.'); + } + } + $next = ModelMetadata::replacement($expected, $replacement); + $columns = []; + $values = []; + foreach (['title' => 'name', 'notes' => 'notes', 'sourceURL' => 'source_url'] as $field => $column) { + if ($next[$field] !== $current[$field]) { + $columns[] = $column . ' = ?'; + $values[] = $next[$field]; + } + } + if ($columns !== []) { + $values[] = $modelId; + $this->execute( + 'UPDATE models SET ' . implode(', ', $columns) . ', updated_at = CURRENT_TIMESTAMP WHERE id = ?', + $values + ); + } + return $next; + }); + } + + public function changeFileStatus(int $fileId, string $expectedStatus, string $targetStatus): array + { + if ($fileId <= 0) { + throw new InvalidArgumentException('File ID must be positive.'); + } + $this->validateStatus($expectedStatus); + $this->validateStatus($targetStatus); + return $this->transaction(function () use ($fileId, $expectedStatus, $targetStatus): array { + $lock = $this->pdo->getAttribute(PDO::ATTR_DRIVER_NAME) === 'pgsql' ? ' FOR UPDATE' : ''; + $file = $this->normalizeFile($this->row('SELECT * FROM files WHERE id = ?' . $lock, [$fileId])); + if ($file === null) { + throw new DomainException('Unknown file.'); + } + if ($file['status'] !== $expectedStatus) { + throw new DomainException('File status has changed.'); + } + if ($expectedStatus === $targetStatus) { + return $file; + } $this->execute( 'UPDATE files SET status = ?, updated_at = CURRENT_TIMESTAMP WHERE id = ?', - [$status, $fileId] - )->rowCount() === 0 - ) { - throw new DomainException('Unknown file.'); - } + [$targetStatus, $fileId] + ); + return $this->file($fileId); + }); } private function validateStatus(string $status): void diff --git a/tests/Integration/CatalogContractsStatement.php b/tests/Integration/CatalogContractsStatement.php new file mode 100644 index 0000000..2c4cd10 --- /dev/null +++ b/tests/Integration/CatalogContractsStatement.php @@ -0,0 +1,25 @@ +trace[] = $this->queryString; + return parent::execute($params); + } +} diff --git a/tests/Integration/CatalogContractsTest.php b/tests/Integration/CatalogContractsTest.php new file mode 100644 index 0000000..3313402 --- /dev/null +++ b/tests/Integration/CatalogContractsTest.php @@ -0,0 +1,433 @@ +physicalRoot = RepositoryDatabase::directory('contract-files'); + $ids = []; + foreach (['parts/one.stl', 'parts/two.stl', 'images/photo.png', 'docs/manual.pdf'] as $path) { + if (!is_dir(dirname($this->physicalRoot . '/' . $path))) { + mkdir(dirname($this->physicalRoot . '/' . $path)); + } + $bytes = "fixture\0" . $path; + file_put_contents($this->physicalRoot . '/' . $path, $bytes); + touch($this->physicalRoot . '/' . $path, 1000000000); + $ids[] = $this->service->registerFile($owner, $path, hash('sha256', $bytes), strlen($bytes)); + } + return $ids; + } + + protected function setUp(): void + { + $this->database = tempnam(sys_get_temp_dir(), 'meshvault-contracts-'); + $this->pdo = RepositoryDatabase::migrate($this->database); + $this->repository = new PdoCatalogRepository($this->pdo); + $this->service = new CatalogService($this->repository); + } + + protected function tearDown(): void + { + unset($this->service, $this->repository, $this->pdo); + gc_collect_cycles(); + RepositoryDatabase::cleanup($this->database); + if (isset($this->physicalRoot)) { + RepositoryDatabase::remove($this->physicalRoot); + } + } + + public function testServiceBoundaryAndNormalizedReadOnlyLookups(): void + { + self::assertInstanceOf(CatalogServiceInterface::class, $this->service); + foreach ([$this->repository, $this->service] as $catalog) { + self::assertNull($catalog->file(999)); + self::assertNull($catalog->fileByManagedPath('absent.stl')); + } + self::assertSame(0, (int) $this->pdo->query('SELECT COUNT(*) FROM folders')->fetchColumn()); + $id = $this->service->registerFile(null, 'physical/nested/a.stl', 'hash', 0); + $file = $this->service->file($id); + self::assertSame($file, $this->repository->fileByManagedPath('physical/nested/a.stl')); + self::assertSame($id, $file['id']); + self::assertNull($file['model_id']); + self::assertIsInt($file['folder_id']); + self::assertSame(0, $file['size']); + $this->pdo->exec('UPDATE files SET hash = NULL, size = NULL'); + self::assertNull($this->service->file($id)['hash']); + self::assertNull($this->service->file($id)['size']); + } + + public function testRegistrationDerivesFoldersAndReplayPreservesEveryColumn(): void + { + $owner = $this->service->createModel('/home', 'Model'); + $id = $this->service->registerFile($owner, 'physical/nested/a.stl', 'hash', 1, 'model/stl', 'Missing'); + $before = $this->service->file($id); + self::assertSame($owner, $before['model_id']); + self::assertSame('/physical/nested', $this->pdo->query( + 'SELECT path FROM folders WHERE id = ' . $before['folder_id'] + )->fetchColumn()); + self::assertSame($id, $this->service->registerFile( + $owner, + 'physical/nested/a.stl', + 'hash', + 1, + 'other', + 'Present' + )); + self::assertSame($before, $this->service->file($id)); + self::assertNotSame($id, $this->service->registerFile(null, 'b.stl', 'hash', 1)); + $this->service->detachFiles($owner, [$id]); + $this->expectException(DomainException::class); + $this->expectExceptionMessage('Managed path already records a different file.'); + $this->service->registerFile($owner, 'physical/nested/a.stl', 'hash', 1); + } + + #[DataProvider('invalidRegistration')] + public function testDirectRegistrationValidation(array $arguments): void + { + try { + $this->repository->recordFile(...$arguments); + self::fail('Malformed registration must fail'); + } catch (InvalidArgumentException) { + self::assertSame(0, (int) $this->pdo->query('SELECT COUNT(*) FROM folders')->fetchColumn()); + } + } + + public function testMissingOwnerIsDomainFailureWithoutFolderWrites(): void + { + try { + $this->repository->recordFile(999, 'new/nested/a.stl', 'hash', 0); + self::fail('Missing owner must fail'); + } catch (DomainException $error) { + self::assertSame('Unknown model.', $error->getMessage()); + self::assertSame(0, (int) $this->pdo->query('SELECT COUNT(*) FROM folders')->fetchColumn()); + } + } + + public function testNullStoredSizeDoesNotReplayAsZero(): void + { + $this->repository->recordFile(null, 'a.stl', 'hash', 0); + $this->pdo->exec('UPDATE files SET size = NULL'); + $this->expectException(DomainException::class); + $this->repository->recordFile(null, 'a.stl', 'hash', 0); + } + + public function testAtomicReturnRollbackAndFailurePreservesCallerWrites(): void + { + self::assertSame('result', $this->service->atomic(static fn (): string => 'result')); + $this->pdo->beginTransaction(); + $owner = $this->service->createModel('/home', 'Earlier'); + $error = new RuntimeException('later failure'); + try { + $this->service->atomic(function () use ($owner, $error): void { + $this->service->registerFile($owner, 'new/nested/a.stl', 'hash', 1); + throw $error; + }); + self::fail('Failure must escape'); + } catch (RuntimeException $caught) { + self::assertSame($error, $caught); + } + self::assertTrue($this->pdo->inTransaction()); + self::assertNotNull($this->service->aggregate($owner)); + self::assertNull($this->service->fileByManagedPath('new/nested/a.stl')); + self::assertSame(2, (int) $this->pdo->query('SELECT COUNT(*) FROM folders')->fetchColumn()); + $id = $this->service->registerFile(null, 'a.stl', 'hash', 1); + self::assertTrue($this->pdo->inTransaction()); + $this->pdo->rollBack(); + self::assertNull($this->service->file($id)); + self::assertNull($this->service->aggregate($owner)); + } + + public function testLookupValidationIsEnforcedByRepositoryAndService(): void + { + foreach ([$this->repository, $this->service] as $catalog) { + foreach ( + [['file', 0], ['file', -1], ['fileByManagedPath', '../a'], + ['fileByManagedPath', "\xff.stl"], ['fileByManagedPath', 'a//b']] as [$method, $value] + ) { + try { + $catalog->$method($value); + self::fail('Invalid lookup must fail'); + } catch (InvalidArgumentException) { + self::assertSame(0, (int) $this->pdo->query('SELECT COUNT(*) FROM folders')->fetchColumn()); + } + } + } + } + + public function testMetadataAndStatusBoundaryExists(): void + { + foreach ([$this->repository, $this->service] as $catalog) { + foreach (['modelMetadata', 'updateModelMetadata', 'changeFileStatus'] as $method) { + self::assertTrue(method_exists($catalog, $method), $method); + } + self::assertFalse(method_exists($catalog, 'setFileStatus')); + } + } + + public function testMetadataRawNamedTupleNoopLegacyAndAba(): void + { + $id = $this->service->createModel('/home', 'T'); + $file = $this->service->registerFile($id, 'a.stl', 'hash', 1); + $this->pdo->exec("UPDATE models SET notes = '', source_url = 'bad URL', updated_at = '2000-01-01'"); + $before = $this->service->aggregate($id); + $raw = ['title' => 'T', 'notes' => '', 'sourceURL' => 'bad URL']; + self::assertSame($raw, $this->repository->modelMetadata($id)); + self::assertSame($raw, $this->service->updateModelMetadata($id, array_reverse($raw, true), $raw)); + self::assertSame($before, $this->service->aggregate($id)); + $edited = array_replace($raw, ['title' => 'New']); + self::assertSame($edited, $this->repository->updateModelMetadata($id, $raw, $edited)); + $after = $this->service->aggregate($id); + foreach (['id', 'folder_id', 'added_at', 'notes', 'source_url'] as $key) { + self::assertSame($before['model'][$key], $after['model'][$key]); + } + self::assertSame($before['files'], $after['files']); + self::assertNotSame($before['model']['updated_at'], $after['model']['updated_at']); + $this->service->updateModelMetadata($id, $edited, $raw); + self::assertSame($edited, $this->service->updateModelMetadata($id, $raw, $edited)); + self::assertSame($file, $after['files'][0]['id']); + $cleared = array_replace($edited, ['sourceURL' => null]); + self::assertSame($cleared, $this->service->updateModelMetadata($id, $edited, $cleared)); + } + + public function testMetadataStaleBeforeValidationOrNoopAndIndependentFieldsConflict(): void + { + $id = $this->service->createModel('/', 'T'); + $raw = $this->service->modelMetadata($id); + $current = array_replace($raw, ['notes' => 'changed']); + $this->service->updateModelMetadata($id, $raw, $current); + foreach ([$current, array_replace($raw, ['title' => "bad\0"]), $raw] as $replacement) { + try { + $this->repository->updateModelMetadata($id, $raw, $replacement); + self::fail('Stale tuple must fail'); + } catch (DomainException $error) { + self::assertSame('Model metadata has changed.', $error->getMessage()); + } + } + self::assertSame($current, $this->service->modelMetadata($id)); + } + + public function testEveryStatusPairPreservesOtherColumnsAndNoopTimestamp(): void + { + $id = $this->service->registerFile(null, 'a.stl', 'hash', 5, 'model/stl'); + foreach (['Present', 'Missing', 'Updated', 'Duplicate'] as $from) { + foreach (['Present', 'Missing', 'Updated', 'Duplicate'] as $to) { + $this->pdo->exec("UPDATE files SET status = '$from', updated_at = '2000-01-01'"); + $before = $this->service->file($id); + $after = $this->service->changeFileStatus($id, $from, $to); + self::assertSame($to, $after['status']); + self::assertSame($after, $this->repository->file($id)); + if ($from === $to) { + self::assertSame($before, $after); + } else { + self::assertNotSame($before['updated_at'], $after['updated_at']); + } + unset($before['status'], $before['updated_at'], $after['status'], $after['updated_at']); + self::assertSame($before, $after); + } + } + $this->expectException(DomainException::class); + $this->expectExceptionMessage('File status has changed.'); + $this->repository->changeFileStatus($id, 'Present', 'Duplicate'); + } + + public function testCreationRejectsNulThroughBothPaths(): void + { + foreach (['createModel', 'ensureModel'] as $method) { + try { + $this->repository->$method('/', "bad\0title"); + self::fail('NUL must fail'); + } catch (InvalidArgumentException) { + self::assertSame(0, (int) $this->pdo->query('SELECT COUNT(*) FROM models')->fetchColumn()); + } + } + } + + public function testMutationValidationAndMissingIdentities(): void + { + $tuple = ['title' => 'T', 'notes' => null, 'sourceURL' => null]; + foreach ([$this->repository, $this->service] as $catalog) { + self::assertNull($catalog->modelMetadata(999)); + $invalid = [ + ['modelMetadata', [0]], ['modelMetadata', [-1]], + ['updateModelMetadata', [0, $tuple, $tuple]], + ['updateModelMetadata', [999, [], $tuple]], + ['updateModelMetadata', [999, $tuple, []]], + ['changeFileStatus', [0, 'Present', 'Present']], + ['changeFileStatus', [999, 'invalid', 'Present']], + ['changeFileStatus', [999, 'Present', 'invalid']], + ]; + foreach ($invalid as [$method, $args]) { + try { + $catalog->$method(...$args); + self::fail('Invalid input must fail'); + } catch (InvalidArgumentException) { + self::assertNull($catalog->modelMetadata(999)); + } + } + foreach ( + [['updateModelMetadata', [999, $tuple, $tuple], 'Unknown model.'], + ['changeFileStatus', [999, 'Present', 'Present'], 'Unknown file.']] as [$method, $args, $message] + ) { + try { + $catalog->$method(...$args); + self::fail('Missing identity must fail'); + } catch (DomainException $error) { + self::assertSame($message, $error->getMessage()); + } + } + } + } + + public function testMetadataEffectiveNoopAndRawNullDistinction(): void + { + $id = $this->service->createModel('/', 'T'); + $this->pdo->exec("UPDATE models SET updated_at = '2000-01-01'"); + $raw = $this->service->modelMetadata($id); + $before = $this->service->aggregate($id); + self::assertSame($raw, $this->service->updateModelMetadata( + $id, + $raw, + array_replace($raw, ['notes' => '', 'sourceURL' => '']) + )); + self::assertSame($before, $this->service->aggregate($id)); + $this->pdo->exec("UPDATE models SET source_url = ''"); + try { + $this->service->updateModelMetadata($id, $raw, $raw); + self::fail('Raw null must not match empty string'); + } catch (DomainException $error) { + self::assertSame('Model metadata has changed.', $error->getMessage()); + } + $raw = $this->service->modelMetadata($id); + self::assertSame('', $this->service->updateModelMetadata($id, $raw, $raw)['sourceURL']); + self::assertNull($this->service->updateModelMetadata( + $id, + $raw, + array_replace($raw, ['sourceURL' => null]) + )['sourceURL']); + } + + public function testPhysicalSnapshotsSurviveMutationsNoopsAndRejectedChanges(): void + { + $owner = $this->service->createModel('/', 'Multipart'); + $other = $this->service->createModel('/other', 'Other'); + $ids = $this->physicalFiles($owner); + $snapshot = PhysicalFileSnapshot::capture($this->physicalRoot); + $raw = $this->service->modelMetadata($owner); + $edited = array_replace($raw, ['title' => 'Edited', 'notes' => 'Notes']); + $operations = [ + fn () => $this->service->updateModelMetadata($owner, $raw, $edited), + fn () => $this->service->updateModelMetadata($owner, $edited, $edited), + fn () => $this->service->changeFileStatus($ids[0], 'Present', 'Missing'), + fn () => $this->service->changeFileStatus($ids[0], 'Missing', 'Missing'), + fn () => $this->service->reassignFiles($owner, $other, $ids), + fn () => $this->repository->changeFileOwner($ids, $other, $other), + fn () => $this->service->detachFiles($other, $ids), + fn () => $this->repository->changeFileOwner($ids, null, null), + fn () => $this->service->assignFiles($owner, $ids), + ]; + foreach ($operations as $index => $operation) { + $operation(); + self::assertSame($snapshot, PhysicalFileSnapshot::capture($this->physicalRoot), "Mutation $index"); + } + $before = $this->service->aggregate($owner); + $rejections = [ + fn () => $this->service->updateModelMetadata($owner, $raw, $edited), + fn () => $this->service->updateModelMetadata($owner, $edited, array_replace($edited, ['title' => ''])), + fn () => $this->service->changeFileStatus($ids[0], 'Present', 'Missing'), + fn () => $this->service->changeFileStatus($ids[0], 'Missing', 'invalid'), + fn () => $this->service->reassignFiles($other, $owner, $ids), + ]; + foreach ($rejections as $index => $operation) { + try { + $operation(); + self::fail('Expected rejected mutation'); + } catch (DomainException | InvalidArgumentException) { + self::assertSame($before, $this->service->aggregate($owner)); + self::assertSame($snapshot, PhysicalFileSnapshot::capture($this->physicalRoot), "Rejection $index"); + } + } + } + + public function testMutationsRollbackWithCallerAndLaterSqlFailure(): void + { + $id = $this->service->createModel('/', 'T'); + $files = $this->physicalFiles($id); + $file = $files[0]; + $snapshot = PhysicalFileSnapshot::capture($this->physicalRoot); + $raw = $this->service->modelMetadata($id); + $new = array_replace($raw, ['title' => 'New']); + $before = $this->service->file($file); + $this->pdo->beginTransaction(); + $earlier = $this->service->createModel('/', 'Earlier'); + $postgres = $this->pdo->getAttribute(\PDO::ATTR_DRIVER_NAME) === 'pgsql'; + if ($postgres) { + $this->pdo->exec('ALTER TABLE files ADD CONSTRAINT "forced later write" CHECK (status <> \'Missing\')'); + } else { + $this->pdo->exec("CREATE TRIGGER fail_status BEFORE UPDATE OF status ON files + BEGIN SELECT RAISE(ABORT, 'forced later write'); END"); + } + try { + $this->service->atomic(function () use ($id, $file, $files, $raw, $new): void { + $this->service->detachFiles($id, $files); + $this->service->updateModelMetadata($id, $raw, $new); + $this->repository->changeFileStatus($file, 'Present', 'Missing'); + }); + self::fail('SQL failure must escape'); + } catch (\PDOException $error) { + self::assertStringContainsString('forced later write', $error->getMessage()); + } + self::assertTrue($this->pdo->inTransaction()); + self::assertNotNull($this->service->aggregate($earlier)); + self::assertSame($raw, $this->service->modelMetadata($id)); + self::assertSame($before, $this->service->file($file)); + self::assertSame($files, array_column($this->service->aggregate($id)['files'], 'id')); + self::assertSame($snapshot, PhysicalFileSnapshot::capture($this->physicalRoot)); + $this->pdo->exec($postgres + ? 'ALTER TABLE files DROP CONSTRAINT "forced later write"' + : 'DROP TRIGGER fail_status'); + $this->service->updateModelMetadata($id, $raw, $new); + $this->service->changeFileStatus($file, 'Present', 'Missing'); + $this->service->detachFiles($id, $files); + self::assertSame($snapshot, PhysicalFileSnapshot::capture($this->physicalRoot)); + self::assertTrue($this->pdo->inTransaction()); + $this->pdo->rollBack(); + self::assertSame($raw, $this->service->modelMetadata($id)); + self::assertSame($before, $this->service->file($file)); + self::assertNull($this->service->aggregate($earlier)); + self::assertSame($files, array_column($this->service->aggregate($id)['files'], 'id')); + self::assertSame($snapshot, PhysicalFileSnapshot::capture($this->physicalRoot)); + } + + public static function invalidRegistration(): array + { + return array_map(static fn (array $args): array => [$args], [ + [null, 'a.stl', '', 0], [null, 'a.stl', str_repeat('a', 65), 0], + [null, 'a.stl', "bad\0hash", 0], [null, 'a.stl', "\xff", 0], + [null, 'a.stl', 'hash', -1], [0, 'a.stl', 'hash', 0], + [null, 'a.stl', 'hash', 0, str_repeat('a', 129)], + [null, 'a.stl', 'hash', 0, "bad\0mime"], [null, 'a.stl', 'hash', 0, "\xff"], + [null, "\xff.stl", 'hash', 0], [null, '../a.stl', 'hash', 0], + [null, 'a.stl', 'hash', 0, null, 'invalid'], + ]); + } +} diff --git a/tests/Integration/ImportPipelineTest.php b/tests/Integration/ImportPipelineTest.php index 4478903..2272888 100644 --- a/tests/Integration/ImportPipelineTest.php +++ b/tests/Integration/ImportPipelineTest.php @@ -57,7 +57,7 @@ final class ImportPipelineTest extends TestCase return new ImportService( new FileEnumerator(new LocalFilesystem(), $safety), $this->jobs, - $this->catalog, + new \MeshVault\Indexer\LegacyGuessedImportGroupingAdapter($this->catalog), $space ?? new SpaceChecker(), $this->library, ); @@ -69,7 +69,7 @@ final class ImportPipelineTest extends TestCase $sourceSafety = new PathSafety($this->source); return new ImportWorker( $this->jobs, - $this->catalog, + new \MeshVault\Catalog\CatalogService($this->catalog), new StreamingHasher($sourceSafety), new VerifiedCopier($safety), new StlValidator($sourceSafety), @@ -93,7 +93,113 @@ final class ImportPipelineTest extends TestCase return $path; } - public function testImportCreatesJobWithExplicitSubfolderGrouping(): void + public function testNullOwnerIsRegisteredAndScanFailuresAreNot(): void + { + $source = $this->writeStl('loose.stl'); + $job = $this->jobs->create($this->source, [ + ['source_path' => $source, 'size' => filesize($source), 'model_id' => null], + ['source_path' => 'unreadable', 'size' => 0, 'model_id' => null, + 'status' => 'Failed', 'failure_reason' => 'SCAN_READ_ERROR'], + ]); + $this->worker()->runOnce(); + $files = $this->pdo->query('SELECT * FROM files')->fetchAll(PDO::FETCH_ASSOC); + self::assertCount(1, $files); + self::assertNull($files[0]['model_id']); + self::assertSame('Completed', $this->jobs->items($job)[0]['status']); + } + + public function testRegistrationConflictReportsCatalogConflictAndPreservesBytes(): void + { + $source = $this->writeStl('planes/wing.stl'); + $job = $this->service()->import($this->source); + $other = $this->catalog->ensureModel('/other', 'other'); + $id = $this->catalog->recordFile($other, 'planes/wing.stl', hash_file('sha256', $source), filesize($source)); + $sourceBefore = PhysicalFileSnapshot::capture($this->source); + $this->worker()->runOnce(); + $item = $this->jobs->items($job)[0]; + self::assertSame('Failed', $item['status']); + self::assertStringStartsWith('CATALOG_CONFLICT:', $item['failure_reason']); + self::assertSame($other, (int) $this->catalog->file($id)['model_id']); + self::assertSame($sourceBefore, PhysicalFileSnapshot::capture($this->source)); + self::assertSame( + $sourceBefore['planes/wing.stl']['bytes'], + file_get_contents($this->library . '/planes/wing.stl') + ); + } + + public function testCompletionRegistersBeforeUpdatingItemPath(): void + { + if ($this->pdo->getAttribute(PDO::ATTR_DRIVER_NAME) !== 'sqlite') { + self::markTestSkipped('SQLite ordering assertion trigger.'); + } + $this->writeStl('planes/wing.stl'); + $job = $this->service()->import($this->source); + $this->pdo->exec("CREATE TRIGGER catalog_first BEFORE UPDATE OF managed_path ON import_items" + . " WHEN OLD.hash IS NOT NULL AND NOT EXISTS (SELECT 1 FROM files WHERE managed_path = NEW.managed_path)" + . " BEGIN SELECT RAISE(ABORT, 'catalog must precede progress'); END"); + $this->worker()->runOnce(); + self::assertSame('Completed', $this->jobs->items($job)[0]['status']); + } + + public function testMissingNonNullOwnerFailsRatherThanBecomingUnassigned(): void + { + if ($this->pdo->getAttribute(PDO::ATTR_DRIVER_NAME) !== 'sqlite') { + self::markTestSkipped('SQLite fixture deliberately simulates a stale manifest owner.'); + } + $source = $this->writeStl('loose.stl'); + $this->pdo->exec('PRAGMA foreign_keys = OFF'); + $job = $this->jobs->create($this->source, [ + ['source_path' => $source, 'size' => filesize($source), 'model_id' => 999999], + ]); + $this->pdo->exec('PRAGMA foreign_keys = ON'); + $sourceBefore = PhysicalFileSnapshot::capture($this->source); + $this->worker()->runOnce(); + $item = $this->jobs->items($job)[0]; + self::assertSame('Failed', $item['status']); + self::assertStringStartsWith('CATALOG_CONFLICT:', $item['failure_reason']); + self::assertSame(0, (int) $this->pdo->query('SELECT COUNT(*) FROM files')->fetchColumn()); + self::assertSame($sourceBefore, PhysicalFileSnapshot::capture($this->source)); + self::assertSame($sourceBefore['loose.stl']['bytes'], file_get_contents($this->library . '/unfiled/loose.stl')); + } + + public function testCompletionSqlFailureRollsBackCatalogPathAndProgressInsideCallerTransaction(): void + { + if ($this->pdo->getAttribute(PDO::ATTR_DRIVER_NAME) !== 'sqlite') { + self::markTestSkipped('SQLite failure-injection trigger.'); + } + $source = $this->writeStl('planes/wing.stl'); + $job = $this->service()->import($this->source); + mkdir($this->library . '/planes'); + file_put_contents($this->library . '/planes/wing.stl', 'different bytes'); + $sourceBefore = PhysicalFileSnapshot::capture($this->source); + $libraryBefore = PhysicalFileSnapshot::capture($this->library); + // Fail after registration and item-path update, during progress refresh. + $this->pdo->exec("CREATE TRIGGER reject_completion BEFORE UPDATE ON import_jobs" + . " WHEN NEW.status = 'Completed' BEGIN SELECT RAISE(ABORT, 'injected completion failure'); END"); + $this->pdo->beginTransaction(); + $sentinel = $this->catalog->createModel('/sentinel', 'sentinel'); + $this->worker()->runOnce(); + self::assertTrue($this->pdo->inTransaction()); + self::assertNotNull($this->catalog->aggregate($sentinel)); + self::assertSame(0, (int) $this->pdo->query('SELECT COUNT(*) FROM files')->fetchColumn()); + $item = $this->jobs->items($job)[0]; + self::assertSame('planes/wing.stl', $item['managed_path']); + self::assertSame('Failed', $item['status']); + self::assertStringStartsWith('COPY_FAILED:', $item['failure_reason']); + self::assertSame('Failed', $this->jobs->get($job)['status']); + self::assertSame($sourceBefore, PhysicalFileSnapshot::capture($this->source)); + $published = PhysicalFileSnapshot::capture($this->library); + self::assertSame($libraryBefore['planes/wing.stl'], $published['planes/wing.stl']); + self::assertSame($sourceBefore['planes/wing.stl']['bytes'], $published['planes/wing_001.stl']['bytes']); + self::assertSame($sourceBefore['planes/wing.stl']['size'], $published['planes/wing_001.stl']['size']); + $this->pdo->rollBack(); + self::assertSame($sourceBefore, PhysicalFileSnapshot::capture($this->source)); + self::assertSame($published, PhysicalFileSnapshot::capture($this->library)); + self::assertNull($this->catalog->aggregate($sentinel)); + self::assertSame('Pending', $this->jobs->items($job)[0]['status']); + } + + public function testImportCreatesJobWithLegacyGuessedSubfolderGrouping(): void { $this->writeStl('planes/wing.stl'); $this->writeStl('planes/body.stl'); @@ -108,7 +214,7 @@ final class ImportPipelineTest extends TestCase $aggregate = $this->catalog->aggregate((int) $item['model_id']); $models[$aggregate['model']['name']] = $aggregate['folder']['path']; } - // Explicit policy: one model per immediate source subfolder; root files join + // Legacy guessed grouping: one model per immediate source subfolder; root files join // a model named after the source directory itself. self::assertSame('/planes', $models['planes']); self::assertSame('/' . basename($this->source), $models[basename($this->source)]); @@ -353,7 +459,7 @@ final class ImportPipelineTest extends TestCase $sourceSafety = new PathSafety($this->source); $worker = new ImportWorker( $this->jobs, - $this->catalog, + new \MeshVault\Catalog\CatalogService($this->catalog), new StreamingHasher($sourceSafety), new VerifiedCopier($safety), new StlValidator($sourceSafety), @@ -390,7 +496,7 @@ final class ImportPipelineTest extends TestCase $sourceSafety = new PathSafety($this->source); $worker = new ImportWorker( $this->jobs, - $this->catalog, + new \MeshVault\Catalog\CatalogService($this->catalog), new StreamingHasher($sourceSafety), new VerifiedCopier($safety), new StlValidator($sourceSafety), diff --git a/tests/Integration/LegacyGroupingAdapterTest.php b/tests/Integration/LegacyGroupingAdapterTest.php new file mode 100644 index 0000000..e321d81 --- /dev/null +++ b/tests/Integration/LegacyGroupingAdapterTest.php @@ -0,0 +1,192 @@ +database = tempnam(sys_get_temp_dir(), 'meshvault-grouping-'); + $this->pdo = RepositoryDatabase::migrate($this->database); + $this->catalog = new PdoCatalogRepository($this->pdo); + $this->jobs = new PdoImportJobRepository($this->pdo); + $this->adapter = new LegacyGuessedImportGroupingAdapter($this->catalog); + $this->library = RepositoryDatabase::directory('library'); + $this->source = RepositoryDatabase::directory('source'); + } + + protected function tearDown(): void + { + unset($this->adapter, $this->catalog, $this->jobs, $this->pdo); + RepositoryDatabase::cleanup($this->database); + RepositoryDatabase::remove($this->library); + RepositoryDatabase::remove($this->source); + } + + public function testSameAdapterResolvesPersistedModelAfterRollback(): void + { + $file = $this->file('planes/wing.stl'); + $before = $this->snapshot(); + $failure = new RuntimeException('Force rollback after grouping.'); + try { + $this->jobs->atomic(function () use ($file, $failure): void { + $id = $this->adapter->modelFor($this->source, $file); + self::assertSame($id, (int) $this->pdo->query('SELECT id FROM models')->fetchColumn()); + throw $failure; + }); + self::fail('The transaction must roll back.'); + } catch (RuntimeException $error) { + self::assertSame($failure, $error); + } + self::assertSame($before, $this->snapshot()); + + // SQLite can reuse rolled-back IDs: require an actual persisted row, not a different ID. + $id = $this->adapter->modelFor($this->source, $file); + $rows = $this->pdo->query( + 'SELECT m.id, m.name, f.path FROM models m JOIN folders f ON f.id = m.folder_id' + )->fetchAll(PDO::FETCH_ASSOC); + self::assertCount(1, $rows); + self::assertSame($id, (int) $rows[0]['id']); + self::assertSame('planes', $rows[0]['name']); + self::assertSame('/planes', $rows[0]['path']); + self::assertSame($id, $this->adapter->modelFor($this->source, $file)); + } + + public function testSameAdapterRejectsAmbiguityIntroducedBetweenCalls(): void + { + $file = $this->file('planes/wing.stl'); + $first = $this->adapter->modelFor($this->source, $file); + $second = $this->catalog->createModel('/planes', 'planes'); + self::assertNotSame($first, $second); + $before = $this->snapshot(); + + try { + $this->adapter->modelFor($this->source, $file); + self::fail('A previously resolved title must not hide new ambiguity.'); + } catch (DomainException $error) { + self::assertSame('Ambiguous model title in home folder.', $error->getMessage()); + } + self::assertSame($before, $this->snapshot()); + } + + public function testRepeatedImportsReuseModelsWithTheSameServiceAndAdapter(): void + { + $service = $this->service([$this->file('planes/wing.stl'), $this->file('planes/nested/body.stl')]); + $first = $service->import($this->source); + $before = $this->snapshot(); + $second = $service->import($this->source); + + self::assertNotSame($first, $second); + $firstItems = $this->jobs->items($first); + $secondItems = $this->jobs->items($second); + self::assertCount(2, $firstItems); + self::assertCount(2, $secondItems); + $model = (int) $firstItems[0]['model_id']; + self::assertGreaterThan(0, $model); + foreach (array_merge($firstItems, $secondItems) as $item) { + self::assertSame($model, (int) $item['model_id']); + self::assertSame('Pending', $item['status']); + } + $after = $this->snapshot(); + self::assertSame($before['models'], $after['models']); + self::assertSame($before['folders'], $after['folders']); + self::assertCount(1, $after['models']); + self::assertCount(2, $after['import_jobs']); + self::assertCount(4, $after['import_items']); + } + + + public function testAmbiguityBetweenImportsRollsBackEarlierManifestModelCreation(): void + { + $plane = $this->file('planes/wing.stl'); + $service = $this->service([$plane], [$this->file('new-group/part.stl'), $plane]); + $firstJob = $service->import($this->source); + $firstModel = (int) $this->jobs->items($firstJob)[0]['model_id']; + $duplicate = $this->catalog->createModel('/planes', 'planes'); + self::assertNotSame($firstModel, $duplicate); + $before = $this->snapshot(); + + try { + // The ordered scan creates /new-group before encountering the now-ambiguous title. + $service->import($this->source); + self::fail('An ambiguous title must reject the entire import.'); + } catch (ImportException $error) { + self::assertSame('DB_ERROR', $error->reason); + self::assertInstanceOf(DomainException::class, $error->getPrevious()); + self::assertSame('Ambiguous model title in home folder.', $error->getPrevious()->getMessage()); + } + + self::assertFalse($this->pdo->inTransaction()); + self::assertSame($before, $this->snapshot(), 'No model, folder, job or item may survive the failed import.'); + self::assertSame(0, (int) $this->pdo->query( + "SELECT COUNT(*) FROM folders WHERE path = '/new-group'" + )->fetchColumn()); + } + + private function file(string $relative): ScannedFile + { + return new ScannedFile($this->source . '/' . $relative, $relative, 12, 'model/stl'); + } + + /** @param list $files @param list|null $nextFiles */ + private function service(array $files, ?array $nextFiles = null): ImportService + { + // Control scan order so creation precedes ambiguity. Grouping and persistence remain real. + $enumerator = new class ($files, $nextFiles) implements FileEnumeratorInterface { + public function __construct(private array $files, private ?array $nextFiles) + { + } + + public function enumerate(string $sourceRoot, array $extensions = self::DEFAULT_EXTENSIONS): ScanResult + { + $files = $this->files; + $this->files = $this->nextFiles ?? $files; + $bytes = array_sum(array_map(static fn (ScannedFile $file): int => $file->size, $files)); + return new ScanResult($files, $bytes, []); + } + }; + + return new ImportService( + $enumerator, + $this->jobs, + $this->adapter, + new SpaceChecker(0.05, static fn (string $path): float => 1000000.0), + $this->library, + ); + } + + /** @return array>> */ + private function snapshot(): array + { + $snapshot = []; + foreach (['folders', 'models', 'import_jobs', 'import_items'] as $table) { + $snapshot[$table] = $this->pdo->query('SELECT * FROM ' . $table . ' ORDER BY id') + ->fetchAll(PDO::FETCH_ASSOC); + } + return $snapshot; + } +} diff --git a/tests/Integration/PhysicalFileSnapshot.php b/tests/Integration/PhysicalFileSnapshot.php new file mode 100644 index 0000000..0c8bf5e --- /dev/null +++ b/tests/Integration/PhysicalFileSnapshot.php @@ -0,0 +1,30 @@ +getPathname(); + clearstatcache(true, $path); + $relative = str_replace('\\', '/', substr($path, strlen($root) + 1)); + $snapshot[$relative] = [ + 'bytes' => file_get_contents($path), + 'size' => filesize($path), + 'mtime' => filemtime($path), + ]; + } + ksort($snapshot); + return $snapshot; + } +} diff --git a/tests/Integration/PostgresCatalogContractsTest.php b/tests/Integration/PostgresCatalogContractsTest.php new file mode 100644 index 0000000..e2d2c67 --- /dev/null +++ b/tests/Integration/PostgresCatalogContractsTest.php @@ -0,0 +1,306 @@ +database = tempnam(sys_get_temp_dir(), 'meshvault-contracts-'); + $this->a = RepositoryDatabase::migrate($this->database); + $this->b = RepositoryDatabase::connect($this->database); + foreach ([$this->a, $this->b] as $pdo) { + $pdo->exec("SET lock_timeout = '250ms'"); + $pdo->exec("SET statement_timeout = '5s'"); + } + self::assertNotSame( + $this->a->query('SELECT pg_backend_pid()')->fetchColumn(), + $this->b->query('SELECT pg_backend_pid()')->fetchColumn(), + ); + self::assertSame( + $this->a->query('SELECT current_schema()')->fetchColumn(), + $this->b->query('SELECT current_schema()')->fetchColumn(), + ); + $this->first = new PdoCatalogRepository($this->a); + $this->second = new PdoCatalogRepository($this->b); + } + + protected function tearDown(): void + { + foreach (['a', 'b'] as $property) { + if (isset($this->$property) && $this->$property->inTransaction()) { + $this->$property->rollBack(); + } + } + unset($this->first, $this->second, $this->a, $this->b); + if (isset($this->database)) { + RepositoryDatabase::cleanup($this->database); + } + } + + private function timeout(callable $operation): void + { + try { + $operation(); + self::fail('Expected PostgreSQL lock timeout; a replay/no-op must not escape the lock.'); + } catch (PDOException $error) { + self::assertSame('55P03', $error->getCode()); + self::assertStringContainsString('lock timeout', $error->getMessage()); + } + } + + private function conflict(callable $operation, string $message): void + { + try { + $operation(); + self::fail('Expected stale state rejection.'); + } catch (DomainException $error) { + self::assertSame($message, $error->getMessage()); + } + } + + public static function ownershipCases(): iterable + { + foreach (['assign', 'reassign', 'detach', 'noop', 'null-noop'] as $kind) { + foreach ([true, false] as $commit) { + yield $kind . ($commit ? '-commit' : '-rollback') => [$kind, $commit]; + } + } + } + + #[DataProvider('ownershipCases')] + public function testRegistrationAgainstOwnershipAndReplayRetention(string $kind, bool $commit): void + { + $source = $this->first->createModel('/home', 'Source'); + $target = $this->first->createModel('/home', 'Target'); + $old = in_array($kind, ['assign', 'null-noop'], true) ? null : $source; + $next = match ($kind) { + 'detach', 'null-noop' => null, + 'noop' => $source, + default => $target, + }; + $file = $this->first->recordFile($old, 'physical/nested/part.stl', 'hash', 1); + $this->a->beginTransaction(); + $this->first->changeFileOwner([$file], $old, $next); + $this->timeout(fn () => $this->second->recordFile($old, 'physical/nested/part.stl', 'hash', 1)); + $commit ? $this->a->commit() : $this->a->rollBack(); + $current = $commit ? $next : $old; + if ($current !== $old) { + $this->conflict( + fn () => $this->second->recordFile($old, 'physical/nested/part.stl', 'hash', 1), + 'Managed path already records a different file.', + ); + } + $this->b->beginTransaction(); + self::assertSame($file, $this->second->recordFile($current, 'physical/nested/part.stl', 'hash', 1)); + $this->timeout(fn () => $this->first->changeFileOwner([$file], $current, $current)); + $this->timeout(fn () => $this->first->createModel('/unrelated', 'Root retained')); + $this->b->commit(); + $this->first->changeFileOwner([$file], $current, $current); + self::assertSame($current, $this->first->file($file)['model_id']); + } + + #[DataProvider('ownershipCases')] + public function testMetadataKeyShareComposition(string $kind, bool $commit): void + { + // Reverse backend roles on rollback as well as exercising both transaction outcomes. + [$a, $b, $first, $second] = $commit + ? [$this->a, $this->b, $this->first, $this->second] + : [$this->b, $this->a, $this->second, $this->first]; + $model = $first->createModel('/home', 'Metadata target'); + $source = $first->createModel('/home', 'Other owner'); + $owner = match ($kind) { + 'assign', 'null-noop' => null, + 'noop', 'detach' => $model, + default => $source, + }; + $target = in_array($kind, ['detach', 'null-noop'], true) ? null : $model; + $file = $first->recordFile($owner, 'physical/part.stl', 'hash', 1); + $old = $first->modelMetadata($model); + $new = array_replace($old, ['notes' => 'metadata']); + $a->beginTransaction(); + $first->updateModelMetadata($model, $old, $new); + $b->beginTransaction(); + $second->changeFileOwner([$file], $owner, $owner); + // B holds the file, A retains metadata after its operation savepoint times out. + $this->timeout(fn () => $first->changeFileOwner([$file], $owner, $target)); + // Real FK acquisition for assign/reassign, not a simulated KEY SHARE query. + $second->changeFileOwner([$file], $owner, $target); + $this->timeout(fn () => $second->updateModelMetadata($model, $old, $old)); + $commit ? $b->commit() : $b->rollBack(); + $current = $commit ? $target : $owner; + $first->changeFileOwner([$file], $current, $target); + self::assertSame($new, $first->modelMetadata($model)); + self::assertSame($target, $first->file($file)['model_id']); + $a->commit(); + self::assertSame($new, $second->modelMetadata($model)); + self::assertSame($target, $second->file($file)['model_id']); + } + + public function testIndependentStatusOwnershipAndNoUnnecessaryRootLocks(): void + { + $model = $this->first->createModel('/home', 'Model'); + $file = $this->first->recordFile($model, 'physical/part.stl', 'hash', 1); + $old = $this->first->modelMetadata($model); + $this->a->beginTransaction(); + $this->a->exec("UPDATE folders SET name = name WHERE path = '/'"); + $this->second->updateModelMetadata($model, $old, $old); + $this->second->changeFileStatus($file, 'Present', 'Missing'); + $this->second->changeFileOwner([$file], $model, null); + $this->a->rollBack(); + $this->b->beginTransaction(); + $before = $this->second->file($file); + self::assertSame($before, $this->second->changeFileStatus($file, 'Missing', 'Missing')); + $this->timeout(fn () => $this->first->changeFileOwner([$file], null, $model)); + $this->b->commit(); + $this->first->changeFileOwner([$file], null, $model); + self::assertSame($file, $this->first->recordFile($model, 'physical/part.stl', 'hash', 1)); + self::assertSame('Missing', $this->first->file($file)['status']); + self::assertSame($model, $this->first->file($file)['model_id']); + self::assertSame($before['updated_at'], $this->first->file($file)['updated_at']); + $this->b->beginTransaction(); + $this->second->updateModelMetadata($model, $old, $old); + $this->timeout(fn () => $this->first->updateModelMetadata($model, $old, $old)); + $this->b->rollBack(); + self::assertSame($old, $this->first->updateModelMetadata($model, $old, $old)); + } + + #[DataProvider('outcomes')] + public function testNewPathSerializationAndPhysicalFolders(bool $commit): void + { + $model = $this->first->createModel('/organizational', 'Model'); + $this->a->beginTransaction(); + $file = $this->first->recordFile($model, 'physical/nested/part.stl', 'hash', 1); + $this->timeout(fn () => $this->second->recordFile(null, 'physical/nested/part.stl', 'hash', 1)); + self::assertSame(0, (int) $this->b->query('SELECT COUNT(*) FROM files')->fetchColumn()); + $commit ? $this->a->commit() : $this->a->rollBack(); + if ($commit) { + $this->conflict( + fn () => $this->second->recordFile(null, 'physical/nested/part.stl', 'hash', 1), + 'Managed path already records a different file.', + ); + self::assertSame($file, $this->second->recordFile($model, 'physical/nested/part.stl', 'hash', 1)); + } else { + self::assertSame(0, (int) $this->b->query( + "SELECT COUNT(*) FROM folders WHERE path LIKE '/physical%'", + )->fetchColumn()); + $file = $this->second->recordFile(null, 'physical/nested/part.stl', 'hash', 1); + } + self::assertSame(1, (int) $this->b->query('SELECT COUNT(*) FROM files')->fetchColumn()); + $folder = $this->second->file($file)['folder_id']; + self::assertSame('/physical/nested', $this->b->query( + 'SELECT path FROM folders WHERE id = ' . $folder, + )->fetchColumn()); + self::assertNotSame((int) $this->second->aggregate($model)['model']['folder_id'], $folder); + } + + public function testRegistrationTraceAndSqlFailureRollback(): void + { + $model = $this->first->createModel('/home', 'Model'); + $trace = new \ArrayObject(); + $this->a->setAttribute(PDO::ATTR_STATEMENT_CLASS, [CatalogContractsStatement::class, [$trace]]); + $file = $this->first->recordFile($model, 'physical/nested/part.stl', 'hash', 1); + $sql = $trace->getArrayCopy(); + $insert = array_key_last($sql); + self::assertStringContainsString('INSERT INTO files', $sql[$insert]); + self::assertStringContainsString('SELECT id FROM folders', $sql[$insert - 1]); + foreach ([false, true] as $conflict) { + $trace->exchangeArray([]); + if ($conflict) { + $this->conflict( + fn () => $this->first->recordFile(null, 'physical/nested/part.stl', 'hash', 1), + 'Managed path already records a different file.', + ); + } else { + self::assertSame($file, $this->first->recordFile($model, 'physical/nested/part.stl', 'hash', 1)); + } + $sql = $trace->getArrayCopy(); + self::assertCount(4, $sql); + self::assertSame("UPDATE folders SET name = name WHERE path = '/'", $sql[2]); + self::assertSame('SELECT * FROM files WHERE managed_path = ? FOR UPDATE', $sql[3]); + } + $this->a->exec("ALTER TABLE files ADD CONSTRAINT reject_test_hash CHECK (hash <> 'rejected')"); + $this->a->beginTransaction(); + $this->a->exec("INSERT INTO tags (name) VALUES ('sentinel')"); + try { + $this->first->recordFile(null, 'rollback/nested/part.stl', 'rejected', 1); + self::fail('Expected actual SQL constraint failure.'); + } catch (PDOException $error) { + self::assertSame('23514', $error->getCode()); + } + self::assertTrue($this->a->inTransaction()); + self::assertSame('sentinel', $this->a->query('SELECT name FROM tags')->fetchColumn()); + self::assertSame(0, (int) $this->a->query( + "SELECT COUNT(*) FROM folders WHERE path LIKE '/rollback%'", + )->fetchColumn()); + // The operation's root lock was released, while earlier caller work remains uncommitted. + $this->second->createModel('/after-failure', 'Independent'); + self::assertSame(0, (int) $this->b->query('SELECT COUNT(*) FROM tags')->fetchColumn()); + $this->a->commit(); + self::assertSame('sentinel', $this->b->query('SELECT name FROM tags')->fetchColumn()); + } + + public static function outcomes(): array + { + return ['commit' => [true], 'rollback' => [false]]; + } + + #[DataProvider('outcomes')] + public function testMetadataAndStatusStaleEditsIncludingEqualTarget(bool $commit): void + { + $model = $this->first->createModel('/home', 'Original'); + $file = $this->first->recordFile($model, 'physical/part.stl', 'hash', 1); + $old = $this->first->modelMetadata($model); + $new = array_replace($old, ['notes' => 'winner']); + $this->a->beginTransaction(); + $this->first->updateModelMetadata($model, $old, $new); + $this->first->changeFileStatus($file, 'Present', 'Missing'); + $this->b->beginTransaction(); + $this->b->exec("INSERT INTO tags (name) VALUES ('caller survives')"); + foreach ([$old, $new] as $replacement) { + $this->timeout(fn () => $this->second->updateModelMetadata($model, $old, $replacement)); + } + foreach (['Present', 'Missing'] as $target) { + $this->timeout(fn () => $this->second->changeFileStatus($file, 'Present', $target)); + } + self::assertTrue($this->b->inTransaction()); + self::assertSame('caller survives', $this->b->query('SELECT name FROM tags')->fetchColumn()); + $commit ? $this->a->commit() : $this->a->rollBack(); + if ($commit) { + $this->conflict( + fn () => $this->second->updateModelMetadata($model, $old, $new), + 'Model metadata has changed.', + ); + $this->conflict( + fn () => $this->second->changeFileStatus($file, 'Present', 'Missing'), + 'File status has changed.', + ); + } else { + self::assertSame($new, $this->second->updateModelMetadata($model, $old, $new)); + self::assertSame('Missing', $this->second->changeFileStatus($file, 'Present', 'Missing')['status']); + } + self::assertTrue($this->b->inTransaction()); + $this->b->rollBack(); + self::assertSame($commit ? $new : $old, $this->first->modelMetadata($model)); + self::assertSame($commit ? 'Missing' : 'Present', $this->first->file($file)['status']); + self::assertSame(0, (int) $this->a->query('SELECT COUNT(*) FROM tags')->fetchColumn()); + } +} diff --git a/tests/Integration/RepositoryTest.php b/tests/Integration/RepositoryTest.php index 2d6dfda..5732126 100644 --- a/tests/Integration/RepositoryTest.php +++ b/tests/Integration/RepositoryTest.php @@ -52,7 +52,7 @@ final class RepositoryTest extends TestCase $file = $catalog->recordFile($model, 'vehicles/aircraft/wing.stl', str_repeat('a', 64), 12, 'model/stl'); self::assertSame($file, $catalog->recordFile($model, 'vehicles/aircraft/wing.stl', str_repeat('a', 64), 12)); foreach (['Missing', 'Updated', 'Duplicate', 'Present'] as $status) { - $catalog->setFileStatus($file, $status); + $catalog->changeFileStatus($file, $catalog->file($file)['status'], $status); self::assertSame($status, $catalog->aggregate($model)['files'][0]['status']); } $aggregate = $catalog->aggregate($model); diff --git a/tests/Unit/Catalog/ModelMetadataTest.php b/tests/Unit/Catalog/ModelMetadataTest.php new file mode 100644 index 0000000..1a946f3 --- /dev/null +++ b/tests/Unit/Catalog/ModelMetadataTest.php @@ -0,0 +1,100 @@ +expectException(InvalidArgumentException::class); + ModelMetadata::validateTitle($title); + } + + public static function invalidTitles(): array + { + return [[''], [" \t\n"], ["a\0b"], ["\xff"], [str_repeat('é', 256)]]; + } + + public function testPreservesValidAndUnchangedLegacyValues(): void + { + $old = ['title' => '', 'notes' => "\xff", 'sourceURL' => 'not a URL']; + self::assertSame($old, ModelMetadata::replacement($old, $old)); + $new = ['title' => ' ' . str_repeat('é', 253) . ' ', 'notes' => "\xff", 'sourceURL' => 'not a URL']; + self::assertSame($new, ModelMetadata::replacement($old, $new)); + $old = ['title' => 'T', 'notes' => 'old', 'sourceURL' => 'old']; + self::assertSame(['title' => 'T', 'notes' => null, 'sourceURL' => null], ModelMetadata::replacement( + $old, + ['title' => 'T', 'notes' => '', 'sourceURL' => ''] + )); + ModelMetadata::validateTitle('é'); + self::assertSame(" \n", ModelMetadata::replacement($old, array_replace($old, ['notes' => " \n"]))['notes']); + } + + #[DataProvider('urls')] + public function testUrls(string $url, bool $valid): void + { + $old = ['title' => 'T', 'notes' => null, 'sourceURL' => null]; + if (!$valid) { + $this->expectException(InvalidArgumentException::class); + } + $result = ModelMetadata::replacement($old, array_replace($old, ['sourceURL' => $url])); + self::assertSame($url, $result['sourceURL']); + } + + public static function urls(): array + { + $valid = ['https://example.com/a?q=x#f', 'http://127.0.0.1:80/', 'https://[::1]/', + 'https://xn--bcher-kva.example/%C3%A9', 'https://example.com/' . str_repeat('a', 2028)]; + $invalid = ['relative', 'ftp://example.com', 'https://', 'https://u:p@example.com', + 'https://u@example.com', "https://example.com/\n", 'https://example.com/a b', + 'https://example.com/\\x', 'https://example.com/%zz', 'https://example.com/%', + 'https://example.com/é', 'http://example.com:65536', 'http://example.com:abc', + 'https://example.com/' . str_repeat('a', 2030)]; + return array_merge( + array_map(static fn ($v) => [$v, true], $valid), + array_map(static fn ($v) => [$v, false], $invalid) + ); + } + + #[DataProvider('badShapes')] + public function testExactShape(array $shape): void + { + $this->expectException(InvalidArgumentException::class); + ModelMetadata::validateShape($shape); + } + + public static function badShapes(): array + { + return [[[]], [['title' => 'T', 'notes' => null]], + [['title' => null, 'notes' => null, 'sourceURL' => null]], + [['title' => 'T', 'notes' => 1, 'sourceURL' => null]], + [['title' => 'T', 'notes' => null, 'sourceURL' => false]], + [['title' => 'T', 'notes' => null, 'sourceURL' => null, 'extra' => null]]]; + } + + #[DataProvider('badNotes')] + public function testChangedNotesMustBePortable(string $notes): void + { + $old = ['title' => 'T', 'notes' => null, 'sourceURL' => null]; + $this->expectException(InvalidArgumentException::class); + ModelMetadata::replacement($old, array_replace($old, ['notes' => $notes])); + } + + public static function badNotes(): array + { + return [["\xff"], ["a\0b"]]; + } +} diff --git a/tests/Unit/Console/ContainerWiringTest.php b/tests/Unit/Console/ContainerWiringTest.php index 3ab31b2..84eae3b 100644 --- a/tests/Unit/Console/ContainerWiringTest.php +++ b/tests/Unit/Console/ContainerWiringTest.php @@ -37,6 +37,38 @@ final class ContainerWiringTest extends TestCase $this->pdo->exec('PRAGMA foreign_keys = ON'); } + public function testActualFactoryInvocationUsesSharedServiceAndStopCallback(): void + { + $config = AppConfig::fromEnvironment([ + 'MESHVAULT_LIBRARY_DIR' => dirname(__DIR__, 3), 'MESHVAULT_DB_HOST' => 'unused', + 'MESHVAULT_DB_NAME' => 'unused', 'MESHVAULT_DB_USER' => 'unused', + 'MESHVAULT_DB_PASSWORD' => 'unused', + ]); + $definitions = require dirname(__DIR__, 3) . '/config/container.php'; + $builder = new \DI\ContainerBuilder(); + $builder->useAutowiring(false); + $builder->addDefinitions($definitions($config)); + $builder->addDefinitions([ + PDO::class => $this->pdo, + \Psr\Log\LoggerInterface::class => new \Psr\Log\NullLogger(), + ]); + $container = $builder->build(); + $stop = static fn (): bool => true; + $worker = ($container->get(\MeshVault\Console\ImportWorkerFactory::class))($stop); + $service = $container->get(\MeshVault\Catalog\CatalogServiceInterface::class); + self::assertSame($container->get(\MeshVault\Catalog\CatalogService::class), $service); + $reflection = new \ReflectionObject($worker); + self::assertSame($service, $reflection->getProperty('catalog')->getValue($worker)); + self::assertSame($stop, $reflection->getProperty('shouldStop')->getValue($worker)); + foreach ([CatalogRepositoryInterface::class, ImportJobRepositoryInterface::class] as $class) { + $repository = $container->get($class); + $property = (new \ReflectionObject($repository))->getProperty('pdo'); + self::assertSame($this->pdo, $property->getValue($repository)); + } + $import = $container->get(\MeshVault\Indexer\ImportService::class); + self::assertInstanceOf(\MeshVault\Indexer\ImportService::class, $import); + } + public function testRepositoriesShareSamePdoInstance(): void { // L5: assert that the container would wire both repositories to the SAME PDO