-
Notifications
You must be signed in to change notification settings - Fork 30
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
Add middleware feature #47
base: master
Are you sure you want to change the base?
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,33 @@ | ||
<?php | ||
|
||
namespace AutoMapperPlus; | ||
|
||
use AutoMapperPlus\Exception\UnregisteredMappingException; | ||
|
||
/** | ||
* Interface ClassMapperInterface | ||
* | ||
* @package AutoMapperPlus | ||
*/ | ||
interface ClassMapperInterface | ||
{ | ||
/** | ||
* Maps an object to an instance of class $to, provided a mapping is | ||
* configured. | ||
* | ||
* @param array|object $source | ||
* The source object. | ||
* @param string $targetClass | ||
* The target classname. | ||
* @param array $context | ||
* An arbitrary array of values that will be passed to supporting | ||
* mapping operations (e.g. MapFrom) to alter their behaviour based on | ||
* the context. | ||
* This is not explicitly required on the interface yet to preserve | ||
* backwards compatibility, but will be added in version 2.0. | ||
* @return mixed | ||
* An instance of class $to. | ||
* @throws UnregisteredMappingException | ||
*/ | ||
public function map($source, string $targetClass/**, array $context = [] */); | ||
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. Since this is a completely new interface, we can actually make the 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. Oh wait, the existing interface extends this one, disregard! |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,13 @@ | |
|
||
namespace AutoMapperPlus\Configuration; | ||
|
||
use AutoMapperPlus\Middleware\DefaultMapperMiddleware; | ||
use AutoMapperPlus\Middleware\DefaultMiddleware; | ||
use AutoMapperPlus\Middleware\DefaultPropertyMiddleware; | ||
use AutoMapperPlus\Middleware\MapperMiddleware; | ||
use AutoMapperPlus\Middleware\Middleware; | ||
use AutoMapperPlus\Middleware\PropertyMiddleware; | ||
|
||
/** | ||
* Class AutoMapperConfig | ||
* | ||
|
@@ -14,6 +21,22 @@ class AutoMapperConfig implements AutoMapperConfigInterface | |
*/ | ||
private $mappings = []; | ||
|
||
/** @var MapperMiddleware */ | ||
private $defaultMapperMiddleware; | ||
|
||
/** @var PropertyMiddleware */ | ||
private $defaultPropertyMiddleware; | ||
|
||
/** | ||
* @var MapperMiddleware[] | ||
*/ | ||
private $mapperMiddlewares = []; | ||
|
||
/** | ||
* @var PropertyMiddleware[] | ||
*/ | ||
private $propertyMiddlewares = []; | ||
|
||
/** | ||
* @var Options | ||
*/ | ||
|
@@ -30,6 +53,8 @@ public function __construct(callable $configurator = null) | |
if ($configurator !== null) { | ||
$configurator($this->options); | ||
} | ||
$this->defaultMapperMiddleware = new DefaultMapperMiddleware(); | ||
$this->defaultPropertyMiddleware = new DefaultPropertyMiddleware(); | ||
} | ||
|
||
/** | ||
|
@@ -202,11 +227,64 @@ public function registerMapping( | |
return $mapping; | ||
} | ||
|
||
|
||
public function registerMiddlewares(Middleware ...$middlewares): AutoMapperConfigInterface | ||
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. This way you can only register all middlewares at once. It might be interesting to add an 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. Ok, i'll change this ! Sorry for the delay, i'm busy on one project right now, but I use this current fork for it and it really solves many use cases :). I'll try to update the pull request tomorrow. 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. No problem at all; I took a long time to reply myself :) 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. @mark-gerarts I'm reading this pull request again, and it's not required to register all middlewares at once, because registerMiddlewares is chainable, and invoking registerMiddleswares won't remove already registered middlewares. So you can add middlewares one by one, by calling registerMiddlewares as many times as you want. Do you plan to merge this PR and release 2.0 ? 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. Yes, I want to include this in release 2.0, which I aim to release this summer. As for the adding of middlewares: registerMiddlewares does indeed allow for chaining, but maybe It's useful to add some utility methods such as add. I'll look into this into detail once I start merging the PR. 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. Hi @mark-gerarts, any ETA for this release ? 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've set up a board for the release. The scope is rather limited, but my time is as well at the moment. To be honest, I have not really been able to work on the release the past months. I should be able to finally work a bit on the release again the following weeks (:crossed_fingers:). The question is if it will be enough to finish all open tasks. I'll keep you posted once I've worked myself in again and have a better overview! |
||
{ | ||
foreach ($middlewares as $middleware) { | ||
if ($middleware instanceof MapperMiddleware) { | ||
$this->mapperMiddlewares[] = $middleware; | ||
if ($middleware instanceof DefaultMiddleware) { | ||
$this->defaultMapperMiddleware = $middleware; | ||
} | ||
} | ||
if ($middleware instanceof PropertyMiddleware) { | ||
$this->propertyMiddlewares[] = $middleware; | ||
if ($middleware instanceof DefaultMiddleware) { | ||
$this->defaultPropertyMiddleware = $middleware; | ||
} | ||
} | ||
} | ||
|
||
return $this; | ||
} | ||
|
||
/** | ||
* @inheritdoc | ||
*/ | ||
public function getOptions(): Options | ||
{ | ||
return $this->options; | ||
} | ||
|
||
/** | ||
* @return PropertyMiddleware | ||
*/ | ||
public function getDefaultPropertyMiddleware(): PropertyMiddleware | ||
{ | ||
return $this->defaultPropertyMiddleware; | ||
} | ||
|
||
/** | ||
* @return MapperMiddleware | ||
*/ | ||
public function getDefaultMapperMiddleware(): MapperMiddleware | ||
{ | ||
return $this->defaultMapperMiddleware; | ||
} | ||
|
||
/** | ||
* @inheritdoc | ||
*/ | ||
public function getMapperMiddlewares() | ||
{ | ||
return $this->mapperMiddlewares; | ||
} | ||
|
||
/** | ||
* @inheritdoc | ||
*/ | ||
public function getPropertyMiddlewares() | ||
{ | ||
return $this->propertyMiddlewares; | ||
} | ||
} |
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.
Note that CustomMapper is deprecated, and map method of the AbstractMapper class won't be invoked anymore (object is constructed through the automapper configuration, and then passed to mapToObject method)