From ecc40056c09de5a5e8715263fcfce4feaf974c0d Mon Sep 17 00:00:00 2001 From: Oliver Drotbohm Date: Thu, 15 Aug 2024 08:38:21 +0200 Subject: [PATCH] Avoid affecting serialization of custom Page implementations in legacy mode. Registering a StdConverter with Jackson to log a warning about the Page serialization mode causes the target serializer to be only built for Page losing additional properties defined on extensions. We now instead register a no-op BeanSerializerModifier that issues the warning and doesn't affect the serializer selection. Fixes GH-3137. --- .../SpringDataJacksonConfiguration.java | 37 +++++++++------- .../PageImplJsonSerializationUnitTests.java | 24 ++++++++++- ...ringDataJacksonConfigurationUnitTests.java | 42 ------------------- 3 files changed, 45 insertions(+), 58 deletions(-) delete mode 100644 src/test/java/org/springframework/data/web/config/SpringDataJacksonConfigurationUnitTests.java diff --git a/src/main/java/org/springframework/data/web/config/SpringDataJacksonConfiguration.java b/src/main/java/org/springframework/data/web/config/SpringDataJacksonConfiguration.java index 4d87e14c2e..460abe39bd 100644 --- a/src/main/java/org/springframework/data/web/config/SpringDataJacksonConfiguration.java +++ b/src/main/java/org/springframework/data/web/config/SpringDataJacksonConfiguration.java @@ -15,6 +15,8 @@ */ package org.springframework.data.web.config; +import java.util.List; + import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; @@ -27,8 +29,12 @@ import org.springframework.lang.Nullable; import org.springframework.util.ClassUtils; +import com.fasterxml.jackson.databind.BeanDescription; +import com.fasterxml.jackson.databind.SerializationConfig; import com.fasterxml.jackson.databind.annotation.JsonSerialize; import com.fasterxml.jackson.databind.module.SimpleModule; +import com.fasterxml.jackson.databind.ser.BeanPropertyWriter; +import com.fasterxml.jackson.databind.ser.BeanSerializerModifier; import com.fasterxml.jackson.databind.ser.std.ToStringSerializerBase; import com.fasterxml.jackson.databind.util.StdConverter; @@ -83,7 +89,8 @@ public PageModule(@Nullable SpringDataWebSettings settings) { addSerializer(UNPAGED_TYPE, new UnpagedAsInstanceSerializer()); if (settings == null || settings.pageSerializationMode() == PageSerializationMode.DIRECT) { - setMixInAnnotation(PageImpl.class, WarningMixing.class); + setSerializerModifier(new WarningLoggingModifier()); + } else { setMixInAnnotation(PageImpl.class, WrappingMixing.class); } @@ -109,14 +116,6 @@ public String valueToString(@Nullable Object value) { } } - /** - * A mixin for PageImpl to register a converter issuing the serialization warning. - * - * @author Oliver Drotbohm - */ - @JsonSerialize(converter = PlainPageSerializationWarning.class) - abstract class WarningMixing {} - @JsonSerialize(converter = PageModelConverter.class) abstract class WrappingMixing {} @@ -129,27 +128,35 @@ public PagedModel convert(@Nullable Page value) { } } - static class PlainPageSerializationWarning extends StdConverter, Page> { + /** + * A {@link BeanSerializerModifier} that logs a warning message if an instance of {@link Page} will be rendered. + * + * @author Oliver Drotbohm + */ + static class WarningLoggingModifier extends BeanSerializerModifier { - private static final Logger LOGGER = LoggerFactory.getLogger(PlainPageSerializationWarning.class); + private static final Logger LOGGER = LoggerFactory.getLogger(WarningLoggingModifier.class); private static final String MESSAGE = """ Serializing PageImpl instances as-is is not supported, meaning that there is no guarantee about the stability of the resulting JSON structure! For a stable JSON structure, please use Spring Data's PagedModel (globally via @EnableSpringDataWebSupport(pageSerializationMode = VIA_DTO)) or Spring HATEOAS and Spring Data's PagedResourcesAssembler as documented in https://docs.spring.io/spring-data/commons/reference/repositories/core-extensions.html#core.web.pageables. """; + private static final long serialVersionUID = 954857444010009875L; + private boolean warningRendered = false; - @Nullable @Override - public Page convert(@Nullable Page value) { + public List changeProperties(SerializationConfig config, BeanDescription beanDesc, + List beanProperties) { + + if (Page.class.isAssignableFrom(beanDesc.getBeanClass()) && !warningRendered) { - if (!warningRendered) { this.warningRendered = true; LOGGER.warn(MESSAGE); } - return value; + return super.changeProperties(config, beanDesc, beanProperties); } } } diff --git a/src/test/java/org/springframework/data/web/PageImplJsonSerializationUnitTests.java b/src/test/java/org/springframework/data/web/PageImplJsonSerializationUnitTests.java index 6a52519517..e5a7d34ed6 100644 --- a/src/test/java/org/springframework/data/web/PageImplJsonSerializationUnitTests.java +++ b/src/test/java/org/springframework/data/web/PageImplJsonSerializationUnitTests.java @@ -45,7 +45,16 @@ void serializesPageImplAsPagedModel() { assertJsonRendering(PageSerializationMode.VIA_DTO, "$.content", "$.page"); } + @Test // GH-3137 + void serializesCustomPageAsPageImpl() { + assertJsonRendering(PageSerializationMode.DIRECT, new Extension<>("header"), "$.pageable", "$.last", "$.first"); + } + private static void assertJsonRendering(PageSerializationMode mode, String... jsonPaths) { + assertJsonRendering(mode, new PageImpl<>(Collections.emptyList()), jsonPaths); + } + + private static void assertJsonRendering(PageSerializationMode mode, PageImpl page, String... jsonPaths) { SpringDataWebSettings settings = new SpringDataWebSettings(mode); @@ -54,11 +63,24 @@ private static void assertJsonRendering(PageSerializationMode mode, String... js assertThatNoException().isThrownBy(() -> { - String result = mapper.writeValueAsString(new PageImpl<>(Collections.emptyList())); + String result = mapper.writeValueAsString(page); for (String jsonPath : jsonPaths) { assertThat(JsonPath. read(result, jsonPath)).isNotNull(); } }); } + + static class Extension extends PageImpl { + + private Object header; + + public Extension(Object header) { + super(Collections.emptyList()); + } + + public Object getHeader() { + return header; + } + } } diff --git a/src/test/java/org/springframework/data/web/config/SpringDataJacksonConfigurationUnitTests.java b/src/test/java/org/springframework/data/web/config/SpringDataJacksonConfigurationUnitTests.java deleted file mode 100644 index 386c63c888..0000000000 --- a/src/test/java/org/springframework/data/web/config/SpringDataJacksonConfigurationUnitTests.java +++ /dev/null @@ -1,42 +0,0 @@ -/* - * Copyright 2024 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.data.web.config; - -import static org.assertj.core.api.Assertions.*; - -import org.junit.jupiter.api.Test; -import org.springframework.data.domain.PageImpl; -import org.springframework.data.web.config.SpringDataJacksonConfiguration.PageModule; -import org.springframework.data.web.config.SpringDataJacksonConfiguration.PageModule.WarningMixing; - -import com.fasterxml.jackson.databind.ObjectMapper; - -/** - * Unit tests for {@link SpringDataJacksonConfiguration}. - * - * @author Oliver Drotbohm - */ -class SpringDataJacksonConfigurationUnitTests { - - @Test // GH-3101 - void usesDirectRenderingIfNoSpringDataWebSettingsArePresent() { - - ObjectMapper mapper = new ObjectMapper(); - mapper.registerModule(new PageModule(null)); - - assertThat(mapper.getSerializationConfig().findMixInClassFor(PageImpl.class)).isEqualTo(WarningMixing.class); - } -}