Skip to content

Conversation

@Spomky
Copy link
Contributor

@Spomky Spomky commented Dec 15, 2025

Q A
Branch? 4.2
Tickets none
License MIT
Doc PR not needed

This PR follows up on #7601 and applies the same fix to the ObjectMapperProcessor.

While #7601 addressed the provider side by properly using the operation output class when defined, the processor was still relying on $operation->getClass(). This change updates the processor to use the operation’s output DTO ($operation->getOutput()['class'] ?? $operation->getClass()), ensuring consistent behavior between providers and processors.

ping @yceruto

@Spomky Spomky changed the title fix(metadata): enhance resource class determination before Object Map… fix(metadata): enhance resource class determination before Object Mapper Processor return Dec 15, 2025
@Spomky Spomky changed the title fix(metadata): enhance resource class determination before Object Mapper Processor return fix(metadata): Use operation output DTO in ObjectMapperProcessor Dec 15, 2025
@yceruto
Copy link

yceruto commented Dec 15, 2025

Thanks @Spomky for addressing this 🤟

Yes, this will cover the use case of having a custom output DTO for state processors, but I just realized it will only work if a custom input DTO is also defined; otherwise, it won't reach the final lines where the output is mapped.

We may need to refactor the entire method to explicitly handle all possible combinations:

  • no custom input and no custom output (supported) --> returns persisted data
  • no custom input with a custom output (unsupported) <-- missing scenario, should return output DTO
  • custom input with no custom output (supported) -> returns persisted data
  • custom input with a custom output (adding support in this PR) -> returns output DTO

@yceruto
Copy link

yceruto commented Dec 15, 2025

I'm afraid that touching ObjectMapperMetadataCollectionFactory may also be necessary, since it is the one that sets map=true for this operation

@Spomky
Copy link
Contributor Author

Spomky commented Dec 16, 2025

Correct.
I will update my PR.

@Spomky Spomky force-pushed the bug/processor-output branch from 11d74db to a5cbb02 Compare December 16, 2025 16:48
@Spomky
Copy link
Contributor Author

Spomky commented Dec 16, 2025

Modifications done.

I see 4 cases: no mapping, input mapping, output mapping, and input/output mapping.
The last changes take these scenarios.
For the factory, the change should be sufficient.

I will add some tests to make sure it is fine.

@Spomky
Copy link
Contributor Author

Spomky commented Dec 16, 2025

Tests are failing because it is really not that simple.
THe entity class and the resource class may be different. This means there could be another mapping in between.
This may also be the case for the provider. I will dig into it and update this PR.

@Spomky Spomky force-pushed the bug/processor-output branch 2 times, most recently from 1853722 to 0587ff6 Compare December 16, 2025 18:02
@Spomky Spomky force-pushed the bug/processor-output branch from 0587ff6 to 5b38dea Compare December 16, 2025 19:17
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