(mvc) minor performance and style fixes

src/opnsense/mvc/app/models/OPNsense/Base/BaseModel.php

@@ -69,6 +69,12 @@ abstract class BaseModel
      */
     private $internal_current_model_version = null;
 
+    /**
+     * cache classes
+     * @var null
+     */
+    private static $internalCacheReflectionClasses = null;
+
     /**
      * If the model needs a custom initializer, override this init() method
      * Default behaviour is to do nothing in this init.
@@ -86,7 +92,7 @@ abstract class BaseModel
     private function parseOptionData($xmlNode)
     {
         if ($xmlNode->count() == 0) {
-            $result = $xmlNode->__toString();
+            $result = (string)$xmlNode;
         } else {
             $result = array();
             foreach ($xmlNode->children() as $childNode) {
@@ -96,6 +102,37 @@ abstract class BaseModel
         return $result;
     }
 
+    /**
+     * fetch reflection class (cached by field type)
+     * @param $classname classname to construct
+     */
+    private function getNewField($classname)
+    {
+        if (self::$internalCacheReflectionClasses === null) {
+            self::$internalCacheReflectionClasses = array();
+        }
+        if (!isset(self::$internalCacheReflectionClasses[$classname])) {
+            $is_derived_from_basefield = false;
+            if (class_exists($classname)) {
+                $field_rfcls = new \ReflectionClass($classname);
+                $check_derived = $field_rfcls->getParentClass();
+                while ($check_derived != false) {
+                    if ($check_derived->name == 'OPNsense\Base\FieldTypes\BaseField') {
+                        $is_derived_from_basefield = true;
+                        break;
+                    }
+                    $check_derived = $check_derived->getParentClass();
+                }
+            }
+            if (!$is_derived_from_basefield) {
+                // class found, but of wrong type. raise an exception.
+                throw new ModelException("class ".$field_rfcls->name." of wrong type in model definition");
+            }
+            self::$internalCacheReflectionClasses[$classname] = $field_rfcls ;
+        }
+        return self::$internalCacheReflectionClasses[$classname];
+    }
+
     /**
      * parse model and config xml to object model using types in FieldTypes
      * @param SimpleXMLElement $xml model xml data (from items section)
@@ -103,7 +140,7 @@ abstract class BaseModel
      * @param BaseField $internal_data output structure using FieldTypes,rootnode is internalData
      * @throws ModelException parse error
      */
-    private function parseXml($xml, &$config_data, &$internal_data)
+    private function parseXml(&$xml, &$config_data, &$internal_data)
     {
         // copy xml tag attributes to Field
         if ($config_data != null) {
@@ -117,27 +154,13 @@ abstract class BaseModel
             $tagName = $xmlNode->getName();
             // every item results in a Field type object, the first step is to determine which object to create
             // based on the input model spec
-            $fieldObject = null;
-            $classname = "OPNsense\\Base\\FieldTypes\\".$xmlNode->attributes()["type"];
-            if (class_exists($classname)) {
+            $xmlNodeType = $xmlNode->attributes()["type"];
+            if (!empty($xmlNodeType)) {
                 // construct field type object
-                $field_rfcls = new \ReflectionClass($classname);
-                $check_derived = $field_rfcls->getParentClass();
-                $is_derived_from_basefield = false;
-                while ($check_derived != false) {
-                    if ($check_derived->name == 'OPNsense\Base\FieldTypes\BaseField') {
-                        $is_derived_from_basefield = true;
-                        break;
-                    }
-                    $check_derived = $check_derived->getParentClass();
-                }
-                if (!$is_derived_from_basefield) {
-                    // class found, but of wrong type. raise an exception.
-                    throw new ModelException("class ".$field_rfcls->name." of wrong type in model definition");
-                }
+                $field_rfcls = $this->getNewField("OPNsense\\Base\\FieldTypes\\".$xmlNodeType);
             } else {
                 // no type defined, so this must be a standard container (without content)
-                $field_rfcls = new \ReflectionClass('OPNsense\Base\FieldTypes\ContainerField');
+                $field_rfcls = $this->getNewField('OPNsense\Base\FieldTypes\ContainerField');
             }
 
             // generate full object name ( section.section.field syntax ) and create new Field
@@ -183,7 +206,6 @@ abstract class BaseModel
                                 $tagUUID = $internal_data->generateUUID();
                             }
 
-
                             // iterate array items from config data
                             $child_node = new ContainerField($fieldObject->__reference . "." . $tagUUID, $tagName);
                             $this->parseXml($xmlNode, $conf_section, $child_node);
@@ -494,7 +516,7 @@ abstract class BaseModel
         $node = $this->internalData;
         while (count($parts)>0) {
             $childName = array_shift($parts);
-            if (array_key_exists($childName, $node->getChildren())) {
+            if (isset($node->getChildren()[$childName])) {
                 $node = $node->getChildren()[$childName];
             } else {
                 return null;

src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/ArrayField.php

@@ -114,7 +114,7 @@ class ArrayField extends BaseField
      */
     public function del($index)
     {
-        if (array_key_exists((string)$index, $this->internalChildnodes)) {
+        if (isset($this->internalChildnodes[(string)$index])) {
             unset($this->internalChildnodes[$index]);
             return true;
         } else {
@@ -145,7 +145,7 @@ class ArrayField extends BaseField
             // populate sort key
             $sortKey = '';
             foreach ($fieldNames as $fieldName) {
-                if (array_key_exists($fieldName, $node->internalChildnodes)) {
+                if (isset($node->internalChildnodes[$fieldName])) {
                     if (is_numeric((string)$node->$fieldName)) {
                         // align numeric values right for sorting, not perfect but works for integer type values
                         $sortKey .=  sprintf("%".$MAX_KEY_LENGTH."s,", $node->$fieldName);

src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/AuthenticationServerField.php

@@ -76,7 +76,7 @@ class AuthenticationServerField extends BaseField
      */
     public function eventPostLoading()
     {
-        if (!array_key_exists($this->internalCacheKey, self::$internalOptionList)) {
+        if (!isset(self::$internalOptionList[$this->internalCacheKey])) {
             self::$internalOptionList[$this->internalCacheKey] = array();
 
             $authFactory = new \OPNsense\Auth\AuthenticationFactory;

src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/BaseField.php

@@ -233,7 +233,7 @@ abstract class BaseField
      */
     public function __get($name)
     {
-        if (array_key_exists($name, $this->internalChildnodes)) {
+        if (isset($this->internalChildnodes[$name])) {
             return $this->internalChildnodes[$name];
         } elseif ($name == '__items') {
             // return all (no virtual/hidden) items
@@ -265,7 +265,7 @@ abstract class BaseField
      */
     public function __set($name, $value)
     {
-        if (array_key_exists($name, $this->internalChildnodes)) {
+        if (isset($this->internalChildnodes[$name])) {
             $this->internalChildnodes[$name]->setValue($value);
         } else {
             throw new \InvalidArgumentException($name." not an attribute of ". $this->internalReference);
@@ -515,7 +515,7 @@ abstract class BaseField
     {
         // update structure with new content
         foreach ($this->__items as $key => $node) {
-            if ($data != null && array_key_exists($key, $data)) {
+            if ($data != null && isset($data[$key])) {
                 if ($node->isContainer()) {
                     if (is_array($data[$key])) {
                         $node->setNodes($data[$key]);
@@ -531,7 +531,7 @@ abstract class BaseField
         // add new items to array type objects
         if (get_class($this) == "OPNsense\\Base\\FieldTypes\\ArrayField") {
             foreach ($data as $dataKey => $dataValue) {
-                if (!array_key_exists($dataKey, $this->__items)) {
+                if (!isset($this->__items[$dataKey])) {
                     $node = $this->add();
                     $node->setNodes($dataValue);
                 }

src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/CSVListField.php

@@ -88,7 +88,7 @@ class CSVListField extends BaseField
 
         foreach ($selectlist as $optKey) {
             if (strlen($optKey) > 0) {
-                if (array_key_exists($optKey, $result)) {
+                if (isset($result[$optKey])) {
                     $result[$optKey]["selected"] = 1;
                 } else {
                     $result[$optKey] = array("value"=>$optKey, "selected" => 1);

src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/CertificateField.php

@@ -98,7 +98,7 @@ class CertificateField extends BaseField
      */
     public function eventPostLoading()
     {
-        if (!array_key_exists($this->certificateType, self::$internalOptionList)) {
+        if (!isset(self::$internalOptionList[$this->certificateType])) {
              self::$internalOptionList[$this->certificateType] = array();
              $configObj = Config::getInstance()->object();
             foreach ($configObj->{$this->certificateType} as $cert) {

src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/ConfigdActionsField.php

@@ -68,7 +68,7 @@ class ConfigdActionsField extends BaseField
      */
     public function eventPostLoading()
     {
-        if (!array_key_exists($this->internalCacheKey, self::$internalOptionList)) {
+        if (!isset(self::$internalOptionList[$this->internalCacheKey])) {
             self::$internalOptionList[$this->internalCacheKey] = array();
 
             $backend = new Backend();
@@ -95,7 +95,7 @@ class ConfigdActionsField extends BaseField
                 // use filters to determine relevance
                 $isMatched = true;
                 foreach ($this->internalFilters as $filterKey => $filterData) {
-                    if (array_key_exists($filterKey, $value)) {
+                    if (isset($value[$filterKey])) {
                         $fieldData = $value[$filterKey];
                         if (!preg_match($filterData, $fieldData)) {
                             $isMatched = false;

src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/InterfaceField.php

@@ -122,7 +122,7 @@ class InterfaceField extends BaseField
      */
     public function eventPostLoading()
     {
-        if (!array_key_exists($this->internalCacheKey, self::$internalOptionList)) {
+        if (!isset(self::$internalOptionList[$this->internalCacheKey])) {
             self::$internalOptionList[$this->internalCacheKey] = array();
 
             $allInterfaces = array();

src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/ModelRelationField.php

@@ -79,24 +79,24 @@ class ModelRelationField extends BaseField
 
         // only collect options once per source/filter combination, we use a static to save our unique option
         // combinations over the running application.
-        if (!array_key_exists($this->internalCacheKey, self::$internalOptionList)) {
+        if (!isset(self::$internalOptionList[$this->internalCacheKey])) {
             self::$internalOptionList[$this->internalCacheKey] = array();
             foreach ($mdlStructure as $modelData) {
                 // only handle valid model sources
-                if (array_key_exists('source', $modelData) && array_key_exists('items', $modelData) &&
-                    array_key_exists('display', $modelData)) {
+                if (isset($modelData['source']) && isset($modelData['items']) && isset($modelData['display'])) {
                     $className = str_replace(".", "\\", $modelData['source']);
                     $modelObj = new $className;
                     foreach ($modelObj->getNodeByReference($modelData['items'])->__items as $node) {
                         $displayKey = $modelData['display'];
-                        if (array_key_exists("uuid", $node->getAttributes()) && $node->$displayKey != null) {
+                        if (isset($node->getAttributes()["uuid"]) && $node->$displayKey != null) {
                             // check for filters and apply if found
                             $isMatched = true;
-                            if (array_key_exists("filters", $modelData)) {
+                            if (isset($modelData['filters'])) {
                                 foreach ($modelData['filters'] as $filterKey => $filterValue) {
                                     $fieldData = $node->$filterKey;
                                     if (!preg_match($filterValue, $fieldData) && $fieldData != null) {
                                         $isMatched = false;
+                                        break;
                                     }
                                 }
                             }
@@ -133,7 +133,7 @@ class ModelRelationField extends BaseField
     public function getNodeData()
     {
         $result = array ();
-        if (array_key_exists($this->internalCacheKey, self::$internalOptionList) &&
+        if (isset(self::$internalOptionList[$this->internalCacheKey]) &&
             is_array(self::$internalOptionList[$this->internalCacheKey])) {
             // if relation is not required, add empty option
             if (!$this->internalIsRequired) {