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.

Sign up for GitHub

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

Jump to bottom

The SPI for media types needs to reflect it's ONLY for internal usage. #1304

Closed
gregturn opened this issue Jun 4, 2020 · 4 comments
Closed

The SPI for media types needs to reflect it's ONLY for internal usage. #1304

gregturn opened this issue Jun 4, 2020 · 4 comments
Assignees
@gregturn
Labels
in: configuration Configuration and setup in: documentation Reference documentation in: mediatypes Media type related functionality process: waiting for feedback type: bug
Milestone
1.2 M1

Comments

@gregturn
Copy link
Contributor

gregturn commented Jun 4, 2020

On gitter, @toedter reported having an issue registering his custom mediatype for JSON:API.

Turns out, the problem was that when loading a provider from spring.factories, Spring HATEOAS filters against the list of media types registered via @EnableHypermediaSupport. Further digging shows that we don't actually have any tests exercising a custom media type loaded from spring.factories.
@gregturn gregturn self-assigned this Jun 4, 2020
@gregturn gregturn added in: configuration Configuration and setup in: mediatypes Media type related functionality labels Jun 4, 2020
@gregturn gregturn added this to the 1.2.0-M1 milestone Jun 4, 2020
gregturn added a commit that referenced this issue Jun 4, 2020
@gregturn
#1304 - Properly handle custom mediatypes loaded from spring.factories.
43e25c3 
If a project-provided mediatype should only be activated when explicitly requested via @EnableHypermediaSupport. However, a custom mediatype should never be filted out because of this.

To handle this, a marker interface is defined that all the project-specific mediatypes can implement. This was, the enabling code can test whether or not the project-specific mediatypes "support" the declared mediatypes. However, mediatypes that do NOT have this marker interface (custom ones), should be registered unconditionally.
@odrotbohm
Copy link
Member

odrotbohm commented Jul 22, 2020 •
edited by gregturn
Loading

I don't actually think, we need a fix for that but in the documentation. The spring.factories approach is just necessary for the media types that we ship support for OOTB. For custom media types, registering the HypermediaMappingInformation as bean should be sufficient to get activated. IIRC the reference docs were just wrongly suggesting spring.factories is needed.

@toedter – Would you mind trying whether this solves your problem?

@odrotbohm
Copy link
Member

odrotbohm commented Jul 28, 2020 •
edited by gregturn
Loading

  • Remove section on MediaTypeConfigurationProvider from the reference documentation.
  • Update Javadoc of MediaTypeConfigurationProvider to note that this is not needed for a custom media type implementation but rather an internal artifact

@gregturn gregturn changed the title The SPI for custom media types doesn't handle loading from spring.factories properly. The SPI for custom media types needs to reflect it's ONLY for internal usage. Jul 28, 2020
@gregturn gregturn changed the title The SPI for custom media types needs to reflect it's ONLY for internal usage. The SPI for media types needs to reflect it's ONLY for internal usage. Jul 28, 2020
gregturn added a commit that referenced this issue Jul 28, 2020
@gregturn
#1304 - Update ref docs to properly show how to register custom media…
bfdf0d2 
… types.

Custom media types do NOT require registering a MediaTypeConfigurationProvider implementation with spring.factories. The reference docs must be updated to illustrate this. Also add to the javadocs so users are properly warned.
gregturn added a commit that referenced this issue Jul 28, 2020
@gregturn
#1304 - Update ref docs to properly show how to register custom media…
4720e56 
… types.

Custom media types do NOT require registering a MediaTypeConfigurationProvider implementation with spring.factories. The reference docs must be updated to illustrate this. Also add to the javadocs so users are properly warned.
@gregturn gregturn mentioned this issue Jul 28, 2020
Closed
@gregturn gregturn added the in: documentation Reference documentation label Jul 28, 2020
gregturn added a commit that referenced this issue Jul 28, 2020
@gregturn
#1338 - Backport the adjustment to ref docs about custom media types.
2f7a302 
Custom media types do NOT require registering a MediaTypeConfigurationProvider implementation with spring.factories. The reference docs must be updated to illustrate this. Also add to the javadocs so users are properly warned.

Original issue: #1304
@gregturn gregturn mentioned this issue Jul 28, 2020
Closed
@ingogriebsch ingogriebsch mentioned this issue Jul 29, 2020
Closed
@gregturn
Copy link
Contributor Author

Resolved via 4720e56.

@toedter
Copy link

toedter commented Jul 29, 2020

I am currently trying....

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@gregturn gregturn

Labels
in: configuration Configuration and setup in: documentation Reference documentation in: mediatypes Media type related functionality process: waiting for feedback type: bug
Projects
None yet
Milestone
1.2 M1
Development

No branches or pull requests
3 participants
@odrotbohm @gregturn @toedter