Skip to content

Commit e6113e8

Browse files
dereuromarkclaude
andcommitted
Improve error handling, validation and code quality
Applied fixes for issues #2, #3, #4 and medium priority improvements: **Issue #2: Add preg_match() validation in Short converter** - src/Converter/Short.php: Check preg_match return value before using matches - Throw descriptive RuntimeException if hex format is invalid **Issue #3: Add descriptive exception messages to Reverser classes** - src/Reverser/ReverseBinary.php: Add error message with actual byte length - src/Reverser/ReverseHex.php: Add error message showing expected vs actual format - src/Reverser/ReverseShortened.php: Add error message for full-length UUID rejection **Issue #4: Fix misleading PHPDoc in ExposeBehavior** - src/Model/Behavior/ExposeBehavior.php: Remove incorrect @throws tag about 'slug' key - The method signature already shows it takes string $uuid directly **Medium #1: Add file validation for migration command** - src/Command/AddExposedFieldCommand.php: Validate migration path exists and is writable - Add error handling for file_put_contents() failure **Medium #3: Improve error message in ExposeBehavior** - src/Model/Behavior/ExposeBehavior.php: Show actual column type in error message - Helps debugging when column type cannot be determined **Medium #6: Document SuperimposeComponent config** - src/Controller/Component/SuperimposeComponent.php: Add inline comment explaining actions array All changes improve error handling, make debugging easier, and prevent potential bugs from unchecked return values. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent ddf649c commit e6113e8

File tree

7 files changed

+21
-8
lines changed

7 files changed

+21
-8
lines changed

src/Command/AddExposedFieldCommand.php

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,16 @@ public function execute(Arguments $args, ConsoleIo $io): ?int {
101101
if ($this->migrationExists($migrationName, $this->migrationPath)) {
102102
$io->abort('File already exists: ' . $migrationFilePath);
103103
}
104-
file_put_contents($migrationFilePath, $migrationContent);
104+
if (!is_dir($this->migrationPath)) {
105+
$io->abort('Migration path does not exist: ' . $this->migrationPath);
106+
}
107+
if (!is_writable($this->migrationPath)) {
108+
$io->abort('Migration path is not writable: ' . $this->migrationPath);
109+
}
110+
$bytesWritten = @file_put_contents($migrationFilePath, $migrationContent);
111+
if ($bytesWritten === false) {
112+
$io->abort('Failed to write migration file: ' . $migrationFilePath);
113+
}
105114
$io->success('Migration file created. Now you can migrate your table(s) using `bin/cake migrations migrate`.');
106115
} else {
107116
$io->out('--- ' . $migrationName . '.php' . ' ---');

src/Controller/Component/SuperimposeComponent.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ class SuperimposeComponent extends Component {
1616
* @var array<string, mixed>
1717
*/
1818
protected array $_defaultConfig = [
19-
'actions' => [],
19+
'actions' => [], // Array of action names (strings) to apply superimposition to
2020
];
2121

2222
/**

src/Converter/Short.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,10 @@ protected function stringToNum(string $string): BigInteger {
109109
*/
110110
protected function formatHex(string $hex): string {
111111
$hex = str_pad($hex, 32, '0');
112-
preg_match('/([a-f0-9]{8})([a-f0-9]{4})([a-f0-9]{4})([a-f0-9]{4})([a-f0-9]{12})/', $hex, $matches);
112+
$matched = preg_match('/([a-f0-9]{8})([a-f0-9]{4})([a-f0-9]{4})([a-f0-9]{4})([a-f0-9]{12})/', $hex, $matches);
113+
if ($matched === 0) {
114+
throw new RuntimeException('Invalid hex string format: ' . $hex);
115+
}
113116
array_shift($matches);
114117

115118
return implode('-', $matches);

src/Model/Behavior/ExposeBehavior.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,6 @@ public function getExposedKey(bool $prefixed = false): string {
9393
*
9494
* @param \Cake\ORM\Query\SelectQuery $query
9595
* @param string $uuid
96-
* @throws \InvalidArgumentException If the 'slug' key is missing in options
9796
* @return \Cake\ORM\Query\SelectQuery
9897
*/
9998
public function findExposed(SelectQuery $query, string $uuid): SelectQuery {
@@ -205,7 +204,9 @@ public function initExposedField(array $params = []): int {
205204
protected function generateExposedField(string $field): string {
206205
$fieldType = $this->_table->getSchema()->getColumnType($field);
207206
if (!$fieldType) {
208-
throw new RuntimeException('Cannot determine column type of field `' . $field . '`');
207+
$schema = $this->_table->getSchema()->getColumn($field);
208+
209+
throw new RuntimeException('Cannot determine column type of field `' . $field . '`. Found type: ' . ($schema['type'] ?? 'unknown'));
209210
}
210211

211212
$type = TypeFactory::build($fieldType);

src/Reverser/ReverseBinary.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ class ReverseBinary implements ReverseStrategyInterface {
1313
*/
1414
public function reverse(string $uuid): string {
1515
if (strlen($uuid) !== 16) {
16-
throw new RuntimeException();
16+
throw new RuntimeException('Expected 16-byte binary UUID, got ' . strlen($uuid) . ' bytes');
1717
}
1818

1919
return (string)(new BinaryUuidType())->toPHP($uuid, new Mysql());

src/Reverser/ReverseHex.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ public function reverse(string $uuid): string {
1818
return (string)(new BinaryUuidType())->toPHP($binaryUuid, new Mysql());
1919
}
2020

21-
throw new RuntimeException();
21+
throw new RuntimeException('Expected hex format starting with "0x" and length 34, got: ' . $uuid);
2222
}
2323

2424
}

src/Reverser/ReverseShortened.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ class ReverseShortened implements ReverseStrategyInterface {
1212
*/
1313
public function reverse(string $uuid): string {
1414
if (strlen($uuid) === 36) {
15-
throw new RuntimeException();
15+
throw new RuntimeException('Cannot reverse full-length UUID (36 chars), expected shortened format');
1616
}
1717

1818
return ConverterFactory::getConverter()->decode($uuid);

0 commit comments

Comments
 (0)