refine more exception classes in port management
authorDenis Ovsienko <denis@ovsienko.info>
Mon, 8 May 2017 11:52:13 +0000 (12:52 +0100)
committerDenis Ovsienko <denis@ovsienko.info>
Mon, 8 May 2017 12:05:51 +0000 (13:05 +0100)
InvalidArgException means that a parameter has an invalid value in
general, whereas InvalidRequestArgException specifically means that
in addition to that the parameter value is the user input from a more
or less interactive session.

In the latter case the error message should be displayed back to the user
or, if the code does not expect the IRAE class for whatever reason, the
exception should land in the [supposedly present] more general IAE catch
block.

With that in mind fix a long-standing anomaly by making commitAddPort()
and commitUpdatePort() throw an IAE object instead of IRAE. Make changes
to addPortForObject(), editPortForObject(), autoPopulateUCS() and
renameObjectPorts() such that those functions expect an IAE and can
reasonably handle it. Amend test declarations as necessary.

While at it, move code around in syncObjectPorts() to make sure all
exceptions cause the tables lock released soonest possible.

tests/PortTriggerTest.php
wwwroot/inc/database.php
wwwroot/inc/functions.php
wwwroot/inc/ophandlers.php

index a948dddd5cfed81689b386309f1f88aa7831ccbe..a058a89fbf450fb5609fde2955e361ff77046d4f 100644 (file)
@@ -36,7 +36,7 @@ class PortTriggerTest extends PHPUnit_Framework_TestCase
 
        /**
         * @group small
-        * @expectedException InvalidRequestArgException
+        * @expectedException InvalidArgException
         */
        public function testUniqueMacAdd()
        {
@@ -45,7 +45,7 @@ class PortTriggerTest extends PHPUnit_Framework_TestCase
 
        /**
         * @group small
-        * @expectedException InvalidRequestArgException
+        * @expectedException InvalidArgException
         */
        public function testUniqueMacUpdate()
        {
index d22396d475a3bc77a44cbeb3acade0777ab5d48f..207dbb349c8dd993a90a3553facd7404ac5f2776 100644 (file)
@@ -1690,7 +1690,7 @@ function commitAddPort ($object_id, $port_name, $port_type_id, $port_label, $por
        try
        {
                if ($db_l2address != '' && alreadyUsedL2Address ($db_l2address, $object_id))
-                       throw new InvalidRequestArgException ('port_l2address', $port_l2address, 'address belongs to another object');
+                       throw new InvalidArgException ('port_l2address', $port_l2address, 'address belongs to another object');
                $ret = commitAddPortReal ($object_id, $port_name, $iif_id, $oif_id, $port_label, $db_l2address);
        }
        catch (Exception $e)
@@ -1743,7 +1743,7 @@ function commitUpdatePort ($object_id, $port_id, $port_name, $port_type_id, $por
        try
        {
                if ($db_l2address != '' && alreadyUsedL2Address ($db_l2address, $object_id))
-                       throw new InvalidRequestArgException ('port_l2address', $port_l2address, 'address belongs to another object');
+                       throw new InvalidArgException ('port_l2address', $port_l2address, 'address belongs to another object');
                commitUpdatePortReal ($object_id, $port_id, $port_name, $iif_id, $oif_id, $port_label, $db_l2address, $port_reservation_comment);
        }
        catch (Exception $e)
index 0886f14e6a1fda1f028aab46155181af7d0ecaed..2fc9720fe4378d1f0552f09184e7276302df7682 100644 (file)
@@ -6629,13 +6629,13 @@ function syncObjectPorts ($object_id, $desiredPorts)
                $real_ports[$key] = 1;
        }
        $to_add = array_diff_key ($desiredPorts, $real_ports);
-       foreach (array_merge ($to_update, $to_add) as $port)
-               if ($port['l2address'] != '' && alreadyUsedL2Address ($port['l2address'], $object_id))
-                       throw new InvalidRequestArgException ('l2address', $port['l2address'], 'address belongs to another object');
 
-       // Make the actual changes.
        try
        {
+               foreach (array_merge ($to_update, $to_add) as $port)
+                       if ($port['l2address'] != '' && alreadyUsedL2Address ($port['l2address'], $object_id))
+                               throw new InvalidRequestArgException ('l2address', $port['l2address'], 'address belongs to another object');
+               // Make the actual changes.
                foreach ($to_delete as $port)
                        if ($port['link_count'] != 0)
                                showWarning (sprintf ("Port %s should be deleted, but it's used", formatPort ($port)));
index bd75cf211da7811df9be239b21b55aeca4788fc0..904c4e5e449df0ebe177a4c2015938a3d59aaa64 100644 (file)
@@ -815,14 +815,25 @@ function addPortForObject ()
 {
        setFuncMessages (__FUNCTION__, array ('OK' => 48));
        genericAssertion ('port_name', 'string');
-       commitAddPort
-       (
-               getBypassValue(),
-               trim ($_REQUEST['port_name']),
-               genericAssertion ('port_type_id', 'string'),
-               trim ($_REQUEST['port_label']),
-               trim (genericAssertion ('port_l2address', 'l2address0'))
-       );
+       try
+       {
+               commitAddPort
+               (
+                       getBypassValue(),
+                       trim ($_REQUEST['port_name']),
+                       genericAssertion ('port_type_id', 'string'),
+                       trim ($_REQUEST['port_label']),
+                       trim (genericAssertion ('port_l2address', 'l2address0'))
+               );
+       }
+       catch (InvalidRequestArgException $irae)
+       {
+               throw $irae;
+       }
+       catch (InvalidArgException $iae)
+       {
+               throw $iae->newIRAE();
+       }
        showFuncMessage (__FUNCTION__, 'OK', array ($_REQUEST['port_name']));
 }
 
@@ -831,16 +842,27 @@ function editPortForObject ()
        setFuncMessages (__FUNCTION__, array ('OK' => 6));
        global $sic;
        $port_id = assertUIntArg ('port_id');
-       commitUpdatePort
-       (
-               getBypassValue(),
-               $port_id,
-               genericAssertion ('name', 'string'),
-               assertStringArg ('port_type_id'),
-               genericAssertion ('label', 'string0'),
-               genericAssertion ('l2address', 'l2address0'),
-               assertStringArg ('reservation_comment', TRUE)
-       );
+       try
+       {
+               commitUpdatePort
+               (
+                       getBypassValue(),
+                       $port_id,
+                       genericAssertion ('name', 'string'),
+                       assertStringArg ('port_type_id'),
+                       genericAssertion ('label', 'string0'),
+                       genericAssertion ('l2address', 'l2address0'),
+                       assertStringArg ('reservation_comment', TRUE)
+               );
+       }
+       catch (InvalidRequestArgException $irae)
+       {
+               throw $irae;
+       }
+       catch (InvalidArgException $iae)
+       {
+               throw $iae->newIRAE();
+       }
        if (array_key_exists ('cable', $_REQUEST))
                commitUpdatePortLink ($port_id, $sic['cable']);
        showFuncMessage (__FUNCTION__, 'OK', array ($_REQUEST['name']));
@@ -3363,9 +3385,17 @@ function autoPopulateUCS()
                case 'VnicPort':
                        $spname = preg_replace ('#^([^/]+)/ls-([^/]+)/([^/]+)$#', '${2}', $item['DN']) . "(" . $oinfo['name'] . ")";
                        $porttype = preg_replace ('#^([^/]+)/([^/]+)/([^-/]+)-.+$#', '${3}', $item['DN']);
-                       #        Add "virtual"(1469) ports for associated blades only
-                       if ($spid = $spname_id[$spname])
-                               commitAddPort ($spid, $item['name'], 1469, $porttype, $item['addr']);
+                       try
+                       {
+                               // Add "virtual" (1469) ports for associated blades only. The attempt may fail
+                               // due to incorrect port type or MAC address.
+                               if ($spid = $spname_id[$spname])
+                                       commitAddPort ($spid, $item['name'], 1469, $porttype, $item['addr']);
+                       }
+                       catch (InvalidArgException $iae)
+                       {
+                               showError ($iae->getMessage());
+                       }
                        break;
                case 'ComputeRackUnit':
                        if ($item['assigned'] == '')
@@ -3741,8 +3771,15 @@ function renameObjectPorts()
                $canon_pn = shortenPortName ($port['name'], $port['object_id']);
                if ($canon_pn != $port['name'])
                {
-                       commitUpdatePort ($object_id, $port['id'], $canon_pn, $port['oif_id'], $port['label'], $port['l2address'], $port['reservation_comment']);
-                       $n++;
+                       try
+                       {
+                               commitUpdatePort ($object_id, $port['id'], $canon_pn, $port['oif_id'], $port['label'], $port['l2address'], $port['reservation_comment']);
+                               $n++;
+                       }
+                       catch (InvalidArgException $iae)
+                       {
+                               showError ($iae->getMessage());
+                       }
                }
        }
        if ($n)