Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Android Record Module #227

Merged
merged 4 commits into from
Nov 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions android-record/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
Module that allows deserialization into records on Android,
where java records are supported through desugaring,
and Jackson's built-in support for records doesn't work,
since the desugared classes have a non-standard super class,
and record component-related reflection methods are missing.

See [Android Developers Blog article](https://android-developers.googleblog.com/2023/06/records-in-android-studio-flamingo.html)

Note: this module is a no-op when no Android-desugared records are being deserialized,
so it is safe to use in code shared between Android and non-Android platforms.

## Usage

Functionality can be used by registering the module and then just deserializing things
using regular API:

```java
ObjectMapper mapper = JsonMapper.builder() // or mapper for other dataformats
.addModule(new AndroidRecordModule())
// add other modules, configure, etc
.build();
```

Maven information for jar is:

* Group id: `com.fasterxml.jackson.module`
* Artifact id: `jackson-module-android-record`

## Other

For Javadocs, Download, see: [Wiki](../../wiki).
97 changes: 97 additions & 0 deletions android-record/pom.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<!-- This module was also published with a richer model, Gradle metadata, -->
<!-- which should be used instead. Do not delete the following line which -->
<!-- is to indicate to Gradle or any Gradle module metadata file consumer -->
<!-- that they should prefer consuming it instead. -->
<!-- do_not_remove: published-with-gradle-metadata -->
<parent>
<groupId>com.fasterxml.jackson.module</groupId>
<artifactId>jackson-modules-base</artifactId>
<version>2.16.0-SNAPSHOT</version>
</parent>
<modelVersion>4.0.0</modelVersion>

<artifactId>jackson-module-android-record</artifactId>
<name>Jackson module: Android Record Support</name>
<packaging>bundle</packaging>

<description>Support deserialization into records on Android</description>
<url>https://github.com/FasterXML/jackson-modules-base</url>

<licenses>
<license>
<name>The Apache Software License, Version 2.0</name>
<url>https://www.apache.org/licenses/LICENSE-2.0.txt</url>
<distribution>repo</distribution>
</license>
</licenses>

<properties>
<!-- Generate PackageVersion.java into this directory. -->
<packageVersion.dir>com/fasterxml/jackson/module/androidrecord</packageVersion.dir>
<packageVersion.package>com.fasterxml.jackson.module.androidrecord</packageVersion.package>
</properties>

<dependencies>
<dependency>
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-core</artifactId>
</dependency>
<dependency>
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-annotations</artifactId>
</dependency>
<dependency>
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-databind</artifactId>
</dependency>
</dependencies>

<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<inherited>true</inherited>
<configuration>
<compilerArgs>
<!-- Module uses constructor parameter names (and types) to identify the canonical constructor -->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah. Because unlike Records there's no specified metadata to fix ordering, so names need to be used.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. I've been wondering if I should document this requirement, but based on my local testing, it seems that the android build system always stores parameter names for desugared record canonical constructors in class files, at least with SDK 34.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added note to doc in #230.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @eranl --merged!

<arg>-parameters</arg>
</compilerArgs>
<fork>true</fork>
<useIncrementalCompilation>true</useIncrementalCompilation>
</configuration>
</plugin>

<plugin>
<!-- Inherited from oss-base. Generate PackageVersion.java.-->
<groupId>com.google.code.maven-replacer-plugin</groupId>
<artifactId>replacer</artifactId>
<executions>
<execution>
<id>process-packageVersion</id>
<phase>generate-sources</phase>
</execution>
</executions>
</plugin>

<plugin>
<groupId>org.moditect</groupId>
<artifactId>moditect-maven-plugin</artifactId>
</plugin>

<plugin>
<groupId>org.codehaus.mojo</groupId>
<artifactId>animal-sniffer-maven-plugin</artifactId>
<version>${version.plugin.animal-sniffer}</version>
<configuration>
<signature>
<groupId>com.toasttab.android</groupId>
<artifactId>gummy-bears-api-${version.android.sdk}</artifactId>
<version>${version.android.sdk.signature}</version>
</signature>
</configuration>
</plugin>
</plugins>
</build>
</project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,166 @@
package com.fasterxml.jackson.module.androidrecord;

import com.fasterxml.jackson.annotation.JacksonInject;
import com.fasterxml.jackson.databind.AnnotationIntrospector;
import com.fasterxml.jackson.databind.BeanDescription;
import com.fasterxml.jackson.databind.DeserializationConfig;
import com.fasterxml.jackson.databind.JavaType;
import com.fasterxml.jackson.databind.PropertyName;
import com.fasterxml.jackson.databind.cfg.MapperConfig;
import com.fasterxml.jackson.databind.deser.CreatorProperty;
import com.fasterxml.jackson.databind.deser.SettableBeanProperty;
import com.fasterxml.jackson.databind.deser.ValueInstantiator;
import com.fasterxml.jackson.databind.deser.std.StdValueInstantiator;
import com.fasterxml.jackson.databind.introspect.AccessorNamingStrategy;
import com.fasterxml.jackson.databind.introspect.AnnotatedClass;
import com.fasterxml.jackson.databind.introspect.AnnotatedConstructor;
import com.fasterxml.jackson.databind.introspect.AnnotatedMethod;
import com.fasterxml.jackson.databind.introspect.AnnotatedParameter;
import com.fasterxml.jackson.databind.introspect.BasicClassIntrospector;
import com.fasterxml.jackson.databind.introspect.DefaultAccessorNamingStrategy;
import com.fasterxml.jackson.databind.introspect.POJOPropertiesCollector;
import com.fasterxml.jackson.databind.module.SimpleModule;
import com.fasterxml.jackson.databind.util.ClassUtil;

import java.lang.reflect.Field;
import java.lang.reflect.Modifier;
import java.lang.reflect.Parameter;
import java.lang.reflect.Type;
import java.util.Arrays;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;


/**
* Module that allows (de)serialization of records using the canonical constructor and accessors on Android,
* where java records are supported through desugaring, and Jackson's built-in support for records doesn't work,
* since the desugared classes have a non-standard super class,
* and record component-related reflection methods are missing.
*
* <p>
* See <a href="https://android-developers.googleblog.com/2023/06/records-in-android-studio-flamingo.html">
* Android Developers Blog article</a>
*
* <p>
* An attempt was made to make this module as consistent with Jackson's built-in support for records as possible,
* but gaps exist when using some of Jackson's advanced mapping features.
*
* <p>
* Note: this module is a no-op when no Android-desugared records are being (de)serialized,
* so it is safe to use in code shared between Android and non-Android platforms.
*
* <p>
* Note: the canonical record constructor is found through matching of parameter names and types with fields.
* Therefore, this module doesn't allow a deserialized desugared record to have a custom constructor
* with the same set of parameter names and types as the canonical one.
*
* @author Eran Leshem
**/
public class AndroidRecordModule extends SimpleModule {
private static final class AndroidRecordNaming
extends DefaultAccessorNamingStrategy
{
/**
* Names of actual Record components from definition; auto-detected.
*/
private final Set<String> _componentNames;

private AndroidRecordNaming(MapperConfig<?> config, AnnotatedClass forClass) {
super(config, forClass,
// no setters for (immutable) Records:
null,
"get", "is", null);
_componentNames = getDesugaredRecordComponents(forClass.getRawType()).map(Field::getName)
.collect(Collectors.toSet());
}

@Override
public String findNameForRegularGetter(AnnotatedMethod am, String name)
{
// By default, field names are un-prefixed, but verify so that we will not
// include "toString()" or additional custom methods (unless latter are
// annotated for inclusion)
if (_componentNames.contains(name)) {
return name;
}
// but also allow auto-detecting additional getters, if any?
return super.findNameForRegularGetter(am, name);
}
}

private static class AndroidRecordClassIntrospector extends BasicClassIntrospector {
@Override
protected POJOPropertiesCollector collectProperties(MapperConfig<?> config, JavaType type, MixInResolver r,
boolean forSerialization) {
if (isDesugaredRecordClass(type.getRawClass())) {
AnnotatedClass classDef = _resolveAnnotatedClass(config, type, r);
AccessorNamingStrategy accNaming = new AndroidRecordNaming(config, classDef);
return constructPropertyCollector(config, classDef, type, forSerialization, accNaming);
}

return super.collectProperties(config, type, r, forSerialization);
}
}

@Override
public void setupModule(SetupContext context) {
super.setupModule(context);
context.addValueInstantiators(AndroidRecordModule::findValueInstantiator);
context.setClassIntrospector(new AndroidRecordClassIntrospector());
Copy link
Member

@cowtowncoder cowtowncoder Nov 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmmmh. This is somewhat problematic as it does not compose; ideally Modules wouldn't have to override it, as only one module can really change it (others would override it).

I'll need to think about this a bit: I can see why this is necessary here, and how it otherwise is quite elegant approach. Just sub-classing part is challenging wrt (lack of) composeability.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does that mean that my statement in the doc that "this module is a no-op when no Android-desugared records are being (de)serialized" is not accurate anymore? That is problematic.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only if some other module tries to override ClassIntrospector -- it is not problem if this is the only module to do so.
So unlike, say, AnnotationIntrospectors that can be chained, there's no existing mechanism to combine ClassIntrospectors

Copy link
Contributor Author

@eranl eranl Nov 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How common is it for modules to do that? Should I document this limitation?
Also, can/should I implement a ClassIntrospector chaining feature?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eranl It might be best to add something in README to mention this.
I don't remember any other module overriding it, so this is theoretical problem, not actual one.
(at least wrt modules under FasterXML)

On chaining... I thought about that, but in general I'm not sure how commonly this is needed etc.
Due to timing it needs to wait until 2.17 (that is, post 2.16.0 and cannot go in a patch) so there's no hurry.
So what I am thinking is that for 2.17 we could consider various approaches to remove this problem; and not have to make last minute changes that may turn out problematic.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eranl It might be best to add something in README to mention this. I don't remember any other module overriding it, so this is theoretical problem, not actual one. (at least wrt modules under FasterXML)

Done. see #230.

On chaining... I thought about that, but in general I'm not sure how commonly this is needed etc. Due to timing it needs to wait until 2.17 (that is, post 2.16.0 and cannot go in a patch) so there's no hurry. So what I am thinking is that for 2.17 we could consider various approaches to remove this problem; and not have to make last minute changes that may turn out problematic.

Sure, I meant in a future version. Please let me know when is a good time, and what's your preferred approach.

}

static boolean isDesugaredRecordClass(Class<?> raw) {
return raw.getSuperclass() != null && raw.getSuperclass().getName().equals("com.android.tools.r8.RecordTag");
}

private static ValueInstantiator findValueInstantiator(DeserializationConfig config, BeanDescription beanDesc,
ValueInstantiator defaultInstantiator) {
Class<?> raw = beanDesc.getType().getRawClass();
if (! defaultInstantiator.canCreateFromObjectWith() && defaultInstantiator instanceof StdValueInstantiator
&& isDesugaredRecordClass(raw)) {
Map<String, Type> components = getDesugaredRecordComponents(raw)
.collect(Collectors.toMap(Field::getName, Field::getGenericType));
cowtowncoder marked this conversation as resolved.
Show resolved Hide resolved
boolean found = false;
for (AnnotatedConstructor constructor: beanDesc.getConstructors()) {
Parameter[] parameters = constructor.getAnnotated().getParameters();
Map<String, Type> parameterTypes = Arrays.stream(parameters)
.collect(Collectors.toMap(Parameter::getName, Parameter::getParameterizedType));
cowtowncoder marked this conversation as resolved.
Show resolved Hide resolved
if (! parameterTypes.equals(components)) {
continue;
}

if (found) {
throw new IllegalArgumentException(String.format(
"Multiple constructors match set of components for record %s", raw.getName()));
}

AnnotationIntrospector intro = config.getAnnotationIntrospector();
SettableBeanProperty[] properties = new SettableBeanProperty[parameters.length];
for (int i = 0; i < parameters.length; i++) {
AnnotatedParameter parameter = constructor.getParameter(i);
JacksonInject.Value injectable = intro.findInjectableValue(parameter);
PropertyName name = intro.findNameForDeserialization(parameter);
if (name == null || name.isEmpty()) {
name = PropertyName.construct(parameters[i].getName());
}

properties[i] = CreatorProperty.construct(name, parameter.getType(),
null, null, parameter.getAllAnnotations(), parameter, i, injectable, null);
}

((StdValueInstantiator) defaultInstantiator).configureFromObjectSettings(null, null, null, null,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks dangerous... having mostly nulls, and overwriting internal state.
Ideally I'd rather create a new instance (even if of type StdValueInstantiator) instead of forcibly trying to modify passed-in instantiator

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've improved the Property construction code. Please take a look.
Regarding modifying the instantiator, I followed this example. Should I use a different pattern?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do. I think that usage pattern is probably fine -- will add a comment if I remember reasons to use different approach.

constructor, properties);
ClassUtil.checkAndFixAccess(constructor.getAnnotated(), false);
found = true;
}
}

return defaultInstantiator;
}

private static Stream<Field> getDesugaredRecordComponents(Class<?> raw) {
return Arrays.stream(raw.getDeclaredFields()).filter(field -> ! Modifier.isStatic(field.getModifiers()));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package @package@;

import com.fasterxml.jackson.core.Version;
import com.fasterxml.jackson.core.Versioned;
import com.fasterxml.jackson.core.util.VersionUtil;

/**
* Automatically generated from PackageVersion.java.in during
* packageVersion-generate execution of maven-replacer-plugin in
* pom.xml.
*/
public final class PackageVersion implements Versioned {
public final static Version VERSION = VersionUtil.parseVersion(
"@projectversion@", "@projectgroupid@", "@projectartifactid@");

@Override
public Version version() {
return VERSION;
}
}
8 changes: 8 additions & 0 deletions android-record/src/main/resources/META-INF/LICENSE
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
This copy of Jackson JSON processor `jackson-module-android-record` module is licensed under the
Apache (Software) License, version 2.0 ("the License").
See the License for details about distribution rights, and the
specific rights regarding derivative works.

You may obtain a copy of the License at:

http://www.apache.org/licenses/LICENSE-2.0
20 changes: 20 additions & 0 deletions android-record/src/main/resources/META-INF/NOTICE
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# Jackson JSON processor

Jackson is a high-performance, Free/Open Source JSON processing library.
It was originally written by Tatu Saloranta (tatu.saloranta@iki.fi), and has
been in development since 2007.
It is currently developed by a community of developers, as well as supported
commercially by FasterXML.com.

## Licensing

Jackson core and extension components may licensed under different licenses.
To find the details that apply to this artifact see the accompanying LICENSE file.
For more information, including possible other licensing options, contact
FasterXML.com (http://fasterxml.com).

## Credits

A list of contributors may be found from CREDITS file, which is included
in some artifacts (usually source distributions); but is always available
from the source code management (SCM) system project uses.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
com.fasterxml.jackson.module.androidrecord.AndroidRecordModule
11 changes: 11 additions & 0 deletions android-record/src/moditect/module-info.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
module com.fasterxml.jackson.module.androidrecord {

requires com.fasterxml.jackson.core;
requires com.fasterxml.jackson.annotation;
requires com.fasterxml.jackson.databind;
eranl marked this conversation as resolved.
Show resolved Hide resolved

exports com.fasterxml.jackson.module.androidrecord;

provides com.fasterxml.jackson.databind.Module with
com.fasterxml.jackson.module.androidrecord.AndroidRecordModule;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package com.android.tools.r8;

/**
* Simulates the super class of Android-desugared records.
*
* @author Eran Leshem
**/
public class RecordTag {
}
Loading