List.GetChangesAsync() alters the list internal state

[luciusbv]

Category

• Bug

Describe the bug

Method GetChangesAsync() called on a List instance does so much more than just getting the changes for that list. It alters the list instance state by clearing all requestable collections it finds inside the list.
As a consequence after the call to this method, the fields collection from the list disappears and have to be reloaded again to be used later in the code flow.

Steps to reproduce

// get a list instance by its Guid, including the fields collection
IList changedList = await pnpContext.Web.Lists.GetByIdAsync(listId, p => p.Id, p => p.Title, a => a.Fields.QueryProperties(f => f.InternalName, f => f.TypeAsString, f => f.FieldTypeKind, f => f.Title));
...
//for that list try to get recent changes 
IList<IChange> changedList= await list.GetChangesAsync(new ChangeQueryOptions()
			{
				Item = true,
				Folder = false,
				Add = includeAddChanges,
				Update = includeUpdateChanges,
				DeleteObject = includeDeleteChanges,
				ChangeTokenStart = lastChangeToken,
				FetchLimit = 1000
			});
...
//Trying to enumerate changedList.Fields after this point returns an empty collection

Expected behavior

The state of a list instance should not be changed by a call to GetChangesAsync() method.
GetChangesAsync() should do what its name suggest, not messing with the internal state of the list

Environment details (development & target environment)

• SDK version: [1.5.0]
• OS: [e.g. Windows 10]
• SDK used in: [Azure function]
• Framework: [NET Core v3.1 ]
• Browser(s): [N/A]
• Tooling: [ Visual Studio 2019]
• Additional details: Check next section details.

Additional context

The problem is one related to the fact the List instance is used as a mutable object. The call to GetChangesAsync will cause following relevant effects:

• a batch request is created in order to get the changes data from the server
• the list instance is added inside the batch request component as "Model" for the request
  
  -in BatchClient when method ExecuteBatch is called as a preparing step, all requestable collections are cleared, including the Fields one
  

A possible fix might be: the Model for the Batch request should be a clone of the list instance, not the list itself, but not sure about that...

[luciusbv]

This is a follow up on my comments from #449

[jansenbe]

Hi @luciusbv ,

You're right here, in this case the model collections should not be cleared. I've pushed a fix for this which will be part of the next nightly and upcoming 1.6 release. Thanks for providing this feedback and apologies it took so long to get this issue processed. Closing the issue now, feel free to re-open or create a new one when you still face issues.