-
-
Notifications
You must be signed in to change notification settings - Fork 218
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
Feature: property-morphable abstract data #921
base: main
Are you sure you want to change the base?
Changes from all commits
c88b8fb
a17d5f9
9e6e93d
2764e83
1f630b1
46c9551
549889b
b978297
17be5be
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 |
---|---|---|
@@ -0,0 +1,8 @@ | ||
<?php | ||
|
||
namespace Spatie\LaravelData\Contracts; | ||
|
||
interface PropertyMorphableData | ||
{ | ||
public static function morph(array $properties): ?string; | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -7,6 +7,7 @@ | |||||||||||||||||
use Illuminate\Validation\Rule; | ||||||||||||||||||
use Spatie\LaravelData\Attributes\Validation\ArrayType; | ||||||||||||||||||
use Spatie\LaravelData\Attributes\Validation\Present; | ||||||||||||||||||
use Spatie\LaravelData\Contracts\PropertyMorphableData; | ||||||||||||||||||
use Spatie\LaravelData\Support\DataClass; | ||||||||||||||||||
use Spatie\LaravelData\Support\DataConfig; | ||||||||||||||||||
use Spatie\LaravelData\Support\DataProperty; | ||||||||||||||||||
|
@@ -34,6 +35,15 @@ public function execute( | |||||||||||||||||
): array { | ||||||||||||||||||
$dataClass = $this->dataConfig->getDataClass($class); | ||||||||||||||||||
|
||||||||||||||||||
if ($this->isPropertyMorphable($dataClass)) { | ||||||||||||||||||
/** | ||||||||||||||||||
* @var class-string<PropertyMorphableData> $class | ||||||||||||||||||
*/ | ||||||||||||||||||
$payload = $path->isRoot() ? $fullPayload : Arr::get($fullPayload, $path->get(), []); | ||||||||||||||||||
$class = $class::morph($payload) ?? $class; | ||||||||||||||||||
$dataClass = $this->dataConfig->getDataClass($class); | ||||||||||||||||||
Comment on lines
+42
to
+44
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's rewrite this to:
Suggested change
|
||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
$withoutValidationProperties = []; | ||||||||||||||||||
|
||||||||||||||||||
foreach ($dataClass->properties as $dataProperty) { | ||||||||||||||||||
|
@@ -278,4 +288,9 @@ protected function inferRulesForDataProperty( | |||||||||||||||||
$path | ||||||||||||||||||
); | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
protected function isPropertyMorphable(DataClass $dataClass): bool | ||||||||||||||||||
{ | ||||||||||||||||||
return $dataClass->isAbstract && $dataClass->propertyMorphable; | ||||||||||||||||||
} | ||||||||||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -53,6 +53,10 @@ | |
use Spatie\LaravelData\Tests\Fakes\NestedLazyData; | ||
use Spatie\LaravelData\Tests\Fakes\NestedModelCollectionData; | ||
use Spatie\LaravelData\Tests\Fakes\NestedModelData; | ||
use Spatie\LaravelData\Tests\Fakes\PropertyMorphableData\AbstractPropertyMorphableData; | ||
use Spatie\LaravelData\Tests\Fakes\PropertyMorphableData\NestedPropertyMorphableData; | ||
use Spatie\LaravelData\Tests\Fakes\PropertyMorphableData\PropertyMorphableDataA; | ||
use Spatie\LaravelData\Tests\Fakes\PropertyMorphableData\PropertyMorphableDataB; | ||
Comment on lines
+56
to
+59
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's try to inline these classes as much as possible, the Fakes directory is already sooo big with a lot of specific classes. My goal is trying to eliminate 90% of them one day so that the definitions of the classes are closer to the tests and so that we don't have to come up with more class names 😄. Feel free to use anonymous classes or inline classes (just prefix the names with Test). If a class is being used a lot it might be a fit to put in Fakes but most of the cases it isn't. Except for classes with class attributes because they aren't parsed when written inline if I'm correct. |
||
use Spatie\LaravelData\Tests\Fakes\SimpleData; | ||
use Spatie\LaravelData\Tests\Fakes\SimpleDataWithoutConstructor; | ||
|
||
|
@@ -1227,3 +1231,79 @@ public static function pipeline(): DataPipeline | |
[10, SimpleData::from('Hello World')] | ||
); | ||
})->todo(); | ||
|
||
it('will allow property-morphable data to be created', function () { | ||
$dataA = AbstractPropertyMorphableData::from([ | ||
'variant' => 'a', | ||
'a' => 'foo', | ||
'enum' => 'foo', | ||
]); | ||
|
||
expect($dataA) | ||
->toBeInstanceOf(PropertyMorphableDataA::class) | ||
->variant->toEqual('a') | ||
->a->toEqual('foo') | ||
->enum->toEqual(DummyBackedEnum::FOO); | ||
|
||
$dataB = AbstractPropertyMorphableData::from([ | ||
'variant' => 'b', | ||
'b' => 'bar', | ||
]); | ||
|
||
expect($dataB) | ||
->toBeInstanceOf(PropertyMorphableDataB::class) | ||
->variant->toEqual('b') | ||
->b->toEqual('bar'); | ||
}); | ||
|
||
it('will allow property-morphable data to be created from concrete', function () { | ||
$dataA = PropertyMorphableDataA::from([ | ||
'a' => 'foo', | ||
'enum' => 'foo', | ||
]); | ||
|
||
expect($dataA) | ||
->toBeInstanceOf(PropertyMorphableDataA::class) | ||
->variant->toEqual('a') | ||
->a->toEqual('foo') | ||
->enum->toEqual(DummyBackedEnum::FOO); | ||
}); | ||
|
||
it('will allow property-morphable data to be created from a nested collection', function () { | ||
$data = NestedPropertyMorphableData::from([ | ||
'nestedCollection' => [ | ||
['variant' => 'a', 'a' => 'foo', 'enum' => 'foo'], | ||
['variant' => 'b', 'b' => 'bar'], | ||
], | ||
]); | ||
|
||
expect($data->nestedCollection[0]) | ||
->toBeInstanceOf(PropertyMorphableDataA::class) | ||
->variant->toEqual('a') | ||
->a->toEqual('foo') | ||
->enum->toEqual(DummyBackedEnum::FOO); | ||
|
||
expect($data->nestedCollection[1]) | ||
->toBeInstanceOf(PropertyMorphableDataB::class) | ||
->variant->toEqual('b') | ||
->b->toEqual('bar'); | ||
}); | ||
|
||
|
||
it('will allow property-morphable data to be created as a collection', function () { | ||
$collection = AbstractPropertyMorphableData::collect([ | ||
['variant' => 'a', 'a' => 'foo', 'enum' => DummyBackedEnum::FOO->value], | ||
['variant' => 'b', 'b' => 'bar'], | ||
]); | ||
|
||
expect($collection[0]) | ||
->toBeInstanceOf(PropertyMorphableDataA::class) | ||
->variant->toEqual('a') | ||
->a->toEqual('foo') | ||
->enum->toEqual(DummyBackedEnum::FOO); | ||
|
||
expect($collection[1]) | ||
->toBeInstanceOf(PropertyMorphableDataB::class) | ||
->variant->toEqual('b') | ||
->b->toEqual('bar'); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
<?php | ||
|
||
namespace Spatie\LaravelData\Tests\Fakes\Models; | ||
|
||
use Illuminate\Database\Eloquent\Model; | ||
use Spatie\LaravelData\Tests\Fakes\PropertyMorphableData\AbstractPropertyMorphableData; | ||
use Spatie\LaravelData\Tests\Fakes\SimpleDataCollection; | ||
|
||
class DummyModelWithPropertyMorphableCast extends Model | ||
{ | ||
protected $casts = [ | ||
'data' => AbstractPropertyMorphableData::class, | ||
'data_collection' => SimpleDataCollection::class.':'.AbstractPropertyMorphableData::class, | ||
]; | ||
|
||
protected $table = 'dummy_model_with_casts'; | ||
|
||
public $timestamps = false; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
<?php | ||
|
||
namespace Spatie\LaravelData\Tests\Fakes\PropertyMorphableData; | ||
|
||
use Spatie\LaravelData\Attributes\Validation\In; | ||
use Spatie\LaravelData\Contracts\PropertyMorphableData; | ||
use Spatie\LaravelData\Data; | ||
|
||
abstract class AbstractPropertyMorphableData extends Data implements PropertyMorphableData | ||
{ | ||
public function __construct( | ||
#[In('a', 'b')] | ||
public string $variant, | ||
) { | ||
} | ||
|
||
public static function morph(array $properties): ?string | ||
{ | ||
return match ($properties['variant'] ?? null) { | ||
'a' => PropertyMorphableDataA::class, | ||
'b' => PropertyMorphableDataB::class, | ||
default => null, | ||
}; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
<?php | ||
|
||
namespace Spatie\LaravelData\Tests\Fakes\PropertyMorphableData; | ||
|
||
use Spatie\LaravelData\Data; | ||
use Spatie\LaravelData\DataCollection; | ||
|
||
class NestedPropertyMorphableData extends Data | ||
{ | ||
public function __construct( | ||
/** @var AbstractPropertyMorphableData[] */ | ||
public ?DataCollection $nestedCollection, | ||
) { | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
<?php | ||
|
||
namespace Spatie\LaravelData\Tests\Fakes\PropertyMorphableData; | ||
|
||
use Spatie\LaravelData\Tests\Fakes\Enums\DummyBackedEnum; | ||
|
||
class PropertyMorphableDataA extends AbstractPropertyMorphableData | ||
{ | ||
public function __construct( | ||
public string $a, | ||
public DummyBackedEnum $enum, | ||
) { | ||
parent::__construct('a'); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
<?php | ||
|
||
namespace Spatie\LaravelData\Tests\Fakes\PropertyMorphableData; | ||
|
||
class PropertyMorphableDataB extends AbstractPropertyMorphableData | ||
{ | ||
public function __construct( | ||
public string $b, | ||
) { | ||
parent::__construct('b'); | ||
} | ||
} |
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.
I'm not completely sure why we first run the pipeline here with the abstract class and then run it again with the inherited class? That first run seems to be completely unnecessary right?