From e7b708185ebfbd8050c9d0fbe2ff79552c21a79a Mon Sep 17 00:00:00 2001 From: Dennis Eichhorn Date: Sun, 4 Feb 2024 20:34:13 +0000 Subject: [PATCH] update --- Algorithm/Graph/MarkovChain.php | 4 +- Algorithm/Rating/TrueSkill.php | 3 +- Api/Address/Validation.php | 5 - Business/Sales/MarketShareEstimation.php | 2 +- Business/Warehouse/OrderSuggestion.php | 44 +++++ .../Database/Mapper/DataMapperFactory.php | 8 +- DataStorage/Database/Mapper/DeleteMapper.php | 15 +- DataStorage/Database/Mapper/ReadMapper.php | 157 +++++++++--------- DataStorage/Database/Mapper/UpdateMapper.php | 13 +- DataStorage/Database/Mapper/WriteMapper.php | 39 ++--- DataStorage/Database/Query/Builder.php | 4 - .../Database/Query/Grammar/Grammar.php | 7 +- .../Database/Schema/Grammar/SQLiteGrammar.php | 70 +++++--- Localization/BaseStringL11n.php | 11 +- .../LanguageDetection/NgramParser.php | 4 +- Math/Geometry/ConvexHull/GrahamScan.php | 1 - Math/Matrix/Vector.php | 23 ++- Math/Optimization/Simplex.php | 7 +- Math/Solver/Root/Illinois.php | 6 +- Message/Http/HttpRequest.php | 4 +- Message/Http/Rest.php | 7 +- Message/Mail/Email.php | 44 ++++- Message/Mail/MailHandler.php | 8 +- Message/Mail/Smtp.php | 4 +- Message/RequestAbstract.php | 4 +- Message/ResponseAbstract.php | 19 +++ Module/ModuleAbstract.php | 17 +- Module/ModuleManager.php | 9 +- Stdlib/Base/FloatInt.php | 8 +- Stdlib/Base/SmartDateTime.php | 24 +-- Uri/HttpUri.php | 7 +- Utils/Barcode/TwoDAbstract.php | 1 + Utils/Git/Repository.php | 36 +--- Utils/Parser/Markdown/Markdown.php | 72 +++----- Utils/RnG/File.php | 4 +- Utils/RnG/Phone.php | 4 +- Utils/RnG/Text.php | 4 +- Utils/TaskSchedule/Schedule.php | 4 - .../Database/Schema/BuilderTest.php | 3 +- tests/Stdlib/Graph/NodeTest.php | 3 +- tests/Utils/TaskSchedule/ScheduleTest.php | 2 - 41 files changed, 377 insertions(+), 334 deletions(-) delete mode 100644 Api/Address/Validation.php create mode 100644 Business/Warehouse/OrderSuggestion.php diff --git a/Algorithm/Graph/MarkovChain.php b/Algorithm/Graph/MarkovChain.php index 93f50cde2..5cca43932 100644 --- a/Algorithm/Graph/MarkovChain.php +++ b/Algorithm/Graph/MarkovChain.php @@ -144,9 +144,7 @@ final class MarkovChain } // Couldn't find possible key - if ($new === null) { - $new = $orderValues[\array_rand($orderValues)]; - } + $new ??= $orderValues[\array_rand($orderValues)]; $output[] = $new; $key[] = $new; diff --git a/Algorithm/Rating/TrueSkill.php b/Algorithm/Rating/TrueSkill.php index 96e50e92e..e695262dc 100644 --- a/Algorithm/Rating/TrueSkill.php +++ b/Algorithm/Rating/TrueSkill.php @@ -25,7 +25,8 @@ use phpOMS\Math\Stochastic\Distribution\NormalDistribution; * @since 1.0.0 * @see https://www.moserware.com/assets/computing-your-skill/The%20Math%20Behind%20TrueSkill.pdf * - * @todo implement https://github.com/sublee/trueskill/blob/master/trueskill/__init__.py + * @todo Implement https://github.com/sublee/trueskill/blob/master/trueskill/__init__.py + * https://github.com/Karaka-Management/phpOMS/issues/337 */ class TrueSkill { diff --git a/Api/Address/Validation.php b/Api/Address/Validation.php deleted file mode 100644 index fa06e7d02..000000000 --- a/Api/Address/Validation.php +++ /dev/null @@ -1,5 +0,0 @@ -getProperty($propertyName); @@ -538,9 +536,7 @@ class DataMapperFactory \settype($objId, static::COLUMNS[static::PRIMARYFIELD]['type']); if (static::COLUMNS[static::PRIMARYFIELD]['private'] ?? false) { - if ($refClass === null) { - $refClass = new \ReflectionClass($obj); - } + $refClass ??= new \ReflectionClass($obj); $refProp = $refClass->getProperty($propertyName); $refProp->setValue($obj, $objId); diff --git a/DataStorage/Database/Mapper/DeleteMapper.php b/DataStorage/Database/Mapper/DeleteMapper.php index 7a79ec264..992deaaf5 100755 --- a/DataStorage/Database/Mapper/DeleteMapper.php +++ b/DataStorage/Database/Mapper/DeleteMapper.php @@ -78,10 +78,10 @@ final class DeleteMapper extends DataMapperAbstract return null; } - $this->deleteSingleRelation($obj, $this->mapper::BELONGS_TO, $refClass); + $this->deleteSingleRelation($obj, $this->mapper::OWNS_ONE, $refClass); $this->deleteHasMany($obj, $objId, $refClass); $this->deleteModel($objId); - $this->deleteSingleRelation($obj, $this->mapper::OWNS_ONE, $refClass); + $this->deleteSingleRelation($obj, $this->mapper::BELONGS_TO, $refClass); return $objId; } @@ -141,9 +141,7 @@ final class DeleteMapper extends DataMapperAbstract $value = null; if ($isPrivate) { - if ($refClass === null) { - $refClass = new \ReflectionClass($obj); - } + $refClass ??= new \ReflectionClass($obj); $refProp = $refClass->getProperty($member); $value = $refProp->getValue($obj); @@ -173,7 +171,6 @@ final class DeleteMapper extends DataMapperAbstract } foreach ($this->mapper::HAS_MANY as $member => $rel) { - // always if (!isset($this->with[$member]) && !isset($rel['external'])) { continue; } @@ -183,9 +180,7 @@ final class DeleteMapper extends DataMapperAbstract $values = null; if ($isPrivate) { - if ($refClass === null) { - $refClass = new \ReflectionClass($obj); - } + $refClass ??= new \ReflectionClass($obj); $refProp = $refClass->getProperty($member); $values = $refProp->getValue($obj); @@ -252,7 +247,7 @@ final class DeleteMapper extends DataMapperAbstract ->where($this->mapper::HAS_MANY[$member]['table'] . '.' . $this->mapper::HAS_MANY[$member]['self'], '=', $objId); if ($objIds !== null) { - $relQuery->where($this->mapper::HAS_MANY[$member]['table'] . '.' . $this->mapper::HAS_MANY[$member]['external'], 'in', $objIds); + $relQuery->where($this->mapper::HAS_MANY[$member]['table'] . '.' . $this->mapper::HAS_MANY[$member]['external'], 'IN', $objIds); } $sth = $this->db->con->prepare($relQuery->toSql()); diff --git a/DataStorage/Database/Mapper/ReadMapper.php b/DataStorage/Database/Mapper/ReadMapper.php index e59176c5c..d96470bb8 100755 --- a/DataStorage/Database/Mapper/ReadMapper.php +++ b/DataStorage/Database/Mapper/ReadMapper.php @@ -26,8 +26,9 @@ use phpOMS\Utils\ArrayUtils; * @link https://jingga.app * @since 1.0.0 * - * @todo Add memory cache per read mapper parent call (These should be cached: attribute types, file types, etc.) * @todo Add getArray functions to get array instead of object + * https://github.com/Karaka-Management/phpOMS/issues/350 + * * @todo Allow to define columns in all functions instead of members? * * @template R @@ -558,6 +559,9 @@ final class ReadMapper extends DataMapperAbstract // This is necessary for special cases, e.g. when joining in the other direction // Example: Show all profiles who have written a news article. // "with()" only allows to go from articles to accounts but we want to go the other way + // @feature Create join functionality for mappers which supports joining and filtering based on other tables + // Example: show all profiles which have written a news article + // https://github.com/Karaka-Management/phpOMS/issues/253 foreach ($this->join as $member => $values) { if (($col = $this->mapper::getColumnByMember($member)) === null) { continue; @@ -573,20 +577,22 @@ final class ReadMapper extends DataMapperAbstract if (isset($join['mapper']::HAS_MANY[$join['value']])) { if (isset($join['mapper']::HAS_MANY[$join['value']]['external'])) { + $relJoinTable = $join['mapper']::HAS_MANY[$join['value']]['table']; + // join with relation table - $query->join($join['mapper']::HAS_MANY[$join['value']]['table'], $join['type'], $join['mapper']::HAS_MANY[$join['value']]['table'] . '_d' . ($this->depth + 1) . $this->joinAlias) + $query->join($relJoinTable, $join['type'], $relJoinTable . '_d' . ($this->depth + 1) . $this->joinAlias) ->on( $this->mapper::TABLE . '_d' . $this->depth . $this->joinAlias . '.' . $col, '=', - $join['mapper']::HAS_MANY[$join['value']]['table'] . '_d' . ($this->depth + 1) . $this->joinAlias . '.' . $join['mapper']::HAS_MANY[$join['value']]['external'], + $relJoinTable . '_d' . ($this->depth + 1) . $this->joinAlias . '.' . $join['mapper']::HAS_MANY[$join['value']]['external'], 'AND', - $join['mapper']::HAS_MANY[$join['value']]['table'] . '_d' . ($this->depth + 1) . $this->joinAlias + $relJoinTable . '_d' . ($this->depth + 1) . $this->joinAlias ); // join with model table $query->join($join['mapper']::TABLE, $join['type'], $join['mapper']::TABLE . '_d' . ($this->depth + 1) . $this->joinAlias) ->on( - $join['mapper']::HAS_MANY[$join['value']]['table'] . '_d' . ($this->depth + 1) . $this->joinAlias . '.' . $join['mapper']::HAS_MANY[$join['value']]['self'], + $relJoinTable . '_d' . ($this->depth + 1) . $this->joinAlias . '.' . $join['mapper']::HAS_MANY[$join['value']]['self'], '=', $join['mapper']::TABLE . '_d' . ($this->depth + 1) . $this->joinAlias . '.' . $join['mapper']::PRIMARYFIELD, 'AND', @@ -755,8 +761,9 @@ final class ReadMapper extends DataMapperAbstract $query->orderBy($this->mapper::TABLE . '_d' . $this->depth . $this->joinAlias . '.' . $column, $sort['order']); + // @bug It looks like that only one sort parameter is supported despite SQL supporting multiple + // https://github.com/Karaka-Management/phpOMS/issues/364 break; // there is only one root element (one element with child === '') - // @todo Is this true? sort can have multiple sort components!!! } } @@ -792,28 +799,28 @@ final class ReadMapper extends DataMapperAbstract { $refClass = null; + $aValue = null; + $arrayPath = ''; + foreach ($this->mapper::COLUMNS as $column => $def) { $alias = $column . '_d' . $this->depth . $this->joinAlias; if (!\array_key_exists($alias, $result)) { continue; } - $value = $result[$alias]; - + $value = $result[$alias]; $hasPath = false; - $aValue = []; - $arrayPath = ''; $refProp = null; $isPrivate = $def['private'] ?? false; - $member = ''; + $member = $def['internal']; if ($isPrivate && $refClass === null) { $refClass = new \ReflectionClass($obj); } - if (\stripos($def['internal'], '/') !== false) { + if (\stripos($member, '/') !== false) { $hasPath = true; - $path = \explode('/', \ltrim($def['internal'], '/')); + $path = \explode('/', \ltrim($member, '/')); $member = $path[0]; if ($isPrivate) { @@ -825,15 +832,12 @@ final class ReadMapper extends DataMapperAbstract \array_shift($path); $arrayPath = \implode('/', $path); - } else { - if ($isPrivate) { - $refProp = $refClass->getProperty($def['internal']); - } - - $member = $def['internal']; + } elseif ($isPrivate) { + $refProp = $refClass->getProperty($member); } - if (isset($this->mapper::OWNS_ONE[$def['internal']])) { + $type = $def['type']; + if (isset($this->mapper::OWNS_ONE[$member])) { $default = null; if (!isset($this->with[$member]) && ($isPrivate ? $refProp->isInitialized($obj) : isset($obj->{$member})) @@ -841,15 +845,8 @@ final class ReadMapper extends DataMapperAbstract $default = $isPrivate ? $refProp->getValue($obj) : $obj->{$member}; } - $value = $this->populateOwnsOne($def['internal'], $result, $default); - - if (empty($value)) { - // @todo find better solution. this was because of a bug with the sales billing list query depth = 4. - // The address was set (from the client, referral or creator) but then somehow there was a second address element which was all null and null cannot be assigned to a string variable (e.g. country). - // The problem with this solution is that if the model expects an initialization (e.g. at lest set the elements to null, '', 0 etc.) this is now not done. - $value = $isPrivate ? $refProp->getValue($obj) : $obj->{$member}; - } - } elseif (isset($this->mapper::BELONGS_TO[$def['internal']])) { + $value = $this->populateOwnsOne($member, $result, $default); + } elseif (isset($this->mapper::BELONGS_TO[$member])) { $default = null; if (!isset($this->with[$member]) && ($isPrivate ? $refProp->isInitialized($obj) : isset($obj->{$member})) @@ -857,39 +854,39 @@ final class ReadMapper extends DataMapperAbstract $default = $isPrivate ? $refProp->getValue($obj) : $obj->{$member}; } - $value = $this->populateBelongsTo($def['internal'], $result, $default); - } elseif (\in_array($def['type'], ['string', 'compress', 'int', 'float', 'bool'])) { - if ($value !== null && $def['type'] === 'compress') { - $def['type'] = 'string'; - + $value = $this->populateBelongsTo($member, $result, $default); + } elseif (\in_array($type, ['string', 'compress', 'int', 'float', 'bool'])) { + if ($value !== null && $type === 'compress') { + $type = 'string'; $value = \gzinflate($value); } - $mValue = $isPrivate ? $refProp->getValue($obj) : $obj->{$member}; - if ($value !== null || $mValue !== null) { - \settype($value, $def['type']); + if ($value !== null + || ($isPrivate ? $refProp->getValue($obj) !== null : $obj->{$member} !== null) + ) { + \settype($value, $type); } if ($hasPath) { $value = ArrayUtils::setArray($arrayPath, $aValue, $value, '/', true); } - } elseif ($def['type'] === 'DateTime') { + } elseif ($type === 'DateTime') { $value = $value === null ? null : new \DateTime($value); if ($hasPath) { $value = ArrayUtils::setArray($arrayPath, $aValue, $value, '/', true); } - } elseif ($def['type'] === 'DateTimeImmutable') { + } elseif ($type === 'DateTimeImmutable') { $value = $value === null ? null : new \DateTimeImmutable($value); if ($hasPath) { $value = ArrayUtils::setArray($arrayPath, $aValue, $value, '/', true); } - } elseif ($def['type'] === 'Json') { + } elseif ($type === 'Json') { if ($hasPath) { $value = ArrayUtils::setArray($arrayPath, $aValue, $value, '/', true); } $value = \json_decode($value, true); - } elseif ($def['type'] === 'Serializable') { + } elseif ($type === 'Serializable') { $mObj = $isPrivate ? $refProp->getValue($obj) : $obj->{$member}; if ($mObj !== null && $value !== null) { @@ -905,10 +902,19 @@ final class ReadMapper extends DataMapperAbstract } } - // @todo How is this allowed? at the bottom we set $obj->hasMany = value. A hasMany should be always an array?! + if (empty($this->with)) { + return $obj; + } + + // This is only for hasMany elements where only one hasMany child object is loaded + // Example: A model usually only loads one l11n element despite having localizations for multiple languages + // @todo The code below is basically a copy of the foreach from above. + // Maybe we can combine them in a smart way without adding much overhead foreach ($this->mapper::HAS_MANY as $member => $def) { + // Only if column is defined do we have a pseudo 1-to-1 relation + // The content of the column will be loaded directly in the member variable if (!isset($this->with[$member]) - || !isset($def['column']) // @todo is this required? The code below indicates that this might be stupid + || !isset($def['column']) ) { continue; } @@ -922,8 +928,6 @@ final class ReadMapper extends DataMapperAbstract $value = $result[$alias]; $hasPath = false; - $aValue = null; - $arrayPath = '/'; $refProp = null; $isPrivate = $def['private'] ?? false; @@ -933,16 +937,18 @@ final class ReadMapper extends DataMapperAbstract if (\stripos($member, '/') !== false) { $hasPath = true; - $path = \explode('/', $member); + $path = \explode('/', \ltrim($member, '/')); $member = $path[0]; if ($isPrivate) { $refProp = $refClass->getProperty($path[0]); + $aValue = $refProp->getValue($obj); + } else { + $aValue = $obj->{$path[0]}; } \array_shift($path); $arrayPath = \implode('/', $path); - $aValue = $isPrivate ? $refProp->getValue($obj) : $obj->{$path[0]}; } elseif ($isPrivate) { $refProp = $refClass->getProperty($member); } @@ -964,12 +970,12 @@ final class ReadMapper extends DataMapperAbstract $value = ArrayUtils::setArray($arrayPath, $aValue, $value, '/', true); } } elseif ($type === 'DateTime') { - $value ??= new \DateTime($value); + $value = $value === null ? null : new \DateTime($value); if ($hasPath) { $value = ArrayUtils::setArray($arrayPath, $aValue, $value, '/', true); } } elseif ($type === 'DateTimeImmutable') { - $value ??= new \DateTimeImmutable($value); + $value = $value === null ? null : new \DateTimeImmutable($value); if ($hasPath) { $value = ArrayUtils::setArray($arrayPath, $aValue, $value, '/', true); } @@ -1022,13 +1028,9 @@ final class ReadMapper extends DataMapperAbstract } else { return $default; } - } - - if (isset($this->mapper::OWNS_ONE[$member]['column'])) { + } elseif (isset($this->mapper::OWNS_ONE[$member]['column'])) { return $result[$mapper::getColumnByMember($this->mapper::OWNS_ONE[$member]['column']) . '_d' . $this->depth . '_' . $member]; - } - - if (!isset($result[$mapper::PRIMARYFIELD . '_d' . ($this->depth + 1) . '_' . $member])) { + } elseif (!isset($result[$mapper::PRIMARYFIELD . '_d' . ($this->depth + 1) . '_' . $member])) { return $mapper::createNullModel(); } @@ -1057,28 +1059,23 @@ final class ReadMapper extends DataMapperAbstract $mapper = $this->mapper::BELONGS_TO[$member]['mapper']; if (!isset($this->with[$member])) { - if (\array_key_exists($this->mapper::BELONGS_TO[$member]['external'] . '_d' . $this->depth . '_' . $member, $result)) { + if (\array_key_exists($this->mapper::BELONGS_TO[$member]['external'] . '_d' . $this->depth . $this->joinAlias, $result)) { return isset($this->mapper::BELONGS_TO[$member]['column']) - ? $result[$this->mapper::BELONGS_TO[$member]['external'] . '_d' . $this->depth . '_' . $member] - : $mapper::createNullModel($result[$this->mapper::BELONGS_TO[$member]['external'] . '_d' . $this->depth . '_' . $member]); + ? $result[$this->mapper::BELONGS_TO[$member]['external'] . '_d' . $this->depth . $this->joinAlias] + : $mapper::createNullModel($result[$this->mapper::BELONGS_TO[$member]['external'] . '_d' . $this->depth . $this->joinAlias]); } else { return $default; } - } - - if (isset($this->mapper::BELONGS_TO[$member]['column'])) { + } elseif (isset($this->mapper::BELONGS_TO[$member]['column'])) { return $result[$mapper::getColumnByMember($this->mapper::BELONGS_TO[$member]['column']) . '_d' . $this->depth . '_' . $member]; - } - - if (!isset($result[$mapper::PRIMARYFIELD . '_d' . ($this->depth + 1) . '_' . $member])) { + } elseif (!isset($result[$mapper::PRIMARYFIELD . '_d' . ($this->depth + 1) . '_' . $member])) { return $mapper::createNullModel(); - } + } elseif (isset($this->mapper::BELONGS_TO[$member]['by'])) { + // get the belongs to based on a different column (not primary key) + // this is often used if the value is actually a different model: + // you want the profile but the account id is referenced + // in this case you can get the profile by loading the profile based on the account reference column - // get the belongs to based on a different column (not primary key) - // this is often used if the value is actually a different model: - // you want the profile but the account id is referenced - // in this case you can get the profile by loading the profile based on the account reference column - if (isset($this->mapper::BELONGS_TO[$member]['by'])) { /** @var self $belongsToMapper */ $belongsToMapper = $this->createRelationMapper($mapper::get($this->db), $member); $belongsToMapper->depth = $this->depth + 1; @@ -1086,7 +1083,7 @@ final class ReadMapper extends DataMapperAbstract $belongsToMapper->where( $this->mapper::BELONGS_TO[$member]['by'], - $result[$mapper::getColumnByMember($this->mapper::BELONGS_TO[$member]['by']) . '_d' . ($this->depth + 1) . $this->joinAlias], + $result[$mapper::getColumnByMember($this->mapper::BELONGS_TO[$member]['by']) . '_d' . ($this->depth + 1) . '_' . $member], '=' ); @@ -1162,9 +1159,7 @@ final class ReadMapper extends DataMapperAbstract } if ($isPrivate) { - if ($refClass === null) { - $refClass = new \ReflectionClass($obj); - } + $refClass ??= new \ReflectionClass($obj); foreach ($primaryKeys as $idx => $key) { if (!isset($objects[$key])) { @@ -1208,9 +1203,7 @@ final class ReadMapper extends DataMapperAbstract $tempObjs = []; if ($isPrivate) { - if ($refClass === null) { - $refClass = new \ReflectionClass($obj); - } + $refClass ??= new \ReflectionClass($obj); $refProp = $refClass->getProperty($member); @@ -1250,8 +1243,9 @@ final class ReadMapper extends DataMapperAbstract $refClass = null; - // @todo check if there are more cases where the relation is already loaded with joins etc. - // there can be pseudo hasMany elements like localizations. They are hasMany but these are already loaded with joins! + // @performance Check if there are more cases where the relation is already loaded with joins etc. + // There can be pseudo hasMany elements like localizations. They are hasMany but these are already loaded with joins! + // Variation of https://github.com/Karaka-Management/phpOMS/issues/363 foreach ($this->with as $member => $withData) { if (isset($this->mapper::HAS_MANY[$member])) { $many = $this->mapper::HAS_MANY[$member]; @@ -1259,7 +1253,6 @@ final class ReadMapper extends DataMapperAbstract continue; } - // @todo withData doesn't store this directly, it is in [0]['private] ?!?! $isPrivate = $many['private'] ?? false; $objectMapper = $this->createRelationMapper($many['mapper']::exists(db: $this->db), $member); @@ -1295,9 +1288,7 @@ final class ReadMapper extends DataMapperAbstract $isPrivate = $relation['private'] ?? false; if ($isPrivate) { - if ($refClass === null) { - $refClass = new \ReflectionClass($obj); - } + $refClass ??= new \ReflectionClass($obj); $refProp = $refClass->getProperty($member); return $relMapper->hasManyRelations($refProp->getValue($obj)); diff --git a/DataStorage/Database/Mapper/UpdateMapper.php b/DataStorage/Database/Mapper/UpdateMapper.php index 3b7f07a85..9407d2006 100755 --- a/DataStorage/Database/Mapper/UpdateMapper.php +++ b/DataStorage/Database/Mapper/UpdateMapper.php @@ -115,7 +115,10 @@ final class UpdateMapper extends DataMapperAbstract ->where($this->mapper::TABLE . '.' . $this->mapper::PRIMARYFIELD, '=', $objId); foreach ($this->mapper::COLUMNS as $column) { - $propertyName = \stripos($column['internal'], '/') !== false ? \explode('/', $column['internal'])[0] : $column['internal']; + $propertyName = \stripos($column['internal'], '/') !== false + ? \explode('/', $column['internal'])[0] + : $column['internal']; + if (isset($this->mapper::HAS_MANY[$propertyName]) || $column['internal'] === $this->mapper::PRIMARYFIELD || (($column['readonly'] ?? false) && !isset($this->with[$propertyName])) @@ -129,9 +132,7 @@ final class UpdateMapper extends DataMapperAbstract $tValue = null; if ($isPrivate) { - if ($refClass === null) { - $refClass = new \ReflectionClass($obj); - } + $refClass ??= new \ReflectionClass($obj); $property = $refClass->getProperty($propertyName); $tValue = $property->getValue($obj); @@ -273,9 +274,7 @@ final class UpdateMapper extends DataMapperAbstract $values = null; if ($isPrivate) { - if ($refClass === null) { - $refClass = new \ReflectionClass($obj); - } + $refClass ??= new \ReflectionClass($obj); $property = $refClass->getProperty($propertyName); $values = $property->getValue($obj); diff --git a/DataStorage/Database/Mapper/WriteMapper.php b/DataStorage/Database/Mapper/WriteMapper.php index 1ac994088..fe87a3018 100755 --- a/DataStorage/Database/Mapper/WriteMapper.php +++ b/DataStorage/Database/Mapper/WriteMapper.php @@ -119,9 +119,7 @@ final class WriteMapper extends DataMapperAbstract $tValue = null; if ($column['private'] ?? false) { - if ($refClass === null) { - $refClass = new \ReflectionClass($obj); - } + $refClass ??= new \ReflectionClass($obj); $property = $refClass->getProperty($propertyName); $tValue = $property->getValue($obj); @@ -227,7 +225,6 @@ final class WriteMapper extends DataMapperAbstract if (isset($this->mapper::BELONGS_TO[$propertyName]['by'])) { // has by (obj is stored as a different model e.g. model = profile but reference/db is account) - if ($this->mapper::BELONGS_TO[$propertyName]['private'] ?? false) { $refClass = new \ReflectionClass($obj); $refProp = $refClass->getProperty($this->mapper::BELONGS_TO[$propertyName]['by']); @@ -241,7 +238,8 @@ final class WriteMapper extends DataMapperAbstract $mapper = $this->mapper::BELONGS_TO[$propertyName]['mapper']; $primaryKey = $mapper::getObjectId($obj); - // @todo the $mapper::create() might cause a problem if 'by' is set. because we don't want to create this obj but the child obj. + // @bug The $mapper::create() might cause a problem if 'by' is set. + // This is because we don't want to create this obj but the child obj. return empty($primaryKey) ? $mapper::create(db: $this->db)->execute($obj) : $primaryKey; @@ -272,9 +270,7 @@ final class WriteMapper extends DataMapperAbstract $values = null; if ($isPrivate) { - if ($refClass === null) { - $refClass = new \ReflectionClass($obj); - } + $refClass ??= new \ReflectionClass($obj); $property = $refClass->getProperty($propertyName); $values = $property->getValue($obj); @@ -285,13 +281,11 @@ final class WriteMapper extends DataMapperAbstract /** @var class-string $mapper */ $mapper = $this->mapper::HAS_MANY[$propertyName]['mapper']; $internalName = $mapper::COLUMNS[$this->mapper::HAS_MANY[$propertyName]['self']]['internal'] ?? 'ERROR-BAD-SELF'; - - // @todo this or $isRelPrivate is wrong, don't know which one. - $isInternalPrivate = $mapper::COLUMNS[$this->mapper::HAS_MANY[$propertyName]['self']]['private'] ?? false; + $isRelPrivate = $mapper::COLUMNS[$this->mapper::HAS_MANY[$propertyName]['self']]['private'] ?? false; if (\is_object($values)) { // conditionals - if ($isInternalPrivate) { + if ($isRelPrivate) { $relReflectionClass = new \ReflectionClass($values); $relProperty = $relReflectionClass->getProperty($internalName); @@ -302,16 +296,13 @@ final class WriteMapper extends DataMapperAbstract $mapper::create(db: $this->db)->execute($values); continue; - } - - if (!\is_array($values)) { - // @todo conditionals??? + } elseif (!\is_array($values) || empty($values)) { + // @todo conditionals? continue; } $objsIds = []; - $isRelPrivate = $mapper::COLUMNS[$this->mapper::HAS_MANY[$propertyName]['self']]['private'] ?? false; - $relReflectionClass = $isRelPrivate && !empty($values) ? new \ReflectionClass(\reset($values)) : null; + $relReflectionClass = $isRelPrivate ? new \ReflectionClass(\reset($values)) : null; foreach ($values as $key => $value) { if (!\is_object($value)) { @@ -337,7 +328,6 @@ final class WriteMapper extends DataMapperAbstract $relProperty = $relReflectionClass->getProperty($internalName); } - // @todo maybe consider to just set the column type to object, and then check for that (might be faster) if (isset($mapper::BELONGS_TO[$internalName]) || isset($mapper::OWNS_ONE[$internalName]) ) { @@ -353,11 +343,12 @@ final class WriteMapper extends DataMapperAbstract } } - // @todo This inserts one element at a time. SQL allows to insert multiple rows. - // The problem with this is, that we then need to manually calculate the objIds - // since lastInsertId returns the first generated id. - // However, the current use case in Jingga only rarely has multiple hasMany during the creation - // since we are calling the API individually. + // @performance This inserts one element at a time. SQL allows to insert multiple rows. + // The problem with this is, that we then need to manually calculate the objIds + // since lastInsertId returns the first generated id. + // However, the current use case in Jingga only rarely has multiple hasMany during the creation + // since we are calling the API individually. + // https://github.com/Karaka-Management/phpOMS/issues/370 $objsIds[$key] = $mapper::create(db: $this->db)->execute($value); } diff --git a/DataStorage/Database/Query/Builder.php b/DataStorage/Database/Query/Builder.php index eb3b28f6d..03477610e 100755 --- a/DataStorage/Database/Query/Builder.php +++ b/DataStorage/Database/Query/Builder.php @@ -841,10 +841,6 @@ class Builder extends BuilderAbstract */ public function count(string $table = '*') : self { - /** - * @todo Don't do this as a string, create a new object $this->select(new Count($table)). - * The parser should be able to handle this much better - */ return $this->select('COUNT(' . $table . ')'); } diff --git a/DataStorage/Database/Query/Grammar/Grammar.php b/DataStorage/Database/Query/Grammar/Grammar.php index 1b7597bdd..87e54c638 100755 --- a/DataStorage/Database/Query/Grammar/Grammar.php +++ b/DataStorage/Database/Query/Grammar/Grammar.php @@ -459,9 +459,10 @@ class Grammar extends GrammarAbstract $expression .= '(' . \rtrim($this->compileWhereQuery($element['column']), ';') . ')'; } - // @todo on doesn't allow string values as value (only table column names). This is bad and needs to be fixed! - // Solution could be to use ColumnName as internal object and then pass it to compileValue in all cases - // Other types such as int etc. are kind of possible + // @bug The on part of a join doesn't allow string values because they conflict with column name + // Other data types are possible because they don't conflict with the data type of columns (string) + // Consider to create a ColumnName() class. + // https://github.com/Karaka-Management/phpOMS/issues/369 if (isset($element['value'])) { $expression .= ' ' . \strtoupper($element['operator']) . ' ' . (\is_string($element['value']) ? $this->compileSystem($element['value']) : $element['value']); diff --git a/DataStorage/Database/Schema/Grammar/SQLiteGrammar.php b/DataStorage/Database/Schema/Grammar/SQLiteGrammar.php index 744dcd6e1..5411b321b 100755 --- a/DataStorage/Database/Schema/Grammar/SQLiteGrammar.php +++ b/DataStorage/Database/Schema/Grammar/SQLiteGrammar.php @@ -14,6 +14,7 @@ declare(strict_types=1); namespace phpOMS\DataStorage\Database\Schema\Grammar; +use phpOMS\DataStorage\Database\BuilderAbstract; use phpOMS\DataStorage\Database\Query\Builder; use phpOMS\DataStorage\Database\Schema\Builder as SchemaBuilder; @@ -44,14 +45,7 @@ class SQLiteGrammar extends Grammar public string $systemIdentifierEnd = '`'; /** - * Compile from. - * - * @param SchemaBuilder $query Builder - * @param array $table Tables - * - * @return string - * - * @since 1.0.0 + * {@inheritdoc} */ protected function compileSelectTables(SchemaBuilder $query, array $table) : string { @@ -64,14 +58,7 @@ class SQLiteGrammar extends Grammar } /** - * Compile from. - * - * @param SchemaBuilder $query Builder - * @param string $table Tables - * - * @return string - * - * @since 1.0.0 + * {@inheritdoc} */ protected function compileSelectFields(SchemaBuilder $query, string $table) : string { @@ -84,22 +71,55 @@ class SQLiteGrammar extends Grammar } /** - * Compile create table fields query. + * Parses the sql data types to the appropriate SQLite data types * - * @param SchemaBuilder $query Query - * @param array $fields Fields to create + * @param string $type Data type * * @return string * * @since 1.0.0 */ + private function parseFieldType(string $type) : string + { + $type = \strtoupper($type); + + if (\str_starts_with($type, 'INT') + || \str_starts_with($type, 'TINYINT') + || \str_starts_with($type, 'SMALLINT') + || \str_starts_with($type, 'BIGINT') + ) { + return 'INTEGER'; + } elseif (\str_starts_with($type, 'VARCHAR')) { + return 'TEXT'; + } elseif (\str_starts_with($type, 'DATETIME')) { + return 'TEXT'; + } elseif (\str_starts_with($type, 'DECIMAL')) { + return 'REAL'; + } elseif (\stripos($type, 'BINARY') !== false) { + return 'BLOB'; + } + + return $type; + } + + /** + * {@inheritdoc} + */ + protected function compileCreateTable(BuilderAbstract $query, string $table) : string + { + return 'CREATE TABLE ' . $this->expressionizeTableColumn([$table]); + } + + /** + * {@inheritdoc} + */ protected function compileCreateFields(SchemaBuilder $query, array $fields) : string { $fieldQuery = ''; $keys = ''; foreach ($fields as $name => $field) { - $fieldQuery .= ' ' . $this->expressionizeTableColumn([$name]) . ' ' . $field['type']; + $fieldQuery .= ' ' . $this->expressionizeTableColumn([$name]) . ' ' . $this->parseFieldType($field['type']); if (isset($field['default']) || ($field['default'] === null && ($field['null'] ?? false))) { $fieldQuery .= ' DEFAULT ' . $this->compileValue($query, $field['default']); @@ -109,16 +129,16 @@ class SQLiteGrammar extends Grammar $fieldQuery .= ' ' . ($field['null'] ? '' : 'NOT ') . 'NULL'; } + if ($field['primary'] ?? false) { + $keys .= ' PRIMARY KEY'; + } + if ($field['autoincrement'] ?? false) { - $fieldQuery .= ' AUTO_INCREMENT'; + $fieldQuery .= ' AUTOINCREMENT'; } $fieldQuery .= ','; - if ($field['primary'] ?? false) { - $keys .= ' PRIMARY KEY (' . $this->expressionizeTableColumn([$name]) . '),'; - } - if ($field['unique'] ?? false) { $keys .= ' UNIQUE KEY (' . $this->expressionizeTableColumn([$name]) . '),'; } diff --git a/Localization/BaseStringL11n.php b/Localization/BaseStringL11n.php index 17bebc279..326a633b3 100755 --- a/Localization/BaseStringL11n.php +++ b/Localization/BaseStringL11n.php @@ -40,11 +40,12 @@ class BaseStringL11n implements \JsonSerializable */ public string $name = ''; - // @todo Karaka/phpOMS#357 this feels like $name and $type accomplish the same thing - // maybe we can always use $type and remove $name. - // This would require some smart mapper adjustment where the name is part of the l11n model, - // maybe use the path definition in the mapper which is used by arrays (e.g. type/name) - // More maybe: $name might have been intended as internal value? -> Makes no sense because only string + // @question Check if $name and $type accomplish the same thing + // Maybe we can always use $type and remove $name. + // This would require some smart mapper adjustment where the name is part of the l11n model, + // Maybe use the path definition in the mapper which is used by arrays (e.g. type/name) + // More maybe: $name might have been intended as internal value? -> Makes no sense because only string + // https://github.com/Karaka-Management/phpOMS/issues/357 public ?BaseStringL11nType $type = null; /** diff --git a/Localization/LanguageDetection/NgramParser.php b/Localization/LanguageDetection/NgramParser.php index 06e2cfc6b..20568df8d 100755 --- a/Localization/LanguageDetection/NgramParser.php +++ b/Localization/LanguageDetection/NgramParser.php @@ -69,9 +69,7 @@ abstract class NgramParser */ private function tokenize(string $str) : array { - if ($this->tokenizer === null) { - $this->tokenizer = new WhitespaceTokenizer(); - } + $this->tokenizer ??= new WhitespaceTokenizer(); return $this->tokenizer->tokenize($str); } diff --git a/Math/Geometry/ConvexHull/GrahamScan.php b/Math/Geometry/ConvexHull/GrahamScan.php index 681cbeeaa..24dbab119 100644 --- a/Math/Geometry/ConvexHull/GrahamScan.php +++ b/Math/Geometry/ConvexHull/GrahamScan.php @@ -69,7 +69,6 @@ final class GrahamScan /** @var array $subpoints */ $subpoints = \array_slice($points, 2, $n); \usort($subpoints, function (array $a, array $b) use ($c) : int { - // @todo Might be wrong order of comparison return \atan2($a['y'] - $c['y'], $a['x'] - $c['x']) <=> \atan2($b['y'] - $c['y'], $b['x'] - $c['x']); }); diff --git a/Math/Matrix/Vector.php b/Math/Matrix/Vector.php index 42eb90985..2ee9e3cf0 100755 --- a/Math/Matrix/Vector.php +++ b/Math/Matrix/Vector.php @@ -124,7 +124,7 @@ final class Vector extends Matrix } /** - * Calculate the eucledian dot product + * Calculate the euclidean dot product * * @param self $vector Vector * @@ -200,4 +200,25 @@ final class Vector extends Matrix return self::fromArray($crossArray); } + + /* + public function cross(self $vector) : float + { + $mat = []; + for ($i = 0; $i < $this->n; ++$i) { + for ($j = 0; $j < $this->n; ++$j) { + $mat[$i][$j] = ($i === 0) + ? $this->matrix[$j][0] + : (($i === 1) + ? $vector->matrix[$j][0] + : 0 + ); + } + } + + $matrix = Matrix::fromArray($mat); + + return $matrix->det(); + } + */ } diff --git a/Math/Optimization/Simplex.php b/Math/Optimization/Simplex.php index 439085865..88d06a93c 100644 --- a/Math/Optimization/Simplex.php +++ b/Math/Optimization/Simplex.php @@ -370,8 +370,11 @@ final class Simplex $this->b = $b; $this->c = $c; - // @todo createSlackForm() required? - // @todo create minimize + // @question Consider to generate slack form. It this required? + // https://github.com/Karaka-Management/phpOMS/issues/349 + + // @feature Handle minimize and maximize in Simplex + // https://github.com/Karaka-Management/phpOMS/issues/368 $this->m = \count($A); if ($this->m < 1) { diff --git a/Math/Solver/Root/Illinois.php b/Math/Solver/Root/Illinois.php index 4d9b1a7bc..1e7965762 100644 --- a/Math/Solver/Root/Illinois.php +++ b/Math/Solver/Root/Illinois.php @@ -61,7 +61,7 @@ final class Illinois public static function root(callable $func, float $a, float $b, int $maxIterations = 100) : float { if ($func($a) * $func($b) >= 0) { - throw new \Exception("Function values at endpoints must have opposite signs."); + throw new \Exception('Function values at endpoints must have opposite signs.'); } $c = $b; @@ -76,9 +76,7 @@ final class Illinois return $c; } - // @todo c might be wrong, could be that if and else must be switched - // @see https://en.wikipedia.org/wiki/Regula_falsi#The_Illinois_algorithm - if ($y * $fa < 0) { + if ($fa < 0) { $c = $sign === (int) ($y >= 0) ? (0.5 * $a * $fb - $b * $fa) / (0.5 * $fb - $fa) : ($a * $fb - $b * $fa) / ($fb - $fa); diff --git a/Message/Http/HttpRequest.php b/Message/Http/HttpRequest.php index 2be173972..361e422d8 100755 --- a/Message/Http/HttpRequest.php +++ b/Message/Http/HttpRequest.php @@ -220,9 +220,7 @@ final class HttpRequest extends RequestAbstract // @codeCoverageIgnoreStart // Tested but coverage doesn't show up if (\str_starts_with($lineRaw, '--')) { - if ($boundary === null) { - $boundary = \rtrim($lineRaw); - } + $boundary ??= \rtrim($lineRaw); continue; } diff --git a/Message/Http/Rest.php b/Message/Http/Rest.php index 9dd6b6adf..7c4efb92d 100755 --- a/Message/Http/Rest.php +++ b/Message/Http/Rest.php @@ -78,8 +78,6 @@ final class Rest break; } - // @todo how to implement GET request with $request->data (should it alter the uri or still get put into the body?) - // handle none-get if ($request->getMethod() !== RequestMethod::GET && !empty($request->data)) { // handle different content types @@ -92,11 +90,12 @@ final class Rest } elseif (\in_array(MimeType::M_MULT, $contentType)) { $boundary = '----' . \uniqid(); - /* @phpstan-ignore-next-line */ $data = self::createMultipartData($boundary, $request->data); // @todo Replace boundary/ with the correct boundary= in the future. - // Currently this cannot be done due to a bug. If we do it now the server cannot correctly populate php://input + // Currently this cannot be done due to a bug. + // If we do it now the server cannot correctly populate php://input + // https://github.com/Karaka-Management/phpOMS/issues/345 $headers['Content-Type'] = 'Content-Type: multipart/form-data; boundary/' . $boundary; $headers['content-length'] = 'Content-Length: ' . \strlen($data); diff --git a/Message/Mail/Email.php b/Message/Mail/Email.php index ba725ada4..389258b2f 100755 --- a/Message/Mail/Email.php +++ b/Message/Mail/Email.php @@ -118,6 +118,14 @@ class Email implements MessageInterface */ public string $mailer = SubmitType::MAIL; + /** + * Template strings + * + * @var array + * @since 1.0.0 + */ + public array $template = []; + /** * Mail from. * @@ -615,6 +623,20 @@ class Email implements MessageInterface return $addresses; } + public function parseTemplate() : void + { + if (empty($this->template)) { + return; + } + + $keys = \array_keys($this->template); + $values = \array_values($this->template); + + $this->subject = \str_replace($keys, $values, $this->subject); + $this->body = \str_replace($keys, $values, $this->body); + $this->bodyAlt = \str_replace($keys, $values, $this->bodyAlt); + } + /** * Pre-send preparations * @@ -626,12 +648,20 @@ class Email implements MessageInterface */ public function preSend(string $mailer) : bool { + if (empty($this->from) + || (empty($this->to) && empty($this->cc) && empty($this->bcc)) + ) { + return false; + } + $this->header = ''; $this->mailer = $mailer; - if (empty($this->to) && empty($this->cc) && empty($this->bcc)) { - return false; - } + $tempSubject = $this->subject; + $tempBody = $this->body; + $tempBodyAlt = $this->bodyAlt; + + $this->parseTemplate(); if (!empty($this->bodyAlt)) { $this->contentType = MimeType::M_ALT; @@ -642,9 +672,9 @@ class Email implements MessageInterface $this->headerMime = ''; $this->bodyMime = $this->createBody(); - $tempheaders = $this->headerMime; + $tempHeaders = $this->headerMime; $this->headerMime = $this->createHeader(); - $this->headerMime .= $tempheaders; + $this->headerMime .= $tempHeaders; if ($this->mailer === SubmitType::MAIL) { $this->header .= empty($this->to) @@ -674,6 +704,10 @@ class Email implements MessageInterface self::normalizeBreaks($headerDkim, self::$LE) . self::$LE; } + $this->subject = $tempSubject; + $this->body = $tempBody; + $this->bodyAlt = $tempBodyAlt; + return true; } diff --git a/Message/Mail/MailHandler.php b/Message/Mail/MailHandler.php index e3ab31749..5baabf506 100755 --- a/Message/Mail/MailHandler.php +++ b/Message/Mail/MailHandler.php @@ -480,17 +480,13 @@ class MailHandler */ public function smtpConnect(?array $options = null) : bool { - if ($this->smtp === null) { - $this->smtp = new Smtp(); - } + $this->smtp ??= new Smtp(); if ($this->smtp->isConnected()) { return true; } - if ($options === null) { - $options = $this->smtpOptions; - } + $options ??= $this->smtpOptions; $this->smtp->timeout = $this->timeout; $this->smtp->doVerp = $this->useVerp; diff --git a/Message/Mail/Smtp.php b/Message/Mail/Smtp.php index e2898156e..339949152 100755 --- a/Message/Mail/Smtp.php +++ b/Message/Mail/Smtp.php @@ -163,9 +163,7 @@ class Smtp protected function getSMTPConnection(string $host, int $port = 25, int $timeout = 30, array $options = []) : mixed { static $streamok; - if ($streamok === null) { - $streamok = \function_exists('stream_socket_client'); - } + $streamok ??= \function_exists('stream_socket_client'); $errno = 0; $errstr = ''; diff --git a/Message/RequestAbstract.php b/Message/RequestAbstract.php index f9cac264f..2fb181750 100755 --- a/Message/RequestAbstract.php +++ b/Message/RequestAbstract.php @@ -275,9 +275,7 @@ abstract class RequestAbstract implements MessageInterface } $json = \json_decode($this->data[$key], true); /** @phpstan-ignore-line */ - if ($json === null) { - $json = $this->data[$key]; - } + $json ??= $this->data[$key]; return \is_array($json) ? $json : [$json]; } diff --git a/Message/ResponseAbstract.php b/Message/ResponseAbstract.php index 47b01277c..40c8eb54c 100755 --- a/Message/ResponseAbstract.php +++ b/Message/ResponseAbstract.php @@ -309,6 +309,25 @@ abstract class ResponseAbstract implements \JsonSerializable, MessageInterface $this->data[$key] = $response; } + /** + * Add response. + * + * @param mixed $key Response id + * @param mixed $response Response to add + * + * @return void + * + * @since 1.0.0 + */ + public function add(mixed $key, mixed $response) : void + { + if (!isset($this->data[$key])) { + $this->data[$key] = []; + } + + $this->data[$key][] = $response; + } + /** * {@inheritdoc} */ diff --git a/Module/ModuleAbstract.php b/Module/ModuleAbstract.php index 7f5b767a5..d1d12e6fd 100755 --- a/Module/ModuleAbstract.php +++ b/Module/ModuleAbstract.php @@ -901,11 +901,8 @@ abstract class ModuleAbstract * * @return void * - * @feature Implement softDelete functionality. - * Models which have a soft delete cannot be used, read or modified unless a person has soft delete permissions - * In addition to DELETE permissions we now need SOFTDELETE as well. - * There also needs to be an undo function for this soft delete - * In a backend environment a soft delete would be very helpful!!! + * @question Consider to implement softDelete functionality + * https://github.com/Karaka-Management/Karaka/issues/276 * * @since 1.0.0 */ @@ -975,7 +972,7 @@ abstract class ModuleAbstract * * @param int $account Account id * @param mixed $rel1 Object relation1 - * @param mixed $rel2 Object relation2 + * @param mixed $rel2 Object relation2 (null = remove all related to $rel1) * @param string $mapper Object mapper * @param string $field Relation field * @param string $trigger Trigger for the event manager @@ -990,7 +987,13 @@ abstract class ModuleAbstract $trigger = static::NAME . '-' . $trigger . '-relation-delete'; $this->app->eventManager->triggerSimilar('PRE:Module:' . $trigger, '', $rel1); - $mapper::remover()->deleteRelationTable($field, \is_array($rel2) ? $rel2 : [$rel2], $rel1); + $mapper::remover()->deleteRelationTable( + $field, + $rel2 === null + ? null + : (\is_array($rel2) ? $rel2 : [$rel2]), + $rel1 + ); $data = [ $account, diff --git a/Module/ModuleManager.php b/Module/ModuleManager.php index cb10e3fc0..539c012ed 100755 --- a/Module/ModuleManager.php +++ b/Module/ModuleManager.php @@ -657,10 +657,11 @@ final class ModuleManager * @return object|\phpOMS\Module\ModuleAbstract * * @todo Remove docblock type hint hack "object". - * The return type object is only used to stop the annoying warning that a method doesn't exist - * if you chain call the methods part of the returned ModuleAbstract implementation. - * Remove it once alternative inline type hinting is possible for the specific returned implementation. - * This also causes phpstan type inspection errors, which we have to live with or ignore in the settings + * The return type object is only used to stop the annoying warning that a method doesn't exist + * if you chain call the methods part of the returned ModuleAbstract implementation. + * Remove it once alternative inline type hinting is possible for the specific returned implementation. + * This also causes phpstan type inspection errors, which we have to live with or ignore in the settings + * https://github.com/Karaka-Management/phpOMS/issues/300 * * @since 1.0.0 */ diff --git a/Stdlib/Base/FloatInt.php b/Stdlib/Base/FloatInt.php index ccd4f4b02..5207d5dc9 100755 --- a/Stdlib/Base/FloatInt.php +++ b/Stdlib/Base/FloatInt.php @@ -186,9 +186,7 @@ class FloatInt implements SerializableInterface throw new \Exception(); // @codeCoverageIgnore } - if ($decimals === null) { - $decimals = \strlen(\rtrim($right, '0')); - } + $decimals ??= \strlen(\rtrim($right, '0')); return $decimals > 0 ? \number_format((float) $left, 0, $this->decimal, $this->thousands) . $this->decimal . \substr($right, 0, $decimals) @@ -224,9 +222,7 @@ class FloatInt implements SerializableInterface throw new \Exception(); // @codeCoverageIgnore } - if ($decimals === null) { - $decimals = \strlen(\rtrim($right, '0')); - } + $decimals ??= \strlen(\rtrim($right, '0')); return $decimals > 0 ? \number_format((float) $left, 0, $this->decimal, '') . $this->decimal . \substr($right, 0, $decimals) diff --git a/Stdlib/Base/SmartDateTime.php b/Stdlib/Base/SmartDateTime.php index 1d7ef7ee0..976c8b8d9 100755 --- a/Stdlib/Base/SmartDateTime.php +++ b/Stdlib/Base/SmartDateTime.php @@ -365,13 +365,13 @@ class SmartDateTime extends \DateTime * * @param int $month Start of the year (i.e. fiscal year) * - * @return \DateTime + * @return SmartDateTime * * @since 1.0.0 */ - public static function startOfYear(int $month = 1) : \DateTime + public static function startOfYear(int $month = 1) : SmartDateTime { - return new \DateTime(\date('Y') . '-' . \sprintf('%02d', $month) . '-01'); + return new SmartDateTime(\date('Y') . '-' . \sprintf('%02d', $month) . '-01'); } /** @@ -379,37 +379,37 @@ class SmartDateTime extends \DateTime * * @param int $month Start of the year (i.e. fiscal year) * - * @return \DateTime + * @return SmartDateTime * * @since 1.0.0 */ - public static function endOfYear(int $month = 1) : \DateTime + public static function endOfYear(int $month = 1) : SmartDateTime { - return new \DateTime(\date('Y') . '-' . self::calculateMonthIndex(13 - $month, $month) . '-31'); + return new SmartDateTime(\date('Y') . '-' . self::calculateMonthIndex(13 - $month, $month) . '-31'); } /** * Get the start of the month * - * @return \DateTime + * @return SmartDateTime * * @since 1.0.0 */ - public static function startOfMonth() : \DateTime + public static function startOfMonth() : SmartDateTime { - return new \DateTime(\date('Y-m') . '-01'); + return new SmartDateTime(\date('Y-m') . '-01'); } /** * Get the end of the month * - * @return \DateTime + * @return SmartDateTime * * @since 1.0.0 */ - public static function endOfMonth() : \DateTime + public static function endOfMonth() : SmartDateTime { - return new \DateTime(\date('Y-m-t')); + return new SmartDateTime(\date('Y-m-t')); } /** diff --git a/Uri/HttpUri.php b/Uri/HttpUri.php index 95c50ed2f..45e53d7f7 100755 --- a/Uri/HttpUri.php +++ b/Uri/HttpUri.php @@ -404,8 +404,11 @@ final class HttpUri implements UriInterface $this->queryString .= $toAdd; - // @todo handle existing string at the end of uri (e.g. #fragment) - $this->uri .= $toAdd; + if (empty($this->fragment)) { + $this->uri .= $toAdd; + } else { + $this->uri = \substr_replace($this->uri, $toAdd, \strrpos($this->uri, '#'), 0); + } } /** diff --git a/Utils/Barcode/TwoDAbstract.php b/Utils/Barcode/TwoDAbstract.php index 313e6a970..8a9196b3f 100755 --- a/Utils/Barcode/TwoDAbstract.php +++ b/Utils/Barcode/TwoDAbstract.php @@ -81,6 +81,7 @@ abstract class TwoDAbstract extends CodeAbstract $locationX = $this->margin; // @todo Allow manual dimensions + // https://github.com/Karaka-Management/phpOMS/issues/346 for ($posX = 0; $posX < $width; ++$posX) { $locationY = $this->margin; diff --git a/Utils/Git/Repository.php b/Utils/Git/Repository.php index 30329ed83..3efbacdef 100755 --- a/Utils/Git/Repository.php +++ b/Utils/Git/Repository.php @@ -692,13 +692,8 @@ class Repository */ public function getContributors(?\DateTime $start = null, ?\DateTime $end = null) : array { - if ($start === null) { - $start = new \DateTime('1970-12-31'); - } - - if ($end === null) { - $end = new \DateTime('now'); - } + $start ??= new \DateTime('1970-12-31'); + $end ??= new \DateTime('now'); $lines = $this->run('shortlog -s -n --since="' . $start->format('Y-m-d') . '" --before="' . $end->format('Y-m-d') . '" --all'); $contributors = []; @@ -732,13 +727,8 @@ class Repository */ public function getCommitsCount(?\DateTime $start = null, ?\DateTime $end = null) : array { - if ($start === null) { - $start = new \DateTime('1970-12-31'); - } - - if ($end === null) { - $end = new \DateTime('now'); - } + $start ??= new \DateTime('1970-12-31'); + $end ??= new \DateTime('now'); $lines = $this->run('shortlog -s -n --since="' . $start->format('Y-m-d') . '" --before="' . $end->format('Y-m-d') . '" --all'); $commits = []; @@ -768,13 +758,8 @@ class Repository */ public function getAdditionsRemovalsByContributor(Author $author, ?\DateTime $start = null, ?\DateTime $end = null) : array { - if ($start === null) { - $start = new \DateTime('1900-01-01'); - } - - if ($end === null) { - $end = new \DateTime('now'); - } + $start ??= new \DateTime('1900-01-01'); + $end ??= new \DateTime('now'); $addremove = ['added' => 0, 'removed' => 0]; $lines = $this->run( @@ -819,13 +804,8 @@ class Repository */ public function getCommitsBy(?\DateTime $start = null, ?\DateTime $end = null, ?Author $author = null) : array { - if ($start === null) { - $start = new \DateTime('1970-12-31'); - } - - if ($end === null) { - $end = new \DateTime('now'); - } + $start ??= new \DateTime('1970-12-31'); + $end ??= new \DateTime('now'); $author = $author === null ? '' : ' --author=' . \escapeshellarg($author->name) . ''; diff --git a/Utils/Parser/Markdown/Markdown.php b/Utils/Parser/Markdown/Markdown.php index 1966520ec..245db4a54 100755 --- a/Utils/Parser/Markdown/Markdown.php +++ b/Utils/Parser/Markdown/Markdown.php @@ -33,7 +33,7 @@ use phpOMS\Uri\UriFactory; * @see https://github.com/doowzs/parsedown-extreme * @since 1.0.0 * - * @todo Add + * @todo Add special markdown content * 1. Calendar (own widget) * 2. Event (own widget) * 3. Tasks (own widget) @@ -49,6 +49,7 @@ use phpOMS\Uri\UriFactory; * 13. Checklist (own widget) * 14. Gallery * 15. Form (own widget) + * https://github.com/Karaka-Management/phpOMS/issues/290 */ class Markdown { @@ -2560,39 +2561,18 @@ class Markdown return null; } - // @todo We are parsing the language here and further down. Shouldn't one time be enough? - // Both variations seem to result in the same result?! $language = \trim(\preg_replace('/^`{3}([^\s]+)(.+)?/s', '$1', $line['text'])); - // Handle diagrams if (!($this->options['diagrams'] ?? true) || !\in_array($language, ['mermaid', 'chart']) ) { - $infostring = \trim(\substr($line['text'], $openerLength), "\t "); - if (\strpos($infostring, '`') !== false) { - return null; - } - + // Is code block $element = [ 'name' => 'code', 'text' => '', ]; - if ($infostring !== '') { - /** - * https://www.w3.org/TR/2011/WD-html5-20110525/elements.html#classes - * Every HTML element may have a class attribute specified. - * The attribute, if specified, must have a value that is a set - * of space-separated tokens representing the various classes - * that the element belongs to. - * [...] - * The space characters, for the purposes of this specification, - * are U+0020 SPACE, U+0009 CHARACTER TABULATION (tab), - * U+000A LINE FEED (LF), U+000C FORM FEED (FF), and - * U+000D CARRIAGE RETURN (CR). - */ - $language = \substr($infostring, 0, \strcspn($infostring, " \t\n\f\r")); - + if ($language !== '```' && !empty($language)) { $element['attributes'] = ['class' => "language-{$language}"]; } @@ -2604,26 +2584,7 @@ class Markdown 'element' => $element, ], ]; - } - - if (\strtolower($language) === 'mermaid') { - // Mermaid.js https://mermaidjs.github.io - $element = [ - 'text' => '', - ]; - - return [ - 'char' => $marker, - 'openerLength' => $openerLength, - 'element' => [ - 'element' => $element, - 'name' => 'div', - 'attributes' => [ - 'class' => 'mermaid', - ], - ], - ]; - } elseif (\strtolower($language) === 'chart') { + } elseif (\strtolower($language) === 'chartjs') { // Chart.js https://www.chartjs.org/ $element = [ 'text' => '', @@ -2640,8 +2601,27 @@ class Markdown ], ], ]; + } elseif (\in_array(\strtolower($language), ['mermaid', 'tuichart'])) { + // Mermaid.js https://mermaidjs.github.io + // TUI.chart https://github.com/nhn/tui.chart + $element = [ + 'text' => '', + ]; + + return [ + 'char' => $marker, + 'openerLength' => $openerLength, + 'element' => [ + 'element' => $element, + 'name' => 'div', + 'attributes' => [ + 'class' => \strtolower($language), + ], + ], + ]; } + return null; } @@ -2677,6 +2657,8 @@ class Markdown return null; } + // @performance Optimize away the child element for spoilers (if reasonable) + // https://github.com/Karaka-Management/phpOMS/issues/367 return [ 'char' => $marker, 'openerLength' => $openerLength, @@ -2690,7 +2672,7 @@ class Markdown 'text' => $summary, ], [ - 'name' => 'span', // @todo check if without span possible + 'name' => 'span', 'text' => '', ], ], diff --git a/Utils/RnG/File.php b/Utils/RnG/File.php index 79fd29055..87288bf29 100755 --- a/Utils/RnG/File.php +++ b/Utils/RnG/File.php @@ -65,9 +65,7 @@ final class File */ public static function generateExtension(?array $source = null) : string { - if ($source === null) { - $source = self::$extensions; - } + $source ??= self::$extensions; $key = \array_rand($source, 1); diff --git a/Utils/RnG/Phone.php b/Utils/RnG/Phone.php index 09a4c4157..805c42f3b 100755 --- a/Utils/RnG/Phone.php +++ b/Utils/RnG/Phone.php @@ -56,9 +56,7 @@ final class Phone $numberString = $struct; if ($isInt) { - if ($countries === null) { - $countries = ['de' => 49, 'us' => 1]; - } + $countries ??= ['de' => 49, 'us' => 1]; $numberString = \str_replace( '$1', diff --git a/Utils/RnG/Text.php b/Utils/RnG/Text.php index 5618771e2..d07cc0236 100755 --- a/Utils/RnG/Text.php +++ b/Utils/RnG/Text.php @@ -103,9 +103,7 @@ final class Text return ''; } - if ($words === null) { - $words = self::LOREM_IPSUM; - } + $words ??= self::LOREM_IPSUM; $punctuation = $this->generatePunctuation($length); $punctuationCount = \array_count_values( diff --git a/Utils/TaskSchedule/Schedule.php b/Utils/TaskSchedule/Schedule.php index d0ec5dfcf..27f116aea 100755 --- a/Utils/TaskSchedule/Schedule.php +++ b/Utils/TaskSchedule/Schedule.php @@ -39,10 +39,6 @@ class Schedule extends TaskAbstract */ public static function createWith(array $jobData) : TaskAbstract { - /** - * @todo Karaka/phpOMS#231 - * Use the interval for generating a schedule - */ $job = new self($jobData[1], $jobData[8], $jobData[7]); $job->status = (int) $jobData[3]; diff --git a/tests/DataStorage/Database/Schema/BuilderTest.php b/tests/DataStorage/Database/Schema/BuilderTest.php index 03c827f8c..f5a70693e 100755 --- a/tests/DataStorage/Database/Schema/BuilderTest.php +++ b/tests/DataStorage/Database/Schema/BuilderTest.php @@ -170,7 +170,6 @@ final class BuilderTest extends \PHPUnit\Framework\TestCase $iS = $con->getGrammar()->systemIdentifierStart; $iE = $con->getGrammar()->systemIdentifierEnd; - // @todo fix, this is not correct for sqlite $query = new Builder($con); $sql = ''; @@ -181,7 +180,7 @@ final class BuilderTest extends \PHPUnit\Framework\TestCase } elseif ($con instanceof SqlServerConnection) { $sql = 'CREATE TABLE IF NOT EXISTS [user_roles] ([user_id] INT AUTO_INCREMENT, [role_id] VARCHAR(10) DEFAULT \'1\' NULL, PRIMARY KEY ([user_id]), FOREIGN KEY ([user_id]) REFERENCES [users] ([ext1_id]), FOREIGN KEY ([role_id]) REFERENCES [roles] ([ext2_id]));'; } elseif ($con instanceof SQLiteConnection) { - $sql = 'CREATE TABLE IF NOT EXISTS [user_roles] ([user_id] INT AUTO_INCREMENT, [role_id] VARCHAR(10) DEFAULT \'1\' NULL, PRIMARY KEY ([user_id]), FOREIGN KEY ([user_id]) REFERENCES [users] ([ext1_id]), FOREIGN KEY ([role_id]) REFERENCES [roles] ([ext2_id]));'; + $sql = 'CREATE TABLE [user_roles] ([user_id] INTEGER PRIMARY KEY AUTOINCREMENT, [role_id] TEXT DEFAULT \'1\' NULL, PRIMARY KEY ([user_id]), FOREIGN KEY ([user_id]) REFERENCES [users] ([ext1_id]), FOREIGN KEY ([role_id]) REFERENCES [roles] ([ext2_id]));'; } $sql = \strtr($sql, '[]', $iS . $iE); diff --git a/tests/Stdlib/Graph/NodeTest.php b/tests/Stdlib/Graph/NodeTest.php index f2668ff70..a399d7706 100755 --- a/tests/Stdlib/Graph/NodeTest.php +++ b/tests/Stdlib/Graph/NodeTest.php @@ -143,7 +143,8 @@ final class NodeTest extends \PHPUnit\Framework\TestCase * @covers phpOMS\Stdlib\Graph\Node * @group framework * - * @todo is there bug where directed graphs return invalid neighbors? + * @bug Directed graphs may return invalid neighbors + * https://github.com/Karaka-Management/phpOMS/issues/366 */ public function testNeighborsInputOutput() : void { diff --git a/tests/Utils/TaskSchedule/ScheduleTest.php b/tests/Utils/TaskSchedule/ScheduleTest.php index 1d167e192..e5bacad7e 100755 --- a/tests/Utils/TaskSchedule/ScheduleTest.php +++ b/tests/Utils/TaskSchedule/ScheduleTest.php @@ -39,8 +39,6 @@ final class ScheduleTest extends \PHPUnit\Framework\TestCase * @testdox A task can be created from an array and rendered * @covers phpOMS\Utils\TaskSchedule\Schedule * - * @todo the interval has to be implemented! - * * @group framework */ public function testCreateJobWithData() : void