From 53260156a5370e238f63809325a25d2ab665282c Mon Sep 17 00:00:00 2001 From: Michael Dowling Date: Tue, 22 Jan 2013 23:53:52 -0800 Subject: [PATCH] Refactoring how operation responses are parsed Fixing a bug that did not allow clients to set response processing of a command Adding a VisitorFlyweight for creating request and response visitors Removing the owning of visitors from request and response parsers/serializers Adding before() method to response location visitors Moving the default parsing logic of JSON and XML from operation parser to location visitor before() methods Only triggering after() commands on location visitors associated with an operation Closes #214 --- .../Service/Command/AbstractCommand.php | 11 +- .../Command/DefaultRequestSerializer.php | 132 ++++++++-------- .../Service/Command/DefaultResponseParser.php | 5 +- .../Response/AbstractResponseVisitor.php | 6 + .../LocationVisitor/Response/JsonVisitor.php | 42 +++-- .../Response/ResponseVisitorInterface.php | 11 +- .../LocationVisitor/Response/XmlVisitor.php | 10 ++ .../LocationVisitor/VisitorFlyweight.php | 149 ++++++++++++++++++ .../NoTranslationOperationResponseParser.php | 29 ---- .../Service/Command/OperationCommand.php | 32 ++-- .../Command/OperationResponseParser.php | 108 +++++-------- tests/Guzzle/Tests/Service/ClientTest.php | 10 ++ .../Command/DefaultRequestSerializerTest.php | 8 +- .../Response/JsonVisitorTest.php | 14 ++ .../Response/XmlVisitorTest.php | 14 ++ .../LocationVisitor/VisitorFlyweightTest.php | 53 +++++++ .../Service/Command/OperationCommandTest.php | 24 ++- .../Command/OperationResponseParserTest.php | 96 +++++++---- 18 files changed, 515 insertions(+), 239 deletions(-) create mode 100644 src/Guzzle/Service/Command/LocationVisitor/VisitorFlyweight.php delete mode 100644 src/Guzzle/Service/Command/NoTranslationOperationResponseParser.php create mode 100644 tests/Guzzle/Tests/Service/Command/LocationVisitor/VisitorFlyweightTest.php diff --git a/src/Guzzle/Service/Command/AbstractCommand.php b/src/Guzzle/Service/Command/AbstractCommand.php index 49529fb9..69c54f0e 100644 --- a/src/Guzzle/Service/Command/AbstractCommand.php +++ b/src/Guzzle/Service/Command/AbstractCommand.php @@ -32,6 +32,7 @@ abstract class AbstractCommand extends Collection implements CommandInterface // Different response types that commands can use const TYPE_RAW = 'raw'; const TYPE_MODEL = 'model'; + const TYPE_NO_TRANSLATION = 'no_translation'; // Option used to change the entity body used to store a response const RESPONSE_BODY = 'command.response_body'; @@ -95,11 +96,6 @@ abstract class AbstractCommand extends Collection implements CommandInterface $this->setOnComplete($onComplete); } - // If no response processing value was specified, then attempt to use the highest level of processing - if (!$this->get(self::RESPONSE_PROCESSING)) { - $this->set(self::RESPONSE_PROCESSING, self::TYPE_MODEL); - } - $this->init(); } @@ -264,6 +260,11 @@ abstract class AbstractCommand extends Collection implements CommandInterface throw new CommandException('A client must be associated with the command before it can be prepared.'); } + // If no response processing value was specified, then attempt to use the highest level of processing + if (!$this->hasKey(self::RESPONSE_PROCESSING)) { + $this->set(self::RESPONSE_PROCESSING, self::TYPE_MODEL); + } + // Notify subscribers of the client that the command is being prepared $this->client->dispatch('command.before_prepare', array('command' => $this)); diff --git a/src/Guzzle/Service/Command/DefaultRequestSerializer.php b/src/Guzzle/Service/Command/DefaultRequestSerializer.php index 00bed453..96181a02 100644 --- a/src/Guzzle/Service/Command/DefaultRequestSerializer.php +++ b/src/Guzzle/Service/Command/DefaultRequestSerializer.php @@ -2,17 +2,11 @@ namespace Guzzle\Service\Command; +use Guzzle\Http\Message\RequestInterface; use Guzzle\Http\Url; use Guzzle\Parser\ParserRegistry; use Guzzle\Service\Command\LocationVisitor\Request\RequestVisitorInterface; -use Guzzle\Service\Command\LocationVisitor\Request\BodyVisitor; -use Guzzle\Service\Command\LocationVisitor\Request\ResponseBodyVisitor; -use Guzzle\Service\Command\LocationVisitor\Request\HeaderVisitor; -use Guzzle\Service\Command\LocationVisitor\Request\JsonVisitor; -use Guzzle\Service\Command\LocationVisitor\Request\QueryVisitor; -use Guzzle\Service\Command\LocationVisitor\Request\PostFieldVisitor; -use Guzzle\Service\Command\LocationVisitor\Request\PostFileVisitor; -use Guzzle\Service\Command\LocationVisitor\Request\XmlVisitor; +use Guzzle\Service\Command\LocationVisitor\VisitorFlyweight; /** * Default request serializer that transforms command options and operation parameters into a request @@ -20,17 +14,17 @@ use Guzzle\Service\Command\LocationVisitor\Request\XmlVisitor; class DefaultRequestSerializer implements RequestSerializerInterface { /** - * @var array Location visitors attached to the command + * @var VisitorFlyweight $factory Visitor factory */ - protected $visitors = array(); + protected $factory; /** - * @var array Cached instance with default visitors + * @var self */ protected static $instance; /** - * Get a default instance that includes that default location visitors + * Get a cached default instance of the class * * @return self * @codeCoverageIgnore @@ -38,31 +32,22 @@ class DefaultRequestSerializer implements RequestSerializerInterface public static function getInstance() { if (!self::$instance) { - self::$instance = new self(array( - 'header' => new HeaderVisitor(), - 'query' => new QueryVisitor(), - 'body' => new BodyVisitor(), - 'json' => new JsonVisitor(), - 'postFile' => new PostFileVisitor(), - 'postField' => new PostFieldVisitor(), - 'xml' => new XmlVisitor(), - 'response_body' => new ResponseBodyVisitor() - )); + self::$instance = new self(VisitorFlyweight::getInstance()); } return self::$instance; } /** - * @param array $visitors Visitors to attache + * @param VisitorFlyweight $factory Factory to use when creating visitors */ - public function __construct(array $visitors = array()) + public function __construct(VisitorFlyweight $factory) { - $this->visitors = $visitors; + $this->factory = $factory; } /** - * Add a location visitor to the command + * Add a location visitor to the serializer * * @param string $location Location to associate with the visitor * @param RequestVisitorInterface $visitor Visitor to attach @@ -71,7 +56,7 @@ class DefaultRequestSerializer implements RequestSerializerInterface */ public function addVisitor($location, RequestVisitorInterface $visitor) { - $this->visitors[$location] = $visitor; + $this->factory->addRequestVisitor($location, $visitor); return $this; } @@ -80,52 +65,75 @@ class DefaultRequestSerializer implements RequestSerializerInterface * {@inheritdoc} */ public function prepare(CommandInterface $command) + { + $request = $this->createRequest($command); + // Keep an array of visitors found in the operation + $foundVisitors = array(); + + // Add arguments to the request using the location attribute + foreach ($command->getOperation()->getParams() as $name => $arg) { + /** @var $arg \Guzzle\Service\Description\Parameter */ + if ($location = $arg->getLocation()) { + // Skip 'uri' locations because they've already been processed + if ($location == 'uri') { + continue; + } + // Instantiate visitors as they are detected in the properties + if (!isset($foundVisitors[$location])) { + $foundVisitors[$location] = $this->factory->getRequestVisitor($location); + } + // Ensure that a value has been set for this parameter + $value = $command->get($name); + if ($value !== null) { + // Apply the parameter value with the location visitor + $foundVisitors[$location]->visit($command, $request, $arg, $value); + } + } + } + + // Call the after method on each visitor found in the operation + foreach ($foundVisitors as $visitor) { + $visitor->after($command, $request); + } + + return $request; + } + + /** + * Create a request for the command and operation + * + * @param CommandInterface $command Command to create a request for + * + * @return RequestInterface + */ + protected function createRequest(CommandInterface $command) { $operation = $command->getOperation(); $client = $command->getClient(); - $uri = $operation->getUri(); - if (!$uri) { - $url = $client->getBaseUrl(); - } else { - // Get the path values and use the client config settings - $variables = $client->getConfig()->getAll(); - foreach ($operation->getParams() as $name => $arg) { - if ($arg->getLocation() == 'uri' && $command->hasKey($name)) { + // If the command does not specify a template, then assume the base URL of the client + if (!($uri = $operation->getUri())) { + return $client->createRequest($operation->getHttpMethod(), $client->getBaseUrl()); + } + + // Get the path values and use the client config settings + $variables = array(); + foreach ($operation->getParams() as $name => $arg) { + if ($arg->getLocation() == 'uri') { + if ($command->hasKey($name)) { $variables[$name] = $arg->filter($command->get($name)); if (!is_array($variables[$name])) { $variables[$name] = (string) $variables[$name]; } } } - // Merge the client's base URL with an expanded URI template - $url = (string) Url::factory($client->getBaseUrl()) - ->combine(ParserRegistry::getInstance()->getParser('uri_template')->expand($uri, $variables)); } - // Inject path and base_url values into the URL - $request = $client->createRequest($operation->getHttpMethod(), $url); - - // Add arguments to the request using the location attribute - foreach ($operation->getParams() as $name => $arg) { - /** @var $arg \Guzzle\Service\Description\Parameter */ - $location = $arg->getLocation(); - // Visit with the associated visitor - if (isset($this->visitors[$location])) { - // Ensure that a value has been set for this parameter - $value = $command->get($name); - if ($value !== null) { - // Apply the parameter value with the location visitor - $this->visitors[$location]->visit($command, $request, $arg, $value); - } - } - } - - // Call the after method on each visitor - foreach ($this->visitors as $visitor) { - $visitor->after($command, $request); - } - - return $request; + // Merge the client's base URL with an expanded URI template + return $client->createRequest( + $operation->getHttpMethod(), + (string) Url::factory($client->getBaseUrl()) + ->combine(ParserRegistry::getInstance()->getParser('uri_template')->expand($uri, $variables)) + ); } } diff --git a/src/Guzzle/Service/Command/DefaultResponseParser.php b/src/Guzzle/Service/Command/DefaultResponseParser.php index f73e5f98..599c4ca4 100644 --- a/src/Guzzle/Service/Command/DefaultResponseParser.php +++ b/src/Guzzle/Service/Command/DefaultResponseParser.php @@ -16,7 +16,6 @@ class DefaultResponseParser implements ResponseParserInterface /** * Get a cached instance of the default response parser - * * @return self * @codeCoverageIgnore */ @@ -43,13 +42,13 @@ class DefaultResponseParser implements ResponseParserInterface $contentType = (string) $response->getHeader('Content-Type'); } - return $this->parseForContentType($command, $response, $contentType); + return $this->handleParsing($command, $response, $contentType); } /** * {@inheritdoc} */ - public function parseForContentType(AbstractCommand $command, Response $response, $contentType) + protected function handleParsing(AbstractCommand $command, Response $response, $contentType) { $result = $response; if ($result->getBody()) { diff --git a/src/Guzzle/Service/Command/LocationVisitor/Response/AbstractResponseVisitor.php b/src/Guzzle/Service/Command/LocationVisitor/Response/AbstractResponseVisitor.php index c737be88..a4526634 100644 --- a/src/Guzzle/Service/Command/LocationVisitor/Response/AbstractResponseVisitor.php +++ b/src/Guzzle/Service/Command/LocationVisitor/Response/AbstractResponseVisitor.php @@ -11,6 +11,12 @@ use Guzzle\Service\Description\Parameter; */ abstract class AbstractResponseVisitor implements ResponseVisitorInterface { + /** + * {@inheritdoc} + * @codeCoverageIgnore + */ + public function before(CommandInterface $command, array &$result) {} + /** * {@inheritdoc} * @codeCoverageIgnore diff --git a/src/Guzzle/Service/Command/LocationVisitor/Response/JsonVisitor.php b/src/Guzzle/Service/Command/LocationVisitor/Response/JsonVisitor.php index 8846fb54..0a243b5c 100644 --- a/src/Guzzle/Service/Command/LocationVisitor/Response/JsonVisitor.php +++ b/src/Guzzle/Service/Command/LocationVisitor/Response/JsonVisitor.php @@ -16,6 +16,15 @@ use Guzzle\Service\Command\CommandInterface; */ class JsonVisitor extends AbstractResponseVisitor { + /** + * {@inheritdoc} + */ + public function before(CommandInterface $command, array &$result) + { + // Ensure that the result of the command is always rooted with the parsed JSON data + $result = $command->getResponse()->json(); + } + /** * {@inheritdoc} */ @@ -40,26 +49,29 @@ class JsonVisitor extends AbstractResponseVisitor */ protected function recursiveProcess(Parameter $param, &$value) { - if ($value !== null) { + if ($value === null) { + return; + } + + if (is_array($value)) { $type = $param->getType(); - if (is_array($value)) { - if ($type == 'array') { - foreach ($value as &$item) { - $this->recursiveProcess($param->getItems(), $item); - } - } elseif ($type == 'object' && !isset($value[0])) { - // On the above line, we ensure that the array is associative and not numerically indexed - if ($properties = $param->getProperties()) { - foreach ($properties as $property) { - $name = $property->getName(); - if (isset($value[$name])) { - $this->recursiveProcess($property, $value[$name]); - } + if ($type == 'array') { + foreach ($value as &$item) { + $this->recursiveProcess($param->getItems(), $item); + } + } elseif ($type == 'object' && !isset($value[0])) { + // On the above line, we ensure that the array is associative and not numerically indexed + if ($properties = $param->getProperties()) { + foreach ($properties as $property) { + $name = $property->getName(); + if (isset($value[$name])) { + $this->recursiveProcess($property, $value[$name]); } } } } - $value = $param->filter($value); } + + $value = $param->filter($value); } } diff --git a/src/Guzzle/Service/Command/LocationVisitor/Response/ResponseVisitorInterface.php b/src/Guzzle/Service/Command/LocationVisitor/Response/ResponseVisitorInterface.php index 27bdf35e..20db76e7 100644 --- a/src/Guzzle/Service/Command/LocationVisitor/Response/ResponseVisitorInterface.php +++ b/src/Guzzle/Service/Command/LocationVisitor/Response/ResponseVisitorInterface.php @@ -11,10 +11,19 @@ use Guzzle\Service\Command\CommandInterface; */ interface ResponseVisitorInterface { + /** + * Called before visiting all parameters. This can be used for seeding the result of a command with default + * data (e.g. populating with JSON data in the response then adding to the parsed data). + * + * @param CommandInterface $command Command being visited + * @param array $result Result value to update if needed (e.g. parsing XML or JSON into an array) + */ + public function before(CommandInterface $command, array &$result); + /** * Called after visiting all parameters * - * @param CommandInterface $command Command being visited + * @param CommandInterface $command Command being visited */ public function after(CommandInterface $command); diff --git a/src/Guzzle/Service/Command/LocationVisitor/Response/XmlVisitor.php b/src/Guzzle/Service/Command/LocationVisitor/Response/XmlVisitor.php index 07179d66..664a5b53 100644 --- a/src/Guzzle/Service/Command/LocationVisitor/Response/XmlVisitor.php +++ b/src/Guzzle/Service/Command/LocationVisitor/Response/XmlVisitor.php @@ -2,6 +2,7 @@ namespace Guzzle\Service\Command\LocationVisitor\Response; +use Guzzle\Common\Exception\RuntimeException; use Guzzle\Http\Message\Response; use Guzzle\Service\Description\Parameter; use Guzzle\Service\Command\CommandInterface; @@ -11,6 +12,15 @@ use Guzzle\Service\Command\CommandInterface; */ class XmlVisitor extends AbstractResponseVisitor { + /** + * {@inheritdoc} + */ + public function before(CommandInterface $command, array &$result) + { + // Set the result of the command to the array conversion of the XML body + $result = json_decode(json_encode($command->getResponse()->xml()), true); + } + /** * {@inheritdoc} */ diff --git a/src/Guzzle/Service/Command/LocationVisitor/VisitorFlyweight.php b/src/Guzzle/Service/Command/LocationVisitor/VisitorFlyweight.php new file mode 100644 index 00000000..fb7b24bd --- /dev/null +++ b/src/Guzzle/Service/Command/LocationVisitor/VisitorFlyweight.php @@ -0,0 +1,149 @@ + 'Guzzle\Service\Command\LocationVisitor\Request\BodyVisitor', + 'request.header' => 'Guzzle\Service\Command\LocationVisitor\Request\HeaderVisitor', + 'request.json' => 'Guzzle\Service\Command\LocationVisitor\Request\JsonVisitor', + 'request.postField' => 'Guzzle\Service\Command\LocationVisitor\Request\PostFieldVisitor', + 'request.postFile' => 'Guzzle\Service\Command\LocationVisitor\Request\PostFileVisitor', + 'request.query' => 'Guzzle\Service\Command\LocationVisitor\Request\QueryVisitor', + 'request.response_body' => 'Guzzle\Service\Command\LocationVisitor\Request\ResponseBodyVisitor', + 'request.xml' => 'Guzzle\Service\Command\LocationVisitor\Request\XmlVisitor', + 'response.body' => 'Guzzle\Service\Command\LocationVisitor\Response\BodyVisitor', + 'response.header' => 'Guzzle\Service\Command\LocationVisitor\Response\HeaderVisitor', + 'response.json' => 'Guzzle\Service\Command\LocationVisitor\Response\JsonVisitor', + 'response.reasonPhrase' => 'Guzzle\Service\Command\LocationVisitor\Response\ReasonPhraseVisitor', + 'response.statusCode' => 'Guzzle\Service\Command\LocationVisitor\Response\StatusCodeVisitor', + 'response.xml' => 'Guzzle\Service\Command\LocationVisitor\Response\XmlVisitor' + ); + + /** + * @var array Array of mappings of location names to classes + */ + protected $mappings; + + /** + * @var array Cache of instantiated visitors + */ + protected $cache = array(); + + /** + * Get a cached instance of the flyweight factory + * + * @return self + * @codeCoverageIgnore + */ + public static function getInstance() + { + if (!self::$instance) { + self::$instance = new self(); + } + + return self::$instance; + } + + /** + * Create a new flyweight + * + * @param array $mappings Array mapping request.name and response.name to location visitor classes. Leave null to + * use the default values. + */ + public function __construct(array $mappings = null) + { + $this->mappings = $mappings === null ? self::$defaultMappings : $mappings; + } + + /** + * Get an instance of a request visitor by location name + * + * @param string $visitor Visitor name + * + * @return RequestVisitorInterface + */ + public function getRequestVisitor($visitor) + { + return $this->getKey('request.' . $visitor); + } + + /** + * Get an instance of a response visitor by location name + * + * @param string $visitor Visitor name + * + * @return ResponseVisitorInterface + */ + public function getResponseVisitor($visitor) + { + return $this->getKey('response.' . $visitor); + } + + /** + * Add a response visitor to the factory by name + * + * @param string $name Name of the visitor + * @param RequestVisitorInterface $visitor Visitor to add + * + * @return self + */ + public function addRequestVisitor($name, RequestVisitorInterface $visitor) + { + $this->cache['request.' . $name] = $visitor; + + return $this; + } + + /** + * Add a response visitor to the factory by name + * + * @param string $name Name of the visitor + * @param ResponseVisitorInterface $visitor Visitor to add + * + * @return self + */ + public function addResponseVisitor($name, ResponseVisitorInterface $visitor) + { + $this->cache['response.' . $name] = $visitor; + + return $this; + } + + /** + * Get a visitor by key value name + * + * @param string $key Key name to retrieve + * + * @return mixed + * @throws InvalidArgumentException + */ + private function getKey($key) + { + if (!isset($this->cache[$key])) { + if (!isset($this->mappings[$key])) { + list($type, $name) = explode('.', $key); + throw new InvalidArgumentException("No {$type} visitor has been mapped for {$name}"); + } + $this->cache[$key] = new $this->mappings[$key]; + } + + return $this->cache[$key]; + } +} diff --git a/src/Guzzle/Service/Command/NoTranslationOperationResponseParser.php b/src/Guzzle/Service/Command/NoTranslationOperationResponseParser.php deleted file mode 100644 index 9cd11faa..00000000 --- a/src/Guzzle/Service/Command/NoTranslationOperationResponseParser.php +++ /dev/null @@ -1,29 +0,0 @@ -requestSerializer) { + // Use the default request serializer if none was found $this->requestSerializer = DefaultRequestSerializer::getInstance(); } return $this->requestSerializer; } + /** + * Get the response parser used for the operation + * + * @return ResponseParserInterface + */ + public function getResponseParser() + { + if (!$this->responseParser) { + // Use the default response parser if none was found + $this->responseParser = OperationResponseParser::getInstance(); + } + + return $this->responseParser; + } + /** * {@inheritdoc} */ @@ -70,14 +86,6 @@ class OperationCommand extends AbstractCommand { // Prepare and serialize the request $this->request = $this->getRequestSerializer()->prepare($this); - - // If no response parser is set, add the default parser if a model matching the responseClass is found - if (!$this->responseParser) { - $this->responseParser = $this->operation->getResponseType() == OperationInterface::TYPE_MODEL - && $this->get(self::RESPONSE_PROCESSING) == self::TYPE_MODEL - ? OperationResponseParser::getInstance() - : DefaultResponseParser::getInstance(); - } } /** @@ -85,9 +93,9 @@ class OperationCommand extends AbstractCommand */ protected function process() { - // Do not process the response if 'command.raw_response' is set - $this->result = $this->get(self::RESPONSE_PROCESSING) != self::TYPE_RAW - ? $this->responseParser->parse($this) - : $this->request->getResponse(); + // Do not process the response if 'command.response_processing' is set to 'raw' + $this->result = $this->get(self::RESPONSE_PROCESSING) == self::TYPE_RAW + ? $this->request->getResponse() + : $this->getResponseParser()->parse($this); } } diff --git a/src/Guzzle/Service/Command/OperationResponseParser.php b/src/Guzzle/Service/Command/OperationResponseParser.php index 569d84ba..15c6a9dc 100644 --- a/src/Guzzle/Service/Command/OperationResponseParser.php +++ b/src/Guzzle/Service/Command/OperationResponseParser.php @@ -3,14 +3,10 @@ namespace Guzzle\Service\Command; use Guzzle\Http\Message\Response; -use Guzzle\Service\Command\LocationVisitor\Response\HeaderVisitor; -use Guzzle\Service\Command\LocationVisitor\Response\StatusCodeVisitor; -use Guzzle\Service\Command\LocationVisitor\Response\ReasonPhraseVisitor; -use Guzzle\Service\Command\LocationVisitor\Response\BodyVisitor; -use Guzzle\Service\Command\LocationVisitor\Response\JsonVisitor; -use Guzzle\Service\Command\LocationVisitor\Response\XmlVisitor; +use Guzzle\Service\Command\LocationVisitor\VisitorFlyweight; use Guzzle\Service\Command\LocationVisitor\Response\ResponseVisitorInterface; use Guzzle\Service\Description\Parameter; +use Guzzle\Service\Description\OperationInterface; use Guzzle\Service\Description\Operation; use Guzzle\Service\Resource\Model; @@ -20,17 +16,17 @@ use Guzzle\Service\Resource\Model; class OperationResponseParser extends DefaultResponseParser { /** - * @var array Location visitors attached to the command + * @var VisitorFlyweight $factory Visitor factory */ - protected $visitors = array(); + protected $factory; /** - * @var array Cached instance with default visitors + * @var self */ protected static $instance; /** - * Get a default instance that includes that default location visitors + * Get a cached default instance of the Operation response parser that uses default visitors * * @return self * @codeCoverageIgnore @@ -38,25 +34,18 @@ class OperationResponseParser extends DefaultResponseParser public static function getInstance() { if (!static::$instance) { - static::$instance = new static(array( - 'statusCode' => new StatusCodeVisitor(), - 'reasonPhrase' => new ReasonPhraseVisitor(), - 'header' => new HeaderVisitor(), - 'body' => new BodyVisitor(), - 'json' => new JsonVisitor(), - 'xml' => new XmlVisitor() - )); + static::$instance = new static(VisitorFlyweight::getInstance()); } return static::$instance; } /** - * @param array $visitors Visitors to attach + * @param VisitorFlyweight $factory Factory to use when creating visitors */ - public function __construct(array $visitors = array()) + public function __construct(VisitorFlyweight $factory) { - $this->visitors = $visitors; + $this->factory = $factory; } /** @@ -69,7 +58,7 @@ class OperationResponseParser extends DefaultResponseParser */ public function addVisitor($location, ResponseVisitorInterface $visitor) { - $this->visitors[$location] = $visitor; + $this->factory->addResponseVisitor($location, $visitor); return $this; } @@ -77,47 +66,22 @@ class OperationResponseParser extends DefaultResponseParser /** * {@inheritdoc} */ - public function parseForContentType(AbstractCommand $command, Response $response, $contentType) + protected function handleParsing(AbstractCommand $command, Response $response, $contentType) { - // Perform that default native processing - $result = parent::parseForContentType($command, $response, $contentType); - $operation = $command->getOperation(); - $model = $operation->getResponseType() == 'model' - && $command->get(AbstractCommand::RESPONSE_PROCESSING) == AbstractCommand::TYPE_MODEL + $model = $operation->getResponseType() == OperationInterface::TYPE_MODEL ? $operation->getServiceDescription()->getModel($operation->getResponseClass()) : null; - // No further processing is needed if the responseType is not model or using native responses, or the model - // cannot be found if (!$model) { - return $result; + // Return basic processing if the responseType is not model or the model cannot be found + return parent::handleParsing($command, $response, $contentType); + } elseif ($command->get(AbstractCommand::RESPONSE_PROCESSING) != AbstractCommand::TYPE_MODEL) { + // Returns a model with no visiting if the command response processing is not model + return new Model(parent::handleParsing($command, $response, $contentType), $model); + } else { + return new Model($this->visitResult($model, $command, $response), $model); } - - if ($result instanceof \SimpleXMLElement) { - $result = $this->xmlToArray($result, $operation, $model); - } elseif ($result instanceof Response) { - $result = array(); - } - - // Perform transformations on the result using location visitors - $this->visitResult($model, $command, $response, $result); - - return new Model($result, $model); - } - - /** - * Parse a SimpleXMLElement into an array - * - * @param \SimpleXMLElement $xml XML to parse - * @param Operation $operation Operation that owns the model - * @param Parameter $model Model object - * - * @return array - */ - protected function xmlToArray(\SimpleXMLElement $xml, Operation $operation, Parameter $model) - { - return json_decode(json_encode($xml), true); } /** @@ -126,28 +90,36 @@ class OperationResponseParser extends DefaultResponseParser * @param Parameter $model Model that defines the structure * @param CommandInterface $command Command that performed the operation * @param Response $response Response received - * @param array $result Result array - * @param mixed $context Parsing context + * + * @return array Returns the array of result data */ protected function visitResult( Parameter $model, CommandInterface $command, - Response $response, - array &$result, - $context = null + Response $response ) { + // Determine what visitors are associated with the model + $foundVisitors = $result = array(); + foreach ($model->getProperties() as $schema) { - /** @var $arg Parameter */ - $location = $schema->getLocation(); - // Visit with the associated visitor - if (isset($this->visitors[$location])) { - // Apply the parameter value with the location visitor - $this->visitors[$location]->visit($command, $response, $schema, $result); + if ($location = $schema->getLocation()) { + $foundVisitors[$location] = $this->factory->getResponseVisitor($location); + $foundVisitors[$location]->before($command, $result); } } - foreach ($this->visitors as $visitor) { + foreach ($model->getProperties() as $schema) { + /** @var $arg Parameter */ + if ($location = $schema->getLocation()) { + // Apply the parameter value with the location visitor + $foundVisitors[$location]->visit($command, $response, $schema, $result); + } + } + + foreach ($foundVisitors as $visitor) { $visitor->after($command); } + + return $result; } } diff --git a/tests/Guzzle/Tests/Service/ClientTest.php b/tests/Guzzle/Tests/Service/ClientTest.php index 25d97344..3b70981b 100644 --- a/tests/Guzzle/Tests/Service/ClientTest.php +++ b/tests/Guzzle/Tests/Service/ClientTest.php @@ -11,6 +11,7 @@ use Guzzle\Service\Exception\CommandTransferException; use Guzzle\Service\Description\ServiceDescription; use Guzzle\Tests\Service\Mock\Command\MockCommand; use Guzzle\Service\Resource\ResourceIteratorClassFactory; +use Guzzle\Service\Command\AbstractCommand; /** * @group server @@ -41,6 +42,15 @@ class ClientTest extends \Guzzle\Tests\GuzzleTestCase $this->service = ServiceDescription::factory(__DIR__ . '/../TestData/test_service.json'); } + public function testAllowsCustomClientParameters() + { + $client = new Mock\MockClient(null, array( + Client::COMMAND_PARAMS => array(AbstractCommand::RESPONSE_PROCESSING => 'foo') + )); + $command = $client->getCommand('mock_command'); + $this->assertEquals('foo', $command->get(AbstractCommand::RESPONSE_PROCESSING)); + } + /** * @covers Guzzle\Service\Client::factory */ diff --git a/tests/Guzzle/Tests/Service/Command/DefaultRequestSerializerTest.php b/tests/Guzzle/Tests/Service/Command/DefaultRequestSerializerTest.php index be934453..dcd0d725 100644 --- a/tests/Guzzle/Tests/Service/Command/DefaultRequestSerializerTest.php +++ b/tests/Guzzle/Tests/Service/Command/DefaultRequestSerializerTest.php @@ -8,6 +8,7 @@ use Guzzle\Service\Client; use Guzzle\Service\Description\Operation; use Guzzle\Service\Description\Parameter; use Guzzle\Service\Command\LocationVisitor\Request\HeaderVisitor; +use Guzzle\Service\Command\LocationVisitor\VisitorFlyweight; /** * @covers Guzzle\Service\Command\DefaultRequestSerializer @@ -75,10 +76,11 @@ class DefaultRequestSerializerTest extends \Guzzle\Tests\GuzzleTestCase $this->assertEquals('http://foo.com/baz/bar/123', (string) $request->getUrl()); } - public function testConstructorAddsVisitors() + public function testAllowsCustomFactory() { - $serializer = new DefaultRequestSerializer(array()); - $this->assertEmpty($this->readAttribute($serializer, 'visitors')); + $f = new VisitorFlyweight(); + $serializer = new DefaultRequestSerializer($f); + $this->assertSame($f, $this->readAttribute($serializer, 'factory')); } public function testMixedParams() diff --git a/tests/Guzzle/Tests/Service/Command/LocationVisitor/Response/JsonVisitorTest.php b/tests/Guzzle/Tests/Service/Command/LocationVisitor/Response/JsonVisitorTest.php index b23c5acf..e4393a11 100644 --- a/tests/Guzzle/Tests/Service/Command/LocationVisitor/Response/JsonVisitorTest.php +++ b/tests/Guzzle/Tests/Service/Command/LocationVisitor/Response/JsonVisitorTest.php @@ -11,6 +11,20 @@ use Guzzle\Service\Command\LocationVisitor\Response\JsonVisitor as Visitor; */ class JsonVisitorTest extends AbstractResponseVisitorTest { + public function testBeforeMethodParsesXml() + { + $visitor = new Visitor(); + $command = $this->getMockBuilder('Guzzle\Service\Command\AbstractCommand') + ->setMethods(array('getResponse')) + ->getMockForAbstractClass(); + $command->expects($this->once()) + ->method('getResponse') + ->will($this->returnValue(new Response(200, null, '{"foo":"bar"}'))); + $result = array(); + $visitor->before($command, $result); + $this->assertEquals(array('foo' => 'bar'), $result); + } + public function testVisitsLocation() { $visitor = new Visitor(); diff --git a/tests/Guzzle/Tests/Service/Command/LocationVisitor/Response/XmlVisitorTest.php b/tests/Guzzle/Tests/Service/Command/LocationVisitor/Response/XmlVisitorTest.php index dd635a7e..ab9b5564 100644 --- a/tests/Guzzle/Tests/Service/Command/LocationVisitor/Response/XmlVisitorTest.php +++ b/tests/Guzzle/Tests/Service/Command/LocationVisitor/Response/XmlVisitorTest.php @@ -11,6 +11,20 @@ use Guzzle\Service\Command\LocationVisitor\Response\XmlVisitor as Visitor; */ class XmlVisitorTest extends AbstractResponseVisitorTest { + public function testBeforeMethodParsesXml() + { + $visitor = new Visitor(); + $command = $this->getMockBuilder('Guzzle\Service\Command\AbstractCommand') + ->setMethods(array('getResponse')) + ->getMockForAbstractClass(); + $command->expects($this->once()) + ->method('getResponse') + ->will($this->returnValue(new Response(200, null, 'test'))); + $result = array(); + $visitor->before($command, $result); + $this->assertEquals(array('Bar' => 'test'), $result); + } + public function testCanExtractAndRenameTopLevelXmlValues() { $visitor = new Visitor(); diff --git a/tests/Guzzle/Tests/Service/Command/LocationVisitor/VisitorFlyweightTest.php b/tests/Guzzle/Tests/Service/Command/LocationVisitor/VisitorFlyweightTest.php new file mode 100644 index 00000000..a252ffe6 --- /dev/null +++ b/tests/Guzzle/Tests/Service/Command/LocationVisitor/VisitorFlyweightTest.php @@ -0,0 +1,53 @@ +assertInstanceOf('Guzzle\Service\Command\LocationVisitor\Request\JsonVisitor', $f->getRequestVisitor('json')); + $this->assertInstanceOf('Guzzle\Service\Command\LocationVisitor\Response\JsonVisitor', $f->getResponseVisitor('json')); + } + + public function testCanUseCustomMappings() + { + $f = new VisitorFlyweight(array()); + $this->assertEquals(array(), $this->readAttribute($f, 'mappings')); + } + + /** + * @expectedException \InvalidArgumentException + * @expectedExceptionMessage No request visitor has been mapped for foo + */ + public function testThrowsExceptionWhenRetrievingUnknownVisitor() + { + VisitorFlyweight::getInstance()->getRequestVisitor('foo'); + } + + public function testCachesVisitors() + { + $f = new VisitorFlyweight(); + $v1 = $f->getRequestVisitor('json'); + $this->assertSame($v1, $f->getRequestVisitor('json')); + } + + public function testAllowsAddingVisitors() + { + $f = new VisitorFlyweight(); + $j1 = new JsonRequestVisitor(); + $j2 = new JsonResponseVisitor(); + $f->addRequestVisitor('json', $j1); + $f->addResponseVisitor('json', $j2); + $this->assertSame($j1, $f->getRequestVisitor('json')); + $this->assertSame($j2, $f->getResponseVisitor('json')); + } +} diff --git a/tests/Guzzle/Tests/Service/Command/OperationCommandTest.php b/tests/Guzzle/Tests/Service/Command/OperationCommandTest.php index aed90efb..39dfe65c 100644 --- a/tests/Guzzle/Tests/Service/Command/OperationCommandTest.php +++ b/tests/Guzzle/Tests/Service/Command/OperationCommandTest.php @@ -10,6 +10,7 @@ use Guzzle\Service\Description\Operation; use Guzzle\Service\Description\ServiceDescription; use Guzzle\Service\Command\DefaultRequestSerializer; use Guzzle\Service\Resource\Model; +use Guzzle\Service\Command\LocationVisitor\VisitorFlyweight; /** * @covers Guzzle\Service\Command\OperationCommand @@ -20,7 +21,7 @@ class OperationCommandTest extends \Guzzle\Tests\GuzzleTestCase { $operation = new OperationCommand(); $a = $operation->getRequestSerializer(); - $b = new DefaultRequestSerializer(); + $b = new DefaultRequestSerializer(VisitorFlyweight::getInstance()); $operation->setRequestSerializer($b); $this->assertNotSame($a, $operation->getRequestSerializer()); } @@ -57,19 +58,28 @@ class OperationCommandTest extends \Guzzle\Tests\GuzzleTestCase public function testParsesResponsesUsingModelParserWhenMatchingModelIsFound() { - $description = new ServiceDescription(array( - 'operations' => array('foo' => array('responseClass' => 'bar', 'responseType' => 'model')), - 'models' => array('bar' => array()) + $description = ServiceDescription::factory(array( + 'operations' => array( + 'foo' => array('responseClass' => 'bar', 'responseType' => 'model') + ), + 'models' => array( + 'bar' => array( + 'type' => 'object', + 'properties' => array( + 'Baz' => array('type' => 'string', 'location' => 'xml') + ) + ) + ) )); + $op = new OperationCommand(array(), $description->getOperation('foo')); $op->setClient(new Client()); $request = $op->prepare(); $request->setResponse(new Response(200, array( 'Content-Type' => 'application/xml' ), 'Bar'), true); - $this->assertEquals(new Model(array( - 'Baz' => 'Bar' - ), $description->getModel('bar')), $op->execute()); + $result = $op->execute(); + $this->assertEquals(new Model(array('Baz' => 'Bar'), $description->getModel('bar')), $result); } public function testAllowsRawResponses() diff --git a/tests/Guzzle/Tests/Service/Command/OperationResponseParserTest.php b/tests/Guzzle/Tests/Service/Command/OperationResponseParserTest.php index cd37f088..701a27e0 100644 --- a/tests/Guzzle/Tests/Service/Command/OperationResponseParserTest.php +++ b/tests/Guzzle/Tests/Service/Command/OperationResponseParserTest.php @@ -11,8 +11,9 @@ use Guzzle\Service\Description\Operation; use Guzzle\Service\Description\ServiceDescription; use Guzzle\Service\Command\LocationVisitor\Response\StatusCodeVisitor; use Guzzle\Service\Command\LocationVisitor\Response\ReasonPhraseVisitor; +use Guzzle\Service\Command\LocationVisitor\Response\JsonVisitor; use Guzzle\Service\Command\LocationVisitor\Response\BodyVisitor; -use Guzzle\Service\Resource\Model; +use Guzzle\Service\Command\LocationVisitor\VisitorFlyweight; /** * @covers Guzzle\Service\Command\OperationResponseParser @@ -21,15 +22,15 @@ class OperationResponseParserTest extends \Guzzle\Tests\GuzzleTestCase { public function testHasVisitors() { - $p = new OperationResponseParser(); + $p = new OperationResponseParser(new VisitorFlyweight(array())); $visitor = new BodyVisitor(); $p->addVisitor('foo', $visitor); - $this->assertSame(array('foo' => $visitor), $this->readAttribute($p, 'visitors')); + $this->assertSame($visitor, $this->readAttribute($p, 'factory')->getResponseVisitor('foo')); } public function testUsesParentParser() { - $p = new OperationResponseParser(); + $p = new OperationResponseParser(new VisitorFlyweight()); $operation = new Operation(); $operation->setServiceDescription(new ServiceDescription()); $op = new OperationCommand(array(), $operation); @@ -38,31 +39,12 @@ class OperationResponseParserTest extends \Guzzle\Tests\GuzzleTestCase $this->assertInstanceOf('SimpleXMLElement', $op->execute()); } - public function testConvertsSimpleXMLElementToArrayWhenModelIsFound() - { - $parser = new OperationResponseParser(); - $op = new OperationCommand(array(), $this->getDescription()->getOperation('test')); - $op->setResponseParser($parser)->setClient(new Client()); - $op->prepare()->setResponse(new Response(200, array('Content-Type' => 'application/xml'), 'C'), true); - $this->assertInstanceOf('Guzzle\Service\Resource\Model', $op->execute()); - $this->assertEquals('C', $op->getResult()->get('B')); - } - - public function testUsesNativeResultWhenInstructed() - { - $parser = new OperationResponseParser(); - $op = new OperationCommand(array(), $this->getDescription()->getOperation('test')); - $op->setResponseParser($parser)->setClient(new Client()); - $op->prepare()->setResponse(new Response(200, array('Content-Type' => 'application/xml'), 'C'), true); - $op->set(AbstractCommand::RESPONSE_PROCESSING, 'native'); - $this->assertInstanceOf('SimpleXMLElement', $op->execute()); - } - public function testVisitsLocations() { - $parser = new OperationResponseParser(); + $parser = new OperationResponseParser(new VisitorFlyweight(array())); $parser->addVisitor('statusCode', new StatusCodeVisitor()); $parser->addVisitor('reasonPhrase', new ReasonPhraseVisitor()); + $parser->addVisitor('json', new JsonVisitor()); $op = new OperationCommand(array(), $this->getDescription()->getOperation('test')); $op->setResponseParser($parser)->setClient(new Client()); $op->prepare()->setResponse(new Response(201), true); @@ -73,19 +55,25 @@ class OperationResponseParserTest extends \Guzzle\Tests\GuzzleTestCase public function testVisitsLocationsForJsonResponse() { - $parser = new OperationResponseParser(); - $op = new OperationCommand(array(), $this->getDescription()->getOperation('test')); + $parser = OperationResponseParser::getInstance(); + $operation = $this->getDescription()->getOperation('test'); + $op = new OperationCommand(array(), $operation); $op->setResponseParser($parser)->setClient(new Client()); $op->prepare()->setResponse(new Response(200, array( 'Content-Type' => 'application/json' - ), '{"baz":"bar"}'), true); + ), '{"baz":"bar","enigma":"123"}'), true); $result = $op->execute(); - $this->assertEquals(array('baz' => 'bar'), $result->toArray()); + $this->assertEquals(array( + 'baz' => 'bar', + 'enigma' => '123', + 'code' => 200, + 'phrase' => 'OK' + ), $result->toArray()); } public function testSkipsUnkownModels() { - $parser = new OperationResponseParser(); + $parser = OperationResponseParser::getInstance(); $operation = $this->getDescription()->getOperation('test'); $operation->setResponseClass('Baz')->setResponseType('model'); $op = new OperationCommand(array(), $operation); @@ -94,16 +82,56 @@ class OperationResponseParserTest extends \Guzzle\Tests\GuzzleTestCase $this->assertInstanceOf('Guzzle\Http\Message\Response', $op->execute()); } - protected function getDescription() + public function testAllowsModelProcessingToBeDisabled() { - return new ServiceDescription(array( + $parser = OperationResponseParser::getInstance(); + $operation = $this->getDescription()->getOperation('test'); + $op = new OperationCommand(array('command.response_processing' => 'native'), $operation); + $op->setResponseParser($parser)->setClient(new Client()); + $op->prepare()->setResponse(new Response(200, array( + 'Content-Type' => 'application/json' + ), '{"baz":"bar","enigma":"123"}'), true); + $result = $op->execute(); + $this->assertInstanceOf('Guzzle\Service\Resource\Model', $result); + $this->assertEquals(array( + 'baz' => 'bar', + 'enigma' => '123' + ), $result->toArray()); + } + + public function testDoesNotParseXmlWhenNotUsingXmlVisitor() + { + $parser = OperationResponseParser::getInstance(); + $description = ServiceDescription::factory(array( + 'operations' => array('test' => array('responseClass' => 'Foo')), + 'models' => array( + 'Foo' => array( + 'type' => 'object', + 'properties' => array('baz' => array('location' => 'body')) + ) + ) + )); + $operation = $description->getOperation('test'); + $op = new OperationCommand(array(), $operation); + $op->setResponseParser($parser)->setClient(new Client()); + $brokenXml = '<><><>>>>'; + $op->prepare()->setResponse(new Response(200, array( + 'Content-Type' => 'application/xml' + ), $brokenXml), true); + $result = $op->execute(); + $this->assertEquals(array('baz'), $result->getKeys()); + $this->assertEquals($brokenXml, (string) $result['baz']); + } + + protected function getDescription() + { + return ServiceDescription::factory(array( 'operations' => array('test' => array('responseClass' => 'Foo')), 'models' => array( 'Foo' => array( - 'name' => 'Foo', 'type' => 'object', 'properties' => array( - 'baz' => array('type' => 'string'), + 'baz' => array('type' => 'string', 'location' => 'json'), 'code' => array('location' => 'statusCode'), 'phrase' => array('location' => 'reasonPhrase'), )