Adding name attribute to parameters that are a child of an array. Adding plugins to service builders. Adding the ability for the XML response parser to map collections into flattened arrays.

src/Guzzle/Service/Builder/ServiceBuilder.php

@@ -6,6 +6,7 @@ use Guzzle\Common\AbstractHasDispatcher;
 use Guzzle\Http\ClientInterface;
 use Guzzle\Service\Exception\ServiceBuilderException;
 use Guzzle\Service\Exception\ServiceNotFoundException;
+use Symfony\Component\EventDispatcher\EventSubscriberInterface;
 
 /**
  * Service builder to generate service builders and service clients from configuration settings
@@ -27,6 +28,11 @@ class ServiceBuilder extends AbstractHasDispatcher implements ServiceBuilderInte
      */
     protected static $cachedFactory;
 
+    /**
+     * @var array Plugins to attach to each client created by the service builder
+     */
+    protected $plugins = array();
+
     /**
      * Create a new ServiceBuilder using configuration data sourced from an
      * array, .json|.js file, SimpleXMLElement, or .xml file.
@@ -90,6 +96,20 @@ class ServiceBuilder extends AbstractHasDispatcher implements ServiceBuilderInte
         return json_encode($this->builderConfig);
     }
 
+    /**
+     * Attach a plugin to every client created by the builder
+     *
+     * @param EventSubscriberInterface $plugin Plugin to attach to each client
+     *
+     * @return self
+     */
+    public function addGlobalPlugin(EventSubscriberInterface $plugin)
+    {
+        $this->plugins[] = $plugin;
+
+        return $this;
+    }
+
     /**
      * {@inheritdoc}
      */
@@ -117,6 +137,10 @@ class ServiceBuilder extends AbstractHasDispatcher implements ServiceBuilderInte
             $this->clients[$name] = $client;
         }
 
+        foreach ($this->plugins as $plugin) {
+            $client->addSubscriber($plugin);
+        }
+
         // Dispatch an event letting listeners know a client was created
         $this->dispatch('service_builder.create_client', array(
             'client' => $client

src/Guzzle/Service/Command/LocationVisitor/Request/XmlVisitor.php

@@ -99,13 +99,13 @@ class XmlVisitor extends AbstractRequestVisitor
     protected function addXml(\SimpleXMLElement $xml, Parameter $param, $value)
     {
         // Determine the name of the element
-        $node = $param->getRename() ?: $param->getName();
+        $node = $param->getKey();
         // Check if this property has a particular namespace
         $namespace = $param->getData('namespace') ?: null;
 
         if ($param->getType() == 'array') {
             if ($items = $param->getItems()) {
-                $name = $items->getRename();
+                $name = $items->getKey();
                 foreach ($value as $v) {
                     if ($items->getType() == 'object' || $items->getType() == 'array') {
                         $child = $xml->addChild($name, null, $namespace);

src/Guzzle/Service/Command/LocationVisitor/Response/XmlVisitor.php

@@ -35,29 +35,50 @@ class XmlVisitor extends AbstractResponseVisitor
      */
     protected function recursiveProcess(Parameter $param, &$value)
     {
-        if ($value !== null) {
+        $type = $param->getType();
-            $type = $param->getType();
+
-            if (is_array($value)) {
+        if (is_array($value)) {
-                if ($type == 'array') {
+
-                    // Convert the node if it was meant to be an array
+            if ($type == 'array') {
-                    if (!isset($value[0])) {
+                // Convert the node if it was meant to be an array
+                if (!isset($value[0])) {
+                    // Collections fo nodes are sometimes wrapped in an additional array. For example:
+                    // <Items><Item><a>1</a></Item><Item><a>2</a></Item></Items> should become:
+                    // array('Items' => array(array('a' => 1), array('a' => 2))
+                    // Some nodes are not wrapped. For example: <Foo><a>1</a></Foo><Foo><a>2</a></Foo>
+                    // should become array('Foo' => array(array('a' => 1), array('a' => 2))
+                    if ($param->getItems() && isset($value[$param->getItems()->getName()])) {
+                        // Account for the case of a collection wrapping wrapped nodes: Items => Item[]
+                        $value = $value[$param->getItems()->getName()];
+                        // If the wrapped node only had one value, then make it an array of nodes
+                        if (!isset($value[0])) {
+                            $value = array($value);
+                        }
+                    } elseif (!empty($value)) {
+                        // Account for repeated nodes that must be an array: Foo => Baz, Foo => Baz, but only if the
+                        // value is set and not empty
                         $value = array($value);
                     }
-                    foreach ($value as &$item) {
+                }
-                        $this->recursiveProcess($param->getItems(), $item);
+
-                    }
+                foreach ($value as &$item) {
-                } elseif ($type == 'object' && !isset($value[0])) {
+                    $this->recursiveProcess($param->getItems(), $item);
-                    // On the above line, we ensure that the array is associative and not numerically indexed
+                }
-                    if ($properties = $param->getProperties()) {
+
-                        foreach ($properties as $property) {
+            } elseif ($type == 'object' && !isset($value[0])) {
-                            $name = $property->getName();
+                // On the above line, we ensure that the array is associative and not numerically indexed
-                            if (isset($value[$name])) {
+                if ($properties = $param->getProperties()) {
-                                $this->recursiveProcess($property, $value[$name]);
+                    foreach ($properties as $property) {
-                            }
+                        $name = $property->getName();
+                        if (isset($value[$name])) {
+                            $this->recursiveProcess($property, $value[$name]);
                         }
                     }
                 }
             }
+        }
+
+        if ($value !== null) {
             $value = $param->filter($value);
         }
     }

src/Guzzle/Service/Description/Parameter.php

@@ -131,10 +131,15 @@ class Parameter
     public function toArray()
     {
         $result = array();
-        foreach (array(
+        $checks = array('required', 'description', 'static', 'type', 'instanceOf', 'location', 'rename', 'pattern',
-            'required', 'description', 'static', 'type', 'instanceOf', 'location', 'rename', 'pattern', 'minimum',
+            'minimum', 'maximum', 'minItems', 'maxItems', 'minLength', 'maxLength', 'data', 'enum', 'filters');
-            'maximum', 'minItems', 'maxItems', 'minLength', 'maxLength', 'data', 'enum', 'filters'
+
-        ) as $c) {
+        // Anything that is in the `Items` attribute of an array *must* include it's name if available
+        if ($this->parent instanceof self && $this->parent->getType() == 'array' && isset($this->name)) {
+            $result['name'] = $this->name;
+        }
+
+        foreach ($checks as $c) {
             if ($value = $this->{$c}) {
                 $result[$c] = $value;
             }

src/Guzzle/Service/Plugin/PluginCollectionPlugin.php

@@ -1,49 +0,0 @@
-<?php
-
-namespace Guzzle\Service\Plugin;
-
-use Guzzle\Common\Event;
-use Symfony\Component\EventDispatcher\EventSubscriberInterface;
-
-/**
- * Service builder plugin used to add plugins to all clients created by a {@see Guzzle\Service\Builder\ServiceBuilder}
- *
- * @author Gordon Franke <info@nevalon.de>
- */
-class PluginCollectionPlugin implements EventSubscriberInterface
-{
-    /**
-     * @var $plugins array plugins to add
-     */
-    private $plugins = array();
-
-    /**
-     * @param array $plugins plugins to add
-     */
-    public function __construct(array $plugins)
-    {
-        $this->plugins = $plugins;
-    }
-
-    /**
-     * {@inheritdoc}
-     */
-    public static function getSubscribedEvents()
-    {
-        return array(
-            'service_builder.create_client' => 'onCreateClient'
-        );
-    }
-
-    /**
-     * Adds plugins to clients as they are created by the service builder
-     *
-     * @param Event $event Event emitted
-     */
-    public function onCreateClient(Event $event)
-    {
-        foreach ($this->plugins as $plugin) {
-            $event['client']->addSubscriber($plugin);
-        }
-    }
-}

tests/Guzzle/Tests/Service/Builder/ServiceBuilderTest.php

@@ -2,7 +2,7 @@
 
 namespace Guzzle\Tests\Service;
 
-use Guzzle\Cache\DoctrineCacheAdapter;
+use Guzzle\Plugin\History\HistoryPlugin;
 use Guzzle\Service\Builder\ServiceBuilder;
 use Guzzle\Service\Client;
 use Guzzle\Service\Exception\ServiceNotFoundException;
@@ -15,7 +15,7 @@ class ServiceBuilderTest extends \Guzzle\Tests\GuzzleTestCase
 {
     protected $arrayData = array(
         'michael.mock' => array(
-            'class' => 'Guzzle\\Tests\\Service\\Mock\\MockClient',
+            'class' => 'Guzzle\Tests\Service\Mock\MockClient',
             'params' => array(
                 'username' => 'michael',
                 'password' => 'testing123',
@@ -23,7 +23,7 @@ class ServiceBuilderTest extends \Guzzle\Tests\GuzzleTestCase
             ),
         ),
         'billy.mock' => array(
-            'class' => 'Guzzle\\Tests\\Service\\Mock\\MockClient',
+            'class' => 'Guzzle\Tests\Service\Mock\MockClient',
             'params' => array(
                 'username' => 'billy',
                 'password' => 'passw0rd',
@@ -42,12 +42,12 @@ class ServiceBuilderTest extends \Guzzle\Tests\GuzzleTestCase
         'cache.adapter' => array(
             'class'  => 'Guzzle\Cache\CacheAdapterFactory',
             'params' => array(
-                'cache.adapter'  => 'Guzzle.Cache.DoctrineCacheAdapter',
+                'cache.adapter'  => 'Guzzle\Cache\DoctrineCacheAdapter',
-                'cache.provider' => 'Doctrine.Common.Cache.ArrayCache'
+                'cache.provider' => 'Doctrine\Common\Cache\ArrayCache'
             )
         ),
         'service_uses_cache' => array(
-            'class'  => 'Guzzle\\Tests\\Service\\Mock\\MockClient',
+            'class'  => 'Guzzle\Tests\Service\Mock\MockClient',
             'params' => array(
                 'cache'     => '{cache.adapter}',
                 'username'  => 'foo',
@@ -68,7 +68,7 @@ class ServiceBuilderTest extends \Guzzle\Tests\GuzzleTestCase
     {
         $builder = ServiceBuilder::factory($this->arrayData);
         $c = $builder->get('michael.mock');
-        $this->assertInstanceOf('Guzzle\\Tests\\Service\\Mock\\MockClient', $c);
+        $this->assertInstanceOf('Guzzle\Tests\Service\Mock\MockClient', $c);
     }
 
     /**
@@ -84,13 +84,13 @@ class ServiceBuilderTest extends \Guzzle\Tests\GuzzleTestCase
     {
         $builder = ServiceBuilder::factory($this->arrayData);
         $client = $builder->get('michael.mock');
-        $this->assertInstanceOf('Guzzle\\Tests\\Service\\Mock\\MockClient', $client);
+        $this->assertInstanceOf('Guzzle\Tests\Service\Mock\MockClient', $client);
         $this->assertEquals('http://127.0.0.1:8124/v1/michael', $client->getBaseUrl());
         $this->assertEquals($client, $builder->get('michael.mock'));
 
         // Get another client but throw this one away
         $client2 = $builder->get('billy.mock', true);
-        $this->assertInstanceOf('Guzzle\\Tests\\Service\\Mock\\MockClient', $client2);
+        $this->assertInstanceOf('Guzzle\Tests\Service\Mock\MockClient', $client2);
         $this->assertEquals('http://127.0.0.1:8124/v1/billy', $client2->getBaseUrl());
 
         // Make sure the original client is still there and set
@@ -110,7 +110,7 @@ class ServiceBuilderTest extends \Guzzle\Tests\GuzzleTestCase
     {
         $s = new ServiceBuilder(array(
             'michael.mock' => array(
-                'class' => 'Guzzle\\Tests\\Service\\Mock\\MockClient',
+                'class' => 'Guzzle\Tests\Service\Mock\MockClient',
                 'params' => array(
                     'base_url' => 'http://www.test.com/',
                     'subdomain' => 'michael',
@@ -129,7 +129,7 @@ class ServiceBuilderTest extends \Guzzle\Tests\GuzzleTestCase
     {
         $s = new ServiceBuilder(array(
             'michael.mock' => array(
-                'class' => 'Guzzle\\Tests\\Service\\Mock\\MockClient',
+                'class' => 'Guzzle\Tests\Service\Mock\MockClient',
                 'params' => array(
                     'base_url' => 'http://www.test.com/',
                     'subdomain' => 'michael',
@@ -141,7 +141,7 @@ class ServiceBuilderTest extends \Guzzle\Tests\GuzzleTestCase
         ));
 
         $c = $s->get('michael.mock');
-        $this->assertInstanceOf('Guzzle\\Tests\\Service\\Mock\\MockClient', $c);
+        $this->assertInstanceOf('Guzzle\Tests\Service\Mock\MockClient', $c);
     }
 
     public function testUsedAsArray()
@@ -149,13 +149,13 @@ class ServiceBuilderTest extends \Guzzle\Tests\GuzzleTestCase
         $b = ServiceBuilder::factory($this->arrayData);
         $this->assertTrue($b->offsetExists('michael.mock'));
         $this->assertFalse($b->offsetExists('not_there'));
-        $this->assertInstanceOf('Guzzle\\Service\\Client', $b['michael.mock']);
+        $this->assertInstanceOf('Guzzle\Service\Client', $b['michael.mock']);
 
         unset($b['michael.mock']);
         $this->assertFalse($b->offsetExists('michael.mock'));
 
         $b['michael.mock'] = new Client('http://www.test.com/');
-        $this->assertInstanceOf('Guzzle\\Service\\Client', $b['michael.mock']);
+        $this->assertInstanceOf('Guzzle\Service\Client', $b['michael.mock']);
     }
 
     public function testFactoryCanCreateFromJson()
@@ -188,7 +188,7 @@ class ServiceBuilderTest extends \Guzzle\Tests\GuzzleTestCase
     {
         $builder = ServiceBuilder::factory(array(
             'a' => array(
-                'class' => 'Guzzle\\Tests\\Service\\Mock\\MockClient',
+                'class' => 'Guzzle\Tests\Service\Mock\MockClient',
                 'params' => array(
                     'other_client' => '{{ b }}',
                     'username'     => 'x',
@@ -197,7 +197,7 @@ class ServiceBuilderTest extends \Guzzle\Tests\GuzzleTestCase
                 )
             ),
             'b' => array(
-                'class' => 'Guzzle\\Tests\\Service\\Mock\\MockClient',
+                'class' => 'Guzzle\Tests\Service\Mock\MockClient',
                 'params' => array(
                     'username'  => '1',
                     'password'  => '2',
@@ -220,7 +220,7 @@ class ServiceBuilderTest extends \Guzzle\Tests\GuzzleTestCase
         // Create a test service builder
         $builder = ServiceBuilder::factory(array(
             'a' => array(
-                'class' => 'Guzzle\\Tests\\Service\\Mock\\MockClient',
+                'class' => 'Guzzle\Tests\Service\Mock\MockClient',
                 'params' => array(
                     'username'  => 'test',
                     'password'  => '123',
@@ -281,4 +281,12 @@ class ServiceBuilderTest extends \Guzzle\Tests\GuzzleTestCase
             $builder['service_uses_cache']->getConfig('cache')
         );
     }
+
+    public function testAddsGlobalPlugins()
+    {
+        $b = new ServiceBuilder($this->arrayData);
+        $b->addGlobalPlugin(new HistoryPlugin());
+        $s = $b->get('michael.mock');
+        $this->assertTrue($s->getEventDispatcher()->hasListeners('request.complete'));
+    }
 }

tests/Guzzle/Tests/Service/Command/LocationVisitor/Response/XmlVisitorTest.php

@@ -21,7 +21,7 @@ class XmlVisitorTest extends AbstractResponseVisitorTest
         $this->assertEquals('test', $value['foo']);
     }
 
-    public function testEnsuresArraysAreInCorrectLocations()
+    public function testEnsuresRepeatedArraysAreInCorrectLocations()
     {
         $visitor = new Visitor();
         $param = new Parameter(array(
@@ -33,13 +33,16 @@ class XmlVisitorTest extends AbstractResponseVisitorTest
                 'type' => 'object',
                 'properties' => array(
                     'Bar' => array('type' => 'string'),
-                    'Baz' => array('type' => 'string')
+                    'Baz' => array('type' => 'string'),
+                    'Bam' => array('type' => 'string')
                 )
             )
         ));
 
         $xml = new \SimpleXMLElement('<Test><Foo><Bar>1</Bar><Baz>2</Baz></Foo></Test>');
         $value = json_decode(json_encode($xml), true);
+        // Set a null value to ensure it is ignored
+        $value['foo'][0]['Bam'] = null;
         $visitor->visit($this->command, $this->response, $param, $value);
         $this->assertEquals(array(
             'foo' => array(
@@ -50,4 +53,50 @@ class XmlVisitorTest extends AbstractResponseVisitorTest
             )
         ), $value);
     }
+
+    public function xmlDataProvider()
+    {
+        $param = new Parameter(array(
+            'location' => 'xml',
+            'name'     => 'Items',
+            'type'     => 'array',
+            'items'    => array(
+                'type' => 'object',
+                'name' => 'Item',
+                'properties' => array(
+                    'Bar' => array('type' => 'string'),
+                    'Baz' => array('type' => 'string')
+                )
+            )
+        ));
+
+        return array(
+            array($param, '<Test><Items><Item><Bar>1</Bar></Item><Item><Bar>2</Bar></Item></Items></Test>', array(
+                'Items' => array(
+                    array('Bar' => 1),
+                    array('Bar' => 2)
+                )
+            )),
+            array($param, '<Test><Items><Item><Bar>1</Bar></Item></Items></Test>', array(
+                'Items' => array(
+                    array('Bar' => 1)
+                )
+            )),
+            array($param, '<Test><Items /></Test>', array(
+                'Items' => array()
+            ))
+        );
+    }
+
+    /**
+     * @dataProvider xmlDataProvider
+     */
+    public function testEnsuresWrappedArraysAreInCorrectLocations($param, $xml, $result)
+    {
+        $visitor = new Visitor();
+        $xml = new \SimpleXMLElement($xml);
+        $value = json_decode(json_encode($xml), true);
+        $visitor->visit($this->command, $this->response, $param, $value);
+        $this->assertEquals($result, $value);
+    }
 }

tests/Guzzle/Tests/Service/Description/ParameterTest.php

@@ -324,4 +324,25 @@ class ParameterTest extends \Guzzle\Tests\GuzzleTestCase
         $p->setRename(null);
         $this->assertEquals('foo', $p->getKey());
     }
+
+    public function testIncludesNameInToArrayWhenItemsAttriubuteHasName()
+    {
+        $p = new Parameter(array(
+            'type' => 'array',
+            'name' => 'Abc',
+            'items' => array(
+                'name' => 'Foo',
+                'type' => 'object'
+            )
+        ));
+        $result = $p->toArray();
+        $this->assertEquals(array(
+            'type' => 'array',
+            'items' => array(
+                'name' => 'Foo',
+                'type' => 'object',
+                'additionalProperties' => true
+            )
+        ), $result);
+    }
 }

tests/Guzzle/Tests/Service/Plugin/PluginCollectionPluginTest.php

@@ -1,44 +0,0 @@
-<?php
-
-namespace Guzzle\Tests\Service;
-
-use Guzzle\Service\Builder\ServiceBuilder;
-use Guzzle\Service\Plugin\PluginCollectionPlugin;
-
-use Symfony\Component\EventDispatcher\EventSubscriberInterface;
-
-class PluginCollectionPluginTest extends \Guzzle\Tests\GuzzleTestCase
-{
-    /**
-     * @covers Guzzle\Service\Plugin\PluginCollectionPlugin
-     */
-    public function testPluginPassPluginsThroughToClients()
-    {
-        $s = new ServiceBuilder(array(
-            'michael.mock' => array(
-                'class' => 'Guzzle\\Tests\\Service\\Mock\\MockClient',
-                'params' => array(
-                    'base_url' => 'http://www.test.com/',
-                    'subdomain' => 'michael',
-                    'password' => 'test',
-                    'username' => 'michael',
-                )
-            )
-        ));
-
-        $plugin = $this->getMock('Symfony\Component\EventDispatcher\EventSubscriberInterface');
-
-        $plugin::staticExpects($this->any())
-             ->method('getSubscribedEvents')
-             ->will($this->returnValue(array('client.create_request' => 'onRequestCreate')));
-
-        $s->addSubscriber(new PluginCollectionPlugin(array($plugin)));
-
-        $c = $s->get('michael.mock');
-        $this->assertTrue($c->getEventDispatcher()->hasListeners('client.create_request'));
-
-        $listeners = $c->getEventDispatcher()->getListeners('client.create_request');
-        $this->assertSame($plugin, $listeners[0][0]);
-        $this->assertEquals('onRequestCreate', $listeners[0][1]);
-    }
-}