codeconsole commented on code in PR #16237:
URL: https://github.com/apache/grails-core/pull/16237#discussion_r4213525378
##########
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 ?
Review Comment:
Fixed in be751d3846. `respond` of a `ProblemDetail` takes the Spring path
whatever the setting: the status comes from the problem, the type is
`application/problem+json`, and an absent `instance` is filled from the
resource path. Covered by `DefaultJsonRendererSpec` 'an explicit ProblemDetail
controls the status content type and instance' and
`ControllerJsonSerializationSpec` 'respond of an explicit problem preserves its
HTTP semantics'.
##########
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)) {
+ return true
+ }
+ if (legacyFallbackReported.compareAndSet(false, true)) {
+ log.warn('respond() renders JSON with the legacy
grails.converters.JSON converter because the ' +
+ 'application customizes it, for example with
JSON.registerObjectMarshaller or an ' +
+ 'ObjectMarshallerRegisterer bean. Legacy marshaller
registration is deprecated for removal: ' +
+ 'replace it with Jackson serializers and set
grails.web.rendering.json.spring to true, or set it ' +
+ 'to false to keep the legacy converter.')
+ }
+ return false
+ }
+
+ private boolean renderWithSpringConverter(Object object, MediaType
mediaType, RenderContext context) {
+ Class<?> objectType = object?.getClass() ?: Object
+ HttpMessageConverter<Object> converter =
(HttpMessageConverter<Object>) resolveSpringHttpMessageConverters().find {
+ HttpMessageConverter<?> candidate ->
candidate.canWrite(objectType, mediaType) &&
+ candidate.getSupportedMediaTypes(objectType).any {
MediaType supported ->
Review Comment:
Fixed in be751d3846: `text/json` selects the converter as
`application/json`, and the response keeps `text/json`.
`DefaultJsonRendererSpec` covers it, and c76fe76b8e adds a `respond` case
(`ControllerTextJsonSpec`, a byte array the legacy converter would write as a
number array).
##########
grails-rest-transforms/src/main/groovy/org/grails/plugins/web/rest/render/SpringMessageConverters.groovy:
##########
@@ -0,0 +1,60 @@
+/*
+ * 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.web.rest.render
+
+import groovy.transform.CompileStatic
+
+import org.springframework.http.converter.HttpMessageConverter
+import org.springframework.web.servlet.config.annotation.WebMvcConfigurer
+
+/**
+ * Captures the message converters Spring MVC ends up configured with.
+ *
+ * <p>Renderers need the same converter list, in the same order, that the
handler adapter uses.
+ * Injecting the adapter to read them forces the whole MVC infrastructure to
be created from a
+ * renderer bean, which risks circular dependencies and defeats lazy startup.
Spring calls
+ * {@link #extendMessageConverters} once with the final list instead, after
every
+ * {@code WebMvcConfigurer} has contributed, so the ordering applications
configure is preserved.</p>
+ *
+ * @since 9.0
+ */
+@CompileStatic
+class SpringMessageConverters implements WebMvcConfigurer {
+
+ private volatile List<HttpMessageConverter<?>> converters = List.of()
+
+ @Override
+ void extendMessageConverters(List<HttpMessageConverter<?>> converters) {
Review Comment:
Fixed in be751d3846. `SpringMessageConverters` no longer implements
`WebMvcConfigurer`. It reads the `requestMappingHandlerAdapter` converters
through an `ObjectProvider` when a response is written. With Spring JSON
enabled and no converters available, the renderer logs once and uses the legacy
converter. Covered by `DefaultRendererRegistrySpec` 'converters come from the
final MVC adapter without a deprecated configurer callback' and
`DefaultJsonRendererSpec` 'Spring JSON without MVC converters falls back to the
legacy converter'.
##########
grails-converters/src/main/groovy/grails/converters/json/NamedJsonConfigurationRegistry.java:
##########
@@ -0,0 +1,131 @@
+/*
+ * 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 grails.converters.json;
+
+import java.io.IOException;
+import java.io.Writer;
+import java.util.List;
+import java.util.Objects;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+import java.util.function.Consumer;
+import java.util.function.Supplier;
+
+import tools.jackson.databind.ObjectWriter;
+import tools.jackson.databind.json.JsonMapper;
+
+import org.grails.web.converters.jackson.GrailsJsonMapperCustomizer;
+
+/**
+ * Registry for request-safe named Jackson response configurations.
+ *
+ * @since 9.0
+ */
+public final class NamedJsonConfigurationRegistry {
+
+ private final Supplier<JsonMapper> jsonMapper;
+ private volatile JsonMapper resolvedMapper;
+ private final ConcurrentMap<String, NamedJsonConfiguration> configurations
= new ConcurrentHashMap<>();
+
+ public NamedJsonConfigurationRegistry(JsonMapper jsonMapper) {
+ this(() -> Objects.requireNonNull(jsonMapper, "jsonMapper"));
+ }
+
+ /**
+ * @param jsonMapper supplies the mapper each configuration derives from,
resolved when a writer
+ * is first needed. Deferring it means the registry can be created before
Jackson
+ * auto-configuration has produced Spring Boot's mapper, and still derive
from that mapper
+ * rather than from a separately configured one. The first successful
resolution is cached;
+ * a missing mapper is retried on the next write.
+ */
+ public NamedJsonConfigurationRegistry(Supplier<JsonMapper> jsonMapper) {
+ this.jsonMapper = Objects.requireNonNull(jsonMapper, "jsonMapper");
+ }
+
+ public void register(String name, Consumer<NamedJsonConfiguration>
customizer) {
+ if (name == null || name.isBlank()) {
+ throw new IllegalArgumentException("Named JSON configuration name
must not be blank.");
+ }
+ Objects.requireNonNull(customizer, "customizer");
+ NamedJsonConfiguration configuration = new
NamedJsonConfiguration(name);
+ customizer.accept(configuration);
+ configurations.put(name, configuration);
+ }
+
+ public boolean contains(String name) {
+ return configurations.containsKey(name);
+ }
+
+ /** @param name the registered name, or null to use the default Grails
writer */
+ public ObjectWriter writer(String name) {
+ NamedJsonConfiguration configuration = name == null ? null :
configurations.get(name);
+ if (name != null && configuration == null) {
+ throw new IllegalArgumentException("Named JSON configuration [" +
name + "] is not registered.");
+ }
+ JsonMapper mapper = resolveMapper();
+ if (mapper == null) {
+ throw new IllegalStateException("Named JSON configuration [" +
name +
+ "] cannot be used: no JsonMapper is available. Spring
Boot's Jackson " +
+ "auto-configuration normally provides one.");
+ }
+ return configuration == null ? mapper.writer() :
configuration.writer(mapper);
+ }
+
+ private JsonMapper resolveMapper() {
+ JsonMapper mapper = resolvedMapper;
+ if (mapper == null) {
+ mapper = jsonMapper.get();
+ if (mapper != null) {
+ resolvedMapper = mapper;
+ }
+ }
+ return mapper;
+ }
+
+ public String writeValueAsString(String name, Object value) {
+ return writer(name).writeValueAsString(value);
+ }
+
+ public void writeValue(String name, Writer output, Object value) throws
IOException {
+ writer(name).writeValue(output, value);
+ }
+
+ /**
+ * Writes with a per-response include/exclude projection applied on top of
the named
+ * configuration, so that selecting a configuration does not discard the
projection.
+ *
+ * @param name the registered configuration
+ * @param output the response writer
+ * @param value the value to write
+ * @param includes property names to include, or null for all
+ * @param excludes property names to exclude, or null for none
+ * @throws IOException if writing fails
+ */
+ public void writeValue(String name, Writer output, Object value,
+ List<String> includes, List<String> excludes) throws IOException {
+ ObjectWriter writer = writer(name);
+ if (includes != null && !includes.isEmpty()) {
+ writer =
writer.withAttribute(GrailsJsonMapperCustomizer.INCLUDES_ATTRIBUTE, includes);
Review Comment:
Applied rather than rejected, in 816df8923f. As with the legacy converter,
`includes`/`excludes` apply to the properties of the written type, or of each
element's type for a collection or array, for domain objects and beans alike. A
serializer modifier on the Grails mapper wraps each bean property writer and
matches bean names as well as Jackson-renamed JSON names. Your `Credentials`
example writes `{"user":"a"}`. Maps and scalars throw
`IllegalArgumentException` before anything is written, and a warning names any
type whose own serializer bypasses the projection. With
`grails.web.rendering.json.spring: true`, `respond x, includes:` without a name
takes the same path, so the serializer no longer depends on whether a
configuration was named. Section 3 describes this. Covered in
`NamedJsonConfigurationRegistrySpec`, `DefaultJsonRendererSpec` and
`ControllerJsonSerializationSpec` 'render json and respond apply a projection
to a bean'.
##########
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);
Review Comment:
Fixed in d81e3956db. When the unwrapped value's class differs from the
entity the serializer was built for, the value is written through
`context.writeValue`, so the subclass's serializer is used.
`GrailsJsonMapperCustomizerSpec` 'a narrowed proxy renders the unwrapped
subclass properties and class name' covers a proxy around a subclass inside a
map.
--
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]
