-
Notifications
You must be signed in to change notification settings - Fork 2
add extension api #137
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: 2.0.x
Are you sure you want to change the base?
add extension api #137
Changes from all commits
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,25 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace Patchlevel\Hydrator; | ||
|
|
||
| use Patchlevel\Hydrator\Guesser\BuiltInGuesser; | ||
| use Patchlevel\Hydrator\Guesser\Guesser; | ||
| use Patchlevel\Hydrator\Middleware\Middleware; | ||
| use Patchlevel\Hydrator\Middleware\TransformMiddleware; | ||
|
|
||
| final class CoreExtension implements MiddlewareProvider, GuesserProvider | ||
| { | ||
| /** @return iterable<Middleware|array{0: Middleware, 1?: int}> */ | ||
| public function middlewares(): iterable | ||
| { | ||
| yield new TransformMiddleware(); | ||
| } | ||
|
|
||
| /** @return iterable<Guesser|array{0: Guesser, 1?: int}> */ | ||
| public function guesser(): iterable | ||
| { | ||
| yield new BuiltInGuesser(); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace Patchlevel\Hydrator\Cryptography; | ||
|
|
||
| use Patchlevel\Hydrator\Metadata\MetadataEnricher; | ||
| use Patchlevel\Hydrator\MetadataEnricherProvider; | ||
| use Patchlevel\Hydrator\Middleware\Middleware; | ||
| use Patchlevel\Hydrator\MiddlewareProvider; | ||
|
|
||
| final class CryptographyExtension implements MiddlewareProvider, MetadataEnricherProvider | ||
| { | ||
| public function __construct( | ||
| private readonly PayloadCryptographer $cryptography, | ||
| ) { | ||
| } | ||
|
|
||
| /** @return iterable<Middleware|array{0: Middleware, 1?: int}> */ | ||
| public function middlewares(): iterable | ||
| { | ||
| yield [new CryptographyMiddleware($this->cryptography), 64]; | ||
| } | ||
|
|
||
| /** @return iterable<MetadataEnricher|array{0: MetadataEnricher, 1?: int}> */ | ||
| public function metadataEnrichers(): iterable | ||
| { | ||
| yield [new CryptographyMetadataEnricher(), 64]; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,13 @@ | ||||||
| <?php | ||||||
|
|
||||||
| declare(strict_types=1); | ||||||
|
|
||||||
| namespace Patchlevel\Hydrator; | ||||||
|
|
||||||
| use Patchlevel\Hydrator\Guesser\Guesser; | ||||||
|
|
||||||
| interface GuesserProvider | ||||||
| { | ||||||
| /** @return iterable<Guesser|array{0: Guesser, 1?: int}> */ | ||||||
|
Member
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.
Suggested change
I think reducing it to this is a better api, otherwise we have 2 version's to omit the priority as the also allow here the priority to be missing. wdyt?
Member
Author
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. I do not think either "type" is harmful, and I like the DX of the first one. Besides, it is practically the same principle as in Symfony. |
||||||
| public function guesser(): iterable; | ||||||
| } | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace Patchlevel\Hydrator\Metadata; | ||
|
|
||
| interface MetadataEnricher | ||
| { | ||
| public function enrich(ClassMetadata $classMetadata): void; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace Patchlevel\Hydrator; | ||
|
|
||
| use Patchlevel\Hydrator\Metadata\MetadataEnricher; | ||
|
|
||
| interface MetadataEnricherProvider | ||
| { | ||
| /** @return iterable<MetadataEnricher|array{0: MetadataEnricher, 1?: int}> */ | ||
|
Member
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. same here |
||
| public function metadataEnrichers(): iterable; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace Patchlevel\Hydrator; | ||
|
|
||
| use Patchlevel\Hydrator\Middleware\Middleware; | ||
|
|
||
| interface MiddlewareProvider | ||
| { | ||
| /** @return iterable<Middleware|array{0: Middleware, 1?: int}> */ | ||
| public function middlewares(): iterable; | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.
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.
other idea which i have in mind is: instead of this provider use attributes to define middleware/metadataenricher?
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.
No, I do not want to work with attributes here. I think attributes are good if you want to enrich your own code (from your domain) with information from external sources. But not as an extension point for the library. I would always prefer interfaces for that.
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 dont want to remove the interface (I missed it in the small code example) it's more about the provider / configuration of the priority
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 extension is there to register the middleware, etc. I find it unnecessary to read the attributes from it. In my opinion, that adds no value and only complicates things.
I am also considering whether interfaces like
MiddlewareProviderare overkill, and whether it would be better to simply provide an abstract classExtensionthat predefines methods with empty arrays. That would also make things simpler.