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

Change signature of ReactiveRepresentationModelAssembler #1796

Closed
toedter opened this issue May 31, 2022 · 4 comments
Closed

Change signature of ReactiveRepresentationModelAssembler #1796

toedter opened this issue May 31, 2022 · 4 comments
Assignees
Labels
Milestone

Comments

@toedter
Copy link

toedter commented May 31, 2022

The current signature of ReactiveRepresentationModelAssembler is

public interface ReactiveRepresentationModelAssembler<T, D extends RepresentationModel<D>>

The non-reactive RepresentationModelAssembler has the signature:

public interface RepresentationModelAssembler<T, D extends RepresentationModel<?>>

which makes it possible to work together with custom media type implementations (JSON:API, Siren, ...) that provide public RepresentationModel<?> API rather than exposing their internal RepresentationModel implementations as public API.

Would it make sense to change the signature of ReactiveRepresentationModelAssembler to

public interface ReactiveRepresentationModelAssembler<T, D extends RepresentationModel<?>>

?

This would have the following 2 advantages:

  1. Custom media type implementations could be used out of the box
  2. API consistency between reactive and non-reactive RepresentationModelAssemblers
@odrotbohm
Copy link
Member

Thanks for filing this, Kai. This looks like an oversight to me. I'll check what the compatibility implications are of this, but I am afraid we at least cause binary incompatibility here. I guess the first version we can ship this in is 2.0 then, as there's no 1.6 release planned anymore.

@odrotbohm odrotbohm self-assigned this May 31, 2022
@toedter
Copy link
Author

toedter commented May 31, 2022

Thx for the quick response, Oli!

odrotbohm added a commit that referenced this issue Jun 1, 2022
We now relax the generic bound of D to … extends RepresentationModel<?> as otherwise implementation code of, for example, toModel(…) has to produce a concrete representation model and cannot be typed to return RepresentationModel<?>.
@odrotbohm
Copy link
Member

This should be in place for the 2.0 snapshots. Please let me know if we should investigate how a merge into 1.5.x would affect compatibility.

@odrotbohm odrotbohm added this to the 2.0 M4 milestone Jun 1, 2022
@toedter
Copy link
Author

toedter commented Jun 2, 2022

For me it would be fine to have it only in 2.x.

On the other side, I would expect this to be a backward-compatible change, that could potentially also applied to 1.5.x. But I am NOT an expert in this, and it is probably not worth spending much time investigating :)

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

No branches or pull requests

2 participants