codeconsole commented on code in PR #16237: URL: https://github.com/apache/grails-core/pull/16237#discussion_r4213526340
########## grails-converters/src/main/groovy/org/grails/web/converters/jackson/GrailsDomainJsonSerializer.java: ########## @@ -0,0 +1,161 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 + * + * http://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.grails.web.converters.jackson; + +import java.util.Collection; +import java.util.List; +import java.util.Map; + +import tools.jackson.core.JacksonException; +import tools.jackson.core.JsonGenerator; +import tools.jackson.databind.SerializationContext; +import tools.jackson.databind.ValueSerializer; + +import org.springframework.beans.BeanWrapper; +import org.springframework.beans.BeanWrapperImpl; + +import grails.core.support.proxy.EntityProxyHandler; +import grails.core.support.proxy.ProxyHandler; +import org.grails.core.util.IncludeExcludeSupport; +import org.grails.datastore.mapping.model.PersistentEntity; +import org.grails.datastore.mapping.model.PersistentProperty; +import org.grails.datastore.mapping.model.types.Association; +import org.grails.datastore.mapping.model.types.ManyToOne; +import org.grails.datastore.mapping.model.types.OneToOne; + +/** Serializes a mapped Grails domain type using its persistent metadata. */ +final class GrailsDomainJsonSerializer extends ValueSerializer<Object> { + + // Stateless, and consulted once per property of every serialized object. + private static final IncludeExcludeSupport<String> INCLUDE_EXCLUDE_SUPPORT = new IncludeExcludeSupport<>(); + + private final PersistentEntity entity; + private final ProxyHandler proxyHandler; + private final boolean includeVersion; + private final boolean includeClass; + + GrailsDomainJsonSerializer(PersistentEntity entity, ProxyHandler proxyHandler, + boolean includeVersion, boolean includeClass) { + this.entity = entity; + this.proxyHandler = proxyHandler; + this.includeVersion = includeVersion; + this.includeClass = includeClass; + } + + @Override + public void serialize(Object value, JsonGenerator generator, SerializationContext context) throws JacksonException { + Object unwrapped = proxyHandler.unwrapIfProxy(value); + BeanWrapper bean = new BeanWrapperImpl(unwrapped); + List<String> includes = properties(context, GrailsJsonMapperCustomizer.INCLUDES_ATTRIBUTE, unwrapped.getClass()); + List<String> excludes = properties(context, GrailsJsonMapperCustomizer.EXCLUDES_ATTRIBUTE, unwrapped.getClass()); + + generator.writeStartObject(); + if (includeClass && shouldInclude(includes, excludes, "class")) { + generator.writeStringProperty("class", entity.getName()); + } + // An unsaved instance has neither yet; the legacy marshaller leaves them out rather than writing null + writePropertyIfSet(entity.getIdentity(), bean, generator, context, includes, excludes); + if (includeVersion) { + writePropertyIfSet(entity.getVersion(), bean, generator, context, includes, excludes); + } + for (PersistentProperty property : entity.getPersistentProperties()) { + if (!property.equals(entity.getVersion())) { + writeProperty(property, bean, generator, context, includes, excludes); + } + } + generator.writeEndObject(); + } + + private void writePropertyIfSet(PersistentProperty property, BeanWrapper bean, JsonGenerator generator, + SerializationContext context, List<String> includes, List<String> excludes) throws JacksonException { + if (property != null && bean.getPropertyValue(property.getName()) != null) { + writeProperty(property, bean, generator, context, includes, excludes); + } + } + + private void writeProperty(PersistentProperty property, BeanWrapper bean, JsonGenerator generator, + SerializationContext context, List<String> includes, List<String> excludes) throws JacksonException { + if (property == null || !shouldInclude(includes, excludes, property.getName())) { + return; + } + Object propertyValue = bean.getPropertyValue(property.getName()); + generator.writeName(property.getName()); + if (property instanceof Association association && !association.isEmbedded() && + (property instanceof OneToOne || property instanceof ManyToOne)) { + writeAssociationReference(propertyValue, association.getAssociatedEntity(), generator, context); + } + else if (property instanceof Association association && !association.isEmbedded() && + propertyValue instanceof Collection<?> collection) { + generator.writeStartArray(); + for (Object associated : collection) { + writeAssociationReference(associated, association.getAssociatedEntity(), generator, context); + } + generator.writeEndArray(); + } + else if (property instanceof Association association && !association.isEmbedded() && + propertyValue instanceof Map<?, ?> map) { + generator.writeStartObject(); + for (Map.Entry<?, ?> entry : map.entrySet()) { + generator.writeName(String.valueOf(entry.getKey())); + writeAssociationReference(entry.getValue(), association.getAssociatedEntity(), generator, context); + } + generator.writeEndObject(); + } + else { + context.writeValue(generator, propertyValue); Review Comment: Fixed in ae91c5e23b rather than documented. The domain serializer tracks the domain objects in progress for each write. It answers a repeat as the legacy converter does: `{"_ref":"..","class":"..."}` by default, with one `..` per enclosing object or array, or as `grails.converters.json.circular.reference.behaviour` selects (`EXCEPTION`, `INSERT_NULL`, `IGNORE`, `PATH`). `GrailsJsonMapperCustomizerSpec` covers each behaviour with an embedded value whose nested bean points back at its owner. A cycle made only of non-domain beans follows Jackson, as in a plain `@RestController`, and section 2 says so. ########## grails-xml/src/main/groovy/org/grails/plugins/web/rest/render/xml/DefaultXmlRenderer.groovy: ########## @@ -108,18 +137,67 @@ class DefaultXmlRenderer<T> implements Renderer<T> { * @param context */ protected void renderXml(Object object, RenderContext context) { + HttpMessageConverter<Object> springConverter = findSpringConverter(object, context) + if (springConverter != null) { + renderWithSpringConverter(springConverter, object, context) + return + } + XML converter if (namedConfiguration) { XML.use(namedConfiguration) { - converter = object as XML + converter = new XML(object) } } else { - converter = object as XML + converter = new XML(object) } renderXml(converter, context) } + private HttpMessageConverter<Object> findSpringConverter(Object object, RenderContext context) { + if (!resolveSpringHttpMessageConverters() || namedConfiguration || context.includes || context.excludes) { + return null + } + if (object == null || object instanceof Errors || object instanceof Map || object instanceof Collection || + object.getClass().isArray()) { + return null + } + MediaType mediaType = MediaType.parseMediaType((context.acceptMimeType ?: MimeType.XML).name) + return (HttpMessageConverter<Object>) resolveSpringHttpMessageConverters().find { HttpMessageConverter<?> converter -> + converter.canWrite(object.getClass(), mediaType) && + converter.getSupportedMediaTypes(object.getClass()).any { MediaType supported -> + supported.subtype == 'xml' || supported.subtype.endsWith('+xml') + } + } + } + + private void renderWithSpringConverter( + HttpMessageConverter<Object> converter, Object object, RenderContext context) { + // Write in the configured encoding rather than the converter's default so the bytes it + // produces and the characters decoded back out agree, and stream them through instead of + // holding the whole response in memory. + Charset charset = Charset.forName(encoding) Review Comment: Fixed in be751d3846. The XML renderer uses UTF-8 for the intermediate bytes when the converter is a Jackson one, as the JSON renderer does. `SpringXmlRendererSpec` 'Jackson XML bytes round trip through a non UTF response encoding' writes `café` with `ISO-8859-1`. ########## grails-xml/src/main/groovy/org/grails/plugins/web/rest/render/xml/DefaultXmlRenderer.groovy: ########## @@ -108,18 +137,67 @@ class DefaultXmlRenderer<T> implements Renderer<T> { * @param context */ protected void renderXml(Object object, RenderContext context) { + HttpMessageConverter<Object> springConverter = findSpringConverter(object, context) + if (springConverter != null) { + renderWithSpringConverter(springConverter, object, context) + return + } + XML converter if (namedConfiguration) { XML.use(namedConfiguration) { - converter = object as XML + converter = new XML(object) } } else { - converter = object as XML + converter = new XML(object) } renderXml(converter, context) } + private HttpMessageConverter<Object> findSpringConverter(Object object, RenderContext context) { + if (!resolveSpringHttpMessageConverters() || namedConfiguration || context.includes || context.excludes) { Review Comment: Done in be751d3846. `grails.web.rendering.xml.spring` (default `false`) is an explicit switch. Adding `jackson-dataformat-xml` alone therefore leaves registered XML marshallers in charge. Domain objects stay on the Grails XML converter even when the switch is `true`. Section 1 says both. Covered by `SpringXmlRendererSpec` 'adding Jackson XML does not bypass registered XML marshallers by default' and 'domain responses keep legacy marshalling even with Spring XML enabled'. ########## grails-xml/src/main/groovy/org/grails/plugins/xml/XmlGrailsPlugin.groovy: ########## @@ -0,0 +1,94 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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.grails.plugins.xml + +import groovy.transform.CompileStatic + +import org.springframework.beans.factory.BeanRegistrar +import org.springframework.beans.factory.BeanRegistry +import org.springframework.core.env.Environment + +import grails.converters.XML +import grails.plugins.Plugin +import grails.util.GrailsUtil +import org.grails.plugins.codecs.XMLCodec +import org.grails.web.converters.configuration.ObjectMarshallerRegisterer +import org.grails.plugins.web.rest.render.SpringMessageConverters +import org.grails.plugins.web.rest.render.xml.DefaultXmlRenderer +import org.grails.web.gsp.io.GrailsConventionGroovyPageLocator +import org.grails.web.converters.configuration.XmlConvertersConfigurationInitializer +import org.grails.web.converters.marshaller.xml.ValidationErrorsMarshaller +import org.grails.web.databinding.bindingsource.HalXmlDataBindingSourceCreator +import org.grails.web.databinding.bindingsource.XmlDataBindingSourceCreator + +/** + * Provides optional XML conversion, rendering, and request binding support. + * + * @since 9.0 + */ +@CompileStatic +class XmlGrailsPlugin extends Plugin { + + def version = GrailsUtil.getGrailsVersion() + def dependsOn = [converters: version, dataBinding: version, restResponder: version] + def providedArtefacts = [XMLCodec] + + private static <T extends DefaultXmlRenderer> T configure(T renderer, Environment environment, + SpringMessageConverters converters) { + renderer.encoding = environment.getProperty('grails.converters.encoding', 'UTF-8') + if (converters != null) { + renderer.springHttpMessageConvertersSupplier = converters::getConverters + } + return renderer + } + + @Override + BeanRegistrar beanRegistrar() { + return { BeanRegistry registry, Environment environment -> + registry.registerBean('xmlErrorsMarshaller', ValidationErrorsMarshaller) + registry.registerBean('xmlConvertersConfigurationInitializer', XmlConvertersConfigurationInitializer) + registry.registerBean('xmlDataBindingSourceCreator', XmlDataBindingSourceCreator) + registry.registerBean('halXmlDataBindingSourceCreator', HalXmlDataBindingSourceCreator) + // Contributed as Renderer beans, which DefaultRendererRegistry autowires: registering + // them from a bean that holds a registry reference can write into an instance nothing + // reads, because the harness rebuilds that singleton. + registry.registerBean('xmlRenderer', DefaultXmlRenderer) { Review Comment: Fixed in 9727910c7c. The plugin's XML renderer bean is an `XmlFallbackRenderer`, marked with `FallbackRenderer`. The registry routes only marked beans through `addDefaultRenderer`, so every other renderer bean keeps its 8.0.x precedence, including one targeting `Object`. be751d3846 had sent every `Object`-targeted bean to the defaults, which let the registry's own JSON default replace an application `Object` JSON renderer. `XmlGrailsPluginSpec` covers your interface case and an application `Object` renderer in both collection orders; `DefaultRendererRegistrySpec` covers the JSON case. ########## grails-testing-support-web/src/main/groovy/org/grails/testing/spock/WebSetupSpecInterceptor.groovy: ########## @@ -73,7 +76,14 @@ class WebSetupSpecInterceptor implements IMethodInterceptor { GrailsApplication grailsApplication = test.grailsApplication Map<String, String> groovyPages = test.views - test.defineBeans(new ConvertersGrailsPlugin()) + SpringMessageConverters converters = test.applicationContext.getBean(SpringMessageConverters) + JsonMapper mapper = test.applicationContext.getBeanProvider(JsonMapper).getIfUnique() ?: + test.applicationContext.getBean('jacksonJsonMapper', JsonMapper) + converters.extendMessageConverters([ Review Comment: Fixed in e1a8c3cd4e rather than documented. The web test traits build the converter list as Spring MVC does: Spring's server defaults for the test classpath with JSON on Boot's mapper, then Boot's `ServerHttpMessageConvertersCustomizer`s (the harness registers `HttpMessageConvertersAutoConfiguration`), then the test's `WebMvcConfigurer` beans. In grails-xml, `ControllerUnitTestMessageConvertersSpec` writes XML through Jackson XML, with the converter an application `WebMvcConfigurer` installed. `unitTesting.adoc` describes this. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
