HalBuilder / halbuilder-core

HalBuilder Core
38 stars 26 forks source link

Add static factory method for JsonRepresentationReader mentioned in documentation. #38

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).

The documentation mentions a static method

JsonRepresentationReader.create(ObjectMapper objectMapper)

However this method does not exist. Currently there is a constructor that creates a new ObjectMapper:

public JsonRepresentationReader() {
  this.mapper = new ObjectMapper();
}

A static method that can be used to inject the ObjectMapper provided by the JAX-RS ContextResolver would be quite helpful. Also making the constructor private and instead adding the static factory methods

JsonRepresentationReader.create(ObjectMapper objectMapper)

and

JsonRepresentationReader.create(Module... modules)

would make JsonRepresenationReader very much consistent with JsonRepresentationWriter.

Regards, Markus

talios commented 7 years ago

Sounds good - it's funny, very little feedback on the SNAPSHOTs until a release - always the way. All of these look small and minor and should be easily done this week.

mg-2014 commented 7 years ago

Excellent.

it's funny, very little feedback on the SNAPSHOTs until a release - always the way. All of these look small and minor and should be easily done this week.

One of the problems with SNAPSHOTs is that repository managers are usually configured to not allow snapshots from Maven Central (or any public repository). So I couldn't use the 5.0.1-SNAPSHOT.

We've been using HalBuilder 4.x for a while. But so far we've only had pretty simple HAL+JSON payloads. Last week we needed to model some more complex stuff. And I've found a couple of issues with v4.x, e.g. the @JsonIgnore mentioned in another ticket or links other than self that are single element arrays instead of an object. So I tried HalBuilder 5.0.1-SNAPSHOT. And once you start working with a library you will stumble upon these issues.

IMO it's a good idea to integrate the entire feature set of the many artifacts into a single halbuilder5 artifact. Not only do we need only a single artifact now but we don't need to find the (different) versions, e.g. halbuilder-core and -json 4.0.6 but halbuilder-standard 4.0.1 and halbuilder-jaxrs 2.0.1.

Thanks a lot for the effort you put into halbuilder. It's a great and lightweight solution for building HAL+JSON payloads. Sure, there is also spring-hateoas but we don't want to use any Spring stuff.

talios commented 7 years ago

One of the reasons for modularity tho is being able to fix pieces individually, jaxrs is of little concern to me so doesn't really need to be in core. Likewise the XML variant, theres no need to add in XML dependencies if the primary use case doesn't include it.

The great thing about how I've restructured things however, is theres less stateful coupling between pieces ( i.e. none ) - so things like halbuilder-standard isn't even required anymore.