From a89866e81eee19defa87581c9d886bc77f798133 Mon Sep 17 00:00:00 2001 From: steve-lad <72376554+steve-lad@users.noreply.github.com> Date: Wed, 24 Feb 2021 15:22:04 +0100 Subject: [PATCH] Bug Fixes and optimisations as a result of issue 106 Fix bugs in Response Class causing exceptions Update extconfigs class to correctly check database connectors and improve feedback Move some constructors to start of class for lisibility Tidy up Response class with Typing to eliminate unnecessary tests --- Sccp_manager.class.php | 12 +- .../aminterface/Message.class.php | 16 +- .../aminterface/Response.class.php | 185 ++++++++---------- .../aminterface/aminterface.class.php | 4 +- Sccp_manager.inc/extconfigs.class.php | 73 +++---- views/server.info.php | 2 +- 6 files changed, 135 insertions(+), 157 deletions(-) diff --git a/Sccp_manager.class.php b/Sccp_manager.class.php index dba8c84..7af48b9 100644 --- a/Sccp_manager.class.php +++ b/Sccp_manager.class.php @@ -269,25 +269,25 @@ class Sccp_manager extends \FreePBX_Helpers implements \BMO { /* unused but FPBX API requires it */ public function install() { - + } /* unused but FPBX API requires it */ public function uninstall() { - + } /* unused but FPBX API requires it */ public function backup() { - + } /* unused but FPBX API requires it */ public function restore($backup) { - + } public function getActionBar($request) { @@ -815,7 +815,7 @@ class Sccp_manager extends \FreePBX_Helpers implements \BMO { $hw_list[] = array('name' => $idv); } if ($idv == 'all') { - + } } } @@ -2093,7 +2093,7 @@ class Sccp_manager extends \FreePBX_Helpers implements \BMO { } // [Namesoftkeyset] // type=softkeyset - // + // // ----- It is a very bad idea to add an external configuration file "sccp_custom.conf" !!!! // This will add problems when solving problems caused by unexpected solutions from users. // diff --git a/Sccp_manager.inc/aminterface/Message.class.php b/Sccp_manager.inc/aminterface/Message.class.php index aedabc4..b109512 100644 --- a/Sccp_manager.inc/aminterface/Message.class.php +++ b/Sccp_manager.inc/aminterface/Message.class.php @@ -27,6 +27,14 @@ abstract class Message protected $createdDate; private $_responseHandler; + public function __construct() + { + $this->lines = array(); + $this->variables = array(); + $this->keys = array(); + $this->createdDate = time(); + } + public function _ToDebug($level, $msg) { } @@ -217,14 +225,6 @@ abstract class Message { return array('lines', 'variables', 'keys', 'createdDate'); } - - public function __construct() - { - $this->lines = array(); - $this->variables = array(); - $this->keys = array(); - $this->createdDate = time(); - } } abstract class IncomingMessage extends Message diff --git a/Sccp_manager.inc/aminterface/Response.class.php b/Sccp_manager.inc/aminterface/Response.class.php index c7415fd..d898efa 100644 --- a/Sccp_manager.inc/aminterface/Response.class.php +++ b/Sccp_manager.inc/aminterface/Response.class.php @@ -21,6 +21,14 @@ abstract class Response extends IncomingMessage protected $_completed; protected $keys; + public function __construct($rawContent) + { + parent::__construct($rawContent); + $this->_events = array(); + $this->_eventsCount = 0; + $this->_completed = !$this->isList(); + } + public function isComplete() { return $this->_completed; @@ -51,6 +59,7 @@ abstract class Response extends IncomingMessage public function isSuccess() { + // returns true if response message does not contain error return stristr($this->getKey('Response'), 'Error') === false; } @@ -84,14 +93,6 @@ abstract class Response extends IncomingMessage } } } - - public function __construct($rawContent) - { - parent::__construct($rawContent); - $this->_events = array(); - $this->_eventsCount = 0; - $this->_completed = !$this->isList(); - } } //**************************************************************************** class Generic_Response extends Response @@ -174,6 +175,13 @@ class SCCPGeneric_Response extends Response protected $_tables; private $_temptable; + public function __construct($rawContent) + { + parent::__construct($rawContent); + $_fields = array("EventList" => "EventList:", "Message" => "Message:"); + $this->_completed = !$this->isList(); + } + public function addEvent($event) { // not eventlist (start/complete) @@ -185,7 +193,6 @@ class SCCPGeneric_Response extends Response // Nothing to do with this - we need a table start if (stristr($event->getEventList(), 'start')) { return; } - // This is empty as soon as we have received a TableStart. // The next message is the first of the data sets // We use this variable in the switch to add set entries @@ -239,27 +246,21 @@ class SCCPGeneric_Response extends Response } } - protected function ConvertTableData($_tablename, $_fkey, $_fields) + protected function ConvertTableData(String $_tablename, Array $_fkey, Array $_fields) { - $_rawtable = $this->Table2Array($_tablename); $result = array(); + $_rawtable = $this->Table2Array($_tablename); // Check that there is actually data to be converted if (empty($_rawtable)) { return $result;} + foreach ($_rawtable as $_row) { $all_key_ok = true; - if (is_array($_fkey)) { - foreach ($_fkey as $_fid) { - if (empty($_row[$_fid])) { - $all_key_ok = false; - } else { - $set_name[$_fid] = $_row[$_fid]; - } - } - } else { - if (empty($_row[$_fkey])) { + // No need to test if $_fkey is arrray as array required + foreach ($_fkey as $_fid) { + if (empty($_row[$_fid])) { $all_key_ok = false; } else { - $set_name[$_fkey] = $_row[$_fkey]; + $set_name[$_fid] = $_row[$_fid]; } } $Data = &$result; @@ -267,6 +268,7 @@ class SCCPGeneric_Response extends Response foreach ($set_name as $value_id) { $Data = &$Data[$value_id]; } + // Label converter in case labels and keys are different - not actually required. foreach ($_fields as $value_key => $value_id) { $Data[$value_id] = $_row[$value_key]; } @@ -275,7 +277,7 @@ class SCCPGeneric_Response extends Response return $result; } - protected function ConvertEventData($_fkey, $_fields) + protected function ConvertEventData(Array $_fkey, Array $_fields) { $result = array(); @@ -283,19 +285,12 @@ class SCCPGeneric_Response extends Response $all_key_ok = true; $tmp_result = $_row->getKeys(); $set_name = array(); - if (is_array($_fkey)) { - foreach ($_fkey as $_fid) { - if (empty($tmp_result[$_fid])) { - $all_key_ok = false; - } else { - $set_name[$_fid] = $tmp_result[$_fid]; - } - } - } else { - if (empty($tmp_result[$_fkey])) { + // No need to test if $_fkey is arrray as array required + foreach ($_fkey as $_fid) { + if (empty($tmp_result[$_fid])) { $all_key_ok = false; } else { - $set_name[$_fkey] = $tmp_result[$_fkey]; + $set_name[$_fid] = $tmp_result[$_fid]; } } $Data = &$result; @@ -303,6 +298,7 @@ class SCCPGeneric_Response extends Response foreach ($set_name as $value_id) { $Data = &$Data[$value_id]; } + // Label converter in case labels and keys are different - not actually required. foreach ($_fields as $value_id) { $Data[$value_id] = $tmp_result[$value_id]; } @@ -311,59 +307,53 @@ class SCCPGeneric_Response extends Response return $result; } - - public function hasTable() +/* public function hasTable() { if (is_array($this->_tables)) { return true; } return false; } + public function getTableNames() { return (is_array($this->_tables)) ? array_keys($this->_tables) : null; } - - public function Table2Array($tablename = '') +*/ + public function Table2Array( String $tablename ) { $result =array(); - if (!is_string($tablename) || empty($tablename)) { - return false; - } - if ($this->hasTable()) { - foreach ($this->_tables[$tablename]['Entries'] as $trow) { - $result[]= $trow->getKeys(); - } + if (empty($tablename) || !is_array($this->_tables)) { return $result; - } else { - return false; } + foreach ($this->_tables[$tablename]['Entries'] as $trow) { + $result[]= $trow->getKeys(); + } + return $result; } - public function Events2Array() + +/* public function Events2Array() { $result =array(); - if (is_array($this->_events)) { - foreach ($this->_events as $trow) { - $tmp_result = $trow->getKeys(); - if (is_array($tmp_result)) { - $result = array_merge($result, $tmp_result); - } else { - $result [] = $tmp_result; - } - } - return $result; - } else { - return false; + foreach ($this->_events as $trow) { + // $tmp_result = $trow->getKeys(); + // if (is_array($tmp_result)) { + $result = array_merge($result, $trow->getKeys()); + //} else { + // $result [] = $tmp_result; + // } } + return $result; } public function getTable($tablename) { - if ($this->hasTable() && array_key_exists($tablename, $this->_tables)) { + if (is_array($this->_tables) && array_key_exists($tablename, $this->_tables)) { return $this->_tables[$tablename]; } throw new PAMIException("No such table."); } + public function getJSON() { if (strlen($this->getKey('JSON')) > 0) { @@ -373,23 +363,16 @@ class SCCPGeneric_Response extends Response } throw new AMIException("No JSON Key found to return."); } - - public function __construct($rawContent) - { - parent::__construct($rawContent); - $_fields = array("EventList" => "EventList:", "Message" => "Message:"); -// $this->getVariable($rawContent, $_fields); - $this->_completed = !$this->isList(); - } - +*/ public function getResult() { - if ($this->getKey('JSON') != null) { - $result = $this->getJSON(); + if ($this->getKey('JSON') !== null && !empty($this->getKey('JSON'))) { + if (($json = json_decode($this->getKey('JSON'), true)) != false) { + return $json; + } } else { - $result = $this->getMessage(); + return $this->getMessage(); } - return $result; } } @@ -399,10 +382,8 @@ class SCCPJSON_Response extends Response public function __construct($rawContent) { parent::__construct($rawContent); - $_fields = array("DataType" => "DataType:", "JSONRAW" => "JSON:"); - $this->getVariable($rawContent, $_fields); - $js_res = $this->getKey('JSONRAW'); - if (isset($js_res)) { + $this->getVariable($rawContent, array("DataType" => "DataType:", "JSONRAW" => "JSON:")); + if (null !== $this->getKey('JSONRAW')) { $this->setKey('Response', 'Success'); } return $this->isSuccess(); @@ -417,9 +398,13 @@ class SCCPShowSoftkeySets_Response extends SCCPGeneric_Response } public function getResult() { - $_fields = array('description'=>'description','label'=>'label','lblid'=>'lblid'); - $result = $this->ConvertTableData('SoftKeySets', array('set','mode'), $_fields); - return $result; + // $_fields = array('description'=>'description','label'=>'label','lblid'=>'lblid'); + return $this->ConvertTableData( + 'SoftKeySets', + array('set','mode'), + array('description'=>'description','label'=>'label','lblid'=>'lblid') + ); + // return $result; } } @@ -431,10 +416,15 @@ class SCCPShowDevices_Response extends SCCPGeneric_Response } public function getResult() { - $_fields = array('mac'=>'mac','address'=>'address','descr'=>'descr','regstate'=>'status', - 'token'=>'token','act'=>'act', 'lines'=>'lines','nat'=>'nat','regtime'=>'regtime'); - $result = $this->ConvertTableData('Devices', array('mac'), $_fields); - return $result; +// $_fields = array('mac'=>'mac','address'=>'address','descr'=>'descr','regstate'=>'status', +// 'token'=>'token','act'=>'act', 'lines'=>'lines','nat'=>'nat','regtime'=>'regtime'); + return $this->ConvertTableData( + 'Devices', + array('mac'), + array('mac'=>'mac','address'=>'address','descr'=>'descr','regstate'=>'status', + 'token'=>'token','act'=>'act', 'lines'=>'lines','nat'=>'nat','regtime'=>'regtime') + ); +// return $result; } } @@ -447,16 +437,15 @@ class SCCPShowDevice_Response extends SCCPGeneric_Response public function getResult() { $result = array(); - $result = $this->Events2Array(); + foreach ($this->_events as $trow) { + $result = array_merge($result, $trow->getKeys()); + } $result['Buttons'] = $this->ConvertTableData( 'Buttons', array('id'), array('id'=>'id','channelobjecttype'=>'channelobjecttype','inst'=>'inst', - 'typestr'=>'typestr', - 'type'=>'type', - 'pendupdt'=>'pendupdt', - 'penddel'=>'penddel', - 'default'=>'default') + 'typestr'=>'typestr', 'type'=>'type', 'pendupdt'=>'pendupdt', 'penddel'=>'penddel', 'default'=>'default' + ) ); $result['SpeeddialButtons'] = $this->ConvertTableData( 'Buttons', @@ -467,18 +456,18 @@ class SCCPShowDevice_Response extends SCCPGeneric_Response 'CallStatistics', array('type'), array('type'=>'type','channelobjecttype'=>'channelobjecttype','calls'=>'calls','pcktsnt'=>'pcktsnt','pcktrcvd'=>'pcktrcvd', - 'lost'=>'lost','jitter'=>'jitter','latency'=>'latency', 'quality'=>'quality','avgqual'=>'avgqual','meanqual'=>'meanqual', - 'maxqual'=>'maxqual', - 'rconceal'=>'rconceal', - 'sconceal'=>'sconceal') + 'lost'=>'lost','jitter'=>'jitter','latency'=>'latency', 'quality'=>'quality','avgqual'=>'avgqual','meanqual'=>'meanqual', + 'maxqual'=>'maxqual', 'rconceal'=>'rconceal', 'sconceal'=>'sconceal' + ) ); $result['SCCP_Vendor'] = array('vendor' => strtok($result['skinnyphonetype'], ' '), 'model' => strtok('('), 'model_id' => strtok(')'), 'vendor_addon' => strtok($result['configphonetype'], ' '), 'model_addon' => strtok(' ')); if (empty($result['SCCP_Vendor']['vendor']) || $result['SCCP_Vendor']['vendor'] == 'Undefined') { - $result['SCCP_Vendor'] = array('vendor' => 'Undefined', 'model' => $result['configphonetype'], - 'model_id' => '', 'vendor_addon' => $result['SCCP_Vendor']['vendor_addon'], - 'model_addon' => $result['SCCP_Vendor']['model_addon']); + $result['SCCP_Vendor'] = array('vendor' => 'Undefined', 'model' => $result['configphonetype'], + 'model_id' => '', 'vendor_addon' => $result['SCCP_Vendor']['vendor_addon'], + 'model_addon' => $result['SCCP_Vendor']['model_addon'] + ); } $result['MAC_Address'] =$result['macaddress']; return $result; diff --git a/Sccp_manager.inc/aminterface/aminterface.class.php b/Sccp_manager.inc/aminterface/aminterface.class.php index 4ad5f19..e9fdaf0 100644 --- a/Sccp_manager.inc/aminterface/aminterface.class.php +++ b/Sccp_manager.inc/aminterface/aminterface.class.php @@ -521,9 +521,7 @@ class aminterface $result = array(); if ($this->_connect_state) { $_action = new \FreePBX\modules\Sccp_manager\aminterface\ReloadAction('chan_sccp'); -// $_action = new \FreePBX\modules\Sccp_manager\aminterface\CommandAction('sccp reload force'); // No Response Result !! - $_response = $this->send($_action); - $result = $_response->getMessage(); + $result = ['Response' => $this->send($_action)->getMessage(), 'data' => '']; } return $result; } diff --git a/Sccp_manager.inc/extconfigs.class.php b/Sccp_manager.inc/extconfigs.class.php index b108389..8a5461c 100644 --- a/Sccp_manager.inc/extconfigs.class.php +++ b/Sccp_manager.inc/extconfigs.class.php @@ -414,27 +414,29 @@ class extconfigs return $base_config; } - public function validate_RealTime($realm = '') + public function validate_RealTime( String $connector ) { + // This method only checks that asterisk is correctly configured for Realtime + // It is preventative and does not change anything for Sccp_manager global $amp_conf; $res = array(); -/* if (empty($realm)) { - $realm = 'sccp'; +/* if (empty($connector)) { + $connector = 'sccp'; } -*/ $cnf_int = \FreePBX::Config(); + $cnf_int = \FreePBX::Config(); $cnf_wr = \FreePBX::WriteConfig(); +*/ $cnf_read = \FreePBX::LoadConfig(); - $def_config = array('sccpdevice' => 'mysql,' . $realm . ',sccpdeviceconfig', 'sccpline' => 'mysql,' . $realm . ',sccpline'); + // We are running inside FreePBX so must use the same database + $def_config = array('sccpdevice' => 'mysql,' . $amp_conf['AMPDBNAME'] . ',sccpdeviceconfig', 'sccpline' => 'mysql,' . $amp_conf['AMPDBNAME'] . ',sccpline'); $backup_ext = array('_custom.conf', '.conf', '_additional.conf'); $def_bd_config = array('dbhost' => $amp_conf['AMPDBHOST'], 'dbname' => $amp_conf['AMPDBNAME'], - 'dbuser' => $amp_conf['AMPDBUSER'], 'dbpass' => $amp_conf['AMPDBPASS'], - 'dbport' => '3306', 'dbsock' => '/var/lib/mysql/mysql.sock'); - $def_bd_sec = 'sccp'; - - $dir = $cnf_int->get('ASTETCDIR'); + 'dbuser' => $amp_conf['AMPDBUSER'], 'dbpass' => $amp_conf['AMPDBPASS'], + 'dbport' => '3306', 'dbsock' => '/var/lib/mysql/mysql.sock' + ); + $dir = $amp_conf['ASTETCDIR']; $res_conf_sql = ini_get('pdo_mysql.default_socket'); - $res_conf_old = ''; $res_conf = ''; $ext_conf = ''; @@ -442,19 +444,18 @@ class extconfigs if (file_exists($dir . '/extconfig' . $fext)) { $ext_conf = $cnf_read->getConfig('extconfig' . $fext); if (!empty($ext_conf['settings']['sccpdevice'])) { - // Add chek line - if (strtolower($ext_conf['settings']['sccpdevice']) == strtolower($def_config['sccpdevice'])) { + if ($ext_conf['settings']['sccpdevice'] === $def_config['sccpdevice']) { $res['sccpdevice'] = 'OK'; $res['extconfigfile'] = 'extconfig' . $fext; } else { - $res['sccpdevice'] = 'Error in line sccpdevice ' . $res['sccpdevice']; + $res['sccpdevice'] .= ' Error in line sccpdevice '; } } if (!empty($ext_conf['settings']['sccpline'])) { - if (strtolower($ext_conf['settings']['sccpline']) == strtolower($def_config['sccpline'])) { + if ($ext_conf['settings']['sccpline'] === $def_config['sccpline']) { $res['sccpline'] = 'OK'; } else { - $res['sccpline'] = 'Error in line sccpline'; + $res['sccpline'] .= ' Error in line sccpline '; } } } @@ -473,41 +474,31 @@ class extconfigs $res['extconfig'] = 'File extconfig.conf does not exist'; } - if (!empty($res_conf_sql)) { if (file_exists($res_conf_sql)) { $def_bd_config['dbsock'] = $res_conf_sql; } } - if (file_exists($dir . '/res_mysql.conf')) { - $res_conf = $cnf_read->getConfig('res_mysql.conf'); - if (empty($res_conf[$realm])) { - $res['mysqlconfig'] = 'Config not found in file: res_mysql.conf'; - } else { - if ($res_conf[$realm]['dbsock'] != $def_bd_config['dbsock']) { - $res['mysqlconfig'] = 'Mysql Socket Error in file: res_mysql.conf'; + // Check for mysql config files - should only be one depending on version + $mySqlConfigFiles = [ 'res_mysql.conf', 'res_config_mysql.conf' ]; + foreach ($mySqlConfigFiles as $sqlConfigFile) { + if (file_exists( $dir . '/' . $sqlConfigFile )) { + $res_conf = $cnf_read->getConfig($sqlConfigFile); + if (empty($res_conf[$connector])) { + $res['mysqlconfig'] = 'Config not found in file: ' . $sqlConfigFile; + } else { + if ($res_conf[$connector]['dbsock'] != $def_bd_config['dbsock']) { + $res['mysqlconfig'] = 'Mysql Socket Error in file: ' . $sqlConfigFile; + } + } + if (empty($res['mysqlconfig'])) { + $res['mysqlconfig'] = 'OK'; } - } - if (empty($res['mysqlconfig'])) { - $res['mysqlconfig'] = 'OK'; } } - if (file_exists($dir . '/res_config_mysql.conf')) { - $res_conf = $cnf_read->getConfig('res_config_mysql.conf'); - if (empty($res_conf[$realm])) { - $res['mysqlconfig'] = 'Not Config in file: res_config_mysql.conf'; - } else { - if ($res_conf[$realm]['dbsock'] != $def_bd_config['dbsock']) { - $res['mysqlconfig'] = 'Mysql Socket Error in file: res_config_mysql.conf'; - } - } - if (empty($res['mysqlconfig'])) { - $res['mysqlconfig'] = 'OK'; - } - } if (empty($res['mysqlconfig'])) { - $res['mysqlconfig'] = 'Realtime Error: not found res_config_mysql.conf or res_mysql.conf configutation on the path :' . $dir; + $res['mysqlconfig'] = 'Realtime Error: neither res_config_mysql.conf nor res_mysql.conf found in the path : ' . $dir; } return $res; } diff --git a/views/server.info.php b/views/server.info.php index 3982fed..d69bd5f 100644 --- a/views/server.info.php +++ b/views/server.info.php @@ -17,7 +17,7 @@ $ast_realtime = $this->srvinterface->sccp_realtime_status(); // if there are multiple connections, this will only return the first. foreach ($ast_realtime as $key => $value) { if (empty($ast_realm)) { - if ($value['status'] == 'OK') { + if ($value['status'] === 'OK') { $ast_realm = $key; } }