Page MenuHomePhabricator

D17424.id41893.diff
No OneTemporary

D17424.id41893.diff

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -3163,6 +3163,7 @@
'PhabricatorOwnersPackage' => 'applications/owners/storage/PhabricatorOwnersPackage.php',
'PhabricatorOwnersPackageAuditingTransaction' => 'applications/owners/xaction/PhabricatorOwnersPackageAuditingTransaction.php',
'PhabricatorOwnersPackageAutoreviewTransaction' => 'applications/owners/xaction/PhabricatorOwnersPackageAutoreviewTransaction.php',
+ 'PhabricatorOwnersPackageContextFreeGrammar' => 'applications/owners/lipsum/PhabricatorOwnersPackageContextFreeGrammar.php',
'PhabricatorOwnersPackageDatasource' => 'applications/owners/typeahead/PhabricatorOwnersPackageDatasource.php',
'PhabricatorOwnersPackageDescriptionTransaction' => 'applications/owners/xaction/PhabricatorOwnersPackageDescriptionTransaction.php',
'PhabricatorOwnersPackageDominionTransaction' => 'applications/owners/xaction/PhabricatorOwnersPackageDominionTransaction.php',
@@ -3181,11 +3182,13 @@
'PhabricatorOwnersPackageSearchEngine' => 'applications/owners/query/PhabricatorOwnersPackageSearchEngine.php',
'PhabricatorOwnersPackageStatusTransaction' => 'applications/owners/xaction/PhabricatorOwnersPackageStatusTransaction.php',
'PhabricatorOwnersPackageTestCase' => 'applications/owners/storage/__tests__/PhabricatorOwnersPackageTestCase.php',
+ 'PhabricatorOwnersPackageTestDataGenerator' => 'applications/owners/lipsum/PhabricatorOwnersPackageTestDataGenerator.php',
'PhabricatorOwnersPackageTransaction' => 'applications/owners/storage/PhabricatorOwnersPackageTransaction.php',
'PhabricatorOwnersPackageTransactionEditor' => 'applications/owners/editor/PhabricatorOwnersPackageTransactionEditor.php',
'PhabricatorOwnersPackageTransactionQuery' => 'applications/owners/query/PhabricatorOwnersPackageTransactionQuery.php',
'PhabricatorOwnersPackageTransactionType' => 'applications/owners/xaction/PhabricatorOwnersPackageTransactionType.php',
'PhabricatorOwnersPath' => 'applications/owners/storage/PhabricatorOwnersPath.php',
+ 'PhabricatorOwnersPathContextFreeGrammar' => 'applications/owners/lipsum/PhabricatorOwnersPathContextFreeGrammar.php',
'PhabricatorOwnersPathsController' => 'applications/owners/controller/PhabricatorOwnersPathsController.php',
'PhabricatorOwnersPathsSearchEngineAttachment' => 'applications/owners/engineextension/PhabricatorOwnersPathsSearchEngineAttachment.php',
'PhabricatorOwnersSchemaSpec' => 'applications/owners/storage/PhabricatorOwnersSchemaSpec.php',
@@ -8285,6 +8288,7 @@
),
'PhabricatorOwnersPackageAuditingTransaction' => 'PhabricatorOwnersPackageTransactionType',
'PhabricatorOwnersPackageAutoreviewTransaction' => 'PhabricatorOwnersPackageTransactionType',
+ 'PhabricatorOwnersPackageContextFreeGrammar' => 'PhutilContextFreeGrammar',
'PhabricatorOwnersPackageDatasource' => 'PhabricatorTypeaheadDatasource',
'PhabricatorOwnersPackageDescriptionTransaction' => 'PhabricatorOwnersPackageTransactionType',
'PhabricatorOwnersPackageDominionTransaction' => 'PhabricatorOwnersPackageTransactionType',
@@ -8303,11 +8307,13 @@
'PhabricatorOwnersPackageSearchEngine' => 'PhabricatorApplicationSearchEngine',
'PhabricatorOwnersPackageStatusTransaction' => 'PhabricatorOwnersPackageTransactionType',
'PhabricatorOwnersPackageTestCase' => 'PhabricatorTestCase',
+ 'PhabricatorOwnersPackageTestDataGenerator' => 'PhabricatorTestDataGenerator',
'PhabricatorOwnersPackageTransaction' => 'PhabricatorModularTransaction',
'PhabricatorOwnersPackageTransactionEditor' => 'PhabricatorApplicationTransactionEditor',
'PhabricatorOwnersPackageTransactionQuery' => 'PhabricatorApplicationTransactionQuery',
'PhabricatorOwnersPackageTransactionType' => 'PhabricatorModularTransactionType',
'PhabricatorOwnersPath' => 'PhabricatorOwnersDAO',
+ 'PhabricatorOwnersPathContextFreeGrammar' => 'PhutilContextFreeGrammar',
'PhabricatorOwnersPathsController' => 'PhabricatorOwnersController',
'PhabricatorOwnersPathsSearchEngineAttachment' => 'PhabricatorSearchEngineAttachment',
'PhabricatorOwnersSchemaSpec' => 'PhabricatorConfigSchemaSpec',
diff --git a/src/applications/owners/lipsum/PhabricatorOwnersPackageContextFreeGrammar.php b/src/applications/owners/lipsum/PhabricatorOwnersPackageContextFreeGrammar.php
new file mode 100644
--- /dev/null
+++ b/src/applications/owners/lipsum/PhabricatorOwnersPackageContextFreeGrammar.php
@@ -0,0 +1,60 @@
+<?php
+
+final class PhabricatorOwnersPackageContextFreeGrammar
+ extends PhutilContextFreeGrammar {
+
+ protected function getRules() {
+ return array(
+ 'start' => array(
+ '[package]',
+ ),
+ 'package' => array(
+ '[adjective] [noun]',
+ '[adjective] [noun]',
+ '[adjective] [noun]',
+ '[adjective] [noun]',
+ '[adjective] [adjective] [noun]',
+ '[adjective] [noun] [noun]',
+ '[adjective] [adjective] [noun] [noun]',
+ ),
+ 'adjective' => array(
+ 'Temporary',
+ 'Backend',
+ 'External',
+ 'Emergency',
+ 'Applied',
+ 'Advanced',
+ 'Experimental',
+ 'Logging',
+ 'Test',
+ 'Network',
+ 'Ephemeral',
+ 'Clustered',
+ 'Mining',
+ 'Core',
+ 'Remote',
+ ),
+ 'noun' => array(
+ 'Support',
+ 'Services',
+ 'Infrastructure',
+ 'Mail',
+ 'Security',
+ 'Application',
+ 'Microservices',
+ 'Monoservices',
+ 'Megaservices',
+ 'API',
+ 'Storage',
+ 'Records',
+ 'Package',
+ 'Directories',
+ 'Library',
+ 'Concern',
+ 'Cluster',
+ 'Engine',
+ ),
+ );
+ }
+
+}
diff --git a/src/applications/owners/lipsum/PhabricatorOwnersPackageTestDataGenerator.php b/src/applications/owners/lipsum/PhabricatorOwnersPackageTestDataGenerator.php
new file mode 100644
--- /dev/null
+++ b/src/applications/owners/lipsum/PhabricatorOwnersPackageTestDataGenerator.php
@@ -0,0 +1,89 @@
+<?php
+
+final class PhabricatorOwnersPackageTestDataGenerator
+ extends PhabricatorTestDataGenerator {
+
+ const GENERATORKEY = 'owners';
+
+ public function getGeneratorName() {
+ return pht('Owners Packages');
+ }
+
+ public function generateObject() {
+ $author = $this->loadRandomUser();
+
+ $name = id(new PhabricatorOwnersPackageContextFreeGrammar())
+ ->generate();
+
+ switch ($this->roll(1, 4)) {
+ case 1:
+ case 2:
+ // Most packages own only one path.
+ $path_count = 1;
+ break;
+ case 3:
+ // Some packages own a few paths.
+ $path_count = mt_rand(1, 4);
+ break;
+ case 4:
+ // Some packages own a very large number of paths.
+ $path_count = mt_rand(1, 1024);
+ break;
+ }
+
+ $xactions = array();
+
+ $xactions[] = array(
+ 'type' => 'name',
+ 'value' => $name,
+ );
+
+ $xactions[] = array(
+ 'type' => 'owners',
+ 'value' => array($author->getPHID()),
+ );
+
+ $dominion = PhabricatorOwnersPackage::getDominionOptionsMap();
+ $dominion = array_rand($dominion);
+ $xactions[] = array(
+ 'type' => 'dominion',
+ 'value' => $dominion,
+ );
+
+ $paths = id(new PhabricatorOwnersPathContextFreeGrammar())
+ ->generateSeveral($path_count, "\n");
+ $paths = explode("\n", $paths);
+ $paths = array_unique($paths);
+
+ $repository_phid = $this->loadOneRandom('PhabricatorRepository')
+ ->getPHID();
+
+ $paths_value = array();
+ foreach ($paths as $path) {
+ $paths_value[] = array(
+ 'repositoryPHID' => $repository_phid,
+ 'path' => $path,
+
+ // Excluded paths are relatively rare.
+ 'excluded' => (mt_rand(1, 10) == 1),
+ );
+ }
+
+ $xactions[] = array(
+ 'type' => 'paths.set',
+ 'value' => $paths_value,
+ );
+
+ $params = array(
+ 'transactions' => $xactions,
+ );
+
+ $result = id(new ConduitCall('owners.edit', $params))
+ ->setUser($author)
+ ->execute();
+
+ return $result['object']['phid'];
+ }
+
+
+}
diff --git a/src/applications/owners/lipsum/PhabricatorOwnersPathContextFreeGrammar.php b/src/applications/owners/lipsum/PhabricatorOwnersPathContextFreeGrammar.php
new file mode 100644
--- /dev/null
+++ b/src/applications/owners/lipsum/PhabricatorOwnersPathContextFreeGrammar.php
@@ -0,0 +1,48 @@
+<?php
+
+final class PhabricatorOwnersPathContextFreeGrammar
+ extends PhutilContextFreeGrammar {
+
+ protected function getRules() {
+ return array(
+ 'start' => array(
+ '[path]',
+ ),
+ 'path' => array(
+ '/',
+ '/[directories]',
+ ),
+ 'directories' => array(
+ '[directory-name]',
+ '[directories][directory-name]',
+ ),
+ 'directory-name' => array(
+ '[directory-part]/',
+ ),
+ 'directory-part' => array(
+ 'src',
+ 'doc',
+ 'bin',
+ 'tmp',
+ 'log',
+ 'bak',
+ 'applications',
+ 'var',
+ 'home',
+ 'user',
+ 'lib',
+ 'tests',
+ 'webroot',
+ 'externals',
+ 'third-party',
+ 'libraries',
+ 'config',
+ 'media',
+ 'resources',
+ 'support',
+ 'scripts',
+ ),
+ );
+ }
+
+}
diff --git a/src/applications/owners/query/PhabricatorOwnersPackageQuery.php b/src/applications/owners/query/PhabricatorOwnersPackageQuery.php
--- a/src/applications/owners/query/PhabricatorOwnersPackageQuery.php
+++ b/src/applications/owners/query/PhabricatorOwnersPackageQuery.php
@@ -353,6 +353,9 @@
$packages = $this->controlResults;
$weak_dominion = PhabricatorOwnersPackage::DOMINION_WEAK;
+ $path_fragments = PhabricatorOwnersPackage::splitPath($path);
+ $fragment_count = count($path_fragments);
+
$matches = array();
foreach ($packages as $package_id => $package) {
$best_match = null;
@@ -365,13 +368,11 @@
continue;
}
- foreach ($package->getPaths() as $package_path) {
- if ($package_path->getRepositoryPHID() != $repository_phid) {
- // If this path is for some other repository, skip it.
- continue;
- }
-
- $strength = $package_path->getPathMatchStrength($path);
+ $repository_paths = $package->getPathsForRepository($repository_phid);
+ foreach ($repository_paths as $package_path) {
+ $strength = $package_path->getPathMatchStrength(
+ $path_fragments,
+ $fragment_count);
if ($strength > $best_match) {
$best_match = $strength;
$include = !$package_path->getExcluded();
diff --git a/src/applications/owners/storage/PhabricatorOwnersPackage.php b/src/applications/owners/storage/PhabricatorOwnersPackage.php
--- a/src/applications/owners/storage/PhabricatorOwnersPackage.php
+++ b/src/applications/owners/storage/PhabricatorOwnersPackage.php
@@ -26,6 +26,7 @@
private $paths = self::ATTACHABLE;
private $owners = self::ATTACHABLE;
private $customFields = self::ATTACHABLE;
+ private $pathRepositoryMap = array();
const STATUS_ACTIVE = 'active';
const STATUS_ARCHIVED = 'archived';
@@ -369,6 +370,10 @@
public function attachPaths(array $paths) {
assert_instances_of($paths, 'PhabricatorOwnersPath');
$this->paths = $paths;
+
+ // Drop this cache if we're attaching new paths.
+ $this->pathRepositoryMap = array();
+
return $this;
}
@@ -376,6 +381,23 @@
return $this->assertAttached($this->paths);
}
+ public function getPathsForRepository($repository_phid) {
+ if (isset($this->pathRepositoryMap[$repository_phid])) {
+ return $this->pathRepositoryMap[$repository_phid];
+ }
+
+ $map = array();
+ foreach ($this->getPaths() as $path) {
+ if ($path->getRepositoryPHID() == $repository_phid) {
+ $map[] = $path;
+ }
+ }
+
+ $this->pathRepositoryMap[$repository_phid] = $map;
+
+ return $this->pathRepositoryMap[$repository_phid];
+ }
+
public function attachOwners(array $owners) {
assert_instances_of($owners, 'PhabricatorOwnersOwner');
$this->owners = $owners;
diff --git a/src/applications/owners/storage/PhabricatorOwnersPath.php b/src/applications/owners/storage/PhabricatorOwnersPath.php
--- a/src/applications/owners/storage/PhabricatorOwnersPath.php
+++ b/src/applications/owners/storage/PhabricatorOwnersPath.php
@@ -7,6 +7,9 @@
protected $path;
protected $excluded;
+ private $fragments;
+ private $fragmentCount;
+
protected function getConfiguration() {
return array(
self::CONFIG_TIMESTAMPS => false,
@@ -74,19 +77,21 @@
* Get the number of directory matches between this path specification and
* some real path.
*/
- public function getPathMatchStrength($path) {
- $this_path = $this->getPath();
+ public function getPathMatchStrength($path_fragments, $path_count) {
+ $this_path = $this->path;
if ($this_path === '/') {
// The root path "/" just matches everything with strength 1.
return 1;
}
- $self_fragments = PhabricatorOwnersPackage::splitPath($this_path);
- $path_fragments = PhabricatorOwnersPackage::splitPath($path);
+ if ($this->fragments === null) {
+ $this->fragments = PhabricatorOwnersPackage::splitPath($this_path);
+ $this->fragmentCount = count($this->fragments);
+ }
- $self_count = count($self_fragments);
- $path_count = count($path_fragments);
+ $self_fragments = $this->fragments;
+ $self_count = $this->fragmentCount;
if ($self_count > $path_count) {
// If this path is longer (and therefore more specific) than the target
// path, we don't match it at all.

File Metadata

Mime Type
text/plain
Expires
Thu, Mar 20, 5:46 AM (4 d, 2 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7323912
Default Alt Text
D17424.id41893.diff (13 KB)

Event Timeline