Make mixins public to make re-use of Jackson2HalModule easier

[jstaffans]

I would like to be able to use my own implementation of Jackson2HalModule but re-use the existing mixin classes found in the org.springframework.hateoas.hal package. One mixin was already declared public (the ResourcesMixin class). This PR just gives the two other mixins the same visibility.

[jtheuer]

+1
Would be great if this simple PR got merged!

[Berastau]

+1

[jtheuer]

@olivergierke any concerns about this PR? Is it not the intended way to use the mixins? Anything we can do to get it merged sooner? Thanks!

[gregturn]

@jstaffans Exactly how are you running into a visibility issue? Are you trying to use the MixIns outside of org.springframework.hateoas.hal?

[jstaffans]

@gregturn yes, something like that. I was trying to customise the way links and embeds were serialised. I'm however no longer working on the project for which I needed the customisation and am a bit hazy on the details.

If nothing else though, the changes in this PR make the mixin classes consistent in their visibility (the ResourceSupportMixin class is already public).

[gregturn]

Thanks for getting back to me.

[gregturn]

Resolved via 135aca9

[odrotbohm]

Can we take a step back and reconsider? "Something like that" is not a very good justification to make things public that don't need to be public. I'd love to learn how "trying to customize the way links and embeds" makes sense in a HAL context, as it defines the format pretty precisely.

[odrotbohm]

Actually, ResourcesMixin being public seems to be an oversight rather than something we should align with. So I'd rather reduce its visibility to package protected than the other way round.

Happy to reconsider if we learn about a use case in detail.

[gregturn]

I didn't agree with extensibility but I did agree with consistency. I was just picking the option that had 0% chance of breaking backward compatibility.

I can make them all package protected D that's preferred.

[gregturn]

See #549

[odrotbohm]

I'd vote for resetting master to before the commit and waiting for further feedback. Feels like @jstaffans indeed has a use case, I'd just love to learn more about it before opting into maintaining more public API 🙂.

[Berastau]

@olivergierke "...as it defines the format pretty precisely..." - not exactly, for instance look at the issue with the single item in lists - #288. The specification says that _list is

either a Link Object or an array of Link Objects

Which is ambiguous in case of the single-item array. The latter is the reason I voted +1 for this PR since I would be able to change current behavior (such array represented as a single item) and fit it for our project needs (represented as a 1-item array).

[odrotbohm]

We already expose a boolean flag to control whether to prefer collection relations (defaulting to true). A lot of the actual handling of that is in HalEmbeddedBuilder, which makes me think that just arbitrarily making the mixins public feels like changing things for the sake of changing things so that you of course can do the heavy lifting yourself but on a very low abstraction level. I wonder whether we couldn't rather use that abstractions we already have in place and change them so that you can actually implement what you want on that level of abstraction.

Actually that's the reason I'd like to learn about the actual use case. What are you trying to do exactly? What is the representation you see? What do you want to see? What do you think prevents you from achieving the latter. "I need the types to be public" is way too much thinking in implementation, i.e. the solution space. "I don't have fine grained enough control over whether a single link shall be rendered as array or not" is thinking in the problem space. We need to do the latter before we do the former. 🙂

[Berastau]

@olivergierke We already expose a boolean flag to control whether to prefer collection relations... and which is being completely ignored here: https://github.com/spring-projects/spring-hateoas/blob/master/src/main/java/org/springframework/hateoas/hal/Jackson2HalModule.java#L445
hence the #288
I agree with you that the contract you give us should be either fully open or fully closed. There's no sense in making public a part that can't be used without another marked as private. I guess the difference here is responsibility - if you restrict it then you're responsible for providing flexible enough api that would allow anyone achieve what they want. If you open it (public) - you like saying 'I don't know your exact use case but here's the set of tools you can use to achieve your goals'.

Personally I would prefer having you guys closed this one (with ResourcesMixin made package/protected) and provided a solution for the #288 - because 288 is exactly (and you are absolutely right here) what I'm trying to have resolved in the first place, and this request (492) was just one of the ways to achieve that.
But I also agree that there may be another valid justification for the public mixins from others.

[jstaffans]

My use case was serialising an empty collection as an empty JSON array instead of leaving out the embed completely. As far as I can recall, this would have required a modification of Jackson2HalModule (I might be wrong though, this PR was just the end result of some frustrated hacking and I no longer have access to the code of the project where this was a relevant issue).

"_embedded": {
        "users": []
}

The HAL specification is pretty light on details for cases like this, I think.

I agree with the comment above by @Berastau about it being a matter of either designing for flexibility through configuration or extensibility.

[elnur]

I'm running into the same issue with _embedded.someList being completely omitted when the list is empty. The suggested workaround is ugly because it makes me change the return type from Resources<Some> to Resources<Object> or Resources<?>.

[pivotal-issuemaster]

@jstaffans Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

[pivotal-issuemaster]

@jstaffans Thank you for signing the Contributor License Agreement!

[gregturn]

We currently have HalConfiguration, which provides the option to render a single link as a single item or as a 1-item array.

It's not outrageous to extend this to ALSO define an option for rendering empty stuff.

The code inside Jackson2HalModule.OptionalListJackson2Serializer currently reads:

public void serialize(Object value, JsonGenerator jgen, SerializerProvider provider) throws IOException {

	List<?> list = (List<?>) value;

	if (list.isEmpty()) {
		return;
	}

	if (list.size() == 1 && this.halConfiguration.getRenderSingleLinks() == RenderSingleLinks.AS_SINGLE) {
		serializeContents(list.iterator(), jgen, provider);
			return;
	}

	jgen.writeStartArray();
	serializeContents(list.iterator(), jgen, provider);
	jgen.writeEndArray();
}

It wouldn't be hard to add another setting for RenderEmptyLinks.AS_ARRAY and RenderEmptyLinks.NONE.

[gregturn]

I have poked at this, and don't really see a path forward.

@Test // #492
void renderEmbeddedWithEmptyCollection() throws JsonProcessingException {

	mapper = HalTestUtils.halObjectMapper(new HalConfiguration().withRenderEmptyCollections(true));

	CollectionModel<?> model = CollectionModel.empty();
	model.add(Link.of("http://example.com"));

	String serialized = mapper.writeValueAsString(model);

	assertThat(serialized).isEqualTo("{\"_embedded\":{},\"_links\":{\"self\":{\"href\":\"http://example.com\"}}}");
}

This prototype only gets you an empty collection, not a link relation with an empty list.

The only thing that appears to work, is to use HAL directly, like this:

@Test // #492
void renderEmbeddedWithEmptyLinkRelation() throws JsonProcessingException {

	CollectionModel<?> model = CollectionModel
			.of(Arrays.asList(new EmbeddedWrappers(false).emptyCollectionOf(SomeSample.class)));
	model.add(Link.of("http://example.com"));

	String serialized = mapper.writeValueAsString(model);

	assertThat(serialized)
			.isEqualTo("{\"_embedded\":{\"someSample\":[]},\"_links\":{\"self\":{\"href\":\"http://example.com\"}}}");
}

I don't see how this can be picked up and generalized for the vendor-neutral API.

Essentially, what you are asking HAL to do in the event of an empty collection, is glean what the generic type is without there being any data there.

Java doesn't make it easy to find a generic parameter unless something is there. Unless you manually create a list with the type in it. Framework code doesn't lend itself to this sort of general behavior.

Suggestions welcome!