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

[Microsoft Graph v6.x.x] Adding a new attendee to an event override the existing attendees. #1898

Open
delenikov opened this issue Mar 22, 2024 · 8 comments
Labels

Comments

@delenikov
Copy link

delenikov commented Mar 22, 2024

I am experiencing an issue with the Microsoft Graph Java SDK after upgrading Microsoft Graph
from version 5.80.0 to 6.4.0 where adding a new attendee to an event overrides the previous attendees.
This problem is similar to this issue reported for the .NET SDK.

Terminology Clarification

In the context of my application, when I refer to "subscribing to an event," it means "adding a new attendee to an existing event" using the Microsoft Graph API. So, each subscription action in my application translates to adding a new attendee entry in the event's attendee list through the Microsoft Graph API.

Expected behavior

There is an Event with 0 attendees.
User A subscribed to this event. Total attendees: 1 (User A)
User B subscribed to this event. Total attendees: 2 (User A, User B)
User C subscribed to this event. Total attendees: 3 (User A, User B, User C)

Actual behavior

There is an Event with 0 attendees.
User A subscribed to this event. Total attendees: 1 (User A)
User B subscribed to this event. Total attendees: 1 (User B)
User C subscribed to this event. Total attendees: 1 (User C)
User A subscribed to this event. Total attendees: 1 (User A)

There is an Event with 3 attendees (User A, User B, User C).
User D subscribed to this event. Total attendees: 1 (User D)

My previous working implementation (microsoft-graph: 5.80.0)

  public boolean subscribeToEvent(String eventId, String calendarId, String email, String remoteUser) {
    String calendarEmail = calendarService.getCalendarEmail(calendarId, remoteUser);
    com.microsoft.graph.models.Event existingEvent = graphClient
        .users(calendarEmail)
        .events(eventId)
        .buildRequest()
        .get();

    com.microsoft.graph.models.Attendee attendee = new com.microsoft.graph.models.Attendee();
    attendee.emailAddress = new com.microsoft.graph.models.EmailAddress();
    attendee.emailAddress.address = email;

    if (existingEvent.attendees == null) {
      existingEvent.attendees = new LinkedList<>();
    }

    existingEvent.attendees.add(attendee);

    graphClient.users(calendarEmail).events(eventId)
      .buildRequest()
      .patch(existingEvent);

    return true;
  } 

Current adapted implementation with issues (microsoft-graph: 6.4.0):

To align with the best practices recommended by Microsoft, I created a new Event object in my application for updating event details, particularly for managing attendees. According to the Microsoft Graph API documentation (updating an event).

In the request body, supply only the values for properties that should be updated. Existing properties that aren't included in the request body maintains their previous values or be recalculated based on changes to other property values.

  @Override
  public boolean subscribeToEvent(String eventId, String calendarId, String email, String remoteUser) {
    String calendarEmail = calendarService.getCalendarEmail(calendarId, remoteUser);
    com.microsoft.graph.models.Event existingEvent = graphClient
      .users().byUserId(calendarEmail)
      .events().byEventId(eventId)
      .get();

    com.microsoft.graph.models.Attendee attendee = new com.microsoft.graph.models.Attendee();

    EmailAddress emailAddress = new com.microsoft.graph.models.EmailAddress();
    emailAddress.setAddress(email);
    attendee.setEmailAddress(emailAddress);

    if (existingEvent.getAttendees() == null) {
      existingEvent.setAttendees(new LinkedList<>());
    }
    existingEvent.getAttendees().add(attendee);

    com.microsoft.graph.models.Event patchedEvent = new com.microsoft.graph.models.Event();
    patchedEvent.setAttendees(existingEvent.getAttendees());

    graphClient.users().byUserId(calendarEmail)
      .events().byEventId(eventId)
      .patch(patchedEvent);

    return true;
  }

Steps to reproduce the behavior

  1. Upgrade the Microsoft Graph SDK from version 5.80.0 to 6.4.0.
  2. Subscribe a user to an event using the application.
  3. Subscribe a different user to the same event.
  4. Observe that the previous subscription is overridden by the most recent subscription.
@ccea-dev-team
Copy link

ccea-dev-team commented Mar 22, 2024

I think you are supposed to send the new full list of attendees and set it in the event object - I don't think just calling add or remove on the existing list works.

image

Edit- above is for the OnlineMeeting object but I assume the Event is the same.

@delenikov
Copy link
Author

delenikov commented Mar 22, 2024

I think you are supposed to send the new full list of attendees and set it in the event object - I don't think just calling add or remove on the existing list works.

image

Edit- above is for the OnlineMeeting object but I assume the Event is the same.

Firstly, I add the new attendee to the existing attendees list:

existingEvent.getAttendees().add(attendee);

Then, I add the list of current attendees (existing attendees + the new attendee) to the new Event object:

patchedEvent.setAttendees(existingEvent.getAttendees());

Finally, I sent the new Event object with only one attribute which is the list of the current attendees (existing attendees + the new attendees):

graphClient.users().byUserId(calendarEmail)
      .events().byEventId(eventId)
      .patch(patchedEvent);

@baywet
Copy link
Member

baywet commented Mar 22, 2024

Thanks for using the SDK and for reporting this.
I created an issue in the upstream dependency. microsoft/kiota-java#1141

@baywet baywet added the bug label Mar 22, 2024
@ccea-dev-team
Copy link

ccea-dev-team commented Mar 22, 2024

@delenikov

Here's my code - it's working fine for me. I send the new updated list of attendees as a List of Strings of email addresses and call .setAttendees() and it behaves as expected.

Event event = new Event();
	
List<Attendee> attendeeList = new ArrayList<>();
	
for(String attendeeEmail : attendeesEmailList)
{
    Attendee att = new Attendee();
		
    EmailAddress attEmail = new EmailAddress();
		
    attEmail.setAddress(attendeeEmail);
		
    att.setEmailAddress(attEmail);
		
    attendeeList.add(att);
}
	
event.setAttendees(attendeeList);
	
Event updatedEvent = graphClient.users().byUserId(eventOwnerEmail).events().byEventId(eventId).patch(event);

@delenikov
Copy link
Author

Thanks for your reply @ccea-dev-team,

I suppose that attendeesEmailList elements are the current attendees of the event, retrieved with:

graphClient.users().byUserId(meetingOwnerEmail).events().byEventId(eventId).get().getAttendees();

If that's the case, the code seems to be reconstructing (remapping) each attendee into a new Attendee object, which could be inefficient and unnecessary.

There are a few potential issues with this approach:

  1. Performance and Memory problems: For events with a large number of attendees, this method could lead to substantial memory usage and performance degradation. Specifically, if an event has many (N) attendees, creating a new Attendee object for each one can rapidly increase the memory footprint because you will always have to create N+1 objects in memory.
    For instance, adding the 501st attendee would unnecessarily create 500 temporary Attendee objects, and this number increases with each new addition.

  2. Loss of Attendee Attributes: By remapping the current attendees to new Attendee objects, there's a risk of losing critical attributes such as status and type. These attributes are important for preserving the detailed context of each attendee within the event, and losing them could compromise the event's data integrity and functionality.

This should not be the case, this is more of a inefficient workaround/hack to overcome the current issue in the 6.x.x version.
The behavior in version 6.x.x should ideally match that of version 5.x.x, eliminating the need to remap each attendee.

@ccea-dev-team
Copy link

ccea-dev-team commented Mar 22, 2024

@delenikov

Thanks for your reply,

The maximum number of attendees to an event is 5000 (used to be 500) and each attendee object is very small - few bytes each for emails and name etc so it won't cause any memory issues.

We maintain a list of the attendees in the application that is calling the webservice - i.e. a list of email addresses of the attendees is stored in a DB table and if the users edits this list through the UI we send the new full list to the web service call. This means we do not have to retrieve the existing Event object to get it's attendee list.

As the doc says the only things that are changed in an update is if the value itself is different - so you won't lose any status or type data of an attendee (unless you have manually set that in the update).

The practice of sending the full new list means that ms graph will automatically detect who is removed and who is added, whereas in your case it looks like you might have seperate methods for adding and removing attendees and have to check the existing list to determine this? i.e. if you send 1 attendee to the web service how do you know if this is someone to add or someone to remove from the event? Do you have 2 different web service calls for this?

Also we have it set up that all events are online meetings and as is says in the docs for OnlineMeetings it needs the full list sent again anyway, so this works for us. Though it is strange that they changed the behaviour so much from version 5.

1

@ccea-dev-team
Copy link

@baywet

In regards to my post above with the documentation of OnlineMeeting saying it "always requires the full list of attendees in the request body" - that bug you have raised states you are going to make sure all collections behave the same. Does this mean you will be changing how OnlineMeeting deals with attendee updates? Also consider that Event objects and OnlineMeeting objects are closely related if the Event has the online meeting flag set. For example if you want to change the co organizer of an online meeting created through an Event you can't do this through the Event object, you first have to get the backing online meeting using the joinurl and set the role on that.

@baywet
Copy link
Member

baywet commented Mar 22, 2024

We're not going to change the behaviour of the underlying REST API as we (my team) don't own the service.
The behaviour of the backing store is going to change slightly so the dirty tracking works closer to what was intended originally and so the serialized value for the patch includes the other members.
This way your application doesn't have to loop through the data and remap it to circumvent the tracking behaviour.
I hope that clarifies the approach?

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

No branches or pull requests

3 participants