Allow users to manage package dominion rules

Summary: Ref T10939. This adds UI, transactions, etc, to adjust dominion rules.

Test Plan:
  - Read documentation.
  - Changed dominion rules.
  - Created packages on `/` ("A") and `/x` ("B") with "Auto Review: Review".
  - Touched `/x`.
  - Verified that A and B were added with strong dominion.
  - Verified that only B was added when A was set to weak dominion.
  - Viewed file in Diffusion, saw correct ownership with strong/weak dominion rules.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10939

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

resources/sql/autopatches/20160516.owners.01.dominion.sql

@@ -0,0 +1,2 @@
+ALTER TABLE {$NAMESPACE}_owners.owners_package
+  ADD dominion VARCHAR(32) NOT NULL COLLATE {$COLLATE_TEXT};

resources/sql/autopatches/20160516.owners.02.dominionstrong.sql

@@ -0,0 +1,2 @@
+UPDATE {$NAMESPACE}_owners.owners_package
+  SET dominion = 'strong' WHERE dominion = '';

src/applications/differential/editor/DifferentialTransactionEditor.php

@@ -1723,6 +1723,10 @@ final class DifferentialTransactionEditor
       $paths[] = $path_prefix.'/'.$changeset->getFilename();
     }
 
+    // Save the affected paths; we'll use them later to query Owners. This
+    // uses the un-expanded paths.
+    $this->affectedPaths = $paths;
+
     // Mark this as also touching all parent paths, so you can see all pending
     // changes to any file within a directory.
     $all_paths = array();
@@ -1733,9 +1737,6 @@ final class DifferentialTransactionEditor
     }
     $all_paths = array_keys($all_paths);
 
-    // Save the affected paths; we'll use them later to query Owners.
-    $this->affectedPaths = $all_paths;
-
     $path_ids =
       PhabricatorRepositoryCommitChangeParserWorker::lookupOrCreatePaths(
         $all_paths);

src/applications/owners/controller/PhabricatorOwnersDetailController.php

@@ -184,6 +184,13 @@ final class PhabricatorOwnersDetailController
     }
     $view->addProperty(pht('Owners'), $owner_list);
 
+
+    $dominion = $package->getDominion();
+    $dominion_map = PhabricatorOwnersPackage::getDominionOptionsMap();
+    $spec = idx($dominion_map, $dominion, array());
+    $name = idx($spec, 'short', $dominion);
+    $view->addProperty(pht('Dominion'), $name);
+
     $auto = $package->getAutoReview();
     $autoreview_map = PhabricatorOwnersPackage::getAutoreviewOptionsMap();
     $spec = idx($autoreview_map, $auto, array());

src/applications/owners/editor/PhabricatorOwnersPackageEditEngine.php

@@ -87,6 +87,9 @@ EOTEXT
     $autoreview_map = PhabricatorOwnersPackage::getAutoreviewOptionsMap();
     $autoreview_map = ipull($autoreview_map, 'name');
 
+    $dominion_map = PhabricatorOwnersPackage::getDominionOptionsMap();
+    $dominion_map = ipull($dominion_map, 'name');
+
     return array(
       id(new PhabricatorTextEditField())
         ->setKey('name')
@@ -103,6 +106,16 @@ EOTEXT
         ->setDatasource(new PhabricatorProjectOrUserDatasource())
         ->setIsCopyable(true)
         ->setValue($object->getOwnerPHIDs()),
+      id(new PhabricatorSelectEditField())
+        ->setKey('dominion')
+        ->setLabel(pht('Dominion'))
+        ->setDescription(
+          pht('Change package dominion rules.'))
+        ->setTransactionType(
+          PhabricatorOwnersPackageTransaction::TYPE_DOMINION)
+        ->setIsCopyable(true)
+        ->setValue($object->getDominion())
+        ->setOptions($dominion_map),
       id(new PhabricatorSelectEditField())
         ->setKey('autoReview')
         ->setLabel(pht('Auto Review'))

src/applications/owners/editor/PhabricatorOwnersPackageTransactionEditor.php

@@ -21,6 +21,7 @@ final class PhabricatorOwnersPackageTransactionEditor
     $types[] = PhabricatorOwnersPackageTransaction::TYPE_PATHS;
     $types[] = PhabricatorOwnersPackageTransaction::TYPE_STATUS;
     $types[] = PhabricatorOwnersPackageTransaction::TYPE_AUTOREVIEW;
+    $types[] = PhabricatorOwnersPackageTransaction::TYPE_DOMINION;
 
     $types[] = PhabricatorTransactions::TYPE_VIEW_POLICY;
     $types[] = PhabricatorTransactions::TYPE_EDIT_POLICY;
@@ -50,6 +51,8 @@ final class PhabricatorOwnersPackageTransactionEditor
         return $object->getStatus();
       case PhabricatorOwnersPackageTransaction::TYPE_AUTOREVIEW:
         return $object->getAutoReview();
+      case PhabricatorOwnersPackageTransaction::TYPE_DOMINION:
+        return $object->getDominion();
     }
   }
 
@@ -62,6 +65,7 @@ final class PhabricatorOwnersPackageTransactionEditor
       case PhabricatorOwnersPackageTransaction::TYPE_DESCRIPTION:
       case PhabricatorOwnersPackageTransaction::TYPE_STATUS:
       case PhabricatorOwnersPackageTransaction::TYPE_AUTOREVIEW:
+      case PhabricatorOwnersPackageTransaction::TYPE_DOMINION:
         return $xaction->getNewValue();
       case PhabricatorOwnersPackageTransaction::TYPE_PATHS:
         $new = $xaction->getNewValue();
@@ -120,6 +124,9 @@ final class PhabricatorOwnersPackageTransactionEditor
       case PhabricatorOwnersPackageTransaction::TYPE_AUTOREVIEW:
         $object->setAutoReview($xaction->getNewValue());
         return;
+      case PhabricatorOwnersPackageTransaction::TYPE_DOMINION:
+        $object->setDominion($xaction->getNewValue());
+        return;
     }
 
     return parent::applyCustomInternalTransaction($object, $xaction);
@@ -135,6 +142,7 @@ final class PhabricatorOwnersPackageTransactionEditor
       case PhabricatorOwnersPackageTransaction::TYPE_AUDITING:
       case PhabricatorOwnersPackageTransaction::TYPE_STATUS:
       case PhabricatorOwnersPackageTransaction::TYPE_AUTOREVIEW:
+      case PhabricatorOwnersPackageTransaction::TYPE_DOMINION:
         return;
       case PhabricatorOwnersPackageTransaction::TYPE_OWNERS:
         $old = $xaction->getOldValue();
@@ -249,6 +257,26 @@ final class PhabricatorOwnersPackageTransactionEditor
           }
         }
         break;
+      case PhabricatorOwnersPackageTransaction::TYPE_DOMINION:
+        $map = PhabricatorOwnersPackage::getDominionOptionsMap();
+        foreach ($xactions as $xaction) {
+          $new = $xaction->getNewValue();
+
+          if (empty($map[$new])) {
+            $valid = array_keys($map);
+
+            $errors[] = new PhabricatorApplicationTransactionValidationError(
+              $type,
+              pht('Invalid'),
+              pht(
+                'Dominion setting "%s" is not valid. '.
+                'Valid settings are: %s.',
+                $new,
+                implode(', ', $valid)),
+              $xaction);
+          }
+        }
+        break;
       case PhabricatorOwnersPackageTransaction::TYPE_PATHS:
         if (!$xactions) {
           continue;

src/applications/owners/query/PhabricatorOwnersPackageQuery.php

@@ -351,6 +351,7 @@ final class PhabricatorOwnersPackageQuery
     }
 
     $packages = $this->controlResults;
+    $weak_dominion = PhabricatorOwnersPackage::DOMINION_WEAK;
 
     $matches = array();
     foreach ($packages as $package_id => $package) {
@@ -373,6 +374,7 @@ final class PhabricatorOwnersPackageQuery
       if ($best_match && $include) {
         $matches[$package_id] = array(
           'strength' => $best_match,
+          'weak' => ($package->getDominion() == $weak_dominion),
           'package' => $package,
         );
       }
@@ -381,6 +383,18 @@ final class PhabricatorOwnersPackageQuery
     $matches = isort($matches, 'strength');
     $matches = array_reverse($matches);
 
+    $first_id = null;
+    foreach ($matches as $package_id => $match) {
+      if ($first_id === null) {
+        $first_id = $package_id;
+        continue;
+      }
+
+      if ($match['weak']) {
+        unset($matches[$package_id]);
+      }
+    }
+
     return array_values(ipull($matches, 'package'));
   }
 

src/applications/owners/storage/PhabricatorOwnersPackage.php

@@ -21,6 +21,7 @@ final class PhabricatorOwnersPackage
   protected $status;
   protected $viewPolicy;
   protected $editPolicy;
+  protected $dominion;
 
   private $paths = self::ATTACHABLE;
   private $owners = self::ATTACHABLE;
@@ -51,6 +52,7 @@ final class PhabricatorOwnersPackage
     return id(new PhabricatorOwnersPackage())
       ->setAuditingEnabled(0)
       ->setAutoReview(self::AUTOREVIEW_NONE)
+      ->setDominion(self::DOMINION_STRONG)
       ->setViewPolicy($view_policy)
       ->setEditPolicy($edit_policy)
       ->attachPaths(array())
@@ -83,6 +85,19 @@ final class PhabricatorOwnersPackage
     );
   }
 
+  public static function getDominionOptionsMap() {
+    return array(
+      self::DOMINION_STRONG => array(
+        'name' => pht('Strong (Control All Paths)'),
+        'short' => pht('Strong'),
+      ),
+      self::DOMINION_WEAK => array(
+        'name' => pht('Weak (Control Unowned Paths)'),
+        'short' => pht('Weak'),
+      ),
+    );
+  }
+
   protected function getConfiguration() {
     return array(
       // This information is better available from the history table.
@@ -97,6 +112,7 @@ final class PhabricatorOwnersPackage
         'mailKey' => 'bytes20',
         'status' => 'text32',
         'autoReview' => 'text32',
+        'dominion' => 'text32',
       ),
     ) + parent::getConfiguration();
   }
@@ -193,7 +209,7 @@ final class PhabricatorOwnersPackage
     foreach (array_chunk(array_keys($fragments), 128) as $chunk) {
       $rows[] = queryfx_all(
         $conn,
-        'SELECT pkg.id, "strong" dominion, p.excluded, p.path
+        'SELECT pkg.id, pkg.dominion, p.excluded, p.path
           FROM %T pkg JOIN %T p ON p.packageID = pkg.id
           WHERE p.path IN (%Ls) %Q',
         $package->getTableName(),

src/applications/owners/storage/PhabricatorOwnersPackageTransaction.php

@@ -11,6 +11,7 @@ final class PhabricatorOwnersPackageTransaction
   const TYPE_PATHS = 'owners.paths';
   const TYPE_STATUS = 'owners.status';
   const TYPE_AUTOREVIEW = 'owners.autoreview';
+  const TYPE_DOMINION = 'owners.dominion';
 
   public function getApplicationName() {
     return 'owners';
@@ -156,6 +157,18 @@ final class PhabricatorOwnersPackageTransaction
           $this->renderHandleLink($author_phid),
           $old,
           $new);
+      case self::TYPE_DOMINION:
+        $map = PhabricatorOwnersPackage::getDominionOptionsMap();
+        $map = ipull($map, 'short');
+
+        $old = idx($map, $old, $old);
+        $new = idx($map, $new, $new);
+
+        return pht(
+          '%s adjusted package dominion rules from "%s" to "%s".',
+          $this->renderHandleLink($author_phid),
+          $old,
+          $new);
     }
 
     return parent::getTitle();

src/docs/user/userguide/owners.diviner

@@ -45,6 +45,38 @@ belonging to the package when you look at them in Diffusion, or look at changes
 which affect them in Diffusion or Differential.
 
 
+Dominion
+========
+
+The **Dominion** option allows you to control how ownership cascades when
+multiple packages own a path. The dominion rules are:
+
+**Strong Dominion.** This is the default. In this mode, the package will always
+own all files matching its configured paths, even if another package also owns
+them.
+
+For example, if the package owns `a/`, it will always own `a/b/c.z` even if
+another package owns `a/b/`. In this case, both packages will own `a/b/c.z`.
+
+This mode prevents users from stealing files away from the package by defining
+more narrow ownership rules in new packages, but enforces hierarchical
+ownership rules.
+
+**Weak Dominion.** In this mode, the package will only own files which do not
+match a more specific path in another package.
+
+For example, if the package owns `a/` but another package owns `a/b/`, the
+package will no longer consider `a/b/c.z` to be a file it owns because another
+package matches the path with a more specific rule.
+
+This mode lets you to define rules without implicit hierarchical ownership,
+but allows users to steal files away from a package by defining a more
+specific package.
+
+For more details on files which match multiple packages, see
+"Files in Multiple Packages", below.
+
+
 Auto Review
 ===========
 
@@ -93,4 +125,6 @@ configuration, these files are part of three packages: "iOS Application",
 "Android Application", and "Design Assets".
 
 (You can use an "exclude" rule if you want to make a different package with a
-more specific claim the owner of a file or subdirectory.)
+more specific claim the owner of a file or subdirectory. You can also change
+the **Dominion** setting for a package to let it give up ownership of paths
+owned by another package.)