Merge pull request #229 from Seldaek/detect-bad-groups

Detect malformed group names

Metadata/Driver/AnnotationDriver.php

@@ -55,6 +55,7 @@ use JMS\SerializerBundle\Annotation\ReadOnly;
 use JMS\SerializerBundle\Metadata\ClassMetadata;
 use JMS\SerializerBundle\Metadata\PropertyMetadata;
 use JMS\SerializerBundle\Metadata\VirtualPropertyMetadata;
+use JMS\SerializerBundle\Exception\InvalidArgumentException;
 use JMS\SerializerBundle\Annotation\XmlAttributeMap;
 use Metadata\Driver\DriverInterface;
 
@@ -175,6 +176,15 @@ class AnnotationDriver implements DriverInterface
                         $accessor = array($annot->getter, $annot->setter);
                     } else if ($annot instanceof Groups) {
                         $propertyMetadata->groups = $annot->groups;
+                        foreach ((array) $propertyMetadata->groups as $groupName) {
+                            if (false !== strpos($groupName, ',')) {
+                                throw new InvalidArgumentException(sprintf(
+                                    'Invalid group name "%s" on "%s", did you mean to create multiple groups?',
+                                    implode(', ', $propertyMetadata->groups),
+                                    $propertyMetadata->class.'->'.$propertyMetadata->name
+                                ));
+                            }
+                        }
                     } else if ($annot instanceof Inline) {
                         $propertyMetadata->inline = true;
                     } else if ($annot instanceof XmlAttributeMap) {

Tests/Fixtures/InvalidGroupsObject.php

@@ -0,0 +1,31 @@
+<?php
+
+/*
+ * Copyright 2011 Johannes M. Schmitt <schmittjoh@gmail.com>
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+namespace JMS\SerializerBundle\Tests\Fixtures;
+
+use JMS\SerializerBundle\Annotation\Groups;
+use JMS\SerializerBundle\Annotation\Type;
+
+class InvalidGroupsObject
+{
+    /**
+     * @Groups({"foo, bar"})
+     * @Type("string")
+     */
+    private $foo;
+}

Tests/Serializer/BaseSerializationTest.php

@@ -58,6 +58,7 @@ use JMS\SerializerBundle\Tests\Fixtures\CurrencyAwarePrice;
 use JMS\SerializerBundle\Tests\Fixtures\CustomDeserializationObject;
 use JMS\SerializerBundle\Tests\Fixtures\GetSetObject;
 use JMS\SerializerBundle\Tests\Fixtures\GroupsObject;
+use JMS\SerializerBundle\Tests\Fixtures\InvalidGroupsObject;
 use JMS\SerializerBundle\Tests\Fixtures\IndexedCommentsBlogPost;
 use JMS\SerializerBundle\Tests\Fixtures\InlineParent;
 use JMS\SerializerBundle\Tests\Fixtures\Log;
@@ -491,6 +492,17 @@ abstract class BaseSerializationTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals($this->getContent('groups_default'), $this->serializer->serialize($groupsObject, $this->getFormat()));
     }
 
+    /**
+     * @expectedException JMS\SerializerBundle\Exception\InvalidArgumentException
+     * @expectedExceptionMessage Invalid group name "foo, bar" on "JMS\SerializerBundle\Tests\Fixtures\InvalidGroupsObject->foo", did you mean to create multiple groups?
+     */
+    public function testInvalidGroupName()
+    {
+        $groupsObject = new InvalidGroupsObject();
+
+        $this->serializer->serialize($groupsObject, $this->getFormat());
+    }
+
     public function testVirtualProperty()
     {
         $this->assertEquals($this->getContent('virtual_properties'), $this->serialize(new ObjectWithVirtualProperties()));