Fix undefined behaviour in backing store

[Ndiritu]

Fixes a bug encountered while marking all properties in an InMemoryBackingStore as dirty.

Context

When iterating over a Map entrySet(), any modification to the underlying map causes undefined behavior during the iteration.

The InMemoryBackingStore iterates over the underlying store's entrySet() and currently calls ensureCollectionPropertyIsConsistent() which may update the underlying Map by calling set() to update the collection size & flag the collection as dirty in case of additions/deletions.

Undefined behaviour manifests as repeated iterations over the same set of items in the underlying store causing performance issues. The situation is amplified when we have a collection of BackedModels where each BackedModel has a collection property that has been updated and we need to mark all properties of all the BackedModels as dirty.

Changes

This PR:

• Updates check for collection size consistency to prevent updating the underlying map during iteration.
• Reduces redundant recursive calls when cascading dirty tracking to nested BackedModel elements
• Separates concerns by allowing get() to determine whether a value is dirty or not which matches its interface definition. getValueFromWrapper() now only unwraps values from the underlying store
• Fixes bug to ensure collection properties are checked for consistency before determining if they are dirty when we want to return only changed values

The sample test case would previously fail after running for ~60s vs with this fix, it passes in ~3s on the same hardware.

closes microsoftgraph/msgraph-sdk-java#2106

[baywet]

You might want to look at the follow PRs and see if any of the fixes/optimizations could be ported here.
microsoft/kiota-dotnet#95
microsoft/kiota-dotnet#311
microsoft/kiota-dotnet#326

[Ndiritu]

You might want to look at the follow PRs and see if any of the fixes/optimizations could be ported here. microsoft/kiota-dotnet#95 microsoft/kiota-dotnet#311 microsoft/kiota-dotnet#326

Missing KiotaSerialization helper updates. To be tracked by #1561 next sprint

[baywet]

I'd like @andrueastman revie as well before we merge this one

[andrueastman]

Just a couple of comments from me. Otherwise this looks good to me.
Thanks for looking into this @Ndiritu

[andrueastman]

I think for this we may need to handle the case for maps in separately as this may be an incorrect assumption for them.
Since getObjectArrayFromCollectionWrapper will also unwrap the values of a map as an array, the key values pairs in the map are not guaranteed to have the same type. A map i.e. additionalData can have primitives and complex backed models too..

[andrueastman]

Similar concern here, the cast will most likely fail if we called getObjectArrayFromCollectionWrapper on a map that has mixed types and the first item happened to be a backed model.

[Ndiritu]

Thanks for catching this. Made these updates & added tests.

Tests also exposed a bug where we now need to propagate returnOnlyChangedValues to nested BackedModels to ensure nested models with collection properties are checked. We have limited the collection checks to happen during enumeration only when we want to return changed values.

[sonarqubecloud]

Quality Gate passed

Issues
3 New issues
0 Accepted issues

Measures
0 Security Hotspots
92.9% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

[andrueastman]

👍🏼