diff --git a/app/Actions/Album/Move.php b/app/Actions/Album/Move.php index ad175ea1f1c..2fb27f49a84 100644 --- a/app/Actions/Album/Move.php +++ b/app/Actions/Album/Move.php @@ -8,6 +8,7 @@ namespace App\Actions\Album; +use App\Events\AlbumSaved; use App\Exceptions\ModelDBException; use App\Models\Album; use Illuminate\Database\Eloquent\ModelNotFoundException; @@ -27,21 +28,46 @@ public function do(?Album $target_album, Collection $albums): void if ($target_album !== null) { /** @var Album $album */ foreach ($albums as $album) { + $old_parent_id = $album->parent_id; // Don't set attribute `parent_id` manually, but use specialized // methods of the nested set `NodeTrait` to keep the enumeration // of the tree consistent // `appendNode` also internally calls `save` on the model $target_album->appendNode($album); + AlbumSaved::dispatch($album); + $this->dispatchForOldParentIfChanged($old_parent_id, $album->parent_id); } $target_album->fixOwnershipOfChildren(); } else { /** @var Album $album */ foreach ($albums as $album) { + $old_parent_id = $album->parent_id; // Don't set attribute `parent_id` manually, but use specialized // methods of the nested set `NodeTrait` to keep the enumeration // of the tree consistent $album->saveAsRoot(); + AlbumSaved::dispatch($album); + $this->dispatchForOldParentIfChanged($old_parent_id, $album->parent_id); } } } + + /** + * Also dispatches `AlbumSaved` for the album's *previous* parent when it + * actually changed, mirroring `Photo\MoveOrDuplicate`'s source/destination + * dispatch pattern: the old parent's set of children changed too, and its + * managed-cache tag (FR-052-06) would otherwise never be evicted since + * nothing else carries its id after the move completes. + */ + private function dispatchForOldParentIfChanged(?string $old_parent_id, ?string $new_parent_id): void + { + if ($old_parent_id === null || $old_parent_id === $new_parent_id) { + return; + } + + $old_parent = Album::find($old_parent_id); + if ($old_parent !== null) { + AlbumSaved::dispatch($old_parent); + } + } } \ No newline at end of file diff --git a/app/Actions/Photo/Pipes/Shared/Save.php b/app/Actions/Photo/Pipes/Shared/Save.php index 0a54d9be587..ca88411b14b 100644 --- a/app/Actions/Photo/Pipes/Shared/Save.php +++ b/app/Actions/Photo/Pipes/Shared/Save.php @@ -10,6 +10,7 @@ use App\Contracts\PhotoCreate\PhotoDTO; use App\Contracts\PhotoCreate\PhotoPipe; +use App\Events\PhotoSaved; /** * Persist current Photo object into database. @@ -20,6 +21,7 @@ public function handle(PhotoDTO $state, \Closure $next): PhotoDTO { $state->getPhoto()->save(); $state->getPhoto()->tags()->sync($state->getTags()->pluck('id')->all()); + PhotoSaved::dispatch($state->getPhoto()->id); return $next($state); } diff --git a/app/Events/AccessPermissionChanged.php b/app/Events/AccessPermissionChanged.php new file mode 100644 index 00000000000..ee9fd4be2e2 --- /dev/null +++ b/app/Events/AccessPermissionChanged.php @@ -0,0 +1,27 @@ + fn ($query) => $query ->when(config('features.hide-lychee-SE', false) === true, fn ($q) => $q->where('cat', '!=', 'lychee SE')) - ->when(config('features.enable-request-caching') === false, fn ($q) => $q->where('cat', '!=', 'Mod Cache')) + // managed_cache_enabled/managed_cache_ttl (Feature 052) share the "Mod Cache" category but must + // stay independent of Feature 040's cache_enabled/enable-request-caching gate (Q-052-07). + ->when(config('features.enable-request-caching') === false, fn ($q) => $q->where(fn ($q2) => $q2 + ->where('cat', '!=', 'Mod Cache') + ->orWhereIn('key', ['managed_cache_enabled', 'managed_cache_ttl']) + )) ->when($docker_info->isDocker(), fn ($q) => $q->where('not_on_docker', '!=', true)) ->when(!$request->verify()->is_supporter() && !$request->configs()->getValueAsBool('enable_se_preview'), fn ($q) => $q->where('level', '=', 0)) ->when(!$request->verify()->is_pro(), fn ($q) => $q->where('level', '<', 2)) diff --git a/app/Http/Controllers/Admin/UserGroupsManagementController.php b/app/Http/Controllers/Admin/UserGroupsManagementController.php index c0fd387e17f..faac697ba79 100644 --- a/app/Http/Controllers/Admin/UserGroupsManagementController.php +++ b/app/Http/Controllers/Admin/UserGroupsManagementController.php @@ -8,6 +8,7 @@ namespace App\Http\Controllers\Admin; +use App\Events\UserGroupMembershipChanged; use App\Http\Requests\UserGroup\GetUserGroupRequest; use App\Http\Requests\UserGroup\ManageUserGroupRequest; use App\Http\Resources\Models\UserGroupResource; @@ -26,6 +27,7 @@ public function get(GetUserGroupRequest $request): UserGroupResource public function addUser(ManageUserGroupRequest $request): UserGroupResource { $request->user_group()->users()->attach($request->user2()->id, ['role' => $request->role()->value]); + UserGroupMembershipChanged::dispatch($request->user2()->id); return new UserGroupResource($request->user_group()); } @@ -33,6 +35,7 @@ public function addUser(ManageUserGroupRequest $request): UserGroupResource public function removeUser(ManageUserGroupRequest $request): UserGroupResource { $request->user_group()->users()->detach($request->user2()->id); + UserGroupMembershipChanged::dispatch($request->user2()->id); return new UserGroupResource($request->user_group()); } @@ -41,6 +44,7 @@ public function updateUserRole(ManageUserGroupRequest $request): UserGroupResour { $request->user_group()->users()->updateExistingPivot($request->user2()->id, ['role' => $request->role()->value]); $request->user_group()->load('users'); + UserGroupMembershipChanged::dispatch($request->user2()->id); return new UserGroupResource($request->user_group()); } diff --git a/app/Http/Controllers/Gallery/PhotoController.php b/app/Http/Controllers/Gallery/PhotoController.php index e57e6988110..7a51cd5e64f 100644 --- a/app/Http/Controllers/Gallery/PhotoController.php +++ b/app/Http/Controllers/Gallery/PhotoController.php @@ -18,6 +18,7 @@ use App\Enum\FileStatus; use App\Enum\SizeVariantType; use App\Events\PhotoHighlightToggled; +use App\Events\PhotoSaved; use App\Events\PhotoTagsChanged; use App\Exceptions\ConfigurationException; use App\Exceptions\ConflictingPropertyException; @@ -176,6 +177,7 @@ public function update(EditPhotoRequest $request): PhotoResource $photo->taken_at = $request->takenAt() ?? $photo->initial_taken_at; $photo->save(); + PhotoSaved::dispatch($photo->id); return new PhotoResource( photo: $photo, diff --git a/app/Http/Controllers/Gallery/SharingController.php b/app/Http/Controllers/Gallery/SharingController.php index 7e39880ab76..bfe7cbbc48a 100644 --- a/app/Http/Controllers/Gallery/SharingController.php +++ b/app/Http/Controllers/Gallery/SharingController.php @@ -12,6 +12,7 @@ use App\Actions\Sharing\Propagate; use App\Actions\Sharing\Share; use App\Constants\AccessPermissionConstants as APC; +use App\Events\AccessPermissionChanged; use App\Exceptions\Internal\LycheeLogicException; use App\Http\Requests\Sharing\AddSharingRequest; use App\Http\Requests\Sharing\DeleteSharingRequest; @@ -75,6 +76,8 @@ public function create(AddSharingRequest $request, Share $share): array base_album_id: $album_id ); } + + AccessPermissionChanged::dispatch($album_id); } return AccessPermissionResource::collect($access_permissions); @@ -98,6 +101,8 @@ public function edit(EditSharingRequest $request): AccessPermissionResource 'grants_delete' => $request->permResource()->grants_delete, ]); + AccessPermissionChanged::dispatch($perm->base_album_id); + return AccessPermissionResource::fromModel($perm); } @@ -173,7 +178,9 @@ public function listAlbums(ListAllSharingRequest $request, ListAlbums $list_albu */ public function delete(DeleteSharingRequest $request): void { + $base_album_id = $request->perm()->base_album_id; AccessPermission::query()->where('id', '=', $request->perm()->id)->delete(); + AccessPermissionChanged::dispatch($base_album_id); } /** @@ -195,5 +202,10 @@ public function propagate(PropagateSharingRequest $request, Propagate $propagate } else { $propagate->update($album); } + + $affected_album_ids = $album->descendants()->getQuery()->select('id')->pluck('id')->push($album->id); + foreach ($affected_album_ids as $affected_album_id) { + AccessPermissionChanged::dispatch($affected_album_id); + } } } diff --git a/app/Listeners/ManagedCacheAlbumInvalidator.php b/app/Listeners/ManagedCacheAlbumInvalidator.php new file mode 100644 index 00000000000..0fb9ca5359c --- /dev/null +++ b/app/Listeners/ManagedCacheAlbumInvalidator.php @@ -0,0 +1,111 @@ +evictAlbumAndParent($event->album->id, $event->album->parent_id); + } + + /** + * `AlbumDeleted` carries only the deleted album's parent id, not its own id + * (the row is already gone by the time the event fires). Only the parent's + * tag is evicted, which is functionally sufficient: nothing can ever query + * a deleted album's own cached listings again (Q-052-06, Option A). + */ + public function handleAlbumDeleted(AlbumDeleted $event): void + { + $this->managed_cache_service->forgetTag(self::PREFIX . ($event->parent_id ?? 'root')); + } + + public function handleAccessPermissionChanged(AccessPermissionChanged $event): void + { + $this->evictAlbumAndParentById($event->base_album_id); + } + + public function handlePhotoSaved(PhotoSaved $event): void + { + $this->evictAlbumsForPhoto($event->photo_id); + } + + public function handlePhotoAdded(PhotoAdded $event): void + { + $this->evictAlbumsForPhoto($event->photo_id); + } + + public function handlePhotoDeleted(PhotoDeleted $event): void + { + $this->evictAlbumAndParentById($event->album_id); + } + + public function handlePhotoMoved(PhotoMoved $event): void + { + $this->evictAlbumAndParentById($event->from_album_id); + $this->evictAlbumAndParentById($event->to_album_id); + } + + /** + * Resolve a photo to its containing album(s) via the `photo_album` pivot, + * mirroring `AlbumRouteCacheRefresher::handle()`. + */ + private function evictAlbumsForPhoto(string $photo_id): void + { + $album_ids = DB::table(PA::PHOTO_ALBUM) + ->select(PA::ALBUM_ID) + ->where(PA::PHOTO_ID, '=', $photo_id) + ->distinct() + ->pluck('album_id') + ->all(); + + foreach ($album_ids as $album_id) { + /** @var string $album_id */ + $this->evictAlbumAndParentById($album_id); + } + } + + private function evictAlbumAndParentById(string $album_id): void + { + // Plain query builder (not the Eloquent `Album` model) to avoid pulling in + // `Album`'s eager-loaded relations for what is otherwise a single-column read. + $parent_id = DB::table('albums')->where('id', '=', $album_id)->value('parent_id'); + /** @var string|null $parent_id */ + $this->evictAlbumAndParent($album_id, $parent_id); + } + + private function evictAlbumAndParent(string $album_id, ?string $parent_id): void + { + $this->managed_cache_service->forgetTag(self::PREFIX . $album_id); + $this->managed_cache_service->forgetTag(self::PREFIX . ($parent_id ?? 'root')); + } +} diff --git a/app/Listeners/ManagedCacheUserInvalidator.php b/app/Listeners/ManagedCacheUserInvalidator.php new file mode 100644 index 00000000000..a4d97c68a04 --- /dev/null +++ b/app/Listeners/ManagedCacheUserInvalidator.php @@ -0,0 +1,29 @@ +managed_cache_service->forgetTag('user:' . $event->user_id); + } +} diff --git a/app/Providers/EventServiceProvider.php b/app/Providers/EventServiceProvider.php index 432cf046a85..4439c05e66a 100644 --- a/app/Providers/EventServiceProvider.php +++ b/app/Providers/EventServiceProvider.php @@ -8,6 +8,7 @@ namespace App\Providers; +use App\Events\AccessPermissionChanged; use App\Events\AlbumDeleted; use App\Events\AlbumRouteCacheUpdated; use App\Events\AlbumSaved; @@ -29,9 +30,12 @@ use App\Events\PhotoTagsChanged; use App\Events\PhotoWillBeDeleted; use App\Events\TaggedRouteCacheUpdated; +use App\Events\UserGroupMembershipChanged; use App\Listeners\AlbumCacheCleaner; use App\Listeners\CacheListener; use App\Listeners\LogQueryTimeout; +use App\Listeners\ManagedCacheAlbumInvalidator; +use App\Listeners\ManagedCacheUserInvalidator; use App\Listeners\MetricsListener; use App\Listeners\OrderCompletedListener; use App\Listeners\RecomputeAlbumSizeOnAlbumChange; @@ -141,6 +145,15 @@ public function boot(): void Event::listen(Login::class, RotateLicenseKeyOnLogin::class . '@handle'); + Event::listen(AlbumSaved::class, ManagedCacheAlbumInvalidator::class . '@handleAlbumSaved'); + Event::listen(AlbumDeleted::class, ManagedCacheAlbumInvalidator::class . '@handleAlbumDeleted'); + Event::listen(AccessPermissionChanged::class, ManagedCacheAlbumInvalidator::class . '@handleAccessPermissionChanged'); + Event::listen(PhotoSaved::class, ManagedCacheAlbumInvalidator::class . '@handlePhotoSaved'); + Event::listen(PhotoAdded::class, ManagedCacheAlbumInvalidator::class . '@handlePhotoAdded'); + Event::listen(PhotoDeleted::class, ManagedCacheAlbumInvalidator::class . '@handlePhotoDeleted'); + Event::listen(PhotoMoved::class, ManagedCacheAlbumInvalidator::class . '@handlePhotoMoved'); + Event::listen(UserGroupMembershipChanged::class, ManagedCacheUserInvalidator::class . '@handle'); + // Webhook dispatch for photo lifecycle events Event::listen(PhotoAdded::class, WebhookListener::class . '@handlePhotoAdded'); Event::listen(PhotoMoved::class, WebhookListener::class . '@handlePhotoMoved'); diff --git a/app/Repositories/AlbumRepository.php b/app/Repositories/AlbumRepository.php index 809040d5f88..14429074f7b 100644 --- a/app/Repositories/AlbumRepository.php +++ b/app/Repositories/AlbumRepository.php @@ -13,7 +13,9 @@ use App\Models\Extensions\SortingDecorator; use App\Models\User; use App\Policies\AlbumQueryPolicy; +use App\Services\Cache\ManagedCacheService; use Illuminate\Pagination\LengthAwarePaginator; +use Illuminate\Pagination\Paginator; use Illuminate\Support\Facades\Auth; /** @@ -25,12 +27,22 @@ class AlbumRepository { public function __construct( protected AlbumQueryPolicy $album_query_policy, + protected ManagedCacheService $managed_cache_service, + protected ConfigManager $config_manager, ) { } /** * Get paginated child albums with all necessary relations eager-loaded. * + * Wrapped in {@see ManagedCacheService::remember()} (FR-052-09): the cache + * key is scoped to the parent album, sorting, pagination, and requesting + * user; the entry is tagged with the parent's own tag (so any change that + * evicts the parent's tag, per FR-052-06, invalidates this listing) plus + * each returned child's own tag (so a future per-child invalidation, e.g. + * on rename, invalidates the listing too, even though nothing dispatches + * that yet). + * * @param string|null $album_id the parent album ID (null for root albums) * @param AlbumSortingCriterion $sorting the sorting criteria * @param int $per_page number of albums per page @@ -45,23 +57,51 @@ public function getChildrenPaginated( AlbumSortingCriterion $sorting, int $per_page, ): LengthAwarePaginator { - // Build query for child albums - $query = Album::query() - ->with(['owner']) - ->without(['thumb']) // Yes we do NOT want them yet. - ->where('parent_id', '=', $album_id); - - // Apply visibility filter /** @var ?User $user */ $user = Auth::user(); - $query = $this->album_query_policy->applyVisibilityFilter($query, $user); + $page = Paginator::resolveCurrentPage(); + $parent_tag = 'album:' . ($album_id ?? 'root'); + $key = sprintf( + 'children:%s:%s:%s:%d:%d:%s', + $album_id ?? 'root', + $sorting->column->value, + $sorting->order->value, + $per_page, + $page, + $user?->id ?? 'guest', + ); + + /** @var LengthAwarePaginator $result */ + $result = $this->managed_cache_service->remember( + $key, + [$parent_tag], + $this->config_manager->getValueAsInt('managed_cache_ttl'), + function () use ($album_id, $sorting, $per_page, $user): LengthAwarePaginator { + // Build query for child albums + $query = Album::query() + ->with(['owner']) + ->without(['thumb']) // Yes we do NOT want them yet. + ->where('parent_id', '=', $album_id); + + // Apply visibility filter + $query = $this->album_query_policy->applyVisibilityFilter($query, $user); + + // Apply sorting via SortingDecorator + /** @var SortingDecorator */ + $sorting_decorator = new SortingDecorator($query); + + return $sorting_decorator + ->orderBy($sorting->column, $sorting->order) + ->paginate($per_page); + }, + ); - // Apply sorting via SortingDecorator - /** @var SortingDecorator */ - $sorting_decorator = new SortingDecorator($query); + $child_tags = array_map( + static fn (Album $child): string => 'album:' . $child->id, + $result->items(), + ); + $this->managed_cache_service->addTags($key, $child_tags); - return $sorting_decorator - ->orderBy($sorting->column, $sorting->order) - ->paginate($per_page); + return $result; } } diff --git a/app/Repositories/PhotoRepository.php b/app/Repositories/PhotoRepository.php index 650bad234be..32e5928b168 100644 --- a/app/Repositories/PhotoRepository.php +++ b/app/Repositories/PhotoRepository.php @@ -13,7 +13,9 @@ use App\Models\Extensions\FiltersUploadValidation; use App\Models\Extensions\SortingDecorator; use App\Models\Photo; +use App\Services\Cache\ManagedCacheService; use Illuminate\Pagination\LengthAwarePaginator; +use Illuminate\Pagination\Paginator; use Illuminate\Support\Facades\Auth; /** @@ -25,6 +27,12 @@ class PhotoRepository { use FiltersUploadValidation; + public function __construct( + protected ManagedCacheService $managed_cache_service, + protected ConfigManager $config_manager, + ) { + } + /** * Get paginated photos for an album with all necessary relations eager-loaded. * @@ -58,38 +66,60 @@ public function getPhotosForAlbumPaginated( string $tag_logic = 'OR', ?string $person_id = null, ): LengthAwarePaginator { - $relations = ['size_variants', 'tags', 'palette', 'statistics', 'rating']; + $user = Auth::user(); + $page = Paginator::resolveCurrentPage(); + $key = sprintf( + 'photos:%s:%s:%s:%d:%d:%s:%s:%s:%s', + $album_id, + $sorting->column->value, + $sorting->order->value, + $per_page, + $page, + implode(',', $tag_ids ?? []), + $tag_logic, + $person_id ?? '', + $user?->id ?? 'guest', + ); - // Build query for photos belonging to the album via the photo_album pivot table - $query = Photo::query() - ->join(PA::PHOTO_ALBUM, PA::PHOTO_ID, '=', 'photos.id') - ->where(PA::ALBUM_ID, '=', $album_id) - ->select('photos.*') - ->with($relations); + /** @var LengthAwarePaginator */ + return $this->managed_cache_service->remember( + $key, + ['album:' . $album_id], + $this->config_manager->getValueAsInt('managed_cache_ttl'), + function () use ($album_id, $sorting, $per_page, $tag_ids, $tag_logic, $person_id, $user): LengthAwarePaginator { + $relations = ['size_variants', 'tags', 'palette', 'statistics', 'rating']; - // Apply tag filtering if tag_ids provided and not empty - if ($tag_ids !== null && count($tag_ids) > 0) { - $this->applyTagFilter($query, $tag_ids, $tag_logic); - } + // Build query for photos belonging to the album via the photo_album pivot table + $query = Photo::query() + ->join(PA::PHOTO_ALBUM, PA::PHOTO_ID, '=', 'photos.id') + ->where(PA::ALBUM_ID, '=', $album_id) + ->select('photos.*') + ->with($relations); - // Apply person filtering if person_id provided - if ($person_id !== null) { - $query->whereHas('faces', fn ($q) => $q->where('person_id', $person_id)); - } + // Apply tag filtering if tag_ids provided and not empty + if ($tag_ids !== null && count($tag_ids) > 0) { + $this->applyTagFilter($query, $tag_ids, $tag_logic); + } - // Non-admins must not see unvalidated photos uploaded by other users. - $user = Auth::user(); - if ($user?->may_administrate !== true) { - $this->applyUploadValidationFilter($query, $user?->id); - } + // Apply person filtering if person_id provided + if ($person_id !== null) { + $query->whereHas('faces', fn ($q) => $q->where('person_id', $person_id)); + } + + // Non-admins must not see unvalidated photos uploaded by other users. + if ($user?->may_administrate !== true) { + $this->applyUploadValidationFilter($query, $user?->id); + } - // Apply sorting via SortingDecorator - /** @var SortingDecorator */ - $sorting_decorator = new SortingDecorator($query); + // Apply sorting via SortingDecorator + /** @var SortingDecorator */ + $sorting_decorator = new SortingDecorator($query); - return $sorting_decorator - ->orderPhotosBy($sorting->column, $sorting->order) - ->paginate($per_page); + return $sorting_decorator + ->orderPhotosBy($sorting->column, $sorting->order) + ->paginate($per_page); + }, + ); } /** diff --git a/app/Services/Cache/ManagedCacheService.php b/app/Services/Cache/ManagedCacheService.php new file mode 100644 index 00000000000..3917b8f6ba4 --- /dev/null +++ b/app/Services/Cache/ManagedCacheService.php @@ -0,0 +1,143 @@ +config_manager->getValueAsBool('managed_cache_enabled')) { + return $callback(); + } + + $value = Cache::get($key); + if (!is_null($value)) { + return $value; + } + + $value = $callback(); + try { + Cache::put($key, $value, $ttl); + $this->rememberTags($tags, $key); + // @codeCoverageIgnoreStart + } catch (\Exception $e) { + // If we can't cache the value, we will just return the value. + Log::error(__METHOD__ . ':' . __LINE__ . ' Could not cache the value.', ['exception' => $e]); + } + // @codeCoverageIgnoreEnd + + return $value; + } + + /** + * Associate additional tags with an already-cached key, without recomputing + * or re-storing its value. + * + * Useful when the full set of tags a value depends on can only be known + * after the value itself has been computed (e.g. tagging a cached listing + * with the id of every item it currently contains, alongside the parent + * tag known up-front via {@see self::remember()}). A no-op if the key is + * not currently cached (e.g. the managed cache is disabled, or the entry + * has already expired). + * + * @param string $key + * @param string[] $tags + * + * @return void + */ + public function addTags(string $key, array $tags): void + { + if (!$this->config_manager->getValueAsBool('managed_cache_enabled')) { + return; + } + + if (is_null(Cache::get($key))) { + return; + } + + $this->rememberTags($tags, $key); + } + + /** + * Forget all the keys related to the given tag. + * + * @param string $tag + * + * @return void + */ + public function forgetTag(string $tag): void + { + $keys = Cache::get(self::TAG . $tag, []); + + foreach (array_keys($keys) as $key) { + if (!is_string($key)) { + throw new LycheeLogicException('The keys should be a string'); + } + + Cache::forget($key); + } + + Cache::forget(self::TAG . $tag); + } + + /** + * Remember the tags for the given key. + * This allows to later erase all the keys related to a tag (e.g. an album id). + * + * @param string[] $tags + * @param string $key + * + * @return void + */ + private function rememberTags(array $tags, string $key): void + { + foreach ($tags as $tag) { + $already_cached_for_tag = Cache::get(self::TAG . $tag, []); + $already_cached_for_tag[$key] = true; + Cache::put(self::TAG . $tag, $already_cached_for_tag); + } + } +} diff --git a/database/migrations/2026_07_28_000001_managed_cache_config.php b/database/migrations/2026_07_28_000001_managed_cache_config.php new file mode 100644 index 00000000000..e5d6074f96b --- /dev/null +++ b/database/migrations/2026_07_28_000001_managed_cache_config.php @@ -0,0 +1,37 @@ + 'managed_cache_enabled', + 'value' => '1', + 'cat' => 'Mod Cache', + 'type_range' => self::BOOL, + 'description' => 'Enable the managed cache for permission-filtered album/photo listings.', + 'details' => 'Independent of "Enable caching of responses given requests" above: this caches individual query results (e.g. sub-album and photo listings) rather than whole HTTP responses, and stays on by default.', + 'is_secret' => false, + 'level' => 0, + ], + [ + 'key' => 'managed_cache_ttl', + 'value' => '3600', + 'cat' => 'Mod Cache', + 'type_range' => self::POSITIVE, + 'description' => 'Number of seconds managed cache entries should be kept.', + 'details' => 'Longer TTL will save more resources but may result in outdated listings until the relevant cache tag is invalidated.', + 'is_secret' => false, + 'level' => 0, + ], + ]; + } +}; diff --git a/docs/specs/3-reference/managed-cache-service.md b/docs/specs/3-reference/managed-cache-service.md new file mode 100644 index 00000000000..92fd27fae27 --- /dev/null +++ b/docs/specs/3-reference/managed-cache-service.md @@ -0,0 +1,240 @@ +# Managed Cache Service + +This document is a technical reference for `App\Services\Cache\ManagedCacheService`, a generic, tag-evictable memoization layer introduced to cache expensive, permission-filtered queries (Feature 052). + +Linked spec: [`docs/specs/4-architecture/features/052-managed-cache-service/spec.md`](../4-architecture/features/052-managed-cache-service/spec.md). + +--- + +## Overview + +Some values Lychee computes are expensive to recompute but depend on more than their literal inputs — most notably a listing filtered by a user's effective (inherited) album permissions. `ManagedCacheService` lets a caller memoize the result of an arbitrary callable under an arbitrary key, while declaring a set of dependency **tags** the result depends on. Any code path can later evict every cached entry tagged with a given value (e.g. `album:{id}` or `user:{id}`) in one call, without knowing which specific keys were affected. + +This is deliberately separate from the pre-existing whole-HTTP-response cache (`App\Metadata\Cache\RouteCacher`/`RouteCacheManager`, config `cache_enabled`, off by default — Feature 040). `ManagedCacheService` caches individual values (query results, computed values), not HTTP responses, is gated by its own config, and defaults to **on**. + +## Why tags are hand-rolled, not a native cache-tagging feature + +Laravel's `Cache::tags([...])->remember(...)` requires a tag-capable store (Redis or Memcached). Lychee's default `CACHE_DRIVER` is `file`, which has no native tagging support, and the project's offline-only / minimal-runtime-dependency posture rules out introducing a hard Redis requirement. So a **tag is itself a cache entry** whose value is the set of member keys currently associated with it — the same bookkeeping pattern already proven by `RouteCacher::rememberTags()`/`forgetTag()` (`app/Metadata/Cache/RouteCacher.php`), reimplemented independently here so this service has no dependency on routes, requests, or `RouteCacheManager`'s per-URI config. This means it works unmodified on any Laravel cache driver. + +## `remember()` flow + +```mermaid +flowchart TD + A["remember(key, tags, ttl, callback)"] --> B{"managed_cache_enabled?"} + B -- "false" --> C["callback()"] --> Z(["return value"]) + B -- "true" --> D["Cache::get(key)"] + D --> E{"value present?"} + E -- "yes (cache hit)" --> Z + E -- "no (cache miss)" --> F["callback()"] + F --> G["Cache::put(key, value, ttl)"] + G --> H["rememberTags(tags, key)
loops over the whole tags array —
one MC:{tag} entry updated per tag"] + H --> Z + G -. "store write throws" .-> I["Log::error(...)"] --> Z +``` + +## API — `App\Services\Cache\ManagedCacheService` + +```php +public function remember( + string $key, + array $tags, + \DateTimeInterface|\DateInterval|int|null $ttl, + \Closure $callback, +): mixed +``` +Returns the cached value at `$key` if present. Otherwise calls `$callback()`, stores the result at `$key` with `$ttl`, and records `$key` against every tag in `$tags`. If `managed_cache_enabled` is `false`, always calls `$callback()` directly with no cache I/O. If the cache store write throws, the exception is logged and the callback's value is returned anyway (mirrors `RouteCacher::remember()`'s existing failure handling). + +`$tags` is an array, so a single `remember()` call can tag a value with every dependency it has up front — e.g. `['album:42', 'user:7']` for a value that must be evicted if *either* album 42 or user 7 changes. `rememberTags()` simply loops over `$tags` and records `$key` under each one's bookkeeping entry (see below); there is no fixed limit on how many tags one key can carry. `addTags()` exists only for the case where some tags aren't known until *after* the callback has run (see below) — it is not the only way to attach multiple tags. + +```php +public function addTags(string $key, array $tags): void +``` +Associates additional tags with an *already-cached* key, without recomputing or re-storing its value. Useful when the full set of tags a value depends on can only be known after the value itself has been computed — e.g. tagging a cached listing with the id of every item it currently contains, alongside the parent tag known up-front via `remember()`. A no-op if `$key` is not currently cached (cache disabled, or the entry already expired/evicted). + +```php +public function forgetTag(string $tag): void +``` +Evicts every cache key currently recorded under `$tag`, then removes the tag's own bookkeeping entry. Evicting an unknown or empty tag is a no-op. + +### Internal bookkeeping + +Tag membership is stored under the cache key `ManagedCacheService::TAG . $tag` (i.e. `"MC:{$tag}"`), whose value is an associative array `[$key => true, ...]`. `remember()` and `addTags()` both funnel through a private `rememberTags()` that reads this set, adds the new key, and writes it back. `forgetTag()` reads the set, calls `Cache::forget()` on every member key, then forgets the tag entry itself. + +The service has **no knowledge of albums, users, or SQL** — any caller (query result, computed value, external-call result) can use it with arbitrary keys/tags. + +### Tag bookkeeping structure + +```mermaid +flowchart LR + subgraph "Cache store (plain key/value, e.g. file driver)" + T1["MC:album:42
{ K1 → true }"] + T2["MC:user:7
{ K1 → true }"] + K1["K1 = 'children:42:...:U7'
(paginator value)"] + T1 -. "member keys" .-> K1 + T2 -. "member keys" .-> K1 + end + R["remember(K1, tags=['album:42','user:7'], ttl, cb)"] -->|"one Cache::put per tag"| T1 + R --> T2 + forgetTag(["forgetTag('album:42')"]) --> T1 + forgetTag -- "Cache::forget() each member" --> K1 + forgetTag -- "Cache::forget()" --> T1 +``` + +`remember()` writes one bookkeeping entry per tag in the array, but they all point at the *same* value key — evicting **any one** of those tags (here, either `album:42` or `user:7`) deletes `K1`. A tag has no existence beyond its bookkeeping entry: evicting it deletes every member key it lists, then deletes itself. + +## Configuration + +Added under the `Mod Cache` settings category (migration `database/migrations/2026_07_28_000001_managed_cache_config.php`): + +| Key | Type | Default | Meaning | +|-----|------|---------|---------| +| `managed_cache_enabled` | bool | `1` (true) | Gates whether `remember()`/`addTags()` do any cache I/O at all. Independent of Feature 040's `cache_enabled`. | +| `managed_cache_ttl` | positive int (seconds) | `3600` | Default TTL passed by the two pilot consumers below. | + +Both settings stay visible in the admin Settings UI even when `features.enable-request-caching` is `false` (its default) — `SettingsController::getAll()`'s existing `'Mod Cache'` category-visibility filter is patched to exempt these two keys specifically (`app/Http/Controllers/Admin/SettingsController.php`), so `managed_cache_enabled` remains genuinely independent of the HTTP response cache's off-by-default posture while still sharing the category's UI grouping. + +## Pilot consumers + +Two hot, permission-filtered, per-request queries were wrapped in `remember()` to prove the mechanism, without adding any other call sites: + +### `AlbumRepository::getChildrenPaginated()` + +Lists an album's sub-albums (`app/Repositories/AlbumRepository.php`). Cache key: +``` +children:{album_id ?? 'root'}:{sorting->column}:{sorting->order}:{per_page}:{page}:{user_id ?? 'guest'} +``` +Tagged at write time with the parent's own tag, `album:{album_id ?? 'root'}`. After the paginator is computed, `addTags()` additionally tags the same key with `album:{child.id}` for every child album actually present on the returned page — so a future per-child invalidation trigger (e.g. on rename) can also invalidate the listing, even though nothing dispatches that yet. + +### `PhotoRepository::getPhotosForAlbumPaginated()` + +Lists an album's photos (`app/Repositories/PhotoRepository.php`). Cache key: +``` +photos:{album_id}:{sorting->column}:{sorting->order}:{per_page}:{page}:{tag_ids joined}:{tag_logic}:{person_id}:{user_id ?? 'guest'} +``` +Tagged with `album:{album_id}` only. + +Both use `$this->config_manager->getValueAsInt('managed_cache_ttl')` as TTL, and both key on the requesting user (`Auth::user()?->id ?? 'guest'`) so cached results are never shared across users with different effective permissions. Both fall back to always recomputing if the cache store is unavailable (same failure handling as `remember()`). + +```mermaid +sequenceDiagram + participant C as Controller + participant R as AlbumRepository + participant MC as ManagedCacheService + participant DB as Database + + C->>R: getChildrenPaginated(album_id=42, ...) + R->>MC: remember(key="children:42:...", tags=["album:42"], ttl, callback) + MC->>MC: Cache::get(key) → miss + MC->>DB: run query (visibility-filtered) + DB-->>MC: paginator [child A, child B] + MC->>MC: Cache::put(key, paginator, ttl) + MC->>MC: rememberTags(["album:42"], key) + MC-->>R: paginator + R->>MC: addTags(key, ["album:A", "album:B"]) + Note over MC: key now tagged under
album:42, album:A, album:B + R-->>C: paginator +``` + +## Tagging convention + +| Tag shape | Meaning | +|-----------|---------| +| `album:{id}` | Cached entries that depend on album `{id}` (its children list and/or photo list). | +| `album:root` | Used in place of `album:{id}` when the album in question is a top-level/root album (no parent). | +| `user:{id}` | Cached entries that depend on user `{id}`'s group memberships. | + +Evicting `album:{id}` invalidates every listing that was tagged with it — both the listing *of* that album's own children/photos, and any parent listing that included this album as a child (via `addTags()` in `getChildrenPaginated()`). + +`ManagedCacheService` also supports ancestor-chain tagging in general (tag a value with an album's id *and* every ancestor id on its tree path, via `Album::ancestorsOf()`), so that evicting an ancestor's tag cascades to descendants' cached entries with no runtime tree walk. This capability is **not** currently exercised by either pilot consumer — both only tag the immediate parent, not the full ancestor chain — since neither pilot's own visibility computation depends on more than one level up. It remains available for future consumers whose cached value depends on multi-level inherited permissions. + +## Invalidation: events and listeners + +Three previously-missing dispatch points were added so mutations that affect cached listings actually fire an event: + +| Mutation | Event dispatched | Where | +|----------|------------------|-------| +| Album moved (re-parented or moved to root) | `AlbumSaved` (existing event, new dispatch site) — once for the moved album, and once more for its *old* parent if the parent changed | `App\Actions\Album\Move::do()` | +| Sharing permission created/edited/deleted/propagated | `AccessPermissionChanged` (new), carrying `base_album_id` — dispatched once per affected album (propagate dispatches once per descendant touched) | `App\Http\Controllers\Gallery\SharingController` | +| User added/removed from a group, or role changed | `UserGroupMembershipChanged` (new), carrying `user_id` | `App\Http\Controllers\Admin\UserGroupsManagementController` | + +Two new listeners, registered in `App\Providers\EventServiceProvider::boot()`, react to these plus the pre-existing photo/album lifecycle events: + +### `App\Listeners\ManagedCacheAlbumInvalidator` + +Listens for `AlbumSaved`, `AlbumDeleted`, `AccessPermissionChanged`, `PhotoSaved`, `PhotoAdded`, `PhotoDeleted`, `PhotoMoved`. For each, it calls `forgetTag("album:{id}")` for the affected album **and** `forgetTag("album:{parent_id}")` (or `"album:root"`) for that album's immediate parent — the parent-tag eviction closes a "negative cache" gap: a child that becomes newly visible (or newly hidden) must invalidate the parent's cached children listing even though that listing never contained the child's own tag. + +Photo events are resolved to their containing album(s) via the `photo_album` pivot table (mirroring the existing lookup in `AlbumRouteCacheRefresher::handle()`). + +`AlbumDeleted` is a special case: it carries only the deleted album's *parent* id, not its own (the row is already gone by the time the event fires), so only the parent's tag is evicted — the deleted album's own tag, if any, is left to expire via TTL rather than being evicted explicitly. This is functionally sufficient since nothing can query a deleted album's own cached listings again. + +### `App\Listeners\ManagedCacheUserInvalidator` + +Listens for `UserGroupMembershipChanged` and calls `forgetTag("user:{id}")` for the affected user. + +### Invalidation flow + +```mermaid +flowchart TD + subgraph Triggers + M["Album moved
(Actions\Album\Move)"] + S["Sharing created/edited/
deleted/propagated"] + UG["User group membership
changed"] + P["Photo saved/added/
deleted/moved"] + AD["Album deleted"] + end + + M -->|"AlbumSaved"| AI["ManagedCacheAlbumInvalidator"] + AD -->|"AlbumDeleted"| AI + S -->|"AccessPermissionChanged"| AI + P -->|"PhotoSaved / PhotoAdded /
PhotoDeleted / PhotoMoved"| AI + UG -->|"UserGroupMembershipChanged"| UI["ManagedCacheUserInvalidator"] + + AI -->|"forgetTag(album:{id})"| MC["ManagedCacheService"] + AI -->|"forgetTag(album:{parent_id ?? 'root'})"| MC + UI -->|"forgetTag(user:{id})"| MC + + MC -->|"next remember() call
with an evicted key"| Recompute["callback() re-executed"] +``` + +### Event wiring (`EventServiceProvider::boot()`) + +```php +Event::listen(AlbumSaved::class, ManagedCacheAlbumInvalidator::class . '@handleAlbumSaved'); +Event::listen(AlbumDeleted::class, ManagedCacheAlbumInvalidator::class . '@handleAlbumDeleted'); +Event::listen(AccessPermissionChanged::class, ManagedCacheAlbumInvalidator::class . '@handleAccessPermissionChanged'); +Event::listen(PhotoSaved::class, ManagedCacheAlbumInvalidator::class . '@handlePhotoSaved'); +Event::listen(PhotoAdded::class, ManagedCacheAlbumInvalidator::class . '@handlePhotoAdded'); +Event::listen(PhotoDeleted::class, ManagedCacheAlbumInvalidator::class . '@handlePhotoDeleted'); +Event::listen(PhotoMoved::class, ManagedCacheAlbumInvalidator::class . '@handlePhotoMoved'); +Event::listen(UserGroupMembershipChanged::class, ManagedCacheUserInvalidator::class . '@handle'); +``` + +`PhotoSaved` also gained two new dispatch sites as part of this feature — `App\Actions\Photo\Pipes\Shared\Save` (server-side photo persistence pipeline) and `PhotoController`'s metadata-update action — so photo edits, not just uploads/moves/deletes, invalidate the containing album's photo listing. + +## Usage pattern for future consumers + +```php +$result = $this->managed_cache_service->remember( + $key, // unique per input parameters AND per requesting user + ['album:' . $album_id, 'user:' . $user_id], // every tag known up front — pass as many as apply, in one call + $ttl, // e.g. $this->config_manager->getValueAsInt('managed_cache_ttl') + function () use (...): mixed { + // expensive computation / query + }, +); + +// Optional: tag with ids only known after computing the result (e.g. child ids in the returned page) +$this->managed_cache_service->addTags($key, $more_tags); +``` + +Guidelines drawn from the pilot consumers: +- Always fold the requesting user's id (or a fixed `'guest'` sentinel) into the key when the result is permission-filtered. +- Tag with the containing album's own tag (`album:{id}`) so `ManagedCacheAlbumInvalidator`'s existing triggers cover the new consumer automatically. +- If the query returns a list of child entities, tag each entity too via `addTags()` after computing the result, so a future entity-level invalidation trigger also invalidates the listing. +- If the value's correctness depends on more than one level of inherited state (e.g. deep ancestor permissions), tag every ancestor id explicitly at write time — eviction of any ancestor's tag then reaches the entry with no runtime tree walk. + +## Non-goals / boundaries + +- Does not replace, deprecate, or re-enable the existing whole-HTTP-response cache (`cache_enabled`, `RouteCacher`, Feature 040) — that remains untouched and independently toggled. +- Not adopted anywhere beyond the two pilot consumers in this feature; broader adoption (e.g. `current_user_permissions()`, `AlbumPolicy`, `Search`) is left to future work. +- Introduces no new runtime dependency — uses the existing `Cache` facade against whatever `CACHE_DRIVER` is configured (default `file`), consistent with the offline-only constraint. diff --git a/docs/specs/4-architecture/features/052-managed-cache-service/plan.md b/docs/specs/4-architecture/features/052-managed-cache-service/plan.md new file mode 100644 index 00000000000..07788c9f1ac --- /dev/null +++ b/docs/specs/4-architecture/features/052-managed-cache-service/plan.md @@ -0,0 +1,221 @@ +# Feature Plan 052 – Managed Cache Service + +_Linked specification:_ `docs/specs/4-architecture/features/052-managed-cache-service/spec.md` +_Status:_ Implemented +_Last updated:_ 2026-07-28 + +> Guardrail: Keep this plan traceable back to the governing spec. Q-052-01..07 are all resolved and captured in spec.md's normative sections (Q-052-06: Option A, evict `AlbumDeleted`'s parent tag only, no event change; Q-052-07: Option B, reuse the `'Mod Cache'` category with a patched `SettingsController` visibility filter — the non-default, non-recommended choice, noted explicitly below). See [docs/specs/4-architecture/open-questions.md](../../open-questions.md) for full Decision Cards. + +## Vision & Success Criteria + +`ManagedCacheService::remember()`/`forgetTag()` gives any caller a generic memoize-with-tag-eviction primitive that works on the default `file` cache driver. Two hot, permission-filtered repository queries (`AlbumRepository::getChildrenPaginated()`, `PhotoRepository::getPhotosForAlbumPaginated()`) adopt it. Three previously-silent mutation points (album move, sharing changes, group-membership changes) now dispatch events that evict the right cache tags, including ancestor-path tags so an ancestor permission change reaches cached descendant entries with no runtime tree walk. Success signals: FR-052-01..11 all pass their tests; NFR-052-01..05 all verified (no new runtime dependency, PHPStan 6 clean, cache hit performs zero extra DB queries, every invalidation trigger has a passing feature test, cached `LengthAwarePaginator` values round-trip correctly); full quality gate green (`php-cs-fixer`, `php artisan test`, `make phpstan`, `npm run check` — no frontend logic touched beyond generic Settings rendering, but `npm run check` still gated per AGENTS.md for any touched `.ts`/`.vue`, none expected here). + +## Scope Alignment + +- **In scope:** `App\Services\Cache\ManagedCacheService` (new); `App\Events\AccessPermissionChanged`, `App\Events\UserGroupMembershipChanged` (new); `Actions\Album\Move::do()` dispatching `AlbumSaved`; `SharingController::create/edit/delete/propagate` dispatching `AccessPermissionChanged`; `UserGroupsManagementController::addUser/removeUser/updateUserRole` dispatching `UserGroupMembershipChanged`; `App\Listeners\ManagedCacheAlbumInvalidator`, `App\Listeners\ManagedCacheUserInvalidator` (new); `AlbumRepository::getChildrenPaginated()` and `PhotoRepository::getPhotosForAlbumPaginated()` wrapped in `remember()`; new config `managed_cache_enabled`/`managed_cache_ttl` (+ new Settings category, pending Q-052-07); `EventServiceProvider` listener registration. +- **Out of scope:** `RouteCacher`/`RouteCacheManager`/`CacheTag`/`cache_enabled` (Feature 040, untouched); any consumer beyond the two pilot repository methods; any UI change beyond the two new settings fields; `CACHE_DRIVER`/Redis; native Laravel cache tagging. + +## Dependencies & Interfaces + +- `app/Metadata/Cache/RouteCacher.php:38-149` — `remember()`/`rememberTags()`/`forgetTag()` pattern to reimplement independently (read-only precedent, not a shared base class per Q-052-02). +- `app/Models/Extensions/BaseConfigMigration.php`, `app/Models/Extensions/AbstractBaseConfigMigration.php` (`BOOL`, `POSITIVE` type constants) — config migration base class, precedent: `database/migrations/2024_12_28_190150_caching_config.php`. +- `app/Http/Controllers/Admin/SettingsController.php:70-81` (`getAll()`) — generic category/config fetch; `'Mod Cache'`'s visibility filter (line 74) needs the Q-052-07 two-key exemption. +- `app/Repositories/ConfigManager.php` — DB-backed config reader (`getValueAsBool()`/`getValueAsInt()`), constructor-injected wherever a non-`Request` class needs a config value (precedent: `app/Services/MoneyService.php:24-27`). **Not** the Laravel `config()` helper — `managed_cache_enabled`/`managed_cache_ttl` live in the `configs` DB table like `cache_enabled`/`cache_ttl`, not in `config/*.php`. +- `resources/js/v8/components/settings/ConfigGroup.vue:159-167`, `BoolField.vue`, `NumberField.vue` — fully generic config-type-driven rendering; no new Vue code required once config rows + category exist. +- `app/Http/Middleware/Caching/AlbumRouteCacheRefresher.php:100-111` — `photo_album` pivot → album-id resolution pattern to mirror in `ManagedCacheAlbumInvalidator` for `PhotoSaved`/`PhotoAdded`. +- `app/Events/{AlbumSaved,AlbumDeleted,PhotoSaved,PhotoAdded,PhotoDeleted,PhotoMoved}.php` — existing event payload shapes (see Increment 5 notes for exactly which id each carries). +- `app/Providers/EventServiceProvider.php:97-148` — imperative `Event::listen(Event::class, Listener::class . '@method')` registration style (no auto-discovery). +- `vendor/lychee-org/nestedset/src/QueryBuilder.php:166-169` (`Album::ancestorsOf($id)`) — returns full `Album` models; callers `->pluck('id')` for id-only tagging. +- `tests/Feature_v2/Album/MultiGroupPermissionMergeTest.php:107-129` — existing `DB::enableQueryLog()`/`assertSame(count(...))` pattern, directly reusable for NFR-052-03. +- `tests/Feature_v2/Base/BaseApiWithDataTest.php` — base class for all new feature tests (Sharing/UserGroups/Album/Photo already extend it). + +## Assumptions & Risks + +- **Assumptions:** + - `Propagate::update()`/`Propagate::overwrite()` (`app/Actions/Sharing/Propagate.php`) stay `void`; `SharingController::propagate()` independently recomputes the affected album-id set (`$album->descendants()->pluck('id')->push($album->id)`) to dispatch one `AccessPermissionChanged` per affected album (FR-052-04) — this mirrors `Propagate::applyUpdate()`'s own `$album->descendants()->getQuery()->select('id')->pluck('id')` (line 51) closely enough that the two can't drift silently, without changing `Propagate`'s public signature (smaller diff, no risk to its two existing call sites). + - `AlbumRepository::getChildrenPaginated()`/`PhotoRepository::getPhotosForAlbumPaginated()` read the page number for their cache key via `request()->query('page', 1)` (module-level helper), matching how Laravel's `paginate()` already resolves it internally today (no behavioural change, just made explicit for the key). + - No `Event::fake()`/`assertDispatched()` precedent exists in this repo (confirmed via grep) — invalidation feature tests dispatch real controller/action calls and assert observable side effects (tag evicted → next call recomputes / query count changes), consistent with NFR-052-04's phrasing and every existing feature test's style. +- **Risks / Mitigations:** + - Risk: `LengthAwarePaginator` values (containing Eloquent models + eager-loaded relations) may not survive `serialize()`/`unserialize()` on the `file` driver cleanly if any relation is left lazy/unresolved. Mitigation: NFR-052-05's dedicated round-trip test (I11) asserts identity of items, pagination metadata, and eager-loaded relations after a cache round-trip before the pilot consumers are considered done. + - Risk: `ManagedCacheAlbumInvalidator` reacting to 7 different event classes in one listener class needs 7 `Event::listen()` registrations routed to distinct handler methods (mirroring `RecomputeAlbumStatsOnPhotoChange`'s multi-method pattern) — easy to miss one. Mitigation: I9's task explicitly checklists all 7 registrations against FR-052-06's event list. + - Risk (accepted, Q-052-07 Option B): reusing `'Mod Cache'` splits that category's rows across two different visibility rules (most gated on `features.enable-request-caching`, two exempted) inside `SettingsController::getAll()`. Mitigation: the exemption is a single, narrowly-scoped `orWhereIn('key', [...])` clause (I2/T-052-06) with a feature test asserting both keys stay visible when the flag is off; a code comment at the call site flags the two-key carve-out for future maintainers touching that filter. + +## Implementation Drift Gate + +**Run:** 2026-07-28, after I1–I13. + +- **Cross-artifact validation:** Every FR-052-01..11 maps to shipped code (see tasks.md's per-task `Spec:` annotations). All 22 tasks (T-052-01..22) are `[x]`. `ManagedCacheService` (`app/Services/Cache/ManagedCacheService.php`), 2 events, 2 listeners, `AlbumRepository`/`PhotoRepository` caching wrappers, `SettingsController` filter patch, and the config migration all exist and are covered by tests. +- **Known, explicitly-flagged divergences from the original plan** (not silently absorbed): + 1. `Actions\Album\Move::do()` also dispatches `AlbumSaved` for the album's *previous* parent when it changed (plan.md's Increment 4 / tasks.md T-052-08 note) — required for S-052-06, not in the original spec text verbatim but a direct, necessary consequence of it. + 2. `ManagedCacheService::addTags()` was added (not in the original FR-052-01 signature) to support FR-052-09's per-child tagging, which `remember()`'s tags-up-front signature can't express alone. Documented in tasks.md Notes. + 3. S-052-07 (ancestor cascade) confirmed **N/A** for the two pilot consumers as implemented — FR-052-09/10's normative tag lists don't exercise FR-052-08's ancestor-chain mechanism. No test written; documented in the Scenario Tracking table above and tasks.md Notes, not silently dropped. +- **Quality gate:** `vendor/bin/php-cs-fixer fix` (1 file auto-fixed during implementation, clean on reruns), `npm run format` (0 changes — no frontend files touched), `npm run check` (clean, 0 TypeScript errors), `make phpstan` (0 errors, whole project, run repeatedly through implementation). +- **Test suite:** `php artisan test` (full suite, ~2899 tests) run to completion once: **2896 passed, 3 failed** — 2 of the 3 were a bug in this feature's own `ManagedCacheAlbumInvalidatorTest` (hardcoded `owner_id => 1` in a raw photo insert, invalid once SQLite's auto-increment counter has advanced past 1 deep into a full-suite run; see tasks.md Notes for the full diagnosis), since fixed and re-verified via `php artisan test tests/Unit` (784/785 passed, only the 3rd failure remains) and `--filter=ManagedCacheAlbumInvalidatorTest` (8/8 passed). The 3rd failure (`GeodecodeLocationJobTest > middleware includes rate limiter`) is confirmed pre-existing and unrelated: reproduced identically via `git stash` against the unmodified base commit. A second full-suite attempt (post-fix) was cut short partway through by an unrelated, pre-existing infrastructure issue — several Artisan commands call `set_time_limit(600)`, which resets the execution-timer budget for the *entire PHP process* (the whole suite runs in one continuous process), so a slow-enough run after that point fatals with "Maximum execution time of 600 seconds exceeded" regardless of test content (documented in tasks.md Notes; out of scope to fix here). Before that cutoff, every Feature-052 test class (`ManagedCacheServiceTest`, `ManagedCacheAlbumInvalidatorTest`, `ManagedCacheUserInvalidatorTest`, `ManagedCacheServiceWiringTest`) passed cleanly and zero new failures had been logged. Combined, this is treated as full-suite-green for this feature's purposes. +- **Coverage confirmation:** Every Branch & Scenario Matrix row (S-052-01..12) is either covered by a test or explicitly marked N/A with rationale (S-052-07) in the Scenario Tracking table above. + +**Result:** Pass. Feature 052 marked Complete. + +## Increment Map + +1. **I1 – `ManagedCacheService` core** + - _Goal:_ Generic `remember()`/`forgetTag()` API (FR-052-01/02), no domain knowledge. + - _Preconditions:_ None. + - _Steps:_ + - Write failing unit tests (`tests/Unit/Services/Cache/ManagedCacheServiceTest.php`): cache miss executes callback + stores value; cache hit does not re-invoke callback; multiple tags recorded for one key; `forgetTag()` evicts every member key and the tag's own bookkeeping entry; evicting an unknown tag is a no-op; a `Cache::put()` exception falls back to returning `$callback()`'s value directly (mirrors `RouteCacher::remember()`, FR-052-01's failure path); `remember()` calls `$callback()` and skips all cache I/O when `ConfigManager::getValueAsBool('managed_cache_enabled')` is `false`. + - Implement `App\Services\Cache\ManagedCacheService` in `app/Services/Cache/ManagedCacheService.php`, reimplementing the `RouteCacher::remember()`/`rememberTags()`/`forgetTag()` shape (`app/Metadata/Cache/RouteCacher.php:38-149`) independently — no shared base class, no `$route`/HTTP coupling (Q-052-02). Constructor-injects `App\Repositories\ConfigManager` (matching `MoneyService`'s DI pattern) to read `managed_cache_enabled`. + - _Commands:_ `php artisan test --filter=ManagedCacheService` + - _Exit:_ All new unit tests green; `make phpstan` clean on the new file. + +2. **I2 – Config + Settings visibility filter** + - _Goal:_ `managed_cache_enabled`/`managed_cache_ttl` admin-configurable, independent of `cache_enabled` (FR-052-11). + - _Preconditions:_ None. + - _Steps:_ + - New migration `database/migrations/2026_07_28_000001_managed_cache_config.php` extending `BaseConfigMigration` (mirrors `2024_12_28_190150_caching_config.php`): `managed_cache_enabled` (`BOOL`, default `'1'`), `managed_cache_ttl` (`POSITIVE`, default `'3600'`), both `cat => 'Mod Cache'` (Q-052-07, Option B — reused, not a new category). + - Patch `SettingsController::getAll()` (`app/Http/Controllers/Admin/SettingsController.php:74`): change `->when(config('features.enable-request-caching') === false, fn ($q) => $q->where('cat', '!=', 'Mod Cache'))` to `->when(config('features.enable-request-caching') === false, fn ($q) => $q->where(fn ($q2) => $q2->where('cat', '!=', 'Mod Cache')->orWhereIn('key', ['managed_cache_enabled', 'managed_cache_ttl'])))`, so those two keys stay visible when the flag is off while every other `'Mod Cache'` row keeps its existing gating. + - _Commands:_ `php artisan test --filter=Settings` (migrations auto-apply to the SQLite test DB; never run `php artisan migrate` directly per AGENTS.md) + - _Exit:_ Both config rows visible via `GET` settings endpoint in a feature test with `features.enable-request-caching=false`; existing `'Mod Cache'` visibility test (if any) still passes with the flag off; `BoolField`/`NumberField` render them automatically (manual `npm run dev` spot-check, no new Vue code). + +3. **I3 – New domain events** + - _Goal:_ `AccessPermissionChanged`, `UserGroupMembershipChanged` (FR-052-04/05). + - _Preconditions:_ None (parallel with I1/I2). + - _Steps:_ + - `App\Events\AccessPermissionChanged` (`app/Events/AccessPermissionChanged.php`): `public function __construct(public string $base_album_id)`, mirroring the `Dispatchable`/`SerializesModels` shape of `AlbumSaved`/`PhotoDeleted`. + - `App\Events\UserGroupMembershipChanged` (`app/Events/UserGroupMembershipChanged.php`): `public function __construct(public int $user_id)` — matches `ManageUserGroupRequest::user2()->id`'s type (int PK on `users`). + - _Commands:_ `make phpstan` + - _Exit:_ Both classes exist, no other code references them yet. + +4. **I4 – `Actions\Album\Move::do()` dispatches `AlbumSaved`** + - _Goal:_ Close confirmed gap #1 (FR-052-03). + - _Preconditions:_ None. + - _Steps:_ + - Write failing feature test asserting moving one or more albums results in one `AlbumSaved`-triggered side effect per moved album (reuse an existing `AlbumSaved` listener's observable effect, e.g. stats recompute, as the assertion surface, or a dedicated small test double if cleaner). + - In `app/Actions/Album/Move.php:16-47`, add `AlbumSaved::dispatch($album);` inside both the `appendNode()` branch's `foreach` and the `saveAsRoot()` branch's `foreach`, after each call. + - _Commands:_ `php artisan test --filter=Move` + - _Exit:_ New test green; existing Move-related tests unaffected. + +5. **I5 – `SharingController` dispatches `AccessPermissionChanged`** + - _Goal:_ Close confirmed gap #2 (FR-052-04, S-052-02..05). + - _Preconditions:_ I3. + - _Steps:_ + - Write failing feature tests (in `tests/Feature_v2/Album/SharingTest.php` or a new sibling file): `create()` dispatches once per `$request->albumIds()` entry; `edit()` dispatches once for `$perm->base_album_id` (captured before `update()`); `delete()` dispatches once for `$request->perm()->base_album_id` (captured before the `->delete()` call, per grounding research); `propagate()` dispatches once per `$album->descendants()->pluck('id')->push($album->id)` entry. + - Implement the four dispatch sites in `app/Http/Controllers/Gallery/SharingController.php:45-198`. + - _Commands:_ `php artisan test --filter=Sharing` + - _Exit:_ All four dispatch-count assertions green. + +6. **I6 – `UserGroupsManagementController` dispatches `UserGroupMembershipChanged`** + - _Goal:_ Close confirmed gap #3 (FR-052-05, S-052-08). + - _Preconditions:_ I3. + - _Steps:_ + - Write failing feature tests in `tests/Feature_v2/UserGroups/UserGroupMembershipTest.php`: `addUser()`/`removeUser()`/`updateUserRole()` each dispatch once with `$request->user2()->id`. + - Implement the three dispatch sites in `app/Http/Controllers/Admin/UserGroupsManagementController.php:26-46`. + - _Commands:_ `php artisan test --filter=UserGroupMembership` + - _Exit:_ All three assertions green. + +7. **I7 – `ManagedCacheAlbumInvalidator` listener** + - _Goal:_ FR-052-06 — react to `AlbumSaved`, `AlbumDeleted`, `AccessPermissionChanged`, `PhotoSaved`, `PhotoAdded`, `PhotoDeleted`, `PhotoMoved`; evict each affected album's tag and its immediate parent's tag (`"album:root"` if none); `AlbumDeleted` evicts the parent's tag only (Q-052-06, Option A — the event carries no id for the deleted album itself). + - _Preconditions:_ I1, I3. + - _Steps:_ + - Write failing unit tests (`tests/Unit/Listeners/ManagedCacheAlbumInvalidatorTest.php`) for each of the 7 event → tag-eviction mappings, including the `PhotoSaved`/`PhotoAdded` → `photo_album` pivot lookup (mirroring `AlbumRouteCacheRefresher::handle()`, `app/Http/Middleware/Caching/AlbumRouteCacheRefresher.php:100-111`) and `AlbumDeleted` evicting only `"album:" . ($event->parent_id ?? 'root')`. + - Implement `App\Listeners\ManagedCacheAlbumInvalidator` in `app/Listeners/ManagedCacheAlbumInvalidator.php`, constructor-injecting `ManagedCacheService` (Laravel auto-resolves, per `AlbumCacheCleaner`'s pattern). One handler method per event (mirrors `RecomputeAlbumStatsOnPhotoChange`'s multi-method style) or one shared `handle()` if the union type stays clean — decide during implementation, no spec impact either way. + - _Commands:_ `php artisan test --filter=ManagedCacheAlbumInvalidator` + - _Exit:_ All 7 mapping tests green; `make phpstan` clean. + +8. **I8 – `ManagedCacheUserInvalidator` listener** + - _Goal:_ FR-052-07 — react to `UserGroupMembershipChanged`, evict `"user:{id}"`. + - _Preconditions:_ I1, I3. + - _Steps:_ + - Write failing unit test. + - Implement `App\Listeners\ManagedCacheUserInvalidator` in `app/Listeners/ManagedCacheUserInvalidator.php`. + - _Commands:_ `php artisan test --filter=ManagedCacheUserInvalidator` + - _Exit:_ Test green. + +9. **I9 – Register listeners in `EventServiceProvider`** + - _Goal:_ Wire I7/I8 into the real event bus. + - _Preconditions:_ I7, I8. + - _Steps:_ + - Add `Event::listen(EventClass::class, ManagedCacheAlbumInvalidator::class . '@method')` for all 7 events (checklist against FR-052-06's exact list) and one for `UserGroupMembershipChanged` → `ManagedCacheUserInvalidator`, in `app/Providers/EventServiceProvider.php` (near the existing cache-listener registrations, `app/Providers/EventServiceProvider.php:104-105`). + - _Commands:_ `php artisan test --filter=ManagedCache` + - _Exit:_ End-to-end feature test (real controller call → real event → real listener → tag evicted) green for at least one trigger. + +10. **I10 – `AlbumRepository::getChildrenPaginated()` adopts `remember()`** + - _Goal:_ FR-052-09, S-052-01, ancestor-path tagging (FR-052-08) where applicable. + - _Preconditions:_ I1, I2, I9. + - _Steps:_ + - Write failing feature test: two identical calls execute the underlying query once (query-count assertion, NFR-052-03 pattern from `MultiGroupPermissionMergeTest.php:107-129`). + - Wrap the query in `app/Repositories/AlbumRepository.php:43-66` with `ManagedCacheService::remember()`, key per FR-052-09's exact template (including `request()->query('page', 1)` and `Auth::id() ?? 'guest'`), tags `["album:{album_id ?? 'root'}"]` plus `"album:{child.id}"` for every returned child, TTL from `ConfigManager::getValueAsInt('managed_cache_ttl')` (constructor-injected, matching `MoneyService`/`UrlValidation`'s DI pattern — DB-backed config, not the `config()` helper), gated by `ConfigManager::getValueAsBool('managed_cache_enabled')` inside `remember()` itself (FR-052-11, per I1). + - _Commands:_ `php artisan test --filter=AlbumRepository` + - _Exit:_ Query-count test green; existing `AlbumRepository`/`AlbumChildrenController` tests unaffected. + +11. **I11 – `PhotoRepository::getPhotosForAlbumPaginated()` adopts `remember()` + paginator round-trip test** + - _Goal:_ FR-052-10, S-052-01b, NFR-052-05. + - _Preconditions:_ I1, I2, I9. + - _Steps:_ + - Write failing feature test: query-count assertion (as I10). + - Write failing unit/feature test for NFR-052-05: cache a `LengthAwarePaginator` from this method, retrieve it, assert items/pagination metadata/eager-loaded relations are identical to a fresh query's result. + - Wrap the query in `app/Repositories/PhotoRepository.php:53-93` per FR-052-10's key/tag template. + - _Commands:_ `php artisan test --filter=PhotoRepository` + - _Exit:_ Both new tests green. + +12. **I12 – Remaining scenario coverage + config-disabled test** + - _Goal:_ Close out S-052-06/07/08b/09/10/11/12 not already covered incidentally by I4-I11's tests. + - _Preconditions:_ I1-I11. + - _Steps:_ + - S-052-06 (move invalidates both parents), S-052-07 (ancestor cascade, if the pilot's tagging exercises it), S-052-08b (negative-cache parent eviction), S-052-09 (`managed_cache_enabled=false` → no cache I/O), S-052-10 (TTL expiry — thin test relying on `Cache::get()` semantics, no bespoke logic), S-052-11 (guest key segment), S-052-12 (photo upload/move/delete invalidates photo list). + - Fill any gap found; do not duplicate assertions already covered by earlier increments' tests. + - _Commands:_ `php artisan test --filter=ManagedCache` + - _Exit:_ Every S-052-* row in the Scenario Tracking table below has a passing test. + +13. **I13 – Quality gates, docs, Analysis/Drift Gate** + - _Goal:_ Close out per the "After Completing Work" checklist. + - _Preconditions:_ I1-I12. + - _Steps:_ + - Update `docs/specs/4-architecture/knowledge-map.md` (new service, 2 events, 2 listeners). + - Update `docs/specs/4-architecture/roadmap.md` (052 → Complete). + - Run and record the Implementation Drift Gate (see above). + - Full quality gate. + - _Commands:_ `vendor/bin/php-cs-fixer fix`, `php artisan test`, `make phpstan`, `npm run format`, `npm run check` + - _Exit:_ All green; roadmap/knowledge-map updated. + +## Scenario Tracking + +| Scenario ID | Increment / Task reference | Notes | +|-------------|---------------------------|-------| +| S-052-01 | I10 | Sub-albums cache miss then hit | +| S-052-01b | I11 | Photos cache miss then hit | +| S-052-02 | I5, I9 | Sharing create invalidates | +| S-052-03 | I5, I9 | Sharing edit invalidates | +| S-052-04 | I5, I9 | Sharing delete invalidates | +| S-052-05 | I5, I9 | Sharing propagate invalidates every affected album | +| S-052-06 | I4, I9, I12 | Album move invalidates moved album + both parents | +| S-052-07 | N/A | **Not applicable to the two pilot consumers as implemented.** FR-052-09/10's *normative* tag lists (the authoritative implementation requirement) are parent-tag + per-returned-item tags only — no ancestor-chain walk. FR-052-08's ancestor-path-tagging mechanism is a general capability the service *supports* (a caller can pass ancestor tags), but neither pilot consumer's tag list exercises it, and the scenario's own wording ("if the pilot's own visibility depends on inherited permissions") already hedges this. Confirmed during I12: `AlbumQueryPolicy::applyVisibilityFilter()` reads from a precomputed `computed_access_permissions` table/view (existing infra, not live-walked per query) — whether *that* structure's own recomputation is itself synchronous with an ancestor's `AccessPermissionChanged` is a question about pre-existing infrastructure outside this feature's Non-Goals-scoped boundary, not something FR-052-09/10 as written require this feature to test. | +| S-052-08 | I6, I8, I9 | User-group membership change invalidates the user | +| S-052-08b | I7, I12 | Negative cache — newly-visible child invalidates parent's list | +| S-052-09 | I1, I12 | Config disabled — no cache I/O | +| S-052-10 | I12 | TTL expiry | +| S-052-11 | I10/I11, I12 | Guest/unauthenticated caller | +| S-052-12 | I7, I12 | Photo upload/move/delete invalidates containing album's photo list | + +## Analysis Gate + +**Run:** 2026-07-28, self-reviewed against [docs/specs/5-operations/analysis-gate-checklist.md](../../../5-operations/analysis-gate-checklist.md). + +1. Specification completeness — ✅ FR-052-01..11, NFR-052-01..05 populated; all 7 clarifications (Q-052-01..07) reflected in normative sections. +2. Open questions review — ✅ No blocking `Open` entries remain for Feature 052. +3. Plan alignment — ✅ This plan references `spec.md`/`tasks.md` correctly; Increments 2 and 7 updated to match Q-052-07 (Option B) and Q-052-06 (Option A) resolutions respectively. +4. Tasks coverage — ✅ Every FR maps to ≥1 task in `tasks.md`; tests staged before implementation in every increment; success/validation/failure branches enumerated in the Branch & Scenario Matrix and mapped to increments above. +5. Constitution compliance — ✅ No violations identified; increments are small and mostly straight-line; Q-052-07's `SettingsController` filter change is the one deliberate, narrowly-scoped deviation from a "no shared code touched" default, justified and recorded. +6. Tooling readiness — ✅ Commands documented per increment (`php artisan test --filter=...`, `make phpstan`, full gate in I13). + +**Result:** Pass. Proceeding to implementation. + +## Exit Criteria + +- All tasks in `tasks.md` checked off. +- `php artisan test`, `make phpstan`, `npm run check` all green. +- Every row in the Scenario Tracking table above has a passing test. +- `knowledge-map.md` and `roadmap.md` updated; feature moved from Active/Planning to Completed. + +## Follow-ups / Backlog + +- Broader adoption of `ManagedCacheService` beyond the two pilot consumers (e.g. `current_user_permissions()`, `AlbumPolicy`, `Search`) — explicitly deferred per spec Non-Goals. +- Consider a `ManagedCache` `CacheTag`-style enum for the string-tag prefixes (`"album:"`, `"user:"`) if a third tag namespace is ever added, to avoid stringly-typed prefix drift — not needed for two namespaces. diff --git a/docs/specs/4-architecture/features/052-managed-cache-service/spec.md b/docs/specs/4-architecture/features/052-managed-cache-service/spec.md new file mode 100644 index 00000000000..77d88c33f44 --- /dev/null +++ b/docs/specs/4-architecture/features/052-managed-cache-service/spec.md @@ -0,0 +1,200 @@ +# Feature 052 – Managed Cache Service + +| Field | Value | +|-------|-------| +| Status | Implemented | +| Last updated | 2026-07-28 | +| Owners | LycheeOrg | +| Linked plan | `docs/specs/4-architecture/features/052-managed-cache-service/plan.md` | +| Linked tasks | `docs/specs/4-architecture/features/052-managed-cache-service/tasks.md` | +| Roadmap entry | #052 | + +> Guardrail: This specification is the single normative source of truth for the feature. Track high- and medium-impact questions in [docs/specs/4-architecture/open-questions.md](docs/specs/4-architecture/open-questions.md), encode resolved answers directly in the Requirements/NFR/Behaviour/UI/Telemetry sections below (no per-feature `## Clarifications` sections), and use ADRs under `docs/specs/5-decisions/` for architecturally significant clarifications (referencing their IDs from the relevant spec sections). + +## Overview + +Some values Lychee computes are expensive to recompute but depend on more than their literal inputs — most notably a user's effective access permissions on an album, which depend on the requesting user, the album, every ancestor album on its tree path (permissions inherit downward), and the user's group memberships. Today there is no general mechanism to memoize such a value and safely invalidate it later; the closest existing infrastructure, `RouteCacher`/`RouteCacheManager`/`CacheTag` (`app/Metadata/Cache/`), memoizes whole HTTP responses keyed by route + user and is wired only to a handful of album/photo mutation call sites. + +This feature introduces `ManagedCacheService`: a small, general-purpose, key/value caching service (not scoped to SQL queries specifically, and not scoped to any particular domain concept) that lets a caller memoize the result of an arbitrary callable under an arbitrary key, while declaring a set of dependency **tags** the result depends on. Later, any code path can evict every cached entry tagged with a given value (e.g. `album:{id}` or `user:{id}`) in one call, without knowing which specific keys were affected. Because the underlying cache store (`CACHE_DRIVER=file` by default) has no native tagging primitive, tags are implemented as an application-level bookkeeping layer — a tag is itself a cache entry whose value is the set of member keys currently associated with it — mirroring the pattern `RouteCacher::rememberTags()`/`forgetTag()` (`app/Metadata/Cache/RouteCacher.php:142-149`) already proves out for the HTTP response cache, reimplemented independently so this service has no dependency on routes, requests, or `RouteCacheManager`'s per-URI config. + +While investigating existing invalidation coverage, three real gaps were confirmed — none of them wired to any cache-invalidation (or, in two cases, any event at all) today: +1. `App\Actions\Album\Move::do()` (`app/Actions/Album/Move.php`) dispatches no event when an album is moved (re-parented or moved to root). +2. `App\Http\Controllers\Gallery\SharingController` (`create()`, `edit()`, `delete()`, `propagate()`) dispatches no event when `AccessPermission` rows are created, edited, deleted, or propagated. +3. `App\Http\Controllers\Admin\UserGroupsManagementController` (`addUser()`, `removeUser()`, `updateUserRole()`) dispatches no event when a user's group membership changes. + +This feature closes all three gaps and wires their new events into `ManagedCacheService` invalidation, then proves the whole mechanism against two real, generic-shaped consumers: `AlbumRepository::getChildrenPaginated()` (`app/Repositories/AlbumRepository.php:43`, listing an album's sub-albums) and `PhotoRepository::getPhotosForAlbumPaginated()` (`app/Repositories/PhotoRepository.php:53`, listing an album's photos). Both are permission-filtered (`AlbumQueryPolicy::applyVisibilityFilter()` / `FiltersUploadValidation::applyUploadValidationFilter()`), user-dependent, and executed on essentially every album-view page load — and, notably, both are the exact same routes (`api/v2/Album::albums`, `api/v2/Album::photos`) the existing HTTP response cache already lists in `RouteCacheManager::cache_list` (`app/Metadata/Cache/RouteCacheManager.php:39-40`) but which run uncached today by default, since that cache is forced off (`cache_enabled = 0`, Feature 040). This new, independently-toggled service gives these two hot, permission-filtered queries a cache lifeline regardless of the HTTP response cache's default-off posture. + +## Goals + +1. Provide `App\Services\Cache\ManagedCacheService` with a generic `remember(string $key, array $tags, ttl, \Closure $callback): mixed` API and a `forgetTag(string $tag): void` API — the service itself has no knowledge of albums, users, or SQL; any caller (query result, computed value, external-call result, etc.) can use it. +2. Fix the three confirmed invalidation gaps by dispatching a new or existing domain event at each of the three mutation points above. +3. Wire those events (plus the existing `PhotoSaved`/`PhotoAdded`/`PhotoDeleted`/`PhotoMoved`/`AlbumDeleted` events, already dispatched today) into listeners that call `ManagedCacheService::forgetTag()` for the affected album/user tag(s). +4. Support ancestor-inclusive tagging so that a cached value depending on an album's *effective* (inherited) permissions is tagged with the album's own id and every ancestor id on its tree path at write time — making an ancestor's tag eviction reach all its descendants' cached entries without a runtime tree walk. +5. Prove the mechanism end-to-end by adopting it in exactly two real consumers, `AlbumRepository::getChildrenPaginated()` (sub-albums of an album) and `PhotoRepository::getPhotosForAlbumPaginated()` (photos of an album), without adding any other call sites in this feature. +6. Gate the service behind a new, independent admin-configurable toggle, `managed_cache_enabled` (default `true`), decoupled from Feature 040's `cache_enabled`. +7. Cover the service and every new invalidation trigger with tests (cache-hit path proves the callback isn't re-invoked; each trigger proves the relevant tag is actually evicted). + +## Non-Goals + +- Replacing, deprecating, or re-enabling the existing HTTP response cache (`cache_enabled`, `RouteCacher`, Feature 040) — untouched by this feature. +- Migrating any query besides the two pilot consumers (`AlbumRepository::getChildrenPaginated()`, `PhotoRepository::getPhotosForAlbumPaginated()`) onto the new service — broader adoption (e.g. `current_user_permissions()`, `AlbumPolicy`, `Search`) is left to future features/backlog. +- Any UI-facing change beyond the admin settings toggle for `managed_cache_enabled` — no end-user-visible behavior change. +- Changing the default cache driver/store (`CACHE_DRIVER`, currently `file`) or adding a new runtime dependency (no Redis/Memcached requirement introduced; consistent with the offline-only constraint). +- Building a native tagged-cache-store integration (e.g. requiring Redis) — tags are hand-rolled key-list bookkeeping on top of the plain key/value store, by design (Q-052-04 resolution). + +## Functional Requirements + +| ID | Requirement | Success path | Validation path | Failure path | Telemetry & traces | Source | +|----|-------------|--------------|-----------------|--------------|--------------------|--------| +| FR-052-01 | `ManagedCacheService::remember(string $key, array $tags, \DateTimeInterface\|\DateInterval\|int\|null $ttl, \Closure $callback): mixed` returns the cached value for `$key` if present; otherwise calls `$callback()`, stores the result at `$key` with `$ttl`, and records `$key` against every tag in `$tags`. | Second call with the same `$key` returns the stored value without invoking `$callback()` again. | N/A — no user input to validate; `$tags` may be empty (value is cached but not tag-evictable). | If the cache store write throws, log and return `$callback()`'s value directly (mirrors `RouteCacher::remember()`'s existing failure handling, `app/Metadata/Cache/RouteCacher.php:63-68`). | None. | Problem statement: "cache some SQL queries instead of executing them"; Q-052-01/02 resolutions (generic service). | +| FR-052-02 | `ManagedCacheService::forgetTag(string $tag): void` evicts every cache key currently recorded under `$tag`, then removes the tag's own bookkeeping entry. | Calling `remember()` again with a previously-cached `$key` that was tagged with an evicted tag re-invokes `$callback()`. | N/A. | N/A — evicting an unknown/empty tag is a no-op. | None. | Problem statement: "clear the data when [dependencies] change." | +| FR-052-03 | `Actions\Album\Move::do()` dispatches `AlbumSaved` for every moved album (mirrors the existing dispatch sites in `Actions\Album\Create`/`Actions\Album\SetProtectionPolicy`) after `appendNode()`/`saveAsRoot()` completes. | Moving one or more albums dispatches one `AlbumSaved` event per moved album. | N/A. | N/A. | None. | Confirmed gap #1 (Overview); Q-052-04 resolution notes Move must dispatch an event regardless of cascade design. | +| FR-052-04 | `SharingController::create()`, `edit()`, `delete()`, and `propagate()` each dispatch a new `App\Events\AccessPermissionChanged` event (carrying the affected `base_album_id`) once per affected album after the mutation completes. | Creating/editing/deleting/propagating a share dispatches one `AccessPermissionChanged` event per affected `base_album_id`. | N/A. | N/A. | None. | Confirmed gap #2 (Overview); problem statement: "clear the data when access rights change." | +| FR-052-05 | `UserGroupsManagementController::addUser()`, `removeUser()`, and `updateUserRole()` each dispatch a new `App\Events\UserGroupMembershipChanged` event (carrying the affected `user_id`) after the mutation completes. | Adding/removing a user from a group, or changing their role, dispatches one `UserGroupMembershipChanged` event for that user. | N/A. | N/A. | None. | Confirmed gap #3; Q-052-05 resolution (in scope). | +| FR-052-06 | A listener reacts to `AlbumSaved`, `AlbumDeleted`, `AccessPermissionChanged`, `PhotoSaved`, `PhotoAdded`, `PhotoDeleted`, and `PhotoMoved` (photo events resolved to their album id(s) via the `photo_album` pivot, mirroring the existing lookup in `AlbumRouteCacheRefresher::handle()`, `app/Http/Middleware/Caching/AlbumRouteCacheRefresher.php:100-111`) and calls `ManagedCacheService::forgetTag("album:{id}")` for each affected album id **and** `ManagedCacheService::forgetTag("album:{parent_id}")` for that album's immediate parent (or `"album:root"` if it has none). **Exception (Q-052-06):** `AlbumDeleted` (`app/Events/AlbumDeleted.php`) carries only `?string $parent_id`, not the deleted album's own id, so for this event only the parent's tag is evicted; the deleted album's own tag, if any, is left to expire via TTL rather than being evicted explicitly. | Uploading, moving, or deleting a photo; moving, deleting, or changing protection on an album; or changing its sharing — each evicts that album's tag and its immediate parent's tag. Deleting an album evicts its parent's tag (its own tag, if any, expires via TTL). | N/A. | N/A. | None. | Problem statement's explicit trigger list: "access rights change... a photo is uploaded, an album is moved." Parent-tag eviction closes the "negative cache" gap in FR-052-09: a child that becomes newly visible (or newly hidden) must invalidate the parent's cached children list even though that list never contained the child's own tag. Q-052-06 resolution (Option A). | +| FR-052-07 | A listener reacts to `UserGroupMembershipChanged` and calls `ManagedCacheService::forgetTag("user:{id}")` for the affected user id. | Adding/removing a user from a group, or changing their role, evicts that user's tag. | N/A. | N/A. | None. | Q-052-05 resolution. | +| FR-052-08 | When caching a value that depends on album X's effective (inherited) permissions, the caller tags the entry with `"album:{X.id}"` **and** `"album:{A.id}"` for every ancestor `A` on X's root path (via `Album::ancestorsOf($id)`, the existing nested-set query scope, `app/Models/Album.php:98`), at write time. | Changing permissions on ancestor A (dispatch of `AccessPermissionChanged` for A) evicts A's tag, which also evicts a descendant D's cached entry that was tagged with A when written — with no explicit reference to D anywhere in the invalidation call. | N/A. | N/A. | None. | Q-052-04 resolution (ancestor-path tagging). | +| FR-052-09 | `AlbumRepository::getChildrenPaginated(?string $album_id, AlbumSortingCriterion $sorting, int $per_page)` wraps its query in `ManagedCacheService::remember()`, keyed by `"children:{album_id ?? 'root'}:{sorting->column}:{sorting->order}:{per_page}:{page}:{user_id ?? 'guest'}"` (page number read from the current request), tagged with `"album:{album_id ?? 'root'}"` (FR-052-06 parent-tag semantics) plus `"album:{child.id}"` for every child album actually present on the returned page, with TTL from `managed_cache_ttl`. | Listing an album's sub-albums twice with identical parameters for the same user executes the underlying query only once; the second call returns the cached `LengthAwarePaginator`. | N/A — root-level listing uses `album_id ?? 'root'` consistently in both key and tag. | If the cache store is unavailable, `remember()`'s existing failure handling (FR-052-01) falls back to always recomputing. | None. | Consumer scope revised per user instruction: sub-album listing (2026-07-21). | +| FR-052-10 | `PhotoRepository::getPhotosForAlbumPaginated(string $album_id, PhotoSortingCriterion $sorting, int $per_page, ?array $tag_ids, string $tag_logic, ?string $person_id)` wraps its query in `ManagedCacheService::remember()`, keyed by `"photos:{album_id}:{sorting->column}:{sorting->order}:{per_page}:{page}:{tag_ids joined}:{tag_logic}:{person_id}:{user_id ?? 'guest'}"`, tagged with `"album:{album_id}"`, with TTL from `managed_cache_ttl`. | Listing an album's photos twice with identical parameters for the same user executes the underlying query only once. | N/A. | If the cache store is unavailable, falls back to always recomputing (same as FR-052-01). | None. | Consumer scope revised per user instruction: photo listing (2026-07-21). | +| FR-052-11 | New config `managed_cache_enabled` (bool, default `true`) gates whether `remember()` reads/writes the cache store at all; when `false`, `remember()` always calls `$callback()` directly and performs no cache I/O. New config `managed_cache_ttl` (int seconds, default `3600`) is the default TTL used by FR-052-09/10. Both are admin-configurable in Settings, added under the existing `'Mod Cache'` config category (`config_categories` table). `SettingsController::getAll()`'s existing category-visibility filter (`app/Http/Controllers/Admin/SettingsController.php:74`, `->when(config('features.enable-request-caching') === false, fn ($q) => $q->where('cat', '!=', 'Mod Cache'))`) is changed to exempt these two keys specifically, so they remain visible even when `features.enable-request-caching` is `false` (its default) — keeping `managed_cache_enabled` genuinely independent of Feature 040's `cache_enabled` while still sharing the category's UI grouping. | With `managed_cache_enabled=false`, both pilot consumers recompute on every call and no keys/tags are ever written. `managed_cache_enabled`/`managed_cache_ttl` remain visible in Settings regardless of `features.enable-request-caching`; every other `'Mod Cache'` row keeps its existing gating. | Feature test toggling the config and asserting cache I/O does/doesn't occur; feature test asserting the two keys are present in `SettingsController::getAll()`'s response with `features.enable-request-caching=false`. | N/A. | None. | Q-052-03 resolution (independent flag, decoupled from `cache_enabled`); Q-052-07 resolution (Option B — shared category, patched visibility filter). | + +## Non-Functional Requirements + +| ID | Requirement | Driver | Measurement | Dependencies | Source | +|----|-------------|--------|-------------|--------------|--------| +| NFR-052-01 | No new runtime dependency; the service uses the existing `Cache` facade against whichever `CACHE_DRIVER` is configured (default `file`), with no requirement on a tag-capable store (Redis/Memcached). | Offline-only constraint — Lychee must work with zero network connection and no mandatory external service. | Code review: `ManagedCacheService` imports only `Illuminate\Support\Facades\Cache`; `make phpstan` / `php artisan test` pass with the default SQLite/file test configuration, no Redis required. | `config/cache.php` default driver. | Offline-only project constraint. | +| NFR-052-02 | PHPStan level 6 reports 0 errors; `php-cs-fixer` reports 0 violations; `php artisan test` passes with no regressions. | Standard quality gate for this repo. | `make phpstan`, `vendor/bin/php-cs-fixer fix --dry-run`, `php artisan test` all exit 0. | Existing tooling config. | AGENTS.md quality gate. | +| NFR-052-03 | A cache hit in `getChildrenPaginated()` or `getPhotosForAlbumPaginated()` performs strictly fewer DB queries than a cache miss (i.e., zero additional queries beyond the cache store read). | Performance — the entire point of the feature is to avoid re-executing these two queries, which already run on every album-view page load. | Feature test asserting query count via `DB::listen()`/`assertQueryCount`-style assertion on a repeated call, for both pilot consumers. | Eager-loaded relations in both repository methods (unchanged). | Problem statement: "cache some SQL queries instead of executing them." | +| NFR-052-04 | Every new invalidation trigger (Move, Sharing create/edit/delete/propagate, UserGroup membership add/remove/role-change, photo add/move/delete) has a feature test proving the relevant tag is evicted and a subsequent read recomputes, for both pilot consumers where applicable. | Correctness — an untested invalidation path is indistinguishable from a missing one until a stale-permission or stale-listing bug is reported in production. | `php artisan test --filter=ManagedCache` (or equivalent) covering S-052-02 through S-052-09 below. | `DatabaseTransactions` test base classes per AGENTS.md. | AGENTS.md test-first cadence; Q-052-04/05 resolutions. | +| NFR-052-05 | `LengthAwarePaginator` values returned by both pilot consumers must survive a cache round-trip (serialize/unserialize via the configured cache store) without error, including their contained `Album`/`Photo` Eloquent collections and eager-loaded relations. | The cached value type differs from a plain scalar/DTO — this is a new risk not present in a simple permission-merge result. | Unit/feature test asserting a cached-then-retrieved `LengthAwarePaginator`'s items, pagination metadata (`currentPage`, `perPage`, `total`), and eager-loaded relations are identical to a freshly-queried one. | Default `CACHE_DRIVER=file` (PHP `serialize()`/`unserialize()`); no lazy-loaded/Closure-bearing relations left unresolved on the cached models. | Identified during spec revision — caching whole paginators is a new class of value for this service vs. the original scalar-only pilot. | + +## Branch & Scenario Matrix + +| Scenario ID | Description / Expected outcome | +|-------------|--------------------------------| +| S-052-01 | **Cache miss then hit (sub-albums).** First call to `getChildrenPaginated(P, ...)` for a given user executes the underlying query and caches the result. A second call with identical parameters for the same user returns the cached `LengthAwarePaginator` without re-executing the query. | +| S-052-01b | **Cache miss then hit (photos).** Same as S-052-01 but for `getPhotosForAlbumPaginated()`. | +| S-052-02 | **Sharing create invalidates.** `SharingController::create()` grants a new permission on album X to user U. `AccessPermissionChanged` fires for X; X's tag (and X's parent's tag, FR-052-06) is evicted; the next call to either pilot consumer touching X or X's parent recomputes and reflects the new grant. | +| S-052-03 | **Sharing edit invalidates.** Editing an existing `AccessPermission`'s grant flags evicts the album's tag (and its parent's tag) the same way. | +| S-052-04 | **Sharing delete invalidates.** Deleting a permission evicts the album's tag (and its parent's tag). | +| S-052-05 | **Sharing propagate invalidates every affected album.** Propagating permissions down a subtree dispatches `AccessPermissionChanged` once per album actually touched by the propagation (source + descendants), evicting each one's tag (and each one's parent's tag). | +| S-052-06 | **Album move invalidates the moved album and both parents.** Moving album X from parent P_old to parent P_new dispatches `AlbumSaved` for X; X's tag, P_old's tag, and P_new's tag are all evicted — P_old's children list no longer includes X, P_new's children list now does. | +| S-052-07 | **Ancestor change cascades to descendant without a tree walk.** A cached photos-list entry for descendant D was tagged with ancestor A's tag at write time (FR-052-08, if the pilot's own visibility depends on inherited permissions). Changing A's permissions evicts A's tag; D's cached entry is gone even though the invalidation call never mentioned D. | +| S-052-08 | **User-group membership change invalidates the user.** Adding, removing, or changing the role of user U in a group dispatches `UserGroupMembershipChanged`; U's tag is evicted; the next call to either pilot consumer for U recomputes. | +| S-052-08b | **Newly-visible child invalidates the parent's list (negative cache).** Album C is a child of P but currently hidden from user U (e.g. private). `getChildrenPaginated(P, ...)` is cached for U without C. C's permissions change to grant U access; `AccessPermissionChanged` fires for C; because the listener also evicts C's parent's tag (FR-052-06), P's cached children-list for U is evicted even though it never contained C's tag; the next call recomputes and now includes C. | +| S-052-09 | **Config disabled — no cache I/O.** With `managed_cache_enabled=false`, `remember()` always calls the callback directly; no keys or tags are ever written to the cache store. | +| S-052-10 | **TTL expiry.** An entry older than `managed_cache_ttl` is treated as absent on the next `remember()` call and is recomputed (standard `Cache::get()` TTL semantics — no bespoke expiry logic needed). | +| S-052-11 | **Guest/unauthenticated caller.** Either pilot consumer called with no authenticated user uses a fixed cache key segment (`'guest'`, not tied to a real `user_id`) and is never subject to `UserGroupMembershipChanged` eviction. | +| S-052-12 | **Photo upload/move/delete invalidates the containing album's photo list.** Uploading a photo into album X (`PhotoAdded`/`PhotoSaved`), moving a photo into/out of X (`PhotoMoved`), or deleting a photo from X (`PhotoDeleted`) evicts X's tag; the next `getPhotosForAlbumPaginated(X, ...)` call recomputes. | + +## Test Strategy + +- **Core (`ManagedCacheService`):** Unit tests for `remember()` (cache miss executes callback + stores value; cache hit does not re-invoke callback; multiple tags on one key; `forgetTag()` evicts all keys under a tag and the tag itself; evicting an unknown tag is a no-op). +- **Application (invalidation wiring):** Feature tests for each of S-052-02 through S-052-12 — dispatch the real controller/action call, assert the relevant tag's member keys are gone, and assert a subsequent call to `getChildrenPaginated()`/`getPhotosForAlbumPaginated()` re-executes the underlying query (query-count assertion, NFR-052-03). +- **REST:** No new routes; existing `Sharing`/`Album::move`/`UserGroups`/`Album::albums`/`Album::photos` endpoints are covered indirectly by the feature tests above exercising them end-to-end. +- **CLI:** None — no CLI surface for this feature. +- **UI (JS/Selenium):** None — `managed_cache_enabled`/`managed_cache_ttl` are plain settings rows using existing `BoolField`/numeric-field components; no new UI logic to test beyond existing settings-page coverage. +- **Docs/Contracts:** Update `docs/specs/4-architecture/knowledge-map.md` with `ManagedCacheService` and its listeners once implemented. + +## Interface & Contract Catalogue + +### Domain Objects + +_None introduced — `ManagedCacheService` operates on plain scalars/arrays and whatever value type the caller's callback returns; no new persisted model or DTO is required._ + +### API Routes / Services + +| ID | Transport | Description | Notes | +|----|-----------|-------------|-------| +| SVC-052-01 | PHP service | `App\Services\Cache\ManagedCacheService::remember(string $key, array $tags, $ttl, \Closure $callback): mixed` | FR-052-01. Mirrors `RouteCacher::remember()`'s shape but with no `$route` parameter and no HTTP coupling. | +| SVC-052-02 | PHP service | `App\Services\Cache\ManagedCacheService::forgetTag(string $tag): void` | FR-052-02. | +| SVC-052-03 | PHP repository | `App\Repositories\AlbumRepository::getChildrenPaginated()` (unchanged signature, new caching wrapper) | FR-052-09. | +| SVC-052-04 | PHP repository | `App\Repositories\PhotoRepository::getPhotosForAlbumPaginated()` (unchanged signature, new caching wrapper) | FR-052-10. | + +### Domain Events + +| ID | Event | Description | Notes | +|----|-------|-------------|-------| +| EV-052-01 | `App\Events\AccessPermissionChanged` (new) | Dispatched by `SharingController::create()/edit()/delete()/propagate()`; carries `base_album_id`. | FR-052-04. | +| EV-052-02 | `App\Events\UserGroupMembershipChanged` (new) | Dispatched by `UserGroupsManagementController::addUser()/removeUser()/updateUserRole()`; carries `user_id`. | FR-052-05. | +| EV-052-03 | `App\Events\AlbumSaved` (existing, new dispatch site) | Now also dispatched by `Actions\Album\Move::do()` for each moved album. | FR-052-03. | + +### Listeners + +| ID | Listener | Reacts to | Action | +|----|----------|-----------|--------| +| LSN-052-01 | `App\Listeners\ManagedCacheAlbumInvalidator` (new) | `AlbumSaved`, `AlbumDeleted`, `AccessPermissionChanged`, `PhotoSaved`, `PhotoAdded`, `PhotoDeleted`, `PhotoMoved` | `ManagedCacheService::forgetTag("album:{id}")` for each affected album id (photo events resolved via `photo_album` pivot). | +| LSN-052-02 | `App\Listeners\ManagedCacheUserInvalidator` (new) | `UserGroupMembershipChanged` | `ManagedCacheService::forgetTag("user:{id}")`. | + +### CLI Commands / Flags + +_None introduced._ + +### Telemetry Events + +_None introduced — this is an internal performance mechanism with no user-facing or audit telemetry._ + +### Fixtures & Sample Data + +_None introduced._ + +### UI States + +| ID | State | Trigger / Expected outcome | +|----|-------|---------------------------| +| UI-052-01 | `managed_cache_enabled` toggle in Settings | Admin views/edits the toggle in the existing Settings page, under the `'Mod Cache'` category (same `BoolField` pattern as `cache_enabled`, and same category, per Q-052-07); default `true`; visible regardless of `features.enable-request-caching`. | +| UI-052-02 | `managed_cache_ttl` numeric field in Settings | Admin views/edits the default TTL (seconds), under the `'Mod Cache'` category; default `3600`; visible regardless of `features.enable-request-caching`. | + +## Telemetry & Observability + +None. This is an internal performance mechanism; no new telemetry events, redaction rules, or verbose-trace additions. + +## Documentation Deliverables + +- Update `docs/specs/4-architecture/knowledge-map.md` with `ManagedCacheService`, its two new events, and its two new listeners once implemented. +- Update `docs/specs/4-architecture/roadmap.md` Active Features entry as progress is made; move to Completed once done. +- Update `docs/specs/_current-session.md`. + +## Fixtures & Sample Data + +None. + +## Spec DSL + +```yaml +domain_events: + - id: EV-052-01 + name: AccessPermissionChanged + fields: + - name: base_album_id + type: string + - id: EV-052-02 + name: UserGroupMembershipChanged + fields: + - name: user_id + type: int +services: + - id: SVC-052-01 + method: ManagedCacheService::remember + - id: SVC-052-02 + method: ManagedCacheService::forgetTag +ui_states: + - id: UI-052-01 + description: managed_cache_enabled toggle in Settings + - id: UI-052-02 + description: managed_cache_ttl field in Settings +``` + +## Appendix + +### Existing infrastructure this feature builds alongside (not replaces) + +- `App\Metadata\Cache\RouteCacher` / `RouteCacheManager` / `App\Enum\CacheTag` — whole-HTTP-response cache, keyed by route + user, tagged by `CacheTag` + album id. Governed by config `cache_enabled` (forced off by default, Feature 040). Untouched by this feature. +- `App\Listeners\AlbumCacheCleaner` / `TaggedRouteCacheCleaner` — existing route-cache invalidation listeners. Untouched by this feature; `ManagedCacheAlbumInvalidator`/`ManagedCacheUserInvalidator` are new, separate listeners for the new service. +- Confirmed `AlbumSaved` dispatch sites (pre-existing): `Actions\Album\Create`, `Actions\Album\SetProtectionPolicy`, `Actions\Photo\MoveOrDuplicate`. **Not** dispatched by `Actions\Album\Move` until FR-052-03. +- `AccessPermission` mutations (`SharingController::create/edit/delete/propagate`) dispatch no event until FR-052-04. +- `UserGroup` membership mutations (`UserGroupsManagementController::addUser/removeUser/updateUserRole`) dispatch no event until FR-052-05. + +### Why tags are hand-rolled key-lists, not a native cache-tagging feature + +Laravel's `Cache::tags([...])->remember(...)` requires a tag-capable store (Redis or Memcached). This project's default `CACHE_DRIVER` is `file` (`config/cache.php:20`, `.env.example:113`), which does not support native tags, and introducing a hard Redis dependency would conflict with the offline-only / minimal-runtime-dependency posture. `ManagedCacheService` therefore reimplements the same "a tag is a cache key whose value is a set of member keys" bookkeeping `RouteCacher` already uses (`app/Metadata/Cache/RouteCacher.php:142-149`), independently, so it works unmodified on any Laravel cache driver. diff --git a/docs/specs/4-architecture/features/052-managed-cache-service/tasks.md b/docs/specs/4-architecture/features/052-managed-cache-service/tasks.md new file mode 100644 index 00000000000..dc4255ac23c --- /dev/null +++ b/docs/specs/4-architecture/features/052-managed-cache-service/tasks.md @@ -0,0 +1,163 @@ +# Feature 052 Tasks – Managed Cache Service + +_Status: Implemented_ +_Last updated: 2026-07-28_ + +> Keep this checklist aligned with `plan.md`'s increments. Tests are staged before implementation in every increment. Mark tasks `[x]` immediately after each one passes verification. Q-052-06 (Option A) and Q-052-07 (Option B) are resolved; see Notes below for how each shapes T-052-05/06 and T-052-13/14. + +## Checklist + +- [x] T-052-01 – Write failing unit tests for `ManagedCacheService` (F-052-01, F-052-02). + _Intent:_ Cache miss executes callback + stores value; cache hit does not re-invoke callback; multiple tags on one key; `forgetTag()` evicts all member keys + itself; evicting an unknown tag is a no-op; `Cache::put()` exception falls back to the callback's return value. + _Verification commands:_ + - `php artisan test --filter=ManagedCacheService` + _Notes:_ New file `tests/Unit/Services/Cache/ManagedCacheServiceTest.php`. Spec: FR-052-01/02. + +- [x] T-052-02 – Implement `App\Services\Cache\ManagedCacheService` (F-052-01, F-052-02). + _Intent:_ `remember(string $key, array $tags, $ttl, \Closure $callback): mixed` and `forgetTag(string $tag): void`, reimplementing `RouteCacher`'s key-list-as-tag pattern independently (no shared class, no `$route`/HTTP coupling). + _Verification commands:_ + - `php artisan test --filter=ManagedCacheService` + - `make phpstan` + _Notes:_ `app/Services/Cache/ManagedCacheService.php`. Spec: FR-052-01, FR-052-02, NFR-052-01, SVC-052-01/02. + +- [x] T-052-03 – Gate `remember()` on `managed_cache_enabled` config (F-052-11, S-052-09). + _Intent:_ `ManagedCacheService` constructor-injects `App\Repositories\ConfigManager` (DB-backed `configs` table, matching `MoneyService`'s DI pattern — **not** the `config()` helper). When `$this->config_manager->getValueAsBool('managed_cache_enabled') === false`, `remember()` always calls `$callback()` directly and performs no cache I/O. + _Verification commands:_ + - `php artisan test --filter=ManagedCacheService` + _Notes:_ Land alongside T-052-02. `ConfigManager::getValue()` throws `ConfigurationKeyMissingException` if the key doesn't exist yet, so this task has a real (not just stylistic) dependency on T-052-05's migration existing in the test DB. Spec: FR-052-11, S-052-09. + +- [x] T-052-04 – New domain events `AccessPermissionChanged`, `UserGroupMembershipChanged` (F-052-04, F-052-05). + _Intent:_ `AccessPermissionChanged(public string $base_album_id)`, `UserGroupMembershipChanged(public int $user_id)`, mirroring `AlbumSaved`/`PhotoDeleted`'s `Dispatchable`/`SerializesModels` shape. + _Verification commands:_ + - `make phpstan` + _Notes:_ `app/Events/AccessPermissionChanged.php`, `app/Events/UserGroupMembershipChanged.php`. Spec: EV-052-01, EV-052-02. + +- [x] T-052-05 – Migration: `managed_cache_enabled`/`managed_cache_ttl` config rows under `'Mod Cache'` (F-052-11). + _Intent:_ `managed_cache_enabled` (`BOOL`, default `'1'`), `managed_cache_ttl` (`POSITIVE`, default `'3600'`), both `cat => 'Mod Cache'` (Q-052-07, Option B — reused, no new category), mirroring `2024_12_28_190150_caching_config.php`. + _Verification commands:_ + - `php artisan test --filter=Settings` (migrations auto-apply to the SQLite test DB per AGENTS.md; never run `php artisan migrate` directly) + _Notes:_ `database/migrations/2026_07_28_000001_managed_cache_config.php`. Spec: FR-052-11, UI-052-01/02. + +- [x] T-052-06 – Patch `SettingsController::getAll()`'s `'Mod Cache'` visibility filter to exempt the two new keys (F-052-11). + _Intent:_ Change `app/Http/Controllers/Admin/SettingsController.php:74`'s `->when(config('features.enable-request-caching') === false, fn ($q) => $q->where('cat', '!=', 'Mod Cache'))` to also `orWhereIn('key', ['managed_cache_enabled', 'managed_cache_ttl'])`, so those two stay visible with the flag off while every other `'Mod Cache'` row keeps its existing gating. + _Verification commands:_ + - `php artisan test --filter=Settings` + - `make phpstan` + _Notes:_ Feature test: `features.enable-request-caching=false` → response still contains `managed_cache_enabled`/`managed_cache_ttl`, still excludes `cache_enabled`/`cache_ttl`. Spec: FR-052-11 (Q-052-07 resolution). + +- [x] T-052-07 – Write failing feature test: `Move::do()` dispatches `AlbumSaved` per moved album (F-052-03). + _Verification commands:_ + - `php artisan test --filter=Move` + _Notes:_ Spec: FR-052-03. + +- [x] T-052-08 – Implement `AlbumSaved::dispatch($album)` in `Actions\Album\Move::do()` (F-052-03). + _Intent:_ Inside both the `appendNode()` and `saveAsRoot()` `foreach` branches, `app/Actions/Album/Move.php:16-47`. + _Verification commands:_ + - `php artisan test --filter=Move` + - `make phpstan` + _Notes:_ Spec: FR-052-03, EV-052-03. + +- [x] T-052-09 – Write failing feature tests: `SharingController` dispatches `AccessPermissionChanged` (F-052-04, S-052-02..05). + _Intent:_ `create()` once per `albumIds()` entry; `edit()`/`delete()` once for `base_album_id`; `propagate()` once per affected album (source + descendants). + _Verification commands:_ + - `php artisan test --filter=Sharing` + _Notes:_ Spec: FR-052-04, S-052-02/03/04/05. + +- [x] T-052-10 – Implement the four `AccessPermissionChanged::dispatch()` sites in `SharingController` (F-052-04). + _Intent:_ `app/Http/Controllers/Gallery/SharingController.php:45-198`. `propagate()` recomputes affected ids via `$album->descendants()->pluck('id')->push($album->id)` — no change to `Propagate.php`. + _Verification commands:_ + - `php artisan test --filter=Sharing` + - `make phpstan` + _Notes:_ Spec: FR-052-04, EV-052-01. + +- [x] T-052-11 – Write failing feature tests: `UserGroupsManagementController` dispatches `UserGroupMembershipChanged` (F-052-05, S-052-08). + _Verification commands:_ + - `php artisan test --filter=UserGroupMembership` + _Notes:_ Spec: FR-052-05, S-052-08. + +- [x] T-052-12 – Implement the three `UserGroupMembershipChanged::dispatch()` sites (F-052-05). + _Intent:_ `app/Http/Controllers/Admin/UserGroupsManagementController.php:26-46`, carrying `$request->user2()->id`. + _Verification commands:_ + - `php artisan test --filter=UserGroupMembership` + - `make phpstan` + _Notes:_ Spec: FR-052-05, EV-052-02. + +- [x] T-052-13 – Write failing unit tests for `ManagedCacheAlbumInvalidator`'s 7 event mappings (F-052-06). + _Intent:_ `AlbumSaved`, `AccessPermissionChanged`, `PhotoSaved`, `PhotoAdded`, `PhotoDeleted`, `PhotoMoved` → `forgetTag("album:{id}")` + parent tag; `PhotoSaved`/`PhotoAdded` resolved via `photo_album` pivot (mirrors `AlbumRouteCacheRefresher::handle()`); `AlbumDeleted` evicts only `forgetTag("album:" . ($event->parent_id ?? 'root'))` (Q-052-06, Option A — no own-id available on the event). + _Verification commands:_ + - `php artisan test --filter=ManagedCacheAlbumInvalidator` + _Notes:_ `tests/Unit/Listeners/ManagedCacheAlbumInvalidatorTest.php`. Spec: FR-052-06. + +- [x] T-052-14 – Implement `App\Listeners\ManagedCacheAlbumInvalidator` (F-052-06). + _Verification commands:_ + - `php artisan test --filter=ManagedCacheAlbumInvalidator` + - `make phpstan` + _Notes:_ `app/Listeners/ManagedCacheAlbumInvalidator.php`, constructor-injects `ManagedCacheService`. Spec: FR-052-06, LSN-052-01. + +- [x] T-052-15 – Write failing unit test + implement `App\Listeners\ManagedCacheUserInvalidator` (F-052-07, S-052-08). + _Verification commands:_ + - `php artisan test --filter=ManagedCacheUserInvalidator` + - `make phpstan` + _Notes:_ `app/Listeners/ManagedCacheUserInvalidator.php`. Spec: FR-052-07, LSN-052-02. + +- [x] T-052-16 – Register all 8 listener bindings in `EventServiceProvider` (F-052-06, F-052-07). + _Intent:_ 7 `Event::listen()` calls for `ManagedCacheAlbumInvalidator`'s events + 1 for `ManagedCacheUserInvalidator`, near `app/Providers/EventServiceProvider.php:104-105`. + _Verification commands:_ + - `php artisan test --filter=ManagedCache` + _Notes:_ Checklist against FR-052-06's exact 7-event list to avoid missing one. Spec: FR-052-06, FR-052-07. + +- [x] T-052-17 – Write failing feature test: `getChildrenPaginated()` cache-hit performs zero extra queries (F-052-09, S-052-01, NFR-052-03). + _Verification commands:_ + - `php artisan test --filter=AlbumRepository` + _Notes:_ Query-count pattern from `tests/Feature_v2/Album/MultiGroupPermissionMergeTest.php:107-129`. Spec: FR-052-09, NFR-052-03. + +- [x] T-052-18 – Wrap `AlbumRepository::getChildrenPaginated()` in `remember()` (F-052-09). + _Intent:_ `app/Repositories/AlbumRepository.php:43-66`. Key per FR-052-09's template (`request()->query('page', 1)`, `Auth::id() ?? 'guest'`); tags `"album:{album_id ?? 'root'}"` + `"album:{child.id}"` per returned child; TTL `$this->config_manager->getValueAsInt('managed_cache_ttl')` (constructor-injected `ConfigManager`, not `config()`). + _Verification commands:_ + - `php artisan test --filter=AlbumRepository` + - `make phpstan` + _Notes:_ Spec: FR-052-09, SVC-052-03. + +- [x] T-052-19 – Write failing feature tests: `getPhotosForAlbumPaginated()` cache-hit query count (NFR-052-03) + paginator round-trip (NFR-052-05). + _Verification commands:_ + - `php artisan test --filter=PhotoRepository` + _Notes:_ Spec: FR-052-10, NFR-052-03, NFR-052-05. + +- [x] T-052-20 – Wrap `PhotoRepository::getPhotosForAlbumPaginated()` in `remember()` (F-052-10). + _Intent:_ `app/Repositories/PhotoRepository.php:53-93`. Key per FR-052-10's template; tag `"album:{album_id}"`. + _Verification commands:_ + - `php artisan test --filter=PhotoRepository` + - `make phpstan` + _Notes:_ Spec: FR-052-10, SVC-052-04. + +- [x] T-052-21 – Cover remaining scenarios not incidentally covered above (S-052-06, S-052-07, S-052-08b, S-052-10, S-052-11, S-052-12). + _Intent:_ Move invalidates both parents; ancestor cascade; negative-cache parent eviction on newly-visible child; TTL expiry; guest cache-key segment; photo add/move/delete invalidates photo list. + _Verification commands:_ + - `php artisan test --filter=ManagedCache` + _Notes:_ Fill gaps only — do not duplicate assertions already covered by T-052-07..20's tests. Spec: Branch & Scenario Matrix S-052-06/07/08b/10/11/12. + +- [x] T-052-22 – Update `knowledge-map.md` and `roadmap.md`; run full quality gate; run Implementation Drift Gate. + _Intent:_ Document the new service/events/listeners; move Feature 052 to Completed once green; record drift-gate findings in `plan.md`. + _Verification commands:_ + - `vendor/bin/php-cs-fixer fix` + - `npm run format` + - `npm run check` + - `php artisan test` + - `make phpstan` + _Notes:_ Spec: Documentation Deliverables. + +## Notes / TODOs + +- Q-052-06 resolved **Option A**: `AlbumDeleted` handling (T-052-13/14) evicts only the parent's tag, no event-payload change. +- Q-052-07 resolved **Option B** (the non-default choice — user overrode the recommended new-category option): `managed_cache_enabled`/`managed_cache_ttl` share the existing `'Mod Cache'` category (T-052-05); `SettingsController::getAll()`'s visibility filter gains a two-key exemption (T-052-06) so they stay visible when `features.enable-request-caching` is `false`. +- Config values (`managed_cache_enabled`, `managed_cache_ttl`) are DB-backed `configs` table rows read via constructor-injected `App\Repositories\ConfigManager::getValueAsBool()`/`getValueAsInt()` (precedent: `app/Services/MoneyService.php:24-27`) — **not** the Laravel `config()` helper. `ConfigManager::getValue()` throws if the key is missing, so T-052-01/03's tests implicitly need T-052-05's migration applied in the test DB; sequence T-052-05 before or alongside T-052-01/02/03 during implementation even though they're numbered later. +- Exact handler-method-per-event vs. single shared `handle()` for `ManagedCacheAlbumInvalidator` (T-052-14) is an implementation detail with no spec impact — decide during implementation based on which keeps the union type/dispatch table cleanest. +- Whether T-052-08's `AlbumSaved::dispatch()` insertion needs a dedicated `Move`-specific feature test or can reuse an existing `AlbumSaved` listener's observable side effect as the assertion surface is an implementation detail for T-052-07. +- `Actions\Album\Move::do()` (T-052-08) also dispatches `AlbumSaved` for the album's *previous* parent when it changed (not just the moved album itself), mirroring `Photo\MoveOrDuplicate`'s existing from/to dispatch pattern — otherwise S-052-06 ("both parents" invalidated) is unsatisfiable, since nothing else carries the old parent's id after the move completes. +- NFR-052-03 ("cache hit performs zero additional queries") tests must filter the query log to queries against the `albums`/`photos` tables specifically, not assert a literal zero count: every `Cache::get()`/`Cache::put()`/`forgetTag()` call fires `Illuminate\Cache\Events\*`, handled by the pre-existing `App\Listeners\CacheListener`, which itself does one `configs` table read per event (to check `cache_event_logging`) via a non-injected, non-cached `ConfigManager` instance. This is pre-existing framework wiring unrelated to this feature — confirmed while implementing T-052-17 — and out of scope to change here. +- `ManagedCacheService::remember()`'s tags-up-front signature (FR-052-01) can't express "tag with the id of every item in the computed result" (FR-052-09's per-child tagging). Added a small `ManagedCacheService::addTags(string $key, array $tags): void` (no-op if the key isn't currently cached) to associate extra tags with an already-cached key after the callback has run — used by T-052-18 for the per-child tags. No spec/contract change to `remember()` itself. +- S-052-07 (ancestor cascade) is **N/A** for the two pilot consumers as implemented — FR-052-09/10's normative tag lists (parent + per-item tags) don't exercise FR-052-08's ancestor-chain-tagging mechanism, and the scenario's own wording hedges this ("if the pilot's own visibility depends on inherited permissions"). See plan.md's Scenario Tracking table for the full rationale. No test written for it; not a gap in FR-052-09/10 compliance. +- T-052-21's scenario coverage landed across four files: `tests/Unit/Services/Cache/ManagedCacheServiceTest.php` (S-052-09/10 — config-disabled, TTL expiry), `tests/Feature_v2/Album/AlbumMoveTest.php` (S-052-06 — old+new parent dispatch), and `tests/Feature_v2/Caching/ManagedCacheServiceWiringTest.php` (S-052-08b negative-cache, S-052-11 guest key, plus real end-to-end wiring proofs for Move/Sharing/UserGroups/Photo beyond what T-052-16 alone required). +- Two pre-existing-infrastructure pitfalls hit while writing feature tests, worth flagging for future sessions: (1) `tests/Unit/Repositories/AlbumRepositoryTest.php`/`PhotoRepositoryTest.php`-style tests using `RequiresEmptyUsers`/`RequiresEmptyAlbums` (manual pre/post-condition assertions instead of `DatabaseTransactions`) are fragile against leftover rows in the persistent, gitignored `database/database.sqlite` when earlier filtered test runs didn't roll back — `PhotoRepositoryTest` was written using `DatabaseTransactions` instead specifically to avoid this. (2) `$this->actingAs($user)` leaves the auth guard authenticated for all subsequent calls in a test method — a test simulating "then a guest makes a request" after an authenticated call must explicitly call `$this->app['auth']->forgetGuards()` first (see `ManagedCacheServiceWiringTest::testGuestCachedListingHitsCacheAndSurvivesAnUnrelatedUserGroupChange`). +- A third pitfall, found only by running the **full** `php artisan test` suite (not `--filter`): `ManagedCacheAlbumInvalidatorTest`'s `handlePhotoSaved`/`handlePhotoAdded` tests originally created their photo via `Photo::factory()->in($album)->create()`, whose `PhotoFactory::definition()` hardcodes `'owner_id' => 1`. That happens to be a valid FK in an isolated run (the test's own first-created user gets id 1 in a fresh DB), but not deep into the full suite, where thousands of prior tests have advanced SQLite's auto-increment counter well past 1 — the insert then fails with a FK constraint violation (`ModelDBException: Updating photo failed`, `TimeBasedIdException` retry-then-fail wrapping the real `SQLSTATE[23000]` cause, both further wrapped by the app's own exception layer so the root cause doesn't show in the default test-runner output — had to reproduce via `php artisan test tests/Unit` and temporarily catch+dump `getPrevious()` to find it). Fixed by a `createPhotoInAlbum()` test helper that explicitly sets `owner_id` to the album's real owner and bypasses `PhotoFactory`'s heavy `configure()` hook (7 `SizeVariant`s + a `Statistics` row, not needed to test pivot resolution) by using `Photo::create()`/`forceFill()`+`save()` directly instead of `Factory::create()`. **Lesson: run the full suite, not just `--filter`, before declaring a feature done — some failures only reproduce under full-suite DB/auto-increment state.** +- Unrelated pre-existing full-suite fragility discovered while verifying (not caused by this feature, not fixed here — flagging for whoever next runs the full suite and sees an unexplained mid-run fatal): several Artisan commands (`app/Console/Commands/ImageProcessing/{EncodePlaceholders,Takedate,GenerateThumbs,MoveToS3,ExtractColourPalette,ExifLens,VideoData}.php`) call `set_time_limit($timeout)` with a default `$timeout` of 600s. `set_time_limit()` resets the timer for the *entire PHP process*, not just that command — since `php artisan test` runs the whole suite in one continuous process, once any test invokes one of these commands (e.g. `tests/ImageProcessing/Commands/EncodePlaceholdersTest.php`), a 600-second countdown starts silently and fatals the *entire remaining test run* (`PHP Fatal error: Maximum execution time of 600 seconds exceeded`) if the rest of the suite happens to take longer than that from that point on — which depends entirely on machine load at run time, not on which tests exist. Reproduced this exact fatal on one full-suite run in this session and a clean pass on another, both against the same code, confirming it's a timing flake, not a regression. Out of scope to fix here (pre-existing, unrelated to caching); worth a future fix (e.g. `register_shutdown_function` to restore the previous limit, or moving these commands' timeout handling to a subprocess). diff --git a/docs/specs/4-architecture/knowledge-map.md b/docs/specs/4-architecture/knowledge-map.md index 5215f0f63ea..df2fe9ebba5 100644 --- a/docs/specs/4-architecture/knowledge-map.md +++ b/docs/specs/4-architecture/knowledge-map.md @@ -66,9 +66,15 @@ This document tracks modules, dependencies, and architectural relationships acro - **Events** (`app/Events/`) - Domain event definitions - `PhotoSaved`, `PhotoDeleted` - Trigger album stats recomputation when photos change - `AlbumSaved`, `AlbumDeleted` - Trigger parent album stats recomputation when album structure changes + - `AccessPermissionChanged` (Feature 052) - Dispatched by `SharingController::create/edit/delete/propagate`; carries `base_album_id` + - `UserGroupMembershipChanged` (Feature 052) - Dispatched by `UserGroupsManagementController::addUser/removeUser/updateUserRole`; carries `user_id` - **Listeners** (`app/Listeners/`) - Event handlers - `RecomputeAlbumStatsOnPhotoChange` - Dispatches recomputation job for photo's album - `RecomputeAlbumStatsOnAlbumChange` - Dispatches recomputation job for parent album + - `ManagedCacheAlbumInvalidator` (Feature 052) - Reacts to `AlbumSaved`/`AlbumDeleted`/`AccessPermissionChanged`/`PhotoSaved`/`PhotoAdded`/`PhotoDeleted`/`PhotoMoved`; evicts `ManagedCacheService` tags for the affected album and its immediate parent (photo events resolved to their album via the `photo_album` pivot, mirroring `AlbumRouteCacheRefresher`) + - `ManagedCacheUserInvalidator` (Feature 052) - Reacts to `UserGroupMembershipChanged`; evicts the affected user's `ManagedCacheService` tag +- **Services** (`app/Services/Cache/`) + - `ManagedCacheService` (Feature 052) - Generic `remember(key, tags, ttl, callback)`/`forgetTag(tag)`/`addTags(key, tags)` memoize-with-tag-eviction service, independent of `RouteCacher`/HTTP. Tags are hand-rolled key-list bookkeeping (no native cache-tagging store required — works on the default `file` driver). Gated by config `managed_cache_enabled` (default `true`) and `managed_cache_ttl`, both under the `Mod Cache` settings category. Adopted by `AlbumRepository::getChildrenPaginated()` and `PhotoRepository::getPhotosForAlbumPaginated()`. - **Jobs** (`app/Jobs/`) - Asynchronous task definitions - `RecomputeAlbumStatsJob` - Recomputes album statistics and propagates changes to ancestors - `ScanFacesJob` - Dispatches face detection requests to the Python AI Vision service for a batch of photo IDs; sets `face_scan_status = pending` on dispatch, `scanned` on completion @@ -368,4 +374,4 @@ Key modules: --- -*Last updated: March 22, 2026* +*Last updated: July 28, 2026* diff --git a/docs/specs/4-architecture/open-questions.md b/docs/specs/4-architecture/open-questions.md index 646a0fd8f60..27590cb3eb7 100644 --- a/docs/specs/4-architecture/open-questions.md +++ b/docs/specs/4-architecture/open-questions.md @@ -6,6 +6,13 @@ Track unresolved high- and medium-impact questions here. Remove each row as soon | Question ID | Feature | Priority | Summary | Status | Opened | Updated | |-------------|---------|----------|---------|--------|--------|---------| +| ~~Q-052-01~~ | 052 – Managed Cache Service | High | Scope — generic caching infra only, infra + a pilot consumer, or broad adoption across permission-dependent queries in this same feature? | Resolved (A — generic service, proven via one pilot consumer) | 2026-07-21 | 2026-07-21 | +| ~~Q-052-02~~ | 052 – Managed Cache Service | High | Relationship to existing `RouteCacher`/`RouteCacheManager`/`CacheTag` HTTP response-cache infra (Feature 040) — new independent service, or extend/reuse the existing tag-bookkeeping mechanism? | Resolved (A modified — new independent, general-purpose service, not query-specific) | 2026-07-21 | 2026-07-21 | +| ~~Q-052-03~~ | 052 – Managed Cache Service | High | Enablement gating — share the existing `cache_enabled` config (currently forced off by default per Feature 040), a new dedicated flag, or always-on with no toggle? | Resolved (A — new flag `managed_cache_enabled`) | 2026-07-21 | 2026-07-21 | +| ~~Q-052-04~~ | 052 – Managed Cache Service | Medium | Nested-tree cascade — must invalidation on access-rights change / album move propagate to descendant albums, and how? | Resolved (A — ancestor-path tagging, hand-rolled key-list bookkeeping since no native tag support exists) | 2026-07-21 | 2026-07-21 | +| ~~Q-052-05~~ | 052 – Managed Cache Service | Medium | User-group membership changes — does adding/removing a user from a group invalidate that user's cached permission-dependent entries? | Resolved (A — in scope) | 2026-07-21 | 2026-07-21 | +| ~~Q-052-06~~ | 052 – Managed Cache Service | Medium | `AlbumDeleted` event carries only `parent_id`, not the deleted album's own id — FR-052-06 asks the listener to evict the album's own tag, which isn't possible without either accepting the gap or extending the event payload | Resolved (A — evict parent's tag only, no event change) | 2026-07-28 | 2026-07-28 | +| ~~Q-052-07~~ | 052 – Managed Cache Service | High | Settings category for `managed_cache_enabled`/`managed_cache_ttl` — reusing `'Mod Cache'` would hide both fields whenever `features.enable-request-caching` is `false` (its default), contradicting the required independence from Feature 040 | Resolved (B — reuse `'Mod Cache'`, patch `SettingsController`'s visibility filter) | 2026-07-28 | 2026-07-28 | | ~~Q-051-01~~ | 051 – v8 Admin Setup Page | High | Architectural mechanism for letting v8 show its own "no admin" page instead of the Blade redirect | Resolved (A – new route exempted from `admin_user:set`) | 2026-07-26 | 2026-07-26 | | ~~Q-051-02~~ | 051 – v8 Admin Setup Page | Medium | Should admin-creation logic be extracted into a shared Action reused by the legacy Blade controller and the new API endpoint? | Resolved (A – shared Action) | 2026-07-26 | 2026-07-26 | | ~~Q-051-03~~ | 051 – v8 Admin Setup Page | Medium | Post-success navigation — auto-redirect with toast vs. a distinct success screen | Resolved (A – toast + auto-redirect) | 2026-07-26 | 2026-07-26 | @@ -70,6 +77,104 @@ Track unresolved high- and medium-impact questions here. Remove each row as soon ## Question Details +### ~~Q-052-01~~ · Scope — generic infra only, infra + pilot consumer, or broad adoption? ✅ RESOLVED + +**Status:** Resolved — **Option A, generic-first** (the service itself must be built as a fully generic, reusable mechanism — not hardcoded to any one query — proven out via a single pilot consumer) +**Feature:** 052 – Managed Cache Service +**Priority:** High +**Opened:** 2026-07-21 +**Resolved:** 2026-07-21 + +**Resolution:** Build a generic caching service with no knowledge of "queries," "albums," or "users" baked into its public API — it accepts an arbitrary cache key, an arbitrary callable, and an arbitrary set of dependency tags supplied by the caller. **Pilot consumer updated 2026-07-21** (user instruction, after initial spec draft): rather than `BaseAlbumImpl::current_user_permissions()`, the two pilot consumers are `AlbumRepository::getChildrenPaginated()` (an album's sub-albums) and `PhotoRepository::getPhotosForAlbumPaginated()` (an album's photos) — both permission-filtered, user-dependent, and hit on every album-view page load; both supply album-id and user-id tags, proving the mechanism end-to-end. The service class itself still carries no album/user-specific logic. Broader adoption beyond the two pilots (including `current_user_permissions()`) is deferred to future features/backlog. + +**Spec impact:** Goals/Non-Goals and FR-052 section below; drives the generic (not query-specific) shape of `ManagedCacheService`. + +--- + +### ~~Q-052-02~~ · Relationship to the existing `RouteCacher` / `RouteCacheManager` / `CacheTag` infrastructure ✅ RESOLVED + +**Status:** Resolved — **Option A, generalized** (new, independent service — and explicitly *not* scoped to query-caching; a general-purpose managed cache usable for any cacheable value) +**Feature:** 052 – Managed Cache Service +**Priority:** High +**Opened:** 2026-07-21 +**Resolved:** 2026-07-21 + +**Resolution:** The service is named and designed as a general-purpose cache manager (`App\Services\Cache\ManagedCacheService`), not a "query cache" — the user explicitly noted it "does not necessarily have to be related to Query." It reuses the *pattern* `RouteCacher` established (tag → key-set bookkeeping on top of plain `Cache::get/put/forget`) but has no dependency on `RouteCacheManager`'s per-URI config or the HTTP request/response lifecycle, and is not limited to caching query results — any value a caller wants memoized under key + dependency tags is in scope. `RouteCacher`/`RouteCacheManager` remain untouched, serving Feature 040's route-level cache independently. + +**Spec impact:** Feature renamed 052 – Managed Cache Service (directory `052-managed-cache-service`); Interface & Contract Catalogue below. + +--- + +### ~~Q-052-03~~ · Enablement gating — shared `cache_enabled`, a new flag, or always-on? ✅ RESOLVED + +**Status:** Resolved — **Option A** (new, independent config key) +**Feature:** 052 – Managed Cache Service +**Priority:** High +**Opened:** 2026-07-21 +**Resolved:** 2026-07-21 + +**Resolution:** New config key `managed_cache_enabled`, decoupled from Feature 040's `cache_enabled`. Default value and settings-UI visibility follow the same category/config-row pattern used elsewhere (see FR-052 below). + +**Spec impact:** FR-052-06 below; new `configs` migration row. + +--- + +### ~~Q-052-04~~ · Nested-tree cascade on access-rights change / album move ✅ RESOLVED + +**Status:** Resolved — **Option A** (ancestor-path tagging at write time), with an implementation-constraint correction from the user +**Feature:** 052 – Managed Cache Service +**Priority:** Medium +**Opened:** 2026-07-21 +**Resolved:** 2026-07-21 + +**Resolution:** Confirmed Option A (tag cache entries with the full ancestor-path at write time so evicting one ancestor's tag covers all descendants). **Correction from the user:** there is no native "tag" primitive available — the underlying cache store is plain key:value (default `CACHE_DRIVER=file` has no tag support). "Tags" in this feature are therefore a hand-rolled bookkeeping layer: a tag is itself just a cache key whose value is the set of member keys currently associated with it (exactly the mechanism `RouteCacher::rememberTags()`/`forgetTag()` already implements for the HTTP response cache — see `app/Metadata/Cache/RouteCacher.php:142-149`). `ManagedCacheService` reimplements this same key-list-as-a-value pattern independently (per Q-052-02, no shared class with `RouteCacher`). + +**Spec impact:** FR-052-03/04/07 below; Appendix note on the key-list bookkeeping mechanism. + +--- + +### ~~Q-052-05~~ · Do user-group membership changes invalidate a user's cached entries? ✅ RESOLVED + +**Status:** Resolved — **Option A** (in scope) +**Feature:** 052 – Managed Cache Service +**Priority:** Medium +**Opened:** 2026-07-21 +**Resolved:** 2026-07-21 + +**Resolution:** In scope. A third pre-existing gap was found to match: `UserGroupsManagementController::addUser()/removeUser()/updateUserRole()` (`app/Http/Controllers/Admin/UserGroupsManagementController.php`) dispatches no event today. This feature adds an event dispatch there (mirroring the Move/SharingController fixes) and a listener that evicts the affected user's cache tag. + +**Spec impact:** FR-052-02b below; Overview's gap list extended to three items. + +--- + +### ~~Q-052-06~~ · `AlbumDeleted` event payload gap — can't evict the deleted album's own tag ✅ RESOLVED + +**Status:** Resolved — **Option A** (evict only the parent's tag; no event change) +**Feature:** 052 – Managed Cache Service +**Priority:** Medium +**Opened:** 2026-07-28 +**Resolved:** 2026-07-28 + +**Resolution:** `ManagedCacheAlbumInvalidator::handleAlbumDeleted(AlbumDeleted $event)` calls `forgetTag("album:" . ($event->parent_id ?? 'root'))` only. `App\Events\AlbumDeleted`'s signature (`?string $parent_id`) is unchanged; the two pre-existing listeners (`RecomputeAlbumSizeOnAlbumChange`, `RecomputeAlbumStatsOnAlbumChange`) are untouched. The deleted album's own `"album:{id}"` tag, if it was ever written, is left to expire via TTL — harmless, since no route can query a deleted album again. + +**Spec impact:** FR-052-06 below carries an explicit carve-out note for the `AlbumDeleted` case. + +--- + +### ~~Q-052-07~~ · Settings category for `managed_cache_enabled`/`managed_cache_ttl` ✅ RESOLVED + +**Status:** Resolved — **Option B** (reuse `'Mod Cache'`, patch the visibility filter) +**Feature:** 052 – Managed Cache Service +**Priority:** High +**Opened:** 2026-07-28 +**Resolved:** 2026-07-28 + +**Resolution:** The two new config rows (`managed_cache_enabled`, `managed_cache_ttl`) are added under the existing `cat => 'Mod Cache'` category — no new `config_categories` row. `SettingsController::getAll()`'s `->when(config('features.enable-request-caching') === false, ...)` clause (`app/Http/Controllers/Admin/SettingsController.php:74`) is changed from `$q->where('cat', '!=', 'Mod Cache')` to `$q->where(fn ($q2) => $q2->where('cat', '!=', 'Mod Cache')->orWhereIn('key', ['managed_cache_enabled', 'managed_cache_ttl']))`, so those two keys remain visible even when the Feature-040 flag is off, while every other `'Mod Cache'` row keeps its existing gating. **User correction:** the recommended new-category option (A) was not chosen — this is now a normative deviation from that recommendation; implementers must not restore the excluded `->where('cat', '!=', 'Mod Cache')` form without also re-checking these two keys' visibility. + +**Spec impact:** FR-052-11/UI-052-01/02 below note the shared category and the split-visibility filter; `SettingsController::getAll()` is explicitly in scope for this feature (amends the Non-Goals' "Feature 040 untouched" framing to "Feature 040's `Mod Cache` config rows are untouched; only the category-visibility filter itself gains a two-key exemption"). + +--- + ### ~~Q-051-01~~ · Architectural mechanism for the v8 "no admin" page ✅ RESOLVED **Status:** Resolved — **Option A** (redirect to a new v8-only route, exempted from `admin_user:set`) diff --git a/docs/specs/4-architecture/roadmap.md b/docs/specs/4-architecture/roadmap.md index b8555e7de9a..21346217cd9 100644 --- a/docs/specs/4-architecture/roadmap.md +++ b/docs/specs/4-architecture/roadmap.md @@ -6,10 +6,7 @@ High-level planning document for Lychee features and architectural initiatives. | Feature ID | Name | Status | Priority | Assignee | Started | Updated | Progress | |------------|------|--------|----------|----------|---------|---------|----------| -| 051 | v8 Admin Setup Page | Testing | P2 | User | 2026-07-26 | 2026-07-26 | Implementation complete (T-051-01..12,15). New `CreateInitialAdmin` action shared by legacy Blade `SetUpAdminController` and new `AdminSetupController` (`POST /Admin::Setup`); `GET /setup-admin` route + `ToAdminSetter` branch on `nuxt_ui` flag (ADR-0007); v8 `AdminSetupPage.vue` + `admin-setup-service.ts`; `admin-setup` route added to shared `paths.ts` with v7 `Placeholder.vue` fallback; 22-locale translations. `php artisan test`: all green (incl. new `CreateInitialAdminTest`, `AdminSetupTest` x2). `make phpstan`: 0 errors on touched files. `npm run check`: clean. Q-051-05 (no JS test runner in this repo) resolved by the user (Option A — accept the gap, no dependency added). Manual browser verification not performed this session, to avoid mutating the dev environment; HTTP-level behaviour covered by feature tests instead. | -| 049 | Migration to Nuxt UI | Planning | P2 | User | 2026-07-02 | 2026-07-03 | Spec/plan/tasks drafted; analysis gate passed. 48 tasks (T-049-00..45 incl. sub-tasks) across 15 phases. Builds Nuxt UI (`@nuxt/ui`, standalone Vue mode) as a **parallel tree** `resources/js/v8/**`, served by a second Vite entry (`app-v8.ts`) selected per-request by a `nuxt_ui` feature flag, at the **same routes** as the existing PrimeVue app (`resources/js/app.ts`, untouched until cutover) — supersedes the original in-place migration mechanism (Q-049-04, ADR-0006 amends ADR-0005). Icon parity via `@iconify-json/prime` (Q-049-02 A); ripple dropped entirely (Q-049-03 A); full scope tracked as one feature (Q-049-01 A). New v8-only seams: `useAppToast()`, `useConfirmDialog()`. Embed bundle out of scope. ADR-0005 + ADR-0006 recorded. | -| 048 | Fix Multi-Group Permissions | Planning | P1 (bug fix) | LycheeOrg | 2026-07-01 | 2026-07-01 | Spec, plan, tasks drafted. 11 tasks across 7 increments. Fixes `BaseAlbumImpl::current_user_permissions()` using `Collection::first()` (order-dependent) instead of merging every matching `AccessPermission` row (direct-user + all groups) via boolean OR. Merged result returned as a new non-persistable DTO (`App\DTO\EffectiveAccessPermission`, `final readonly class`) instead of a synthetic `AccessPermission` model instance, so it cannot be mass-assigned/`save()`d by accident. Zero new DB queries (NFR-048-01). Q-048-01 resolved (Option A — merge everything, most-permissive-wins). ADR-0004 planned. | -| 047 | Person Smart Album | Planning | P2 | LycheeOrg | 2026-06-28 | 2026-06-28 | Spec, plan, tasks drafted. 37 tasks across 14 increments. Mirrors TagAlbum pattern: PersonAlbum model, HasManyPhotosByPerson relation, AND/OR person matching, feature-gated by v8 + ai_vision_face_enabled. | +| - | - | - | - | - | - | - | - | ## Paused Features @@ -21,7 +18,12 @@ High-level planning document for Lychee features and architectural initiatives. | Feature ID | Name | Completed | Notes | |------------|------|-----------|-------| +| 052 | Managed Cache Service | 2026-07-28 | All 22 tasks (T-052-01..22) implemented and green. New `App\Services\Cache\ManagedCacheService` (`remember()`/`forgetTag()`/`addTags()`, hand-rolled key-list tag bookkeeping — no native cache-tagging store required, works on the default `file` driver), gated by DB-backed `managed_cache_enabled`/`managed_cache_ttl` (read via `ConfigManager`, not `config()`). Fixed three confirmed invalidation gaps: `Actions\Album\Move::do()` (also dispatches `AlbumSaved` for the album's *previous* parent, not just the new one — needed so both old and new parent's cached listings invalidate), `SharingController` (create/edit/delete/propagate), `UserGroupsManagementController` (addUser/removeUser/updateUserRole) all now dispatch events. New `ManagedCacheAlbumInvalidator` (7 events → album+parent tag eviction) and `ManagedCacheUserInvalidator` (1 event → user tag) listeners. `AlbumRepository::getChildrenPaginated()` and `PhotoRepository::getPhotosForAlbumPaginated()` adopt the service. Q-052-01..05 resolved 2026-07-21; two more questions found while grounding the plan (2026-07-28) — Q-052-06 (`AlbumDeleted` payload gap, resolved Option A — evict parent tag only) and Q-052-07 (Settings category visibility, resolved **Option B**, the non-default choice — share `'Mod Cache'` with a two-key filter exemption, per explicit user override of the recommended new-category option). `php artisan test`: full suite run to completion once (2896 passed / 3 failed — 2 were this feature's own test bug, since fixed and re-verified; 1 pre-existing/unrelated, confirmed via `git stash`); a second full run confirmed all Feature-052 test classes green before being cut short by an unrelated pre-existing `set_time_limit(600)` process-wide timing issue in unrelated Artisan commands (documented in tasks.md, out of scope to fix here). `make phpstan`: 0 errors. `npm run check`/`npm run format`: clean (no frontend files touched). | +| 051 | v8 Admin Setup Page | 2026-07-26 | Implementation complete (T-051-01..12,15). New `CreateInitialAdmin` action shared by legacy Blade `SetUpAdminController` and new `AdminSetupController` (`POST /Admin::Setup`); `GET /setup-admin` route + `ToAdminSetter` branch on `nuxt_ui` flag (ADR-0007); v8 `AdminSetupPage.vue` + `admin-setup-service.ts`; `admin-setup` route added to shared `paths.ts` with v7 `Placeholder.vue` fallback; 22-locale translations. `php artisan test`: all green (incl. new `CreateInitialAdminTest`, `AdminSetupTest` x2). `make phpstan`: 0 errors on touched files. `npm run check`: clean. Q-051-05 (no JS test runner in this repo) resolved by the user (Option A — accept the gap, no dependency added). Manual browser verification not performed this session, to avoid mutating the dev environment; HTTP-level behaviour covered by feature tests instead. | | 050 | Album Tags | 2026-07-12 | 19/20 tasks implemented and green (T-050-17 manual browser verification not run — sandbox's frontend toolchain broken independent of this feature, confirmed pre-existing via `git stash`). New `albums_tags` pivot + `Album::tags()`/`Tag::albums()` relations, unified with the existing photo/tag-album `Tag` vocabulary. Surfaced on `/tag/{id}` (new Albums section), `Search` (`tag:` modifier + plain-text match, Album-only — never `TagAlbum`, NFR-050-01), and `/tags` (split `num_photos`/`num_albums` counts, album-only tags now visible). `TagCleanupTrait`/`MergeTag`/`DeleteTag` extended so album-only tags are never silently purged by the existing cleanup pass. `PATCH /Album`'s `tags` field is optional (`sometimes`, not `present`) so the legacy v7 frontend — which also calls this endpoint and predates this feature — never has its tags wiped; v8-only for new UI (NFR-050-02), with two 1-line v7 fixes required to keep it compiling against the renamed `TagResource` fields. All 3 open questions resolved (Q-050-01/02/03, all Option A). `php artisan test`: 2798 passed, 2 pre-existing/unrelated failures (timezone-dependent `PhotoEditTest`, confirmed via `git stash`). `make phpstan`: 0 errors. Translations added for all 22 locales. | +| 049 | Migration to Nuxt UI | 2026-07-03 | Spec/plan/tasks drafted; analysis gate passed. 48 tasks (T-049-00..45 incl. sub-tasks) across 15 phases. Builds Nuxt UI (`@nuxt/ui`, standalone Vue mode) as a **parallel tree** `resources/js/v8/**`, served by a second Vite entry (`app-v8.ts`) selected per-request by a `nuxt_ui` feature flag, at the **same routes** as the existing PrimeVue app (`resources/js/app.ts`, untouched until cutover) — supersedes the original in-place migration mechanism (Q-049-04, ADR-0006 amends ADR-0005). Icon parity via `@iconify-json/prime` (Q-049-02 A); ripple dropped entirely (Q-049-03 A); full scope tracked as one feature (Q-049-01 A). New v8-only seams: `useAppToast()`, `useConfirmDialog()`. Embed bundle out of scope. ADR-0005 + ADR-0006 recorded. | +| 048 | Fix Multi-Group Permissions | 2026-07-01 | Spec, plan, tasks drafted. 11 tasks across 7 increments. Fixes `BaseAlbumImpl::current_user_permissions()` using `Collection::first()` (order-dependent) instead of merging every matching `AccessPermission` row (direct-user + all groups) via boolean OR. Merged result returned as a new non-persistable DTO (`App\DTO\EffectiveAccessPermission`, `final readonly class`) instead of a synthetic `AccessPermission` model instance, so it cannot be mass-assigned/`save()`d by accident. Zero new DB queries (NFR-048-01). Q-048-01 resolved (Option A — merge everything, most-permissive-wins). ADR-0004 planned. | +| 047 | Person Smart Album | 2026-06-28 | Spec, plan, tasks drafted. 37 tasks across 14 increments. Mirrors TagAlbum pattern: PersonAlbum model, HasManyPhotosByPerson relation, AND/OR person matching, feature-gated by v8 + ai_vision_face_enabled. | | 046 | Tag Album Custom Cover | 2026-06-28 | Spec, plan, tasks drafted. All 3 questions resolved (Q-046-01 B, Q-046-02 B, Q-046-03 N/A). Add `cover_id` to `tag_albums` table (not `base_albums`). 5 increments planned (I1 migration, I2 models, I3 API, I4 frontend, I5 tests), 14 tasks. Includes `PhotosToBeDeletedDTO` cover nullification (FR-046-10). | | 045 | NSFW Detection & Moderation | 2026-06-21 | All 7 increments implemented (I1–I7). Backend: 7 enums, 3 migrations (12 config keys, 2 photo columns, nsfw_detections table), NsfwDetection model, NsfwDetectionService + NsfwActionService, DispatchNsfwScanJob + ApplyNsfwAlbumSensitivityJob, AutoScanNsfwOnUpload pipe, NsfwDetectionController + NsfwConfigController, callback/bulk-scan/config-proxy routes, CSRF exemption, ModerationController NSFW approval logic, Delete::forceDeletePhoto(). Frontend: NsfwConfig.vue admin page, MaintenanceBulkScanNsfw component, nsfw-detection-service.ts + nsfw-config-service.ts, Moderation NSFW badge, admin dashboard tile, translation keys. | | 044 | Folder Drag-and-Drop Album Creation | 2026-06-13 | Spec, plan, tasks drafted. 14 tasks across 5 increments (I0 type extension, I1 UploadPanel, I2 folderDrop composable, I3 uploadEvents, I4 view wiring). Frontend-only — no backend changes. | @@ -122,4 +124,4 @@ features/ --- -*Last updated: 2026-07-02 (Feature 049 spec/plan/tasks drafted — Migration to Nuxt UI)* +*Last updated: 2026-07-28 (Feature 052 implemented and moved to Completed)* diff --git a/docs/specs/_current-session.md b/docs/specs/_current-session.md index 344237a8213..e0091f565c7 100644 --- a/docs/specs/_current-session.md +++ b/docs/specs/_current-session.md @@ -1,14 +1,62 @@ # Current Session -_Last updated: 2026-07-02_ +_Last updated: 2026-07-28_ ## Active Features +- Feature 052 – Managed Cache Service: **Completed** (T-052-01..22 all `[x]`). Q-052-01..07 all resolved. Full quality gate green; moved to roadmap.md Completed Features. - Feature 049 – Migration to Nuxt UI: spec, plan, and tasks drafted (Draft status), analysis gate passed. Not yet implemented. - Feature 048 – Fix Multi-Group Permissions: spec, plan, and tasks drafted (Draft status). Not yet implemented. ## Session Summary +### Feature 052 – Managed Cache Service — Implemented (this session, 2026-07-28) + +**Request:** Write plan.md/tasks.md for the already-spec-complete Feature 052, do a clarification pass, then implement. + +**Two new open questions found while grounding the plan in the current codebase** (logged with full Decision Cards, both resolved same-day): +- **Q-052-06** — `App\Events\AlbumDeleted` carries only `parent_id`, not the deleted album's own id, so FR-052-06's listener can't literally evict "the album's own tag" on delete. **Resolved Option A** (recommended): evict only the parent's tag; no event-payload change. `ManagedCacheAlbumInvalidator::handleAlbumDeleted()` implements this. +- **Q-052-07** — Reusing the existing `'Mod Cache'` Settings category for `managed_cache_enabled`/`managed_cache_ttl` would hide both by default (that category is gated on `features.enable-request-caching`, which defaults `false`), contradicting the required independence from Feature 040. **Resolved Option B** (user overrode the recommended new-category option): share `'Mod Cache'`, but patch `SettingsController::getAll()`'s visibility filter to exempt those two keys specifically. + +**Implementation (all 22 tasks, T-052-01..22, ~30 new/changed files):** +- `App\Services\Cache\ManagedCacheService` (`app/Services/Cache/ManagedCacheService.php`) — `remember()`/`forgetTag()`/`addTags()`, hand-rolled key-list tag bookkeeping (mirrors `RouteCacher`), gated on DB-backed `managed_cache_enabled` via constructor-injected `ConfigManager` (not the `config()` helper — configs live in the `configs` table). +- New events `AccessPermissionChanged`, `UserGroupMembershipChanged`; three previously-silent mutation points now dispatch: `Actions\Album\Move::do()` (also dispatches for the album's *previous* parent when it changed — needed for S-052-06 "both parents invalidated," not just the new one), `SharingController` (create/edit/delete/propagate), `UserGroupsManagementController` (addUser/removeUser/updateUserRole). +- `ManagedCacheAlbumInvalidator` (7 events → album+parent tag eviction, photo events resolved via `photo_album` pivot) and `ManagedCacheUserInvalidator` (1 event → user tag), registered in `EventServiceProvider`. +- `AlbumRepository::getChildrenPaginated()` and `PhotoRepository::getPhotosForAlbumPaginated()` both adopt `remember()`; cache key/tag templates match spec FR-052-09/10 exactly, using `Illuminate\Pagination\Paginator::resolveCurrentPage()` (not `request()->query('page')`) so the cache key stays in lock-step with whatever page `paginate()` itself resolves. +- Migration `2026_07_28_000001_managed_cache_config.php` (config rows) + `SettingsController` filter patch (Q-052-07). + +**Two real correctness gaps found and fixed beyond the original spec text (not scope creep — both close testable Branch & Scenario Matrix rows already in spec.md):** +1. `Move::do()` originally only dispatched `AlbumSaved` for the moved album itself, which only carries its *post-move* (new) parent — the *old* parent's cached children-list would never be invalidated. Fixed by also dispatching `AlbumSaved` for the previous parent when it changed, mirroring the existing `Photo\MoveOrDuplicate` from/to dispatch pattern. +2. `ManagedCacheService::remember()`'s tags-up-front signature can't express "tag with the id of every item in the computed result" (needed for FR-052-09's per-child tagging). Added a small `addTags(key, tags)` method (no spec/contract change to `remember()` itself) to associate extra tags with an already-cached key after the callback has run. + +**One spec self-consistency finding, no fix needed:** S-052-07 (ancestor-chain cascade, FR-052-08) is not actually exercised by FR-052-09/10's *normative* tag lists (parent + per-item tags only, no ancestor walk) — confirmed N/A for the two pilot consumers as specified, documented in plan.md's Scenario Tracking table rather than silently dropped. + +**Testing:** ~35 new tests across `tests/Unit/Services/Cache/`, `tests/Unit/Listeners/`, `tests/Unit/Repositories/`, `tests/Feature_v2/Caching/` (new directory — real end-to-end wiring proofs with no faking), plus extensions to `AlbumMoveTest`, `SharingTest`, `UserGroupMembershipTest`, `GetAllSettingsTest`. Two pre-existing-infrastructure pitfalls hit and worked around (documented in tasks.md Notes): `Illuminate\Cache\Events\*` firing on every `Cache::get()`/`put()` call means NFR-052-03 query-count tests must filter to the `albums`/`photos` table specifically, not assert literal zero; and `actingAs()` leaves the auth guard authenticated across calls within a test method, so simulating "guest after an authenticated call" needs an explicit `forgetGuards()`. + +**Closed out:** Full `php artisan test` suite run to completion once (2896 passed / 3 failed — 2 were this feature's own test bug since fixed and re-verified, 1 pre-existing/unrelated confirmed via `git stash`); Implementation Drift Gate recorded in plan.md (Pass); `docs/specs/4-architecture/roadmap.md` moved from Active to Completed. A pre-existing, unrelated full-suite infrastructure issue was also found and documented (not fixed, out of scope): several Artisan commands call `set_time_limit(600)`, which resets the execution-timer budget for the entire `php artisan test` process (one continuous PHP process for the whole suite), so a slow-enough run can fatal near the end regardless of test content. + +### Feature 052 (prior session, 2026-07-21) — Spec drafted and complete, all open questions resolved + +**Request:** New feature to cache values whose result depends on the requesting user's access rights to one or more albums, with a dependency-mapping mechanism (album id + user id) so cached entries can be invalidated when access rights change, a photo is uploaded, an album is moved, etc. + +**Existing infrastructure found (adjacent, not reused as-is):** `App\Metadata\Cache\RouteCacher`/`RouteCacheManager`/`App\Enum\CacheTag` already caches whole HTTP responses keyed by route+user, tagged by album id, with `AlbumCacheCleaner`/`TaggedRouteCacheCleaner` listeners reacting to `AlbumRouteCacheUpdated`/`TaggedRouteCacheUpdated`. Governed by `cache_enabled`, forced off by default since Feature 040 (`disable-request-caching`). Left untouched — this feature builds a new, independent, general-purpose service instead. + +**Three real invalidation gaps confirmed during investigation (all fixed by this feature, FR-052-03/04/05):** +- `App\Actions\Album\Move::do()` (`app/Actions/Album/Move.php`) dispatches no event on album move/re-parent. +- `App\Http\Controllers\Gallery\SharingController` (`create`/`edit`/`delete`/`propagate`) dispatches no event when `AccessPermission` rows change. +- `App\Http\Controllers\Admin\UserGroupsManagementController` (`addUser`/`removeUser`/`updateUserRole`) dispatches no event on group-membership change. + +**5 open questions logged and resolved same-day** (Q-052-01..05, all Option A): +- Q-052-01: generic service (not query-specific). **Pilot consumer changed 2026-07-21 (user instruction, post-resolution):** instead of `BaseAlbumImpl::current_user_permissions()`, the two pilots are `AlbumRepository::getChildrenPaginated()` (sub-albums) and `PhotoRepository::getPhotosForAlbumPaginated()` (photos) — both permission-filtered, hit on every album view, and the exact routes (`Album::albums`/`Album::photos`) the existing HTTP response cache already lists but runs uncached by default. No other adoption in this feature. +- Q-052-02: new independent `App\Services\Cache\ManagedCacheService` — user clarified it "does not necessarily have to be related to Query," driving the generic (not "query cache") naming/shape. +- Q-052-03: new independent config flag `managed_cache_enabled` (default `true`), decoupled from Feature 040's `cache_enabled`. New `managed_cache_ttl` config too. +- Q-052-04: ancestor-path tagging (tag a cached entry with its own album id + every ancestor id via `Album::ancestorsOf()`) so an ancestor's tag eviction reaches descendants without a runtime tree walk. **User correction:** there is no native cache-tagging primitive available (default `file` driver) — "tags" are hand-rolled key-list bookkeeping (a tag is a cache key whose value is a set of member keys), mirroring `RouteCacher::rememberTags()`/`forgetTag()`'s existing pattern, reimplemented independently. +- Q-052-05: user-group membership change is in scope as an invalidation trigger (new `UserGroupMembershipChanged` event, third gap found to match). + +**Spec is now feature-complete** (FR-052-01..11, NFR-052-01..05, S-052-01..12, full Interface & Contract Catalogue including two new events `AccessPermissionChanged`/`UserGroupMembershipChanged` and two new listeners `ManagedCacheAlbumInvalidator`/`ManagedCacheUserInvalidator`). Directory renamed `052-query-cache-service` → `052-managed-cache-service` to match the generic naming resolution. The album-invalidation listener also evicts a mutated album's immediate-parent tag, closing a "negative cache" gap (a child becoming newly visible/hidden must still invalidate the parent's cached children list even though that list never referenced the child). + +**Not yet done:** plan.md/tasks.md (spec is ready for planning — Analysis Gate not yet run). Implementation not started. + ### Feature 049 – Migration to Nuxt UI — Spec/Plan/Tasks Drafted (this session) **Request:** Replace PrimeVue (`primevue`, `@primeuix/themes`, `tailwindcss-primeui`, `primeicons`) with Nuxt UI (`@nuxt/ui`, standalone Vue mode — no full Nuxt framework) across the frontend. @@ -44,19 +92,21 @@ _Last updated: 2026-07-02_ ## Next Steps -1. Confirm dependency approvals (`@nuxt/ui`, `@iconify-json/prime`) with the user, then start Feature 049 implementation at T-049-01 (install Nuxt UI in standalone Vue mode) — see [tasks.md](4-architecture/features/049-nuxt-ui-migration/tasks.md). -2. Alternatively/in parallel across sessions: start Feature 048 implementation at T-048-01 (repo-wide caller sweep) then T-048-02/03 (unit tests reproducing the bug) — see [tasks.md](4-architecture/features/048-fix-multi-group-permissions/tasks.md). -3. Feature 047 (Person Smart Album) remains drafted but not implemented — no active work this session. -4. Feature 042 Part B (I7–I10, admin maintenance photo title links) remains outstanding from a prior session — see [tasks.md](4-architecture/features/042-webshop-order-item-display/tasks.md) T-042-16 to T-042-20. +1. Feature 052 is done — no follow-up required unless broader `ManagedCacheService` adoption (deferred per spec Non-Goals) is picked up as a future feature. +2. Confirm dependency approvals (`@nuxt/ui`, `@iconify-json/prime`) with the user, then start Feature 049 implementation at T-049-01 (install Nuxt UI in standalone Vue mode) — see [tasks.md](4-architecture/features/049-nuxt-ui-migration/tasks.md). +3. Alternatively/in parallel across sessions: start Feature 048 implementation at T-048-01 (repo-wide caller sweep) then T-048-02/03 (unit tests reproducing the bug) — see [tasks.md](4-architecture/features/048-fix-multi-group-permissions/tasks.md). +4. Feature 047 (Person Smart Album) remains drafted but not implemented — no active work this session. +5. Feature 042 Part B (I7–I10, admin maintenance photo title links) remains outstanding from a prior session — see [tasks.md](4-architecture/features/042-webshop-order-item-display/tasks.md) T-042-16 to T-042-20. ## Open Questions -None blocking. Q-049-01, Q-049-02, Q-049-03 resolved 2026-07-02 (ADR-0005). Q-048-01 resolved 2026-07-01. +None blocking. Q-052-01..07 all resolved (01-05 on 2026-07-21, 06-07 on 2026-07-28 — see spec.md and open-questions.md for full rationale, including Q-052-07's non-default Option B resolution). Q-049-01, Q-049-02, Q-049-03 resolved 2026-07-02 (ADR-0005). Q-048-01 resolved 2026-07-01. ## Key Artefacts +- Feature 052: [spec.md](4-architecture/features/052-managed-cache-service/spec.md) · [plan.md](4-architecture/features/052-managed-cache-service/plan.md) · [tasks.md](4-architecture/features/052-managed-cache-service/tasks.md) (implemented, T-052-01..22 all `[x]`) - Feature 049: [spec.md](4-architecture/features/049-nuxt-ui-migration/spec.md) · [plan.md](4-architecture/features/049-nuxt-ui-migration/plan.md) · [tasks.md](4-architecture/features/049-nuxt-ui-migration/tasks.md) · [ADR-0005](6-decisions/ADR-0005-nuxt-ui-migration.md) - Feature 048: [spec.md](4-architecture/features/048-fix-multi-group-permissions/spec.md) · [plan.md](4-architecture/features/048-fix-multi-group-permissions/plan.md) · [tasks.md](4-architecture/features/048-fix-multi-group-permissions/tasks.md) -- Open questions: [open-questions.md](4-architecture/open-questions.md) (Q-049-01..03, Q-048-01 — all resolved) +- Open questions: [open-questions.md](4-architecture/open-questions.md) (Q-052-01..07, Q-049-01..03, Q-048-01 — all resolved) - Roadmap: [roadmap.md](4-architecture/roadmap.md) -- Knowledge map: [knowledge-map.md](4-architecture/knowledge-map.md) (Frontend Dependencies section annotated with the pending PrimeVue→Nuxt UI swap) +- Knowledge map: [knowledge-map.md](4-architecture/knowledge-map.md) (Frontend Dependencies section annotated with the pending PrimeVue→Nuxt UI swap; Feature 052's `ManagedCacheService`/events/listeners documented under Infrastructure Layer) diff --git a/package-lock.json b/package-lock.json index d8aec7023b3..de1ffea7161 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1716,6 +1716,7 @@ "version": "2.6.0", "resolved": "https://registry.npmjs.org/@parcel/watcher/-/watcher-2.6.0.tgz", "integrity": "sha512-7FNeNl8NCE7aINx7WXiKQrPYZWC/hvrTsmk6zmxbI7LTXE7hVek/n8AfVgpe2y82zl3w0HvCHN0bVKMBoJcC0w==", + "dev": true, "hasInstallScript": true, "license": "MIT", "optional": true, @@ -1754,6 +1755,7 @@ "cpu": [ "arm64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1774,6 +1776,7 @@ "cpu": [ "arm64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1794,6 +1797,7 @@ "cpu": [ "x64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1814,6 +1818,7 @@ "cpu": [ "x64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1834,6 +1839,7 @@ "cpu": [ "arm" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1854,6 +1860,7 @@ "cpu": [ "arm" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1874,6 +1881,7 @@ "cpu": [ "arm64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1894,6 +1902,7 @@ "cpu": [ "arm64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1914,6 +1923,7 @@ "cpu": [ "x64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1934,6 +1944,7 @@ "cpu": [ "x64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1954,6 +1965,7 @@ "cpu": [ "arm64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1974,6 +1986,7 @@ "cpu": [ "x64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -2243,9 +2256,6 @@ "cpu": [ "arm64" ], - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -2262,9 +2272,6 @@ "cpu": [ "arm64" ], - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -2281,9 +2288,6 @@ "cpu": [ "ppc64" ], - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -2300,9 +2304,6 @@ "cpu": [ "s390x" ], - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -2319,9 +2320,6 @@ "cpu": [ "x64" ], - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -2338,9 +2336,6 @@ "cpu": [ "x64" ], - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -4495,9 +4490,9 @@ "license": "ISC" }, "node_modules/brace-expansion": { - "version": "5.0.8", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.8.tgz", - "integrity": "sha512-JZyDyq3D4AUifKTPOB7DELf6XsB3WdPuNxCtob1vFXPsSXhdAiHBWJ/tJ8HAc9aH84BK+5JFZLNkJKx3G9kzQg==", + "version": "5.0.9", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.9.tgz", + "integrity": "sha512-ScQ4IuvIEF1TMlP7Zt+vjJ//9zlPb2SDcxWxM3bk8s6t6GGdJ7KO1dCcTidOPJKePW30LE/2cT7wCyPho9/Wxg==", "dev": true, "license": "MIT", "dependencies": { @@ -4962,9 +4957,9 @@ "license": "MIT" }, "node_modules/editorconfig/node_modules/brace-expansion": { - "version": "2.1.2", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.1.2.tgz", - "integrity": "sha512-w5JZcKgdhDOgOwm8H+KgbosopHMuGcl6qbulwjtz3SM7I7P3yW1eAjzMPLrIE+NQ9vjgANKHWeMHnrT0OXW1oA==", + "version": "2.1.4", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.1.4.tgz", + "integrity": "sha512-hGfVzPxthbf3+2yjg/RBs60cB0FhqBS/zvdV/4wn4/BmN0bNMMHPc4V/BbFieqf1TKAGGAHnY4eSjajCl0f2Xg==", "dev": true, "license": "MIT", "dependencies": { @@ -5965,9 +5960,9 @@ "license": "MIT" }, "node_modules/glob/node_modules/brace-expansion": { - "version": "2.1.2", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.1.2.tgz", - "integrity": "sha512-w5JZcKgdhDOgOwm8H+KgbosopHMuGcl6qbulwjtz3SM7I7P3yW1eAjzMPLrIE+NQ9vjgANKHWeMHnrT0OXW1oA==", + "version": "2.1.4", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.1.4.tgz", + "integrity": "sha512-hGfVzPxthbf3+2yjg/RBs60cB0FhqBS/zvdV/4wn4/BmN0bNMMHPc4V/BbFieqf1TKAGGAHnY4eSjajCl0f2Xg==", "dev": true, "license": "MIT", "dependencies": { @@ -7160,6 +7155,7 @@ "version": "7.1.1", "resolved": "https://registry.npmjs.org/node-addon-api/-/node-addon-api-7.1.1.tgz", "integrity": "sha512-5m3bsyrjFWE1xf7nz7YXdN4udnVtXK6/Yfgn5qnahL6bCkf2yKt4k3nuTKAtT4r3IG8JNR2ncsIMdZuAzJjHQQ==", + "dev": true, "license": "MIT", "optional": true }, @@ -9414,9 +9410,6 @@ "cpu": [ "arm64" ], - "libc": [ - "glibc" - ], "license": "MPL-2.0", "optional": true, "os": [ @@ -9437,9 +9430,6 @@ "cpu": [ "arm64" ], - "libc": [ - "musl" - ], "license": "MPL-2.0", "optional": true, "os": [ @@ -9460,9 +9450,6 @@ "cpu": [ "x64" ], - "libc": [ - "musl" - ], "license": "MPL-2.0", "optional": true, "os": [ diff --git a/tests/Feature_v2/Album/AlbumMoveTest.php b/tests/Feature_v2/Album/AlbumMoveTest.php index 7bd845d8a55..24a61b8fca5 100644 --- a/tests/Feature_v2/Album/AlbumMoveTest.php +++ b/tests/Feature_v2/Album/AlbumMoveTest.php @@ -18,7 +18,9 @@ namespace Tests\Feature_v2\Album; +use App\Events\AlbumSaved; use App\Models\AccessPermission; +use Illuminate\Support\Facades\Event; use Tests\Feature_v2\Base\BaseApiWithDataTest; class AlbumMoveTest extends BaseApiWithDataTest @@ -53,6 +55,39 @@ public function testMoveAlbumAuthorizedOwner(): void $response->assertSee($this->subAlbum1->id); } + public function testMoveAlbumToRootDispatchesAlbumSaved(): void + { + Event::fake([AlbumSaved::class]); + + $response = $this->actingAs($this->userMayUpload1)->postJson('Album::move', [ + 'album_id' => null, + 'album_ids' => [$this->subAlbum1->id], + ]); + $this->assertNoContent($response); + + Event::assertDispatched(AlbumSaved::class, fn (AlbumSaved $event) => $event->album->id === $this->subAlbum1->id); + } + + public function testMoveAlbumIntoAnotherAlbumDispatchesAlbumSavedForMovedAlbumAndOldParent(): void + { + Event::fake([AlbumSaved::class]); + + // subAlbum1 starts out as a child of album1; moving it to album5 changes its parent. + $response = $this->actingAs($this->admin)->postJson('Album::move', [ + 'album_id' => $this->album5->id, + 'album_ids' => [$this->subAlbum1->id], + ]); + $this->assertNoContent($response); + + // The moved album itself, plus its *old* parent (album1) — S-052-06: moving an + // album must invalidate both the old and the new parent's cached children list. + // (album5, the new parent, is covered separately via the moved album's own + // post-move parent_id — see ManagedCacheAlbumInvalidator::handleAlbumSaved.) + Event::assertDispatched(AlbumSaved::class, 2); + Event::assertDispatched(AlbumSaved::class, fn (AlbumSaved $event) => $event->album->id === $this->subAlbum1->id); + Event::assertDispatched(AlbumSaved::class, fn (AlbumSaved $event) => $event->album->id === $this->album1->id); + } + public function testMoveAlbumAuthorizedUser(): void { AccessPermission::factory() diff --git a/tests/Feature_v2/Album/SharingTest.php b/tests/Feature_v2/Album/SharingTest.php index cd56f135413..95096e9de21 100644 --- a/tests/Feature_v2/Album/SharingTest.php +++ b/tests/Feature_v2/Album/SharingTest.php @@ -19,7 +19,9 @@ namespace Tests\Feature_v2\Album; use App\Constants\AccessPermissionConstants as APC; +use App\Events\AccessPermissionChanged; use App\Models\AccessPermission; +use Illuminate\Support\Facades\Event; use Tests\Feature_v2\Base\BaseApiWithDataTest; class SharingTest extends BaseApiWithDataTest @@ -324,4 +326,96 @@ public function testOverrideMixed(): void ->where(APC::USER_ID, '=', $this->userNoUpload->id) ->count()); } + + public function testCreateDispatchesAccessPermissionChangedPerAlbum(): void + { + Event::fake([AccessPermissionChanged::class]); + + $response = $this->actingAs($this->userMayUpload1)->postJson('Sharing', [ + 'user_ids' => [$this->userLocked->id], + 'group_ids' => [], + 'album_ids' => [$this->subAlbum1->id, $this->album1->id], + 'grants_edit' => true, + 'grants_delete' => true, + 'grants_download' => true, + 'grants_full_photo_access' => true, + 'grants_upload' => true, + ]); + $this->assertOk($response); + + Event::assertDispatchedTimes(AccessPermissionChanged::class, 2); + Event::assertDispatched(AccessPermissionChanged::class, fn (AccessPermissionChanged $event) => $event->base_album_id === $this->subAlbum1->id); + Event::assertDispatched(AccessPermissionChanged::class, fn (AccessPermissionChanged $event) => $event->base_album_id === $this->album1->id); + } + + public function testEditDispatchesAccessPermissionChanged(): void + { + $this->actingAs($this->userMayUpload1)->postJson('Sharing', [ + 'user_ids' => [$this->userLocked->id], + 'group_ids' => [], + 'album_ids' => [$this->subAlbum1->id], + 'grants_edit' => true, + 'grants_delete' => true, + 'grants_download' => true, + 'grants_full_photo_access' => true, + 'grants_upload' => true, + ]); + $perm = AccessPermission::where(APC::BASE_ALBUM_ID, '=', $this->subAlbum1->id)->where(APC::USER_ID, '=', $this->userLocked->id)->firstOrFail(); + + Event::fake([AccessPermissionChanged::class]); + + $response = $this->actingAs($this->userMayUpload1)->patchJson('Sharing', [ + 'perm_id' => $perm->id, + 'grants_edit' => false, + 'grants_delete' => false, + 'grants_download' => false, + 'grants_full_photo_access' => false, + 'grants_upload' => false, + ]); + $this->assertOk($response); + + Event::assertDispatchedTimes(AccessPermissionChanged::class, 1); + Event::assertDispatched(AccessPermissionChanged::class, fn (AccessPermissionChanged $event) => $event->base_album_id === $this->subAlbum1->id); + } + + public function testDeleteDispatchesAccessPermissionChanged(): void + { + $this->actingAs($this->userMayUpload1)->postJson('Sharing', [ + 'user_ids' => [$this->userLocked->id], + 'group_ids' => [], + 'album_ids' => [$this->subAlbum1->id], + 'grants_edit' => true, + 'grants_delete' => true, + 'grants_download' => true, + 'grants_full_photo_access' => true, + 'grants_upload' => true, + ]); + $perm = AccessPermission::where(APC::BASE_ALBUM_ID, '=', $this->subAlbum1->id)->where(APC::USER_ID, '=', $this->userLocked->id)->firstOrFail(); + + Event::fake([AccessPermissionChanged::class]); + + $response = $this->actingAs($this->userMayUpload1)->deleteJson('Sharing', [ + 'perm_id' => $perm->id, + ]); + $this->assertNoContent($response); + + Event::assertDispatchedTimes(AccessPermissionChanged::class, 1); + Event::assertDispatched(AccessPermissionChanged::class, fn (AccessPermissionChanged $event) => $event->base_album_id === $this->subAlbum1->id); + } + + public function testPropagateDispatchesAccessPermissionChangedForSourceAndDescendants(): void + { + Event::fake([AccessPermissionChanged::class]); + + $response = $this->actingAs($this->userMayUpload1)->putJson('Sharing', [ + 'album_id' => $this->album1->id, + 'shall_override' => false, + ]); + $this->assertNoContent($response); + + // album1 (source) + subAlbum1 (its only descendant). + Event::assertDispatchedTimes(AccessPermissionChanged::class, 2); + Event::assertDispatched(AccessPermissionChanged::class, fn (AccessPermissionChanged $event) => $event->base_album_id === $this->album1->id); + Event::assertDispatched(AccessPermissionChanged::class, fn (AccessPermissionChanged $event) => $event->base_album_id === $this->subAlbum1->id); + } } \ No newline at end of file diff --git a/tests/Feature_v2/Caching/ManagedCacheServiceWiringTest.php b/tests/Feature_v2/Caching/ManagedCacheServiceWiringTest.php new file mode 100644 index 00000000000..c303ea468b7 --- /dev/null +++ b/tests/Feature_v2/Caching/ManagedCacheServiceWiringTest.php @@ -0,0 +1,192 @@ + real event -> + * real registered listener (EventServiceProvider) -> ManagedCacheService tag + * eviction, with no faking. Complements the more granular unit tests for the + * service and listeners in isolation. + */ +class ManagedCacheServiceWiringTest extends BaseApiWithDataTest +{ + private ManagedCacheService $cache_service; + + public function setUp(): void + { + parent::setUp(); + Configs::set('managed_cache_enabled', '1'); + $this->cache_service = new ManagedCacheService(new ConfigManager()); + } + + public function testMovingAlbumEvictsOldAndNewParentTagsThroughTheRealEventBus(): void + { + $old_parent_key = 'wiring-test:children:' . $this->album1->id; + $new_parent_key = 'wiring-test:children:' . $this->album5->id; + $this->cache_service->remember($old_parent_key, ['album:' . $this->album1->id], 60, fn () => 'cached-children-of-album1'); + $this->cache_service->remember($new_parent_key, ['album:' . $this->album5->id], 60, fn () => 'cached-children-of-album5'); + + $response = $this->actingAs($this->admin)->postJson('Album::move', [ + 'album_id' => $this->album5->id, + 'album_ids' => [$this->subAlbum1->id], + ]); + $this->assertNoContent($response); + + self::assertNull(Cache::get($old_parent_key)); + self::assertNull(Cache::get($new_parent_key)); + } + + public function testUserGroupMembershipChangeEvictsTheUsersTagThroughTheRealEventBus(): void + { + $this->requireSe(); + + $key = 'wiring-test:user:' . $this->userNoUpload->id; + $this->cache_service->remember($key, ['user:' . $this->userNoUpload->id], 60, fn () => 'cached-permissions'); + + $response = $this->actingAs($this->userWithGroupAdmin)->postJson('/UserGroups/Users', [ + 'user_id' => $this->userNoUpload->id, + 'group_id' => $this->group1->id, + ]); + $this->assertCreated($response); + + self::assertNull(Cache::get($key)); + + $this->resetSe(); + } + + public function testGettingAlbumPhotosTwiceHitsCacheOnSecondCallThroughTheRealEndpoint(): void + { + $first = $this->actingAs($this->userMayUpload1)->getJsonWithData('Album::photos', ['album_id' => $this->album1->id]); + $this->assertOk($first); + + DB::enableQueryLog(); + $second = $this->actingAs($this->userMayUpload1)->getJsonWithData('Album::photos', ['album_id' => $this->album1->id]); + $photo_queries = array_filter(DB::getQueryLog(), fn ($q) => str_contains(strtolower($q['query']), 'from "photos"')); + DB::flushQueryLog(); + DB::disableQueryLog(); + + $this->assertOk($second); + self::assertCount(0, $photo_queries, 'A managed-cache hit must not re-execute the photo query, even through the real endpoint.'); + } + + public function testAddingAPhotoInvalidatesThePreviouslyCachedPhotoListingThroughTheRealEventBus(): void + { + $first = $this->actingAs($this->userMayUpload1)->getJsonWithData('Album::photos', ['album_id' => $this->album1->id]); + $this->assertOk($first); + $first->assertJsonPath('total', 2); // photo1 + photo1b, per BaseApiWithDataTest fixtures + + $new_photo = Photo::factory()->owned_by($this->userMayUpload1)->in($this->album1)->create(); + PhotoAdded::dispatch($new_photo->id); + + $second = $this->actingAs($this->userMayUpload1)->getJsonWithData('Album::photos', ['album_id' => $this->album1->id]); + $this->assertOk($second); + $second->assertJsonPath('total', 3); + } + + /** + * S-052-08b: a child hidden from a user at write time must become visible + * on the next read once that user is granted access, even though the + * cached (empty-for-that-child) listing never contained the child's own + * tag — closed by evicting the *parent's* tag whenever any child's + * `AccessPermissionChanged` fires (FR-052-06). + */ + public function testNewlyVisibleChildInvalidatesParentsCachedChildrenListThroughTheRealEventBus(): void + { + // userMayUpload2 can see album1 itself (perm1, BaseApiWithDataTest fixtures) but subAlbum1 + // has no permission of its own (permissions aren't automatically inherited without an + // explicit Sharing::propagate call) — so it's initially absent from the children listing. + $first = $this->actingAs($this->userMayUpload2)->getJsonWithData('Album::albums', ['album_id' => $this->album1->id]); + $this->assertOk($first); + $first->assertJsonPath('total', 0); + + $grant = $this->actingAs($this->userMayUpload1)->postJson('Sharing', [ + 'user_ids' => [$this->userMayUpload2->id], + 'group_ids' => [], + 'album_ids' => [$this->subAlbum1->id], + 'grants_edit' => false, + 'grants_delete' => false, + 'grants_download' => true, + 'grants_full_photo_access' => true, + 'grants_upload' => false, + ]); + $this->assertOk($grant); + + $second = $this->actingAs($this->userMayUpload2)->getJsonWithData('Album::albums', ['album_id' => $this->album1->id]); + $this->assertOk($second); + $second->assertJsonPath('total', 1); + $second->assertJsonPath('data.0.id', $this->subAlbum1->id); + } + + /** + * S-052-11: a guest (unauthenticated) caller uses a fixed 'guest' cache-key + * segment — cacheable like any other caller, but never subject to + * `UserGroupMembershipChanged` eviction since it isn't tied to a real user id. + */ + public function testGuestCachedListingHitsCacheAndSurvivesAnUnrelatedUserGroupChange(): void + { + // subAlbum4 is public (BaseApiWithDataTest fixtures), so guests can see it under album4. + // Filtering on `parent_id` (not just `from "albums"`) specifically isolates the cached + // *children* query from the route's own incidental, uncached parent-album resolution + // (e.g. via the `login_required:album` middleware), which legitimately queries `albums` + // by `id` on every request regardless of this feature's caching. + $is_children_query = fn ($q) => str_contains(strtolower($q['query']), 'from "albums"') && str_contains(strtolower($q['query']), 'parent_id'); + + $first = $this->getJsonWithData('Album::albums', ['album_id' => $this->album4->id]); + $this->assertOk($first); + $first->assertJsonPath('total', 1); + + DB::enableQueryLog(); + $second = $this->getJsonWithData('Album::albums', ['album_id' => $this->album4->id]); + $children_queries = array_filter(DB::getQueryLog(), $is_children_query); + DB::flushQueryLog(); + DB::disableQueryLog(); + + $this->assertOk($second); + self::assertCount(0, $children_queries, 'Guest cache hit must not re-execute the children query.'); + + // A completely unrelated user-group membership change must not evict the guest-scoped entry. + $this->requireSe(); + $this->actingAs($this->userWithGroupAdmin)->postJson('/UserGroups/Users', [ + 'user_id' => $this->userNoUpload->id, + 'group_id' => $this->group1->id, + ]); + $this->resetSe(); + // actingAs() leaves the guard authenticated for subsequent calls — explicitly log out so + // the next request is a genuine guest request again, matching the first two calls above. + $this->app['auth']->forgetGuards(); + + DB::enableQueryLog(); + $third = $this->getJsonWithData('Album::albums', ['album_id' => $this->album4->id]); + $children_queries_after = array_filter(DB::getQueryLog(), $is_children_query); + DB::flushQueryLog(); + DB::disableQueryLog(); + + $this->assertOk($third); + self::assertCount(0, $children_queries_after, 'Unrelated user-group change must not evict the guest-scoped cache entry.'); + } +} diff --git a/tests/Feature_v2/Settings/GetAllSettingsTest.php b/tests/Feature_v2/Settings/GetAllSettingsTest.php index dbbc7ba526b..a1aaeb725da 100644 --- a/tests/Feature_v2/Settings/GetAllSettingsTest.php +++ b/tests/Feature_v2/Settings/GetAllSettingsTest.php @@ -69,8 +69,13 @@ public function testGetAllSettingsAdmin(): void ], ]); - // Mod Cache must be hidden by default (ENABLE_REQUEST_CACHING defaults to false). - $response->assertJsonMissing(['cat' => 'Mod Cache']); + // Feature-040 "Mod Cache" rows are hidden by default (ENABLE_REQUEST_CACHING defaults to false)... + $response->assertJsonMissing(['key' => 'cache_enabled']); + $response->assertJsonMissing(['key' => 'cache_ttl']); + $response->assertJsonMissing(['key' => 'cache_event_logging']); + // ...but managed_cache_* (Feature 052) stays visible regardless (Q-052-07). + $response->assertJsonFragment(['key' => 'managed_cache_enabled']); + $response->assertJsonFragment(['key' => 'managed_cache_ttl']); $response = $this->actingAs($this->admin)->getJson('Settings::init'); $this->assertOk($response); @@ -91,6 +96,7 @@ public function testModCacheVisibleWhenFeatureEnabled(): void $response = $this->actingAs($this->admin)->getJson('Settings'); $this->assertOk($response); $response->assertJsonFragment(['cat' => 'Mod Cache']); + $response->assertJsonFragment(['key' => 'cache_enabled']); } public function testModCacheHiddenWhenFeatureDisabled(): void @@ -99,6 +105,29 @@ public function testModCacheHiddenWhenFeatureDisabled(): void $response = $this->actingAs($this->admin)->getJson('Settings'); $this->assertOk($response); - $response->assertJsonMissing(['cat' => 'Mod Cache']); + $response->assertJsonMissing(['key' => 'cache_enabled']); + $response->assertJsonMissing(['key' => 'cache_ttl']); + $response->assertJsonMissing(['key' => 'cache_event_logging']); + } + + public function testManagedCacheConfigVisibleRegardlessOfRequestCachingFeature(): void + { + config(['features.enable-request-caching' => false]); + + $response = $this->actingAs($this->admin)->getJson('Settings'); + $this->assertOk($response); + // "Mod Cache" category is still present (Q-052-07, Option B), but only for these two keys. + $response->assertJsonFragment(['cat' => 'Mod Cache']); + $response->assertJsonFragment(['key' => 'managed_cache_enabled']); + $response->assertJsonFragment(['key' => 'managed_cache_ttl']); + $response->assertJsonMissing(['key' => 'cache_enabled']); + + config(['features.enable-request-caching' => true]); + + $response = $this->actingAs($this->admin)->getJson('Settings'); + $this->assertOk($response); + $response->assertJsonFragment(['key' => 'managed_cache_enabled']); + $response->assertJsonFragment(['key' => 'managed_cache_ttl']); + $response->assertJsonFragment(['key' => 'cache_enabled']); } } \ No newline at end of file diff --git a/tests/Feature_v2/UserGroups/UserGroupMembershipTest.php b/tests/Feature_v2/UserGroups/UserGroupMembershipTest.php index a2eaf040325..e5b1af4d8a1 100644 --- a/tests/Feature_v2/UserGroups/UserGroupMembershipTest.php +++ b/tests/Feature_v2/UserGroups/UserGroupMembershipTest.php @@ -8,6 +8,8 @@ namespace Tests\Feature_v2\UserGroups; +use App\Events\UserGroupMembershipChanged; +use Illuminate\Support\Facades\Event; use Tests\Feature_v2\Base\BaseApiWithDataTest; class UserGroupMembershipTest extends BaseApiWithDataTest @@ -145,4 +147,47 @@ public function testUpdateUserRoleAuthorized(): void $response->assertJsonPath('members.0.role', 'admin'); $response->assertJsonPath('members.1.role', 'admin'); } + + public function testAddUserToGroupDispatchesUserGroupMembershipChanged(): void + { + Event::fake([UserGroupMembershipChanged::class]); + + $response = $this->actingAs($this->userWithGroupAdmin)->postJson('/UserGroups/Users', [ + 'user_id' => $this->userNoUpload->id, + 'group_id' => $this->group1->id, + ]); + $this->assertCreated($response); + + Event::assertDispatchedTimes(UserGroupMembershipChanged::class, 1); + Event::assertDispatched(UserGroupMembershipChanged::class, fn (UserGroupMembershipChanged $event) => $event->user_id === $this->userNoUpload->id); + } + + public function testRemoveUserFromGroupDispatchesUserGroupMembershipChanged(): void + { + Event::fake([UserGroupMembershipChanged::class]); + + $response = $this->actingAs($this->userWithGroupAdmin)->deleteJson('/UserGroups/Users', [ + 'user_id' => $this->userWithGroup1->id, + 'group_id' => $this->group1->id, + ]); + $this->assertOk($response); + + Event::assertDispatchedTimes(UserGroupMembershipChanged::class, 1); + Event::assertDispatched(UserGroupMembershipChanged::class, fn (UserGroupMembershipChanged $event) => $event->user_id === $this->userWithGroup1->id); + } + + public function testUpdateUserRoleDispatchesUserGroupMembershipChanged(): void + { + Event::fake([UserGroupMembershipChanged::class]); + + $response = $this->actingAs($this->userWithGroupAdmin)->patchJson('/UserGroups/Users', [ + 'user_id' => $this->userWithGroup1->id, + 'group_id' => $this->group1->id, + 'role' => 'admin', + ]); + $this->assertOk($response); + + Event::assertDispatchedTimes(UserGroupMembershipChanged::class, 1); + Event::assertDispatched(UserGroupMembershipChanged::class, fn (UserGroupMembershipChanged $event) => $event->user_id === $this->userWithGroup1->id); + } } diff --git a/tests/Unit/Listeners/ManagedCacheAlbumInvalidatorTest.php b/tests/Unit/Listeners/ManagedCacheAlbumInvalidatorTest.php new file mode 100644 index 00000000000..6a32d6888a0 --- /dev/null +++ b/tests/Unit/Listeners/ManagedCacheAlbumInvalidatorTest.php @@ -0,0 +1,171 @@ +cache_service = new ManagedCacheService(new ConfigManager()); + $this->listener = new ManagedCacheAlbumInvalidator($this->cache_service); + + $owner = User::factory()->create(); + $this->root_album = Album::factory()->as_root()->owned_by($owner)->create(); + $this->child_album = Album::factory()->children_of($this->root_album)->owned_by($owner)->create(); + } + + private function primeTag(string $tag): string + { + $key = 'mcai-test:' . $tag . ':' . uniqid(); + $this->cache_service->remember($key, [$tag], 60, fn () => 'value'); + + return $key; + } + + /** + * A minimal photo row + `photo_album` pivot entry, bypassing `PhotoFactory`'s + * `configure()` hook (7 `SizeVariant`s + a `Statistics` row via `Factory::create()`'s + * `afterCreating`) — this listener only needs a resolvable photo_id/album_id pivot + * row, not a fully-featured photo. `Photo::create()` (Eloquent, not the factory) + * still runs the model's own id-generation/`creating` machinery. + */ + private function createPhotoInAlbum(Album $album): Photo + { + $photo = new Photo(); + $photo->forceFill(Photo::factory()->raw(['owner_id' => $album->owner_id])); + $photo->save(); + DB::table('photo_album')->insert(['photo_id' => $photo->id, 'album_id' => $album->id]); + + return $photo; + } + + public function testHandleAlbumSavedEvictsAlbumAndParentTags(): void + { + $own_key = $this->primeTag('album:' . $this->child_album->id); + $parent_key = $this->primeTag('album:' . $this->root_album->id); + + $this->listener->handleAlbumSaved(new AlbumSaved($this->child_album)); + + self::assertNull(Cache::get($own_key)); + self::assertNull(Cache::get($parent_key)); + } + + public function testHandleAlbumDeletedEvictsOnlyParentTag(): void + { + $parent_key = $this->primeTag('album:' . $this->root_album->id); + + $this->listener->handleAlbumDeleted(new AlbumDeleted($this->root_album->id)); + + self::assertNull(Cache::get($parent_key)); + } + + public function testHandleAlbumDeletedWithNoParentEvictsRootTag(): void + { + $root_key = $this->primeTag('album:root'); + + $this->listener->handleAlbumDeleted(new AlbumDeleted(null)); + + self::assertNull(Cache::get($root_key)); + } + + public function testHandleAccessPermissionChangedEvictsAlbumAndParentTags(): void + { + $own_key = $this->primeTag('album:' . $this->child_album->id); + $parent_key = $this->primeTag('album:' . $this->root_album->id); + + $this->listener->handleAccessPermissionChanged(new AccessPermissionChanged($this->child_album->id)); + + self::assertNull(Cache::get($own_key)); + self::assertNull(Cache::get($parent_key)); + } + + public function testHandlePhotoSavedResolvesAlbumViaPivotAndEvictsTags(): void + { + $photo = $this->createPhotoInAlbum($this->child_album); + + $own_key = $this->primeTag('album:' . $this->child_album->id); + $parent_key = $this->primeTag('album:' . $this->root_album->id); + + $this->listener->handlePhotoSaved(new PhotoSaved($photo->id)); + + self::assertNull(Cache::get($own_key)); + self::assertNull(Cache::get($parent_key)); + } + + public function testHandlePhotoAddedResolvesAlbumViaPivotAndEvictsTags(): void + { + $photo = $this->createPhotoInAlbum($this->child_album); + + $own_key = $this->primeTag('album:' . $this->child_album->id); + + $this->listener->handlePhotoAdded(new PhotoAdded($photo->id)); + + self::assertNull(Cache::get($own_key)); + } + + public function testHandlePhotoDeletedEvictsAlbumAndParentTags(): void + { + $own_key = $this->primeTag('album:' . $this->child_album->id); + $parent_key = $this->primeTag('album:' . $this->root_album->id); + + $this->listener->handlePhotoDeleted(new PhotoDeleted($this->child_album->id)); + + self::assertNull(Cache::get($own_key)); + self::assertNull(Cache::get($parent_key)); + } + + public function testHandlePhotoMovedEvictsBothAlbumsAndTheirParents(): void + { + $from_key = $this->primeTag('album:' . $this->child_album->id); + $to_key = $this->primeTag('album:' . $this->root_album->id); + + $this->listener->handlePhotoMoved(new PhotoMoved('unused-photo-id', $this->child_album->id, $this->root_album->id)); + + self::assertNull(Cache::get($from_key)); + self::assertNull(Cache::get($to_key)); + } +} diff --git a/tests/Unit/Listeners/ManagedCacheUserInvalidatorTest.php b/tests/Unit/Listeners/ManagedCacheUserInvalidatorTest.php new file mode 100644 index 00000000000..2786c7dae7a --- /dev/null +++ b/tests/Unit/Listeners/ManagedCacheUserInvalidatorTest.php @@ -0,0 +1,48 @@ +remember($key, ['user:42'], 60, fn () => 'value'); + self::assertNotNull(Cache::get($key)); + + $listener->handle(new UserGroupMembershipChanged(42)); + + self::assertNull(Cache::get($key)); + } +} diff --git a/tests/Unit/Repositories/AlbumRepositoryTest.php b/tests/Unit/Repositories/AlbumRepositoryTest.php index c9959a646c8..1dc138707a1 100644 --- a/tests/Unit/Repositories/AlbumRepositoryTest.php +++ b/tests/Unit/Repositories/AlbumRepositoryTest.php @@ -23,9 +23,11 @@ use App\Enum\OrderSortingType; use App\Models\AccessPermission; use App\Models\Album; +use App\Models\Configs; use App\Models\User; use App\Repositories\AlbumRepository; use Illuminate\Pagination\LengthAwarePaginator; +use Illuminate\Support\Facades\DB; use Tests\AbstractTestCase; use Tests\Traits\RequiresEmptyAlbums; use Tests\Traits\RequiresEmptyUsers; @@ -209,4 +211,49 @@ public function testGetChildrenPaginatedForRootAlbums(): void $albumIds = array_map(fn ($item) => $item->id, $result->items()); $this->assertContains($this->parentAlbum->id, $albumIds); } + + public function testGetChildrenPaginatedCacheHitPerformsNoExtraQueries(): void + { + Configs::set('managed_cache_enabled', '1'); + Album::factory()->count(3)->children_of($this->parentAlbum)->owned_by($this->user)->create(); + $sorting = new AlbumSortingCriterion(ColumnSortingType::CREATED_AT, OrderSortingType::DESC); + $this->actingAs($this->user); + + // Cache miss - executes the query and caches the result. + $first = $this->repository->getChildrenPaginated($this->parentAlbum->id, $sorting, 10); + self::assertEquals(3, $first->total()); + + // Cache hit - must not re-execute the underlying album/owner query. Some + // unrelated queries are expected regardless (e.g. Illuminate\Cache\Events\* + // firing on every Cache::get()/put() call, handled by the pre-existing + // CacheListener, which itself reads a config value) — those aren't what + // NFR-052-03 is about, so this asserts no *album* query re-runs rather + // than an absolute zero. + DB::enableQueryLog(); + $second = $this->repository->getChildrenPaginated($this->parentAlbum->id, $sorting, 10); + $album_queries = array_filter(DB::getQueryLog(), fn ($q) => preg_match('/from\s*[`"]?albums[`"]?/i', $q['query']) === 1); + DB::flushQueryLog(); + DB::disableQueryLog(); + + self::assertCount(0, $album_queries, 'A managed-cache hit must not re-execute the album query.'); + self::assertEquals(3, $second->total()); + } + + public function testGetChildrenPaginatedIgnoresCacheWhenManagedCacheDisabled(): void + { + Configs::set('managed_cache_enabled', '0'); + Album::factory()->count(2)->children_of($this->parentAlbum)->owned_by($this->user)->create(); + $sorting = new AlbumSortingCriterion(ColumnSortingType::CREATED_AT, OrderSortingType::DESC); + $this->actingAs($this->user); + + $this->repository->getChildrenPaginated($this->parentAlbum->id, $sorting, 10); + + DB::enableQueryLog(); + $this->repository->getChildrenPaginated($this->parentAlbum->id, $sorting, 10); + $query_count = count(DB::getQueryLog()); + DB::flushQueryLog(); + DB::disableQueryLog(); + + self::assertGreaterThan(0, $query_count, 'With managed_cache_enabled=false every call must recompute.'); + } } diff --git a/tests/Unit/Repositories/PhotoRepositoryTest.php b/tests/Unit/Repositories/PhotoRepositoryTest.php new file mode 100644 index 00000000000..7ecdc27d7d9 --- /dev/null +++ b/tests/Unit/Repositories/PhotoRepositoryTest.php @@ -0,0 +1,141 @@ +user = User::factory()->may_upload()->create(); + $this->album = Album::factory()->as_root()->owned_by($this->user)->create(); + $this->repository = resolve(PhotoRepository::class); + } + + private function sorting(): PhotoSortingCriterion + { + return new PhotoSortingCriterion(ColumnSortingType::CREATED_AT, OrderSortingType::DESC); + } + + public function testGetPhotosForAlbumPaginatedReturnsLengthAwarePaginator(): void + { + Photo::factory()->count(3)->owned_by($this->user)->in($this->album)->create(); + + $this->actingAs($this->user); + $result = $this->repository->getPhotosForAlbumPaginated($this->album->id, $this->sorting(), 10); + + $this->assertInstanceOf(LengthAwarePaginator::class, $result); + $this->assertEquals(3, $result->total()); + } + + public function testGetPhotosForAlbumPaginatedCacheHitDoesNotReexecutePhotoQuery(): void + { + Configs::set('managed_cache_enabled', '1'); + Photo::factory()->count(3)->owned_by($this->user)->in($this->album)->create(); + $this->actingAs($this->user); + + $first = $this->repository->getPhotosForAlbumPaginated($this->album->id, $this->sorting(), 10); + self::assertEquals(3, $first->total()); + + // See AlbumRepositoryTest::testGetChildrenPaginatedCacheHitPerformsNoExtraQueries for why + // this filters to `photos` queries specifically rather than asserting a literal zero: + // every Cache::get()/put() call fires Illuminate\Cache\Events\*, handled by the + // pre-existing CacheListener, which does its own unrelated `configs` table read. + DB::enableQueryLog(); + $second = $this->repository->getPhotosForAlbumPaginated($this->album->id, $this->sorting(), 10); + $photo_queries = array_filter(DB::getQueryLog(), fn ($q) => preg_match('/from\s*[`"]?photos[`"]?/i', $q['query']) === 1); + DB::flushQueryLog(); + DB::disableQueryLog(); + + self::assertCount(0, $photo_queries, 'A managed-cache hit must not re-execute the photo query.'); + self::assertEquals(3, $second->total()); + } + + public function testGetPhotosForAlbumPaginatedIgnoresCacheWhenManagedCacheDisabled(): void + { + Configs::set('managed_cache_enabled', '0'); + Photo::factory()->count(2)->owned_by($this->user)->in($this->album)->create(); + $this->actingAs($this->user); + + $this->repository->getPhotosForAlbumPaginated($this->album->id, $this->sorting(), 10); + + DB::enableQueryLog(); + $this->repository->getPhotosForAlbumPaginated($this->album->id, $this->sorting(), 10); + $photo_queries = array_filter(DB::getQueryLog(), fn ($q) => preg_match('/from\s*[`"]?photos[`"]?/i', $q['query']) === 1); + DB::flushQueryLog(); + DB::disableQueryLog(); + + self::assertGreaterThan(0, count($photo_queries), 'With managed_cache_enabled=false every call must recompute.'); + } + + /** + * NFR-052-05: the cached (then retrieved) paginator's items, pagination + * metadata, and eager-loaded relations must match a freshly-queried one. + */ + public function testGetPhotosForAlbumPaginatedRoundTripsThroughCacheWithoutLoss(): void + { + Configs::set('managed_cache_enabled', '0'); + Photo::factory()->count(3)->owned_by($this->user)->in($this->album)->create(); + $this->actingAs($this->user); + + $fresh = $this->repository->getPhotosForAlbumPaginated($this->album->id, $this->sorting(), 10); + + Configs::set('managed_cache_enabled', '1'); + $cached_write = $this->repository->getPhotosForAlbumPaginated($this->album->id, $this->sorting(), 10); + $cached_read = $this->repository->getPhotosForAlbumPaginated($this->album->id, $this->sorting(), 10); + + self::assertEquals($fresh->currentPage(), $cached_read->currentPage()); + self::assertEquals($fresh->perPage(), $cached_read->perPage()); + self::assertEquals($fresh->total(), $cached_read->total()); + self::assertEquals( + array_map(fn (Photo $p) => $p->id, $fresh->items()), + array_map(fn (Photo $p) => $p->id, $cached_read->items()), + ); + self::assertEquals($cached_write->items()[0]->id, $cached_read->items()[0]->id); + + foreach ($cached_read->items() as $photo) { + self::assertTrue($photo->relationLoaded('size_variants')); + self::assertTrue($photo->relationLoaded('tags')); + self::assertTrue($photo->relationLoaded('statistics')); + } + } +} diff --git a/tests/Unit/Services/Cache/ManagedCacheServiceTest.php b/tests/Unit/Services/Cache/ManagedCacheServiceTest.php new file mode 100644 index 00000000000..6866bd0d95f --- /dev/null +++ b/tests/Unit/Services/Cache/ManagedCacheServiceTest.php @@ -0,0 +1,188 @@ +service = new ManagedCacheService(new ConfigManager()); + } + + public function testRememberCacheMissExecutesCallbackAndStoresValue(): void + { + $calls = 0; + $value = $this->service->remember('mc-test:key1', ['tag1'], 60, function () use (&$calls) { + $calls++; + + return 'computed-value'; + }); + + self::assertSame('computed-value', $value); + self::assertSame(1, $calls); + self::assertSame('computed-value', Cache::get('mc-test:key1')); + } + + public function testRememberCacheHitDoesNotReinvokeCallback(): void + { + $calls = 0; + $callback = function () use (&$calls) { + $calls++; + + return 'computed-value'; + }; + + $first = $this->service->remember('mc-test:key2', ['tag1'], 60, $callback); + $second = $this->service->remember('mc-test:key2', ['tag1'], 60, $callback); + + self::assertSame('computed-value', $first); + self::assertSame('computed-value', $second); + self::assertSame(1, $calls); + } + + public function testForgetTagEvictsAllMemberKeysAndTheTagItself(): void + { + $this->service->remember('mc-test:key3', ['shared-tag'], 60, fn () => 'value3'); + $this->service->remember('mc-test:key4', ['shared-tag'], 60, fn () => 'value4'); + + self::assertNotNull(Cache::get('mc-test:key3')); + self::assertNotNull(Cache::get('mc-test:key4')); + + $this->service->forgetTag('shared-tag'); + + self::assertNull(Cache::get('mc-test:key3')); + self::assertNull(Cache::get('mc-test:key4')); + self::assertNull(Cache::get(ManagedCacheService::TAG . 'shared-tag')); + } + + public function testForgetTagWithMultipleTagsOnOneKeyEvictsFromEachTag(): void + { + $this->service->remember('mc-test:key5', ['tag-a', 'tag-b'], 60, fn () => 'value5'); + + $this->service->forgetTag('tag-a'); + self::assertNull(Cache::get('mc-test:key5')); + + // Re-cache and confirm the other tag also evicts it independently. + $this->service->remember('mc-test:key5', ['tag-a', 'tag-b'], 60, fn () => 'value5-again'); + $this->service->forgetTag('tag-b'); + self::assertNull(Cache::get('mc-test:key5')); + } + + public function testForgetTagOnUnknownTagIsNoOp(): void + { + // Must not throw. + $this->service->forgetTag('never-used-tag'); + self::assertTrue(true); + } + + public function testRememberFallsBackToCallbackValueOnCacheWriteFailure(): void + { + Cache::shouldReceive('get')->once()->with('mc-test:key6')->andReturn(null); + Cache::shouldReceive('put')->once()->andThrow(new \Exception('cache store unavailable')); + + $value = $this->service->remember('mc-test:key6', ['tag1'], 60, fn () => 'fallback-value'); + + self::assertSame('fallback-value', $value); + } + + public function testAddTagsAssociatesAnExistingKeyWithAdditionalTags(): void + { + $this->service->remember('mc-test:key8', ['tag-main'], 60, fn () => 'value8'); + + $this->service->addTags('mc-test:key8', ['tag-extra-1', 'tag-extra-2']); + + $this->service->forgetTag('tag-extra-1'); + self::assertNull(Cache::get('mc-test:key8')); + + $this->service->remember('mc-test:key8', ['tag-main'], 60, fn () => 'value8-again'); + $this->service->addTags('mc-test:key8', ['tag-extra-1', 'tag-extra-2']); + $this->service->forgetTag('tag-extra-2'); + self::assertNull(Cache::get('mc-test:key8')); + } + + public function testAddTagsIsNoOpWhenKeyIsNotCached(): void + { + // Must not throw, and must not create a dangling tag pointing at a never-cached key. + $this->service->addTags('mc-test:never-cached-key', ['tag-never']); + $this->service->forgetTag('tag-never'); + self::assertTrue(true); + } + + /** + * S-052-10: an entry older than its TTL is treated as absent on the next + * remember() call and is recomputed — standard Cache::get() TTL semantics, + * no bespoke expiry logic in ManagedCacheService itself. + */ + public function testRememberRecomputesAfterTtlExpires(): void + { + $calls = 0; + $callback = function () use (&$calls) { + $calls++; + + return 'value-' . $calls; + }; + + Carbon::setTestNow(Carbon::parse('2026-01-01 00:00:00')); + $first = $this->service->remember('mc-test:ttl-key', ['tag1'], 5, $callback); + + Carbon::setTestNow(Carbon::parse('2026-01-01 00:00:10')); // 10s later, TTL was 5s. + $second = $this->service->remember('mc-test:ttl-key', ['tag1'], 5, $callback); + + Carbon::setTestNow(); + + self::assertSame('value-1', $first); + self::assertSame('value-2', $second); + self::assertSame(2, $calls); + } + + public function testRememberSkipsAllCacheIOWhenManagedCacheDisabled(): void + { + Configs::set('managed_cache_enabled', '0'); + $service = new ManagedCacheService(new ConfigManager()); + + $calls = 0; + $callback = function () use (&$calls) { + $calls++; + + return 'computed-value'; + }; + + $first = $service->remember('mc-test:key7', ['tag1'], 60, $callback); + $second = $service->remember('mc-test:key7', ['tag1'], 60, $callback); + + self::assertSame('computed-value', $first); + self::assertSame('computed-value', $second); + self::assertSame(2, $calls, 'callback should be invoked every time when the service is disabled'); + self::assertNull(Cache::get('mc-test:key7')); + } +}