Merge pull request #84 from microsoft/andrueastman/InMemoryBackingStore

Fixes for nested IBackedModel properties in the InMemoryBackingStore

CHANGELOG.md

@@ -9,6 +9,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 
+## [1.1.2] - 2023-05-17
+
+### Changed
+
+- Fixes a bug in the InMemoryBackingStore that would not leave out properties in nested IBackedModel properties.
+
 ## [1.1.1] - 2023-04-06
 
 ### Added

Microsoft.Kiota.Abstractions.Tests/Store/InMemoryBackingStoreTests.cs

@@ -1,4 +1,6 @@
-using System.Collections.Generic;
+using System;
+using System.Collections;
+using System.Collections.Generic;
 using System.Linq;
 using Microsoft.Kiota.Abstractions.Store;
 using Microsoft.Kiota.Abstractions.Tests.Mocks;
@@ -126,6 +128,9 @@ public void TestsBackingStoreEmbeddedInModelWithCollectionPropertyReplacedWithNe
             Assert.NotEmpty(changedValues);
             Assert.Single(changedValues);
             Assert.Equal("businessPhones", changedValues.First().Key);
+            var businessPhones = testUser.BackingStore.Get<List<string>>("businessPhones");
+            Assert.NotNull(businessPhones);
+            Assert.Single(businessPhones);
         }
         [Fact]
         public void TestsBackingStoreEmbeddedInModelWithCollectionPropertyReplacedWithNull()
@@ -152,6 +157,7 @@ public void TestsBackingStoreEmbeddedInModelWithCollectionPropertyReplacedWithNu
             Assert.NotEmpty(changedValuesToNull);
             Assert.Single(changedValuesToNull);
             Assert.Equal("businessPhones", changedValues.First().Key);
+            Assert.Null(changedValues.First().Value);
         }
         [Fact]
         public void TestsBackingStoreEmbeddedInModelWithCollectionPropertyModifiedByAdd()
@@ -174,6 +180,9 @@ public void TestsBackingStoreEmbeddedInModelWithCollectionPropertyModifiedByAdd(
             Assert.NotEmpty(changedValues);
             Assert.Single(changedValues);
             Assert.Equal("businessPhones", changedValues.First().Key);
+            var businessPhones = testUser.BackingStore.Get<List<string>>("businessPhones");
+            Assert.NotNull(businessPhones);
+            Assert.Equal(2,businessPhones.Count);//both items come back as the property is dirty
         }
         [Fact]
         public void TestsBackingStoreEmbeddedInModelWithBySettingNestedIBackedModel()
@@ -195,6 +204,9 @@ public void TestsBackingStoreEmbeddedInModelWithBySettingNestedIBackedModel()
             Assert.NotEmpty(changedValues);
             Assert.Single(changedValues);
             Assert.Equal("manager", changedValues.First().Key);
+            var manager = changedValues.First().Value as TestEntity;
+            Assert.NotNull(manager);
+            Assert.Equal("2fe22fe5-1132-42cf-90f9-1dc17e325a74",manager.Id);
         }
         [Fact]
         public void TestsBackingStoreEmbeddedInModelWithByUpdatingNestedIBackedModel()
@@ -220,6 +232,42 @@ public void TestsBackingStoreEmbeddedInModelWithByUpdatingNestedIBackedModel()
             Assert.NotEmpty(changedValues);
             Assert.Single(changedValues);
             Assert.Equal("manager", changedValues.First().Key);//Backingstore should detect manager property changed
+            var manager = changedValues.First().Value as TestEntity;
+            Assert.NotNull(manager);
+            Assert.Equal("2fe22fe5-1132-42cf-90f9-1dc17e325a74",manager.Id);
+        }
+        [Fact]
+        public void TestsBackingStoreEmbeddedInModelWithByUpdatingNestedIBackedModelReturnsAllNestedProperties()
+        {
+            // Arrange dummy user with initialized backingstore
+            var testUser = new TestEntity
+            {
+                Id = "84c747c1-d2c0-410d-ba50-fc23e0b4abbe",
+                Manager = new TestEntity
+                {
+                    Id = "2fe22fe5-1132-42cf-90f9-1dc17e325a74"
+                }
+            };
+            testUser.BackingStore.InitializationCompleted = testUser.Manager.BackingStore.InitializationCompleted = true;
+            // Act on the data by making a change in the nested Ibackedmodel
+            testUser.Manager.BusinessPhones = new List<string>
+            {
+                "+1 234 567 891"
+            };
+            // Assert by retrieving only changed values
+            testUser.BackingStore.ReturnOnlyChangedValues = true;
+            testUser.Manager.BackingStore.ReturnOnlyChangedValues = true;
+            var changedValues = testUser.BackingStore.Enumerate();
+            Assert.NotEmpty(changedValues);
+            Assert.Single(changedValues);
+            Assert.Equal("manager", changedValues.First().Key);//Backingstore should detect manager property changed
+            var changedNestedValues = testUser.Manager.BackingStore.Enumerate().ToDictionary(x => x.Key, y=> y.Value);
+            Assert.Equal(4,changedNestedValues.Count);
+            Assert.True(changedNestedValues.ContainsKey("id"));
+            Assert.True(changedNestedValues.ContainsKey("businessPhones"));
+            var manager = changedValues.First().Value as TestEntity;
+            Assert.NotNull(manager);
+            Assert.Equal("2fe22fe5-1132-42cf-90f9-1dc17e325a74",manager.Id);
         }
         [Fact]
         public void TestsBackingStoreEmbeddedInModelWithByUpdatingNestedIBackedModelCollectionProperty()
@@ -248,6 +296,121 @@ public void TestsBackingStoreEmbeddedInModelWithByUpdatingNestedIBackedModelColl
             Assert.NotEmpty(changedValues);
             Assert.Single(changedValues);
             Assert.Equal("colleagues", changedValues.First().Key);//Backingstore should detect manager property changed
+            var colleagues = testUser.BackingStore.Get<List<TestEntity>>("colleagues");
+            Assert.NotNull(colleagues);
+            Assert.Equal("2fe22fe5-1132-42cf-90f9-1dc17e325a74",colleagues[0].Id);
+        }
+        [Fact]
+        public void TestsBackingStoreEmbeddedInModelWithByUpdatingNestedIBackedModelCollectionPropertyReturnsAllNestedProperties()
+        {
+            // Arrange dummy user with initialized backingstore
+            var testUser = new TestEntity
+            {
+                Id = "84c747c1-d2c0-410d-ba50-fc23e0b4abbe",
+                Colleagues = new List<TestEntity> 
+                {
+                    new TestEntity
+                    {
+                        Id = "2fe22fe5-1132-42cf-90f9-1dc17e325a74"
+                    }
+                } 
+            };
+            testUser.BackingStore.InitializationCompleted = testUser.Colleagues[0].BackingStore.InitializationCompleted = true;
+            // Act on the data by making a change in the nested Ibackedmodel collection item
+            testUser.Colleagues[0].BusinessPhones = new List<string>
+            {
+                "+1 234 567 891"
+            };
+            // Assert by retrieving only changed values
+            testUser.BackingStore.ReturnOnlyChangedValues = true;
+            testUser.Colleagues[0].BackingStore.ReturnOnlyChangedValues = true; //serializer will do this. 
+            var changedValues = testUser.BackingStore.Enumerate();
+            Assert.NotEmpty(changedValues);
+            Assert.Single(changedValues);
+            Assert.Equal("colleagues", changedValues.First().Key);//Backingstore should detect manager property changed
+            var changedNestedValues = testUser.Colleagues[0].BackingStore.Enumerate().ToDictionary(x => x.Key, y=> y.Value);
+            Assert.Equal(4,changedNestedValues.Count);
+            Assert.True(changedNestedValues.ContainsKey("id"));
+            Assert.True(changedNestedValues.ContainsKey("businessPhones"));
+        }
+        [Fact]
+        public void TestsBackingStoreEmbeddedInModelWithByUpdatingNestedIBackedModelCollectionPropertyWithExtraValueReturnsAllNestedProperties()
+        {
+            // Arrange dummy user with initialized backing store
+            var testUser = new TestEntity
+            {
+                Id = "84c747c1-d2c0-410d-ba50-fc23e0b4abbe",
+                Colleagues = new List<TestEntity> 
+                {
+                    new TestEntity
+                    {
+                        Id = "2fe22fe5-1132-42cf-90f9-1dc17e325a74",
+                        BusinessPhones = new List<string>
+                        {
+                            "+1 234 567 891"
+                        }
+                    }
+                } 
+            };
+            testUser.BackingStore.InitializationCompleted = testUser.Colleagues[0].BackingStore.InitializationCompleted = true;
+            // Act on the data by making a change in the nested Ibackedmodel collection item
+            testUser.Colleagues[0].BusinessPhones.Add("+9 876 543 219");
+            // Assert by retrieving only changed values
+            testUser.BackingStore.ReturnOnlyChangedValues = true;
+            testUser.Colleagues.First().BackingStore.ReturnOnlyChangedValues = true; //serializer will do this. 
+            var changedValues = testUser.BackingStore.Enumerate();
+            Assert.NotEmpty(changedValues);
+            Assert.Single(changedValues);
+            Assert.Equal("colleagues", changedValues.First().Key);//Backingstore should detect manager property changed
+            var changedNestedValues = testUser.Colleagues[0].BackingStore.Enumerate().ToDictionary(x => x.Key, y=> y.Value);
+            Assert.Equal(4,changedNestedValues.Count);
+            Assert.True(changedNestedValues.ContainsKey("id"));
+            Assert.True(changedNestedValues.ContainsKey("businessPhones"));
+            var businessPhones = ((Tuple<ICollection, int>)changedNestedValues["businessPhones"]).Item1;
+            Assert.Equal(2, businessPhones.Count);
+        }
+        [Fact]
+        public void TestsBackingStoreEmbeddedInModelWithByUpdatingNestedIBackedModelCollectionPropertyWithExtraIBackedModelValueReturnsAllNestedProperties()
+        {
+            // Arrange dummy user with initialized backing store
+            var testUser = new TestEntity
+            {
+                Id = "84c747c1-d2c0-410d-ba50-fc23e0b4abbe",
+                Colleagues = new List<TestEntity> 
+                {
+                    new TestEntity
+                    {
+                        Id = "2fe22fe5-1132-42cf-90f9-1dc17e325a74",
+                        BusinessPhones = new List<string>
+                        {
+                            "+1 234 567 891"
+                        }
+                    }
+                } 
+            };
+            testUser.BackingStore.InitializationCompleted = testUser.Colleagues[0].BackingStore.InitializationCompleted = true;
+            // Act on the data by making a change in the nested Ibackedmodel collection item
+            testUser.Colleagues.Add(new TestEntity()
+            {
+                Id = "2fe22fe5-1132-42cf-90f9-1dc17e325a74",
+            });
+            // Assert by retrieving only changed values
+            testUser.BackingStore.ReturnOnlyChangedValues = true;
+            testUser.Colleagues[0].BackingStore.ReturnOnlyChangedValues = true; //serializer will do this. 
+            var changedValues = testUser.BackingStore.Enumerate().ToDictionary(x => x.Key, y=> y.Value);;
+            Assert.NotEmpty(changedValues);
+            Assert.Single(changedValues);
+            Assert.Equal("colleagues", changedValues.First().Key);//Backingstore should detect manager property changed
+            var colleagues = ((Tuple<ICollection, int>)changedValues["colleagues"]).Item1.Cast<TestEntity>().ToList();
+            Assert.Equal(2, colleagues.Count);
+            Assert.Equal("2fe22fe5-1132-42cf-90f9-1dc17e325a74", colleagues[0].Id);
+            Assert.Equal("2fe22fe5-1132-42cf-90f9-1dc17e325a74", colleagues[1].Id);
+            var changedNestedValues = testUser.Colleagues[0].BackingStore.Enumerate().ToDictionary(x => x.Key, y=> y.Value);
+            Assert.Equal(4,changedNestedValues.Count);
+            Assert.True(changedNestedValues.ContainsKey("id"));
+            Assert.True(changedNestedValues.ContainsKey("businessPhones"));
+            var businessPhones = ((Tuple<ICollection, int>)changedNestedValues["businessPhones"]).Item1;
+            Assert.Equal(1, businessPhones.Count);
         }
     }
 }

src/Microsoft.Kiota.Abstractions.csproj

@@ -13,7 +13,7 @@
     <PackageProjectUrl>https://microsoft.github.io/kiota/</PackageProjectUrl>
     <EmbedUntrackedSources>true</EmbedUntrackedSources>
     <Deterministic>true</Deterministic>
-    <VersionPrefix>1.1.1</VersionPrefix>
+    <VersionPrefix>1.1.2</VersionPrefix>
     <VersionSuffix></VersionSuffix>
     <GeneratePackageOnBuild>true</GeneratePackageOnBuild>
     <SignAssembly>false</SignAssembly>

src/store/InMemoryBackingStore.cs

@@ -35,10 +35,10 @@ public class InMemoryBackingStore : IBackingStore
 
             if(store.TryGetValue(key, out var result))
             {
+                EnsureCollectionPropertyIsConsistent(key, result.Item2);
                 var resultObject = result.Item2;
                 if(resultObject is Tuple<ICollection, int> collectionTuple)
                 {
-                    EnsureCollectionPropertyIsConsistent(key, collectionTuple);
                     resultObject = collectionTuple.Item1;// return the actual collection
                 }
 
@@ -70,14 +70,27 @@ public void Set<T>(string key, T? value)
             }
             else if(value is IBackedModel backedModel)
             {// if its the first time adding a IBackedModel property to the store, subscribe to its BackingStore and use the events to flag the property is "dirty"
-                backedModel.BackingStore?.Subscribe((keyString, oldObject, newObject) => Set(key, value));
+                backedModel.BackingStore?.Subscribe((keyString, oldObject, newObject) =>
+                {
+                    backedModel.BackingStore.InitializationCompleted = false;// All its properties are dirty as the model has been touched.
+                    Set(key, value);
+                });
             }
-            else if(value is ICollection collectionValues)
-            {// if its the first time adding a IBackedModel collection property to the store, subscribe to item properties' BackingStores and use the events to flag the collection property is "dirty"
-                collectionValues.OfType<IBackedModel>().ToList().ForEach(model => model.BackingStore?.Subscribe((keyString, oldObject, newObject) => Set(key, value)));
+            // if its an IBackedModel collection property to the store, subscribe to item properties' BackingStores and use the events to flag the collection property is "dirty"
+            if(value is ICollection collectionValues)
+            {
+                // All the list items are dirty as the model has been touched.
+                collectionValues.OfType<IBackedModel>().ToList().ForEach(model =>
+                {
+                    model.BackingStore.InitializationCompleted = false;
+                    model.BackingStore.Subscribe((keyString, oldObject, newObject) =>
+                    {
+                        Set(key, value);
+                    });
+                });
             }
 
-            foreach(var sub in subscriptions.Values)
+            foreach(var sub in subscriptions.Values.ToList())
                 sub.Invoke(key, oldValue?.Item2, value);
         }
 
@@ -155,6 +168,8 @@ public bool InitializationCompleted
                 isInitializationComplete = value;
                 foreach(var key in store.Keys.ToList())
                 {
+                    if(store[key].Item2 is IBackedModel model)
+                        model.BackingStore.InitializationCompleted = value;//forward the initialization status to nested IBackedModel instances
                     EnsureCollectionPropertyIsConsistent(key, store[key].Item2);
                     store[key] = new(!value, store[key].Item2);
                 }
@@ -163,10 +178,21 @@ public bool InitializationCompleted
 
         private void EnsureCollectionPropertyIsConsistent(string key, object? storeItem)
         {
-            if(storeItem is Tuple<ICollection, int> collectionTuple  // check if we put in a collection annotated with the size
-            && collectionTuple.Item2 != collectionTuple.Item1.Count) // and the size has changed since we last updated
+            if(storeItem is Tuple<ICollection, int> collectionTuple)  // check if we put in a collection annotated with the size
+            {
+                // Call Get<>() on nested properties so that this method may be called recursively to ensure collections are consistent
+                collectionTuple.Item1.OfType<IBackedModel>().ToList().ForEach(store => store.BackingStore.Enumerate()
+                    .ToList().ForEach(item => store.BackingStore.Get<object>(item.Key)));
+
+                if(collectionTuple.Item2 != collectionTuple.Item1.Count) // and the size has changed since we last updated)
+                {
+                    Set(key, collectionTuple.Item1); //ensure the store is notified the collection property is "dirty"
+                }
+            }
+            else if(storeItem is IBackedModel backedModel)
             {
-                Set(key, collectionTuple.Item1); //ensure the store is notified the collection property is "dirty"
+                // Call Get<>() on nested properties so that this method may be called recursively to ensure collections are consistent
+                backedModel.BackingStore.Enumerate().ToList().ForEach(item => backedModel.BackingStore.Get<object>(item.Key));
             }
         }
     }