jedis icon indicating copy to clipboard operation
jedis copied to clipboard

Make JSON imports optional in OSGi environments

Open rombert opened this issue 1 year ago • 12 comments

When deploying in OSGi containers the Jedis bundle has two hard requirements on having the GSON and JSON bundles around. Since these depenendencies are optional we mark the imports as such in the OSGi manifest.

If these packages are not exported the bundle will be installed and will be unable to make use of the JSON functionality. When they are present they will be used.

Additionally, allow newer version of the org.json bundle to be used without requiring a new release of Jedis.

Fixes #3956 .

rombert avatar Sep 27 '24 12:09 rombert

I was wondering: if we are able to use the Jedis driver without these two - shouldn't we mark them as optional in the Maven dependency tree too. If we do then the maven-bundle-plugin would pick this up and generate the same result in the MANIFEST.MF, if I am not mistaken.

tishun avatar Oct 10 '24 07:10 tishun

I was wondering: if we are able to use the Jedis driver without these two - shouldn't we mark them as optional in the Maven dependency tree too. If we do then the maven-bundle-plugin would pick this up and generate the same result in the MANIFEST.MF, if I am not mistaken.

Yes, if the Maven dependencies are marked as optional the maven-bundle-plugin should also make the imports optional. The version pinning for the org.json import, if deemed acceptable, still needs to stay.

rombert avatar Oct 10 '24 09:10 rombert

I was wondering: if we are able to use the Jedis driver without these two - shouldn't we mark them as optional in the Maven dependency tree too. If we do then the maven-bundle-plugin would pick this up and generate the same result in the MANIFEST.MF, if I am not mistaken.

Yes, but this is a breaking change and has been considered as unfavorable #3278

sazzad16 avatar Oct 10 '24 10:10 sazzad16

I was wondering: if we are able to use the Jedis driver without these two - shouldn't we mark them as optional in the Maven dependency tree too. If we do then the maven-bundle-plugin would pick this up and generate the same result in the MANIFEST.MF, if I am not mistaken.

Yes, but this is a breaking change and has been considered as unfavorable #3278

Apologies, was not aware of #3278, makes sense.

In this case, for OSGi specifically, this seems a reasonable improvement. AFAIK most (all?) OSGi containers depend on the user to deploy the artefacts (e.g. by using a WAR file or a nested JAR file or some other way) and the MANIFEST.MF only serves to mandate what should be loaded, so we should not have the same issues as the ones described in #3278

tishun avatar Oct 11 '24 08:10 tishun

OSGi containers depend on the user to deploy the artefacts (e.g. by using a WAR file or a nested JAR file or some other way) and the MANIFEST.MF only serves to mandate what should be loaded

Yes, that is correct. In the current form of Jedis the two JSON bundles must be deployed, even if they are not used by the application.

rombert avatar Oct 11 '24 08:10 rombert

@rombert Apologies. We're still wait for a resolution from the higher-ups.

FYI, making the concerned dependencies totally optional in next major release is being considered.

sazzad16 avatar Nov 24 '24 06:11 sazzad16

Ack, thanks @sazzad16 . Let me know if there is information/testing I can provide in the meantime.

rombert avatar Nov 25 '24 08:11 rombert

@sazzad16 - I see the 6.0 version is now out. Has there been any discussion on the topic of optional dependencies with regards to OSGi deployments?

rombert avatar Jun 18 '25 14:06 rombert

@rombert Sorry for the lack of updates — the team got pulled into other priorities, and this one unfortunately slipped through. I’ll make sure to include it in the next major milestone so it stays visible, and I’ll also bring it up for discussion again soon.

ggivo avatar Jun 18 '25 16:06 ggivo

Thanks @ggivo , will keep watching this.

rombert avatar Jun 19 '25 07:06 rombert

Hi @rombert,

Discussed with the team and we decided to review the JSON integration first before proceeding with any changes related to removing the optional dependency.

There are also some other open questions around JSON integration, such as the deprecation of the v1 path, that we want to address.

We’ll plan to move this work to the next release.

ggivo avatar Oct 10 '25 06:10 ggivo

Thanks for the update @ggivo ,

I did not want to touch it as part of my upgrades, but if there is an option to revisit this and use a different third party library I would suggest using the Jakarta EE JSON Processing API ( https://github.com/jakartaee/jsonp-api ) and letting deployers plug in their own implementation.

rombert avatar Oct 10 '25 10:10 rombert