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

Support mapping from json encoded mapping result #136

Closed
wants to merge 8 commits into from
Closed

Support mapping from json encoded mapping result #136

wants to merge 8 commits into from

Conversation

mtouellette
Copy link
Contributor

useful in api integration tests

@mtouellette mtouellette changed the title Support mapping from json encoded result Support mapping from json encoded mapping result May 25, 2022
@romm
Copy link
Member

romm commented May 25, 2022

Hi @mtouellette, thank you for the contribution.

Although I understand your needs, this pull request cannot be merged as it is: the library wont support custom oriented implementations of DateTimeInterface, even if this can be useful in your case.

Still, I think you can manage to make it working on your side, can you try this in your application:

try {
    $result = (new \CuyZ\Valinor\MapperBuilder())
        ->infer(DateTimeInterface::class, fn() => SomeJsonSerializableDateTime::class)
        ->mapper()
        ->map(SomeClass::class, [/* … */]);
} catch (MappingError $error) {
    // …
}

But your proposition reminded me that the infer method actually doesn't support class inheritance, which is a pity. The code below doesn't work for now, but I will certainly add this feature at some point.

try {
    $result = (new \CuyZ\Valinor\MapperBuilder())
        ->infer(DateTime::class, fn() => SomeClassThatInheritsDateTime::class)
        ->mapper()
        ->map(SomeClass::class, [/* … */]);
} catch (MappingError $error) {
    // …
}

@romm romm closed this May 25, 2022
@mtouellette
Copy link
Contributor Author

HI @romm, I figured this pull request was a bit of a reach and ended up getting around the issue for now with a source modifier which isn't great but seems to do the trick. Thanks for reviewing this and for the excellent library.

@romm
Copy link
Member

romm commented May 26, 2022

I'm glad you could solve your issue, what about the solution I posted above? Could it help?

@mtouellette
Copy link
Contributor Author

It looks like it might, though I haven't tried yet. I'm dealing with a lot of auto generated code from an OpenApi specification and the output of the data transfer objects uses DateTime rather than DateTimeInterface. Will infer work with concrete classes as well as interfaces?

@romm
Copy link
Member

romm commented May 26, 2022

Ok, thanks for the info.

That's what I wrote above, unfortunately inferring wont work with classes inheritance for the moment; this is clearly a missing feature that I will be adding to the library at some point: check for next releases!

@mtouellette
Copy link
Contributor Author

Ah, I see. I'll certainly keep an eye out for updates. Thanks again!

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 this pull request may close these issues.

2 participants