codeconsole commented on code in PR #16414:
URL: https://github.com/apache/grails-core/pull/16414#discussion_r4213490099
##########
grails-converters/src/main/groovy/org/grails/web/converters/configuration/ConvertersConfigurationInitializer.java:
##########
@@ -111,26 +132,75 @@ private void initJSONConfiguration() {
List<ObjectMarshaller<JSON>> marshallers = new ArrayList<>();
marshallers.addAll(getPreviouslyConfiguredMarshallers(JSON.class));
- marshallers.add(new
org.grails.web.converters.marshaller.json.ArrayMarshaller());
- marshallers.add(new
org.grails.web.converters.marshaller.json.ByteArrayMarshaller());
- marshallers.add(new
org.grails.web.converters.marshaller.json.CollectionMarshaller());
- marshallers.add(new
org.grails.web.converters.marshaller.json.MapMarshaller());
- marshallers.add(new
org.grails.web.converters.marshaller.json.SimpleEnumMarshaller());
Config grailsConfig = getGrailsConfig();
+ ProxyHandler proxyHandler = getProxyHandler();
+ boolean legacy =
grailsConfig.getProperty(SETTING_CONVERTERS_JSON_LEGACY, Boolean.class, false);
Review Comment:
Thanks both. I've gone with matrei's reading of the precedent: the new
output stays the 9.0 default, with `grails.converters.json.legacy: true` as the
way back. On the timeline, I agree with both of you that day-one removal was
wrong:
- The setting is no longer deprecated, logs no warning, and names no release
that removes it (1b3a980f0d).
- The "removed in Grails 10" statements are gone from the guide, the
marshaller javadocs, `PrettyPrintJSONWriter` and `JSONParser`. The classes the
switch uses stay plain `@Deprecated(since = "9.0")`, without `forRemoval`.
The switch is now a complete way back, so it's safe to rely on:
- Domain classes render with the Grails 8 marshaller body (e41e7e431d).
- It writes with a default `JsonMapper`, so the application's
`spring.jackson.*` settings and modules can't leak into the text.
- Map keys and `toString(false)` behave as in Grails 8.
`Grails8JsonRenderingSpec` checks all of that against 8.0.x output.
##########
grails-converters/src/main/groovy/org/grails/plugins/converters/ConvertersGrailsPlugin.groovy:
##########
@@ -72,6 +79,20 @@ class ConvertersGrailsPlugin extends Plugin {
}
}
+ // Spring Boot registers every JacksonModule bean with the
application's JsonMapper
+ if
(environment.getProperty(ConvertersConfigurationInitializer.SETTING_CONVERTERS_JSON_DOMAIN_JACKSON_ENABLED,
Boolean, true) &&
Review Comment:
Done in 1b3a980f0d. `grails.converters.json.domain.jackson.enabled` now
defaults to `false`, so Boot's `JsonMapper` renders domain classes as Jackson
beans, as in 8.0.x, unless an application opts in.
The setting no longer depends on `grails.converters.json.legacy`: each
switch now does one thing. Section 2 of the guide describes the module as an
opt-in.
`ConvertersGrailsPluginSpec` covers three cases:
- Boot's mapper by default: the shelf is rendered in full, as a bean.
- The module when enabled.
- `enabled: true` together with `legacy: true`.
##########
grails-converters/src/main/groovy/org/grails/web/converters/marshaller/json/DomainClassMarshaller.java:
##########
@@ -120,154 +99,18 @@ public void setIncludeVersion(boolean includeVersion) {
public boolean supports(Object object) {
String name =
ConverterUtil.trimProxySuffix(object.getClass().getName());
- return application.isArtefactOfType(DomainClassArtefactHandler.TYPE,
name);
+ return application != null &&
application.isArtefactOfType(DomainClassArtefactHandler.TYPE, name);
}
- @SuppressWarnings({ "unchecked", "rawtypes" })
public void marshalObject(Object value, JSON json) throws
ConverterException {
- JSONWriter writer = json.getWriter();
- value = proxyHandler.unwrapIfProxy(value);
- Class<?> clazz = value.getClass();
-
- List<String> excludes = json.getExcludes(clazz);
- List<String> includes = json.getIncludes(clazz);
- IncludeExcludeSupport<String> includeExcludeSupport = new
IncludeExcludeSupport<>();
-
- BeanWrapper beanWrapper = new BeanWrapperImpl(value);
-
- writer.object();
-
- if (includeClass && shouldInclude(includeExcludeSupport, includes,
excludes, value, "class")) {
- writer.key("class").value(clazz.getName());
- }
-
- PersistentEntity domainClass = findDomainClass(value);
-
- if (domainClass == null) {
- throw new GrailsConfigurationException("Could not retrieve the
respective entity for domain " + value.getClass().getName() + " in the mapping
context API");
- }
-
- PersistentProperty id = domainClass.getIdentity();
- if (id != null) {
- //Composite keys dont return an identity. They also do not render
in the JSON.
- //If using Composite keys, it may be advisable to use a customer
Marshaller.
- if (shouldInclude(includeExcludeSupport, includes, excludes,
value, id.getName())) {
- Object idValue = extractValue(value, id);
- if (idValue != null) {
- json.property(id.getName(), idValue);
- }
- }
- }
-
- if (shouldInclude(includeExcludeSupport, includes, excludes, value,
GormProperties.VERSION) && isIncludeVersion()) {
- PersistentProperty versionProperty = domainClass.getVersion();
- Object version = extractValue(value, versionProperty);
- if (version != null) {
- json.property(GormProperties.VERSION, version);
- }
- }
-
- List<PersistentProperty> properties =
domainClass.getPersistentProperties();
-
- for (PersistentProperty property : properties) {
- if (property.equals(domainClass.getVersion())) {
- continue;
- }
-
- if (!shouldInclude(includeExcludeSupport, includes, excludes,
value, property.getName())) continue;
-
- writer.key(property.getName());
- if (!(property instanceof Association)) {
- // Write non-relation property
- Object val = beanWrapper.getPropertyValue(property.getName());
- json.convertAnother(val);
- }
- else {
- Object referenceObject =
beanWrapper.getPropertyValue(property.getName());
- if (isRenderDomainClassRelations()) {
- if (referenceObject == null) {
- writer.valueNull();
- }
- else {
- referenceObject =
proxyHandler.unwrapIfProxy(referenceObject);
- if (referenceObject instanceof SortedMap) {
- referenceObject = new TreeMap((SortedMap)
referenceObject);
- }
- else if (referenceObject instanceof SortedSet) {
- referenceObject = new TreeSet((SortedSet)
referenceObject);
- }
- else if (referenceObject instanceof Set) {
- referenceObject = new LinkedHashSet((Set)
referenceObject);
- }
- else if (referenceObject instanceof Map) {
- referenceObject = new LinkedHashMap((Map)
referenceObject);
- }
- else if (referenceObject instanceof Collection) {
- referenceObject = new ArrayList((Collection)
referenceObject);
- }
- json.convertAnother(referenceObject);
- }
- }
- else {
- if (referenceObject == null) {
- json.value(null);
- }
- else {
-
- PersistentEntity referencedDomainClass =
((Association) property).getAssociatedEntity();
-
- // Embedded are now always fully rendered
- if (referencedDomainClass == null || ((Association)
property).isEmbedded() || property.getType().isEnum()) {
- json.convertAnother(referenceObject);
- }
- else if ((property instanceof OneToOne) || (property
instanceof ManyToOne) || ((Association) property).isEmbedded()) {
- asShortObject(referenceObject, json,
referencedDomainClass.getIdentity(), referencedDomainClass);
- }
- else {
- PersistentProperty referencedIdProperty =
referencedDomainClass.getIdentity();
- @SuppressWarnings("unused")
- String refPropertyName = ((Association)
property).getReferencedPropertyName();
- if (referenceObject instanceof Collection) {
- Collection o = (Collection) referenceObject;
- writer.array();
- for (Object el : o) {
- asShortObject(el, json,
referencedIdProperty, referencedDomainClass);
- }
- writer.endArray();
- }
- else if (referenceObject instanceof Map) {
- Map<Object, Object> map = (Map<Object,
Object>) referenceObject;
- for (Map.Entry<Object, Object> entry :
map.entrySet()) {
- String key =
String.valueOf(entry.getKey());
- Object o = entry.getValue();
- writer.object();
- writer.key(key);
- asShortObject(o, json,
referencedIdProperty, referencedDomainClass);
- writer.endObject();
- }
- }
- }
- }
- }
- }
- }
- writer.endObject();
- }
-
- private PersistentEntity findDomainClass(Object value) {
- for (DomainClassFetcher fetcher : domainClassFetchers) {
- PersistentEntity domain = fetcher.findDomainClass(value);
- if (domain != null) {
- return domain;
- }
- }
- return null;
- }
-
- private boolean shouldInclude(IncludeExcludeSupport<String>
includeExcludeSupport, List<String> includes, List<String> excludes, Object
object, String propertyName) {
- return includeExcludeSupport.shouldInclude(includes, excludes,
propertyName) && shouldInclude(object, propertyName);
+ Object object = proxyHandler.unwrapIfProxy(value);
+ JsonMapperValueMarshaller.write(DomainClassSerializer.value(object,
new Rendering(json, object.getClass())), json);
Review Comment:
Done in e41e7e431d. With the switch on,
`DomainClassMarshaller.marshalObject` runs the Grails 8 body, so subclasses
that override its protected hooks still apply, and `DeepDomainClassMarshaller`
inherits it. The body is the 8.0.x code with one change: a to-many `Map`
association is written as one object of its entries. 8.0.x threw on anything
other than exactly one entry, and for one entry the text is identical.
`Grails8DomainRendering` runs 11 scenarios. The same fixture file ran on
8.0.x (232bb34006) to produce the expected text, and I re-ran the final version
there to confirm it's byte-identical. The scenarios cover:
- associations, a null reference, to-many lists, an embedded value and a
one-entry `Map` association;
- a proxy at the root, and a proxy reference written from its identifier
without loading it;
- `includes` and `excludes`;
- `JSON.use('deep')` and `default.deep`;
- an unsaved instance with a null id and version;
- `includeVersion` and `includeClass`;
- a list of instances;
- `Class`, `Date` and `@JsonValue` map keys.
##########
grails-converters/src/main/groovy/org/grails/web/converters/configuration/ConvertersConfigurationInitializer.java:
##########
@@ -65,6 +67,23 @@ public class ConvertersConfigurationInitializer implements
ApplicationContextAwa
public static final String SETTING_CONVERTERS_JSON_DATE =
"grails.converters.json.date";
public static final String SETTING_CONVERTERS_JSON_DEFAULT_DEEP =
"grails.converters.json.default.deep";
+ /**
+ * Whether domain classes are registered with the application's {@code
JsonMapper}, so that it renders them as the
+ * JSON converter does. Defaults to {@code true}.
+ *
+ * @since 9.0
+ */
+ public static final String SETTING_CONVERTERS_JSON_DOMAIN_JACKSON_ENABLED
= "grails.converters.json.domain.jackson.enabled";
+ /**
+ * Whether the JSON converter renders values as Grails 8 did, with the
marshallers of Grails 8 rather than the
+ * application's {@code JsonMapper}, and the application's {@code
JsonMapper} renders domain classes as Jackson beans.
+ * Defaults to {@code false}.
+ *
+ * @since 9.0
+ * @deprecated a transition to the rendering of Grails 9, to be removed in
Grails 10
+ */
+ @Deprecated(since = "9.0", forRemoval = true)
+ public static final String SETTING_CONVERTERS_JSON_LEGACY =
"grails.converters.json.legacy";
Review Comment:
Added both to `additional-spring-configuration-metadata.json` with their
defaults, `false` for each (1b3a980f0d). The legacy one has no deprecation,
since the setting is no longer deprecated.
--
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]
