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 @@ + 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 @@ +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 @@ + 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.