Remove TYPE_INTERFACE transaction from Almanac Device

Summary:
Depends on D19325. Ref T13120. Ref T12414.

This no longer has any callers in the upstream or in Phacility support libraries, so get rid of it.

This will make modularizing Device transactions significantly easier, since the other transactions are reasonable, normal sorts of transactions.

For existing devices, this leaves some "author edited this object." transactions in the log. I might just leave those since they aren't really hurting anything, or maybe I'll clean them up or hide them later once I have more confidence that these changes are stable.

Test Plan: Grepped for `TYPE_INTERFACE` and `AlmanacDeviceTransaction`, found no callsites.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13120, T12414

Differential Revision: https://secure.phabricator.com/D19328

src/applications/almanac/editor/AlmanacDeviceEditor.php

@@ -11,7 +11,7 @@ final class AlmanacDeviceEditor
     $types = parent::getTransactionTypes();
 
     $types[] = AlmanacDeviceTransaction::TYPE_NAME;
-    $types[] = AlmanacDeviceTransaction::TYPE_INTERFACE;
+
     $types[] = PhabricatorTransactions::TYPE_VIEW_POLICY;
     $types[] = PhabricatorTransactions::TYPE_EDIT_POLICY;
 
@@ -35,7 +35,6 @@ final class AlmanacDeviceEditor
 
     switch ($xaction->getTransactionType()) {
       case AlmanacDeviceTransaction::TYPE_NAME:
-      case AlmanacDeviceTransaction::TYPE_INTERFACE:
         return $xaction->getNewValue();
     }
 
@@ -50,8 +49,6 @@ final class AlmanacDeviceEditor
       case AlmanacDeviceTransaction::TYPE_NAME:
         $object->setName($xaction->getNewValue());
         return;
-      case AlmanacDeviceTransaction::TYPE_INTERFACE:
-        return;
     }
 
     return parent::applyCustomInternalTransaction($object, $xaction);
@@ -64,37 +61,6 @@ final class AlmanacDeviceEditor
     switch ($xaction->getTransactionType()) {
       case AlmanacDeviceTransaction::TYPE_NAME:
         return;
-      case AlmanacDeviceTransaction::TYPE_INTERFACE:
-        $old = $xaction->getOldValue();
-        if ($old) {
-          $interface = id(new AlmanacInterfaceQuery())
-            ->setViewer($this->requireActor())
-            ->withIDs(array($old['id']))
-            ->executeOne();
-          if (!$interface) {
-            throw new Exception(pht('Unable to load interface!'));
-          }
-        } else {
-          $interface = AlmanacInterface::initializeNewInterface()
-            ->setDevicePHID($object->getPHID());
-        }
-
-        $new = $xaction->getNewValue();
-        if ($new) {
-          $interface
-            ->setNetworkPHID($new['networkPHID'])
-            ->setAddress($new['address'])
-            ->setPort((int)$new['port']);
-
-          if (idx($new, 'phid')) {
-            $interface->setPHID($new['phid']);
-          }
-
-          $interface->save();
-        } else {
-          $interface->delete();
-        }
-        return;
     }
 
     return parent::applyCustomExternalTransaction($object, $xaction);
@@ -180,153 +146,6 @@ final class AlmanacDeviceEditor
         }
 
         break;
-      case AlmanacDeviceTransaction::TYPE_INTERFACE:
-        // We want to make sure that all the affected networks are visible to
-        // the actor, any edited interfaces exist, and that the actual address
-        // components are valid.
-
-        $network_phids = array();
-        foreach ($xactions as $xaction) {
-          $old = $xaction->getOldValue();
-          $new = $xaction->getNewValue();
-          if ($old) {
-            $network_phids[] = $old['networkPHID'];
-          }
-          if ($new) {
-            $network_phids[] = $new['networkPHID'];
-
-            $address = $new['address'];
-            if (!strlen($address)) {
-              $error = new PhabricatorApplicationTransactionValidationError(
-                $type,
-                pht('Invalid'),
-                pht('Interfaces must have an address.'),
-                $xaction);
-              $errors[] = $error;
-            } else {
-              // TODO: Validate addresses, but IPv6 addresses are not trivial
-              // to validate.
-            }
-
-            $port = $new['port'];
-            if (!strlen($port)) {
-              $error = new PhabricatorApplicationTransactionValidationError(
-                $type,
-                pht('Invalid'),
-                pht('Interfaces must have a port.'),
-                $xaction);
-              $errors[] = $error;
-            } else if ((int)$port < 1 || (int)$port > 65535) {
-              $error = new PhabricatorApplicationTransactionValidationError(
-                $type,
-                pht('Invalid'),
-                pht(
-                  'Port numbers must be between 1 and 65535, inclusive.'),
-                $xaction);
-              $errors[] = $error;
-            }
-
-            $phid = idx($new, 'phid');
-            if ($phid) {
-              $interface_phid_type = AlmanacInterfacePHIDType::TYPECONST;
-              if (phid_get_type($phid) !== $interface_phid_type) {
-                $error = new PhabricatorApplicationTransactionValidationError(
-                  $type,
-                  pht('Invalid'),
-                  pht(
-                    'Precomputed interface PHIDs must be of type '.
-                    'AlmanacInterfacePHIDType.'),
-                  $xaction);
-                $errors[] = $error;
-              }
-            }
-          }
-        }
-
-        if ($network_phids) {
-          $networks = id(new AlmanacNetworkQuery())
-            ->setViewer($this->requireActor())
-            ->withPHIDs($network_phids)
-            ->execute();
-          $networks = mpull($networks, null, 'getPHID');
-        } else {
-          $networks = array();
-        }
-
-        $addresses = array();
-        foreach ($xactions as $xaction) {
-          $old = $xaction->getOldValue();
-          if ($old) {
-            $network = idx($networks, $old['networkPHID']);
-            if (!$network) {
-              $error = new PhabricatorApplicationTransactionValidationError(
-                $type,
-                pht('Invalid'),
-                pht(
-                  'You can not edit an interface which belongs to a '.
-                  'nonexistent or restricted network.'),
-                $xaction);
-              $errors[] = $error;
-            }
-
-            $addresses[] = $old['id'];
-          }
-
-          $new = $xaction->getNewValue();
-          if ($new) {
-            $network = idx($networks, $new['networkPHID']);
-            if (!$network) {
-              $error = new PhabricatorApplicationTransactionValidationError(
-                $type,
-                pht('Invalid'),
-                pht(
-                  'You can not add an interface on a nonexistent or '.
-                  'restricted network.'),
-                $xaction);
-              $errors[] = $error;
-            }
-          }
-        }
-
-        if ($addresses) {
-          $interfaces = id(new AlmanacInterfaceQuery())
-            ->setViewer($this->requireActor())
-            ->withDevicePHIDs(array($object->getPHID()))
-            ->withIDs($addresses)
-            ->execute();
-          $interfaces = mpull($interfaces, null, 'getID');
-        } else {
-          $interfaces = array();
-        }
-
-        foreach ($xactions as $xaction) {
-          $old = $xaction->getOldValue();
-          if ($old) {
-            $interface = idx($interfaces, $old['id']);
-            if (!$interface) {
-              $error = new PhabricatorApplicationTransactionValidationError(
-                $type,
-                pht('Invalid'),
-                pht('You can not edit an invalid or restricted interface.'),
-                $xaction);
-              $errors[] = $error;
-              continue;
-            }
-
-            $new = $xaction->getNewValue();
-            if (!$new) {
-              if ($interface->loadIsInUse()) {
-                $error = new PhabricatorApplicationTransactionValidationError(
-                  $type,
-                  pht('In Use'),
-                  pht('You can not delete an interface which is still in use.'),
-                  $xaction);
-                $errors[] = $error;
-              }
-            }
-          }
-        }
-      break;
     }
 
     return $errors;

src/applications/almanac/storage/AlmanacDeviceTransaction.php

@@ -4,7 +4,6 @@ final class AlmanacDeviceTransaction
   extends AlmanacTransaction {
 
   const TYPE_NAME = 'almanac:device:name';
-  const TYPE_INTERFACE = 'almanac:device:interface';
 
   public function getApplicationName() {
     return 'almanac';
@@ -18,26 +17,6 @@ final class AlmanacDeviceTransaction
     return null;
   }
 
-  public function getRequiredHandlePHIDs() {
-    $phids = parent::getRequiredHandlePHIDs();
-
-    $old = $this->getOldValue();
-    $new = $this->getNewValue();
-
-    switch ($this->getTransactionType()) {
-      case self::TYPE_INTERFACE:
-        if ($old) {
-          $phids[] = $old['networkPHID'];
-        }
-        if ($new) {
-          $phids[] = $new['networkPHID'];
-        }
-        break;
-    }
-
-    return $phids;
-  }
-
   public function getTitle() {
     $author_phid = $this->getAuthorPHID();
 
@@ -58,43 +37,9 @@ final class AlmanacDeviceTransaction
             $new);
         }
         break;
-      case self::TYPE_INTERFACE:
-        if ($old && $new) {
-          return pht(
-            '%s changed interface %s on this device to %s.',
-            $this->renderHandleLink($author_phid),
-            $this->describeInterface($old),
-            $this->describeInterface($new));
-        } else if ($old) {
-          return pht(
-            '%s removed the interface %s from this device.',
-            $this->renderHandleLink($author_phid),
-            $this->describeInterface($old));
-        } else if ($new) {
-          return pht(
-            '%s added the interface %s to this device.',
-            $this->renderHandleLink($author_phid),
-            $this->describeInterface($new));
-        }
     }
 
     return parent::getTitle();
   }
 
-  public function shouldGenerateOldValue() {
-    switch ($this->getTransactionType()) {
-      case self::TYPE_INTERFACE:
-        return false;
-    }
-    return parent::shouldGenerateOldValue();
-  }
-
-  private function describeInterface(array $info) {
-    return pht(
-      '%s:%s (%s)',
-      $info['address'],
-      $info['port'],
-      $this->renderHandleLink($info['networkPHID']));
-  }
-
 }