Skip to content

Commit 373ff7f

Browse files
author
epriestley
committedDec 27, 2015
Read materialized project members instead of real members
Summary: Ref T10010. This will allow us to find superprojects with `withMemberPHIDs(...)` queries. - Copy all the current real member edges to materialized member edges. - Redirect all reads to look at materialized members. - This table is already kept in sync by earlier work with indexing. Basically, flow is: - Writes (joining, leaving, adding/removing members) write to the real member edge type. - After a project's members change, they're copied to the materialized member edge type for that project and all of its superprojects. - Reads look at materialized members, so "Parent" sees the members of "Child" and "Grandchild" as its own members, but we still have the "real members" edge type to keep track of "natural" or "direct" members. Test Plan: - Ran migration. - Ran unit tests. - Saw the same projects as projects I was a member of. - Added some `var_dump()` stuff to verify the Owners changed. - Used `grep` to look for other readers of this edge type. - Made some project updates. Reviewers: chad Reviewed By: chad Maniphest Tasks: T10010 Differential Revision: https://secure.phabricator.com/D14893
1 parent 77897ce commit 373ff7f

File tree

4 files changed

+36
-26
lines changed

4 files changed

+36
-26
lines changed
 
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
/* PhabricatorProjectProjectHasMemberEdgeType::EDGECONST = 13 */
2+
/* PhabricatorProjectMaterializedMemberEdgeType::EDGECONST = 60 */
3+
4+
INSERT IGNORE INTO {$NAMESPACE}_project.edge (src, type, dst, dateCreated)
5+
SELECT src, 60, dst, dateCreated FROM {$NAMESPACE}_project.edge
6+
WHERE type = 13;

‎src/applications/owners/storage/PhabricatorOwnersOwner.php

+13-13
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ public static function loadAffiliatedUserPHIDs(array $package_ids) {
4040
if (!$package_ids) {
4141
return array();
4242
}
43+
4344
$owners = id(new PhabricatorOwnersOwner())->loadAllWhere(
4445
'packageID IN (%Ls)',
4546
$package_ids);
@@ -50,20 +51,19 @@ public static function loadAffiliatedUserPHIDs(array $package_ids) {
5051
PhabricatorPeopleUserPHIDType::TYPECONST,
5152
array());
5253

53-
$users_in_project_phids = array();
54-
$project_phids = idx(
55-
$all_phids,
56-
PhabricatorProjectProjectPHIDType::TYPECONST);
57-
if ($project_phids) {
58-
$query = id(new PhabricatorEdgeQuery())
59-
->withSourcePHIDs($project_phids)
60-
->withEdgeTypes(array(
61-
PhabricatorProjectProjectHasMemberEdgeType::EDGECONST,
62-
));
63-
$query->execute();
64-
$users_in_project_phids = $query->getDestinationPHIDs();
54+
if ($user_phids) {
55+
$projects = id(new PhabricatorProjectQuery())
56+
->setViewer(PhabricatorUser::getOmnipotentUser())
57+
->withMemberPHIDs($user_phids)
58+
->withIsMilestone(false)
59+
->execute();
60+
$project_phids = mpull($projects, 'getPHID');
61+
} else {
62+
$project_phids = array();
6563
}
6664

67-
return array_unique(array_merge($users_in_project_phids, $user_phids));
65+
$all_phids = array_fuse($user_phids) + array_fuse($project_phids);
66+
67+
return array_values($all_phids);
6868
}
6969
}

‎src/applications/project/editor/PhabricatorProjectTransactionEditor.php

+13-9
Original file line numberDiff line numberDiff line change
@@ -509,12 +509,13 @@ protected function requireCapabilities(
509509
}
510510

511511
protected function willPublish(PhabricatorLiskDAO $object, array $xactions) {
512-
$member_phids = PhabricatorEdgeQuery::loadDestinationPHIDs(
513-
$object->getPHID(),
514-
PhabricatorProjectProjectHasMemberEdgeType::EDGECONST);
515-
$object->attachMemberPHIDs($member_phids);
516-
517-
return $object;
512+
// NOTE: We're using the omnipotent user here because the original actor
513+
// may no longer have permission to view the object.
514+
return id(new PhabricatorProjectQuery())
515+
->setViewer(PhabricatorUser::getOmnipotentUser())
516+
->withPHIDs(array($object->getPHID()))
517+
->needMembers(true)
518+
->executeOne();
518519
}
519520

520521
protected function shouldSendMail(
@@ -719,9 +720,12 @@ protected function adjustObjectForPolicyChecks(
719720
$object_phid = $object->getPHID();
720721

721722
if ($object_phid) {
722-
$members = PhabricatorEdgeQuery::loadDestinationPHIDs(
723-
$object_phid,
724-
PhabricatorProjectProjectHasMemberEdgeType::EDGECONST);
723+
$project = id(new PhabricatorProjectQuery())
724+
->setViewer($this->getActor())
725+
->withPHIDs(array($object_phid))
726+
->needMembers(true)
727+
->executeOne();
728+
$members = $project->getMemberPHIDs();
725729
} else {
726730
$members = array();
727731
}

‎src/applications/project/query/PhabricatorProjectQuery.php

+4-4
Original file line numberDiff line numberDiff line change
@@ -201,11 +201,11 @@ protected function willFilterPage(array $projects) {
201201

202202
$viewer_phid = $this->getViewer()->getPHID();
203203

204-
$member_type = PhabricatorProjectProjectHasMemberEdgeType::EDGECONST;
204+
$material_type = PhabricatorProjectMaterializedMemberEdgeType::EDGECONST;
205205
$watcher_type = PhabricatorObjectHasWatcherEdgeType::EDGECONST;
206206

207207
$types = array();
208-
$types[] = $member_type;
208+
$types[] = $material_type;
209209
if ($this->needWatchers) {
210210
$types[] = $watcher_type;
211211
}
@@ -255,7 +255,7 @@ protected function willFilterPage(array $projects) {
255255
if ($any_edges) {
256256
$member_phids = $edge_query->getDestinationPHIDs(
257257
$source_phids,
258-
array($member_type));
258+
array($material_type));
259259
} else {
260260
$member_phids = array();
261261
}
@@ -488,7 +488,7 @@ protected function buildJoinClauseParts(AphrontDatabaseConnection $conn) {
488488
$conn,
489489
'JOIN %T e ON e.src = p.phid AND e.type = %d',
490490
PhabricatorEdgeConfig::TABLE_NAME_EDGE,
491-
PhabricatorProjectProjectHasMemberEdgeType::EDGECONST);
491+
PhabricatorProjectMaterializedMemberEdgeType::EDGECONST);
492492
}
493493

494494
if ($this->slugs !== null) {

0 commit comments

Comments
 (0)
Failed to load comments.