codeconsole commented on code in PR #16237:
URL: https://github.com/apache/grails-core/pull/16237#discussion_r4213522634


##########
grails-rest-transforms/src/main/groovy/org/grails/plugins/web/rest/render/DefaultRendererRegistry.groovy:
##########
@@ -77,13 +80,32 @@ class DefaultRendererRegistry extends 
ClassAndMimeTypeRegistry<Renderer, Rendere
     @Value('${grails.converters.encoding:UTF-8}')
     String encoding = grails.util.GrailsWebUtil.DEFAULT_ENCODING
 
+    @Autowired(required = false)
+    SpringMessageConverters springMessageConverters
+
+    @Autowired(required = false)
+    GrailsJsonMapperCustomizer grailsJsonMapperCustomizer
+
+    @Autowired(required = false)
+    NamedJsonRenderer namedJsonRenderer
+
+    @Autowired(required = false)
+    ValidationProblemDetailFactory validationProblemDetailFactory
+
+    /**
+     * Whether JSON responses are written by Spring's message converters. When 
unset, they are unless
+     * the application customized the legacy {@code grails.converters.JSON} 
converter.
+     */
+    @Value('${grails.web.rendering.json.spring:#{null}}')

Review Comment:
   Inverted in b520a96840. `grails.web.rendering.json.spring` defaults to 
`false`: `respond()` uses the legacy converter and logs one warning naming 
Grails 10 for the default and Grails 11 for removal. Validation errors keep the 
legacy body unless the setting is `true`. Covered by `RespondMethodSpec` 
'respond preserves legacy JSON by default during the Grails 9 migration' and 
`DefaultJsonRendererSpec` 'without a Spring JSON setting the legacy converter 
writes the response and warns'.



##########
grails-rest-transforms/src/main/groovy/org/grails/plugins/web/rest/render/json/DefaultJsonRenderer.groovy:
##########
@@ -108,17 +155,121 @@ class DefaultJsonRenderer<T> implements Renderer<T> {
      * @param context
      */
     protected void renderJson(T object, RenderContext context) {
+        String selectedConfiguration = 
context.arguments?.get('jsonConfiguration')?.toString()
+        if (selectedConfiguration && 
namedJsonRenderer?.contains(selectedConfiguration)) {
+            namedJsonRenderer.render(selectedConfiguration, object, 
context.writer,
+                    context.includes, context.excludes)
+            return
+        }
+        if (!selectedConfiguration && canUseSpringConverter(context)) {
+            Object springValue = object instanceof Errors ?
+                    validationProblemDetailFactory.create((Errors) object, 
errorsHttpStatus) : object
+            MediaType mediaType = object instanceof Errors ?
+                    MediaType.parseMediaType(PROBLEM_JSON.name) :
+                    MediaType.parseMediaType(resolveMimeType(context).name)
+            // Set the content type before writing: once the writer flushes, 
the response is
+            // committed and a later content type change is silently discarded.
+            if (object instanceof Errors) {
+                
context.setContentType(GrailsWebUtil.getContentType(PROBLEM_JSON.name, 
encoding))
+            }
+            if (renderWithSpringConverter(springValue, mediaType, context)) {
+                return
+            }
+            if (object instanceof Errors) {
+                // No converter could write the problem; restore the 
negotiated type for the
+                // legacy converter path below.
+                
context.setContentType(GrailsWebUtil.getContentType(resolveMimeType(context).name,
 encoding))
+            }
+        }
+
         JSON converter
-        if (namedConfiguration) {
-            JSON.use(namedConfiguration) {
-                converter = object as JSON
+        String legacyConfiguration = selectedConfiguration ?: 
namedConfiguration
+        if (legacyConfiguration) {
+            JSON.use(legacyConfiguration) {
+                converter = new JSON(object)
             }
         } else {
-            converter = object as JSON
+            converter = new JSON(object)
         }
         renderJson(converter, context)
     }
 
+    private List<HttpMessageConverter<?>> resolveSpringHttpMessageConverters() 
{
+        List<HttpMessageConverter<?>> supplied = 
springHttpMessageConvertersSupplier?.get()
+        return supplied ?: springHttpMessageConverters
+    }
+
+    private boolean canUseSpringConverter(RenderContext context) {
+        return resolveSpringHttpMessageConverters() && !namedConfiguration &&
+                !context.includes && !context.excludes && springJsonEnabled()
+    }
+
+    private boolean springJsonEnabled() {
+        if (useSpringJson != null) {
+            return useSpringJson
+        }
+        if 
(!ConvertersConfigurationHolder.isDefaultConfigurationCustomized(JSON)) {

Review Comment:
   Removed in b520a96840 and d81e3956db. `markDefaultConfigurationCustomized`, 
`isDefaultConfigurationCustomized` and their callers are gone. Only the setting 
decides, and the warning fires whenever the legacy path renders.



##########
grails-converters/src/main/groovy/org/grails/web/converters/jackson/GrailsJsonMapperCustomizer.java:
##########
@@ -0,0 +1,133 @@
+/*
+ * 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.ArrayList;
+import java.util.List;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+
+import groovy.lang.GString;
+
+import tools.jackson.databind.JacksonModule;
+import tools.jackson.databind.json.JsonMapper;
+import tools.jackson.databind.module.SimpleModule;
+import tools.jackson.databind.ser.std.ToStringSerializer;
+
+import 
org.springframework.boot.jackson.autoconfigure.JsonMapperBuilderCustomizer;
+import org.springframework.validation.Errors;
+
+import grails.core.GrailsApplication;
+import grails.core.support.proxy.DefaultProxyHandler;
+import grails.core.support.proxy.ProxyHandler;
+import org.grails.core.artefact.DomainClassArtefactHandler;
+import org.grails.datastore.mapping.model.MappingContext;
+
+/**
+ * Adds Grails-specific serializers to Spring Boot's configured JSON mapper.
+ *
+ * @since 9.0
+ */
+public final class GrailsJsonMapperCustomizer implements 
JsonMapperBuilderCustomizer {
+
+    /** Writer attribute holding the property names to include, as a List or a 
Map keyed by type. */
+    public static final String INCLUDES_ATTRIBUTE = 
GrailsJsonMapperCustomizer.class.getName() + ".includes";
+
+    /** Writer attribute holding the property names to exclude, as a List or a 
Map keyed by type. */
+    public static final String EXCLUDES_ATTRIBUTE = 
GrailsJsonMapperCustomizer.class.getName() + ".excludes";
+
+    private final ConcurrentMap<JsonMapper, JsonMapper> grailsMappers = new 
ConcurrentHashMap<>();
+    private final GrailsApplication grailsApplication;
+    private final ProxyHandler proxyHandler;
+
+    public GrailsJsonMapperCustomizer() {
+        this(null, new DefaultProxyHandler());
+    }
+
+    public GrailsJsonMapperCustomizer(GrailsApplication grailsApplication) {
+        this(grailsApplication, new DefaultProxyHandler());
+    }
+
+    public GrailsJsonMapperCustomizer(GrailsApplication grailsApplication, 
ProxyHandler proxyHandler) {
+        this.grailsApplication = grailsApplication;
+        this.proxyHandler = proxyHandler;
+    }
+
+    private boolean domainArtefact(Class<?> type) {
+        // Decided from the class itself rather than the artefact registry: 
registry lookups need
+        // the Domain handler to have been registered and raise when it has 
not, whereas this holds
+        // as soon as the class is loaded -- which is the point, since GORM is 
not up yet. The flag
+        // lets a proxy be recognised through its domain superclass.
+        return DomainClassArtefactHandler.isDomainClass(type, true);
+    }
+
+    private MappingContext mappingContext() {
+        return this.grailsApplication == null ? null : 
this.grailsApplication.getMappingContext();
+    }
+
+    private boolean booleanProperty(String key, String fallbackKey) {
+        if (this.grailsApplication == null) {
+            return false;
+        }
+        boolean fallback = 
this.grailsApplication.getConfig().getProperty(fallbackKey, Boolean.class, 
false);
+        return this.grailsApplication.getConfig().getProperty(key, 
Boolean.class, fallback);
+    }
+
+    @Override
+    public void customize(JsonMapper.Builder builder) {
+        SimpleModule module = new SimpleModule("grails-json");
+        module.addSerializer(GString.class, ToStringSerializer.instance);
+        // Resolve messages at write time, after the application context is 
ready.
+        module.addSerializer(Errors.class, new SpringErrorsJsonSerializer(

Review Comment:
   Moved in d81e3956db. The `Errors` serializer is registered only on the 
mapper `forGrails` derives; Boot's shared mapper gets only the `GString` 
serializer. `GrailsJsonMapperCustomizerSpec` 'only the Grails mapper receives 
the validation errors serializer' checks both mappers.



##########
grails-web-databinding/src/main/groovy/org/grails/web/databinding/bindingsource/JsonDataBindingSourceCreator.groovy:
##########
@@ -48,8 +52,35 @@ class JsonDataBindingSourceCreator extends 
AbstractRequestBodyDataBindingSourceC
 
     private static final Pattern INDEX_PATTERN = ~/^(\S+)\[(\d+)\]$/
 
+    // Resolved when a request body is first parsed rather than injected. 
Injecting it pulls
+    // Jackson's auto-configuration into this bean's graph, and 
MimeTypesConfiguration depends on
+    // this creator, so Boot's mapper would be built before GORM has 
initialized.
     @Autowired(required = false)
-    JsonSlurper jsonSlurper = new JsonSlurper()
+    ObjectProvider<JsonMapper> jsonMapperProvider
+
+    private volatile JsonMapper resolvedJsonMapper
+
+    JsonMapper getJsonMapper() {
+        JsonMapper mapper = this.resolvedJsonMapper
+        if (mapper == null) {
+            mapper = jsonMapperProvider?.getIfAvailable() ?: 
JsonMapper.builder().build()
+            this.resolvedJsonMapper = mapper
+        }
+        return mapper
+    }
+
+    void setJsonMapper(JsonMapper jsonMapper) {
+        this.resolvedJsonMapper = jsonMapper
+    }
+
+    /**
+     * Reads untyped JSON values. Decimals are read as {@link BigDecimal} so 
that binding a
+     * fractional value to a BigDecimal property keeps the digits the request 
sent; reading them
+     * as doubles first would round them before the binder ever saw them.
+     */
+    protected ObjectReader untypedReader() {
+        return 
getJsonMapper().reader().forType(Object).with(DeserializationFeature.USE_BIG_DECIMAL_FOR_FLOATS)

Review Comment:
   Done in 4b2ae6733b. `grails.databinding.json.jackson` (default `false`) 
keeps `JsonSlurper` and logs a startup deprecation notice; `true` parses with 
Boot's mapper. Section 5 lists the inputs from your table. 
`JsonDataBindingSourceCreatorSpec` binds the first five under the default and 
rejects them with the opt-in.



-- 
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]

Reply via email to