From 057e7f4c38bff73e3e97bee3cd84b961acf56bde Mon Sep 17 00:00:00 2001 From: Chauncey McAskill Date: Fri, 18 Oct 2019 16:21:17 -0400 Subject: [PATCH 1/6] Reorganize AbstractModel methods --- src/Charcoal/Model/AbstractModel.php | 84 ++++++++++++++-------------- 1 file changed, 42 insertions(+), 42 deletions(-) diff --git a/src/Charcoal/Model/AbstractModel.php b/src/Charcoal/Model/AbstractModel.php index ba2e1ff4..e1401971 100644 --- a/src/Charcoal/Model/AbstractModel.php +++ b/src/Charcoal/Model/AbstractModel.php @@ -106,21 +106,6 @@ public function __construct(array $data = null) } } - /** - * Sets the object data, from an associative array map (or any other Traversable). - * - * @param array $data The entity data. Will call setters. - * @return self - * @see AbstractEntity::setData() - */ - public function setData(array $data) - { - $data = $this->setIdFromData($data); - - parent::setData($data); - return $this; - } - /** * Retrieve the model data as a structure (serialize to array). * @@ -142,6 +127,21 @@ public function data(array $properties = null) return $data; } + /** + * Sets the object data, from an associative array map (or any other Traversable). + * + * @param array $data The entity data. Will call setters. + * @return self + * @see AbstractEntity::setData() + */ + public function setData(array $data) + { + $data = $this->setIdFromData($data); + + parent::setData($data); + return $this; + } + /** * Merge data on the model. * @@ -329,33 +329,6 @@ public function loadFromL10n($key, $value, array $langs) return $lang; } - /** - * Generate a model type identifier from this object's class name. - * - * Based on {@see DescribableTrait::generateMetadataIdent()}. - * - * @return string - */ - public static function objType() - { - $class = get_called_class(); - $ident = preg_replace('/([a-z])([A-Z])/', '$1-$2', $class); - $ident = strtolower(str_replace('\\', '/', $ident)); - return $ident; - } - - /** - * Inject dependencies from a DI Container. - * - * @param Container $container A Pimple DI service container. - * @return void - */ - protected function setDependencies(Container $container) - { - // This method is a stub. - // Reimplement in children method to inject dependencies in your class from a Pimple container. - } - /** * Set the object's ID from an associative array map (or any other Traversable). * @@ -475,4 +448,31 @@ protected function createValidator() $validator = new ModelValidator($this); return $validator; } + + /** + * Inject dependencies from a DI Container. + * + * @param Container $container A Pimple DI service container. + * @return void + */ + protected function setDependencies(Container $container) + { + // This method is a stub. + // Reimplement in children method to inject dependencies in your class from a Pimple container. + } + + /** + * Generate a model type identifier from this object's class name. + * + * Based on {@see DescribableTrait::generateMetadataIdent()}. + * + * @return string + */ + public static function objType() + { + $class = get_called_class(); + $ident = preg_replace('/([a-z])([A-Z])/', '$1-$2', $class); + $ident = strtolower(str_replace('\\', '/', $ident)); + return $ident; + } } From 99f9442d967793d9ca32305be7d79f5f004862f5 Mon Sep 17 00:00:00 2001 From: Chauncey McAskill Date: Fri, 18 Oct 2019 17:35:55 -0400 Subject: [PATCH 2/6] Implement flatData method on AbstractModel --- src/Charcoal/Model/AbstractModel.php | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/Charcoal/Model/AbstractModel.php b/src/Charcoal/Model/AbstractModel.php index e1401971..fe6c677f 100644 --- a/src/Charcoal/Model/AbstractModel.php +++ b/src/Charcoal/Model/AbstractModel.php @@ -243,12 +243,22 @@ public function setFlatData(array $flatData) * * This method returns a 1-dimensional array of the object's values. * - * @todo Implementation required. + * @param array $properties Optional. List of property identifiers + * for retrieving a subset of data. * @return array */ - public function flatData() + public function flatData(array $properties = null) { - return []; + $flatData = []; + $properties = $this->properties($properties); + foreach ($properties as $propertyIdent => $property) { + $value = $this->propertyValue($propertyIdent); + foreach ($property->fields($value) as $field) { + $flatData[$field->ident()] = $field->val(); + } + } + + return $flatData; } /** From 5ba730d2d90b67b43849784a2ed920681325cf6c Mon Sep 17 00:00:00 2001 From: Chauncey McAskill Date: Fri, 18 Oct 2019 17:57:17 -0400 Subject: [PATCH 3/6] Improve field name usage in ExpressionFieldTrait Added: - Method `snakeize()` with static cache to `ExpressionFieldTrait` --- src/Charcoal/Source/ExpressionFieldTrait.php | 33 ++++++++++++++++++-- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/src/Charcoal/Source/ExpressionFieldTrait.php b/src/Charcoal/Source/ExpressionFieldTrait.php index 019c572e..5903b9d2 100644 --- a/src/Charcoal/Source/ExpressionFieldTrait.php +++ b/src/Charcoal/Source/ExpressionFieldTrait.php @@ -32,6 +32,13 @@ trait ExpressionFieldTrait */ protected $table; + /** + * Holds a list of all snake_case strings. + * + * @var string[] + */ + protected static $snakeCache = []; + /** * Set model property key or source field key. * @@ -175,8 +182,7 @@ public function fieldNames() if ($property instanceof StorablePropertyInterface) { return $property->fieldNames(); } else { - // Ensure snake_case - $property = strtolower(preg_replace('/(?snakeize($property); return [ $property ]; } } @@ -193,7 +199,7 @@ public function fieldName() { $property = $this->property(); if ($property instanceof PropertyInterface) { - return $property->ident(); + return $property->fieldIdent(); } else { return $property; } @@ -228,4 +234,25 @@ public function fieldIdentifier() return Expression::quoteIdentifier($fieldName, $tableName); } + + /** + * Transform a string from "camelCase" to "snake_case". + * + * @param string $value The string to snakeize. + * @return string The snake_case string. + */ + protected function snakeize($value) + { + $key = $value; + + if (isset(static::$snakeCache[$key])) { + return static::$snakeCache[$key]; + } + + $value = strtolower(preg_replace('/(? Date: Fri, 18 Oct 2019 18:02:16 -0400 Subject: [PATCH 4/6] Fix various edge cases where SQL is expected Added: - Log when `PropertyField` returns no SQL column definition Fixed: - Edge case when `PropertyField` returns no SQL column definitions - Edge case when an Expression object returns a non-string --- .../Source/Database/DatabaseFilter.php | 2 +- src/Charcoal/Source/DatabaseSource.php | 43 +++++++++++++------ 2 files changed, 32 insertions(+), 13 deletions(-) diff --git a/src/Charcoal/Source/Database/DatabaseFilter.php b/src/Charcoal/Source/Database/DatabaseFilter.php index 2f43d98e..fbfbd813 100644 --- a/src/Charcoal/Source/Database/DatabaseFilter.php +++ b/src/Charcoal/Source/Database/DatabaseFilter.php @@ -141,7 +141,7 @@ protected function byFilters() $filter = $filter->sql(); } - if (strlen($filter) > 0) { + if ($filter && strlen($filter) > 0) { $conditions[] = $filter; } } diff --git a/src/Charcoal/Source/DatabaseSource.php b/src/Charcoal/Source/DatabaseSource.php index 9be42ff3..b575ad5e 100644 --- a/src/Charcoal/Source/DatabaseSource.php +++ b/src/Charcoal/Source/DatabaseSource.php @@ -155,7 +155,10 @@ public function createTable() $fields = $this->getModelFields($model); $columns = []; foreach ($fields as $field) { - $columns[] = $field->sql(); + $fieldSql = $field->sql(); + if ($fieldSql) { + $columns[] = $fieldSql; + } } $query = 'CREATE TABLE `'.$table.'` ('."\n"; @@ -201,10 +204,18 @@ public function alterTable() $ident = $field->ident(); if (!array_key_exists($ident, $cols)) { - // The key does not exist at all. - $query = 'ALTER TABLE `'.$table.'` ADD '.$field->sql(); - $this->logger->debug($query); - $dbh->query($query); + $fieldSql = $field->sql(); + if ($fieldSql) { + // The key does not exist at all. + $query = 'ALTER TABLE `'.$table.'` ADD '.$fieldSql; + $this->logger->debug($query); + $dbh->query($query); + } else { + $this->logger->warning('Empty column definition.', [ + 'table' => $table, + 'field' => $ident, + ]); + } } else { // The key exists. Validate. $col = $cols[$ident]; @@ -226,9 +237,17 @@ public function alterTable() } if ($alter === true) { - $query = 'ALTER TABLE `'.$table.'` CHANGE `'.$ident.'` '.$field->sql(); - $this->logger->debug($query); - $dbh->query($query); + $fieldSql = $field->sql(); + if ($fieldSql) { + $query = 'ALTER TABLE `'.$table.'` CHANGE `'.$ident.'` '.$fieldSql; + $this->logger->debug($query); + $dbh->query($query); + } else { + $this->logger->warning('Empty column definition.', [ + 'table' => $table, + 'field' => $ident, + ]); + } } } } @@ -327,7 +346,7 @@ public function tableStructure() 'Null' => !!$col['notnull'] ? 'NO' : 'YES', 'Default' => $col['dflt_value'], 'Key' => !!$col['pk'] ? 'PRI' : '', - 'Extra' => '' + 'Extra' => '', ]; } return $struct; @@ -873,7 +892,7 @@ public function sqlFilters() ]); $sql = $criteria->sql(); - if (strlen($sql) > 0) { + if ($sql && strlen($sql) > 0) { $sql = ' WHERE '.$sql; } @@ -898,7 +917,7 @@ public function sqlOrders() } $sql = $order->sql(); - if (strlen($sql) > 0) { + if ($sql && strlen($sql) > 0) { $parts[] = $sql; } } @@ -923,7 +942,7 @@ public function sqlPagination() } $sql = $pager->sql(); - if (strlen($sql) > 0) { + if ($sql && strlen($sql) > 0) { $sql = ' '.$sql; } From 19bc4f2d71a3d7ab071154851df4800f1ae916d1 Mon Sep 17 00:00:00 2001 From: Chauncey McAskill Date: Thu, 31 Oct 2019 17:05:58 -0400 Subject: [PATCH 5/6] Improve loading object from an L10n property --- src/Charcoal/Model/AbstractModel.php | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/Charcoal/Model/AbstractModel.php b/src/Charcoal/Model/AbstractModel.php index fe6c677f..b96cb776 100644 --- a/src/Charcoal/Model/AbstractModel.php +++ b/src/Charcoal/Model/AbstractModel.php @@ -305,16 +305,29 @@ public function saveProperties(array $properties = null) * @param string $key Key pointing a column's l10n base ident. * @param mixed $value Value to search in all languages. * @param array $langs List of languages (code, ex: "en") to check into. + * @throws InvalidArgumentException If a language is invalid. * @throws PDOException If the PDO query fails. * @return string The matching language. */ public function loadFromL10n($key, $value, array $langs) { + $binds = [ + 'ident' => $value, + ]; $switch = []; $where = []; foreach ($langs as $lang) { - $switch[] = 'WHEN `'.$key.'_'.$lang.'` = :ident THEN \''.$lang.'\''; - $where[] = '`'.$key.'_'.$lang.'` = :ident'; + if (!is_string($lang)) { + throw new InvalidArgumentException('Language; must be a string'); + } + + $fieldName = $key.'_'.$lang; + $langParam = 'lang_'.$lang; + $binds[$langParam] = $lang; + $langParam = ':'.$langParam; + + $switch[] = 'WHEN `'.$fieldName.'` = :ident THEN '.$langParam; + $where[] = '`'.$fieldName.'` = :ident'; } $source = $this->source(); From ca39455333f2cf37afe1162a88e19351fae44fa9 Mon Sep 17 00:00:00 2001 From: Chauncey McAskill Date: Thu, 31 Oct 2019 17:06:18 -0400 Subject: [PATCH 6/6] [!] Updated to latest charcoal-property Reason: - Charcoal Property adds support for custom storable property fields - Decouples L10N property fields - Class `AbstractModel` is updated to cleanup flat-data assignment Requires: - charcoal-property v0.10.x Changed: - Bump branch-alias to 0.6 - Method `AbstractModel::setFlatData()` to decouple flat-data processing for model properties Added: - Method `setPropertyDataFromFlatData()` to process data from property field names - Method `setPropertyData()` to only assign where datum is a property and return the rest --- composer.json | 4 +- src/Charcoal/Model/AbstractModel.php | 83 +++++++++++++++++--------- src/Charcoal/Source/AbstractSource.php | 1 + 3 files changed, 58 insertions(+), 30 deletions(-) diff --git a/composer.json b/composer.json index 760571f5..617210cf 100644 --- a/composer.json +++ b/composer.json @@ -19,7 +19,7 @@ "prefer-stable": true, "extra": { "branch-alias": { - "dev-master": "0.5.x-dev" + "dev-master": "0.6.x-dev" } }, "require": { @@ -32,7 +32,7 @@ "locomotivemtl/charcoal-cache": "~0.1", "locomotivemtl/charcoal-config": "~0.9", "locomotivemtl/charcoal-factory": "~0.4", - "locomotivemtl/charcoal-property": "~0.9", + "locomotivemtl/charcoal-property": "~0.10", "locomotivemtl/charcoal-view": "~0.3" }, "require-dev": { diff --git a/src/Charcoal/Model/AbstractModel.php b/src/Charcoal/Model/AbstractModel.php index b96cb776..35a8aaa9 100644 --- a/src/Charcoal/Model/AbstractModel.php +++ b/src/Charcoal/Model/AbstractModel.php @@ -142,6 +142,26 @@ public function setData(array $data) return $this; } + /** + * Sets the object data, from an associative array map (or any other Traversable). + * + * @param array $data The model property data. + * @return array Returns the remaining dataset. + */ + public function setPropertyData(array $data) + { + $data = $this->setIdFromData($data); + + foreach ($data as $key => $value) { + if ($this->hasProperty($key)) { + $this[$key] = $value; + unset($data[$key]); + } + } + + return $data; + } + /** * Merge data on the model. * @@ -196,46 +216,53 @@ public function defaultData() /** * Set the model data (from a flattened structure). * - * This method takes a 1-dimensional array and fills the object with its values. - * - * @param array $flatData The model data. + * @param array $flatData The model dataset. * @return self */ public function setFlatData(array $flatData) + { + $flatData = $this->setPropertyDataFromFlatData($flatData); + + // Set remaining (non-property) data. + if (!empty($flatData)) { + $this->setData($flatData); + } + + return $this; + } + + /** + * Set the model property data (from a flattened structure). + * + * This method takes a one-dimensional dataset and, depending on the property's + * {@see \Charcoal\Property\PropertyField::fieldNames() field structure}, + * returns a {@see \Charcoal\Property\PropertyField::parseFromFlatData() complex datum}. + * + * @param array $flatData The model property data. + * @return array Returns the remaining dataset. + */ + public function setPropertyDataFromFlatData(array $flatData) { $flatData = $this->setIdFromData($flatData); - $data = []; + $propData = []; $properties = $this->properties(); foreach ($properties as $propertyIdent => $property) { - $fields = $property->fields(null); - foreach ($fields as $k => $f) { - if (is_string($k)) { - $fid = $f->ident(); - $snake = strtolower(preg_replace('/(?ident(); - if (isset($flatData[$fid])) { - $data[$propertyIdent] = $flatData[$fid]; - unset($flatData[$fid]); - } + $fieldValues = []; + $fieldNames = $property->fieldNames(); + foreach ($fieldNames as $fieldName) { + if (array_key_exists($fieldName, $flatData)) { + $fieldValues[$fieldName] = $flatData[$fieldName]; + unset($flatData[$fieldName]); } } - } - - $this->setData($data); - // Set remaining (non-property) data. - if (!empty($flatData)) { - $this->setData($flatData); + if ($fieldValues) { + $this[$propertyIdent] = $property->parseFromFlatData($fieldValues); + } } - return $this; + return $flatData; } /** @@ -358,7 +385,7 @@ public function loadFromL10n($key, $value, array $langs) * Useful for setting the object ID before the rest of the object's data. * * @param array $data The object data. - * @return array The object data without the pre-set ID. + * @return array Returns the remaining dataset. */ protected function setIdFromData(array $data) { diff --git a/src/Charcoal/Source/AbstractSource.php b/src/Charcoal/Source/AbstractSource.php index 8c5049a9..4c0f116e 100644 --- a/src/Charcoal/Source/AbstractSource.php +++ b/src/Charcoal/Source/AbstractSource.php @@ -270,6 +270,7 @@ public function addFilter($param, $value = null, array $options = null) /** * Process a query filter with the current model. * + * @todo If property is L10N, turn filter into group of filters for each language. * @param FilterInterface $filter The expression object. * @return FilterInterface The parsed expression object. */