Skip to content

Collections not embedded with HAL compatible vendor specific media type #1253

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

Closed
arhohuttunen opened this issue Apr 3, 2020 · 18 comments
Closed
Assignees
Labels
in: configuration Configuration and setup in: mediatypes Media type related functionality type: enhancement
Milestone

Comments

@arhohuttunen
Copy link

arhohuttunen commented Apr 3, 2020

I have been searching for information about vendor specific media types that are HAL compatible, and found several issues related to this. Apparently it is possible to easily add custom media types.

However, I'm unable to make returned collections to be embedded.

Taking the CustomHyperMediaWebMvcTest as an example, and adding a collection:

@GetMapping("/employees")
public CollectionModel<Employee> findOne() {
    List<EntityModel<Employee>> employees = new ArrayList();
    employees.add(new EntityModel(new Employee("Frodo Baggins", "ring bearer"),
            linkTo(methodOn(EmployeeController.class).findOne()).withSelfRel()));
    return new CollectionModel(employees);
}

This will return:

{
  "links": [],
  "content": [
    {
      "name": "Frodo Baggins",
      "role": "ring bearer",
      "links": [
        {
          "rel": "self",
          "href": "http://localhost/employees"
        }
      ]
    }
  ]
}

Making the controller endpoint explicitly produce application/frodo+json has no effect either. However, making the endpoint produce application/hal+json works as expected:

{
  "_embedded": {
    "employees": [
      {
        "name": "Frodo Baggins",
        "role": "ring bearer",
        "_links": {
          "self": {
            "href": "http://localhost/employees"
          }
        }
      }
    ]
  }
}

Is there something fundamental I am missing about the configuration, or is this an actual bug?

@odrotbohm
Copy link
Member

What's the client sending as Accept header? You should definitely see this working when it explicitly requests application/hal+json.

@arhohuttunen
Copy link
Author

arhohuttunen commented Apr 3, 2020

The client is sending application/frodo+json as the Accept header. Basically it's the same as CustomHyperMediaWebMvcTest but with a collection.

I also tried adding Jackson2HalModule into CustomHyperMediaType, which should add the serializers for HAL if I understood correctly, but it didn't seem to help.

Client explicitly requesting for application/hal+json works, but that is not the intent.

edit: I'm actually running this now from CustomHyperMediaWebMvcTest with this result.

@gregturn
Copy link
Contributor

gregturn commented Apr 3, 2020

Can you post your custom media type?

@arhohuttunen
Copy link
Author

arhohuttunen commented Apr 3, 2020

public class CustomHypermediaType implements HypermediaMappingInformation {
    public static final MediaType FRODO_MEDIATYPE = MediaType.parseMediaType("application/frodo+json");

    @Override
    public List<MediaType> getMediaTypes() {
        return Collections.singletonList(FRODO_MEDIATYPE);
    }

    @Override
    public Module getJacksonModule() {
        return new Jackson2HalModule();
    }
}

@gregturn
Copy link
Contributor

gregturn commented Apr 3, 2020

And that class is registered as an @Bean somewhere inside an @Configuration class?

@gregturn
Copy link
Contributor

gregturn commented Apr 3, 2020

The behavior you describe smells like what happens when you request a mediatype that isn't registered.

Spring Framework and it's default converters will bypass HAL and end up with a default handler that responds to application/*+json, which matches applicatio/frodo+json. It then invokes Jackon's "default" rendering, producing what you initially pasted.

Hence, it implies your CustomHypermediaType isn't getting registered in the application context.

@arhohuttunen
Copy link
Author

Sure, the bean is registered right here in https://github.com/spring-projects/spring-hateoas/blob/master/src/test/java/org/springframework/hateoas/config/CustomHypermediaWebMvcTest.java.

I am not running this from my own code. I am running the test code from this repository, with the modifications described above.

@gregturn
Copy link
Contributor

gregturn commented Apr 4, 2020

Okay, I missed that you were tinkering inside our code.

Digging in, I saw what you saw. Which intrigued me. I tried Jackson2UberModule, and Jackson2CollectionJsonModule, and those two worked perfectly. I looked for other test cases that used Jackson2HalModule, and unearthed what was missing.

Basically, you need this:

public class CustomHypermediaType implements HypermediaMappingInformation {

	public static final MediaType FRODO_MEDIATYPE = MediaType.parseMediaType("application/frodo+json");

	/**
	 * {@link MediaType}s this hypermedia can handle.
	 */
	@Override
	public List<MediaType> getMediaTypes() {
		return Collections.singletonList(FRODO_MEDIATYPE);
	}

	/**
	 * Copy the incoming {@link ObjectMapper} and change it's output format along with disabling failure on unknown
	 * properties.
	 */
	@Override
	public ObjectMapper configureObjectMapper(ObjectMapper mapper) {

		mapper.registerModule(getJacksonModule());
		mapper.setHandlerInstantiator(new Jackson2HalModule.HalHandlerInstantiator(new EvoInflectorLinkRelationProvider(),
				CurieProvider.NONE, MessageResolver.DEFAULTS_ONLY));

		mapper.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES);
		mapper.enable(SerializationFeature.INDENT_OUTPUT);

		return mapper;
	}

	@Override
	public Module getJacksonModule() {
		return new Jackson2HalModule();
	}
}

That is the proper way to register HAL support. Once you do that, of course, it breaks the existing test case, because webmvc-frodo.json isn't ACTUAL HAL. It's just unwrapped Jackson.

BTW, the proper way to declare that aggregate root is:

@GetMapping("/employees")
public CollectionModel<EntityModel<Employee>> findMany() {

    List<EntityModel<Employee>> employees = new ArrayList<>();
    employees.add(new EntityModel(new Employee("Frodo Baggins", "ring bearer"),
            linkTo(methodOn(EmployeeController.class).findOne()).withSelfRel()));
    return new CollectionModel(employees, linkTo(methodOn(EmployeeController.class).findMany()).withSelfRel());
}

Cool?

@arhohuttunen
Copy link
Author

Ah, I was basically missing the part with the handler instantiator. I think I need to take a closer look at those handler instantiators, never played with them before.

I actually wrote it a bit differently in the fork to write a separate test case for webmvc-vnd-hal.json, since webmvc-frodo.json is not really HAL. After making the change I can make the test pass.

Thanks for taking the time to check it.

@arhohuttunen
Copy link
Author

Seems like I ran into another problem with @Relation(collectionRelation = "workers") being ignored when using the custom media type. I think I need to dig deeper into implementation of the existing media types to see how these things should be configured.

gregturn added a commit that referenced this issue Apr 5, 2020
By having this custom hypermedia type register ALL the parts of the HAL Jackson module, it provides a better example to users that wish to serve a custom media type that is HAL-like.

Related issues: #1259, #1253
@odrotbohm
Copy link
Member

For reference (originally commented on #1259):

While I think the route via a custom media type registration is a decent workaround, it's not what it was designed for initially and going down that route shouldn't be necessary if all you want to do is register an media type name alias to application/hal+json. As this thread proves, users would have to know all the delicate details of how to properly set up mappers and their moving parts. That's a bit much of the framework falling into one's face for a very simple task to achieve.

The concrete media types a media type implementation is registered under is already obtained via HypermediaMappingInformation.getMediaTypes(). Our HAL support's implementation HalMediaTypeConfiguration currently exposes a hard coded application/hal+json, but could also inspect the HalConfiguration (a bean customizable by the user) it already has available to pick up other media type names. User code could then just list the additional name(s) on that configuration object, and our infrastructure setup would pick that up and make sure the HttpMessageConverter that we register is triggered for that defined alias.

WDYT?

@odrotbohm odrotbohm reopened this Apr 6, 2020
@arhohuttunen
Copy link
Author

That does make a lot of sense since the intent was indeed to just register a media type name alias for application/hal+json. It is a bit involved if you have to know all the details related to mappers and serialization to do so. I have a requirement to use vendor specific media types for API versioning, so there was no way around it.

@sttomm
Copy link

sttomm commented Feb 10, 2021

I am currently running into the same issue, as i also want to apply API versioning via custom media types. Are you planning to integrate the simple solution mentioned by @odrotbohm?

@odrotbohm odrotbohm self-assigned this Feb 10, 2021
@odrotbohm odrotbohm added in: configuration Configuration and setup in: mediatypes Media type related functionality type: enhancement labels Feb 10, 2021
@odrotbohm odrotbohm added this to the 1.3 M2 milestone Feb 10, 2021
odrotbohm added a commit that referenced this issue Feb 10, 2021
HalConfiguration now exposes a withMediaType(…) that adds custom media types in front of the default of `application/hal+json`. This allows developers to use a project specific media type which is treated like it being HAL in the first place.
@odrotbohm
Copy link
Member

This is in place now. Use new HalConfiguration().withMediaType(yourCustomMediaType) to integrate your custom media type.

@filipeamaral
Copy link

filipeamaral commented Jul 29, 2021

@odrotbohm Would it be possible to support this in HalFormsConfiguration as well, or is there any way to propagate the same behaviour from HalConfiguration?

Thank you in advance.

@odrotbohm
Copy link
Member

Sure thing, I've filed and fixed #1591.

@filipeamaral
Copy link

That was quick 😃

Thank you so much! 🙌🏼

@Jelledb
Copy link

Jelledb commented Jan 3, 2022

Seems like I ran into another problem with @Relation(collectionRelation = "workers") being ignored when using the custom media type. I think I need to dig deeper into implementation of the existing media types to see how these things should be configured.

I ran into this same issue and it broke my brain, only to find this thread somewhere in some sketchy part of our code ^^.

We fixed this issue by autowiring whatever LinkRelationProvider is used by HATEOAS, which solved the problem. The LinkRelationProvider of Spring is able to both work with and without the @Relation annotation.

So in your configuration file, make sure to autowire a LinkRelationProvider like this (Kotlin Code)

@Autowired
  private lateinit var linkRelationProvider: LinkRelationProvider

and then replace the EvoInflectorProvider with this linkRelationProvider:

      setHandlerInstantiator(Jackson2HalModule.HalHandlerInstantiator(linkRelationProvider,
          CurieProvider.NONE, MessageResolver.DEFAULTS_ONLY))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: configuration Configuration and setup in: mediatypes Media type related functionality type: enhancement
Projects
None yet
Development

No branches or pull requests

6 participants