Skip to content

Commit

Permalink
Merge branch 'main' into fix-proguard-rules
Browse files Browse the repository at this point in the history
  • Loading branch information
sgjesse authored Aug 21, 2023
2 parents 743d529 + 1e7625b commit 3468700
Show file tree
Hide file tree
Showing 14 changed files with 957 additions and 16 deletions.
19 changes: 19 additions & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,22 @@ jobs:
- name: Build with Maven
# This also runs javadoc:jar to detect any issues with the Javadoc generated during release
run: mvn --batch-mode --update-snapshots --no-transfer-progress verify javadoc:jar

native-image-test:
name: "GraalVM Native Image test"
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v3
- name: "Set up GraalVM"
uses: graalvm/setup-graalvm@v1
with:
java-version: '17'
distribution: 'graalvm'
# According to documentation in graalvm/setup-graalvm this is used to avoid rate-limiting issues
github-token: ${{ secrets.GITHUB_TOKEN }}
cache: 'maven'
- name: Build and run tests
# Only run tests in `graal-native-image-test` (and implicitly build and run tests in `gson`),
# everything else is covered already by regular build job above
run: mvn test --batch-mode --update-snapshots --no-transfer-progress --activate-profiles native-image-test --projects graal-native-image-test --also-make
21 changes: 20 additions & 1 deletion Troubleshooting.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ This guide describes how to troubleshoot common issues when using Gson.
See the [user guide](UserGuide.md#collections-examples) for more information.
- When using `TypeToken` prefer the `Gson.fromJson` overloads with `TypeToken` parameter such as [`fromJson(Reader, TypeToken)`](https://www.javadoc.io/doc/com.google.code.gson/gson/latest/com.google.gson/com/google/gson/Gson.html#fromJson(java.io.Reader,com.google.gson.reflect.TypeToken)).
The overloads with `Type` parameter do not provide any type-safety guarantees.
- When using `TypeToken` make sure you don't capture a type variable. For example avoid something like `new TypeToken<List<T>>()` (where `T` is a type variable). Due to Java type erasure the actual type of `T` is not available at runtime. Refactor your code to pass around `TypeToken` instances or use [`TypeToken.getParameterized(...)`](https://www.javadoc.io/doc/com.google.code.gson/gson/latest/com.google.gson/com/google/gson/reflect/TypeToken.html#getParameterized(java.lang.reflect.Type,java.lang.reflect.Type...)), for example `TypeToken.getParameterized(List.class, elementClass)`.
- When using `TypeToken` make sure you don't capture a type variable. For example avoid something like `new TypeToken<List<T>>()` (where `T` is a type variable). Due to Java [type erasure](https://dev.java/learn/generics/type-erasure/) the actual type of `T` is not available at runtime. Refactor your code to pass around `TypeToken` instances or use [`TypeToken.getParameterized(...)`](https://www.javadoc.io/doc/com.google.code.gson/gson/latest/com.google.gson/com/google/gson/reflect/TypeToken.html#getParameterized(java.lang.reflect.Type,java.lang.reflect.Type...)), for example `TypeToken.getParameterized(List.class, elementType)` where `elementType` is a type you have to provide separately.

## <a id="reflection-inaccessible"></a> `InaccessibleObjectException`: 'module ... does not "opens ..." to unnamed module'

Expand Down Expand Up @@ -336,3 +336,22 @@ For Android you can add this rule to the `proguard-rules.pro` file, see also the
For Android you can alternatively use the [`@Keep` annotation](https://developer.android.com/studio/write/annotations#keep) on the class or constructor you want to keep. That might be easier than having to maintain a custom R8 configuration.

Note that the latest Gson versions (> 2.10.1) specify a default R8 configuration. If your class is a top-level class or is `static`, has a no-args constructor and its fields are annotated with Gson's [`@SerializedName`](https://www.javadoc.io/doc/com.google.code.gson/gson/latest/com.google.gson/com/google/gson/annotations/SerializedName.html), you might not have to perform any additional R8 configuration.

## <a id="typetoken-type-variable"></a> `IllegalArgumentException`: 'TypeToken type argument must not contain a type variable'

**Symptom:** An exception with the message 'TypeToken type argument must not contain a type variable' is thrown

**Reason:** This exception is thrown when you create an anonymous `TypeToken` subclass which captures a type variable, for example `new TypeToken<List<T>>() {}` (where `T` is a type variable). At compile time such code looks safe and you can use the type `List<T>` without any warnings. However, this code is not actually type-safe because at runtime due to [type erasure](https://dev.java/learn/generics/type-erasure/) only the upper bound of the type variable is available. For the previous example that would be `List<Object>`. When using such a `TypeToken` with any Gson methods performing deserialization this would lead to confusing and difficult to debug `ClassCastException`s. For serialization it can in some cases also lead to undesired results.

Note: Earlier version of Gson unfortunately did not prevent capturing type variables, which caused many users to unwittingly write type-unsafe code.

**Solution:**

- Use [`TypeToken.getParameterized(...)`](https://www.javadoc.io/doc/com.google.code.gson/gson/latest/com.google.gson/com/google/gson/reflect/TypeToken.html#getParameterized(java.lang.reflect.Type,java.lang.reflect.Type...)), for example `TypeToken.getParameterized(List.class, elementType)` where `elementType` is a type you have to provide separately.
- For Kotlin users: Use [`reified` type parameters](https://kotlinlang.org/docs/inline-functions.html#reified-type-parameters), that means change `<T>` to `<reified T>`, if possible. If you have a chain of functions with type parameters you will probably have to make all of them `reified`.
- If you don't actually use Gson's `TypeToken` for any Gson method, use a general purpose 'type token' implementation provided by a different library instead, for example Guava's [`com.google.common.reflect.TypeToken`](https://javadoc.io/doc/com.google.guava/guava/latest/com/google/common/reflect/TypeToken.html).

For backward compatibility it is possible to restore Gson's old behavior of allowing `TypeToken` to capture type variables by setting the [system property](https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/lang/System.html#setProperty(java.lang.String,java.lang.String)) `gson.allowCapturingTypeVariables` to `"true"`, **however**:

- This does not solve any of the type-safety problems mentioned above; in the long term you should prefer one of the other solutions listed above. This system property might be removed in future Gson versions.
- You should only ever set the property to `"true"`, but never to any other value or manually clear it. Otherwise this might counteract any libraries you are using which might have deliberately set the system property because they rely on its behavior.
19 changes: 19 additions & 0 deletions graal-native-image-test/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# graal-native-image-test

This Maven module contains integration tests for using Gson in a GraalVM Native Image.

Execution requires using GraalVM as JDK, and can be quite resource intensive. Native Image tests are therefore not enabled by default and the tests are only executed as regular unit tests. To run Native Image tests, make sure your `PATH` and `JAVA_HOME` environment variables point to GraalVM and then run:

```
mvn clean test --activate-profiles native-image-test
```

Technically it would also be possible to directly configure Native Image test execution for the `gson` module instead of having this separate Maven module. However, maintaining the reflection metadata for the unit tests would be quite cumbersome and would hinder future changes to the `gson` unit tests because many of them just happen to use reflection, without all of them being relevant for Native Image testing.

## Reflection metadata

Native Image creation requires configuring which class members are accessed using reflection, see the [GraalVM documentation](https://www.graalvm.org/22.3/reference-manual/native-image/metadata/#specifying-reflection-metadata-in-json).

The file [`reflect-config.json`](./src/test/resources/META-INF/native-image/reflect-config.json) contains this reflection metadata.

You can also run with `-Dagent=true` to let the Maven plugin automatically generate a metadata file, see the [plugin documentation](https://graalvm.github.io/native-build-tools/latest/maven-plugin.html#agent-support-running-tests).
181 changes: 181 additions & 0 deletions graal-native-image-test/pom.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,181 @@
<!--
Copyright 2023 Google Inc.
Licensed 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.
-->
<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 https://maven.apache.org/xsd/maven-4.0.0.xsd">

<modelVersion>4.0.0</modelVersion>

<parent>
<groupId>com.google.code.gson</groupId>
<artifactId>gson-parent</artifactId>
<version>2.10.2-SNAPSHOT</version>
</parent>
<artifactId>graal-native-image-test</artifactId>

<properties>
<!-- GraalVM is JDK >= 17, however for build with regular JDK these tests
are also executed with JDK 11, so for them exclude JDK 17 specific tests -->
<maven.compiler.testRelease>11</maven.compiler.testRelease>
<excludeTestCompilation>**/Java17*</excludeTestCompilation>
</properties>

<dependencies>
<dependency>
<groupId>com.google.code.gson</groupId>
<artifactId>gson</artifactId>
<version>${project.parent.version}</version>
</dependency>

<!-- Graal Native Maven Plugin requires using JUnit Platform (JUnit 5), see
https://graalvm.github.io/native-build-tools/latest/maven-plugin.html#testing-support
This also supports using JUnit Vintage to run JUnit 4 tests, but for simplicity
completely use JUnit 5 here and no JUnit 4 at all -->
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter</artifactId>
<version>5.10.0</version>
<scope>test</scope>
</dependency>

<dependency>
<groupId>com.google.truth</groupId>
<artifactId>truth</artifactId>
<scope>test</scope>
</dependency>
</dependencies>

<build>
<pluginManagement>
<plugins>
<plugin>
<groupId>com.github.siom79.japicmp</groupId>
<artifactId>japicmp-maven-plugin</artifactId>
<configuration>
<!-- This module is not supposed to be consumed as library, so no need to check API -->
<skip>true</skip>
</configuration>
</plugin>
<plugin>
<groupId>org.codehaus.mojo</groupId>
<artifactId>animal-sniffer-maven-plugin</artifactId>
<configuration>
<!-- This module is not supposed to be consumed as library, so no need to check used classes -->
<skip>true</skip>
</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-jar-plugin</artifactId>
<configuration>
<!-- This module has no 'main' source code; skip creating JAR and avoid warning on console -->
<skipIfEmpty>true</skipIfEmpty>
</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-install-plugin</artifactId>
<configuration>
<!-- This module has no 'main' source code, so no JAR is created which could be installed;
see maven-jar-plugin configuration above -->
<skip>true</skip>
</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-deploy-plugin</artifactId>
<configuration>
<!-- Not deployed -->
<skip>true</skip>
</configuration>
</plugin>
</plugins>
</pluginManagement>

<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<executions>
<!-- Adjust standard `default-testCompile` execution -->
<execution>
<id>default-testCompile</id>
<phase>test-compile</phase>
<goals>
<goal>testCompile</goal>
</goals>
<configuration>
<testExcludes>
<exclude>${excludeTestCompilation}</exclude>
</testExcludes>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>

<profiles>
<profile>
<id>JDK17</id>
<activation>
<jdk>[17,)</jdk>
</activation>
<properties>
<maven.compiler.testRelease>17</maven.compiler.testRelease>
<excludeTestCompilation />
</properties>
</profile>

<profile>
<id>native-image-test</id>
<activation>
<activeByDefault>false</activeByDefault>
</activation>
<build>
<plugins>
<plugin>
<groupId>org.graalvm.buildtools</groupId>
<artifactId>native-maven-plugin</artifactId>
<version>0.9.24</version>
<extensions>true</extensions>
<executions>
<execution>
<id>test-native</id>
<goals>
<goal>test</goal>
</goals>
<configuration>
<quickBuild>true</quickBuild>
</configuration>
</execution>
</executions>
<dependencies>
<!-- Workaround for https://github.com/graalvm/native-build-tools/issues/418 -->
<dependency>
<groupId>org.codehaus.plexus</groupId>
<artifactId>plexus-utils</artifactId>
<!-- Don't upgrade version to >= 4 because that seems to contain potentially backward
incompatible changes -->
<version>3.5.1</version>
</dependency>
</dependencies>
</plugin>
</plugins>
</build>
</profile>
</profiles>
</project>
Loading

0 comments on commit 3468700

Please sign in to comment.