feat: introduce java.time methods

[diegomarquezp]

This PR introduces java.time alternatives to existing org.threeten.bp.* methods, as well as switching internal variables (if any) to java.time

The main constraint is to keep the changes backwards compatible, so for each existing threeten method "method1(org.threeten.bp.Duration)" we will add an alternative with a Duration (or Timestamp when applicable) suffix: "method1Duration(java.time.Duration)".

For most cases, the implementation will be held in the java.time method and the old threeten method will just delegate the call to it. However, for the case of abstract classes, the implementation will be kept in the threeten method to avoid breaking changes (i.e. users that already overloaded the method in their user code).

[lqiu96]

qq, why is this DurationSerializer added?

[diegomarquezp]

By default, the Gson serializer will try to access private methods/fields of Duration (and Instant - see the already-existing seralizer above). This seralizer relies on toString() instead.

[lqiu96]

By default, the Gson serializer will try to access private methods/fields of Duration

I'm not following this part. Why does it try to access these methods + fields from Duration? Also, why is this an issue now and not previously?

Is this related to an error message/ stacktrace from a test?

[diegomarquezp]

Why does it try to access these methods + fields from Duration?

I think it's meant to create a JSON string by accessing all its fields (i.e. {"seconds":123,"nanos":123}

Also, why is this an issue now and not previously?

I'm trying to figure this out. The JDK version has both fields as private. So does the backport. Both also implement the same interfaces (Serializable, Comparable, TemporalAmount.
The failing call happens in GSON when it tries to make the fields accessible - see below.

Is this related to an error message/ stacktrace from a test?

com.google.gson.JsonIOException: Failed making field 'java.time.Duration#seconds' accessible; either increase its visibility or write a custom TypeAdapter for its declaring type.
See https://github.com/google/gson/blob/main/Troubleshooting.md#reflection-inaccessible

	at com.google.gson.internal.reflect.ReflectionHelper.makeAccessible(ReflectionHelper.java:76)
	at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory.getBoundFields(ReflectiveTypeAdapterFactory.java:388)
	at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory.create(ReflectiveTypeAdapterFactory.java:161)
	at com.google.gson.Gson.getAdapter(Gson.java:628)
	at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory.createBoundField(ReflectiveTypeAdapterFactory.java:201)
	at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory.getBoundFields(ReflectiveTypeAdapterFactory.java:395)
	at com.google.gson.internal.bind.ReflectiveTypeAdapterFactory.create(ReflectiveTypeAdapterFactory.java:161)
	at com.google.gson.Gson.getAdapter(Gson.java:628)
	at com.google.gson.Gson.toJson(Gson.java:928)
	at com.google.gson.Gson.toJson(Gson.java:899)
	at com.google.gson.Gson.toJson(Gson.java:848)
	at com.google.gson.Gson.toJson(Gson.java:825)
	at com.google.cloud.logging.LogEntry$StructuredLogFormatter.appendField(LogEntry.java:683)
	at com.google.cloud.logging.LogEntry$StructuredLogFormatter.appendField(LogEntry.java:693)
	at com.google.cloud.logging.LogEntry.toStructuredJsonString(LogEntry.java:737)
	at com.google.cloud.logging.LogEntryTest.testStructureLogPresentations(LogEntryTest.java:385)



Caused by: java.lang.reflect.InaccessibleObjectException: Unable to make field private final long java.time.Duration.seconds accessible: module java.base does not "opens java.time" to unnamed module @6996db8
	at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:354)
	at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:297)
	at java.base/java.lang.reflect.Field.checkCanSetAccessible(Field.java:178)
	at java.base/java.lang.reflect.Field.setAccessible(Field.java:172)
	at com.google.gson.internal.reflect.ReflectionHelper.makeAccessible(ReflectionHelper.java:68)
	... 42 more

I'm not sure what this means: java.base does not "opens java.time"

[diegomarquezp]

I believe it has something to do with modules since this behaves perfectly fine if I run the test on java 8.

[lqiu96]

Ok gotcha. Thanks for take a look into this.

Can you see if doing --add-opens java.base/java.time=ALL-UNNAMED would work? Something similar to https://github.com/googleapis/java-datastore/blob/8244ad258a90e5bbc7e7d8ec384555b2be84ab8f/pom.xml#L251. If this does work, I think that could be an option to consider.

Also, can you add a link for that to the PR Description as an explanation for why we are adding this.

The rest LGTM and I think we just need Cindy to review as well.

[diegomarquezp]

Ok, add-opens works but the implementation was slightly different:

• In maven-jar-plugin, we declare a manifest entry that will be detected in Java 9+ only. This is basically the same --add-opens flag and is packed inside the jar as MANIFEST.mf
• Since maven-surefire-plugin does not use the jar directly, I also added a surefire config to include this flag when running the tests

[diegomarquezp]

I oversaw this brings trouble in units(8). Looking into it

[diegomarquezp]

Looks like we are having a limitation. The following code activates the --add-opens flag in tests when the jdk version is >= 9:

java-logging/pom.xml

Lines 305 to 317 in 5a5abd4

	<activation>
	<jdk>[9,)</jdk>
	</activation>
	<build>
	<plugins>
	<plugin>
	<groupId>org.apache.maven.plugins</groupId>
	<artifactId>maven-surefire-plugin</artifactId>
	<configuration>
	<!-- This arg line allows inter-module access when Gson serializes
	the private variables of java.time.Duration -->
	<argLine>--add-opens java.base/java.time=ALL-UNNAMED</argLine>
	</configuration>

However, when running the CIs, we specify the runtime via the -Djvm=... option (absolute path to runtime), which varies among environments - see

java-logging/.github/workflows/ci.yaml

Lines 46 to 58 in 5a5abd4

	with:
	java-version: 8
	distribution: temurin
	- name: "Set jvm system property environment variable for surefire plugin (unit tests)"
	# Maven surefire plugin (unit tests) allows us to specify JVM to run the tests.
	# https://maven.apache.org/surefire/maven-surefire-plugin/test-mojo.html#jvm
	run: echo "SUREFIRE_JVM_OPT=-Djvm=${JAVA_HOME}/bin/java" >> $GITHUB_ENV
	shell: bash
	- uses: actions/setup-java@v4
	with:
	java-version: 17
	distribution: temurin
	- run: .kokoro/build.sh

and

java-logging/.kokoro/build.sh

Lines 49 to 53 in 5a5abd4

	test)
	echo "SUREFIRE_JVM_OPT: ${SUREFIRE_JVM_OPT}"
	mvn test -B -ntp -Dclirr.skip=true -Denforcer.skip=true ${SUREFIRE_JVM_OPT}
	RETURN_CODE=$?
	;;

IIUC we would need a regex matcher for the <activation> entry to only include "paths not containing '1.8'".

[diegomarquezp]

I'll be reverting this maven config in the meantime

[diegomarquezp]

Samples/checkstyle has been failing in other PRs as well

see https://github.com/googleapis/java-logging/actions/runs/11917969552/job/33214317931?pr=1729
see #1728

[lqiu96]

LGTM. Cindy to review for final approval. Adding a Do not merge label until then.