Skip to content

Fix progress being overwritten to 0 when reinstantiating Progressable #20

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

Merged
insign merged 1 commit into from
May 8, 2026
Merged

Fix progress being overwritten to 0 when reinstantiating Progressable #20

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 19 additions & 5 deletions src/Progressable.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,9 +204,21 @@ public function setOverallUniqueName(string $overallUniqueName): static {
* @throws UniqueNameNotSetException
*/
public function makeSureLocalIsPartOfTheCalc(): void {
if ($this->getLocalProgress(0) == 0) {
// This make sure that the class who called this method will be part of the overall progress calculation
$this->resetLocalProgress();
if ($this->progress == 0) {
$progressData = $this->getOverallProgressData();
$localKey = $this->getLocalKey();

if (isset($progressData[$localKey])) {
$localData = $progressData[$localKey];
$this->progress = $localData['progress'] ?? 0;
$this->startTime = $localData['start_time'] ?? null;
$this->statusMessage = $localData['message'] ?? null;
$this->totalSteps = $localData['total_steps'] ?? null;
$this->currentStep = $localData['current_step'] ?? null;
$this->metadata = $localData['metadata'] ?? [];
} else {
$this->resetLocalProgress();
}
}
}

Expand Down Expand Up @@ -442,8 +454,10 @@ public function setLocalKey(string $name): static {
$overallProgressData = $this->getOverallProgressData();

if (isset($overallProgressData[$currentKey])) {
// Rename the local key preserving the data
$overallProgressData[$name] = $overallProgressData[$currentKey];
// Rename the local key preserving the data, but do not overwrite existing target data
if (!isset($overallProgressData[$name])) {
$overallProgressData[$name] = $overallProgressData[$currentKey];
}
unset($overallProgressData[$currentKey]);
Comment on lines +458 to 461
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot May 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Sync local state when key rename hits existing target

When setLocalKey() is called and both the current key and the target key already exist, this branch skips copying data to the target but still deletes the current key. In that case, the instance keeps its old in-memory fields ($progress, metadata, status, etc.) while storage keeps the target key's data, so the object and cache diverge. A later setLocalProgress() call from this instance can then overwrite the target entry with stale values, reintroducing data loss in multi-worker or key-collision scenarios.

Useful? React with 👍 / 👎.

$this->saveOverallProgressData($overallProgressData);
}
Expand Down
28 changes: 28 additions & 0 deletions tests/MakeSureLocalOverwriteBugTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<?php
namespace Verseles\Progressable\Tests;

use Orchestra\Testbench\TestCase;
use Verseles\Progressable\Progressable;

class MakeSureLocalOverwriteBugTest extends TestCase {
public function test_make_sure_local_does_not_overwrite_existing_progress() {
$task1 = new class { use Progressable; };
$task1->setOverallUniqueName('my-group');
$task1->setLocalKey('task-1');
$task1->setLocalProgress(50);

// Assert state correctly saved
$data = $task1->getOverallProgressData();
$this->assertEquals(50, $data['task-1']['progress']);

// Now imagine this is another PHP process/request but handling the same logical task
$task2 = new class { use Progressable; };
$task2->setOverallUniqueName('my-group');
$task2->setLocalKey('task-1');

$data = $task2->getOverallProgressData();

// Progress should still be 50, not reset to 0!
$this->assertEquals(50, $data['task-1']['progress']);
}
}
Loading