Materialize parent project memberships

Summary:
Ref T10010. Subprojects have the following general membership rule: if you are a member of a subproject ("Engineering > Backend"), you are also a member of the parent project.

It would be unreasonably difficult to implement this rule directly in SQL when querying `withMemberPHIDs()`, because we'd have to do an arbitrarily large number of arbitrarily deep joins, or fetch and then requery a lot of data.

Instead, introduce "materailized members", which are just a copy of all the effective members of a project. When a subproject has a membership change, we go recompute the effective membership of all the parent projects. Then we can just JOIN to satisfy `withMemberPHIDs()`.

Having this process avialable will also be useful in the future, when a project's membership might be defined by some external source.

Also make milestones mostly work like we'd expect them to with respect to membership and visibility.

Test Plan:
  - Added and executed unit tests.
  - Changed project members, verified materialized members populated correctly in the database.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10010

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

src/__phutil_library_map__.php

@@ -2855,6 +2855,7 @@ phutil_register_library_map(array(
     'PhabricatorProjectLogicalOrNotDatasource' => 'applications/project/typeahead/PhabricatorProjectLogicalOrNotDatasource.php',
     'PhabricatorProjectLogicalUserDatasource' => 'applications/project/typeahead/PhabricatorProjectLogicalUserDatasource.php',
     'PhabricatorProjectLogicalViewerDatasource' => 'applications/project/typeahead/PhabricatorProjectLogicalViewerDatasource.php',
+    'PhabricatorProjectMaterializedMemberEdgeType' => 'applications/project/edge/PhabricatorProjectMaterializedMemberEdgeType.php',
     'PhabricatorProjectMemberOfProjectEdgeType' => 'applications/project/edge/PhabricatorProjectMemberOfProjectEdgeType.php',
     'PhabricatorProjectMembersDatasource' => 'applications/project/typeahead/PhabricatorProjectMembersDatasource.php',
     'PhabricatorProjectMembersEditController' => 'applications/project/controller/PhabricatorProjectMembersEditController.php',
@@ -2889,6 +2890,7 @@ phutil_register_library_map(array(
     'PhabricatorProjectsEditEngineExtension' => 'applications/project/engineextension/PhabricatorProjectsEditEngineExtension.php',
     'PhabricatorProjectsEditField' => 'applications/transactions/editfield/PhabricatorProjectsEditField.php',
     'PhabricatorProjectsFulltextEngineExtension' => 'applications/project/engineextension/PhabricatorProjectsFulltextEngineExtension.php',
+    'PhabricatorProjectsMembershipIndexEngineExtension' => 'applications/project/engineextension/PhabricatorProjectsMembershipIndexEngineExtension.php',
     'PhabricatorProjectsPolicyRule' => 'applications/project/policyrule/PhabricatorProjectsPolicyRule.php',
     'PhabricatorProjectsSearchEngineAttachment' => 'applications/project/engineextension/PhabricatorProjectsSearchEngineAttachment.php',
     'PhabricatorProjectsSearchEngineExtension' => 'applications/project/engineextension/PhabricatorProjectsSearchEngineExtension.php',
@@ -7189,6 +7191,7 @@ phutil_register_library_map(array(
     'PhabricatorProjectLogicalOrNotDatasource' => 'PhabricatorTypeaheadCompositeDatasource',
     'PhabricatorProjectLogicalUserDatasource' => 'PhabricatorTypeaheadCompositeDatasource',
     'PhabricatorProjectLogicalViewerDatasource' => 'PhabricatorTypeaheadDatasource',
+    'PhabricatorProjectMaterializedMemberEdgeType' => 'PhabricatorEdgeType',
     'PhabricatorProjectMemberOfProjectEdgeType' => 'PhabricatorEdgeType',
     'PhabricatorProjectMembersDatasource' => 'PhabricatorTypeaheadCompositeDatasource',
     'PhabricatorProjectMembersEditController' => 'PhabricatorProjectController',
@@ -7226,6 +7229,7 @@ phutil_register_library_map(array(
     'PhabricatorProjectsEditEngineExtension' => 'PhabricatorEditEngineExtension',
     'PhabricatorProjectsEditField' => 'PhabricatorTokenizerEditField',
     'PhabricatorProjectsFulltextEngineExtension' => 'PhabricatorFulltextEngineExtension',
+    'PhabricatorProjectsMembershipIndexEngineExtension' => 'PhabricatorIndexEngineExtension',
     'PhabricatorProjectsPolicyRule' => 'PhabricatorPolicyRule',
     'PhabricatorProjectsSearchEngineAttachment' => 'PhabricatorSearchEngineAttachment',
     'PhabricatorProjectsSearchEngineExtension' => 'PhabricatorSearchEngineExtension',

src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php

@@ -187,6 +187,68 @@ final class PhabricatorProjectCoreTestCase extends PhabricatorTestCase {
     $this->assertEqual(2, count($projects));
   }
 
+  public function testMemberMaterialization() {
+    $material_type = PhabricatorProjectMaterializedMemberEdgeType::EDGECONST;
+
+    $user = $this->createUser();
+    $user->save();
+
+    $parent = $this->createProject($user);
+    $child = $this->createProject($user, $parent);
+
+    $this->joinProject($child, $user);
+
+    $parent_material = PhabricatorEdgeQuery::loadDestinationPHIDs(
+      $parent->getPHID(),
+      $material_type);
+
+    $this->assertEqual(
+      array($user->getPHID()),
+      $parent_material);
+  }
+
+  public function testMilestones() {
+    $user = $this->createUser();
+    $user->save();
+
+    $parent = $this->createProject($user);
+
+    $m1 = $this->createProject($user, $parent, true);
+    $m2 = $this->createProject($user, $parent, true);
+    $m3 = $this->createProject($user, $parent, true);
+
+    $this->assertEqual(1, $m1->getMilestoneNumber());
+    $this->assertEqual(2, $m2->getMilestoneNumber());
+    $this->assertEqual(3, $m3->getMilestoneNumber());
+  }
+
+  public function testMilestoneMembership() {
+    $user = $this->createUser();
+    $user->save();
+
+    $parent = $this->createProject($user);
+    $milestone = $this->createProject($user, $parent, true);
+
+    $this->joinProject($parent, $user);
+
+    $milestone = id(new PhabricatorProjectQuery())
+      ->setViewer($user)
+      ->withPHIDs(array($milestone->getPHID()))
+      ->executeOne();
+
+    $this->assertTrue($milestone->isUserMember($user->getPHID()));
+
+    $milestone = id(new PhabricatorProjectQuery())
+      ->setViewer($user)
+      ->withPHIDs(array($milestone->getPHID()))
+      ->needMembers(true)
+      ->executeOne();
+
+    $this->assertEqual(
+      array($user->getPHID()),
+      $milestone->getMemberPHIDs());
+  }
+
   public function testParentProject() {
     $user = $this->createUser();
     $user->save();
@@ -396,10 +458,12 @@ final class PhabricatorProjectCoreTestCase extends PhabricatorTestCase {
 
   private function createProject(
     PhabricatorUser $user,
-    PhabricatorProject $parent = null) {
+    PhabricatorProject $parent = null,
+    $is_milestone = false) {
 
     $project = PhabricatorProject::initializeNewProject($user);
 
+
     $name = pht('Test Project %d', mt_rand());
 
     $xactions = array();
@@ -414,6 +478,12 @@ final class PhabricatorProjectCoreTestCase extends PhabricatorTestCase {
         ->setNewValue($parent->getPHID());
     }
 
+    if ($is_milestone) {
+      $xactions[] = id(new PhabricatorProjectTransaction())
+        ->setTransactionType(PhabricatorProjectTransaction::TYPE_MILESTONE)
+        ->setNewValue(true);
+    }
+
     $this->applyTransactions($project, $user, $xactions);
 
     return $project;

src/applications/project/edge/PhabricatorProjectMaterializedMemberEdgeType.php

@@ -0,0 +1,8 @@
+<?php
+
+final class PhabricatorProjectMaterializedMemberEdgeType
+  extends PhabricatorEdgeType {
+
+  const EDGECONST = 60;
+
+}

src/applications/project/editor/PhabricatorProjectTransactionEditor.php

@@ -27,6 +27,7 @@ final class PhabricatorProjectTransactionEditor
     $types[] = PhabricatorProjectTransaction::TYPE_COLOR;
     $types[] = PhabricatorProjectTransaction::TYPE_LOCKED;
     $types[] = PhabricatorProjectTransaction::TYPE_PARENT;
+    $types[] = PhabricatorProjectTransaction::TYPE_MILESTONE;
 
     return $types;
   }
@@ -54,6 +55,7 @@ final class PhabricatorProjectTransactionEditor
       case PhabricatorProjectTransaction::TYPE_LOCKED:
         return (int)$object->getIsMembershipLocked();
       case PhabricatorProjectTransaction::TYPE_PARENT:
+      case PhabricatorProjectTransaction::TYPE_MILESTONE:
         return null;
     }
 
@@ -74,6 +76,20 @@ final class PhabricatorProjectTransactionEditor
       case PhabricatorProjectTransaction::TYPE_LOCKED:
       case PhabricatorProjectTransaction::TYPE_PARENT:
         return $xaction->getNewValue();
+      case PhabricatorProjectTransaction::TYPE_MILESTONE:
+        $current = queryfx_one(
+          $object->establishConnection('w'),
+          'SELECT MAX(milestoneNumber) n
+            FROM %T
+            WHERE parentProjectPHID = %s',
+          $object->getTableName(),
+          $object->getParentProject()->getPHID());
+        if (!$current) {
+          $number = 1;
+        } else {
+          $number = (int)$current['n'] + 1;
+        }
+        return $number;
     }
 
     return parent::getCustomTransactionNewValue($object, $xaction);
@@ -109,6 +125,9 @@ final class PhabricatorProjectTransactionEditor
       case PhabricatorProjectTransaction::TYPE_PARENT:
         $object->setParentProjectPHID($xaction->getNewValue());
         return;
+      case PhabricatorProjectTransaction::TYPE_MILESTONE:
+        $object->setMilestoneNumber($xaction->getNewValue());
+        return;
     }
 
     return parent::applyCustomInternalTransaction($object, $xaction);
@@ -161,6 +180,7 @@ final class PhabricatorProjectTransactionEditor
       case PhabricatorProjectTransaction::TYPE_COLOR:
       case PhabricatorProjectTransaction::TYPE_LOCKED:
       case PhabricatorProjectTransaction::TYPE_PARENT:
+      case PhabricatorProjectTransaction::TYPE_MILESTONE:
         return;
      }
 
@@ -590,4 +610,34 @@ final class PhabricatorProjectTransactionEditor
       ->setProjectPHID($object->getPHID())
       ->save();
   }
+
+
+  protected function applyFinalEffects(
+    PhabricatorLiskDAO $object,
+    array $xactions) {
+
+    $materialize = false;
+    foreach ($xactions as $xaction) {
+      switch ($xaction->getTransactionType()) {
+        case PhabricatorTransactions::TYPE_EDGE:
+          switch ($xaction->getMetadataValue('edge:type')) {
+            case PhabricatorProjectProjectHasMemberEdgeType::EDGECONST:
+              $materialize = true;
+              break;
+          }
+          break;
+        case PhabricatorProjectTransaction::TYPE_PARENT:
+          $materialize = true;
+          break;
+      }
+    }
+
+    if ($materialize) {
+      id(new PhabricatorProjectsMembershipIndexEngineExtension())
+        ->rematerialize($object);
+    }
+
+    return parent::applyFinalEffects($object, $xactions);
+  }
+
 }

src/applications/project/engineextension/PhabricatorProjectsMembershipIndexEngineExtension.php

@@ -0,0 +1,98 @@
+<?php
+
+final class PhabricatorProjectsMembershipIndexEngineExtension
+  extends PhabricatorIndexEngineExtension {
+
+  const EXTENSIONKEY = 'project.members';
+
+  public function getExtensionName() {
+    return pht('Project Members');
+  }
+
+  public function shouldIndexObject($object) {
+    if (!($object instanceof PhabricatorProject)) {
+      return false;
+    }
+
+    return true;
+  }
+
+  public function indexObject(
+    PhabricatorIndexEngine $engine,
+    $object) {
+
+    $this->rematerialize($object);
+  }
+
+  public function rematerialize(PhabricatorProject $project) {
+    $materialize = $project->getAncestorProjects();
+    array_unshift($materialize, $project);
+
+    foreach ($materialize as $project) {
+      $this->materializeProject($project);
+    }
+  }
+
+  private function materializeProject(PhabricatorProject $project) {
+    if ($project->isMilestone()) {
+      return;
+    }
+
+    $material_type = PhabricatorProjectMaterializedMemberEdgeType::EDGECONST;
+    $member_type = PhabricatorProjectProjectHasMemberEdgeType::EDGECONST;
+
+    $project_phid = $project->getPHID();
+
+    $descendants = id(new PhabricatorProjectQuery())
+      ->setViewer($this->getViewer())
+      ->withAncestorProjectPHIDs(array($project->getPHID()))
+      ->withIsMilestone(false)
+      ->withHasSubprojects(false)
+      ->execute();
+    $descendant_phids = mpull($descendants, 'getPHID');
+
+    if ($descendant_phids) {
+      $source_phids = $descendant_phids;
+      $has_subprojects = true;
+    } else {
+      $source_phids = array($project->getPHID());
+      $has_subprojects = false;
+    }
+
+    $conn_w = $project->establishConnection('w');
+
+    $project->openTransaction();
+
+      // Delete any existing materialized member edges.
+      queryfx(
+        $conn_w,
+        'DELETE FROM %T WHERE src = %s AND type = %s',
+        PhabricatorEdgeConfig::TABLE_NAME_EDGE,
+        $project_phid,
+        $material_type);
+
+      // Copy current member edges to create new materialized edges.
+      queryfx(
+        $conn_w,
+        'INSERT IGNORE INTO %T (src, type, dst, dateCreated, seq)
+          SELECT %s, %d, dst, dateCreated, seq FROM %T
+          WHERE src IN (%Ls) AND type = %d',
+        PhabricatorEdgeConfig::TABLE_NAME_EDGE,
+        $project_phid,
+        $material_type,
+        PhabricatorEdgeConfig::TABLE_NAME_EDGE,
+        $source_phids,
+        $member_type);
+
+      // Update the hasSubprojects flag.
+      queryfx(
+        $conn_w,
+        'UPDATE %T SET hasSubprojects = %d WHERE id = %d',
+        $project->getTableName(),
+        (int)$has_subprojects,
+        $project->getID());
+
+    $project->saveTransaction();
+  }
+
+}

src/applications/project/query/PhabricatorProjectQuery.php

@@ -14,6 +14,7 @@ final class PhabricatorProjectQuery
   private $ancestorPHIDs;
   private $parentPHIDs;
   private $isMilestone;
+  private $hasSubprojects;
   private $minDepth;
   private $maxDepth;
 
@@ -89,6 +90,15 @@ final class PhabricatorProjectQuery
     return $this;
   }
 
+  public function withHasSubprojects($has_subprojects) {
+    $this->hasSubprojects = $has_subprojects;
+    return $this;
+  }
+
+  public function getProperty() {
+    return $this->property;
+  }
+
   public function withDepthBetween($min, $max) {
     $this->minDepth = $min;
     $this->maxDepth = $max;
@@ -156,12 +166,8 @@ final class PhabricatorProjectQuery
   }
 
   protected function willFilterPage(array $projects) {
-    $project_phids = array();
     $ancestor_paths = array();
-
     foreach ($projects as $project) {
-      $project_phids[] = $project->getPHID();
-
       foreach ($project->getAncestorProjectPaths() as $path) {
         $ancestor_paths[$path] = $path;
       }
@@ -178,7 +184,6 @@ final class PhabricatorProjectQuery
     $projects = $this->linkProjectGraph($projects, $ancestors);
 
     $viewer_phid = $this->getViewer()->getPHID();
-    $project_phids = mpull($projects, 'getPHID');
 
     $member_type = PhabricatorProjectProjectHasMemberEdgeType::EDGECONST;
     $watcher_type = PhabricatorObjectHasWatcherEdgeType::EDGECONST;
@@ -189,8 +194,18 @@ final class PhabricatorProjectQuery
       $types[] = $watcher_type;
     }
 
+    $all_sources = array();
+    foreach ($projects as $project) {
+      if ($project->isMilestone()) {
+        $phid = $project->getParentProjectPHID();
+      } else {
+        $phid = $project->getPHID();
+      }
+      $all_sources[$phid] = $phid;
+    }
+
     $edge_query = id(new PhabricatorEdgeQuery())
-      ->withSourcePHIDs($project_phids)
+      ->withSourcePHIDs($all_sources)
       ->withEdgeTypes($types);
 
     // If we only need to know if the viewer is a member, we can restrict
@@ -205,8 +220,14 @@ final class PhabricatorProjectQuery
     foreach ($projects as $project) {
       $project_phid = $project->getPHID();
 
+      if ($project->isMilestone()) {
+        $source_phids = array($project->getParentProjectPHID());
+      } else {
+        $source_phids = array($project_phid);
+      }
+
       $member_phids = $edge_query->getDestinationPHIDs(
-        array($project_phid),
+        $source_phids,
         array($member_type));
 
       if (in_array($viewer_phid, $member_phids)) {
@@ -219,7 +240,7 @@ final class PhabricatorProjectQuery
 
       if ($this->needWatchers) {
         $watcher_phids = $edge_query->getDestinationPHIDs(
-          array($project_phid),
+          $source_phids,
           array($watcher_type));
         $project->attachWatcherPHIDs($watcher_phids);
         $project->setIsUserWatcher(
@@ -408,6 +429,13 @@ final class PhabricatorProjectQuery
       }
     }
 
+    if ($this->hasSubprojects !== null) {
+      $where[] = qsprintf(
+        $conn,
+        'hasSubprojects = %d',
+        (int)$this->hasSubprojects);
+    }
+
     if ($this->minDepth !== null) {
       $where[] = qsprintf(
         $conn,

src/applications/project/storage/PhabricatorProjectTransaction.php

@@ -11,6 +11,7 @@ final class PhabricatorProjectTransaction
   const TYPE_COLOR      = 'project:color';
   const TYPE_LOCKED     = 'project:locked';
   const TYPE_PARENT = 'project:parent';
+  const TYPE_MILESTONE = 'project:milestone';
 
   // NOTE: This is deprecated, members are just a normal edge now.
   const TYPE_MEMBERS    = 'project:members';