From 4dc8e2de564f48365da1ec9e1bab38f99cb34d3a Mon Sep 17 00:00:00 2001 From: Austin McKinley Date: Wed, 18 Apr 2018 10:49:22 -0700 Subject: [PATCH] Add unique constraint to AlmanacInterfaces Summary: See discussion in D19379. The 4-tuple of (device, network, address, port) should be unique. Test Plan: Created lots of duplicate interfaces, bound those interfaces to various services, observed migration script clean things up correctly. Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin, PHID-OPKG-gm6ozazyms6q6i22gyam Differential Revision: https://secure.phabricator.com/D19388 --- .../20180418.alamanc.interface.unique.php | 85 +++++++++++++++++++ .../almanac/editor/AlmanacInterfaceEditor.php | 18 ++++ .../almanac/storage/AlmanacInterface.php | 4 + 3 files changed, 107 insertions(+) create mode 100644 resources/sql/autopatches/20180418.alamanc.interface.unique.php diff --git a/resources/sql/autopatches/20180418.alamanc.interface.unique.php b/resources/sql/autopatches/20180418.alamanc.interface.unique.php new file mode 100644 index 0000000000..0ad4fbea12 --- /dev/null +++ b/resources/sql/autopatches/20180418.alamanc.interface.unique.php @@ -0,0 +1,85 @@ +establishConnection('w'); + +queryfx( + $interface_conn, + 'LOCK TABLES %T WRITE, %T WRITE', + $interface_table->getTableName(), + $binding_table->getTableName()); + +$seen = array(); +foreach (new LiskMigrationIterator($interface_table) as $interface) { + $device = $interface->getDevicePHID(); + $network = $interface->getNetworkPHID(); + $address = $interface->getAddress(); + $port = $interface->getPort(); + $key = "{$device}/{$network}/{$address}/{$port}"; + + // If this is the first copy of this row we've seen, mark it as seen and + // move on. + if (empty($seen[$key])) { + $seen[$key] = $interface->getID(); + continue; + } + + $survivor = queryfx_one( + $interface_conn, + 'SELECT * FROM %T WHERE id = %d', + $interface_table->getTableName(), + $seen[$key]); + + $bindings = queryfx_all( + $interface_conn, + 'SELECT * FROM %T WHERE interfacePHID = %s', + $binding_table->getTableName(), + $interface->getPHID()); + + // Repoint bindings to the survivor. + foreach ($bindings as $binding) { + // Check if there's already a binding to the survivor. + $existing = queryfx_one( + $interface_conn, + 'SELECT * FROM %T WHERE interfacePHID = %s and devicePHID = %s and '. + 'servicePHID = %s', + $binding_table->getTableName(), + $survivor['phid'], + $binding['devicePHID'], + $binding['servicePHID']); + + if (!$existing) { + // Reattach this binding to the survivor. + queryfx( + $interface_conn, + 'UPDATE %T SET interfacePHID = %s WHERE id = %d', + $binding_table->getTableName(), + $survivor['phid'], + $binding['id']); + } else { + // Binding to survivor already exists. Remove this now-redundant binding. + queryfx( + $interface_conn, + 'DELETE FROM %T WHERE id = %d', + $binding_table->getTableName(), + $binding['id']); + } + } + + queryfx( + $interface_conn, + 'DELETE FROM %T WHERE id = %d', + $interface_table->getTableName(), + $interface->getID()); +} + +queryfx( + $interface_conn, + 'ALTER TABLE %T ADD UNIQUE KEY `key_unique` '. + '(devicePHID, networkPHID, address, port)', + $interface_table->getTableName()); + +queryfx( + $interface_conn, + 'UNLOCK TABLES'); diff --git a/src/applications/almanac/editor/AlmanacInterfaceEditor.php b/src/applications/almanac/editor/AlmanacInterfaceEditor.php index 865402db36..771076efc9 100644 --- a/src/applications/almanac/editor/AlmanacInterfaceEditor.php +++ b/src/applications/almanac/editor/AlmanacInterfaceEditor.php @@ -15,4 +15,22 @@ final class AlmanacInterfaceEditor return pht('%s created %s.', $author, $object); } + protected function didCatchDuplicateKeyException( + PhabricatorLiskDAO $object, + array $xactions, + Exception $ex) { + + $errors = array(); + + $errors[] = new PhabricatorApplicationTransactionValidationError( + null, + pht('Invalid'), + pht( + 'Interfaces must have a unique combination of network, device, '. + 'address, and port.'), + null); + + throw new PhabricatorApplicationTransactionValidationException($errors); + } + } diff --git a/src/applications/almanac/storage/AlmanacInterface.php b/src/applications/almanac/storage/AlmanacInterface.php index 4002651d08..5c7f65ddd1 100644 --- a/src/applications/almanac/storage/AlmanacInterface.php +++ b/src/applications/almanac/storage/AlmanacInterface.php @@ -35,6 +35,10 @@ final class AlmanacInterface 'key_device' => array( 'columns' => array('devicePHID'), ), + 'key_unique' => array( + 'columns' => array('devicePHID', 'networkPHID', 'address', 'port'), + 'unique' => true, + ), ), ) + parent::getConfiguration(); }