From 03abd3f1691009c62fc43af1691a52b38922d79d Mon Sep 17 00:00:00 2001 From: Oliver Drotbohm Date: Tue, 27 Oct 2020 20:31:51 +0100 Subject: [PATCH] #1382 - Polishing. Let HalForms register a custom ObjectMapper customizer to allow separate customizations. If HAL and HAL Forms are supposed to be customized the same way, the same customizer can be registered on both instances. Tweak the application of the customizer to happen inside the configuration classes, not on the outside. Prepare HalFormsConfiguration to become a fully immutable type in 1.3. Javadoc and assertion polish. Original pull request: #1383. --- .../mediatype/hal/HalConfiguration.java | 76 +++++++------ .../hal/HalMediaTypeConfiguration.java | 2 +- .../hal/forms/HalFormsConfiguration.java | 100 ++++++++++++++++-- .../forms/HalFormsMediaTypeConfiguration.java | 2 +- .../hal/HalObjectMapperCustomizerTest.java | 17 ++- .../HalFormsObjectMapperCustomizerTest.java | 30 ++++-- .../forms/HalFormsWebMvcIntegrationTest.java | 12 +-- 7 files changed, 181 insertions(+), 58 deletions(-) diff --git a/src/main/java/org/springframework/hateoas/mediatype/hal/HalConfiguration.java b/src/main/java/org/springframework/hateoas/mediatype/hal/HalConfiguration.java index 47b72dc9..c36febb2 100644 --- a/src/main/java/org/springframework/hateoas/mediatype/hal/HalConfiguration.java +++ b/src/main/java/org/springframework/hateoas/mediatype/hal/HalConfiguration.java @@ -62,18 +62,17 @@ public class HalConfiguration { * Creates a new default {@link HalConfiguration} rendering single links as immediate sub-document. */ public HalConfiguration() { - - this.renderSingleLinks = RenderSingleLinks.AS_SINGLE; - this.singleLinksPerPattern = new LinkedHashMap<>(); - this.applyPropertyNamingStrategy = true; - this.enforceEmbeddedCollections = true; - this.objectMapperCustomizer = objectMapper -> {}; // Default to no action. + this(RenderSingleLinks.AS_SINGLE, new LinkedHashMap<>(), true, true, __ -> {}); } private HalConfiguration(RenderSingleLinks renderSingleLinks, Map singleLinksPerPattern, boolean applyPropertyNamingStrategy, boolean enforceEmbeddedCollections, Consumer objectMapperCustomizer) { + Assert.notNull(renderSingleLinks, "RenderSingleLinks must not be null!"); + Assert.notNull(singleLinksPerPattern, "Single links per pattern map must not be null!"); + Assert.notNull(objectMapperCustomizer, "ObjectMapper customizer must not be null!"); + this.renderSingleLinks = renderSingleLinks; this.singleLinksPerPattern = singleLinksPerPattern; this.applyPropertyNamingStrategy = applyPropertyNamingStrategy; @@ -102,12 +101,15 @@ public class HalConfiguration { * relation (like {@code search}), take wildcards to e.g. match links of a given curie (like {@code acme:*}) or even * complete URIs (like {@code https://api.acme.com/foo/**}). * - * @param pattern must not be {@literal null}. + * @param pattern must not be {@literal null} or empty. * @param renderSingleLinks must not be {@literal null}. * @return @see PathMatcher */ public HalConfiguration withRenderSingleLinksFor(String pattern, RenderSingleLinks renderSingleLinks) { + Assert.hasText(pattern, "Pattern must not be null or empty!"); + Assert.notNull(renderSingleLinks, "RenderSingleLinks must not be null!"); + Map map = new LinkedHashMap<>(singleLinksPerPattern); map.put(pattern, renderSingleLinks); @@ -132,27 +134,33 @@ public class HalConfiguration { /** * Create a new {@link HalConfiguration} by copying the attributes and replacing the {@literal renderSingleLinks}. * - * @param renderSingleLinks - * @return + * @param renderSingleLinks must not be {@literal null}. + * @return will never be {@literal null}. */ public HalConfiguration withRenderSingleLinks(RenderSingleLinks renderSingleLinks) { - return this.renderSingleLinks == renderSingleLinks ? this - : new HalConfiguration(renderSingleLinks, this.singleLinksPerPattern, this.applyPropertyNamingStrategy, - this.enforceEmbeddedCollections, this.objectMapperCustomizer); + Assert.notNull(renderSingleLinks, "RenderSingleLinks must not be null!"); + + return this.renderSingleLinks == renderSingleLinks // + ? this // + : new HalConfiguration(renderSingleLinks, singleLinksPerPattern, applyPropertyNamingStrategy, + enforceEmbeddedCollections, objectMapperCustomizer); } /** * Create a new {@link HalConfiguration} by copying the attributes and replacing the {@literal singleLinksPattern}. * - * @param singleLinksPerPattern - * @return + * @param singleLinksPerPattern must not be {@literal null}. + * @return will never be {@literal null}. */ private HalConfiguration withSingleLinksPerPattern(Map singleLinksPerPattern) { - return this.singleLinksPerPattern == singleLinksPerPattern ? this - : new HalConfiguration(this.renderSingleLinks, singleLinksPerPattern, this.applyPropertyNamingStrategy, - this.enforceEmbeddedCollections, this.objectMapperCustomizer); + Assert.notNull(singleLinksPerPattern, "Single links per pattern map must not be null!"); + + return this.singleLinksPerPattern == singleLinksPerPattern // + ? this // + : new HalConfiguration(renderSingleLinks, singleLinksPerPattern, applyPropertyNamingStrategy, + enforceEmbeddedCollections, objectMapperCustomizer); } /** @@ -164,9 +172,10 @@ public class HalConfiguration { */ public HalConfiguration withApplyPropertyNamingStrategy(boolean applyPropertyNamingStrategy) { - return this.applyPropertyNamingStrategy == applyPropertyNamingStrategy ? this - : new HalConfiguration(this.renderSingleLinks, this.singleLinksPerPattern, applyPropertyNamingStrategy, - this.enforceEmbeddedCollections, this.objectMapperCustomizer); + return this.applyPropertyNamingStrategy == applyPropertyNamingStrategy // + ? this // + : new HalConfiguration(renderSingleLinks, singleLinksPerPattern, applyPropertyNamingStrategy, + enforceEmbeddedCollections, objectMapperCustomizer); } /** @@ -178,32 +187,37 @@ public class HalConfiguration { */ public HalConfiguration withEnforceEmbeddedCollections(boolean enforceEmbeddedCollections) { - return this.enforceEmbeddedCollections == enforceEmbeddedCollections ? this - : new HalConfiguration(this.renderSingleLinks, this.singleLinksPerPattern, this.applyPropertyNamingStrategy, - enforceEmbeddedCollections, this.objectMapperCustomizer); + return this.enforceEmbeddedCollections == enforceEmbeddedCollections // + ? this // + : new HalConfiguration(renderSingleLinks, singleLinksPerPattern, applyPropertyNamingStrategy, + enforceEmbeddedCollections, objectMapperCustomizer); } public HalConfiguration withObjectMapperCustomizer(Consumer objectMapperCustomizer) { - return this.objectMapperCustomizer == objectMapperCustomizer ? this - : new HalConfiguration(this.renderSingleLinks, this.singleLinksPerPattern, this.applyPropertyNamingStrategy, - this.enforceEmbeddedCollections, objectMapperCustomizer); + return this.objectMapperCustomizer == objectMapperCustomizer // + ? this // + : new HalConfiguration(renderSingleLinks, singleLinksPerPattern, applyPropertyNamingStrategy, + enforceEmbeddedCollections, objectMapperCustomizer); } public RenderSingleLinks getRenderSingleLinks() { - return this.renderSingleLinks; + return renderSingleLinks; } public boolean isApplyPropertyNamingStrategy() { - return this.applyPropertyNamingStrategy; + return applyPropertyNamingStrategy; } public boolean isEnforceEmbeddedCollections() { - return this.enforceEmbeddedCollections; + return enforceEmbeddedCollections; } - public Consumer getObjectMapperCustomizer() { - return this.objectMapperCustomizer; + public HalConfiguration customize(ObjectMapper mapper) { + + this.objectMapperCustomizer.accept(mapper); + + return this; } /** diff --git a/src/main/java/org/springframework/hateoas/mediatype/hal/HalMediaTypeConfiguration.java b/src/main/java/org/springframework/hateoas/mediatype/hal/HalMediaTypeConfiguration.java index be9cd853..fdf9abcc 100644 --- a/src/main/java/org/springframework/hateoas/mediatype/hal/HalMediaTypeConfiguration.java +++ b/src/main/java/org/springframework/hateoas/mediatype/hal/HalMediaTypeConfiguration.java @@ -86,7 +86,7 @@ public class HalMediaTypeConfiguration implements HypermediaMappingInformation { mapper.setHandlerInstantiator(new Jackson2HalModule.HalHandlerInstantiator(relProvider, curieProvider.getIfAvailable(() -> CurieProvider.NONE), resolver, halConfiguration, beanFactory)); - halConfiguration.getObjectMapperCustomizer().accept(mapper); + halConfiguration.customize(mapper); return mapper; } diff --git a/src/main/java/org/springframework/hateoas/mediatype/hal/forms/HalFormsConfiguration.java b/src/main/java/org/springframework/hateoas/mediatype/hal/forms/HalFormsConfiguration.java index 547af6be..d39b73c5 100644 --- a/src/main/java/org/springframework/hateoas/mediatype/hal/forms/HalFormsConfiguration.java +++ b/src/main/java/org/springframework/hateoas/mediatype/hal/forms/HalFormsConfiguration.java @@ -18,9 +18,14 @@ package org.springframework.hateoas.mediatype.hal.forms; import java.util.HashMap; import java.util.Map; import java.util.Optional; +import java.util.function.Consumer; import org.springframework.core.ResolvableType; import org.springframework.hateoas.mediatype.hal.HalConfiguration; +import org.springframework.lang.Nullable; +import org.springframework.util.Assert; + +import com.fasterxml.jackson.databind.ObjectMapper; /** * HAL-FORMS specific configuration extension of {@link HalConfiguration}. @@ -31,26 +36,111 @@ import org.springframework.hateoas.mediatype.hal.HalConfiguration; public class HalFormsConfiguration { private final HalConfiguration halConfiguration; - private final Map, String> patterns = new HashMap<>(); + private final Map, String> patterns; + private final Consumer objectMapperCustomizer; /** * Creates a new {@link HalFormsConfiguration} backed by a default {@link HalConfiguration}. */ public HalFormsConfiguration() { - this.halConfiguration = new HalConfiguration(); + this(new HalConfiguration()); } + /** + * Creates a new {@link HalFormsConfiguration} for the given {@link HalConfiguration}. + * + * @param halConfiguration must not be {@literal null}. + */ public HalFormsConfiguration(HalConfiguration halConfiguration) { - this.halConfiguration = halConfiguration; + this(halConfiguration, new HashMap<>(), __ -> {}); } + private HalFormsConfiguration(HalConfiguration halConfiguration, Map, String> patterns, + @Nullable Consumer objectMapperCustomizer) { + + Assert.notNull(halConfiguration, "HalConfiguration must not be null!"); + Assert.notNull(patterns, "Patterns must not be null!"); + Assert.notNull(objectMapperCustomizer, "ObjectMapper customizer must not be null!"); + + this.halConfiguration = halConfiguration; + this.patterns = patterns; + this.objectMapperCustomizer = objectMapperCustomizer; + } + + /** + * Registers a regular expression pattern to be used for form descriptions of the given type. + * + * @param type must not be {@literal null}. + * @param pattern must not be {@literal null} or empty. + * @return will never be {@literal null}. + * @deprecated prefer {@link #withPattern(Class, String)} that returns a fresh instance, to be removed with 1.3. + */ + @Deprecated public HalFormsConfiguration registerPattern(Class type, String pattern) { + Assert.notNull(type, "Type must not be null!"); + Assert.hasText(pattern, "Pattern must not be null or empty!"); + patterns.put(type, pattern); return this; } + /** + * Registers a regular expression pattern to be used for form descriptions of the given type. + * + * @param type must not be {@literal null}. + * @param pattern must not be {@literal null} or empty. + * @return will never be {@literal null}. + */ + @Deprecated + public HalFormsConfiguration withPattern(Class type, String pattern) { + + Assert.notNull(type, "Type must not be null!"); + Assert.hasText(pattern, "Pattern must not be null or empty!"); + + Map, String> newPatterns = new HashMap<>(patterns); + newPatterns.put(type, pattern); + + return new HalFormsConfiguration(halConfiguration, newPatterns, objectMapperCustomizer); + } + + /** + * Register the given {@link Consumer} to apply additional customizations on the {@link ObjectMapper} used to render + * HAL documents. + * + * @param objectMapperCustomizer must not be {@literal null}. + * @return will never be {@literal null}. + */ + public HalFormsConfiguration withObjectMapperCustomizer(Consumer objectMapperCustomizer) { + + Assert.notNull(objectMapperCustomizer, "ObjectMapper customizer must not be null!"); + + return this.objectMapperCustomizer == objectMapperCustomizer // + ? this // + : new HalFormsConfiguration(halConfiguration, patterns, objectMapperCustomizer); + } + + /** + * Customizes the given {@link ObjectMapper} with the registered callback. + * + * @param mapper must not be {@literal null}. + * @return + * @see #withObjectMapperCustomizer(Consumer) + */ + public HalFormsConfiguration customize(ObjectMapper mapper) { + + Assert.notNull(mapper, "ObjectMapper must not be null!"); + + objectMapperCustomizer.accept(mapper); + + return this; + } + + public HalConfiguration getHalConfiguration() { + return halConfiguration; + } + /** * Returns the regular expression pattern that is registered for the given type. * @@ -60,8 +150,4 @@ public class HalFormsConfiguration { Optional getTypePatternFor(ResolvableType type) { return Optional.ofNullable(patterns.get(type.resolve(Object.class))); } - - public HalConfiguration getHalConfiguration() { - return this.halConfiguration; - } } diff --git a/src/main/java/org/springframework/hateoas/mediatype/hal/forms/HalFormsMediaTypeConfiguration.java b/src/main/java/org/springframework/hateoas/mediatype/hal/forms/HalFormsMediaTypeConfiguration.java index 9c50741f..27595f4d 100644 --- a/src/main/java/org/springframework/hateoas/mediatype/hal/forms/HalFormsMediaTypeConfiguration.java +++ b/src/main/java/org/springframework/hateoas/mediatype/hal/forms/HalFormsMediaTypeConfiguration.java @@ -82,7 +82,7 @@ class HalFormsMediaTypeConfiguration implements HypermediaMappingInformation { mapper.setHandlerInstantiator(new Jackson2HalFormsModule.HalFormsHandlerInstantiator(relProvider, curieProvider.getIfAvailable(() -> CurieProvider.NONE), resolver, configuration, beanFactory)); - configuration.getHalConfiguration().getObjectMapperCustomizer().accept(mapper); + configuration.customize(mapper); return mapper; } diff --git a/src/test/java/org/springframework/hateoas/mediatype/hal/HalObjectMapperCustomizerTest.java b/src/test/java/org/springframework/hateoas/mediatype/hal/HalObjectMapperCustomizerTest.java index 1aaa4ebc..4e07ff77 100644 --- a/src/test/java/org/springframework/hateoas/mediatype/hal/HalObjectMapperCustomizerTest.java +++ b/src/test/java/org/springframework/hateoas/mediatype/hal/HalObjectMapperCustomizerTest.java @@ -1,3 +1,18 @@ +/* + * Copyright 2020 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ package org.springframework.hateoas.mediatype.hal; import static org.assertj.core.api.Assertions.*; @@ -29,7 +44,7 @@ import com.fasterxml.jackson.databind.SerializationFeature; @ExtendWith(SpringExtension.class) @WebAppConfiguration @ContextConfiguration -public class HalObjectMapperCustomizerTest { +class HalObjectMapperCustomizerTest { @Autowired WebApplicationContext context; diff --git a/src/test/java/org/springframework/hateoas/mediatype/hal/forms/HalFormsObjectMapperCustomizerTest.java b/src/test/java/org/springframework/hateoas/mediatype/hal/forms/HalFormsObjectMapperCustomizerTest.java index fe6aca1e..3e93545e 100644 --- a/src/test/java/org/springframework/hateoas/mediatype/hal/forms/HalFormsObjectMapperCustomizerTest.java +++ b/src/test/java/org/springframework/hateoas/mediatype/hal/forms/HalFormsObjectMapperCustomizerTest.java @@ -1,3 +1,18 @@ +/* + * Copyright 2020 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ package org.springframework.hateoas.mediatype.hal.forms; import static org.assertj.core.api.Assertions.*; @@ -12,8 +27,8 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; import org.springframework.hateoas.MappingTestUtils; +import org.springframework.hateoas.MappingTestUtils.ContextualMapper; import org.springframework.hateoas.config.EnableHypermediaSupport; -import org.springframework.hateoas.mediatype.hal.HalConfiguration; import org.springframework.hateoas.support.WebMvcEmployeeController; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit.jupiter.SpringExtension; @@ -30,13 +45,12 @@ import com.fasterxml.jackson.databind.SerializationFeature; @ExtendWith(SpringExtension.class) @WebAppConfiguration @ContextConfiguration -public class HalFormsObjectMapperCustomizerTest { +class HalFormsObjectMapperCustomizerTest { @Autowired WebApplicationContext context; MockMvc mockMvc; - - MappingTestUtils.ContextualMapper mapper = MappingTestUtils.createMapper(getClass()); + ContextualMapper mapper = MappingTestUtils.createMapper(getClass()); @BeforeEach void setUp() { @@ -48,9 +62,9 @@ public class HalFormsObjectMapperCustomizerTest { @Test // #1382 void objectMapperCustomizerShouldBeApplied() throws Exception { - String actualHalFormsJson = this.mockMvc.perform(get("/employees/0")).andReturn().getResponse() + String actualHalFormsJson = mockMvc.perform(get("/employees/0")).andReturn().getResponse() .getContentAsString(); - String expectedHalFormsJson = this.mapper.readFile("hal-forms-custom.json"); + String expectedHalFormsJson = mapper.readFile("hal-forms-custom.json"); assertThat(actualHalFormsJson).isEqualTo(expectedHalFormsJson); } @@ -62,8 +76,8 @@ public class HalFormsObjectMapperCustomizerTest { static class TestConfig { @Bean - HalConfiguration halConfiguration() { - return new HalConfiguration() + HalFormsConfiguration halFormsConfiguration() { + return new HalFormsConfiguration() .withObjectMapperCustomizer(objectMapper -> objectMapper.configure(SerializationFeature.INDENT_OUTPUT, true)); } } diff --git a/src/test/java/org/springframework/hateoas/mediatype/hal/forms/HalFormsWebMvcIntegrationTest.java b/src/test/java/org/springframework/hateoas/mediatype/hal/forms/HalFormsWebMvcIntegrationTest.java index 146bc7f6..4d5cbd01 100644 --- a/src/test/java/org/springframework/hateoas/mediatype/hal/forms/HalFormsWebMvcIntegrationTest.java +++ b/src/test/java/org/springframework/hateoas/mediatype/hal/forms/HalFormsWebMvcIntegrationTest.java @@ -18,7 +18,6 @@ package org.springframework.hateoas.mediatype.hal.forms; import static org.assertj.core.api.Assertions.*; import static org.hamcrest.CoreMatchers.*; import static org.hamcrest.collection.IsCollectionWithSize.*; -import static org.mockito.Mockito.*; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*; import static org.springframework.test.web.servlet.setup.MockMvcBuilders.*; @@ -30,14 +29,13 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; -import org.springframework.core.io.ClassPathResource; import org.springframework.hateoas.Links; +import org.springframework.hateoas.MappingTestUtils; import org.springframework.hateoas.MediaTypes; import org.springframework.hateoas.config.EnableHypermediaSupport; import org.springframework.hateoas.config.EnableHypermediaSupport.HypermediaType; import org.springframework.hateoas.mediatype.hal.HalConfiguration; import org.springframework.hateoas.mediatype.hal.Jackson2HalModule.HalLinkListSerializer; -import org.springframework.hateoas.support.MappingUtils; import org.springframework.hateoas.support.WebMvcEmployeeController; import org.springframework.http.HttpHeaders; import org.springframework.test.context.ContextConfiguration; @@ -123,7 +121,7 @@ class HalFormsWebMvcIntegrationTest { @Test void createNewEmployee() throws Exception { - String specBasedJson = MappingUtils.read(new ClassPathResource("new-employee.json", getClass())); + String specBasedJson = MappingTestUtils.createMapper(getClass()).readFile("new-employee.json"); this.mockMvc.perform(post("/employees") // .content(specBasedJson) // @@ -181,11 +179,7 @@ class HalFormsWebMvcIntegrationTest { @Bean public HalFormsConfiguration halFormsConfiguration() { - - HalFormsConfiguration config = mock(HalFormsConfiguration.class); - when(config.getHalConfiguration()).thenReturn(CONFIG); - - return config; + return new HalFormsConfiguration(CONFIG); } }