From a0227f3fca58c07b79394de4f39082779c4e1df1 Mon Sep 17 00:00:00 2001
From: Ad Schellevis <ad@opnsense.org>
Date: Sun, 28 Jun 2020 09:15:36 +0200
Subject: [PATCH] MVC: minor cleanups and prevent virtual nodes to be persisted
 to disk.

o BaseField: add hasChild() and getChild() so callers don't have to use getChildren() to check if a single item exists.
o BaseField: prevent addToXMLNode() from saving virtual nodes
o BaseModel: refactor getNodeByReference() to use hasChild() and getChild()
---
 .../app/models/OPNsense/Base/BaseModel.php    |  4 +--
 .../OPNsense/Base/FieldTypes/BaseField.php    | 32 +++++++++++++++----
 2 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/src/opnsense/mvc/app/models/OPNsense/Base/BaseModel.php b/src/opnsense/mvc/app/models/OPNsense/Base/BaseModel.php
index f87496946..025c86a7a 100644
--- a/src/opnsense/mvc/app/models/OPNsense/Base/BaseModel.php
+++ b/src/opnsense/mvc/app/models/OPNsense/Base/BaseModel.php
@@ -582,8 +582,8 @@ abstract class BaseModel
         $node = $this->internalData;
         while (count($parts) > 0) {
             $childName = array_shift($parts);
-            if (isset($node->getChildren()[$childName])) {
-                $node = $node->getChildren()[$childName];
+            if ($node->hasChild($childName)) {
+                $node = $node->getChild($childName);
             } else {
                 return null;
             }
diff --git a/src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/BaseField.php b/src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/BaseField.php
index 7d11f0c6d..defc628ae 100644
--- a/src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/BaseField.php
+++ b/src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/BaseField.php
@@ -366,11 +366,7 @@ abstract class BaseField
      */
     public function isFieldChanged()
     {
-        if ($this->internalInitialValue !==  $this->internalValue) {
-            return true;
-        } else {
-            return false;
-        }
+        return $this->internalInitialValue !==  $this->internalValue;
     }
 
     /**
@@ -415,6 +411,26 @@ abstract class BaseField
         return $this->internalChildnodes;
     }
 
+    /**
+     * check for existance of child attribute
+     * @return bool if child exists
+     */
+    public function hasChild($name)
+    {
+        return isset($this->internalChildnodes[$name]);
+    }
+
+    /**
+     * retrieve child object
+     * @return null|object
+     */
+    public function getChild($name)
+    {
+        if ($this->hasChild($name)) {
+            return $this->internalChildnodes[$name];
+        }
+    }
+
     /**
      * check if this field is unused and required
      * @return bool
@@ -506,7 +522,7 @@ abstract class BaseField
     }
 
     /**
-     * returns if this node is virtual, the framework uses this to determine if this node maybe should only be used to
+     * returns if this node is virtual, the framework uses this to determine if this node should only be used to
      * clone children. (using ArrayFields)
      * @return bool is virtual node
      */
@@ -632,6 +648,10 @@ abstract class BaseField
         }
 
         foreach ($this->iterateItems() as $key => $FieldNode) {
+            if ($FieldNode->getInternalIsVirtual()) {
+                // Virtual fields should never be persisted
+                continue;
+            }
             $FieldNode->addToXMLNode($subnode);
         }
     }