Merge pull request #157 from alcaeus/projection-with-numeric-keys

Fix query projections with numeric arrays

CHANGELOG-1.0.md

@@ -3,13 +3,15 @@ CHANGELOG
 
 This changelog references the relevant changes done in minor version updates.
 
-1.0.9 (????-??-??)
+1.0.9 (2017-01-29)
 ------------------
 
 All issues and pull requests under this release may be found under the
 [1.0.9](https://github.com/alcaeus/mongo-php-adapter/issues?q=milestone%3A1.0.9)
 milestone.
 
+ * [#157](https://github.com/alcaeus/mongo-php-adapter/pull/157) fixes a regression
+ introduced in 1.0.8 when using query projection with numeric field names.
  * [#155](https://github.com/alcaeus/mongo-php-adapter/pull/155) fixes the handling
  of BSON types when converting legacy types to BSON types.
  * [#154](https://github.com/alcaeus/mongo-php-adapter/pull/154) makes the `options`

lib/Alcaeus/MongoDbAdapter/TypeConverter.php

@@ -98,6 +98,8 @@ class TypeConverter
      *
      * @param array $fields
      * @return array
+     *
+     * @throws \MongoException
      */
     public static function convertProjection($fields)
     {
@@ -105,7 +107,21 @@ class TypeConverter
             return [];
         }
 
-        $projection = TypeConverter::isNumericArray($fields) ? array_fill_keys($fields, true) : $fields;
+        if (! TypeConverter::isNumericArray($fields)) {
+            $projection = TypeConverter::fromLegacy($fields);
+        } else {
+            $projection = array_fill_keys(
+                array_map(function ($field) {
+                    if (!is_string($field)) {
+                        throw new \MongoException('field names must be strings', 8);
+                    }
+
+                    return $field;
+                }, $fields),
+                true
+            );
+        }
+
         return TypeConverter::fromLegacy($projection);
     }
 
@@ -122,7 +138,14 @@ class TypeConverter
      */
     public static function isNumericArray(array $array)
     {
-        return $array === [] || is_numeric(array_keys($array)[0]);
+        if ($array === []) {
+            return true;
+        }
+
+        $keys = array_keys($array);
+        // array_keys gives us a clean numeric array with keys, so we expect an
+        // array like [0 => 0, 1 => 1, 2 => 2, ..., n => n]
+        return array_values($keys) === array_keys($keys);
     }
 
     /**

tests/Alcaeus/MongoDbAdapter/Mongo/MongoCollectionTest.php

@@ -478,6 +478,40 @@ class MongoCollectionTest extends TestCase
     }
 
     /**
+     * @dataProvider dataFindWithProjectionAndNumericKeys
+     */
+    public function testFindWithProjectionAndNumericKeys($data, $projection, $expected)
+    {
+        $this->getCollection()->insert($data);
+
+        $document = $this->getCollection()->findOne([], $projection);
+        unset($document['_id']);
+        $this->assertSame($expected, $document);
+    }
+
+    public static function dataFindWithProjectionAndNumericKeys()
+    {
+        return [
+            'sequentialIntegersStartingWithOne' => [
+                ['0' => 'foo', '1' => 'bar', '2' => 'foobar'],
+                [1 => true, 2 => true],
+                ['1' => 'bar', '2' => 'foobar'],
+            ],
+            'nonSequentialIntegers' => [
+                ['0' => 'foo', '1' => 'bar', '2' => 'foobar', '3' => 'barfoo'],
+                [1 => true, 3 => true],
+                ['1' => 'bar', '3' => 'barfoo'],
+            ]
+        ];
+    }
+
+    public function testFindWithProjectionAndSequentialNumericKeys()
+    {
+        $this->setExpectedException(\MongoException::class, 'field names must be strings', 8);
+        $this->getCollection()->findOne([], [true, false]);
+    }
+
+    /**
      * @dataProvider dataFindWithProjectionExcludeId
      */
     public function testFindWithProjectionExcludeId($projection)

tests/Alcaeus/MongoDbAdapter/TypeConverterTest.php

@@ -40,4 +40,23 @@ class TypeConverterTest extends TestCase
             ],
         ];
     }
+
+    /**
+     * @dataProvider dataIsNumericArray
+     */
+    public function testIsNumericArray($expected, $array)
+    {
+        $this->assertSame($expected, TypeConverter::isNumericArray($array));
+    }
+
+    public static function dataIsNumericArray()
+    {
+        return [
+            'emptyArray' => [true, []],
+            'arrayWithSequentialIndices' => [true, [1, 2, 3]],
+            'arrayWithSequentialIndicesOneBased' => [false, [1 => 1, 2, 3]],
+            'arrayWithStringKeys' => [false, ['foo' => 'bar']],
+            'arrayWithRandomNumbers' => [false, [15 => 'foo', 20 => 'bar']],
+        ];
+    }
 }