Implement selective children retrieval

References #21
Lawo · Apr 26, 2016 · c8a723e · c8a723e
1 parent 4dfd3a1
commit c8a723e
Show file tree

Hide file tree

Showing 16 changed files with 122 additions and 44 deletions.
diff --git a/Lawo.EmberPlusSharp/Model/CollectionNode.cs b/Lawo.EmberPlusSharp/Model/CollectionNode.cs
@@ -61,17 +61,17 @@ internal sealed override Element ReadNewChildContents(
         }
 
         [SuppressMessage("Microsoft.Design", "CA1062:Validate arguments of public methods", Justification = "Method is not public, CA bug?")]
-        internal sealed override bool ChangeVisibility(Element child)
+        internal sealed override bool ChangeVisibility(IElement child)
         {
             base.ChangeVisibility(child);
 
-            if (child.RetrieveDetails)
+            if (child.IsOnline)
             {
-                this.children.Add((TElement)(IElement)child);
+                this.children.Add((TElement)child);
             }
             else
             {
-                this.children.Remove((TElement)(IElement)child);
+                this.children.Remove((TElement)child);
             }
 
             return true;

diff --git a/Lawo.EmberPlusSharp/Model/DynamicFieldNode.cs b/Lawo.EmberPlusSharp/Model/DynamicFieldNode.cs
@@ -33,7 +33,7 @@ public ReadOnlyObservableCollection<IElement> DynamicChildren
 
         ////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 
-        internal sealed override bool ChangeVisibility(Element child)
+        internal sealed override bool ChangeVisibility(IElement child)
         {
             return DynamicNodeHelper.ChangeVisibility(base.ChangeVisibility, this.dynamicChildren, child);
         }

diff --git a/Lawo.EmberPlusSharp/Model/DynamicNodeHelper.cs b/Lawo.EmberPlusSharp/Model/DynamicNodeHelper.cs
@@ -31,11 +31,11 @@ internal static Element ReadDynamicChildContents(
         }
 
         internal static bool ChangeVisibility(
-            Func<Element, bool> baseImpl, ObservableCollection<IElement> dynamicChildren, Element child)
+            Func<IElement, bool> baseImpl, ObservableCollection<IElement> dynamicChildren, IElement child)
         {
             if (!baseImpl(child))
             {
-                if (child.RetrieveDetails)
+                if (child.IsOnline)
                 {
                     dynamicChildren.Add(child);
                 }

diff --git a/Lawo.EmberPlusSharp/Model/DynamicRoot.cs b/Lawo.EmberPlusSharp/Model/DynamicRoot.cs
@@ -41,7 +41,7 @@ internal sealed override Element ReadNewDynamicChildContents(
             return DynamicNodeHelper.ReadDynamicChildContents(reader, actualType, context, out childRetrievalState);
         }
 
-        internal sealed override bool ChangeVisibility(Element child)
+        internal sealed override bool ChangeVisibility(IElement child)
         {
             return DynamicNodeHelper.ChangeVisibility(base.ChangeVisibility, this.dynamicChildren, child);
         }

diff --git a/Lawo.EmberPlusSharp/Model/Element.cs b/Lawo.EmberPlusSharp/Model/Element.cs
@@ -72,18 +72,11 @@ public bool IsOnline
 
             internal set
             {
+                var oldValue = this.RetrieveDetails;
+
                 if (this.SetValue(ref this.isOnline, value))
                 {
-                    // We're deliberately not simply setting this to Changed here, because we want to correctly handle
-                    // the case when IsOnline is changed twice without being observed between the changes.
-                    if (this.RetrieveDetailsChangeStatus == RetrieveDetailsChangeStatus.Unchanged)
-                    {
-                        this.RetrieveDetailsChangeStatus = RetrieveDetailsChangeStatus.Changed;
-                    }
-                    else if (this.RetrieveDetailsChangeStatus == RetrieveDetailsChangeStatus.Changed)
-                    {
-                        this.RetrieveDetailsChangeStatus = RetrieveDetailsChangeStatus.Unchanged;
-                    }
+                    this.AdaptRetrieveDetailsChangeStatus(oldValue);
                 }
             }
         }
@@ -148,6 +141,23 @@ internal virtual bool RetrieveDetails
 
         internal RetrieveDetailsChangeStatus RetrieveDetailsChangeStatus { get; set; }
 
+        internal void AdaptRetrieveDetailsChangeStatus(bool oldValue)
+        {
+            if (this.RetrieveDetails != oldValue)
+            {
+                // We're deliberately not simply setting this to Changed here, because we want to correctly handle
+                // the case when IsOnline is changed twice without being observed between the changes.
+                if (this.RetrieveDetailsChangeStatus == RetrieveDetailsChangeStatus.Unchanged)
+                {
+                    this.RetrieveDetailsChangeStatus = RetrieveDetailsChangeStatus.Changed;
+                }
+                else if (this.RetrieveDetailsChangeStatus == RetrieveDetailsChangeStatus.Changed)
+                {
+                    this.RetrieveDetailsChangeStatus = RetrieveDetailsChangeStatus.Unchanged;
+                }
+            }
+        }
+
         internal virtual RetrievalState RetrievalState
         {
             get { return RetrievalState.Complete; }
@@ -214,13 +224,13 @@ internal void SetConsumerValue<T>(ref T field, T newValue, [CallerMemberName] st
         /// interested in. In all other cases, the state of the node is lowered to the lowest state of the
         /// interesting children appearing in the payload.</para>
         /// <para>This approach ensures that any node for which incomplete interesting children have been received will
-        /// be visited by <see cref="UpdateRetrievalState"/>. This is necessary because some providers send messages with
-        /// payloads where the same node appears multiple times. For example, the first time the state of the node may
-        /// be set to <see cref="RetrievalState.None"/>, due to the fact that there are indirect children for which a
-        /// <code>getDirectory</code> request needs to be sent. The second time the node appears only with direct and
-        /// indirect children that have the state <see cref="RetrievalState.Complete"/>. Now the state of the node cannot
-        /// be set to <see cref="RetrievalState.Complete"/> because then no <code>getDirectory</code> requests would be
-        /// issued for the children that appeared the first time.</para>
+        /// be visited by <see cref="UpdateRetrievalState"/>. This is necessary because some providers send messages
+        /// with payloads where the same node appears multiple times. For example, the first time the state of the node
+        /// may be set to <see cref="RetrievalState.None"/>, due to the fact that there are indirect children for which
+        /// a <code>getDirectory</code> request needs to be sent. The second time the node appears only with direct and
+        /// indirect children that have the state <see cref="RetrievalState.Complete"/>. Now the state of the node
+        /// cannot be set to <see cref="RetrievalState.Complete"/> because then no <code>getDirectory</code> requests
+        /// would be issued for the children that appeared the first time.</para>
         /// <para>It follows that the state of a node cannot be set to its definitive value while its children are read.
         /// Instead there needs to be a second step that visits all affected nodes and updates their state, which is
         /// implemented by <see cref="UpdateRetrievalState"/>.</para></remarks>

diff --git a/Lawo.EmberPlusSharp/Model/FieldNode.cs b/Lawo.EmberPlusSharp/Model/FieldNode.cs
@@ -57,7 +57,7 @@ internal virtual Element ReadNewDynamicChildContents(
         }
 
         [SuppressMessage("Microsoft.Design", "CA1062:Validate arguments of public methods", Justification = "Method is not public, CA bug?")]
-        internal override bool ChangeVisibility(Element child)
+        internal override bool ChangeVisibility(IElement child)
         {
             base.ChangeVisibility(child);
             MetaElement metaChild;

diff --git a/Lawo.EmberPlusSharp/Model/MetaElement.cs b/Lawo.EmberPlusSharp/Model/MetaElement.cs
@@ -46,7 +46,7 @@ internal abstract Element ReadContents(
 
             internal abstract bool IsAvailable(IParent parent, bool throwIfMissing);
 
-            internal abstract void ChangeVisibility(IParent parent, Element element);
+            internal abstract void ChangeVisibility(IParent parent, IElement element);
 
             ////////////////////////////////////////////////////////////////////////////////////////////////////////////
 

diff --git a/Lawo.EmberPlusSharp/Model/MetaElement1.cs b/Lawo.EmberPlusSharp/Model/MetaElement1.cs
@@ -66,16 +66,16 @@ internal sealed override bool IsAvailable(IParent parent, bool throwIfMissing)
             }
 
             [SuppressMessage("Microsoft.Design", "CA1062:Validate arguments of public methods", Justification = "Method is not public, CA bug?")]
-            internal sealed override void ChangeVisibility(IParent parent, Element element)
+            internal sealed override void ChangeVisibility(IParent parent, IElement element)
             {
-                if (!this.IsOptional && !element.RetrieveDetails)
+                if (!this.IsOptional && !element.IsOnline)
                 {
                     const string Format =
                         "The required property {0}.{1} in the node with the path {2} has been set offline by the provider.";
                     throw CreateRequiredPropertyException(parent, Format);
                 }
 
-                this.set((TMostDerived)parent, (TProperty)(element.RetrieveDetails ? element : null));
+                this.set((TMostDerived)parent, (TProperty)(element.IsOnline ? element : null));
                 parent.OnPropertyChanged(new PropertyChangedEventArgs(this.Property.Name));
             }
 

diff --git a/Lawo.EmberPlusSharp/Model/Node.cs b/Lawo.EmberPlusSharp/Model/Node.cs
@@ -71,9 +71,9 @@ internal virtual bool GetIsRoot()
         }
 
         [SuppressMessage("Microsoft.Design", "CA1062:Validate arguments of public methods", Justification = "Method is not public, CA bug?")]
-        internal override bool ChangeVisibility(Element child)
+        internal override bool ChangeVisibility(IElement child)
         {
-            if (child.RetrieveDetails)
+            if (child.IsOnline)
             {
                 this.observableChildren.Add(child);
             }

diff --git a/Lawo.EmberPlusSharp/Model/NodeBase.cs b/Lawo.EmberPlusSharp/Model/NodeBase.cs
@@ -53,7 +53,9 @@ public ChildrenRetrievalPolicy ChildrenRetrievalPolicy
                             "value");
                     }
 
+                    var oldValue = this.RetrieveDetails;
                     this.SetValue(ref this.childrenRetrievalPolicy, value);
+                    this.AdaptRetrieveDetailsChangeStatus(oldValue);
                 }
             }
         }
@@ -84,6 +86,14 @@ internal NodeBase() : base(RetrievalState.None)
         {
         }
 
+        internal sealed override bool RetrieveDetails
+        {
+            get
+            {
+                return (this.ChildrenRetrievalPolicy != ChildrenRetrievalPolicy.None) && base.RetrieveDetails;
+            }
+        }
+
         internal sealed override void SetContext(Context context)
         {
             base.SetContext(context);
@@ -170,7 +180,7 @@ internal virtual bool ReadChildrenCore(EmberReader reader)
         /// <summary>Changes the visibility of <paramref name="child"/>.</summary>
         /// <param name="child">The child to change the visibility for.</param>
         /// <returns><c>true</c> if the visibility has been changed; otherwise, <c>false</c>.</returns>
-        internal virtual bool ChangeVisibility(Element child)
+        internal virtual bool ChangeVisibility(IElement child)
         {
             return false;
         }

diff --git a/Lawo.EmberPlusSharp/Model/RetrieveDetailsChangeStatus.cs b/Lawo.EmberPlusSharp/Model/RetrieveDetailsChangeStatus.cs
@@ -8,8 +8,8 @@ namespace Lawo.EmberPlusSharp.Model
 {
     internal enum RetrieveDetailsChangeStatus
     {
-        /// <summary><see cref="Element.RetrieveDetails"/> has the value set during initialization or first read operation.
-        /// </summary>
+        /// <summary><see cref="Element.RetrieveDetails"/> has the value set during initialization or first read
+        /// operation.</summary>
         Initialized,
 
         /// <summary><see cref="Element.RetrieveDetails"/> has not been changed since

diff --git a/Lawo.EmberPlusSharpTest/Lawo.EmberPlusSharpTest.csproj b/Lawo.EmberPlusSharpTest/Lawo.EmberPlusSharpTest.csproj
@@ -581,7 +581,17 @@
     <EmbeddedResource Include="Model\Test\EmberDataPayloads\StreamLog.xml" />
   </ItemGroup>
   <ItemGroup>
-    <EmbeddedResource Include="Model\Test\EmberDataPayloads\ChildrenRetrievalPolicyLog.xml">
+    <EmbeddedResource Include="Model\Test\EmberDataPayloads\ChildrenRetrievalPolicyLog1.xml">
+      <SubType>Designer</SubType>
+    </EmbeddedResource>
+  </ItemGroup>
+  <ItemGroup>
+    <EmbeddedResource Include="Model\Test\EmberDataPayloads\ChildrenRetrievalPolicyLog2.xml">
+      <SubType>Designer</SubType>
+    </EmbeddedResource>
+  </ItemGroup>
+  <ItemGroup>
+    <EmbeddedResource Include="Model\Test\EmberDataPayloads\ChildrenRetrievalPolicyLog3.xml">
       <SubType>Designer</SubType>
     </EmbeddedResource>
   </ItemGroup>

diff --git a/Lawo.EmberPlusSharpTest/Model/ConsumerTest.cs b/Lawo.EmberPlusSharpTest/Model/ConsumerTest.cs
@@ -240,6 +240,10 @@ await MonitorConnection(
                             Assert.IsTrue(root.GetPath() == "/");
                             Assert.AreEqual(true, root.IsRoot);
                             Assert.AreEqual(true, root.IsOnline);
+                            Assert.IsNull(root.Tag);
+                            var randomValue = new Random().Next();
+                            root.Tag = randomValue;
+                            Assert.AreEqual(randomValue, root.Tag);
 
                             Assert.AreEqual(null, ((IParameter)root.GetChild("OctetstringParameter")).Minimum);
                             Assert.AreEqual(null, ((IParameter)root.GetChild("OctetstringParameter")).Maximum);
@@ -451,9 +455,9 @@ public void ChildrenRetrievalPolicyTest()
             AsyncPump.Run(
                 async () =>
                 {
-                    await ChildrenRetrievalPolicyTestCoreAsync(ChildrenRetrievalPolicy.None);
-                    await ChildrenRetrievalPolicyTestCoreAsync(ChildrenRetrievalPolicy.DirectOnly);
-                    await ChildrenRetrievalPolicyTestCoreAsync(ChildrenRetrievalPolicy.All);
+                    await ChildrenRetrievalPolicyTestCoreAsync(ChildrenRetrievalPolicy.None, "ChildrenRetrievalPolicyLog1.xml");
+                    await ChildrenRetrievalPolicyTestCoreAsync(ChildrenRetrievalPolicy.DirectOnly, "ChildrenRetrievalPolicyLog2.xml");
+                    await ChildrenRetrievalPolicyTestCoreAsync(ChildrenRetrievalPolicy.All, "ChildrenRetrievalPolicyLog3.xml");
                 });
         }
 
@@ -1584,7 +1588,7 @@ private static void AssertAccess(RecursiveFieldNode node)
             }
         }
 
-        private static Task ChildrenRetrievalPolicyTestCoreAsync(ChildrenRetrievalPolicy policy)
+        private static Task ChildrenRetrievalPolicyTestCoreAsync(ChildrenRetrievalPolicy policy, string logName)
         {
             return TestWithRobot<ModelPayloads>(
                 async client =>
@@ -1611,16 +1615,23 @@ private static Task ChildrenRetrievalPolicyTestCoreAsync(ChildrenRetrievalPolicy
                             AssertThrow<ArgumentException>(() => root.ChildrenRetrievalPolicy -= 1, expectedMessage);
                         }
 
-                        var childPolicy = policy == ChildrenRetrievalPolicy.All ?
-                            ChildrenRetrievalPolicy.All : ChildrenRetrievalPolicy.None;
-                        Assert.AreEqual(childPolicy, root.Node.ChildrenRetrievalPolicy);
+                        if (policy == ChildrenRetrievalPolicy.None)
+                        {
+                            Assert.IsNull(root.Node);
+                        }
+                        else
+                        {
+                            var childPolicy = policy == ChildrenRetrievalPolicy.All ?
+                                ChildrenRetrievalPolicy.All : ChildrenRetrievalPolicy.None;
+                            Assert.AreEqual(childPolicy, root.Node.ChildrenRetrievalPolicy);
+                        }
                     }
                 },
                 null,
                 null,
                 GlowTypes.Instance,
                 false,
-                "ChildrenRetrievalPolicyLog.xml");
+                logName);
         }
 
         [SuppressMessage("Microsoft.Reliability", "CA2000:Dispose objects before losing scope", Justification = "Objects are disposed within the called method.")]

diff --git a/Lawo.EmberPlusSharpTest/Model/Test/EmberDataPayloads/ChildrenRetrievalPolicyLog1.xml b/Lawo.EmberPlusSharpTest/Model/Test/EmberDataPayloads/ChildrenRetrievalPolicyLog1.xml
@@ -0,0 +1,6 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- Copyright 2012-2016 Lawo AG (http://www.lawo.com). -->
+<!-- Distributed under the Boost Software License, Version 1.0. -->
+<!-- (See accompanying file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) -->
+<S101Log>
+</S101Log>
diff --git a/Lawo.EmberPlusSharpTest/Model/Test/EmberDataPayloads/ChildrenRetrievalPolicyLog2.xml b/Lawo.EmberPlusSharpTest/Model/Test/EmberDataPayloads/ChildrenRetrievalPolicyLog2.xml
@@ -0,0 +1,31 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- Copyright 2012-2016 Lawo AG (http://www.lawo.com). -->
+<!-- Distributed under the Boost Software License, Version 1.0. -->
+<!-- (See accompanying file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) -->
+<S101Log>
+  <Event type="Message" timeUtc="12:54:14.19" direction="Send" number="1">
+    <Slot>00</Slot>
+    <Command>EmberData 01 0A 02</Command>
+    <Payload>
+      <Root type="RootElementCollection">
+        <RootElement type="Command">
+          <number type="Integer">32</number>
+        </RootElement>
+      </Root>
+    </Payload>
+  </Event>
+  <Event type="Message" timeUtc="12:54:14.34" direction="Receive" number="1">
+    <Slot>00</Slot>
+    <Command>EmberData 01 0A 02</Command>
+    <Payload>
+      <Root type="RootElementCollection">
+        <RootElement type="Node">
+          <number type="Integer">1</number>
+          <contents type="Set">
+            <identifier type="UTF8String">Node</identifier>
+          </contents>
+        </RootElement>
+      </Root>
+    </Payload>
+  </Event>
+</S101Log>
diff --git a/...taPayloads/ChildrenRetrievalPolicyLog.xml → ...aPayloads/ChildrenRetrievalPolicyLog3.xml b/...taPayloads/ChildrenRetrievalPolicyLog.xml → ...aPayloads/ChildrenRetrievalPolicyLog3.xml