Skip to content

Add HAL JSON UTF-8 variant to MediaTypes #470

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
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/main/java/org/springframework/hateoas/MediaTypes.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
*
* @author Oliver Gierke
* @author Przemek Nowak
* @author Drummond Dawson
*/
public class MediaTypes {

Expand All @@ -34,4 +35,14 @@ public class MediaTypes {
* Public constant media type for {@code application/hal+json}.
*/
public static final MediaType HAL_JSON = MediaType.valueOf(HAL_JSON_VALUE);

/**
* A String equivalent of {@link MediaTypes#HAL_JSON_UTF8}.
*/
public static final String HAL_JSON_UTF8_VALUE = HAL_JSON_VALUE + ";charset=UTF-8";

/**
* Public constant media type for {@code application/hal+json;charset=UTF-8}.
*/
public static final MediaType HAL_JSON_UTF8 = MediaType.valueOf(HAL_JSON_UTF8_VALUE);
}
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ public static List<HttpMessageConverter<?>> getDefaultMessageConverters(List<Med
List<HttpMessageConverter<?>> converters = new ArrayList<HttpMessageConverter<?>>();
converters.add(new StringHttpMessageConverter(Charset.forName("UTF-8")));

if (mediaTypes.contains(MediaTypes.HAL_JSON)) {
if (mediaTypes.contains(MediaTypes.HAL_JSON) || mediaTypes.contains(MediaTypes.HAL_JSON_UTF8)) {
converters.add(getHalConverter());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ private List<HttpMessageConverter<?>> potentiallyRegisterModule(List<HttpMessage

MappingJackson2HttpMessageConverter halConverter = new TypeConstrainedMappingJackson2HttpMessageConverter(
ResourceSupport.class);
halConverter.setSupportedMediaTypes(Arrays.asList(HAL_JSON));
halConverter.setSupportedMediaTypes(Arrays.asList(HAL_JSON, HAL_JSON_UTF8));
halConverter.setObjectMapper(halObjectMapper);

List<HttpMessageConverter<?>> result = new ArrayList<HttpMessageConverter<?>>(converters.size());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,23 +67,22 @@ public class JsonPathLinkDiscoverer implements LinkDiscoverer {
}

private final String pathTemplate;
private final MediaType mediaType;
private final List<MediaType> mediaTypes;

/**
* Creates a new {@link JsonPathLinkDiscoverer} using the given path template supporting the given {@link MediaType}.
* The template has to contain a single {@code %s} placeholder which will be replaced by the relation type.
*
* @param pathTemplate must not be {@literal null} or empty and contain a single placeholder.
* @param mediaType the {@link MediaType} to support.
* @param mediaTypes the {@link MediaType}s to support.
*/
public JsonPathLinkDiscoverer(String pathTemplate, MediaType mediaType) {

public JsonPathLinkDiscoverer(String pathTemplate, MediaType... mediaTypes) {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wasn't quite sure what to do with this constructor. It seems that the MediaType is nullable so I made it varargs. I thought about another constructor with a List of MediaType...

Assert.hasText(pathTemplate, "Path template must not be null!");
Assert.isTrue(StringUtils.countOccurrencesOf(pathTemplate, "%s") == 1,
"Path template must contain a single placeholder!");

this.pathTemplate = pathTemplate;
this.mediaType = mediaType;
this.mediaTypes = Arrays.asList(mediaTypes);
}

/*
Expand Down Expand Up @@ -178,6 +177,16 @@ private List<Link> createLinksFrom(Object parseResult, String rel) {
*/
@Override
public boolean supports(MediaType delimiter) {
return this.mediaType == null ? true : this.mediaType.isCompatibleWith(delimiter);
boolean supports = false;
if (this.mediaTypes == null) {
return true;
} else {
for (MediaType mediaType : this.mediaTypes) {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wasn't sure what to do here either with the List of MediaType and isCompatibleWith

if (mediaType.isCompatibleWith(delimiter)) {
supports = true;
}
}
}
return supports;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
import org.springframework.hateoas.MediaTypes;
import org.springframework.hateoas.core.JsonPathLinkDiscoverer;

import java.util.Arrays;

/**
* {@link LinkDiscoverer} implementation based on HAL link structure.
*
Expand All @@ -27,6 +29,6 @@
public class HalLinkDiscoverer extends JsonPathLinkDiscoverer {

public HalLinkDiscoverer() {
super("$._links..['%s']..href", MediaTypes.HAL_JSON);
super("$._links..['%s']..href", MediaTypes.HAL_JSON, MediaTypes.HAL_JSON_UTF8);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,12 @@ public void returnsDefaultMessageConvertersForHal() {
assertThat(converters, hasSize(2));
assertThat(converters.get(0), is(instanceOf(StringHttpMessageConverter.class)));
assertThat(converters.get(1), is(instanceOf(MappingJackson2HttpMessageConverter.class)));

converters = Traverson.getDefaultMessageConverters(MediaTypes.HAL_JSON_UTF8);

assertThat(converters, hasSize(2));
assertThat(converters.get(0), is(instanceOf(StringHttpMessageConverter.class)));
assertThat(converters.get(1), is(instanceOf(MappingJackson2HttpMessageConverter.class)));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ public void registersLinkDiscoverers() {

assertThat(discoverers, is(notNullValue()));
assertThat(discoverers.getLinkDiscovererFor(MediaTypes.HAL_JSON), is(instanceOf(HalLinkDiscoverer.class)));
assertThat(discoverers.getLinkDiscovererFor(MediaTypes.HAL_JSON_UTF8), is(instanceOf(HalLinkDiscoverer.class)));
assertRelProvidersSetUp(context);
}

Expand All @@ -99,6 +100,7 @@ public void halSetupIsAppliedToAllTransitiveComponentsInRequestMappingHandlerAda
RequestMappingHandlerAdapter adapter = context.getBean(RequestMappingHandlerAdapter.class);

assertThat(adapter.getMessageConverters().get(0).getSupportedMediaTypes(), hasItem(MediaTypes.HAL_JSON));
assertThat(adapter.getMessageConverters().get(0).getSupportedMediaTypes(), hasItem(MediaTypes.HAL_JSON_UTF8));

boolean found = false;

Expand All @@ -114,6 +116,7 @@ public void halSetupIsAppliedToAllTransitiveComponentsInRequestMappingHandlerAda

assertThat(converters.get(0), is(instanceOf(TypeConstrainedMappingJackson2HttpMessageConverter.class)));
assertThat(converters.get(0).getSupportedMediaTypes(), hasItem(MediaTypes.HAL_JSON));
assertThat(converters.get(0).getSupportedMediaTypes(), hasItem(MediaTypes.HAL_JSON_UTF8));
}
}

Expand All @@ -130,6 +133,7 @@ public void registersHttpMessageConvertersForRestTemplate() {
RestTemplate template = context.getBean(RestTemplate.class);

assertThat(template.getMessageConverters().get(0).getSupportedMediaTypes(), hasItem(MediaTypes.HAL_JSON));
assertThat(template.getMessageConverters().get(0).getSupportedMediaTypes(), hasItem(MediaTypes.HAL_JSON_UTF8));
context.close();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,16 @@ public class JsonPathLinkDiscovererUnitTest {

@Test(expected = IllegalArgumentException.class)
public void rejectsNullPattern() {
new JsonPathLinkDiscoverer(null, null);
new JsonPathLinkDiscoverer(null);
}

@Test(expected = IllegalArgumentException.class)
public void rejectsPatternWithWithoutPlaceholder() {
new JsonPathLinkDiscoverer("$links", null);
new JsonPathLinkDiscoverer("$links");
}

@Test(expected = IllegalArgumentException.class)
public void rejectsPatternWithMultiplePlaceholders() {
new JsonPathLinkDiscoverer("$links%s%s", null);
new JsonPathLinkDiscoverer("$links%s%s");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import static org.hamcrest.Matchers.*;
import static org.junit.Assert.*;
import static org.springframework.test.web.client.ExpectedCount.times;
import static org.springframework.test.web.client.MockRestServiceServer.*;
import static org.springframework.test.web.client.match.MockRestRequestMatchers.*;
import static org.springframework.test.web.client.response.MockRestResponseCreators.*;
Expand Down Expand Up @@ -82,12 +83,17 @@ public void setUp() {
@Test
public void usesResourceTypeReference() {

server.expect(requestTo("/resource")).andRespond(withSuccess(RESOURCE, MediaTypes.HAL_JSON));
server.expect(times(1), requestTo("/resource")).andRespond(withSuccess(RESOURCE, MediaTypes.HAL_JSON));
server.expect(times(1), requestTo("/resource")).andRespond(withSuccess(RESOURCE, MediaTypes.HAL_JSON_UTF8));
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicated server expect with MediaTypes.HAL_JSON_UTF8. Not sure the preferred way to test the MediaTypes.HAL_JSON in addition to MediaTypes.HAL_JSON_UTF8


ResponseEntity<Resource<User>> response = template.exchange("/resource", HttpMethod.GET, null,
new ResourceType<User>() {});

assertExpectedUserResource(response.getBody());

response = template.exchange("/resource", HttpMethod.GET, null, new ResourceType<User>() {});

assertExpectedUserResource(response.getBody());
}

/**
Expand All @@ -96,7 +102,8 @@ public void usesResourceTypeReference() {
@Test
public void usesResourcesTypeReference() {

server.expect(requestTo("/resources")).andRespond(withSuccess(RESOURCES_OF_USER, MediaTypes.HAL_JSON));
server.expect(times(1), requestTo("/resources")).andRespond(withSuccess(RESOURCES_OF_USER, MediaTypes.HAL_JSON));
server.expect(times(1), requestTo("/resources")).andRespond(withSuccess(RESOURCES_OF_USER, MediaTypes.HAL_JSON_UTF8));

ResponseEntity<Resources<User>> response = template.exchange("/resources", HttpMethod.GET, null,
new ResourcesType<User>() {});
Expand All @@ -108,6 +115,16 @@ public void usesResourcesTypeReference() {

assertThat(nested, hasSize(1));
assertExpectedUser(nested.iterator().next());

response = template.exchange("/resources", HttpMethod.GET, null, new ResourcesType<User>() {});
body = response.getBody();

assertThat(body.hasLink("self"), is(true));

nested = body.getContent();

assertThat(nested, hasSize(1));
assertExpectedUser(nested.iterator().next());
}

/**
Expand All @@ -116,7 +133,8 @@ public void usesResourcesTypeReference() {
@Test
public void usesResourcesOfResourceTypeReference() {

server.expect(requestTo("/resources")).andRespond(withSuccess(RESOURCES_OF_RESOURCE, MediaTypes.HAL_JSON));
server.expect(times(1), requestTo("/resources")).andRespond(withSuccess(RESOURCES_OF_RESOURCE, MediaTypes.HAL_JSON));
server.expect(times(1), requestTo("/resources")).andRespond(withSuccess(RESOURCES_OF_RESOURCE, MediaTypes.HAL_JSON_UTF8));

ResponseEntity<Resources<Resource<User>>> response = template.exchange("/resources", HttpMethod.GET, null,
new ResourcesType<Resource<User>>() {});
Expand All @@ -128,6 +146,16 @@ public void usesResourcesOfResourceTypeReference() {

assertThat(nested, hasSize(1));
assertExpectedUserResource(nested.iterator().next());

response = template.exchange("/resources", HttpMethod.GET, null, new ResourcesType<Resource<User>>() {});
body = response.getBody();

assertThat(body.hasLink("self"), is(true));

nested = body.getContent();

assertThat(nested, hasSize(1));
assertExpectedUserResource(nested.iterator().next());
}

private static void assertExpectedUserResource(Resource<User> user) {
Expand Down