HalBuilder / halbuilder-core

HalBuilder Core
38 stars 26 forks source link

MessageBodyWriter and MessageBodyReader for HalBuilder 5.0 #37

Closed mg-2014 closed 7 years ago

mg-2014 commented 7 years ago

Hi,

This feature request applies to v5.0.1, 5.0.2-SNAPSHOT (and the current "develop" branch).

Since halbuilder5 uses a different representation object, we cannot use halbuilder-jaxrs. Instead I used a MessageBodyWriter pretty similar to the one from halbuilder-jaxrs. Since JSON reading and writing is included in halbuilder5 it might be a good idea to include the JAX-RS related stuff (MessageBodyReader and MessageBodyWriter) as well.

@Provider
@Produces( { "application/hal+json", "application/hal+xml" } )
public class JaxRsHalBuilderSupport< T > implements MessageBodyWriter< T >
{
   @Context
   private Providers providers;

   @Override
   public boolean isWriteable( Class< ? > type, Type genericType, Annotation[] annotations, MediaType mediaType )
   {
      return ResourceRepresentation.class.isAssignableFrom( type ) && HalBuilderMediaTypes.isSupported( mediaType );
   }

   @Override
   public long getSize( T t, Class< ? > type, Type genericType, Annotation[] annotations, MediaType mediaType )
   {
      return -1;
   }

   @Override
   public void writeTo( T t, Class< ? > type, Type genericType, Annotation[] annotations, MediaType mediaType, MultivaluedMap< String, Object > httpHeaders, OutputStream entityStream ) throws IOException, WebApplicationException
   {
      ResourceRepresentation< ? > representation = (ResourceRepresentation< ? >) t;

      if( mediaType.isCompatible( HalBuilderMediaTypes.HAL_JSON_TYPE ) ) {
         JsonRepresentationWriter writer = Optional
            .ofNullable(providers.getContextResolver( ObjectMapper.class, HalBuilderMediaTypes.HAL_JSON_TYPE ) )
            .map( resolver -> resolver.getContext( ObjectMapper.class ) )
            .map( JsonRepresentationWriter::create )
            .orElseGet( JsonRepresentationWriter::create );
         writer.write( representation, new OutputStreamWriter( entityStream, HalBuilderMediaTypes.DEFAULT_ENCODING ) );
      }
      else if( mediaType.isCompatible( HalBuilderMediaTypes.HAL_XML_TYPE ) ) {
         //TODO Provide a writer for HAL+XML.
         throw new RuntimeException( "No writer available for media type '" + mediaType + "'" );
      }
      else {
         throw new RuntimeException( "No writer available for media type '" + mediaType + "'" );
      }
   }
}

We don't currently need HAL+XML. Thus we throw an exception here. Also I don't have a MessageBodyReader implementation for ResourceRepresentation yet.

Regards, Markus

mg-2014 commented 7 years ago

A MessageBodyReader that works with an already existing ObjectMapper provided by the JAX-RS ContextResolver would need a factory method to inject the ObjectMapper (see #38).

talios commented 7 years ago

halbuilder-jaxrs was never really maintained ( I don't use Jax-rs ), currently neither is HAL+XML ( altho I do have a version of that working for HalBuilder 5 - I had considered moving that into the new single artifact but opted against it ).

Now that I've actually made that leap to a release of 5.x the other should be updated.

mg-2014 commented 7 years ago

The same package that contains the JAX-RS classes should have some public static constants like:

public static final String HAL_JSON = "application/hal+json";
public static final MediaType HAL_JSON_TYPE = MediaType.valueOf( HAL_JSON );

public static final String HAL_XML = "application/hal+xml";
public static final MediaType HAL_XML_TYPE = MediaType.valueOf( HAL_XML );

public static final Charset DEFAULT_ENCODING = Charsets.UTF_8;

In HalBuilder 4.0 the String constants were available via RepresentationFactory of halbuilder-api while the MediaType constants had only package level visibility in class HalBuilderMediaTypes. It's always a good idea when the developer is able to use these constants. At least the String constants should be available. But JAX-RS makes the MediaType constants available in the MediaType class as well. I found these to be quite useful to create own media type constants with additional properties, e.g.

public static final MediaType HAL_JSON_UTF8_TYPE = HAL_JSON_TYPE.withCharset( "UTF_8" );
talios commented 7 years ago

Over on the halbuilder-jaxrs repo, I've added an initial spike at reworking it for HalBuilder 5 ( https://github.com/HalBuilder/halbuilder-jaxrs/commit/0eb4740390bcdd34a80913d1bc7a375cfe453933 ) but all the tests are failing with ClassCastExceptions - I don't know anything about JaxRS so at a bit of a loss at the moment without doing some research on how JaxRS works internally.

Sounds like you know more about the internals - any suggestions?

mg-2014 commented 7 years ago

I might be able to help. Let me have a look at it.

mg-2014 commented 7 years ago

The problem seems to be the use of generics with MessageBodyReader. If you remove the generics, i.e. implement

public class JaxRsHalBuilderSupport implements MessageBodyWriter<ResourceRepresentation<?>>, MessageBodyReader<ResourceRepresentation<?>> {

instead of

public class JaxRsHalBuilderSupport<T extends ResourceRepresentation<?>> implements MessageBodyWriter<T>, MessageBodyReader<T> {

and return

return reader.read(new InputStreamReader(entityStream, Support.DEFAULT_ENCODING));

instead of

return (T) reader.read(new InputStreamReader(entityStream, Support.DEFAULT_ENCODING), aClass);

in readFrom() it seems to work.

There are still failing tests though:

[ERROR]   JaxRsHalBuilderRoundtripTest.embeddedShouldHaveProperPropertyValue:112 expected:<some other value> but was:<null>
[ERROR]   JaxRsHalBuilderRoundtripTest.embeddedShouldHaveProperSelfLink:104 expected:<http://example.org/[extra]> but was:<http://example.org/[]>
[ERROR]   JaxRsHalBuilderRoundtripTest.shouldHaveProperExtraLink:87 expected:<http://example.org/[extra]> but was:<http://example.org/[]>
[ERROR]   JaxRsHalBuilderRoundtripTest.shouldHaveProperPropertyValue:94 expected:<some value> but was:<null>
[ERROR]   ObjectMapperViaContextResolverTest.shouldUseProvidedObjectMapper:58 
Expected: is "{\"_links\":{\"self\":{\"href\":\"http://example.org/\"}},\"alphabet\":\"ABCDEFGHIJKLMNOPQRSTUVWXYZ\"}"
     but: was "{\n  \"alphabet\" : \"ABCDEFGHIJKLMNOPQRSTUVWXYZ\",\n  \"_links\" : {\n    \"self\" : {\n      \"href\" : \"http://example.org/\"\n    }\n  }\n}"

and there is still a parametrized version for the HAL+XML variant in the tests.

The string comparison in the last one is tricky because of whitespaces and re-ordering. It might be a better approach to compare JsonNode objects, e.g.

   @Test
   public void getProducesExpectedJsonString() throws IOException
   {
      Response response = target( "/" ).request().get();
      assertThat( response.getStatus(), is( Status.OK.getStatusCode() ) );

      JsonNode actual = response.readEntity( JsonNode.class );
      JsonNode expected = new ObjectMapper()
         .readTree( new String( "{"
            + "'_links':{'foo':{'href':'/foo'}},"
            + "'_embedded':{'foo':{'_links':{'self':{'href':'/foo'}},'bar':1}}"
            + "}" ).replace( '\'', '"' ) );

      assertThat( actual, is( expected ) );
   }
mg-2014 commented 7 years ago

I created a pull request on the halbuilder-jaxrs repository that fixes the issues with generics and test cases. I added support for HAL+JSON only and removed any occurrence of HAL+XML.

Also I added some dependencies to make the dependency analyzer pass. But I had to add a plugin management section to ignore one of the "declared but unused" dependencies since byte code analysis won't find that it is actually used. And I set the version of Jersey to the latest non-beta version 2.25.1. and JAX-RS API to 2.0 because of:

Please note that JAX-RS 2.1 is not final yet. JAX-RS 2.1 might change, there is no backwards compatibility requirement to keep among 2.26-bXX builds. Use at your own risk. (https://jersey.github.io/download.html)

talios commented 7 years ago

Released in halbuilder-jaxrs:5.1.1