openhab / openhab-core

Core framework of openHAB
https://www.openhab.org/
Eclipse Public License 2.0
925 stars 425 forks source link

Upgrade Gson to 2.10 #3321

Closed jlaur closed 1 year ago

jlaur commented 1 year ago

Now that we have Java 17 support, it would be nice to upgrade Gson to the latest version (currently 2.10.1). Version 2.10 supports Java records: https://github.com/google/gson/releases/tag/gson-parent-2.10. Records are particularly useful when used in combination with Gson for serializing/deserializing from/to DTO's.

I'm working on a new binding (openhab/openhab-addons#14222) where I have initially taken advantage of this:

@NonNullByDefault
public record ElspotpriceRecord(@SerializedName("HourUTC") Instant hour,
        @SerializedName("SpotPriceDKK") double spotPrice) {
}

However, including this Gson dependency within the binding adds more than 250 kB, which is probably not acceptable:

  <dependencies>
    <dependency>
      <groupId>com.google.code.gson</groupId>
      <artifactId>gson</artifactId>
      <version>2.10.1</version>
    </dependency>
  </dependencies>

I tried upgrading from 2.9.1 to 2.10.1 in core, but ended up with a dependency problem:

[ERROR] Failed to execute goal on project org.openhab.core.bom.compile-model: Could not resolve dependencies for project org.openhab.core.bom:org.openhab.core.bom.compile-model:pom:4.0.0-SNAPSHOT: Failed to collect dependencies at org.eclipse.xtext:org.eclipse.xtext.xbase.ide:jar:2.29.0 -> org.eclipse.xtext:org.eclipse.xtext.ide:jar:2.29.0 -> org.eclipse.lsp4j:org.eclipse.lsp4j:jar:0.19.0 -> org.eclipse.lsp4j:org.eclipse.lsp4j.generator:jar:0.19.0 -> org.eclipse.lsp4j:org.eclipse.lsp4j.jsonrpc:jar:0.19.0 -> com.google.code.gson:gson:jar:[2.9.1,2.10): No versions available for com.google.code.gson:gson:jar:[2.9.1,2.10) within specified range -> [Help 1]

I don't know how to fix this, but I'm wondering if it would be possible and acceptable to have both 2.9.1 (because of this dependency) and 2.10.1 in core? At least 2.10.1 would only be included once and all bindings could take advantage of it.

J-N-K commented 1 year ago

Xtext 2.30 is expected at end of February and that updates to GSON 2.10.

wborn commented 1 year ago

I don't know how to fix this, but I'm wondering if it would be possible and acceptable to have both 2.9.1 (because of this dependency) and 2.10.1 in core? At least 2.10.1 would only be included once and all bindings could take advantage of it.

That will cause issues if you develop using Eclipse, because bnd only allows for using one version of a bundle. So it will break either everything Xtext related (depending on the older Gson) or all code depending on the new Gson bundle version.

jlaur commented 1 year ago

For reference: itemis/xtext-reference-projects#318

I don't know how to fix this, but I'm wondering if it would be possible and acceptable to have both 2.9.1 (because of this dependency) and 2.10.1 in core? At least 2.10.1 would only be included once and all bindings could take advantage of it.

That will cause issues if you develop using Eclipse, because bnd only allows for using one version of a bundle. So it will break either everything Xtext related (depending on the older Gson) or all code depending on the new Gson bundle version.

@wborn - hmm, that sounds like a real problem. Unfortunately I'm not very much into Maven and dependencies, but I know the pain from managing dependencies in .NET. This time it seems we might get lucky because xtext is being upgraded soon, but if this wouldn't have happened, our progress could be slowed down a lot when stuck with versions from lowest denominators.

I hope there is some kind of solution for that. I might take the opportunity to learn more about Maven, but would be interested in any tips. 🙂 Anyway, this is slightly off-topic as it can now be considered a more general problem, since we'll hopefully soon be able to upgrade to Gson 2.10 when xtext does, and we are ready to upgrade xtext.

clinique commented 1 year ago

Could we also figure the same with XStream ? Version 1.5.0+ is needed for records.

jlaur commented 1 year ago

xtext 2.30 was released in the beginning of this week: https://www.eclipse.org/Xtext/releasenotes.html#/releasenotes/2023/02/27/version-2-30-0

Previous upgrade for reference: f9a74abf7658ddd4603561fc39cd0bd93dc2d2b1

J-N-K commented 1 year ago

I just tried.. it's a very difficult upgrade. XText now requires reload4j/1.2.24 which is only available in pax-logging-api/2.2.2, our karaf assembly currently provides pax-logging-api/2.2.0. There is now also a require statement in the manifest of org.eclipse.equinox.common for org.eclipse.osgi that we can't easily fulfill. I guess this has to wait.

J-N-K commented 1 year ago

@wborn Do you have an idea how we can get around the equinox runtime requirement for xtext?

wborn commented 1 year ago

I had a look and so far could only get a working distro with Xtext 2.30.0 and Gson 2.10 by manually removing the requirement from the org.eclipse.equinox.common bundle manifest.

J-N-K commented 1 year ago

Do you think we should create our own bundle in openhab-osgiify? Can you push your changes to a branch, I can then check what needs to be done to the bundle.

wborn commented 1 year ago

Have a look at https://github.com/wborn/openhab-core/commit/2a587e2f7e5eeef4cdc1676a4e4d4ed3613a6efe.

I also had to patch the Xtext bundles to work with Pax Logging 2.2.0 but that is hopefully fixed in the next Karaf version. What also seems to fix feature verification is to provide the bundle but use start="false"

jlaur commented 1 year ago

I also had to patch the Xtext bundles to work with Pax Logging 2.2.0 but that is hopefully fixed in the next Karaf version.

It seems with #3814 Pax Logging is at 2.2.3. I'm not sure where that leaves us with Xtext and ultimately Gson?

J-N-K commented 1 year ago

IIRC the issue is that the newer Xtext versions depend on slf4j 2.x and that was not available in pax-logging 2.2.0. pax-logging > 2.2.0 has slf4j, so this should no longer be an issue.

What remains is https://github.com/openhab/openhab-core/issues/3321#issuecomment-1510380505.

wborn commented 1 year ago

I'll have another look and see what happens when upgrading to Xtext 2.32.0 instead of 2.30.0 which I previously tried.

openhab-bot commented 1 year ago

This issue has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/govee-dreamcoulour-led-strips/84435/34

wborn commented 1 year ago

Could we also figure the same with XStream ? Version 1.5.0+ is needed for records.

There doesn't seem to be a XStream 1.5.0 release yet @clinique.