From 21ca2f32b5682901a8ac44bb90e3853b9292fa9d Mon Sep 17 00:00:00 2001 From: Ad Schellevis Date: Wed, 7 Dec 2016 07:07:40 +0100 Subject: [PATCH] (mvc) minor performance and style fixes --- .../app/models/OPNsense/Base/BaseModel.php | 66 ++++++++++++------- .../OPNsense/Base/FieldTypes/ArrayField.php | 4 +- .../FieldTypes/AuthenticationServerField.php | 2 +- .../OPNsense/Base/FieldTypes/BaseField.php | 8 +-- .../OPNsense/Base/FieldTypes/CSVListField.php | 2 +- .../Base/FieldTypes/CertificateField.php | 2 +- .../Base/FieldTypes/ConfigdActionsField.php | 4 +- .../Base/FieldTypes/InterfaceField.php | 2 +- .../Base/FieldTypes/ModelRelationField.php | 12 ++-- 9 files changed, 62 insertions(+), 40 deletions(-) diff --git a/src/opnsense/mvc/app/models/OPNsense/Base/BaseModel.php b/src/opnsense/mvc/app/models/OPNsense/Base/BaseModel.php index 98efb9e1c..f10af1909 100644 --- a/src/opnsense/mvc/app/models/OPNsense/Base/BaseModel.php +++ b/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; diff --git a/src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/ArrayField.php b/src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/ArrayField.php index 919f4d6a8..7ca864137 100644 --- a/src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/ArrayField.php +++ b/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); diff --git a/src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/AuthenticationServerField.php b/src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/AuthenticationServerField.php index 65fbfe203..4d4d12086 100644 --- a/src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/AuthenticationServerField.php +++ b/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; 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 59c77bd7e..1825d941e 100644 --- a/src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/BaseField.php +++ b/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); } diff --git a/src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/CSVListField.php b/src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/CSVListField.php index ff5aad6b4..d2053f01e 100644 --- a/src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/CSVListField.php +++ b/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); diff --git a/src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/CertificateField.php b/src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/CertificateField.php index b25e210c8..12fa00552 100644 --- a/src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/CertificateField.php +++ b/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) { diff --git a/src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/ConfigdActionsField.php b/src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/ConfigdActionsField.php index f39a53b01..a94b9586c 100644 --- a/src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/ConfigdActionsField.php +++ b/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; diff --git a/src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/InterfaceField.php b/src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/InterfaceField.php index b68212030..f70a37ec3 100644 --- a/src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/InterfaceField.php +++ b/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(); diff --git a/src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/ModelRelationField.php b/src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/ModelRelationField.php index b1be57a6e..ffaf7ff3a 100644 --- a/src/opnsense/mvc/app/models/OPNsense/Base/FieldTypes/ModelRelationField.php +++ b/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) {