-
Notifications
You must be signed in to change notification settings - Fork 14
fix: checking the state of a database with a binary field. #240
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
base: master
Are you sure you want to change the base?
Changes from all commits
c95a1ab
03606be
b68918a
4b349e7
ba961a8
bd8ca19
32f7886
f5ade29
1b6250a
9848df9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -35,7 +35,7 @@ public function testInitialization() | |||||||
| $jsonFields = $this->getProtectedProperty($reflectionClass, 'jsonFields', $modelTestState); | ||||||||
| $state = $this->getProtectedProperty($reflectionClass, 'state', $modelTestState); | ||||||||
|
|
||||||||
| $this->assertEquals(['json_field', 'castable_field'], $jsonFields); | ||||||||
| $this->assertEquals(['json_field', 'castable_field', 'binary_field'], $jsonFields); | ||||||||
|
||||||||
| $this->assertEquals(['json_field', 'castable_field', 'binary_field'], $jsonFields); | |
| $this->assertEquals(['json_field', 'castable_field'], $jsonFields); |
AZabolotnikov marked this conversation as resolved.
Show resolved
Hide resolved
AZabolotnikov marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| $changedDatasetMock = collect([[ | |
| $changedDatasetMock = collect([[ |
AZabolotnikov marked this conversation as resolved.
Show resolved
Hide resolved
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test cases only cover updating records with binary fields (null to binary and binary to null), but don't test creating new records with binary fields or deleting records that contain binary fields. Given that the binary conversion logic in TableTestState.php line 67 only applies to changed fields in updated records, there should be test coverage for created and deleted records with binary data to ensure they are handled correctly.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| { | ||
| "updated": [ | ||
| { | ||
| "id": 1, | ||
| "binary_field": null | ||
| } | ||
| ], | ||
| "created": [], | ||
| "deleted": [] | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| { | ||
| "updated": [ | ||
| { | ||
| "id": 1, | ||
| "binary_field": "31ee76261d87fed8cb9d4c465c48158c" | ||
| } | ||
| ], | ||
| "created": [], | ||
| "deleted": [] | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| <?php | ||
|
|
||
| namespace RonasIT\Support\Tests\Support\Mock\Casts; | ||
|
|
||
| use Illuminate\Contracts\Database\Eloquent\CastsAttributes; | ||
|
|
||
| class BinaryCast implements CastsAttributes | ||
| { | ||
| public function get($model, $key, $value, $attributes) | ||
| { | ||
| return is_null($value) ? $value : bin2hex($value); | ||
| } | ||
|
|
||
| public function set($model, $key, $value, $attributes) | ||
| { | ||
| return is_null($value) ? $value : md5($value, true); | ||
| } | ||
| } | ||
AZabolotnikov marked this conversation as resolved.
Show resolved
Hide resolved
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The binary field conversion using
bin2hexis only applied to changed fields in updated records. However, created and deleted records (lines 78-79) may contain full records with binary fields that have not been converted yet. This could cause issues when comparing created/deleted records against fixtures if they contain binary data.Consider extracting the binary conversion logic into a separate method and applying it consistently to all record types (created, updated, deleted) in the
prepareChangesmethod or before passing records toprepareChanges.