Skip to content
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.

Sign up for GitHub

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

Jump to bottom

The Statamic::isWorker() check may need to also apply to console commands #10514

Open
simonworkhouse opened this issue Jul 26, 2024 · 0 comments · May be fixed by #10574
Open

The Statamic::isWorker() check may need to also apply to console commands #10514

simonworkhouse opened this issue Jul 26, 2024 · 0 comments · May be fixed by #10574
Labels
stache

Comments

@simonworkhouse
Copy link
Contributor

Bug description

The entry stache is prone to getting out of sync when console commands are running at the same time as someone is modifying entries in the admin panel.

We have experienced a number of different issues from this, such as:

  • Collection tree with duplicate IDs causing 500s on front-end.
  • Empty/junk entries appear after they have been deleted in the admin panel.
  • Others I cannot think of right now... but nothing fun.

Something that does address this issue is updating Statamic::isWorker() to simply return App::runningInConsole(), although I am not currently sure of the performance implications of doing so.

How to reproduce

See the example test entry_stache_maintains_sync_with_cli_commands here:

cms/tests/Data/Entries/EntryTest.php

Lines 2578 to 2639 in 621180a

#[Test]
public function entry_stache_maintains_sync_with_cli_commands()
{
// Using the filesystem cache may make it easier for us to simulate this issue.
config(['cache.default' => 'file']);
Cache::clear();
// Register an Artisan command that checks the number of entries found and
// asserts that they all have a valid title.
$testInstance = $this;
Artisan::command('testing:entry-stache-test-one {--expected=}', function ($expected) use ($testInstance) {
// Fetch entries and make sure that they all have titles.
$entriesBefore = Collection::find('test')->queryEntries()->get();
$testInstance->assertEquals($expected, $entriesBefore->count());
// NOTE: Must confirm that all entries have valid titles as empty entries
// are one other symptom of out-of-sync stache.
$testInstance->assertEmpty($entriesBefore->pluck('title')->reject(fn ($title) => (bool) $title));
});
// Create a collection.
$collection = tap(Collection::make('test'))->save();
// Create some entries.
EntryFactory::id('alfa-id')->collection('test')->slug('alfa')->data(['title' => 'Alfa'])->create();
EntryFactory::id('bravo-id')->collection('test')->slug('bravo')->data(['title' => 'Bravo'])->create();
EntryFactory::id('charlie-id')->collection('test')->slug('charlie')->data(['title' => 'Charlie'])->create();
EntryFactory::id('donkus-id')->collection('test')->slug('donkus')->data(['title' => 'Donkus'])->create();
EntryFactory::id('eggbert-id')->collection('test')->slug('eggbert')->data(['title' => 'Eggbert'])->create();
// NOTE: In order to simulate the stache behaviour when a long-running CLI command is being
// executed we do the following:
// 1. Run a CLI command that checks the number of entries.
// 2. Save the serialised state of the `EntryQueryBuilder`.
// - The `EntryQueryBuilder` contains the stache store instance.
// - This represents what will be in memory for the running CLI command.
// 3. Delete some entries outside of the CLI command.
// - This represents someone deleting entries via the admin panel.
// 4. Restore the `EntryQueryBuilder` instance from the serialised data.
// 5. Run the CLI command again and confirm that there are now less entries.
// 1. Run a CLI command that checks the number of entries.
$this->artisan('testing:entry-stache-test-one --expected=5');
// 2. Save the serialised state of the `EntryQueryBuilder`.
$serialisedQueryBuilder = serialize(app(EntryQueryBuilder::class));
// 3. Delete some entries outside of the CLI command.
Collection::find('test')->queryEntries()->whereIn('slug', [
'donkus',
'eggbert',
])->get()->each->delete();
// 4. Restore the `EntryQueryBuilder` instance from the serialised data.
$this->app->bind(EntryQueryBuilder::class, function () use ($serialisedQueryBuilder) {
return unserialize($serialisedQueryBuilder);
});
// 5. Run the CLI command again and confirm that there are now less entries.
// NOTE: Uncomment the following line so that `Statamic::isWorker()` returns `true`
// and this test will then pass.
// \Illuminate\Support\Facades\Request::swap(new \Tests\Fakes\FakeArtisanRequest('queue:work'));
$this->artisan('testing:entry-stache-test-one --expected=3');

When uncommenting the following line, you'll see that the test functions correctly.

\Illuminate\Support\Facades\Request::swap(new \Tests\Fakes\FakeArtisanRequest('queue:work'));

Logs

No response

Environment

Environment
Application Name: Testing Things
Laravel Version: 10.48.17
PHP Version: 8.2.18
Composer Version: 2.7.1
Environment: local
Debug Mode: ENABLED
URL: testing-things.localhost
Maintenance Mode: OFF

Installation

Fresh statamic/statamic site via CLI

Additional details

No response
@simonworkhouse simonworkhouse changed the title The Statamic::isWorker() check needs to also apply to console commands The Statamic::isWorker() check may need to also apply to console commands Jul 29, 2024
@kingsven kingsven linked a pull request Aug 6, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
stache
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[5.x] Fix stache consistency issues with console commands kingsven/statamic-cms
2 participants
@duncanmcclean @simonworkhouse