Skip to content

Commit d4e089e

Browse files
dereuromarkclaude
andauthored
Fix critical bugs found in pre-1.0.0 audit (#14)
* Fix critical bugs found in pre-1.0.0 audit This commit fixes three bugs discovered during comprehensive codebase audit: **BUG-001 (CRITICAL): Fix inverted logic in DataTransformer configuration** - Location: FileStorageBehavior.php:404 - Problem: Custom DataTransformer from config was never used due to inverted condition - Fix: Changed `if (!$this->getConfig('dataTransformer') instanceof ...)` to positive check - Impact: Custom transformers can now be properly configured via behavior config **BUG-002 (MEDIUM): Remove commented-out code** - Location: FileStorageBehavior.php:216,220 - Problem: Two commented lines with no explanation - Fix: Removed commented event manager toggles, added explanatory comment - Impact: Clearer code intent, reduced maintenance confusion **BUG-003 (MEDIUM): Fix data corruption in variant getters** - Location: FileStorage.php:68,90 - Problem: array_shift() modified entity data as side effect in getter methods - Fix: Use reset() instead to read first element without array modification - Impact: Prevents progressive data corruption when getters called multiple times All tests passing (42 tests, 146 assertions) PHPStan Level 8 passing See BUG_AUDIT_REPORT.md for complete audit details 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * Delete BUG_AUDIT_REPORT.md --------- Co-authored-by: Claude <[email protected]>
1 parent 0e7f1b3 commit d4e089e

File tree

2 files changed

+16
-7
lines changed

2 files changed

+16
-7
lines changed

src/Model/Behavior/FileStorageBehavior.php

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -213,11 +213,10 @@ public function afterSave(EventInterface $event, EntityInterface $entity, ArrayO
213213
if (empty($tableConfig['className'])) {
214214
$tableConfig['className'] = 'FileStorage.FileStorage';
215215
}
216-
//$this->getEventManager()->off('Model.afterSave');
216+
// Temporarily remove behavior to prevent recursion when saving metadata
217217
$this->table()->removeBehavior('FileStorage');
218218
$this->table()->saveOrFail($entity, ['checkRules' => false]);
219219
$this->table()->addBehavior('FileStorage', $tableConfig);
220-
//$this->getEventManager()->on('Model.afterSave');
221220
} catch (Throwable $exception) {
222221
$this->table()->delete($entity);
223222

@@ -401,7 +400,9 @@ protected function getTransformer(): DataTransformerInterface
401400
return $this->transformer;
402401
}
403402

404-
if (!$this->getConfig('dataTransformer') instanceof DataTransformerInterface) {
403+
if ($this->getConfig('dataTransformer') instanceof DataTransformerInterface) {
404+
$this->transformer = $this->getConfig('dataTransformer');
405+
} else {
405406
$this->transformer = new DataTransformer(
406407
$this->table(),
407408
);

src/Model/Entity/FileStorage.php

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,11 +61,15 @@ public function getVariantUrl(string $variant): ?string
6161
return null;
6262
}
6363

64-
// Until fix is fully applied
6564
if (!is_string($variants[$variant]['url'])) {
6665
Log::write('error', 'Invalid variants url data for ' . $this->id);
6766

68-
return array_shift($variants[$variant]['url']);
67+
// Return first element without modifying the array
68+
if (is_array($variants[$variant]['url']) && count($variants[$variant]['url']) > 0) {
69+
return (string)reset($variants[$variant]['url']);
70+
}
71+
72+
return null;
6973
}
7074

7175
return $variants[$variant]['url'];
@@ -83,11 +87,15 @@ public function getVariantPath(string $variant): ?string
8387
return null;
8488
}
8589

86-
// Until fix is fully applied
8790
if (!is_string($variants[$variant]['path'])) {
8891
Log::write('error', 'Invalid variants path data for ' . $this->id);
8992

90-
return array_shift($variants[$variant]['path']);
93+
// Return first element without modifying the array
94+
if (is_array($variants[$variant]['path']) && count($variants[$variant]['path']) > 0) {
95+
return (string)reset($variants[$variant]['path']);
96+
}
97+
98+
return null;
9199
}
92100

93101
return $variants[$variant]['path'];

0 commit comments

Comments
 (0)