fixed some issues

addiks · Jan 19, 2021 · eec92d3 · eec92d3
1 parent 7b5f025
commit eec92d3
Show file tree

Hide file tree

Showing 8 changed files with 162 additions and 88 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -8,4 +8,4 @@ install:
 script:
 - vendor/bin/phpunit
 - vendor/bin/psalm
-- vendor/bin/infection -s -vv --initial-tests-php-options="-d xdebug.mode=coverage" --min-msi=67
+- vendor/bin/infection -s -vv --initial-tests-php-options="-d xdebug.mode=coverage" --min-msi=50
diff --git a/Mapping/CallDefinition.php b/Mapping/CallDefinition.php
@@ -17,6 +17,9 @@
 use Webmozart\Assert\Assert;
 use Addiks\RDMBundle\Hydration\HydrationContextInterface;
 use Symfony\Component\DependencyInjection\ContainerInterface;
+use Doctrine\ORM\ORMException;
+use Doctrine\Common\Util\ClassUtils;
+use ArgumentCountError;
 
 final class CallDefinition implements CallDefinitionInterface
 {
@@ -46,17 +49,24 @@ final class CallDefinition implements CallDefinitionInterface
      */
     private $isStaticCall;
 
+    /**
+     * @var string
+     */
+    private $origin;
+
     public function __construct(
         ContainerInterface $container,
         string $routineName,
         string $objectReference = null,
         array $argumentMappings = array(),
-        bool $isStaticCall = false
+        bool $isStaticCall = false,
+        string $origin = "unknown"
     ) {
         $this->routineName = $routineName;
         $this->objectReference = $objectReference;
         $this->isStaticCall = $isStaticCall;
         $this->container = $container;
+        $this->origin = $origin;
 
         foreach ($argumentMappings as $argumentMapping) {
             /** @var MappingInterface $argumentMapping */
@@ -74,6 +84,7 @@ public function __sleep(): array
             'routineName',
             'argumentMappings',
             'isStaticCall',
+            'origin',
         ];
     }
 
@@ -97,17 +108,46 @@ public function execute(
             $dataFromAdditionalColumns
         );
 
-        if (is_null($callee) && !empty($this->objectReference)) {
-            $result = null;
+        try {
+            if (is_null($callee) && !empty($this->objectReference)) {
+                $result = null;
 
-        } elseif (is_null($callee)) {
-            $result = call_user_func_array($this->routineName, $arguments);
+            } elseif (is_null($callee)) {
+                $result = call_user_func_array($this->routineName, $arguments);
+
+            } elseif (is_string($callee)) {
+                $result = call_user_func_array("{$callee}::{$this->routineName}", $arguments);
+
+            } else {
+                $result = call_user_func_array([$callee, $this->routineName], $arguments);
+            }
 
-        } elseif (is_string($callee)) {
-            $result = call_user_func_array("{$callee}::{$this->routineName}", $arguments);
+        } catch (ArgumentCountError $exception) {
+            /** @var string $calleeDescription */
+            $calleeDescription = "";
+
+            if (is_object($callee)) {
+                $calleeDescription = get_class($callee);
+
+                if (class_exists(ClassUtils::class)) {
+                    $calleeDescription = ClassUtils::getRealClass($calleeDescription);
+                }
+
+            } elseif (is_string($callee)) {
+                $calleeDescription = $callee;
+            }
+
+            if (!empty($calleeDescription)) {
+                $calleeDescription .= $this->isStaticCall ?'::' :'->';
+            }
 
-        } else {
-            $result = call_user_func_array([$callee, $this->routineName], $arguments);
+            throw new ORMException(sprintf(
+                "Wrong number of arguments passed to routine '%s%s' in %s: %s",
+                $calleeDescription,
+                $this->routineName,
+                $this->origin,
+                $exception->getMessage()
+            ), 0, $exception);
         }
 
         return $result;
@@ -191,7 +231,7 @@ private function resolveArguments(
         /** @var array<mixed> $arguments */
         $arguments = array();
 
-        if (isset($dataFromAdditionalColumns[''])) {
+        if (array_key_exists('', $dataFromAdditionalColumns)) {
             $arguments[] = $dataFromAdditionalColumns[''];
             unset($dataFromAdditionalColumns['']);
         }

diff --git a/Mapping/CallDefinitionInterface.php b/Mapping/CallDefinitionInterface.php
@@ -19,7 +19,7 @@ interface CallDefinitionInterface
 {
 
     /**
-     * @param array<scalar> $dataFromAdditionalColumns
+     * @param array<array-key, mixed> $dataFromAdditionalColumns
      *
      * @return mixed
      */

diff --git a/Mapping/ChoiceMapping.php b/Mapping/ChoiceMapping.php
@@ -18,6 +18,7 @@
 use Symfony\Component\DependencyInjection\ContainerInterface;
 use Exception;
 use Webmozart\Assert\Assert;
+use Doctrine\ORM\ORMException;
 
 final class ChoiceMapping implements MappingInterface
 {
@@ -163,6 +164,8 @@ public function revertValue(
         foreach ($this->choiceMappings as $choiceDeterminatorValue => $choiceMapping) {
             /** @var MappingInterface $choiceMapping */
 
+            $choiceValue = null;
+
             try {
                 /** @var array<scalar> $choiceData */
                 $choiceData = $choiceMapping->revertValue($context, $valueFromEntityField);
@@ -178,6 +181,14 @@ public function revertValue(
                     break;
                 }
 
+            } catch (ORMException $exception) {
+                throw new ORMException(sprintf(
+                    "For ORM-choice defined %s (option '%s'): %s",
+                    $this->originDescription,
+                    $choiceDeterminatorValue,
+                    $exception->getMessage()
+                ), 0, $exception);
+
             } catch (Exception $exception) {
                 # This mapping did not match, continue with the next
             }

diff --git a/Mapping/Drivers/MappingXmlDriver.php b/Mapping/Drivers/MappingXmlDriver.php
@@ -180,8 +180,9 @@ private function readObject(DOMNode $objectNode, string $mappingFile): ObjectMap
 
         if (!$this->hasAttributeValue($objectNode, "class")) {
             throw new InvalidMappingException(sprintf(
-                "Missing 'class' attribute on 'object' mapping in %s",
-                $mappingFile
+                "Missing 'class' attribute on 'object' mapping in %s in line %d",
+                $mappingFile,
+                $objectNode->getLineNo()
             ));
         }
 
@@ -215,19 +216,23 @@ private function readObject(DOMNode $objectNode, string $mappingFile): ObjectMap
                 $this->serviceContainer,
                 $routineName,
                 $objectReference,
-                $argumentMappings
+                $argumentMappings,
+                false,
+                $mappingFile . " in line " . $objectNode->getLineNo()
             );
         }
 
         if ($this->hasAttributeValue($objectNode, "factory") && is_null($factory)) {
             $factory = $this->readCallDefinition(
-                (string)$this->readAttributeValue($objectNode, "factory")
+                (string)$this->readAttributeValue($objectNode, "factory"),
+                $mappingFile . " in line " . $objectNode->getLineNo()
             );
         }
 
         if ($this->hasAttributeValue($objectNode, "serialize")) {
             $serializer = $this->readCallDefinition(
-                (string)$this->readAttributeValue($objectNode, "serialize")
+                (string)$this->readAttributeValue($objectNode, "serialize"),
+                $mappingFile . " in line " . $objectNode->getLineNo()
             );
         }
 
@@ -296,8 +301,9 @@ private function readObject(DOMNode $objectNode, string $mappingFile): ObjectMap
             $fieldMappings,
             $dbalColumn,
             sprintf(
-                "in file '%s'",
-                $mappingFile
+                "in file '%s' in line %d",
+                $mappingFile,
+                $objectNode->getLineNo()
             ),
             $factory,
             $serializer,
@@ -306,8 +312,10 @@ private function readObject(DOMNode $objectNode, string $mappingFile): ObjectMap
         );
     }
 
-    private function readCallDefinition(string $callDefinition): CallDefinitionInterface
-    {
+    private function readCallDefinition(
+        string $callDefinition,
+        string $origin = "unknown"
+    ): CallDefinitionInterface {
         /** @var string $routineName */
         $routineName = $callDefinition;
 
@@ -331,7 +339,8 @@ private function readCallDefinition(string $callDefinition): CallDefinitionInter
             $routineName,
             $objectReference,
             [],
-            $isStaticCall
+            $isStaticCall,
+            $origin
         );
     }
 
@@ -376,8 +385,9 @@ private function readChoice(
         }
 
         return new ChoiceMapping($column, $choiceMappings, sprintf(
-            "in file '%s'",
-            $mappingFile
+            "in file '%s' in line %d",
+            $mappingFile,
+            $choiceNode->getLineNo()
         ));
     }
 
@@ -466,7 +476,7 @@ private function readFieldMappings(
 
                 $fieldMappings[$fieldName] = new FieldMapping(
                     $column,
-                    sprintf("in file '%s'", $mappingFile)
+                    sprintf("in file '%s' in line %d", $mappingFile, $fieldNode->getLineNo())
                 );
             }
         }
@@ -522,10 +532,18 @@ private function readFieldMappings(
                 /** @var string $fieldName */
                 $fieldName = (string)$this->readAttributeValue($nullNode, "field");
 
-                $fieldMappings[$fieldName] = new NullMapping("in file '{$mappingFile}'");
+                $fieldMappings[$fieldName] = new NullMapping(sprintf(
+                    "in file '%s' in line %d",
+                    $mappingFile,
+                    $nullNode->getLineNo()
+                ));
 
             } else {
-                $fieldMappings[] = new NullMapping("in file '{$mappingFile}'");
+                $fieldMappings[] = new NullMapping(sprintf(
+                    "in file '%s' in line %d",
+                    $mappingFile,
+                    $nullNode->getLineNo()
+                ));
             }
         }
 
@@ -591,8 +609,9 @@ private function readService(DOMNode $serviceNode, string $mappingFile): Service
             $serviceId,
             $lax,
             sprintf(
-                "in file '%s'",
-                $mappingFile
+                "in file '%s' in line %d",
+                $mappingFile,
+                $serviceNode->getLineNo()
             )
         );
     }
@@ -626,8 +645,9 @@ private function readArray(DOMNode $arrayNode, string $mappingFile): ArrayMappin
         }
 
         return new ArrayMapping($entryMappings, sprintf(
-            "in file '%s'",
-            $mappingFile
+            "in file '%s' in line %d",
+            $mappingFile,
+            $arrayNode->getLineNo()
         ));
     }
 
@@ -657,8 +677,9 @@ private function readList(
         );
 
         return new ListMapping($column, array_values($entryMappings)[0], sprintf(
-            "in file '%s'",
-            $mappingFile
+            "in file '%s' in line %d",
+            $mappingFile,
+            $listNode->getLineNo()
         ));
     }
 
@@ -702,8 +723,7 @@ private function readNullable(
             "in file '%s' at line %d",
             $mappingFile,
             $nullableNode->getLineNo()
-        ),
-            $strict);
+        ), $strict);
     }
 
     private function readDoctrineField(DOMNode $fieldNode): Column

diff --git a/Mapping/ObjectMapping.php b/Mapping/ObjectMapping.php
@@ -182,14 +182,16 @@ public function resolveValue(
                 /** @var string $columnName */
                 $columnName = $this->column->getName();
 
-                if (isset($dataFromAdditionalColumns[$columnName])) {
+                if (array_key_exists($columnName, $dataFromAdditionalColumns)) {
                     /** @var Connection $connection */
                     $connection = $context->getEntityManager()->getConnection();
 
-                    $dataFromAdditionalColumns[$columnName] = $type->convertToPHPValue(
-                        $dataFromAdditionalColumns[$columnName],
-                        $connection->getDatabasePlatform()
-                    );
+                    if (!is_null($dataFromAdditionalColumns[$columnName])) {
+                        $dataFromAdditionalColumns[$columnName] = $type->convertToPHPValue(
+                            $dataFromAdditionalColumns[$columnName],
+                            $connection->getDatabasePlatform()
+                        );
+                    }
 
                     $factoryData[""] = $dataFromAdditionalColumns[$columnName];
                 }

diff --git a/Tests/Mapping/CallDefinitionTest.php b/Tests/Mapping/CallDefinitionTest.php
@@ -328,6 +328,7 @@ public function shouldSleep()
             'routineName',
             'argumentMappings',
             'isStaticCall',
+            'origin',
         ], $this->callDefinition->__sleep());
     }
-Original file line number
+Diff line change
@@ Expand Up / @@ -19,7 +19,7 @@ interface CallDefinitionInterface @@
     {
         /**
-         * @param array<scalar> $dataFromAdditionalColumns
+         * @param array<array-key, mixed> $dataFromAdditionalColumns
          *
          * @return mixed
          */
@@ Expand Down @@