microsoftgraph / msgraph-sdk-java-core

Microsoft Graph SDK for Java - Core Library
https://docs.microsoft.com/en-us/graph/sdks/sdks-overview
MIT License
54 stars 28 forks source link

Add support for OSGI bundles #414

Open martmoldau opened 2 years ago

martmoldau commented 2 years ago

There are classes used from Gson internal API com.google.gson.internal.Streams is used in com/microsoft/graph/serializer/ODataTypeParametrizedIJsonBackedTypedAdapter.java com.google.gson.internal.bind.ReflectiveTypeAdapterFactory is used in com/microsoft/graph/serializer/FallbackTypeAdapterFactory.java

This is also the reason why this library cannot be used in the OSGI environment as starting from Gson 2.6, the com.google.gson.internal packages are no longer exported in the OSGi bundles.

baywet commented 2 years ago

Hi @martmoldau Thanks for reaching out. FYI we're working on an overhaul of the Java SDK by switching it to our newest generator kiota. This effort will take time, and you should see previews for that newer version in the next few months.

Part of that effort includes changes in the way serialization/deserialization is handled to avoid a strong dependency on any given serialization format/library and allow people to easily replace the implementation should they want to.

We've already had one request for OSGI in the past, could you expand on what scenario it'd unlock for you? what environments are you trying to build a solution for? Thanks!

martmoldau commented 2 years ago

Hi @baywet

Solution I'm working on is integration between different systems including different parts from Microsoft O365. This is based on Apache Camel which runs in OSGI environment Apache Karaf. There are ready made enterprise integration platforms also built on top of these. Red Hat Fuse or Talend for an example. MS Graph API is a big step forward when one has to interface with different parts from MS Cloud. Right now getting it going under the OSGI is quite a hassle as OSGI's classloading is very strict. I had to customise gson package manifest to get around this problem and also a lot of guesswork to explain what packages MS Grapi API needs to import. But it is still worth it. Keep up the good work!

About the OSGI request you were referring. Actually it's still same jar packaging as now. OSGI needs more detailed manifest and that is a task all the build tools can do I guess. I would vote for it:)

baywet commented 2 years ago

Thanks for the additional information. So, if I understand things properly:

Now, here come the questions where I'm still fuzzy on the details:

martmoldau commented 2 years ago

You understood and described things in the first section quite right. Adding manifest properties is actually only reason why they are not OSGI bundles. Adding those properties to manifest will make it OSGI ready, but then it wants to import gson internal classes and this fails because gson OSGI bundle has those internal classes, but for internal use only.

It's not mandatory to create or export any services. Services can be useful when one starts to build some solution specially for OSGI environment, but for a regular java library this is usually not the case. Good enough is when there are package name, description, version, imports and exports listed in the manifest. Imports can be mandatory or optional and also version or version range can be specified e.g. gson >=2.6 and < 3.0.

Dependecies don't need to be OSGI bundles. For a user it is convinient when they are, but adding this extra info into manifest is up to each library maintainer not yours. There can be even different packaging for the same library. for example Kotlin has put together their different libraries into separately packaged kotlin-osgi-bundle. In current ms-graph api dependencies there where only two without OSGI support; com.squareup.okhttp3 and com.squareup.okio if I recall it right. This is usually not a problem when this kind of package has only few dependencies, but can be a headache when there are lot of depencies. Bndtools can be useful in those cases, It scans the code for the imported classes, but cannot help with required library versions or when dynamic classloading is used and this is case with this library too.

baywet commented 2 years ago

Thanks for the additional information.

Okhttp, interestingly enough it seems the only reason why they don't have it is because nobody submitted a pull request for it, having somebody do the work would make all the dependencies compliant according to your analysis.

Would this conflict with JPMS and the META-INF/MANIFESTS.MF we already have in the jar? https://github.com/microsoftgraph/msgraph-sdk-java/issues/297

Our packages are quite large, v1 and beta packages are actually generated from metadata. My search for gradle plugins mostly yielded either compatibility plugins or plugins to help building services. I've found one plugin which would help building the manifest by automatically exploring the sources. Are you aware of anything better? Is the community set on some specific tooling?

martmoldau commented 2 years ago

My analysis may not be comprehensive, but yes it would make life a little easier if one day all dependencies will have entries in manifest for OSGI too. I'm not familiar with gradle otherwise I could do the PR myself. Looks like I should take the time to study it. Meanwhile I can't answer your question about gradle plugins. With Maven I'm using Apache Felix Maven Bundle Plugin wich is using bndtools and seems that there are also couple for Gradle too. But even with the OSGI compliant manifest the main obstacle is still dependency on GSON internal API

JPMS or any other entries in the manifest should not cause problems until the same names are not used for different content and purposes. I found a list of current OSGI header names reference here

baywet commented 2 years ago

Thanks a lot for all those additional details! This is going to be pending on #323 (my initial comment), which is going to take a few months.

baywet commented 2 years ago

In the meantime, would you mind sharing the details for the workaround you used to make the current version work here please?

martmoldau commented 2 years ago

Thank you, @baywet, too! My workaround was to customize gson's package manifest to export the internal packages too, but as this is not a recommended practice I prefer not to share the details here. If one decides to follow my path around gson and installing ms-graph 5.11.0 and ms-graph-core 2.0.10 on Karaf OSGI then following might help a bit:

bundle:install 'wrap:mvn:com.microsoft.graph/microsoft-graph-core/2.0.10/$Bundle-SymbolicName=microsoft-graph-core&Bundle-Version=2.0.10&Export-Package=com.microsoft.graph.authentication;version="2.0.10",com.microsoft.graph.content;version="2.0.10",com.microsoft.graph.core;version="2.0.10",com.microsoft.graph.http;version="2.0.10",com.microsoft.graph.httpcore;version="2.0.10",com.microsoft.graph.httpcore.middlewareoption;version="2.0.10",com.microsoft.graph.logger;version="2.0.10",com.microsoft.graph.options;version="2.0.10",com.microsoft.graph.serializer;version="2.0.10",com.microsoft.graph.tasks;version="2.0.10"&Import-Package=com.microsoft.graph.models,com.microsoft.graph.termstore.models,*;resolution:=optional'

bundle:install 'wrap:mvn:com.microsoft.graph/microsoft-graph/5.11.0/$Bundle-SymbolicName=microsoft-graph&Bundle-Version=5.11.0&Export-Package=com.microsoft.graph.callrecords.models;version="5.11.0",com.microsoft.graph.callrecords.requests;version="5.11.0",com.microsoft.graph.externalconnectors.models;version="5.11.0",com.microsoft.graph.externalconnectors.requests;version="5.11.0",com.microsoft.graph.info;version="5.11.0",com.microsoft.graph.models;version="5.11.0",com.microsoft.graph.requests;version="5.11.0",com.microsoft.graph.termstore.models;version="5.11.0",com.microsoft.graph.termstore.requests;version="5.11.0"'

bundle:install 'mvn:com.google.guava/guava/31.0.1-jre/'
bundle:install 'mvn:com.google.guava/failureaccess/1.0.1/'
bundle:install 'wrap:mvn:com.squareup.okhttp3/okhttp/4.9.3/$Bundle-SymbolicName=okhttp3&Bundle-Version=4.9.3&Export-Package=okhttp3'
bundle:install 'wrap:mvn:com.squareup.okio/okio/2.8.0/$Bundle-SymbolicName=okio3&Bundle-Version=2.8.0'
bundle:install 'mvn:org.jetbrains.kotlin/kotlin-osgi-bundle/1.4.10/'
bundle:install 'mvn:io.projectreactor/reactor-core/3.4.10/'
bundle:install 'mvn:org.reactivestreams/reactive-streams/1.0.3/'

There might be more dependencies needed depending what parts of graph api will be used. In my case it is mostly reading-writing Sharepoint Online lists and Termstore(managed metadata).

baywet commented 2 years ago

thanks for sharing that additional information with the broader community!

baywet commented 2 years ago

To provide additional context on that we're working on a migration to our new SDK generator kiota, this is going to take a while, but part of doing that work will remove a dependency on those internal classes which should unlock that aspect. Version number for core will be 3.X when it reaches preview. We're not doing any work to remove those dependencies on the current major version of core (2.X) because that'd mean breaking current functionality.

maisarissi commented 1 year ago

Adobe have also shown interest for OSGI support in the Java SDK. As mentioned before, OSGI support will be added as part of the core 3.X.

We will consider this feature as part of CY23H2 (2023 second half-year) planning.