MappingFactory: validate property names

This complicates MappingFactory and adds dependence to weird place, but since this lib is aimed at practicality, I believe slightly ignoring invalid properties must not be allowed.
Mikulas · Nov 29, 2015 · 21f5e13 · 21f5e13
1 parent c5d3190
commit 21f5e13
Show file tree

Hide file tree

Showing 7 changed files with 163 additions and 21 deletions.
diff --git a/doc/index.md b/doc/index.md
@@ -30,7 +30,7 @@ class NotesMapper extends Mapper
 
 	protected function createStorageReflection()
 	{
-		$factory = new MappingFactory(parent::createStorageReflection());
+		$factory = new MappingFactory(parent::createStorageReflection(), $this->getRepository()->getEntityMetadata());
 		$factory->addJsonMapping('content');
 
 		return $factory->getReflection();
@@ -99,7 +99,7 @@ class UsersMapper extends Mapper
 
 	protected function createStorageReflection()
 	{
-		$factory = new MappingFactory(parent::createStorageReflection());
+		$factory = new MappingFactory(parent::createStorageReflection(), $this->getRepository()->getEntityMetadata());
 		$factory->addCryptoMapping('email', $this->crypto);
 
 		return $factory->getReflection();
@@ -149,7 +149,7 @@ class BooksMapper extends Mapper
 
 	protected function createStorageReflection()
 	{
-		$factory = new MappingFactory(parent::createStorageReflection());
+		$factory = new MappingFactory(parent::createStorageReflection(), $this->getRepository()->getEntityMetadata());
 		$factory->addStringArrayMapping('authors');
 
 		return $factory->getReflection();
@@ -350,8 +350,28 @@ $doughnut->computedProperty; // 15
 
 `StorageReflection` decorator. Simplifies mapping definitions.
 
+Note that Orm does silently ignores (nonexistent) property names. All calls to `MappingFactory` validate
+ property names and throw, so while it's optional, it's highly recommended to use `MappingFactory`.
+
 - [`MappingFactory`](https://codedoc.pub/Mikulas/nextras-ormext/master/class-Mikulas.OrmExt.MappingFactory.html)
 
 ### `MappingFactory` Example
 
+While almost a decorator for `IMapper`, to validate property names `MappingFactory` must also be constructed with property
+ metadata. Having a mapping factory factory such as this is advised:
+
+```php
+abstract class AMapper extends BaseMapper {
+
+	/**
+	 * @return MappingFactory
+	 */
+	protected function createMappingFactory()
+	{
+		return new MappingFactory(parent::createStorageReflection(), $this->getRepository()->getEntityMetadata());
+	}
+
+}
+```
+
 See [`Json` Example](#json-example) and other snippets on this page.
diff --git a/src/Mikulas/OrmExt/InvalidPropertyException.php b/src/Mikulas/OrmExt/InvalidPropertyException.php
@@ -0,0 +1,36 @@
+<?php
+
+namespace Mikulas\OrmExt;
+
+
+class InvalidPropertyException extends \InvalidArgumentException implements Exception
+{
+
+	/** @var string */
+	private $propertyName;
+
+
+	/**
+	 * @param string     $propertyName
+	 * @param string     $message
+	 * @param \Exception $previous
+	 * @internal
+	 */
+	public function __construct($propertyName, $message, \Exception $previous = NULL)
+	{
+		$this->propertyName = $propertyName;
+		parent::__construct($message, NULL, $previous);
+	}
+
+
+	/**
+	 * @param string     $propertyName
+	 * @param \Exception $previous
+	 * @return InvalidPropertyException
+	 */
+	public static function createNonexistentProperty($propertyName, \Exception $previous = NULL)
+	{
+		return new self($propertyName, "Property '$propertyName' does not exist", $previous);
+	}
+
+}
diff --git a/src/Mikulas/OrmExt/MappingFactory.php b/src/Mikulas/OrmExt/MappingFactory.php
@@ -4,30 +4,40 @@
 
 use Mikulas\OrmExt\Pg\PgArray;
 use Nette\Utils\Json;
+use Nextras\Orm\Entity\Reflection\EntityMetadata;
+use Nextras\Orm\InvalidArgumentException;
+use Nextras\Orm\Mapper\Dbal\StorageReflection\IStorageReflection;
 use Nextras\Orm\Mapper\Dbal\StorageReflection\StorageReflection;
 
 
 class MappingFactory
 {
 
 	/** @var StorageReflection */
-	private $reflection;
+	private $storageReflection;
 
+	/** @var EntityMetadata */
+	private $entityMetadata;
 
-	public function __construct(StorageReflection $reflection)
+
+	public function __construct(IStorageReflection $storageReflection, EntityMetadata $entityMetadata)
 	{
-		$this->reflection = $reflection;
+		$this->storageReflection = $storageReflection;
+		$this->entityMetadata = $entityMetadata;
 	}
 
 
 	/**
 	 * @param string $propertyName
+	 * @throws InvalidPropertyException
 	 */
 	public function addJsonMapping($propertyName)
 	{
-		$this->reflection->addMapping(
+		$this->validateProperty($propertyName);
+
+		$this->storageReflection->addMapping(
 			$propertyName,
-			$this->reflection->convertEntityToStorageKey($propertyName),
+			$this->storageReflection->convertEntityToStorageKey($propertyName),
 			function ($value) {
 				return Json::decode($value, Json::FORCE_ARRAY);
 			},
@@ -42,12 +52,15 @@ function ($value) {
 	 * @param string $propertyName
 	 * @param Crypto $crypto
 	 * @param string $sqlPostfix
+	 * @throws InvalidPropertyException
 	 */
 	public function addCryptoMapping($propertyName, Crypto $crypto, $sqlPostfix = '_encrypted')
 	{
-		$this->reflection->addMapping(
+		$this->validateProperty($propertyName);
+
+		$this->storageReflection->addMapping(
 			$propertyName,
-			$this->reflection->convertEntityToStorageKey($propertyName) . $sqlPostfix,
+			$this->storageReflection->convertEntityToStorageKey($propertyName) . $sqlPostfix,
 			function ($garble) use ($crypto) {
 				return $garble === NULL ? NULL : $crypto->decrypt($garble);
 			},
@@ -62,12 +75,15 @@ function ($plain) use ($crypto) {
 	 * @param string   $propertyName
 	 * @param callable $toEntityTransform
 	 * @param callable $toSqlTransform
+	 * @throws InvalidPropertyException
 	 */
 	public function addGenericArrayMapping($propertyName, callable $toEntityTransform, callable $toSqlTransform)
 	{
-		$this->reflection->addMapping(
+		$this->validateProperty($propertyName);
+
+		$this->storageReflection->addMapping(
 			$propertyName,
-			$this->reflection->convertEntityToStorageKey($propertyName),
+			$this->storageReflection->convertEntityToStorageKey($propertyName),
 			function ($value) use ($toEntityTransform) {
 				return PgArray::parse($value, $toEntityTransform);
 			},
@@ -80,6 +96,7 @@ function ($value) use ($toSqlTransform) {
 
 	/**
 	 * @param string $propertyName
+	 * @throws InvalidPropertyException
 	 */
 	public function addStringArrayMapping($propertyName)
 	{
@@ -96,6 +113,7 @@ public function addStringArrayMapping($propertyName)
 
 	/**
 	 * @param string $propertyName
+	 * @throws InvalidPropertyException
 	 */
 	public function addIntArrayMapping($propertyName)
 	{
@@ -113,6 +131,7 @@ public function addIntArrayMapping($propertyName)
 	/**
 	 * Expects normalized dates without timezones
 	 * @param string $propertyName
+	 * @throws InvalidPropertyException
 	 */
 	public function addDateTimeArrayMapping($propertyName)
 	{
@@ -127,12 +146,26 @@ public function addDateTimeArrayMapping($propertyName)
 	}
 
 
+	/**
+	 * @param string $propertyName
+	 * @throws InvalidPropertyException
+	 */
+	public function validateProperty($propertyName)
+	{
+		try {
+			$this->entityMetadata->getProperty($propertyName);
+		} catch (InvalidArgumentException $e) {
+			throw InvalidPropertyException::createNonexistentProperty($propertyName, $e);
+		}
+	}
+
+
 	/**
 	 * @return StorageReflection
 	 */
-	public function getReflection()
+	public function getStorageReflection()
 	{
-		return $this->reflection;
+		return $this->storageReflection;
 	}
 
 }
diff --git a/tests/cases/unit/CryptoTest.phpt b/tests/cases/unit/CryptoTest.phpt
@@ -1,9 +1,5 @@
 <?php
 
-/**
- * @testCase
- */
-
 namespace Mikulas\OrmExt\Tests;
 
 use Mikulas\OrmExt\Crypto;

diff --git a/tests/cases/unit/MappingFactoryTest.php b/tests/cases/unit/MappingFactoryTest.php
@@ -0,0 +1,48 @@
+<?php
+
+namespace Mikulas\OrmExt\Tests;
+
+use Mikulas\OrmExt\InvalidPropertyException;
+use Mikulas\OrmExt\MappingFactory;
+use Mockery;
+use Nextras\Orm\Entity\Reflection\EntityMetadata;
+use Nextras\Orm\InvalidArgumentException;
+use Nextras\Orm\Mapper\Dbal\StorageReflection\StorageReflection;
+use Tester\Assert;
+
+$dic = require_once __DIR__ . '/../../bootstrap.php';
+
+
+class MappingFactoryTest extends TestCase
+{
+
+	public function testInvalidPropertyThrows()
+	{
+		/** @var Mockery\MockInterface|StorageReflection $storageReflection */
+		$storageReflection = Mockery::mock(StorageReflection::class);
+		$storageReflection->shouldReceive('convertEntityToStorageKey')
+			->andReturnSelf();
+		$storageReflection->shouldReceive('addMapping')
+			->andReturn();
+
+		/** @var Mockery\MockInterface|EntityMetadata $entityMetadata */
+		$entityMetadata = Mockery::mock(EntityMetadata::class);
+		$entityMetadata->shouldReceive('getProperty')
+			->andReturnUsing(function($prop) {
+				if ($prop !== 'exists') {
+					throw new InvalidArgumentException();
+				}
+			});
+
+		$factory = new MappingFactory($storageReflection, $entityMetadata);
+
+		Assert::exception(function() use ($factory) {
+			$factory->addIntArrayMapping('unknown');
+		}, InvalidPropertyException::class);
+
+		$factory->addIntArrayMapping('exists');
+	}
+
+}
+
+(new MappingFactoryTest($dic))->run();
diff --git a/tests/inc/model/Mapper.php b/tests/inc/model/Mapper.php
@@ -2,6 +2,7 @@
 
 namespace Mikulas\OrmExt\Tests;
 
+use Mikulas\OrmExt\MappingFactory;
 use Mikulas\OrmExt\Pg\SelfUpdatingPropertyMapper;
 use Nextras\Orm\Mapper\Dbal\StorageReflection\CamelCaseStorageReflection;
 
@@ -19,4 +20,13 @@ protected function createStorageReflection()
 		);
 	}
 
+
+	/**
+	 * @return MappingFactory
+	 */
+	protected function createMappingFactory()
+	{
+		return new MappingFactory(self::createStorageReflection(), $this->getRepository()->getEntityMetadata());
+	}
+
 }
diff --git a/tests/inc/model/PersonsMapper.php b/tests/inc/model/PersonsMapper.php
@@ -3,7 +3,6 @@
 namespace Mikulas\OrmExt\Tests;
 
 use Mikulas\OrmExt\Crypto;
-use Mikulas\OrmExt\MappingFactory;
 use Nette\Caching\IStorage;
 use Nextras\Dbal\Connection;
 
@@ -30,12 +29,12 @@ protected function getSelfUpdatingProperties()
 
 	protected function createStorageReflection()
 	{
-		$factory = new MappingFactory(parent::createStorageReflection());
+		$factory = $this->createMappingFactory();
 		$factory->addJsonMapping('content');
 		$factory->addCryptoMapping('creditCardNumber', $this->crypto);
 		$factory->addIntArrayMapping('favoriteNumbers');
 
-		return $factory->getReflection();
+		return $factory->getStorageReflection();
 	}
 
 }