From 71a13da6196d6f6c2df5c9318e36b12367ed136c Mon Sep 17 00:00:00 2001 From: Dennis Eichhorn Date: Mon, 21 May 2018 17:48:41 +0200 Subject: [PATCH] Optimize mapper loops --- DataStorage/Database/BuilderAbstract.php | 3 +- DataStorage/Database/DataMapperAbstract.php | 159 +++++++++--------- DataStorage/Database/Query/QueryType.php | 1 + .../Database/Query/QueryTypeTest.php | 3 +- .../Database/TestModel/BaseModelMapper.php | 22 +-- 5 files changed, 92 insertions(+), 96 deletions(-) diff --git a/DataStorage/Database/BuilderAbstract.php b/DataStorage/Database/BuilderAbstract.php index 9331e34b7..f724f9a6f 100644 --- a/DataStorage/Database/BuilderAbstract.php +++ b/DataStorage/Database/BuilderAbstract.php @@ -15,6 +15,7 @@ declare(strict_types=1); namespace phpOMS\DataStorage\Database; use phpOMS\DataStorage\Database\Connection\ConnectionAbstract; +use phpOMS\DataStorage\Database\Query\QueryType; /** * Database query builder. @@ -48,7 +49,7 @@ abstract class BuilderAbstract * @var int * @since 1.0.0 */ - protected $type = null; + protected $type = QueryType::EMPTY; /** * Prefix. diff --git a/DataStorage/Database/DataMapperAbstract.php b/DataStorage/Database/DataMapperAbstract.php index adae9957c..fb82c2845 100644 --- a/DataStorage/Database/DataMapperAbstract.php +++ b/DataStorage/Database/DataMapperAbstract.php @@ -20,6 +20,7 @@ use phpOMS\DataStorage\DataMapperInterface; use phpOMS\Message\RequestAbstract; use phpOMS\DataStorage\Database\Exception\InvalidMapperException; use phpOMS\Util\ArrayUtils; +use phpOMS\DataStorage\Database\Query\QueryType; /** * Datamapper for databases. @@ -400,45 +401,39 @@ class DataMapperAbstract implements DataMapperInterface $query = new Builder(self::$db); $query->prefix(self::$db->getPrefix())->into(static::$table); - $properties = $refClass->getProperties(); - - foreach ($properties as $property) { - $propertyName = $property->getName(); - + foreach (static::$columns as $key => $column) { + $propertyName = $column['internal']; if (isset(static::$hasMany[$propertyName]) || isset(static::$hasOne[$propertyName])) { continue; } + $property = $refClass->getProperty($propertyName); + if (!($isPublic = $property->isPublic())) { $property->setAccessible(true); } - foreach (static::$columns as $key => $column) { - if (isset(static::$ownsOne[$propertyName]) && $column['internal'] === $propertyName) { - $id = self::createOwnsOne($propertyName, $property->getValue($obj)); - $value = self::parseValue($column['type'], $id); + if (isset(static::$ownsOne[$propertyName])) { + $id = self::createOwnsOne($propertyName, $property->getValue($obj)); + $value = self::parseValue($column['type'], $id); - $query->insert($column['name'])->value($value, $column['type']); - break; - } elseif (isset(static::$belongsTo[$propertyName]) && $column['internal'] === $propertyName) { - $id = self::createBelongsTo($propertyName, $property->getValue($obj)); - $value = self::parseValue($column['type'], $id); + $query->insert($column['name'])->value($value, $column['type']); + } elseif (isset(static::$belongsTo[$propertyName])) { + $id = self::createBelongsTo($propertyName, $property->getValue($obj)); + $value = self::parseValue($column['type'], $id); - $query->insert($column['name'])->value($value, $column['type']); - break; - } elseif ($column['internal'] === $propertyName && $column['type'] !== static::$primaryField) { - $tValue = $property->getValue($obj); - if (stripos($column['internal'], '/') !== false) { - $path = explode($column['internal']); - $path = implode('/', array_shift($path)); - $tValue = ArrayUtils::getArray($path, $tValue, '/'); - } - - $value = self::parseValue($column['type'], $tValue); - - $query->insert($column['name'])->value($value, $column['type']); - break; + $query->insert($column['name'])->value($value, $column['type']); + } elseif ($column['name'] !== static::$primaryField) { + $tValue = $property->getValue($obj); + if (stripos($column['internal'], '/') !== false) { + $path = explode($column['internal']); + $path = implode('/', array_shift($path)); + $tValue = ArrayUtils::getArray($path, $tValue, '/'); } + + $value = self::parseValue($column['type'], $tValue); + + $query->insert($column['name'])->value($value, $column['type']); } if (!($isPublic)) { @@ -446,8 +441,9 @@ class DataMapperAbstract implements DataMapperInterface } } - if (static::$table === 'exchange') { - var_dump($query->toSql()); + // if a table only has a single column = primary key column. This must be done otherwise the query is empty + if ($query->getType() === QueryType::EMPTY) { + $query->insert(static::$primaryField)->value(0, static::$columns[static::$primaryField]['type']); } self::$db->con->prepare($query->toSql())->execute(); @@ -470,7 +466,9 @@ class DataMapperAbstract implements DataMapperInterface $query->prefix(self::$db->getPrefix())->into(static::$table); foreach (static::$columns as $key => $column) { - if (isset(static::$hasMany[$key]) || isset(static::$hasOne[$key])) { + if (isset(static::$hasMany[$key]) + || isset(static::$hasOne[$key]) + ) { continue; } @@ -494,7 +492,7 @@ class DataMapperAbstract implements DataMapperInterface $query->insert($column['name'])->value($value, $column['type']); break; - } elseif ($column['internal'] === $path && $column['type'] !== static::$primaryField) { + } elseif ($column['internal'] === $path) { $value = self::parseValue($column['type'], $property); $query->insert($column['name'])->value($value, $column['type']); @@ -519,8 +517,8 @@ class DataMapperAbstract implements DataMapperInterface */ private static function getObjectId($obj, \ReflectionClass $refClass = null) { - $refClass = $refClass ?? new \ReflectionClass($obj); - $refProp = $refClass->getProperty(static::$columns[static::$primaryField]['internal']); + $refClass = $refClass ?? new \ReflectionClass($obj); + $refProp = $refClass->getProperty(static::$columns[static::$primaryField]['internal']); if (!($isPublic = $refProp->isPublic())) { $refProp->setAccessible(true); @@ -539,8 +537,8 @@ class DataMapperAbstract implements DataMapperInterface * Set id to model * * @param \ReflectionClass $refClass Reflection class - * @param Object $obj Object to create - * @param mixed $objId Id to set + * @param Object $obj Object to create + * @param mixed $objId Id to set * * @return void * @@ -566,8 +564,8 @@ class DataMapperAbstract implements DataMapperInterface * Create has many * * @param \ReflectionClass $refClass Reflection class - * @param Object $obj Object to create - * @param mixed $objId Id to set + * @param Object $obj Object to create + * @param mixed $objId Id to set * * @return void * @@ -704,7 +702,7 @@ class DataMapperAbstract implements DataMapperInterface * Create has one * * @param \ReflectionClass $refClass Property name to initialize - * @param Object $obj Object to create + * @param Object $obj Object to create * * @return mixed * @todo implement??? @@ -905,8 +903,8 @@ class DataMapperAbstract implements DataMapperInterface * Update has many * * @param \ReflectionClass $refClass Reflection class - * @param Object $obj Object to create - * @param mixed $objId Id to set + * @param Object $obj Object to create + * @param mixed $objId Id to set * * @return void * @@ -1101,8 +1099,8 @@ class DataMapperAbstract implements DataMapperInterface /** * Update object in db. * - * @param Object $obj Model to update - * @param mixed $objId Model id + * @param Object $obj Model to update + * @param mixed $objId Model id * @param \ReflectionClass $refClass Reflection class * * @return void @@ -1116,48 +1114,43 @@ class DataMapperAbstract implements DataMapperInterface ->update(static::$table) ->where(static::$table . '.' . static::$primaryField, '=', $objId); - $properties = $refClass->getProperties(); - - foreach ($properties as $property) { - $propertyName = $property->getName(); - - if (isset(static::$hasMany[$propertyName]) || isset(static::$hasOne[$propertyName])) { + foreach (static::$columns as $key => $column) { + $propertyName = $column['internal']; + if (isset(static::$hasMany[$propertyName]) + || isset(static::$hasOne[$propertyName]) + || $column['internal'] === static::$primaryField + ) { continue; } + $property = $refClass->getProperty($propertyName); + if (!($isPublic = $property->isPublic())) { $property->setAccessible(true); } - // todo: the order of updating could be a problem. maybe looping through ownsOne and belongsTo first is better. + if (isset(static::$ownsOne[$propertyName])) { + $id = self::updateOwnsOne($propertyName, $property->getValue($obj)); + $value = self::parseValue($column['type'], $id); - foreach (static::$columns as $key => $column) { - if (isset(static::$ownsOne[$propertyName]) && $column['internal'] === $propertyName) { - $id = self::updateOwnsOne($propertyName, $property->getValue($obj)); - $value = self::parseValue($column['type'], $id); + // todo: should not be done if the id didn't change. but for now don't know if id changed + $query->set([static::$table . '.' . $column['name'] => $value], $column['type']); + } elseif (isset(static::$belongsTo[$propertyName])) { + $id = self::updateBelongsTo($propertyName, $property->getValue($obj)); + $value = self::parseValue($column['type'], $id); - // todo: should not be done if the id didn't change. but for now don't know if id changed - $query->set([static::$table . '.' . $column['name'] => $value], $column['type']); - break; - } elseif (isset(static::$belongsTo[$propertyName]) && $column['internal'] === $propertyName) { - $id = self::updateBelongsTo($propertyName, $property->getValue($obj)); - $value = self::parseValue($column['type'], $id); - - // todo: should not be done if the id didn't change. but for now don't know if id changed - $query->set([static::$table . '.' . $column['name'] => $value], $column['type']); - break; - } elseif ($column['internal'] === $propertyName && $column['type'] !== static::$primaryField) { - $tValue = $property->getValue($obj); - if (stripos($column['internal'], '/') !== false) { - $path = explode($column['internal']); - $path = implode('/', array_shift($path)); - $tValue = ArrayUtils::getArray($path, $tValue, '/'); - } - $value = self::parseValue($column['type'], $tValue); - - $query->set([static::$table . '.' . $column['name'] => $value], $column['type']); - break; + // todo: should not be done if the id didn't change. but for now don't know if id changed + $query->set([static::$table . '.' . $column['name'] => $value], $column['type']); + } elseif ($column['name'] !== static::$primaryField) { + $tValue = $property->getValue($obj); + if (stripos($column['internal'], '/') !== false) { + $path = explode($column['internal']); + $path = implode('/', array_shift($path)); + $tValue = ArrayUtils::getArray($path, $tValue, '/'); } + $value = self::parseValue($column['type'], $tValue); + + $query->set([static::$table . '.' . $column['name'] => $value], $column['type']); } if (!($isPublic)) { @@ -1212,10 +1205,10 @@ class DataMapperAbstract implements DataMapperInterface /** * Delete has many * - * @param \ReflectionClass $refClass Reflection class - * @param Object $obj Object to create - * @param mixed $objId Id to set - * @param int $relations Delete all relations as well + * @param \ReflectionClass $refClass Reflection class + * @param Object $obj Object to create + * @param mixed $objId Id to set + * @param int $relations Delete all relations as well * * @return void * @@ -1332,10 +1325,10 @@ class DataMapperAbstract implements DataMapperInterface /** * Delete object in db. * - * @param Object $obj Model to delete - * @param mixed $objId Model id - * @param int $relations Delete all relations as well - * @param \ReflectionClass $refClass Reflection class + * @param Object $obj Model to delete + * @param mixed $objId Model id + * @param int $relations Delete all relations as well + * @param \ReflectionClass $refClass Reflection class * * @return void * diff --git a/DataStorage/Database/Query/QueryType.php b/DataStorage/Database/Query/QueryType.php index 5e89e4c73..ff3a77dfd 100644 --- a/DataStorage/Database/Query/QueryType.php +++ b/DataStorage/Database/Query/QueryType.php @@ -32,4 +32,5 @@ abstract class QueryType extends Enum public const DELETE = 3; public const RANDOM = 4; public const RAW = 5; + public const EMPTY = 6; } diff --git a/tests/DataStorage/Database/Query/QueryTypeTest.php b/tests/DataStorage/Database/Query/QueryTypeTest.php index 43d80b423..a9c8c8469 100644 --- a/tests/DataStorage/Database/Query/QueryTypeTest.php +++ b/tests/DataStorage/Database/Query/QueryTypeTest.php @@ -19,7 +19,7 @@ class QueryTypeTest extends \PHPUnit\Framework\TestCase { public function testEnums() { - self::assertEquals(6, count(QueryType::getConstants())); + self::assertEquals(7, count(QueryType::getConstants())); self::assertEquals(QueryType::getConstants(), array_unique(QueryType::getConstants())); self::assertEquals(0, QueryType::SELECT); @@ -28,5 +28,6 @@ class QueryTypeTest extends \PHPUnit\Framework\TestCase self::assertEquals(3, QueryType::DELETE); self::assertEquals(4, QueryType::RANDOM); self::assertEquals(5, QueryType::RAW); + self::assertEquals(6, QueryType::EMPTY); } } diff --git a/tests/DataStorage/Database/TestModel/BaseModelMapper.php b/tests/DataStorage/Database/TestModel/BaseModelMapper.php index cf67fb31f..2eccc4fb8 100644 --- a/tests/DataStorage/Database/TestModel/BaseModelMapper.php +++ b/tests/DataStorage/Database/TestModel/BaseModelMapper.php @@ -28,17 +28,17 @@ class BaseModelMapper extends DataMapperAbstract * @since 1.0.0 */ protected static $columns = [ - 'test_base_id' => ['name' => 'test_base_id', 'type' => 'int', 'internal' => 'id'], - 'test_base_string' => ['name' => 'test_base_string', 'type' => 'string', 'internal' => 'string'], - 'test_base_int' => ['name' => 'test_base_int', 'type' => 'int', 'internal' => 'int'], - 'test_base_bool' => ['name' => 'test_base_bool', 'type' => 'bool', 'internal' => 'bool'], - 'test_base_null' => ['name' => 'test_base_null', 'type' => 'int', 'internal' => 'null'], - 'test_base_float' => ['name' => 'test_base_float', 'type' => 'float', 'internal' => 'float'], - 'test_base_json' => ['name' => 'test_base_json', 'type' => 'Json', 'internal' => 'json'], - 'test_base_jsonSerialize' => ['name' => 'test_base_jsonSerialize', 'type' => 'jsonSerializable', 'internal' => 'jsonSerialize'], - 'test_base_datetime' => ['name' => 'test_base_datetime', 'type' => 'DateTime', 'internal' => 'datetime'], - 'test_base_owns_one_self' => ['name' => 'test_base_owns_one_self', 'type' => 'int', 'internal' => 'ownsOneSelf'], - 'test_base_belongs_to_one' => ['name' => 'test_base_belongs_to_one', 'type' => 'int', 'internal' => 'belongsToOne'], + 'test_base_id' => ['name' => 'test_base_id', 'type' => 'int', 'internal' => 'id'], + 'test_base_string' => ['name' => 'test_base_string', 'type' => 'string', 'internal' => 'string'], + 'test_base_int' => ['name' => 'test_base_int', 'type' => 'int', 'internal' => 'int'], + 'test_base_bool' => ['name' => 'test_base_bool', 'type' => 'bool', 'internal' => 'bool'], + 'test_base_null' => ['name' => 'test_base_null', 'type' => 'int', 'internal' => 'null'], + 'test_base_float' => ['name' => 'test_base_float', 'type' => 'float', 'internal' => 'float'], + 'test_base_json' => ['name' => 'test_base_json', 'type' => 'Json', 'internal' => 'json'], + 'test_base_json_serializable' => ['name' => 'test_base_json_serializable', 'type' => 'Json', 'internal' => 'jsonSerializable'], + 'test_base_datetime' => ['name' => 'test_base_datetime', 'type' => 'DateTime', 'internal' => 'datetime'], + 'test_base_owns_one_self' => ['name' => 'test_base_owns_one_self', 'type' => 'int', 'internal' => 'ownsOneSelf'], + 'test_base_belongs_to_one' => ['name' => 'test_base_belongs_to_one', 'type' => 'int', 'internal' => 'belongsToOne'], ]; protected static $belongsTo = [