Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 3 additions & 1 deletion src/Testing/TableTestState.php
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ protected function getChanges(): array
$changes = array_diff_assoc($updatedItem, $originItem);

if (!empty($changes)) {
$changes = Arr::map($changes, fn ($field) => (!mb_check_encoding($field, 'UTF-8')) ? bin2hex($field) : $field);
Copy link

Copilot AI Jan 28, 2026

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 bin2hex is 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 prepareChanges method or before passing records to prepareChanges.

Copilot uses AI. Check for mistakes.

$updatedRecords[] = array_merge(['id' => $originItem['id']], $changes);
}

Expand All @@ -88,7 +90,7 @@ protected function prepareChanges(array $changes): array

return array_map(function ($item) use ($jsonFields) {
foreach ($jsonFields as $jsonField) {
if (Arr::has($item, $jsonField)) {
if (Arr::has($item, $jsonField) && is_string($item[$jsonField]) && json_validate($item[$jsonField])) {
$item[$jsonField] = json_decode($item[$jsonField], true);
}
}
Expand Down
4 changes: 2 additions & 2 deletions tests/BaseRequestTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ public function testGetOrderableFields()
{
$result = $this->callEncapsulatedMethod(new BaseRequest(), 'getOrderableFields', TestModel::class);

$expectedResult = 'id,name,json_field,castable_field,*,created_at,updated_at';
$expectedResult = 'id,name,json_field,castable_field,binary_field,*,created_at,updated_at';

$this->assertEquals($expectedResult, $result);
}
Expand All @@ -28,7 +28,7 @@ public function testGetOrderableFieldsWithAdditionalFields()

$result = $this->callEncapsulatedMethod(new BaseRequest(), 'getOrderableFields', ...$args);

$expectedResult = 'id,name,json_field,castable_field,*,created_at,updated_at,additional_field_1,additional_field_2';
$expectedResult = 'id,name,json_field,castable_field,binary_field,*,created_at,updated_at,additional_field_1,additional_field_2';

$this->assertEquals($expectedResult, $result);
}
Expand Down
38 changes: 37 additions & 1 deletion tests/ModelTestStateTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

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

The current implementation treats ALL fields with custom CastsAttributes implementations as JSON fields (including BinaryCast). This is problematic because not all custom casts represent JSON data. The fix in line 93 of TableTestState.php works around this by adding json_validate() checks, but this is a band-aid solution.

A more robust approach would be to:

  1. Introduce a marker interface (e.g., JsonCast) that JSON-based custom casts can implement
  2. Update isJsonCast() to only return true for casts that implement this interface or are the 'array' type
  3. This would prevent binary fields from being incorrectly treated as JSON fields in the first place

The current workaround works but may cause confusion and could lead to similar issues with other non-JSON custom casts in the future.

Suggested change
$this->assertEquals(['json_field', 'castable_field', 'binary_field'], $jsonFields);
$this->assertEquals(['json_field', 'castable_field'], $jsonFields);

Copilot uses AI. Check for mistakes.
$this->assertEquals($originRecords, $state);
}

Expand Down Expand Up @@ -88,6 +88,42 @@ public function testAssertChangesWithoutJsonFields()
$modelTestState->assertChangesEqualsFixture('assertion_fixture_without_json_fields.json');
}

public function testAssertChangesBinaryString()
{
$initialDatasetMock = collect([[
'id' => 1,
'binary_field' => null,
]]);

$changedDatasetMock = collect([[
'id' => 1,
'binary_field' => md5('some_string', true),
]]);

$this->mockGettingDatasetForChanges($changedDatasetMock, $initialDatasetMock, 'test_models');

$modelTestState = new ModelTestState(TestModel::class);
$modelTestState->assertChangesEqualsFixture('null_to_binary_string_changes');
}

public function testAssertChangesBinaryToNull()
{
$initialDatasetMock = collect([[
'id' => 1,
'binary_field' => md5('some_string', true),
]]);

$changedDatasetMock = collect([[
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
$changedDatasetMock = collect([[
$changedDatasetMock = collect([[

'id' => 1,
'binary_field' => null,
]]);

$this->mockGettingDatasetForChanges($changedDatasetMock, $initialDatasetMock, 'test_models');

$modelTestState = new ModelTestState(TestModel::class);
$modelTestState->assertChangesEqualsFixture('binary_string_to_null_changes');
}
Comment on lines +91 to +125
Copy link

Copilot AI Jan 28, 2026

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.

Copilot uses AI. Check for mistakes.

public function testAssertNoChanges()
{
$datasetMock = collect($this->getJsonFixture('get_without_changes/dataset.json'));
Expand Down
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": []
}
18 changes: 18 additions & 0 deletions tests/support/Mock/Casts/BinaryCast.php
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);
}
}
3 changes: 3 additions & 0 deletions tests/support/Mock/Models/TestModel.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use Illuminate\Database\Eloquent\Model;
use Illuminate\Database\Eloquent\Relations\HasMany;
use Illuminate\Database\Eloquent\SoftDeletes;
use RonasIT\Support\Tests\Support\Mock\Casts\BinaryCast;
use RonasIT\Support\Tests\Support\Mock\Casts\JSONCustomCast;
use RonasIT\Support\Traits\ModelTrait;

Expand All @@ -17,11 +18,13 @@ class TestModel extends Model
'name',
'json_field',
'castable_field',
'binary_field',
];

protected $casts = [
'json_field' => 'array',
'castable_field' => JSONCustomCast::class,
'binary_field' => BinaryCast::class,
];

public function relation(): HasMany
Expand Down