From 6baeda8aade33697580b720d5484a68e45b75f0a Mon Sep 17 00:00:00 2001 From: tuomaspelkonen Date: Wed, 20 Apr 2011 15:29:02 -0700 Subject: [PATCH 1/4] Added a conduit method for getting all the diffs for a revision. Summary: Loading all diffs for a differential revision is needed by at least perflab. Test Plan: Created a simple script that queried the conduit and made sure that it returned correct values. Reviewed By: jungejason Reviewers: jungejason CC: epriestley, jungejason Differential Revision: 155 --- src/__phutil_library_map__.php | 2 + ...uitAPI_differential_getalldiffs_Method.php | 60 +++++++++++++++++++ .../differential/getalldiffs/__init__.php | 15 +++++ 3 files changed, 77 insertions(+) create mode 100644 src/applications/conduit/method/differential/getalldiffs/ConduitAPI_differential_getalldiffs_Method.php create mode 100644 src/applications/conduit/method/differential/getalldiffs/__init__.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 8e3bd588ce..0313f14bac 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -78,6 +78,7 @@ phutil_register_library_map(array( 'ConduitAPI_differential_creatediff_Method' => 'applications/conduit/method/differential/creatediff', 'ConduitAPI_differential_createrevision_Method' => 'applications/conduit/method/differential/createrevision', 'ConduitAPI_differential_find_Method' => 'applications/conduit/method/differential/find', + 'ConduitAPI_differential_getalldiffs_Method' => 'applications/conduit/method/differential/getalldiffs', 'ConduitAPI_differential_getcommitmessage_Method' => 'applications/conduit/method/differential/getcommitmessage', 'ConduitAPI_differential_getcommitpaths_Method' => 'applications/conduit/method/differential/getcommitpaths', 'ConduitAPI_differential_getdiff_Method' => 'applications/conduit/method/differential/getdiff', @@ -525,6 +526,7 @@ phutil_register_library_map(array( 'ConduitAPI_differential_creatediff_Method' => 'ConduitAPIMethod', 'ConduitAPI_differential_createrevision_Method' => 'ConduitAPIMethod', 'ConduitAPI_differential_find_Method' => 'ConduitAPIMethod', + 'ConduitAPI_differential_getalldiffs_Method' => 'ConduitAPIMethod', 'ConduitAPI_differential_getcommitmessage_Method' => 'ConduitAPIMethod', 'ConduitAPI_differential_getcommitpaths_Method' => 'ConduitAPIMethod', 'ConduitAPI_differential_getdiff_Method' => 'ConduitAPIMethod', diff --git a/src/applications/conduit/method/differential/getalldiffs/ConduitAPI_differential_getalldiffs_Method.php b/src/applications/conduit/method/differential/getalldiffs/ConduitAPI_differential_getalldiffs_Method.php new file mode 100644 index 0000000000..93eb01d22b --- /dev/null +++ b/src/applications/conduit/method/differential/getalldiffs/ConduitAPI_differential_getalldiffs_Method.php @@ -0,0 +1,60 @@ + 'required list', + ); + } + + public function defineReturnType() { + return 'dict'; + } + + public function defineErrorTypes() { + return array(); + } + + protected function execute(ConduitAPIRequest $request) { + $results = array(); + $revision_ids = $request->getValue('revision_ids'); + + if (!$revision_ids) { + return $results; + } + + $diffs = id(new DifferentialDiff())->loadAllWhere( + 'revisionID IN (%Ld)', + $revision_ids); + + foreach ($diffs as $diff) { + $results[] = array( + 'revision_id' => $diff->getRevisionID(), + 'diff_id' => $diff->getID(), + ); + } + + return $results; + } +} diff --git a/src/applications/conduit/method/differential/getalldiffs/__init__.php b/src/applications/conduit/method/differential/getalldiffs/__init__.php new file mode 100644 index 0000000000..63152910d7 --- /dev/null +++ b/src/applications/conduit/method/differential/getalldiffs/__init__.php @@ -0,0 +1,15 @@ + Date: Tue, 19 Apr 2011 20:03:55 -0700 Subject: [PATCH 2/4] Fixed image macro with '-' in the name. Summary: Fixed the image macro regex not to use '-' as the separator. Also minor improvement to randomon. Test Plan: Tried different image marcors. Reviewed By: jungejason Reviewers: jungejason CC: epriestley, jungejason Differential Revision: 153 --- .../imagemacro/PhabricatorRemarkupRuleImageMacro.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/infrastructure/markup/remarkup/markuprule/imagemacro/PhabricatorRemarkupRuleImageMacro.php b/src/infrastructure/markup/remarkup/markuprule/imagemacro/PhabricatorRemarkupRuleImageMacro.php index 13a887fe44..97137c35bf 100644 --- a/src/infrastructure/markup/remarkup/markuprule/imagemacro/PhabricatorRemarkupRuleImageMacro.php +++ b/src/infrastructure/markup/remarkup/markuprule/imagemacro/PhabricatorRemarkupRuleImageMacro.php @@ -32,13 +32,12 @@ class PhabricatorRemarkupRuleImageMacro $this->images[$row->getName()] = $row->getFilePHID(); } $this->images[self::RANDOM_IMAGE_NAME] = ''; + $this->hash = 0; } public function apply($text) { - $this->hash = 0; - return preg_replace_callback( - '@\b([a-zA-Z0-9_\-]+)\b@U', + '@\b([a-zA-Z0-9_\-]+)\b@', array($this, 'markupImageMacro'), $text); } From bff6aef87a08d4d64dd4c1de2d16b431348a0318 Mon Sep 17 00:00:00 2001 From: leon Date: Mon, 11 Apr 2011 21:51:36 -0700 Subject: [PATCH 3/4] Adding conduit API method to find owner for the affected file Summary: Adding method that given a path will go up the folder hierarchy until it finds the owning package and return owners for that package. Task ID: #403724 Test Plan: Tried the new API call through console on various path combinations Reviewed By: epriestley Reviewers: epriestley, dpepper, tuomaspelkonen CC: epriestley, Leon Revert Plan: n/a Tags: bootcamp, Push Efficiency - begin *PUBLIC* platform impact section - Bugzilla: # - end platform impact - Differential Revision: 126 --- src/__phutil_library_map__.php | 2 + .../ConduitAPI_path_getowners_Method.php | 84 +++++++++++++++++ .../method/path/getowners/__init__.php | 19 ++++ .../package/PhabricatorOwnersPackage.php | 92 +++++++++++++++---- .../owners/storage/package/__init__.php | 1 + 5 files changed, 179 insertions(+), 19 deletions(-) create mode 100644 src/applications/conduit/method/path/getowners/ConduitAPI_path_getowners_Method.php create mode 100644 src/applications/conduit/method/path/getowners/__init__.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 0313f14bac..e7be13bcc8 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -89,6 +89,7 @@ phutil_register_library_map(array( 'ConduitAPI_differential_updaterevision_Method' => 'applications/conduit/method/differential/updaterevision', 'ConduitAPI_diffusion_getcommits_Method' => 'applications/conduit/method/diffusion/getcommits', 'ConduitAPI_file_upload_Method' => 'applications/conduit/method/file/upload', + 'ConduitAPI_path_getowners_Method' => 'applications/conduit/method/path/getowners', 'ConduitAPI_user_find_Method' => 'applications/conduit/method/user/find', 'ConduitAPI_user_whoami_Method' => 'applications/conduit/method/user/whoami', 'ConduitException' => 'applications/conduit/protocol/exception', @@ -537,6 +538,7 @@ phutil_register_library_map(array( 'ConduitAPI_differential_updaterevision_Method' => 'ConduitAPIMethod', 'ConduitAPI_diffusion_getcommits_Method' => 'ConduitAPIMethod', 'ConduitAPI_file_upload_Method' => 'ConduitAPIMethod', + 'ConduitAPI_path_getowners_Method' => 'ConduitAPIMethod', 'ConduitAPI_user_find_Method' => 'ConduitAPIMethod', 'ConduitAPI_user_whoami_Method' => 'ConduitAPIMethod', 'DarkConsoleConfigPlugin' => 'DarkConsolePlugin', diff --git a/src/applications/conduit/method/path/getowners/ConduitAPI_path_getowners_Method.php b/src/applications/conduit/method/path/getowners/ConduitAPI_path_getowners_Method.php new file mode 100644 index 0000000000..50f34cc211 --- /dev/null +++ b/src/applications/conduit/method/path/getowners/ConduitAPI_path_getowners_Method.php @@ -0,0 +1,84 @@ + 'required nonempty string', + 'path' => 'required nonempty string' + ); + } + + public function defineReturnType() { + return 'array of packages containing phid, primary_owner (phid=>username),'. + 'owners(array of phid=>username)'; + } + + public function defineErrorTypes() { + return array( + 'ERR_REP_NOT_FOUND' => 'The repository callsign is not recognized', + 'ERR_PATH_NOT_FOUND' => 'The specified path is not known to any package', + ); + } + + protected function execute(ConduitAPIRequest $request) { + + $repository = id(new PhabricatorRepository())->loadOneWhere('callsign = %s', + $request->getValue('repositoryCallsign')); + + if (empty($repository)) { + throw new ConduitException('ERR_REP_NOT_FOUND'); + } + + $packages = PhabricatorOwnersPackage::loadOwningPackages( + $repository, $request->getValue('path')); + if (empty($packages)) { + throw new ConduitException('ERR_PATH_NOT_FOUND'); + } + + $owner = new PhabricatorOwnersOwner(); + $user = new PhabricatorUser(); + $result = array(); + foreach ($packages as $package) { + $p_result = array(); + $p_result['phid'] = $package->getID(); + $primary_owner_phid = $package->getPrimaryOwnerPHID(); + if (!empty($primary_owner_phid)) { + $p_user = $user->loadOneWhere('phid = %s', + $primary_owner_phid); + $p_result['primaryOwner'] = $p_user->getPhid(); + } + + $p_owners = $owner->loadAllForPackages(array($package)); + $p_users = $user->loadAllWhere('phid IN (%Ls)', + mpull($p_owners, 'getUserPHID')); + + $p_result['owners'] = array_values(mpull($p_users, 'getPhid')); + + $result[] = $p_result; + } + + return $result; + } + +} diff --git a/src/applications/conduit/method/path/getowners/__init__.php b/src/applications/conduit/method/path/getowners/__init__.php new file mode 100644 index 0000000000..0ca5823ef3 --- /dev/null +++ b/src/applications/conduit/method/path/getowners/__init__.php @@ -0,0 +1,19 @@ +establishConnection('r'); + + $repository_clause = qsprintf($conn, 'AND p.repositoryPHID = %s', + $repository->getPHID()); + + $limit_clause = ''; + if (!empty($limit)) { + $limit_clause = qsprintf($conn, 'LIMIT %d', $limit); + } + + $data = queryfx_all( + $conn, + 'SELECT pkg.id FROM %T pkg JOIN %T p ON p.packageID = pkg.id + WHERE p.path IN (%Ls) %Q ORDER BY LENGTH(p.path) DESC %Q', + $package->getTableName(), + $path->getTableName(), + $paths, + $repository_clause, + $limit_clause); + + $ids = ipull($data, 'id'); + + if (empty($ids)) { + return array(); + } + + $order = array(); + foreach ($ids as $id) { + if (empty($order[$id])) { + $order[$id] = true; } } - $package = new PhabricatorOwnersPackage(); - $path = new PhabricatorOwnersPath(); - $data = queryfx_all( - $package->establishConnection('r'), - 'SELECT pkg.* FROM %T pkg JOIN %T p ON p.packageID = pkg.id - WHERE p.repositoryPHID = %s - AND p.path IN (%Ls)', - $package->getTableName(), - $path->getTableName(), - $repository->getPHID(), - array_keys($fragments)); + $packages = $package->loadAllWhere('id in (%Ld)', array_keys($order)); - return $package->loadAllFromArray($data); + $result = array(); + // Reorder packages according to specificity. + foreach ($packages as $package) { + $result[$package->getID()] = $package; + } + + $result = array_select_keys($result, array_keys($order)); + + return $result; } public function save() { @@ -167,4 +209,16 @@ class PhabricatorOwnersPackage extends PhabricatorOwnersDAO { return parent::delete(); } + private static function splitPath($path) { + $result = array(); + $trailing_slash = preg_match('@/$@', $path) ? '/' : ''; + $path = trim($path, '/'); + $parts = explode('/', $path); + while (count($parts)) { + $result['/'.implode('/', $parts).$trailing_slash] = true; + $trailing_slash = '/'; + array_pop($parts); + } + return $result; + } } diff --git a/src/applications/owners/storage/package/__init__.php b/src/applications/owners/storage/package/__init__.php index ea807df659..43fbb38dab 100644 --- a/src/applications/owners/storage/package/__init__.php +++ b/src/applications/owners/storage/package/__init__.php @@ -10,6 +10,7 @@ phutil_require_module('phabricator', 'applications/owners/storage/base'); phutil_require_module('phabricator', 'applications/owners/storage/owner'); phutil_require_module('phabricator', 'applications/owners/storage/path'); phutil_require_module('phabricator', 'applications/phid/storage/phid'); +phutil_require_module('phabricator', 'storage/qsprintf'); phutil_require_module('phabricator', 'storage/queryfx'); phutil_require_module('phutil', 'utils'); From dcb2e439606c9e3394df95ade649e56917462f9d Mon Sep 17 00:00:00 2001 From: leon Date: Thu, 14 Apr 2011 16:17:39 -0700 Subject: [PATCH 4/4] Removing reordering code that wasn't needed Summary: Remove reordering code for package array as it's ordered in the first place Test Plan: Called API through conduit console, still works Reviewed By: epriestley Reviewers: epriestley, tuomaspelkonen CC: ju, dpepper, epriestley Blame Revision: 126 Differential Revision: 141 --- .../storage/package/PhabricatorOwnersPackage.php | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/applications/owners/storage/package/PhabricatorOwnersPackage.php b/src/applications/owners/storage/package/PhabricatorOwnersPackage.php index 18928ed0e9..4bbd924944 100644 --- a/src/applications/owners/storage/package/PhabricatorOwnersPackage.php +++ b/src/applications/owners/storage/package/PhabricatorOwnersPackage.php @@ -135,15 +135,9 @@ class PhabricatorOwnersPackage extends PhabricatorOwnersDAO { $packages = $package->loadAllWhere('id in (%Ld)', array_keys($order)); - $result = array(); - // Reorder packages according to specificity. - foreach ($packages as $package) { - $result[$package->getID()] = $package; - } + $packages = array_select_keys($packages, array_keys($order)); - $result = array_select_keys($result, array_keys($order)); - - return $result; + return $packages; } public function save() {