Merge pull request #1142 from microsoft/andrueastman/backingstoreNesting

andrueastman · web-flow · commit fca249341eaf · 2024-03-27T15:13:55.000+03:00
Fixes for nested IBackedModel properties in the InMemoryBackingStore
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,6 +11,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+## [1.1.2] - 2024-03-26
+
+### Changed
+
+- Fixes a bug in the InMemoryBackingStore that would not leave out properties in nested IBackedModel properties.
+
 ## [1.1.1] - 2024-03-20
 
 ### Changed
diff --git a/components/abstractions/spotBugsExcludeFilter.xml b/components/abstractions/spotBugsExcludeFilter.xml
@@ -50,4 +50,12 @@ xsi:schemaLocation="https://github.com/spotbugs/filter/3.0.0 https://raw.githubu
 		<Bug pattern="EI_EXPOSE_REP" />
 		<Class name="com.microsoft.kiota.serialization.mocks.TestEntity" />
 	</Match>
+    <Match>
+        <Bug pattern="EI_EXPOSE_REP" />
+        <Class name="com.microsoft.kiota.TestEntity" />
+    </Match>
+    <Match>
+        <Bug pattern="NP_LOAD_OF_KNOWN_NULL_VALUE" />
+        <Class name="com.microsoft.kiota.store.InMemoryBackingStore" />
+    </Match>
 </FindBugsFilter>
diff --git a/components/abstractions/src/main/java/com/microsoft/kiota/store/InMemoryBackingStore.java b/components/abstractions/src/main/java/com/microsoft/kiota/store/InMemoryBackingStore.java
@@ -6,6 +6,7 @@
 import jakarta.annotation.Nullable;
 
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -33,15 +34,15 @@ public A getValue0() {
         }
 
         public Pair<A, B> setValue0(A value0) {
-            return new Pair(value0, value1);
+            return new Pair<>(value0, value1);
         }
 
         public B getValue1() {
             return value1;
         }
 
         public Pair<A, B> setValue1(B value1) {
-            return new Pair(value0, value1);
+            return new Pair<>(value0, value1);
         }
     }
 
@@ -55,7 +56,15 @@ public void setIsInitializationCompleted(final boolean value) {
         this.isInitializationCompleted = value;
         for (final Map.Entry<String, Pair<Boolean, Object>> entry : this.store.entrySet()) {
             final Pair<Boolean, Object> wrapper = entry.getValue();
-            final Pair<Boolean, Object> updatedValue = wrapper.setValue0(Boolean.valueOf(!value));
+            if (wrapper.getValue1() instanceof BackedModel) {
+                BackedModel backedModel = (BackedModel) wrapper.getValue1();
+                backedModel
+                        .getBackingStore()
+                        .setIsInitializationCompleted(value); // propagate initialization
+            }
+            ensureCollectionPropertyIsConsistent(
+                    entry.getKey(), this.store.get(entry.getKey()).getValue1());
+            final Pair<Boolean, Object> updatedValue = wrapper.setValue0(!value);
             entry.setValue(updatedValue);
         }
     }
@@ -80,10 +89,12 @@ public void clear() {
         final Map<String, Object> result = new HashMap<>();
         for (final Map.Entry<String, Pair<Boolean, Object>> entry : this.store.entrySet()) {
             final Pair<Boolean, Object> wrapper = entry.getValue();
-            final Object value = this.getValueFromWrapper(wrapper);
+            final Object value = this.getValueFromWrapper(entry.getKey(), wrapper);
 
             if (value != null) {
                 result.put(entry.getKey(), wrapper.getValue1());
+            } else if (Boolean.TRUE.equals(wrapper.getValue0())) {
+                result.put(entry.getKey(), null);
             }
         }
         return result;
@@ -101,13 +112,15 @@ public void clear() {
         return result;
     }
 
-    private Object getValueFromWrapper(final Pair<Boolean, Object> wrapper) {
+    private Object getValueFromWrapper(final String entryKey, final Pair<Boolean, Object> wrapper) {
         if (wrapper != null) {
             final Boolean hasChanged = wrapper.getValue0();
-            if (!this.returnOnlyChangedValues
-                    || (this.returnOnlyChangedValues
-                            && hasChanged != null
-                            && hasChanged.booleanValue())) {
+            if (!this.returnOnlyChangedValues || Boolean.TRUE.equals(hasChanged)) {
+                ensureCollectionPropertyIsConsistent(entryKey, wrapper.getValue1());
+                if (wrapper.getValue1() instanceof Pair) {
+                    Pair<?, ?> collectionTuple = (Pair<?, ?>) wrapper.getValue1();
+                    return collectionTuple.getValue0();
+                }
                 return wrapper.getValue1();
             }
         }
@@ -118,7 +131,7 @@ private Object getValueFromWrapper(final Pair<Boolean, Object> wrapper) {
     @Nullable public <T> T get(@Nonnull final String key) {
         Objects.requireNonNull(key);
         final Pair<Boolean, Object> wrapper = this.store.get(key);
-        final Object value = this.getValueFromWrapper(wrapper);
+        final Object value = this.getValueFromWrapper(key, wrapper);
         try {
             return (T) value;
         } catch (ClassCastException ex) {
@@ -128,11 +141,39 @@ private Object getValueFromWrapper(final Pair<Boolean, Object> wrapper) {
 
     public <T> void set(@Nonnull final String key, @Nullable final T value) {
         Objects.requireNonNull(key);
-        final Pair<Boolean, Object> valueToAdd =
-                new Pair(Boolean.valueOf(this.isInitializationCompleted), value);
+        Pair<Boolean, Object> valueToAdd = new Pair<>(this.isInitializationCompleted, value);
+        if (value instanceof Collection) {
+            valueToAdd = valueToAdd.setValue1(new Pair<>(value, ((Collection<?>) value).size()));
+            final Collection<Object> items = (Collection<Object>) value;
+            setupNestedSubscriptions(items, key, value);
+        } else if (value instanceof Map) {
+            valueToAdd = valueToAdd.setValue1(new Pair<>(value, ((Map<?, ?>) value).size()));
+            final Map<?, Object> items = (Map<?, Object>) value;
+            setupNestedSubscriptions(items.values(), key, value);
+        } else if (value instanceof BackedModel) {
+            final BackedModel backedModel = (BackedModel) value;
+            backedModel
+                    .getBackingStore()
+                    .subscribe(
+                            key,
+                            (keyString, oldObject, newObject) -> {
+                                backedModel
+                                        .getBackingStore()
+                                        .setIsInitializationCompleted(
+                                                false); // All its properties are dirty as the model
+                                // has been touched.
+                                set(key, value);
+                            }); // use property name(key) as subscriptionId to prevent excess
+            // subscription creation in the event this is called again
+        }
+
         final Pair<Boolean, Object> oldValue = this.store.put(key, valueToAdd);
         for (final TriConsumer<String, Object, Object> callback : this.subscriptionStore.values()) {
-            callback.accept(key, oldValue.getValue1(), value);
+            if (oldValue != null) {
+                callback.accept(key, oldValue.getValue1(), value);
+            } else {
+                callback.accept(key, null, value);
+            }
         }
     }
 
@@ -154,4 +195,53 @@ public void subscribe(
         Objects.requireNonNull(subscriptionId);
         this.subscriptionStore.put(subscriptionId, callback);
     }
+
+    private void setupNestedSubscriptions(
+            final Collection<Object> items, final String key, final Object value) {
+        for (final Object item : items) {
+            if (item instanceof BackedModel) {
+                final BackedModel backedModel = (BackedModel) item;
+                backedModel.getBackingStore().setIsInitializationCompleted(false);
+                backedModel
+                        .getBackingStore()
+                        .subscribe(key, (keyString, oldObject, newObject) -> set(key, value));
+            }
+        }
+    }
+
+    private void ensureCollectionPropertyIsConsistent(final String key, final Object storeItem) {
+        if (storeItem instanceof Pair) { // check if we put in a collection annotated with the size
+            final Pair<?, Integer> collectionTuple = (Pair<?, Integer>) storeItem;
+            Object[] items;
+            if (collectionTuple.getValue0() instanceof Collection) {
+                items = ((Collection<Object>) collectionTuple.getValue0()).toArray();
+            } else { // it is a map
+                items = ((Map<?, Object>) collectionTuple.getValue0()).values().toArray();
+            }
+
+            for (final Object item : items) {
+                touchNestedProperties(item); // call get on nested properties
+            }
+
+            if (collectionTuple.getValue1()
+                    != items.length) { // and the size has changed since we last updated
+                set(
+                        key,
+                        collectionTuple.getValue0()); // ensure the store is notified the collection
+                // property is "dirty"
+            }
+        }
+        touchNestedProperties(storeItem); // call get on nested properties
+    }
+
+    private void touchNestedProperties(final Object nestedObject) {
+        if (nestedObject instanceof BackedModel) {
+            // Call Get<>() on nested properties so that this method may be called recursively to
+            // ensure collections are consistent
+            final BackedModel backedModel = (BackedModel) nestedObject;
+            for (final String itemKey : backedModel.getBackingStore().enumerate().keySet()) {
+                backedModel.getBackingStore().get(itemKey);
+            }
+        }
+    }
 }
diff --git a/components/abstractions/src/test/java/com/microsoft/kiota/TestEntity.java b/components/abstractions/src/test/java/com/microsoft/kiota/TestEntity.java
@@ -1,16 +1,101 @@
 package com.microsoft.kiota;
 
+import com.microsoft.kiota.serialization.AdditionalDataHolder;
 import com.microsoft.kiota.serialization.Parsable;
 import com.microsoft.kiota.serialization.ParseNode;
 import com.microsoft.kiota.serialization.SerializationWriter;
+import com.microsoft.kiota.store.BackedModel;
+import com.microsoft.kiota.store.BackingStore;
+import com.microsoft.kiota.store.BackingStoreFactorySingleton;
 
 import jakarta.annotation.Nonnull;
 
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.function.Consumer;
 
-public class TestEntity implements Parsable {
+public class TestEntity implements Parsable, AdditionalDataHolder, BackedModel {
+
+    protected BackingStore backingStore;
+
+    /**
+     * Gets the backingStore property value. Stores model information.
+     * @return a {@link BackingStore}
+     */
+    @jakarta.annotation.Nonnull public BackingStore getBackingStore() {
+        return this.backingStore;
+    }
+
+    public String getOdataType() {
+        return this.backingStore.get("@odata.type");
+    }
+
+    public void setOdataType(String odataType) {
+        this.backingStore.set("@odata.type", odataType);
+    }
+
+    public String getId() {
+        return this.backingStore.get("id");
+    }
+
+    public void setId(String _id) {
+        this.backingStore.set("id", _id);
+    }
+
+    public List<String> getBusinessPhones() {
+        return this.backingStore.get("businessPhones");
+    }
+
+    public void setBusinessPhones(List<String> _businessPhones) {
+        this.backingStore.set("businessPhones", _businessPhones);
+    }
+
+    public TestEntity getManager() {
+        return this.backingStore.get("manager");
+    }
+
+    public void setManager(TestEntity manager) {
+        this.backingStore.set("manager", manager);
+    }
+
+    public List<TestEntity> getColleagues() {
+        return this.backingStore.get("colleagues");
+    }
+
+    public void setColleagues(List<TestEntity> colleagues) {
+        this.backingStore.set("colleagues", colleagues);
+    }
+
+    /**
+     * Instantiates a new {@link TestEntity} and sets the default values.
+     */
+    public TestEntity() {
+        this.backingStore = BackingStoreFactorySingleton.instance.createBackingStore();
+        this.setAdditionalData(new HashMap<>());
+        this.setOdataType("#microsoft.graph.testEntity");
+    }
+
+    /**
+     * Gets the AdditionalData property value. Stores additional data not described in the OpenAPI description found when deserializing. Can be used for serialization as well.
+     * @return a {@link Map<String, Object>}
+     */
+    @jakarta.annotation.Nonnull public Map<String, Object> getAdditionalData() {
+        Map<String, Object> value = this.backingStore.get("additionalData");
+        if (value == null) {
+            value = new HashMap<>();
+            this.setAdditionalData(value);
+        }
+        return value;
+    }
+
+    /**
+     * Sets the AdditionalData property value. Stores additional data not described in the OpenAPI description found when deserializing. Can be used for serialization as well.
+     * @param value Value to set for the AdditionalData property.
+     */
+    public void setAdditionalData(@jakarta.annotation.Nullable final Map<String, Object> value) {
+        this.backingStore.set("additionalData", value);
+    }
 
     @Override
     @Nonnull public Map<String, Consumer<ParseNode>> getFieldDeserializers() {
diff --git a/components/abstractions/src/test/java/com/microsoft/kiota/store/InMemoryBackingStoreTest.java b/components/abstractions/src/test/java/com/microsoft/kiota/store/InMemoryBackingStoreTest.java
diff --git a/gradle.properties b/gradle.properties
