From 212b0d29c54f079dee5643c54b1e930690b4a059 Mon Sep 17 00:00:00 2001 From: Joshua Spence Date: Mon, 22 Sep 2014 18:50:20 +1000 Subject: [PATCH] Add an acceptance test for Celerity maps Summary: Fixes T5374. Add an acceptance test to the `PhabricatorInfrastructureTestCase` class which fails if a Celerity map is not up-to-date. In order to achieve this, a lot of code used to generate Celerity maps was transferred from `CelerityManagementMapWorkflow` to `CelerityResourceMap` and `CelerityResourceMapGenerator`. Test Plan: Ran `arc unit` and noticed that all tests passed. Modified a JavaScript file and ran `arc unit` again (without running `./bin/celerity map`)... this time the test failed, as expected. Reviewers: epriestley, #blessed_reviewers Reviewed By: epriestley, #blessed_reviewers Subscribers: epriestley, Korvin Maniphest Tasks: T5374 Differential Revision: https://secure.phabricator.com/D9817 --- src/__phutil_library_map__.php | 3 + src/__tests__/PhabricatorCelerityTestCase.php | 33 ++ .../celerity/CelerityResourceMap.php | 16 + .../celerity/CelerityResourceMapGenerator.php | 361 ++++++++++++++++++ .../CelerityManagementMapWorkflow.php | 325 +--------------- 5 files changed, 418 insertions(+), 320 deletions(-) create mode 100644 src/__tests__/PhabricatorCelerityTestCase.php create mode 100644 src/infrastructure/celerity/CelerityResourceMapGenerator.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 726f77c881..fc4f8c1b35 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -111,6 +111,7 @@ phutil_register_library_map(array( 'CelerityResourceController' => 'infrastructure/celerity/CelerityResourceController.php', 'CelerityResourceGraph' => 'infrastructure/celerity/CelerityResourceGraph.php', 'CelerityResourceMap' => 'infrastructure/celerity/CelerityResourceMap.php', + 'CelerityResourceMapGenerator' => 'infrastructure/celerity/CelerityResourceMapGenerator.php', 'CelerityResourceTransformer' => 'infrastructure/celerity/CelerityResourceTransformer.php', 'CelerityResourceTransformerTestCase' => 'infrastructure/celerity/__tests__/CelerityResourceTransformerTestCase.php', 'CelerityResources' => 'infrastructure/celerity/resources/CelerityResources.php', @@ -1304,6 +1305,7 @@ phutil_register_library_map(array( 'PhabricatorCalendarSchemaSpec' => 'applications/calendar/storage/PhabricatorCalendarSchemaSpec.php', 'PhabricatorCalendarViewController' => 'applications/calendar/controller/PhabricatorCalendarViewController.php', 'PhabricatorCampfireProtocolAdapter' => 'infrastructure/daemon/bot/adapter/PhabricatorCampfireProtocolAdapter.php', + 'PhabricatorCelerityTestCase' => '__tests__/PhabricatorCelerityTestCase.php', 'PhabricatorChangeParserTestCase' => 'applications/repository/worker/__tests__/PhabricatorChangeParserTestCase.php', 'PhabricatorChangesetResponse' => 'infrastructure/diff/PhabricatorChangesetResponse.php', 'PhabricatorChatLogApplication' => 'applications/chatlog/application/PhabricatorChatLogApplication.php', @@ -4216,6 +4218,7 @@ phutil_register_library_map(array( 'PhabricatorCalendarSchemaSpec' => 'PhabricatorConfigSchemaSpec', 'PhabricatorCalendarViewController' => 'PhabricatorCalendarController', 'PhabricatorCampfireProtocolAdapter' => 'PhabricatorBotBaseStreamingProtocolAdapter', + 'PhabricatorCelerityTestCase' => 'PhabricatorTestCase', 'PhabricatorChangeParserTestCase' => 'PhabricatorWorkingCopyTestCase', 'PhabricatorChangesetResponse' => 'AphrontProxyResponse', 'PhabricatorChatLogApplication' => 'PhabricatorApplication', diff --git a/src/__tests__/PhabricatorCelerityTestCase.php b/src/__tests__/PhabricatorCelerityTestCase.php new file mode 100644 index 0000000000..d36ef051c8 --- /dev/null +++ b/src/__tests__/PhabricatorCelerityTestCase.php @@ -0,0 +1,33 @@ +generate(); + + $this->assertEqual( + $new_map->getNameMap(), + $old_map->getNameMap()); + $this->assertEqual( + $new_map->getSymbolMap(), + $old_map->getSymbolMap()); + $this->assertEqual( + $new_map->getRequiresMap(), + $old_map->getRequiresMap()); + $this->assertEqual( + $new_map->getPackageMap(), + $old_map->getPackageMap()); + } + } + +} diff --git a/src/infrastructure/celerity/CelerityResourceMap.php b/src/infrastructure/celerity/CelerityResourceMap.php index 030fd3233d..5b0c374dab 100644 --- a/src/infrastructure/celerity/CelerityResourceMap.php +++ b/src/infrastructure/celerity/CelerityResourceMap.php @@ -53,6 +53,22 @@ final class CelerityResourceMap { return self::$instances[$name]; } + public function getNameMap() { + return $this->nameMap; + } + + public function getSymbolMap() { + return $this->symbolMap; + } + + public function getRequiresMap() { + return $this->requiresMap; + } + + public function getPackageMap() { + return $this->packageMap; + } + public function getPackagedNamesForSymbols(array $symbols) { $resolved = $this->resolveResources($symbols); return $this->packageResources($resolved); diff --git a/src/infrastructure/celerity/CelerityResourceMapGenerator.php b/src/infrastructure/celerity/CelerityResourceMapGenerator.php new file mode 100644 index 0000000000..b7a1dfb563 --- /dev/null +++ b/src/infrastructure/celerity/CelerityResourceMapGenerator.php @@ -0,0 +1,361 @@ +resources = $resources; + } + + public function getNameMap() { + return $this->nameMap; + } + + public function getSymbolMap() { + return $this->symbolMap; + } + + public function getRequiresMap() { + return $this->requiresMap; + } + + public function getPackageMap() { + return $this->packageMap; + } + + public function setDebug($debug) { + $this->debug = $debug; + return $this; + } + + protected function log($message) { + if ($this->debug) { + $console = PhutilConsole::getConsole(); + $console->writeErr("%s\n", $message); + } + } + + public function generate() { + $binary_map = $this->rebuildBinaryResources($this->resources); + + $this->log(pht('Found %d binary resources.', count($binary_map))); + + $xformer = id(new CelerityResourceTransformer()) + ->setMinify(false) + ->setRawURIMap(ipull($binary_map, 'uri')); + + $text_map = $this->rebuildTextResources($this->resources, $xformer); + + $this->log(pht('Found %d text resources.', count($text_map))); + + $resource_graph = array(); + $requires_map = array(); + $symbol_map = array(); + foreach ($text_map as $name => $info) { + if (isset($info['provides'])) { + $symbol_map[$info['provides']] = $info['hash']; + + // We only need to check for cycles and add this to the requires map + // if it actually requires anything. + if (!empty($info['requires'])) { + $resource_graph[$info['provides']] = $info['requires']; + $requires_map[$info['hash']] = $info['requires']; + } + } + } + + $this->detectGraphCycles($resource_graph); + $name_map = ipull($binary_map, 'hash') + ipull($text_map, 'hash'); + $hash_map = array_flip($name_map); + + $package_map = $this->rebuildPackages( + $this->resources, + $symbol_map, + $hash_map); + + $this->log(pht('Found %d packages.', count($package_map))); + + $component_map = array(); + foreach ($package_map as $package_name => $package_info) { + foreach ($package_info['symbols'] as $symbol) { + $component_map[$symbol] = $package_name; + } + } + + $name_map = $this->mergeNameMaps( + array( + array(pht('Binary'), ipull($binary_map, 'hash')), + array(pht('Text'), ipull($text_map, 'hash')), + array(pht('Package'), ipull($package_map, 'hash')), + )); + $package_map = ipull($package_map, 'symbols'); + + ksort($name_map); + ksort($symbol_map); + ksort($requires_map); + ksort($package_map); + + $this->nameMap = $name_map; + $this->symbolMap = $symbol_map; + $this->requiresMap = $requires_map; + $this->packageMap = $package_map; + + return $this; + } + + public function write() { + $map_content = $this->formatMapContent(array( + 'names' => $this->getNameMap(), + 'symbols' => $this->getSymbolMap(), + 'requires' => $this->getRequiresMap(), + 'packages' => $this->getPackageMap(), + )); + + $map_path = $this->resources->getPathToMap(); + $this->log(pht('Writing map "%s".', Filesystem::readablePath($map_path))); + Filesystem::writeFile($map_path, $map_content); + + return $this; + } + + private function formatMapContent(array $data) { + $content = phutil_var_export($data); + $generated = '@'.'generated'; + + return <<> Resource information map. + */ + private function rebuildBinaryResources( + CelerityPhysicalResources $resources) { + + $binary_map = $resources->findBinaryResources(); + $result_map = array(); + + foreach ($binary_map as $name => $data_hash) { + $hash = $resources->getCelerityHash($data_hash.$name); + + $result_map[$name] = array( + 'hash' => $hash, + 'uri' => $resources->getResourceURI($hash, $name), + ); + } + + return $result_map; + } + + /** + * Find text resources (like JS and CSS) and return information about them. + * + * @param CelerityPhysicalResources Resource map to find text resources for. + * @param CelerityResourceTransformer Configured resource transformer. + * @return map> Resource information map. + */ + private function rebuildTextResources( + CelerityPhysicalResources $resources, + CelerityResourceTransformer $xformer) { + + $text_map = $resources->findTextResources(); + $result_map = array(); + + foreach ($text_map as $name => $data_hash) { + $raw_data = $resources->getResourceData($name); + $xformed_data = $xformer->transformResource($name, $raw_data); + + $data_hash = $resources->getCelerityHash($xformed_data); + $hash = $resources->getCelerityHash($data_hash.$name); + + list($provides, $requires) = $this->getProvidesAndRequires( + $name, + $raw_data); + + $result_map[$name] = array( + 'hash' => $hash, + ); + + if ($provides !== null) { + $result_map[$name] += array( + 'provides' => $provides, + 'requires' => $requires, + ); + } + } + + return $result_map; + } + + /** + * Parse the `@provides` and `@requires` symbols out of a text resource, like + * JS or CSS. + * + * @param string Resource name. + * @param string Resource data. + * @return pair|null> The `@provides` symbol and + * the list of `@requires` symbols. If the resource is not part of the + * dependency graph, both are null. + */ + private function getProvidesAndRequires($name, $data) { + $parser = new PhutilDocblockParser(); + + $matches = array(); + $ok = preg_match('@/[*][*].*?[*]/@s', $data, $matches); + if (!$ok) { + throw new Exception( + pht( + 'Resource "%s" does not have a header doc comment. Encode '. + 'dependency data in a header docblock.', + $name)); + } + + list($description, $metadata) = $parser->parse($matches[0]); + + $provides = preg_split('/\s+/', trim(idx($metadata, 'provides'))); + $requires = preg_split('/\s+/', trim(idx($metadata, 'requires'))); + $provides = array_filter($provides); + $requires = array_filter($requires); + + if (!$provides) { + // Tests and documentation-only JS is permitted to @provide no targets. + return array(null, null); + } + + if (count($provides) > 1) { + throw new Exception( + pht('Resource "%s" must @provide at most one Celerity target.', $name)); + } + + return array(head($provides), $requires); + } + + /** + * Check for dependency cycles in the resource graph. Raises an exception if + * a cycle is detected. + * + * @param map> Map of `@provides` symbols to their + * `@requires` symbols. + * @return void + */ + private function detectGraphCycles(array $nodes) { + $graph = id(new CelerityResourceGraph()) + ->addNodes($nodes) + ->setResourceGraph($nodes) + ->loadGraph(); + + foreach ($nodes as $provides => $requires) { + $cycle = $graph->detectCycles($provides); + if ($cycle) { + throw new Exception( + pht('Cycle detected in resource graph: %s', implode(' > ', $cycle))); + } + } + } + + /** + * Build package specifications for a given resource source. + * + * @param CelerityPhysicalResources Resource source to rebuild. + * @param map Map of `@provides` to hashes. + * @param map Map of hashes to resource names. + * @return map> Package information maps. + */ + private function rebuildPackages( + CelerityPhysicalResources $resources, + array $symbol_map, + array $reverse_map) { + + $package_map = array(); + + $package_spec = $resources->getResourcePackages(); + foreach ($package_spec as $package_name => $package_symbols) { + $type = null; + $hashes = array(); + foreach ($package_symbols as $symbol) { + $symbol_hash = idx($symbol_map, $symbol); + if ($symbol_hash === null) { + throw new Exception( + pht( + 'Package specification for "%s" includes "%s", but that symbol '. + 'is not @provided by any resource.', + $package_name, + $symbol)); + } + + $resource_name = $reverse_map[$symbol_hash]; + $resource_type = $resources->getResourceType($resource_name); + if ($type === null) { + $type = $resource_type; + } else if ($type !== $resource_type) { + throw new Exception( + pht( + 'Package specification for "%s" includes resources of multiple '. + 'types (%s, %s). Each package may only contain one type of '. + 'resource.', + $package_name, + $type, + $resource_type)); + } + + $hashes[] = $symbol.':'.$symbol_hash; + } + + $hash = $resources->getCelerityHash(implode("\n", $hashes)); + $package_map[$package_name] = array( + 'hash' => $hash, + 'symbols' => $package_symbols, + ); + } + + return $package_map; + } + + private function mergeNameMaps(array $maps) { + $result = array(); + $origin = array(); + + foreach ($maps as $map) { + list($map_name, $data) = $map; + foreach ($data as $name => $hash) { + if (empty($result[$name])) { + $result[$name] = $hash; + $origin[$name] = $map_name; + } else { + $old = $origin[$name]; + $new = $map_name; + throw new Exception( + pht( + 'Resource source defines two resources with the same name, '. + '"%s". One is defined in the "%s" map; the other in the "%s" '. + 'map. Each resource must have a unique name.', + $name, + $old, + $new)); + } + } + } + return $result; + } + +} diff --git a/src/infrastructure/celerity/management/CelerityManagementMapWorkflow.php b/src/infrastructure/celerity/management/CelerityManagementMapWorkflow.php index 81179c086e..be844137e2 100644 --- a/src/infrastructure/celerity/management/CelerityManagementMapWorkflow.php +++ b/src/infrastructure/celerity/management/CelerityManagementMapWorkflow.php @@ -42,330 +42,15 @@ final class CelerityManagementMapWorkflow $resources->getName(), get_class($resources))); - $binary_map = $this->rebuildBinaryResources($resources); - - $this->log( - pht( - 'Found %d binary resources.', - new PhutilNumber(count($binary_map)))); - - $xformer = id(new CelerityResourceTransformer()) - ->setMinify(false) - ->setRawURIMap(ipull($binary_map, 'uri')); - - $text_map = $this->rebuildTextResources($resources, $xformer); - - $this->log( - pht( - 'Found %d text resources.', - new PhutilNumber(count($text_map)))); - - $resource_graph = array(); - $requires_map = array(); - $symbol_map = array(); - foreach ($text_map as $name => $info) { - if (isset($info['provides'])) { - $symbol_map[$info['provides']] = $info['hash']; - - // We only need to check for cycles and add this to the requires map - // if it actually requires anything. - if (!empty($info['requires'])) { - $resource_graph[$info['provides']] = $info['requires']; - $requires_map[$info['hash']] = $info['requires']; - } - } - } - - $this->detectGraphCycles($resource_graph); - $name_map = ipull($binary_map, 'hash') + ipull($text_map, 'hash'); - $hash_map = array_flip($name_map); - - $package_map = $this->rebuildPackages( - $resources, - $symbol_map, - $hash_map); - - $this->log( - pht( - 'Found %d packages.', - new PhutilNumber(count($package_map)))); - - $component_map = array(); - foreach ($package_map as $package_name => $package_info) { - foreach ($package_info['symbols'] as $symbol) { - $component_map[$symbol] = $package_name; - } - } - - $name_map = $this->mergeNameMaps( - array( - array(pht('Binary'), ipull($binary_map, 'hash')), - array(pht('Text'), ipull($text_map, 'hash')), - array(pht('Package'), ipull($package_map, 'hash')), - )); - $package_map = ipull($package_map, 'symbols'); - - ksort($name_map); - ksort($symbol_map); - ksort($requires_map); - ksort($package_map); - - $map_content = $this->formatMapContent(array( - 'names' => $name_map, - 'symbols' => $symbol_map, - 'requires' => $requires_map, - 'packages' => $package_map, - )); - - $map_path = $resources->getPathToMap(); - $this->log(pht('Writing map "%s".', Filesystem::readablePath($map_path))); - Filesystem::writeFile($map_path, $map_content); + id(new CelerityResourceMapGenerator($resources)) + ->setDebug(true) + ->generate() + ->write(); } - - /** - * Find binary resources (like PNG and SWF) and return information about - * them. - * - * @param CelerityPhysicalResources Resource map to find binary resources for. - * @return map> Resource information map. - */ - private function rebuildBinaryResources( - CelerityPhysicalResources $resources) { - - $binary_map = $resources->findBinaryResources(); - - $result_map = array(); - foreach ($binary_map as $name => $data_hash) { - $hash = $resources->getCelerityHash($data_hash.$name); - - $result_map[$name] = array( - 'hash' => $hash, - 'uri' => $resources->getResourceURI($hash, $name), - ); - } - - return $result_map; - } - - - /** - * Find text resources (like JS and CSS) and return information about them. - * - * @param CelerityPhysicalResources Resource map to find text resources for. - * @param CelerityResourceTransformer Configured resource transformer. - * @return map> Resource information map. - */ - private function rebuildTextResources( - CelerityPhysicalResources $resources, - CelerityResourceTransformer $xformer) { - - $text_map = $resources->findTextResources(); - - $result_map = array(); - foreach ($text_map as $name => $data_hash) { - $raw_data = $resources->getResourceData($name); - $xformed_data = $xformer->transformResource($name, $raw_data); - - $data_hash = $resources->getCelerityHash($xformed_data); - $hash = $resources->getCelerityHash($data_hash.$name); - - list($provides, $requires) = $this->getProvidesAndRequires( - $name, - $raw_data); - - $result_map[$name] = array( - 'hash' => $hash, - ); - - if ($provides !== null) { - $result_map[$name] += array( - 'provides' => $provides, - 'requires' => $requires, - ); - } - } - - return $result_map; - } - - - /** - * Parse the `@provides` and `@requires` symbols out of a text resource, like - * JS or CSS. - * - * @param string Resource name. - * @param string Resource data. - * @return pair|null> The `@provides` symbol and the - * list of `@requires` symbols. If the resource is not part of the - * dependency graph, both are null. - */ - private function getProvidesAndRequires($name, $data) { - $parser = new PhutilDocblockParser(); - - $matches = array(); - $ok = preg_match('@/[*][*].*?[*]/@s', $data, $matches); - if (!$ok) { - throw new Exception( - pht( - 'Resource "%s" does not have a header doc comment. Encode '. - 'dependency data in a header docblock.', - $name)); - } - - list($description, $metadata) = $parser->parse($matches[0]); - - $provides = preg_split('/\s+/', trim(idx($metadata, 'provides'))); - $requires = preg_split('/\s+/', trim(idx($metadata, 'requires'))); - $provides = array_filter($provides); - $requires = array_filter($requires); - - if (!$provides) { - // Tests and documentation-only JS is permitted to @provide no targets. - return array(null, null); - } - - if (count($provides) > 1) { - throw new Exception( - pht( - 'Resource "%s" must @provide at most one Celerity target.', - $name)); - } - - return array(head($provides), $requires); - } - - - /** - * Check for dependency cycles in the resource graph. Raises an exception if - * a cycle is detected. - * - * @param map> Map of `@provides` symbols to their - * `@requires` symbols. - * @return void - */ - private function detectGraphCycles(array $nodes) { - $graph = id(new CelerityResourceGraph()) - ->addNodes($nodes) - ->setResourceGraph($nodes) - ->loadGraph(); - - foreach ($nodes as $provides => $requires) { - $cycle = $graph->detectCycles($provides); - if ($cycle) { - throw new Exception( - pht( - 'Cycle detected in resource graph: %s', - implode(' > ', $cycle))); - } - } - } - - /** - * Build package specifications for a given resource source. - * - * @param CelerityPhysicalResources Resource source to rebuild. - * @param list Map of `@provides` to hashes. - * @param list Map of hashes to resource names. - * @return map> Package information maps. - */ - private function rebuildPackages( - CelerityPhysicalResources $resources, - array $symbol_map, - array $reverse_map) { - - $package_map = array(); - - $package_spec = $resources->getResourcePackages(); - foreach ($package_spec as $package_name => $package_symbols) { - $type = null; - $hashes = array(); - foreach ($package_symbols as $symbol) { - $symbol_hash = idx($symbol_map, $symbol); - if ($symbol_hash === null) { - throw new Exception( - pht( - 'Package specification for "%s" includes "%s", but that symbol '. - 'is not @provided by any resource.', - $package_name, - $symbol)); - } - - $resource_name = $reverse_map[$symbol_hash]; - $resource_type = $resources->getResourceType($resource_name); - if ($type === null) { - $type = $resource_type; - } else if ($type !== $resource_type) { - throw new Exception( - pht( - 'Package specification for "%s" includes resources of multiple '. - 'types (%s, %s). Each package may only contain one type of '. - 'resource.', - $package_name, - $type, - $resource_type)); - } - - $hashes[] = $symbol.':'.$symbol_hash; - } - - $hash = $resources->getCelerityHash(implode("\n", $hashes)); - $package_map[$package_name] = array( - 'hash' => $hash, - 'symbols' => $package_symbols, - ); - } - - return $package_map; - } - - private function mergeNameMaps(array $maps) { - $result = array(); - $origin = array(); - foreach ($maps as $map) { - list($map_name, $data) = $map; - foreach ($data as $name => $hash) { - if (empty($result[$name])) { - $result[$name] = $hash; - $origin[$name] = $map_name; - } else { - $old = $origin[$name]; - $new = $map_name; - throw new Exception( - pht( - 'Resource source defines two resources with the same name, '. - '"%s". One is defined in the "%s" map; the other in the "%s" '. - 'map. Each resource must have a unique name.', - $name, - $old, - $new)); - } - } - } - return $result; - } - - private function log($message) { + protected function log($message) { $console = PhutilConsole::getConsole(); $console->writeErr("%s\n", $message); } - private function formatMapContent(array $data) { - $content = phutil_var_export($data); - - $generated = '@'.'generated'; - return <<