Merge pull request #438 from christeredvartsen/issue-437

Fix JSON-encoding issues from metadata fetched from the new Mongo adapter
imbo · Mar 11, 2016 · 6158803 · 6158803
2 parents 99f3a66 + 22eb187
commit 6158803
Show file tree

Hide file tree

Showing 6 changed files with 310 additions and 24 deletions.
diff --git a/ChangeLog.markdown b/ChangeLog.markdown
@@ -1,9 +1,15 @@
 Changelog for Imbo
 ==================
 
-Imbo-2.x.x
+Imbo-2.1.1
 ----------
-__N/A__
+__2016-03-11__
+
+* #437: Fixed issue with metadata not being formatted correctly using the new Mongo adapters (Christer Edvartsen)
+
+Imbo-2.1.0
+----------
+__2016-03-10__
 
 * #436: Added new MongoDB adapters that use the mongodb extension (Christer Edvartsen)
 * #434: Custom models for group(s) and access rule(s) (Christer Edvartsen)

diff --git a/library/Imbo/Auth/AccessControl/Adapter/Mongo.php b/library/Imbo/Auth/AccessControl/Adapter/Mongo.php
@@ -13,6 +13,7 @@
 use Imbo\Exception\DatabaseException,
     Imbo\Auth\AccessControl\GroupQuery,
     Imbo\Model\Groups as GroupsModel,
+    Imbo\Helpers\BSONToArray,
     MongoDB\Client as MongoClient,
     MongoDB\Collection as MongoCollection,
     MongoDB\Driver\Exception\Exception as MongoException,
@@ -83,15 +84,23 @@ class Mongo extends AbstractAdapter implements MutableAdapterInterface {
         'options' => ['connect' => true, 'connectTimeoutMS' => 1000],
     ];
 
+    /**
+     * BSONToArray helper
+     *
+     * @var BSONToArray
+     */
+    private $bsonToArray;
+
     /**
      * Class constructor
      *
      * @param array $params Parameters for the driver
      * @param MongoClient $client MongoClient instance
      * @param MongoCollection $aclCollection MongoCollection instance for the acl collection
      * @param MongoCollection $aclGrouplection MongoCollection instance for the acl group collection
+     * @param BSONToArray $bsonToArray BSONToArray helper
      */
-    public function __construct(array $params = null, MongoClient $client = null, MongoCollection $aclCollection = null, MongoCollection $aclGroupCollection = null) {
+    public function __construct(array $params = null, MongoClient $client = null, MongoCollection $aclCollection = null, MongoCollection $aclGroupCollection = null, BSONToArray $bsonToArray = null) {
         if ($params !== null) {
             $this->params = array_replace_recursive($this->params, $params);
         }
@@ -107,6 +116,12 @@ public function __construct(array $params = null, MongoClient $client = null, Mo
         if ($aclGroupCollection !== null) {
             $this->aclGroupCollection = $aclGroupCollection;
         }
+
+        if ($bsonToArray === null) {
+            $bsonToArray = new BSONToArray();
+        }
+
+        $this->bsonToArray = $bsonToArray;
     }
 
     /**
@@ -399,18 +414,7 @@ private function getPublicKeyDetails($publicKey) {
             return [];
         }
 
-        $data = $pubkeyInfo->getArrayCopy();
-        $acl = [];
-
-        foreach ($data['acl'] as $rule) {
-            $rule = $rule->getArrayCopy();
-            $rule['resources'] = $rule['resources']->getArrayCopy();
-            $rule['users'] = $rule['users']->getArrayCopy();
-
-            $acl[] = $rule;
-        }
-
-        $data['acl'] = $acl;
+        $data = $this->bsonToArray->toArray($pubkeyInfo->getArrayCopy());
 
         $this->publicKeys[$publicKey] = $data;
 

diff --git a/library/Imbo/Database/Mongo.php b/library/Imbo/Database/Mongo.php
@@ -14,6 +14,7 @@
     Imbo\Model\Images,
     Imbo\Resource\Images\Query,
     Imbo\Exception\DatabaseException,
+    Imbo\Helpers\BSONToArray,
     MongoDB\Client as MongoClient,
     MongoDB\Driver\Manager,
     MongoDB\Driver\Command,
@@ -70,15 +71,23 @@ class Mongo implements DatabaseInterface {
         'options' => ['connect' => true, 'connectTimeoutMS' => 1000],
     ];
 
+    /**
+     * BSONToArray helper
+     *
+     * @var BSONToArray
+     */
+    private $bsonToArray;
+
     /**
      * Class constructor
      *
      * @param array $params Parameters for the driver
      * @param MongoClient $client MongoClient instance
      * @param MongoCollection $imageCollection MongoCollection instance for the images
      * @param MongoCollection $shortUrlCollection MongoCollection instance for the short URLs
+     * @param BSONToArray $bsonToArray Helper to recursively convert documents to arrays
      */
-    public function __construct(array $params = null, MongoClient $client = null, MongoCollection $imageCollection = null, MongoCollection $shortUrlCollection = null) {
+    public function __construct(array $params = null, MongoClient $client = null, MongoCollection $imageCollection = null, MongoCollection $shortUrlCollection = null, BSONToArray $bsonToArray = null) {
         if ($params !== null) {
             $this->params = array_replace_recursive($this->params, $params);
         }
@@ -94,6 +103,12 @@ public function __construct(array $params = null, MongoClient $client = null, Mo
         if ($shortUrlCollection !== null) {
             $this->collections['shortUrl'] = $shortUrlCollection;
         }
+
+        if ($bsonToArray === null) {
+            $bsonToArray = new BSONToArray();
+        }
+
+        $this->bsonToArray = $bsonToArray;
     }
 
     /**
@@ -209,7 +224,7 @@ public function getMetadata($user, $imageIdentifier) {
             throw new DatabaseException('Image not found', 404);
         }
 
-        return $data['metadata']->getArrayCopy();
+        return $this->bsonToArray->toArray($data['metadata']->getArrayCopy());
     }
 
     /**
@@ -322,13 +337,7 @@ public function getImages(array $users, Query $query, Images $model) {
                 unset($image['_id']);
                 $image['added'] = new DateTime('@' . $image['added'], new DateTimeZone('UTC'));
                 $image['updated'] = new DateTime('@' . $image['updated'], new DateTimeZone('UTC'));
-
-                if (isset($image['metadata'])) {
-                    $metadata = $image['metadata']->getArrayCopy();
-                    $image->offsetSet('metadata', $metadata);
-                }
-
-                $images[] = $image->getArrayCopy();
+                $images[] = $this->bsonToArray->toArray($image->getArrayCopy());
             }
 
             // Update model

diff --git a/library/Imbo/Helpers/BSONToArray.php b/library/Imbo/Helpers/BSONToArray.php
@@ -0,0 +1,62 @@
+<?php
+/**
+ * This file is part of the Imbo package
+ *
+ * (c) Christer Edvartsen <cogo@starzinger.net>
+ *
+ * For the full copyright and license information, please view the LICENSE file that was
+ * distributed with this source code.
+ */
+
+namespace Imbo\Helpers;
+
+use MongoDB\Model\BSONDocument,
+    MongoDB\Model\BSONArray;
+
+/**
+ * Convert BSONDocument and BSONArray to their array counterparts, recursively
+ *
+ * @author Christer Edvartsen <cogo@starzinger.net>
+ * @package Core\Helpers
+ */
+class BSONToArray {
+    /**
+     * Convert to array, recursively
+     *
+     * @param array|BSONDocument|BSONArray $document
+     * @return mixed
+     */
+    public function toArray($document) {
+        if ($this->isBSONModel($document)) {
+            // If the document is a BSON model, get the array copy first
+            $document = $document->getArrayCopy();
+        } else if (!is_array($document)) {
+            // The variable to convert is not an array, simply return it as-is
+            return $document;
+        }
+
+        $result = [];
+
+        foreach ($document as $key => $value) {
+            if ($this->isBSONModel($value)) {
+                // The value is another model, convert it as well
+                $value = $this->toArray($value->getArrayCopy());
+            }
+
+            // Regular value, set it
+            $result[$key] = $value;
+        }
+
+        return $result;
+    }
+
+    /**
+     * Check if the value is a valid BSON model
+     *
+     * @param mixed $value
+     * @return boolean
+     */
+    private function isBSONModel($value) {
+        return ($value instanceof BSONDocument || $value instanceof BSONArray);
+    }
+}
diff --git a/tests/phpunit/ImboIntegrationTest/Database/DatabaseTests.php b/tests/phpunit/ImboIntegrationTest/Database/DatabaseTests.php
@@ -204,6 +204,89 @@ public function testUpdateAndGetMetadata() {
         $this->assertSame(['foo' => 'foo', 'bar' => 'foo'], $this->adapter->getMetadata($user, $imageIdentifier));
     }
 
+    public function testMetadataWithNestedArraysIsRepresetedCorrectly() {
+        if (get_class($this->adapter) === 'Imbo\Database\Doctrine') {
+            $this->markTestSkipped('Skipped for the Doctrine adapter as Doctrine can\'t handle types');
+        }
+
+        $metadata = [
+            'string' => 'bar',
+            'integer' => 1,
+            'float' => 1.1,
+            'boolean' => true,
+            'list' => [1, 2, 3],
+            'assoc' => [
+                'string' => 'bar',
+                'integer' => 1,
+                'float' => 1.1,
+                'boolean' => false,
+                'list' => [1, 2, 3],
+                'assoc' => [
+                    'list' => [
+                        1,
+                        2, [
+                            'list' => [1, 2, 3]
+                        ],
+                        [1, 2, 3],
+                    ],
+                ],
+            ],
+        ];
+
+        $user = 'user';
+        $imageIdentifier = 'id';
+
+        $this->assertTrue($this->adapter->insertImage($user, $imageIdentifier, $this->getImage()));
+        $this->assertTrue($this->adapter->updateMetadata($user, $imageIdentifier, $metadata));
+
+        $this->assertSame($metadata, $this->adapter->getMetadata($user, $imageIdentifier));
+    }
+
+    public function testMetadataWithNestedArraysIsRepresetedCorrectlyWhenFetchingMultipleImages() {
+        if (get_class($this->adapter) === 'Imbo\Database\Doctrine') {
+            $this->markTestSkipped('Skipped for the Doctrine adapter as Doctrine can\'t handle types');
+        }
+
+        $metadata = [
+            'string' => 'bar',
+            'integer' => 1,
+            'float' => 1.1,
+            'boolean' => true,
+            'list' => [1, 2, 3],
+            'assoc' => [
+                'string' => 'bar',
+                'integer' => 1,
+                'float' => 1.1,
+                'boolean' => false,
+                'list' => [1, 2, 3],
+                'assoc' => [
+                    'list' => [
+                        1,
+                        2, [
+                            'list' => [1, 2, 3]
+                        ],
+                        [1, 2, 3],
+                    ],
+                ],
+            ],
+        ];
+
+        $user = 'user';
+        $imageIdentifier = 'id';
+
+        $this->assertTrue($this->adapter->insertImage($user, $imageIdentifier, $this->getImage()));
+        $this->assertTrue($this->adapter->updateMetadata($user, $imageIdentifier, $metadata));
+
+        $query = new Query();
+        $query->returnMetadata(true);
+
+        $images = $this->adapter->getImages(['user'], $query, new Images());
+
+        $this->assertCount(1, $images);
+
+        $this->assertSame($metadata, $images[0]['metadata']);
+    }
+
     public function testUpdateDeleteAndGetMetadata() {
         $user = 'user';
         $imageIdentifier = 'id';