spring-data-rest icon indicating copy to clipboard operation
spring-data-rest copied to clipboard

Content Negotation to use the registered HttpMessageConverters [DATAREST-1235]

Open spring-projects-issues opened this issue 7 years ago • 10 comments

Petar Tahchiev opened DATAREST-1235 and commented

Hello,

I want to get the response from Spring Data REST in XML or CSV or some other format. For that reason Spring Data REST allows me to register some extra HttpMessageConverters  in my RepositoryRestConfigurerAdapter like this:

@Override
public void configureHttpMessageConverters(List<HttpMessageConverter<?>> messageConverters) {
  super.configureHttpMessageConverters(messageConverters);

 messageConverters.add(new MappingJackson2XmlHttpMessageConverter());
 messageConverters.add(new MappingJackson2CborHttpMessageConverter());
 messageConverters.add(new MappingJackson2SmileHttpMessageConverter());
 messageConverters.add(new CsvHttpMessageConverter());
}

but seems like they are not used at all.

However even though I specify correct Accept header I get 406 (Not Acceptable).

I found out that RepositoryRestHandlerMapping sets some fallback media types if the RepositoryEntityController has not specified what the methods produce.

If I comment out the 2 lines (just like I have done in this pull request):

https://github.com/spring-projects/spring-data-rest/pull/292/files

then it all seems to work fine - I can get XML/CSV or whatever I specify in the Accept header. Also if I don't specify Accept header I get the JSON back. So i'm not sure what the two lines are for, but to me it seems they are not needed.

 


Affects: 3.0.6 (Kay SR6)

spring-projects-issues avatar Apr 23 '18 17:04 spring-projects-issues

Petar Tahchiev commented

Any feedback here? How can I get different mediaType that jSON from Spring-data repository?

spring-projects-issues avatar May 28 '18 13:05 spring-projects-issues

Petar Tahchiev commented

Is there any guide on how to achieve this? I'm using the latest KAY snapshot, tried adding both in my RepositoryRestConfigurerAdapter:

    @Override
    public void configureHttpMessageConverters(List<HttpMessageConverter<?>> messageConverters) {
        super.configureHttpMessageConverters(messageConverters);

        messageConverters.add(new MappingJackson2XmlHttpMessageConverter());
        messageConverters.add(new MappingJackson2CborHttpMessageConverter());
        messageConverters.add(new MappingJackson2SmileHttpMessageConverter());
        //messageConverters.add(new CsvHttpMessageConverter(uiRepresentationService, csvMapper));
    }

and in my RepositoryRestMvcConfiguration:

    @Override
    public void configureMessageConverters(List<HttpMessageConverter<?>> converters) {
        super.configureMessageConverters(converters);

        converters.add(new MappingJackson2XmlHttpMessageConverter());
        converters.add(new MappingJackson2CborHttpMessageConverter());
        converters.add(new MappingJackson2SmileHttpMessageConverter());
        //converters.add(new CsvHttpMessageConverter());
    }

and I still get 406 when I try to curl for content-type xml:

curl -k -v -H "Accept:application/xml" https://solar.local:8112/storefront/rest/
< HTTP/1.1 406 
< X-Nemesis-Token: 8922159a-10ed-4079-8ebf-7c9b608fd5a7
< Set-Cookie: REST_SESSION=ODkyMjE1OWEtMTBlZC00MDc5LThlYmYtN2M5YjYwOGZkNWE3; Path=/storefront/rest; Secure; HttpOnly
< Content-Length: 0
< Date: Sat, 21 Jul 2018 06:25:31 GMT

spring-projects-issues avatar Jul 21 '18 06:07 spring-projects-issues

Petar Tahchiev commented

This issue is related to: https://jira.spring.io/browse/DATAREST-59

spring-projects-issues avatar Jul 28 '18 06:07 spring-projects-issues

Petar Tahchiev commented

I added a pull-request here: https://github.com/spring-projects/spring-data-rest/pull/301

However with my changes now there's one test failing:

Failed tests: 
  GemfireWebTests>CommonWebTests.rejectsUnsupportedAcceptTypeForResources:253 Status expected:<406> but was:<200>

Looking at this test:

	@Test // DATAREST-1003
	public void rejectsUnsupportedAcceptTypeForResources() throws Exception {

		for (String string : expectedRootLinkRels()) {

			Link link = client.discoverUnique(string);

			mvc.perform(get(link.expand().getHref())//
					.accept(MediaType.valueOf("application/schema+json")))//
					.andExpect(status().isNotAcceptable());
		}
	}

I see you expect that requests with Accept header application/schema+json are expected to return 406. Why is this? Looking at this issue https://jira.spring.io/browse/DATAREST-1003 it is supposed to fail of the ObjectMapper is not configured but I debugged the test and I think it is configured correctly. This makes me think that the test is wrong. Can you please have a look

spring-projects-issues avatar Jul 28 '18 10:07 spring-projects-issues

Mark Paluch commented

Thanks a lot for submitting a pull request. We'll take a look as soon as it's possible for us to do so.

I've edited your comment to remove "guys". While it may seem a small thing, some people feel excluded by "guys" and we don't want them to

spring-projects-issues avatar Jul 28 '18 19:07 spring-projects-issues

It seems that this is still an issue. Are SDR repositories meant to support json only?

One can "workaround" the issue by calling RepositoryRestConfiguration.setDefaultMediaType(someNonJsonMediaType), but then only a single extra mediatype is supported and support for application/hal+json accept headers goes away.

Ickbinet avatar Nov 25 '21 13:11 Ickbinet

It is still an issue as it's not as easy as plugging different HttpMessageConverters into the processing chain, as the way that the representation models currently get prepared (regarding embedding etc.) doesn't necessarily translate into other media types out of the box. To some degree they might even do, it's just that we don't want to allow users to simply activate new media types without having checked that the exposed HTTP resources actually produce responses that adhere to how they're specified to work. This is primarily the case as most media types require the explicit registration of affordances, and it's not immediately clear whether those can be registered in a way that works for different media types.

It's currently not clear, which media types might be interesting to consider for a more profound investigation, and we simply don't have enough time to verify all of them. If you fancy to play with support for that, I'd suggest trying to start with HAL-FORMS, which has already got general support activated in current releases of Spring Data REST but would need to get the corresponding affordances added in the Repository…Controller methods.

odrotbohm avatar Nov 25 '21 15:11 odrotbohm

We use HAL since many years via SDR repositories as our native format and are really happy with it! Now we also use HAL-FORMS and we are now more happy :) But... Customers often complain about the huge amount of data which these formats (especially forms) produce. It doesn't matter if the overhead is really relevant or not, they complain. This is sometimes very problematic with new customers / customer aquisition. Also sometimes customers prefer XML, which we solved so far via a proxy which 1:1 converts json to xml.

So we tried HypermediaMappingInformation which works perfect, but not for SDR. OK, fallback to HttpMessageConverters. Does not really work too :(

So we now fall back to our query interface (a @RepositoryRestController) to support custom formats like compact json, XML or CSV. But it would be really nice if all SDR endpoints would support it. We could hack it now via RepositoryRestConfiguration.setDefaultMediaType, but I guess this is not the way it was intended for.

I also do not understand what you mean with "we don't want to allow users to simply activate new media types without having checked that the exposed HTTP resources actually produce responses that adhere to how they're specified to work". As long as the Converter supports the HATEOAS types (EntityModel, CollectionModel, PagedModel etc) it should be fine. If not they can handle it in the supports, canRead, canWrite methods of the converter.

We have now CSV and "compact-json" as converters and they seem to work as expected.

Ickbinet avatar Nov 25 '21 20:11 Ickbinet

As long as the Converter supports the HATEOAS types (EntityModel, CollectionModel, PagedModel etc) it should be fine. If not they can handle it in the supports, canRead, canWrite methods of the converter.

I wish it were that simple. Most people look at Spring Data REST and like it because they easily get HTTP resources for their repositories. The primary feature of it though is not the actual exposure of the resources but the derivation of the representation boundaries based on the aggregate structure expressed in the domain model. This includes using HAL's embedding mechanism, applying projections to those values embedded. Properly handling the embedded values if a HAL representation is posted back to the servers.

Without proper testing, we cannot make sure that simply letting users activate a new media type (Collection JSON, Siren etc.) would result in them seeing useful, correct output that lets all exposed HTTP resources work as advertised. I am not even saying it wouldn't work for some media types that are different flavors of "presenting data". I just can't open a door ("You can now simply activate new media types") and then risk having to run after folks showing left and right reporting that "this doesn't work with Siren", "that doesn't work with Collection JSON" and "Oh, this doesn't work with my customly implemented media type.".

For some media types that include forms, this wouldn't work by definition, as our controllers currently don't add any affordances at all. I can imagine proceeding with that to see how that'd work out with HAL FORMS, add tests about the now expected representation and at some point adding yet another media type and see what that results in. That said, that's not a half-a-day effort.

odrotbohm avatar Nov 26 '21 09:11 odrotbohm

HAL-FORMS for SDR repositories would be great! :) Better than the ALPS profiles now.

Ickbinet avatar Nov 26 '21 14:11 Ickbinet