Skip to content
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

Alternative way to define InheritanceMap #66

Closed
rsavuliak-macpaw opened this issue May 22, 2024 · 2 comments · Fixed by #68
Closed

Alternative way to define InheritanceMap #66

rsavuliak-macpaw opened this issue May 22, 2024 · 2 comments · Fixed by #68

Comments

@rsavuliak-macpaw
Copy link

I am wondering if there can be an alternative way to define InheritanceMap? Cause current one expects me to set an attribute on the target abstract class. However, for certain architectures like Onion, Clean Architecture and other layered examples this can be a violation of responsibility, when your classes "know" about classes from higher layer. It happens on mapping from DTO to Entity object. For example:

use Rekalogika\Mapper\Attribute\InheritanceMap;
use Domain\ConcreteClassA;
use Domain\ConcreteClassB;
use Domain\ConcreteClassC;
use Presentation\RequestAObjDTO;
use Presentation\RequestBObjDTO;
use Presentation\RequestCObjDTO;

#[InheritanceMap([
    RequestAObjDTO::class => ConcreteClassA::class,
    RequestBObjDTO::class => ConcreteClassB::class,
    RequestCObjDTO::class => ConcreteClassC::class,
])]
abstract class ConcreteClass
{
}

So in this case, to convert Presentation layer object into Domain layer object I have to define this mapping in the Domain layer. That means I will get an alert in architecture control tools like phparkitect/deptrac that my Domain layer has dependencies on Presentation layer.

I guess it would be nice to have a way to define this inheritance mapping on Presentation layer side, maybe it's possible to do this on the source class providing 2 attributes smth like SourceInheritanceMap and TargetInheritanceMap, depending if this class treats as a source or as a target? Or maybe pass it in Context for this specific mapping action. Just brainstorming 🤷‍♂️

P.S. Offtop, I see you've added support for ramsey/uuid - that's great, cause we are using it widely - can you pls create new release so we don't need to make "dev-master" dependency? 😉

@priyadi
Copy link
Member

priyadi commented May 23, 2024

I have made a release with the ramsey/uuid support. Let me know if there's a problem.

Regarding your main issue, from what I observed, attributes are usually not considered in architectural constraints because the code will still work without the class used in the attributes, a case in point. However, I understand if not everyone feels the same way.

I used to have this in mind: if the target is abstract or an interface, then if a parent or an interface of the source has an InheritanceMap, it will be flipped and used as if the flipped InheritanceMap is on the target side. The upside is that we only need to maintain one InheritanceMap for bidirectional mapping. I'll see if this is possible.

But the long-term solution is a class metadata, so that everything that the attributes can do now will be possible using an external YAML configuration.

@priyadi
Copy link
Member

priyadi commented May 23, 2024

I've implemented the reversed InheritanceMap in the linked PR. Now, you can add InheritanceMap to your DTOs, and it will be considered when you are mapping from the DTOs. Please test it to see if it is sufficient for your use case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants