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

ChangeNotification and ChangeNotificationCollection model classes missing in v6 #1891

Closed
coinzz opened this issue Mar 19, 2024 · 7 comments · Fixed by #2298
Closed

ChangeNotification and ChangeNotificationCollection model classes missing in v6 #1891

coinzz opened this issue Mar 19, 2024 · 7 comments · Fixed by #2298
Assignees
Labels
feature request priority:p1 High priority/Major issue but not blocking or Big percentage of customers affected.Bug SLA <=7days type:feature New experience request

Comments

@coinzz
Copy link

coinzz commented Mar 19, 2024

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
Copy link
Member

baywet commented Mar 19, 2024

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.

@baywet baywet added the bug label Mar 19, 2024
@Ndiritu Ndiritu added the dependency:metadata Awaiting fix from core dependency project module label Sep 23, 2024
@coinzz
Copy link
Author

coinzz commented Sep 27, 2024

@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
Copy link

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
Copy link

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
Copy link

@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
Copy link
Member

baywet commented Nov 10, 2024

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
Copy link
Member

baywet commented Nov 10, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request priority:p1 High priority/Major issue but not blocking or Big percentage of customers affected.Bug SLA <=7days type:feature New experience request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants