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

Make dependency to Okhttp as optional #427

Open
dstoch opened this issue Jan 22, 2025 · 3 comments
Open

Make dependency to Okhttp as optional #427

dstoch opened this issue Jan 22, 2025 · 3 comments

Comments

@dstoch
Copy link

dstoch commented Jan 22, 2025

I reopen an issue from #404.

While the dependency on jakarta.annotation-api is not very problematic, the dependency on okhttp is (these libraries do not have versions as OSGi bundles). Why the need to introduce this dependency from version 1.4.3, it was not there before?
Is it necessary to work always, or only for certain selected functions of moneta-core and could it be optional (Import-package optional)?

BTW:
Generally, using a 3rd party library in such a fairly generic library as Moneta, which in practice provides the appropriate data types, is a bit problematic, if it is not some very standard library. And Okhttp is not a standard one, especially since there are many different http client libraries. If we use different dependencies of this type in the application, such as moneta-core, then in the end it turns out that they transitively pull many different libraries and it often ends with our application suddenly having several different types of libraries for the same purpose (e.g. http client).
I do not know which feature in moneta-core requires http client and whether every application of this library uses it (I suspect it is not necessary). It would probably be better if such http client, if necessary, was pluggable via some interface and we could plug-in any implementation or none at all, if our application does not use this feature.

@keilw
Copy link
Member

keilw commented Jan 22, 2025

Assuming we have enought time and resources (which is rather scarce lately) we could replace Okhttp with the built in JDK HttpClient, but for now Moneta is supposed to still work with Java 8 or 9, and not require 11.

The loaders are part of Moneta-core and I don't think, it could be easily broken up into further modules beyond what is already pretty decoupled. The okhttp package sits next to the legacy defunct urlconnection one, which was error prone and just broken for most conversion rate providers.
The rate providers are modular, the SPI is rather slim and consists of only the Loader feature and some formatting support.
Making the SPI modular, something we did in JSR 363/385 (which is intended for small IoT devices) makes little sense here, especially if we might switch to a "pure" JDK option via HttpClient sooner or later.

The Moneta release intended to have Java 11 as the minimum JDK is 1.5, see #409.

@keilw keilw modified the milestones: 1.6, 1.5 Jan 22, 2025
@dstoch
Copy link
Author

dstoch commented Jan 22, 2025

I don't know moneta features because in our app we use only a couple of simple standard classes/interfaces. I've looked into a code and there is already LoaderService interface with two different implementations: OkHttpLoaderService and URLConnectionLoaderService (deprecated). So there is already an interface (abstraction) which I described in my first post ;).
Interesting thing is that in moneta-core OSGIActivator this old URLConnectionLoaderService is registered as an implementation of the LoaderService. I don't know if it's intentional or an oversight?

PS. Probably for me enough would be to mark all OkHttp imports in Import-Package in MANIFEST.MF as optional.

@keilw
Copy link
Member

keilw commented Jan 23, 2025

I guess in the manifest it may have been overlooked. We could mark it as optional and remove the reference to URLConnectionLoaderService, which should be removed by 1.4.5.
LoaderService is an abstraction, but updating exchange rates among other things will not work unless at least one implementation is also used. As a built-in HTTP Client is going to replace it (whenever we have time to do this, any help is welcome and would speed things up ;-) there is no real reason to create a separate module for the SPI here. Unless you create a very tiny customized IoT version of the JDK yourself, the HTTP Client and its module are also sure to be in every JDK and JRE, so there is no sense in making the loader using it optional.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants