Skip to content

Commit 847187c

Browse files
author
Roelof Roos
authored
Merge pull request #73 from advoor/fix-data-serialisation
Fix double-encoding issues
2 parents 2be40c7 + b6bc57e commit 847187c

10 files changed

+220
-14
lines changed

‎CHANGELOG.md

+3
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [UNRELEASED]
99

10+
### Fixed
11+
- NovaEditorJsCast now properly handles JSON, not double-encoding stuff and decoding double-encoded properties.
12+
1013
## [3.0.3]
1114

1215
### Fixed

‎src/NovaEditorJsCast.php

+12-10
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ class NovaEditorJsCast implements CastsAttributes
1111
/**
1212
* Magic number to describe "this field is broken" in the version
1313
*/
14-
private const BROKEN_VERSION = '2.12.9999';
14+
public const BROKEN_VERSION = '2.12.9999';
1515

1616
/**
1717
* Returns an instance of the EditorJsData field warning the user
@@ -47,14 +47,16 @@ private static function getErrorObject(string $exceptionMessage): NovaEditorJsDa
4747
* @param array $attributes
4848
* @return NovaEditorJsData|null
4949
*/
50-
public function get($model, string $key, $value, array $attributes)
50+
public function get($model, string $key, $value, array $attributes): ?NovaEditorJsData
5151
{
52-
if ($value === null) {
53-
return null;
54-
}
55-
5652
try {
57-
return new NovaEditorJsData(json_decode($value, true, 512, JSON_THROW_ON_ERROR));
53+
// Recursively decode JSON, to solve a bug where the JSON is double-encoded.
54+
while (is_string($value) && ! empty($value)) {
55+
$value = json_decode($value, true, 512, JSON_THROW_ON_ERROR);
56+
}
57+
58+
// Return null if the new value is null
59+
return $value === null ? null : new NovaEditorJsData($value);
5860
} catch (JsonException $exception) {
5961
return self::getErrorObject($exception->getMessage());
6062
}
@@ -67,9 +69,9 @@ public function get($model, string $key, $value, array $attributes)
6769
* @param string $key
6870
* @param mixed $value
6971
* @param array $attributes
70-
* @return mixed
72+
* @return array
7173
*/
72-
public function set($model, string $key, $value, array $attributes)
74+
public function set($model, string $key, $value, array $attributes): array
7375
{
7476
if ($value === null) {
7577
return [
@@ -83,7 +85,7 @@ public function set($model, string $key, $value, array $attributes)
8385
}
8486

8587
return [
86-
$key => json_encode($value),
88+
$key => is_string($value) ? $value : json_encode($value),
8789
];
8890
}
8991
}

‎tests/Fixtures/Models/Dummy.php

+27
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Tests\Fixtures\Models;
6+
7+
use Advoor\NovaEditorJs\NovaEditorJsCast;
8+
use Advoor\NovaEditorJs\NovaEditorJsData;
9+
use Illuminate\Database\Eloquent\Model;
10+
11+
/**
12+
* @property NovaEditorJsData $data
13+
* @method static \Illuminate\Database\Eloquent\Builder|Dummy newModelQuery()
14+
* @method static \Illuminate\Database\Eloquent\Builder|Dummy newQuery()
15+
* @method static \Illuminate\Database\Eloquent\Builder|Dummy query()
16+
* @mixin \Eloquent
17+
*/
18+
class Dummy extends Model
19+
{
20+
protected $casts = [
21+
'data' => NovaEditorJsCast::class,
22+
];
23+
24+
protected $fillable = [
25+
'data',
26+
];
27+
}

‎tests/Fixtures/TestServiceProvider.php

+20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Tests\Fixtures;
6+
7+
use Illuminate\Support\ServiceProvider;
8+
9+
class TestServiceProvider extends ServiceProvider
10+
{
11+
/**
12+
* Bootstrap any package services.
13+
*
14+
* @return void
15+
*/
16+
public function boot()
17+
{
18+
$this->app->useDatabasePath(__DIR__ . '/database/');
19+
}
20+
}

‎tests/Fixtures/database/migrations/2022_07_17_153928_create_dummies_table.php

+34
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
use Illuminate\Database\Migrations\Migration;
6+
use Illuminate\Database\Schema\Blueprint;
7+
use Illuminate\Support\Facades\Schema;
8+
9+
class CreateDummiesTable extends Migration
10+
{
11+
/**
12+
* Run the migrations.
13+
*
14+
* @return void
15+
*/
16+
public function up()
17+
{
18+
Schema::create('dummies', function (Blueprint $table) {
19+
$table->increments('id');
20+
$table->json('data')->nullable();
21+
$table->timestamps();
22+
});
23+
}
24+
25+
/**
26+
* Reverse the migrations.
27+
*
28+
* @return void
29+
*/
30+
public function down()
31+
{
32+
Schema::drop('dummies');
33+
}
34+
}

‎tests/resources/html/editorjs.html ‎tests/Fixtures/resources/html/editorjs.html

File renamed without changes.

‎tests/resources/json/editorjs.json ‎tests/Fixtures/resources/json/editorjs.json

File renamed without changes.

‎tests/TestCase.php

+11-1
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,21 @@
11
<?php
22

3+
declare(strict_types=1);
4+
35
namespace Tests;
46

57
use Advoor\NovaEditorJs\FieldServiceProvider;
8+
use Illuminate\Foundation\Testing\DatabaseMigrations;
69
use Orchestra\Testbench\TestCase as OrchestraTestCase;
10+
use Tests\Fixtures\TestServiceProvider;
711

812
/**
913
* Defines basic system to test with
1014
*/
1115
class TestCase extends OrchestraTestCase
1216
{
17+
use DatabaseMigrations;
18+
1319
/**
1420
* Path to config file from here
1521
*/
@@ -23,7 +29,10 @@ class TestCase extends OrchestraTestCase
2329
*/
2430
protected function getPackageProviders($app)
2531
{
26-
return [FieldServiceProvider::class];
32+
return [
33+
FieldServiceProvider::class,
34+
TestServiceProvider::class,
35+
];
2736
}
2837

2938
/**
@@ -35,5 +44,6 @@ protected function getPackageProviders($app)
3544
protected function getEnvironmentSetUp($app)
3645
{
3746
$app['config']->set('nova-editor-js', require self::CONFIG_PATH);
47+
$app['config']->set('database.default', 'testing');
3848
}
3949
}

‎tests/Unit/JsonContentTest.php

+2-3
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,14 @@
66

77
use Advoor\NovaEditorJs\NovaEditorJs;
88
use Tests\TestCase;
9-
use Illuminate\Foundation\Testing\RefreshDatabase;
109

1110
class JsonContentTest extends TestCase
1211
{
1312
/**
1413
* Path to JSON file with valid contents
1514
*/
16-
private const TEST_FILE_JSON = __DIR__ . '/../resources/json/editorjs.json';
17-
private const TEST_FILE_HTML = __DIR__ . '/../resources/html/editorjs.html';
15+
private const TEST_FILE_JSON = __DIR__ . '/../Fixtures/resources/json/editorjs.json';
16+
private const TEST_FILE_HTML = __DIR__ . '/../Fixtures/resources/html/editorjs.html';
1817

1918
/**
2019
* Returns JSON contents from the file

‎tests/Unit/NovaEditorJsCastTest.php

+111
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Tests\Unit;
6+
7+
use Advoor\NovaEditorJs\NovaEditorJsCast;
8+
use Advoor\NovaEditorJs\NovaEditorJsData;
9+
use Tests\TestCase;
10+
use Illuminate\Foundation\Testing\RefreshDatabase;
11+
use Illuminate\Support\Facades\DB;
12+
use Tests\Fixtures\Models\Dummy;
13+
14+
class NovaEditorJsCastTest extends TestCase
15+
{
16+
use RefreshDatabase;
17+
18+
private const VALID_BLOCK = [
19+
'time' => 1658064476,
20+
'blocks' => [
21+
[
22+
'type' => 'paragraph',
23+
'data' => [
24+
'text' => 'Paragraph'
25+
]
26+
]
27+
],
28+
'version' => '2.3.0',
29+
];
30+
31+
/**
32+
* Test a very basic save-and-decode.
33+
*/
34+
public function testSavingAndDecoding(): void
35+
{
36+
Dummy::create(['data' => self::VALID_BLOCK]);
37+
38+
$instance = Dummy::first();
39+
40+
$this->assertInstanceOf(NovaEditorJsData::class, $instance->data);
41+
$this->assertEquals(self::VALID_BLOCK, $instance->data->getAttributes());
42+
}
43+
44+
/**
45+
* Test saving and decoding a value that's already JSON-encoded.
46+
*/
47+
public function testSavingPreCompiledJson(): void
48+
{
49+
Dummy::create(['data' => json_encode(self::VALID_BLOCK)]);
50+
51+
$instance = Dummy::first();
52+
53+
$this->assertInstanceOf(NovaEditorJsData::class, $instance->data);
54+
$this->assertEquals(self::VALID_BLOCK, $instance->data->getAttributes());
55+
}
56+
57+
/**
58+
* Test decoding a value that's double-JSON-encoded, basically bug mitigation.
59+
*/
60+
public function testReadingDoubleEncodedJson(): void
61+
{
62+
DB::statement('INSERT INTO `dummies` (`data`) VALUES (?)', [json_encode(json_encode(self::VALID_BLOCK))]);
63+
64+
$instance = Dummy::first();
65+
66+
$this->assertInstanceOf(NovaEditorJsData::class, $instance->data);
67+
$this->assertEquals(self::VALID_BLOCK, $instance->data->getAttributes());
68+
}
69+
70+
/**
71+
* Test reading null values in JSON.
72+
*/
73+
public function testReadingNullValues(): void
74+
{
75+
DB::statement('INSERT INTO `dummies` (`data`) VALUES (?), (null)', [json_encode(null)]);
76+
77+
[$jsonInstance, $nullInstance] = Dummy::get();
78+
79+
$this->assertNull($jsonInstance->data);
80+
$this->assertNull($nullInstance->data);
81+
}
82+
83+
/**
84+
* Test writing null values, a json-encoded null value
85+
* should be stored as-is.
86+
*/
87+
public function testReadingNullValue(): void
88+
{
89+
Dummy::create(['data' => null]);
90+
Dummy::create(['data' => json_encode(null)]);
91+
92+
$rows = DB::select('SELECT `id`, `data` FROM `dummies`');
93+
94+
$this->assertNull($rows[0]->data);
95+
$this->assertSame('null', $rows[1]->data);
96+
}
97+
98+
/**
99+
* Finally, check if reading broken JSON is handled properly.
100+
*/
101+
public function testReadingInvalidJson(): void
102+
{
103+
DB::statement('INSERT INTO `dummies` (`data`) VALUES (?)', ['{"}']);
104+
105+
$instance = Dummy::first();
106+
107+
$this->assertInstanceOf(NovaEditorJsData::class, $instance->data);
108+
$this->assertNotNull($instance->data->version, 'Expected version key on data');
109+
$this->assertEquals(NovaEditorJsCast::BROKEN_VERSION, $instance->data->version, 'Expected version to match "broken" version');
110+
}
111+
}

0 commit comments

Comments
 (0)