microsoftgraph / msgraph-sdk-java

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

ChangeNotification and ChangeNotificationCollection model classes missing in v6 #1891

Open coinzz opened 7 months ago

coinzz commented 7 months ago

Expected behavior

Existance of these two classes under the package com.microsoft.graph.models. A removal of these classes has not been stated in the changelog. These are necessary to read ChangeNotifications from e.g. EventHubs and are still used in examples, e.g. here.

Actual behavior

Classes are missing and therefore serializer.deserializeObject(jsonPayload, ChangeNotificationCollection.class) can't be used.

Steps to reproduce the behavior

Download current version of graph 6.4.0

baywet commented 7 months ago

Thank you for reporting this. This is a shortcoming of the new pipeline used to generate the new SDK and I've logged an issue at the source of this issue so we can start addressing it. It's however going to take a while to be addressed end to end, and given enough customer demand for this, we might handcraft the missing types like we've done in dotnet.

coinzz commented 1 month ago

@jasonjoh why not copy paste your already handcrafted classes from your here mentioned merge request? After that we can finally start using the v6 as well. Without these classes we still have to stick to v5.

Tushar1337 commented 1 week ago

I want to use Retry for batching which is available in v6 but this is giving me issues and I'm not able to upgrade any way to tackle this? how to consume outlook notification callback when ChangeNotification is not available ?

BastiKiel commented 1 week ago

In my project I could make good use of the batching enhancements V6 seems to bring. V6 is out quite some time now, still there's no change in regard to adding missing classes. For what it's worth I really like to see those classes added, otherwise upgrading from V5 to V6 is out of the question.

Tushar1337 commented 1 week ago

@baywet can you guide me here on how to make the necessary changes for this can I copy the model from v5.7 to v6.0 and raise a PR will it work? Sorry, I've not really worked in this kind of repository any help would be appreciated here.

baywet commented 2 days ago

Thanks everyone for your patience with that. Since there's a lot of popular demand for that, we might switch our approach and handcraft the types instead of relying on generation. This is because there are no existing ways to express a "callback/webhook/change notification" construct in OData/CSDL today. Which means the types are missing during the generation as they are not referenced anywhere.

Expanding on handcrafted types, implementing this is going to require two parts:

Core SDK

From the following directory in C#, we'll want to port:

From this other directory we'll need to port:

(as static methods since extension methods do not exist in Java)

This repo/service library

From this directory in C#, we'll need to port:

Now, we'll be more than happy to receive and merge pull requests, this is a lot of work however. Please let us know if any of you has interest in driving this forward.

baywet commented 2 days ago

Note: we already had an implementation of change notifications that relied on the earlier designs (serialization, etc...) that's also worth looking at. But it'll need to be ported to the newer serialization infrastructure to function properly.