-
-
Notifications
You must be signed in to change notification settings - Fork 371
Disable response caching functionality #4362
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
c549c74
feat(040): add spec, plan, and tasks for disable-request-caching feature
Copilot e7381d4
docs(040): document General.vue v-if guard and InitConfig analysis in…
Copilot 0a95e64
feat(040): disable request caching by default; hide Mod Cache setting…
Copilot 9b140b4
Merge branch 'master' into copilot/disable-redis-caching-functionality
ildyria File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
37 changes: 37 additions & 0 deletions
37
database/migrations/2026_05_18_000001_disable_request_caching.php
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| <?php | ||
|
|
||
| /** | ||
| * SPDX-License-Identifier: MIT | ||
| * Copyright (c) 2017-2018 Tobias Reich | ||
| * Copyright (c) 2018-2026 LycheeOrg. | ||
| */ | ||
|
|
||
| use Illuminate\Database\Migrations\Migration; | ||
| use Illuminate\Support\Facades\DB; | ||
|
|
||
| /** | ||
| * Force cache_enabled to 0. | ||
| * | ||
| * Request caching is now disabled by default and hidden from the settings UI | ||
| * unless ENABLE_REQUEST_CACHING=true is set in the environment. Any existing | ||
| * installation that had the toggle on must have it silently turned off so that | ||
| * the middleware no longer serves cached responses without the operator | ||
| * explicitly opting back in. | ||
| * | ||
| * The down() method is intentionally a no-op: once caching is disabled we do | ||
| * not restore the previous value on rollback, because we have no record of | ||
| * what that value was. | ||
| */ | ||
| return new class() extends Migration { | ||
| public function up(): void | ||
| { | ||
| DB::table('configs') | ||
| ->where('key', 'cache_enabled') | ||
| ->update(['value' => '0']); | ||
| } | ||
|
|
||
| public function down(): void | ||
| { | ||
| // Intentional no-op: previous value is not known. | ||
| } | ||
| }; | ||
170 changes: 170 additions & 0 deletions
170
docs/specs/4-architecture/features/040-disable-request-caching/plan.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,170 @@ | ||
| # Feature Plan 040 – Disable Request Caching | ||
|
|
||
| _Linked specification:_ `docs/specs/4-architecture/features/040-disable-request-caching/spec.md` | ||
| _Status:_ Draft | ||
| _Last updated:_ 2026-05-18 | ||
|
|
||
| > Guardrail: Keep this plan traceable back to the governing spec. Reference FR/NFR/Scenario IDs from `spec.md` where relevant, log any new high- or medium-impact questions in [docs/specs/4-architecture/open-questions.md](docs/specs/4-architecture/open-questions.md), and assume clarifications are resolved only when the spec's normative sections (requirements/NFR/behaviour/telemetry) and, where applicable, ADRs under `docs/specs/5-decisions/` have been updated. | ||
|
|
||
| ## Vision & Success Criteria | ||
|
|
||
| Redis-backed request caching is disabled by default on all installations. Operators who wish to use it must explicitly opt in by setting `ENABLE_REQUEST_CACHING=true` in `.env`, which makes the caching settings visible in the admin UI. Success is measured by: | ||
|
|
||
| - All existing installations have `cache_enabled = 0` after migration. | ||
| - `GET /api/v2/Settings` omits the `Mod Cache` category when `ENABLE_REQUEST_CACHING` is not set. | ||
| - PHPStan 0 errors, php-cs-fixer clean, all tests pass. | ||
|
|
||
| ## Scope Alignment | ||
|
|
||
| - **In scope:** | ||
| - Database migration to force `cache_enabled = '0'` (FR-040-01). | ||
| - New `enable-request-caching` key in `config/features.php` tied to `ENABLE_REQUEST_CACHING` env variable (FR-040-02). | ||
| - `SettingsController::getAll` filter to hide `Mod Cache` configs when flag is off (FR-040-03). | ||
| - `.env.example` update (FR-040-04). | ||
| - Feature test covering S-040-03 and S-040-05. | ||
| - Quality gates (NFR-040-01 to NFR-040-04). | ||
| - Confirmation that `General.vue` and `InitConfig` require **no changes** (S-040-07 / S-040-08 — handled automatically by the API-level filter). | ||
|
|
||
| - **Out of scope:** | ||
| - Removing or refactoring the caching middleware. | ||
| - Runtime UI toggle for enabling caching. | ||
| - Redis configuration changes. | ||
| - Changes to `InitConfig`, `LycheeStateStore`, or `General.vue` — not required because the existing `v-if="cache_enabled !== undefined"` guard in `General.vue` already hides the toggle when the config is absent from the API response. | ||
|
|
||
| ## Dependencies & Interfaces | ||
|
|
||
| - `database/migrations/` — migration naming convention (`YYYY_MM_DD_HHMMSS_<name>.php`). | ||
| - `App\Models\Extensions\BaseConfigMigration` — not used here; this migration uses a plain `Migration` with a direct `DB::table` update. | ||
| - `config/features.php` — existing feature-flag file; pattern already in use for `hide-lychee-SE`, `webshop`, `webhook`, etc. | ||
| - `App\Http\Controllers\Admin\SettingsController::getAll` — query builder chain that filters configs; pattern already used for `hide-lychee-SE` and `not_on_docker`. | ||
| - `tests/Feature_v2/` — existing PHPUnit feature test suite using `BaseApiWithDataTest`. | ||
|
|
||
| ## Assumptions & Risks | ||
|
|
||
| - **Assumptions:** | ||
| - The `configs` table and the `cache_enabled` key are always present when the new migration runs (introduced in `2024_12_28_190150_caching_config.php`). | ||
| - The `Mod Cache` category string is stable and used only for caching-related configs. | ||
| - **Risks / Mitigations:** | ||
| - If `cache_enabled` key is missing (e.g., very old or incomplete installation), the migration's `DB::table` update will silently affect 0 rows — which is safe. | ||
| - Tests that currently rely on `cache_enabled = 1` being the stored default could fail; check existing test suite for such assumptions before committing. | ||
|
|
||
| ## Implementation Drift Gate | ||
|
|
||
| After each increment, run: | ||
|
|
||
| ```bash | ||
| vendor/bin/php-cs-fixer fix | ||
| php artisan test | ||
| make phpstan | ||
| ``` | ||
|
|
||
| Record results in this plan's Scenario Tracking table. | ||
|
|
||
| ## Increment Map | ||
|
|
||
| ### I1 – Database Migration (≤30 min) | ||
|
|
||
| - _Goal:_ Force `cache_enabled = '0'` for all rows in `configs` (FR-040-01, S-040-01, S-040-02). | ||
| - _Preconditions:_ None. | ||
| - _Steps:_ | ||
| 1. Write the test first: assert `configs` row `cache_enabled` equals `'0'` after the test suite runs (covered implicitly by the full migration run in tests). | ||
| 2. Create `database/migrations/2026_05_18_000001_disable_request_caching.php` extending `Migration`. | ||
| 3. `up()`: `DB::table('configs')->where('key', 'cache_enabled')->update(['value' => '0']);` | ||
| 4. `down()`: no-op (value is not restored — migration is one-directional per FR-040-01 failure path). | ||
| 5. Verify test suite passes, PHPStan clean. | ||
| - _Commands:_ `php artisan test`, `make phpstan`, `vendor/bin/php-cs-fixer fix` | ||
| - _Exit:_ Migration file present and parseable; `php artisan migrate` completes without error; test suite passes. | ||
|
|
||
| ### I2 – Feature Flag in `config/features.php` (≤20 min) | ||
|
|
||
| - _Goal:_ Expose `enable-request-caching` feature flag (FR-040-02, S-040-03, S-040-04, S-040-05). | ||
| - _Preconditions:_ I1 complete. | ||
| - _Steps:_ | ||
| 1. Add a new entry in `config/features.php`: | ||
| ```php | ||
| 'enable-request-caching' => (bool) env('ENABLE_REQUEST_CACHING', false), | ||
| ``` | ||
| 2. Add `ENABLE_REQUEST_CACHING=false` with a descriptive comment to `.env.example` (FR-040-04). | ||
| 3. Run `php artisan config:clear` locally and verify `config('features.enable-request-caching')` defaults to `false`. | ||
| - _Commands:_ `vendor/bin/php-cs-fixer fix`, `make phpstan` | ||
| - _Exit:_ `config/features.php` updated; `.env.example` updated; PHPStan 0 errors. | ||
|
|
||
| ### I3 – Settings Controller Filter (≤30 min) | ||
|
|
||
| - _Goal:_ Hide `Mod Cache` configs from admin settings when feature flag is off (FR-040-03, S-040-03, S-040-04, S-040-05, S-040-07, S-040-08). | ||
| - _Preconditions:_ I2 complete. | ||
| - _Steps:_ | ||
| 1. In `SettingsController::getAll`, add the filter to the `configs` query chain (after the existing `not_on_docker` filter): | ||
| ```php | ||
| ->when(config('features.enable-request-caching') === false, fn ($q) => $q->where('cat', '!=', 'Mod Cache')) | ||
| ``` | ||
| 2. **No changes to `General.vue` or `InitConfig` are required.** `General.vue` already guards the cache toggle with `v-if="cache_enabled !== undefined"`. The `load()` function populates `cache_enabled` via `configurations.find(config => config.key === 'cache_enabled')`. When the API omits the `Mod Cache` category, `cache_enabled.value` is `undefined` and the toggle is automatically hidden. Adding an explicit `is_request_caching_enabled` property to `InitConfig` and `LycheeStateStore` would be redundant. | ||
| 3. Run full test suite to verify no regressions. | ||
| - _Commands:_ `vendor/bin/php-cs-fixer fix`, `php artisan test`, `make phpstan` | ||
| - _Exit:_ Controller updated; tests pass; `General.vue` toggle hidden (S-040-07) confirmed indirectly by REST test. | ||
|
|
||
| ### I4 – Feature Tests (≤40 min) | ||
|
|
||
| - _Goal:_ Provide explicit test coverage for S-040-03 and S-040-05. | ||
| - _Preconditions:_ I3 complete. | ||
| - _Steps:_ | ||
| 1. Locate or create a `Feature_v2` test class for the Settings endpoint (extending `BaseApiWithDataTest`). | ||
| 2. Add test method for S-040-03: with default config (flag off), assert `GET /api/v2/Settings` response does not contain a category named `Mod Cache`. | ||
| 3. Add test method for S-040-05: with `config(['features.enable-request-caching' => true])`, assert `GET /api/v2/Settings` response contains a category named `Mod Cache` with the expected config keys. | ||
| 4. Run test suite, confirm new tests are green. | ||
| - _Commands:_ `php artisan test --filter=Settings`, `make phpstan`, `vendor/bin/php-cs-fixer fix` | ||
| - _Exit:_ New tests pass; no other tests broken. | ||
|
|
||
| ### I5 – Quality Gates & Documentation (≤20 min) | ||
|
|
||
| - _Goal:_ Ensure full pipeline passes and roadmap/session docs are updated. | ||
| - _Preconditions:_ I4 complete. | ||
| - _Steps:_ | ||
| 1. Run complete quality gate: `vendor/bin/php-cs-fixer fix`, `php artisan test`, `make phpstan`. | ||
| 2. Update `roadmap.md`: move Feature 040 from Active to Completed. | ||
| 3. Update `_current-session.md` with session summary for Feature 040. | ||
| - _Commands:_ `vendor/bin/php-cs-fixer fix`, `php artisan test`, `make phpstan` | ||
| - _Exit:_ All gates green; docs updated. | ||
|
|
||
| ## Scenario Tracking | ||
|
|
||
| | Scenario ID | Increment / Task reference | Notes | | ||
| |-------------|---------------------------|-------| | ||
| | S-040-01 | I1 / T-040-01 | Covered by migration `up()` and test suite re-run. | | ||
| | S-040-02 | I1 / T-040-01 | Idempotent `UPDATE` with no-op when already `'0'`. | | ||
| | S-040-03 | I3, I4 / T-040-04, T-040-05 | Controller filter; explicit feature test. | | ||
| | S-040-04 | I3, I4 / T-040-04, T-040-05 | Same filter path as S-040-03. | | ||
| | S-040-05 | I3, I4 / T-040-04, T-040-06 | Controller filter absent when flag is `true`; explicit feature test. | | ||
| | S-040-06 | I1 / T-040-01 | `down()` is a no-op; migrate:rollback safe. | | ||
| | S-040-07 | I3 / T-040-04 | `General.vue` hides toggle automatically — `v-if="cache_enabled !== undefined"` is `false` when config absent from API response. No changes to `General.vue` or `InitConfig` required. | | ||
| | S-040-08 | I3 / T-040-04 | `General.vue` shows toggle when config present in API response. | | ||
|
|
||
| ## Analysis Gate | ||
|
|
||
| _To be completed before coding begins._ | ||
|
|
||
| - [ ] All four FRs are unambiguous and traceable to tasks. | ||
| - [ ] All six scenarios map to at least one increment/task. | ||
|
ildyria marked this conversation as resolved.
|
||
| - [ ] No open questions logged in `open-questions.md` for Feature 040. | ||
| - [ ] Estimated total effort ≤ 140 min (fits within session). | ||
|
|
||
| ## Exit Criteria | ||
|
|
||
| - [ ] Migration `2026_05_18_000001_disable_request_caching.php` created and applied. | ||
| - [ ] `config/features.php` contains `enable-request-caching` key. | ||
| - [ ] `.env.example` documents `ENABLE_REQUEST_CACHING=false`. | ||
| - [ ] `SettingsController::getAll` filters `Mod Cache` configs when flag is off. | ||
| - [ ] Feature tests for S-040-03 and S-040-05 pass. | ||
| - [ ] `vendor/bin/php-cs-fixer fix` exits 0. | ||
| - [ ] `php artisan test` exits 0. | ||
| - [ ] `make phpstan` exits 0. | ||
| - [ ] `roadmap.md` updated. | ||
|
|
||
| ## Follow-ups / Backlog | ||
|
|
||
| - Consider adding a diagnostic warning when `ENABLE_REQUEST_CACHING=true` but Redis is not configured as the cache driver. | ||
| - If the caching feature is later removed entirely, the `Mod Cache` config category and all three config rows can be dropped via a follow-up migration. | ||
|
|
||
| --- | ||
|
|
||
| *Last updated: 2026-05-18* | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.