From 3608b73a1237715d75030137fe11dfc4682ff724 Mon Sep 17 00:00:00 2001
From: Javier Spagnoletti <phansys@gmail.com>
Date: Thu, 9 Apr 2020 16:39:32 -0300
Subject: [PATCH] Fix some API violations

---
 src/Filter/AbstractDateFilter.php   | 18 ++++++++++----
 src/Guesser/AbstractTypeGuesser.php |  4 +--
 src/Model/ModelManager.php          | 18 ++++++++------
 tests/Model/ModelManagerTest.php    | 38 ++++++++++++++++++++++++++++-
 4 files changed, 63 insertions(+), 15 deletions(-)

diff --git a/src/Filter/AbstractDateFilter.php b/src/Filter/AbstractDateFilter.php
index a39c6812..b63d8483 100644
--- a/src/Filter/AbstractDateFilter.php
+++ b/src/Filter/AbstractDateFilter.php
@@ -55,29 +55,37 @@ public function filter(ProxyQueryInterface $queryBuilder, $alias, $field, $data)
 
         switch ($data['type']) {
             case DateType::TYPE_EQUAL:
-                return $this->applyTypeIsEqual($queryBuilder, $field, $data);
+                $this->applyTypeIsEqual($queryBuilder, $field, $data);
+
+                return;
 
             case DateType::TYPE_GREATER_THAN:
                 if (!\array_key_exists('value', $data) || !$data['value']) {
                     return;
                 }
 
-                return $this->applyTypeIsGreaterThan($queryBuilder, $field, $data);
+                $this->applyTypeIsGreaterThan($queryBuilder, $field, $data);
+
+                return;
 
             case DateType::TYPE_LESS_EQUAL:
                 if (!\array_key_exists('value', $data) || !$data['value']) {
                     return;
                 }
 
-                return $this->applyTypeIsLessEqual($queryBuilder, $field, $data);
+                $this->applyTypeIsLessEqual($queryBuilder, $field, $data);
+
+                return;
 
             case DateType::TYPE_NULL:
             case DateType::TYPE_NOT_NULL:
-                return $this->applyType($queryBuilder, $this->getOperator($data['type']), $field, null);
+                $this->applyType($queryBuilder, $this->getOperator($data['type']), $field, null);
+
+                return;
 
             case DateType::TYPE_GREATER_EQUAL:
             case DateType::TYPE_LESS_THAN:
-                return $this->applyType($queryBuilder, $this->getOperator($data['type']), $field, $data['value']);
+                $this->applyType($queryBuilder, $this->getOperator($data['type']), $field, $data['value']);
         }
     }
 
diff --git a/src/Guesser/AbstractTypeGuesser.php b/src/Guesser/AbstractTypeGuesser.php
index 98d1e809..ecd37106 100644
--- a/src/Guesser/AbstractTypeGuesser.php
+++ b/src/Guesser/AbstractTypeGuesser.php
@@ -30,8 +30,8 @@ protected function getParentMetadataForProperty($baseClass, $propertyFullName, M
         try {
             return $modelManager->getParentMetadataForProperty($baseClass, $propertyFullName);
         } catch (MappingException $e) {
-            // no metadata not found.
-            return;
+            // no metadata found.
+            return null;
         }
     }
 }
diff --git a/src/Model/ModelManager.php b/src/Model/ModelManager.php
index b09a96f3..739948e4 100755
--- a/src/Model/ModelManager.php
+++ b/src/Model/ModelManager.php
@@ -152,7 +152,7 @@ public function delete($object)
     public function find($class, $id)
     {
         if (!isset($id)) {
-            return;
+            return null;
         }
 
         $documentManager = $this->getDocumentManager($class);
@@ -185,7 +185,7 @@ public function findOneBy($class, array $criteria = [])
     }
 
     /**
-     * @param string $class
+     * @param object|string $class
      *
      * @throw \RuntimeException
      *
@@ -275,13 +275,17 @@ public function getIdentifierFieldNames($class)
      */
     public function getNormalizedIdentifier($document)
     {
-        if (is_scalar($document)) {
+        if (null === $document) {
+            return null;
+        }
+
+        if (!\is_object($document)) {
             throw new \RunTimeException('Invalid argument, object or null required');
         }
 
         // the document is not managed
-        if (!$document || !$this->getDocumentManager($document)->getUnitOfWork()->isInIdentityMap($document)) {
-            return;
+        if (!$this->getDocumentManager($document)->getUnitOfWork()->isInIdentityMap($document)) {
+            return null;
         }
 
         $values = $this->getIdentifierValues($document);
@@ -292,9 +296,9 @@ public function getNormalizedIdentifier($document)
     /**
      * {@inheritdoc}
      */
-    public function getUrlsafeIdentifier($entity)
+    public function getUrlSafeIdentifier($document)
     {
-        return $this->getNormalizedIdentifier($entity);
+        return $this->getNormalizedIdentifier($document);
     }
 
     /**
diff --git a/tests/Model/ModelManagerTest.php b/tests/Model/ModelManagerTest.php
index 86903007..a71f62bb 100644
--- a/tests/Model/ModelManagerTest.php
+++ b/tests/Model/ModelManagerTest.php
@@ -23,6 +23,42 @@ public function testFilterEmpty(): void
     {
         $registry = $this->createMock(ManagerRegistry::class);
 
-        $manager = new ModelManager($registry);
+        new ModelManager($registry);
+    }
+
+    /**
+     * @dataProvider getWrongDocuments
+     *
+     * @param mixed $document
+     */
+    public function testNormalizedIdentifierException($document): void
+    {
+        $registry = $this->createMock(ManagerRegistry::class);
+
+        $model = new ModelManager($registry);
+
+        $this->expectException(\RuntimeException::class);
+
+        $model->getNormalizedIdentifier($document);
+    }
+
+    public function getWrongDocuments(): iterable
+    {
+        yield [0];
+        yield [1];
+        yield [false];
+        yield [true];
+        yield [[]];
+        yield [''];
+        yield ['sonata-project'];
+    }
+
+    public function testGetNormalizedIdentifierNull(): void
+    {
+        $registry = $this->createMock(ManagerRegistry::class);
+
+        $model = new ModelManager($registry);
+
+        $this->assertNull($model->getNormalizedIdentifier(null));
     }
 }