diff --git a/CHANGELOG.md b/CHANGELOG.md index b660396..9fa08d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ First standalone release, planned as 1.0.0, for PRADO 4.4. SqlMap moves out of t ### Added +- An ``, ``, or `` statement with a `cacheModel` attribute flushes that cache model when it executes, the same as a `` entry. The attribute was previously accepted and ignored (#5). - `composer.json` declares `extra.prado` for PRADO 4.4: `bootstrap` (`TSqlMapConfig`), `error-messages` (`src/Data/SqlMap/DataMapper/messages.txt`), and `class-map` (`config/classes.json`). The legacy top-level `extra.bootstrap` remains for PRADO 4.3. - `config/classes.json` maps the 55 SqlMap short class names (`TSqlMapConfig`, `TSqlMapFifoCache`, ...) to their FQNs. It replaces the SqlMap entries of the framework's `classes.php`. - Message text for five codes that were thrown without a definition: `sqlmap_can_not_extend_select_key`, `sqlmap_configfile_invalid`, `sqlmap_must_enable_custom_paging`, `sqlmap_property_must_be_list`, `sqlmap_use_set_to_store_cache`. @@ -29,3 +30,5 @@ First standalone release, planned as 1.0.0, for PRADO 4.4. SqlMap moves out of t - `TCachingStatement::executeInsert()` delegates to the wrapped statement. It previously called itself until the stack overflowed. - `TParameterProperty::__sleep()` and `TResultProperty::__sleep()` name private properties by their namespaced class, so properties at their default values are left out of the serialized form. `TResultProperty` also misspelled `_propertyName`. - `TResultProperty::getPropertyValueType()` no longer calls a method on `null` when no reflection class is found for the type. +- A `groupBy` result map no longer adds an element to a nested collection when every column of the nested result map is null, as in a LEFT JOIN row with no match. The collection stays empty. A parent row with no nested elements is still returned, in query order (#4). +- `TPropertyAccess::get()` and `has()` read `ArrayAccess` objects through `offsetExists()` and `offsetGet()`. `array_key_exists()` threw a `TypeError` for these objects on PHP 8 (#6). diff --git a/docs/mappings/resultMap.md b/docs/mappings/resultMap.md index 0157cd9..26fd3dc 100644 --- a/docs/mappings/resultMap.md +++ b/docs/mappings/resultMap.md @@ -173,7 +173,7 @@ The corresponding query joins the two tables: ``` -SqlMap iterates the result set, grouping by `Id`. For each unique `Account_Id` value it creates one `AccountWithOrders` object. Each `Order_*` row is mapped via `account-orders` and appended to the parent's `Orders` collection. +SqlMap iterates the result set, grouping by `Id`. For each unique `Account_Id` value it creates one `AccountWithOrders` object. Each `Order_*` row is mapped via `account-orders` and appended to the parent's `Orders` collection. An account without orders has null in every `Order_*` column; it is returned with an empty `Orders` collection. --- diff --git a/docs/statements/delete.md b/docs/statements/delete.md index bf89eb2..f7eda86 100644 --- a/docs/statements/delete.md +++ b/docs/statements/delete.md @@ -111,7 +111,7 @@ A [``](../mappings/parameterMap.md) gives explicit control over ty ### Cache invalidation -Any [``](../configuration/cacheModel.md) that lists this statement's `id` in a `` element is flushed when the delete executes successfully. This behavior is identical to [``](update.md). +Any [``](../configuration/cacheModel.md) that lists this statement's `id` in a `` element is flushed when the delete executes successfully. A `cacheModel` attribute on the `` flushes the named cache model the same way. This behavior is identical to [``](update.md). ```xml diff --git a/docs/statements/insert.md b/docs/statements/insert.md index 7564549..f2421e0 100644 --- a/docs/statements/insert.md +++ b/docs/statements/insert.md @@ -44,7 +44,7 @@ Full example showing both `pre` and `post` key retrieval patterns: | `parameterClass` | No | Fully qualified PHP class name (or primitive alias) of the parameter object. Used when no explicit `parameterMap` is needed; SqlMap maps `#property#` tokens directly to object properties. | | `parameterMap` | No | Reference to a named [``](../mappings/parameter-map.md). Takes precedence over `parameterClass` when both are present. Used for positional `?` placeholders or explicit type/null-value control. | -The `insert` element does not support `resultClass`, `resultMap`, `listClass`, or `cacheModel` — it returns only a generated key or `null`. +The `insert` element does not support `resultClass`, `resultMap`, or `listClass`. It returns only a generated key or `null`. A `cacheModel` attribute names a cache model that the insert flushes when it executes. --- diff --git a/docs/statements/update.md b/docs/statements/update.md index 166b0bb..987b619 100644 --- a/docs/statements/update.md +++ b/docs/statements/update.md @@ -141,6 +141,14 @@ Assign the cache model to the `SELECT` that should be invalidated: When `TSqlMapGateway::update('UpdateAccount', ...)` executes, SqlMap automatically flushes `AccountCache`, so subsequent calls to `GetAccountById` return fresh data. +A `cacheModel` attribute on an ``, ``, or `` statement has the same effect as listing the statement in `flushOnExecute`: + +```xml + + UPDATE accounts SET account_email = #EmailAddress# WHERE account_id = #Id# + +``` + See [cacheModel](../configuration/README.md#cachemodel) for the full cache model reference. --- diff --git a/src/Data/SqlMap/Configuration/TSqlMapXmlConfiguration.php b/src/Data/SqlMap/Configuration/TSqlMapXmlConfiguration.php index 4f43910..4f20826 100644 --- a/src/Data/SqlMap/Configuration/TSqlMapXmlConfiguration.php +++ b/src/Data/SqlMap/Configuration/TSqlMapXmlConfiguration.php @@ -163,14 +163,20 @@ protected function resolveResultMapping() /** * Set the cache for each statement having a cache model property. + * An insert, update, or delete statement with a cache model flushes that cache + * when it executes, the same as a `` entry in the cache model. */ protected function attachCacheModels() { foreach ($this->_manager->getMappedStatements() as $mappedStatement) { - $model = $mappedStatement->getStatement()->getCacheModel(); + $statement = $mappedStatement->getStatement(); + $model = $statement->getCacheModel(); if ($model !== null && strlen($model) > 0) { $cache = $this->_manager->getCacheModel($model); - $mappedStatement->getStatement()->setCache($cache); + $statement->setCache($cache); + if ($statement instanceof TSqlMapInsert || $statement instanceof TSqlMapUpdate) { + $cache->registerTriggerStatement($mappedStatement); + } } } } diff --git a/src/Data/SqlMap/DataMapper/TPropertyAccess.php b/src/Data/SqlMap/DataMapper/TPropertyAccess.php index a2210b4..7165652 100644 --- a/src/Data/SqlMap/DataMapper/TPropertyAccess.php +++ b/src/Data/SqlMap/DataMapper/TPropertyAccess.php @@ -57,12 +57,18 @@ public static function get($object, $path) } $properties = explode('.', $path); foreach ($properties as $prop) { - if (is_array($object) || $object instanceof \ArrayAccess) { + if (is_array($object)) { if (array_key_exists($prop, $object)) { $object = $object[$prop]; } else { throw new TInvalidPropertyException('sqlmap_invalid_property', $path); } + } elseif ($object instanceof \ArrayAccess) { + if ($object->offsetExists($prop)) { + $object = $object->offsetGet($prop); + } else { + throw new TInvalidPropertyException('sqlmap_invalid_property', $path); + } } elseif (is_object($object)) { $getter = 'get' . $prop; if (method_exists($object, $getter) && is_callable([$object, $getter])) { @@ -93,12 +99,18 @@ public static function has($object, $path) } $properties = explode('.', $path); foreach ($properties as $prop) { - if (is_array($object) || $object instanceof \ArrayAccess) { + if (is_array($object)) { if (array_key_exists($prop, $object)) { $object = $object[$prop]; } else { return false; } + } elseif ($object instanceof \ArrayAccess) { + if ($object->offsetExists($prop)) { + $object = $object->offsetGet($prop); + } else { + return false; + } } elseif (is_object($object)) { $getter = 'get' . $prop; if (method_exists($object, $getter) && is_callable([$object, $getter])) { diff --git a/src/Data/SqlMap/Statements/TMappedStatement.php b/src/Data/SqlMap/Statements/TMappedStatement.php index f028e65..1c575c5 100644 --- a/src/Data/SqlMap/Statements/TMappedStatement.php +++ b/src/Data/SqlMap/Statements/TMappedStatement.php @@ -664,7 +664,8 @@ protected function fillResultMap($resultMapName, $row, $parentGroup = null, &$re /** * ResultMap with GroupBy property. Save object collection graph in a tree - * and collect the result later. + * and collect the result later. A nested result map without row data, such as + * the right side of a LEFT JOIN with no match, adds no element to the collection. * @param TResultMap $resultMap result mapping details. * @param array $row a result set row retrieved from the database * @param mixed $parent @@ -688,6 +689,9 @@ protected function addResultMapGroupBy($resultMap, $row, $parent, &$resultObject //nested property if ($this->getManager()->getResultMaps()->contains($nested)) { $nestedMap = $this->getManager()->getResultMap($nested); + if (!$this->hasResultMapRowData($nestedMap, $row)) { + continue; + } $groupKey = $this->getResultMapGroupKey($nestedMap, $row); //add the node reference first @@ -710,6 +714,38 @@ protected function addResultMapGroupBy($resultMap, $row, $parent, &$resultObject return $resultObject; } + /** + * Checks whether a row holds a value for any column of a result map. + * A LEFT JOIN row without a matching right-side row is null in every column + * of the nested result map. Column names match case-insensitively when no + * exact match exists. + * @param TResultMap $resultMap result mapping details. + * @param array $row a result set row retrieved from the database + * @return bool true if a mapped column is not null or the map has no column mappings. + */ + protected function hasResultMapRowData($resultMap, $row) + { + $hasColumns = false; + foreach ($resultMap->getColumns() as $property) { + $column = $property->getColumn(); + if ($column === null || $column === '') { + continue; + } + $hasColumns = true; + $index = $property->getColumnIndex(); + if (($index > 0 && isset($row[$index])) || isset($row[$column])) { + return true; + } + $columnLower = strtolower($column); + foreach ($row as $key => $value) { + if ($value !== null && strtolower((string) $key) === $columnLower) { + return true; + } + } + } + return !$hasColumns; + } + /** * Gets the result 'group by' groupping key for each row. * @param TResultMap $resultMap result mapping details. diff --git a/src/Data/SqlMap/Statements/TSqlMapObjectCollectionTree.php b/src/Data/SqlMap/Statements/TSqlMapObjectCollectionTree.php index 3d7cdcb..d3dacb5 100644 --- a/src/Data/SqlMap/Statements/TSqlMapObjectCollectionTree.php +++ b/src/Data/SqlMap/Statements/TSqlMapObjectCollectionTree.php @@ -61,13 +61,13 @@ public function add($parent, $node, $object = '') } $this->_entries[$node] = $object; if (empty($parent)) { - if (isset($this->_entries[$node])) { - return; + if (!isset($this->_tree[$node])) { + $this->_tree[$node] = []; } - $this->_tree[$node] = []; + return; } $found = $this->addNode($this->_tree, $parent, $node); - if (!$found && !empty($parent)) { + if (!$found) { $this->_tree[$parent] = []; if (!isset($this->_entries[$parent]) || $object !== '') { $this->_entries[$parent] = $object; @@ -98,13 +98,22 @@ protected function addNode(&$childs, $parent, $node) } /** + * Collects the root objects in the order they were added. A root object + * without child nodes is included with its collection property unchanged. * @return array object collection */ public function collect() { + $roots = array_keys($this->_tree); while (count($this->_tree) > 0) { $this->collectChildren(null, $this->_tree); } + foreach ($roots as $root) { + $entry = $this->_entries[$root] ?? null; + if (is_array($entry) && $entry['property'] === null) { + $this->_list[] = $entry['object']; + } + } return $this->getCollection(); } @@ -176,10 +185,6 @@ protected function onChildNodesVisited($parent, $nodes) if (is_array($list)) { TPropertyAccess::set($parentObject, $property, $list); } - - if ($this->_entries[$parent]['property'] === null) { - $this->_list[] = $parentObject; - } } /** diff --git a/tests/unit/Data/SqlMap/Configuration/TSqlMapCacheModelFlushTest.php b/tests/unit/Data/SqlMap/Configuration/TSqlMapCacheModelFlushTest.php new file mode 100644 index 0000000..147d534 --- /dev/null +++ b/tests/unit/Data/SqlMap/Configuration/TSqlMapCacheModelFlushTest.php @@ -0,0 +1,46 @@ +manager = new TSqlMapManager(); + $this->manager->configureXml(__DIR__ . '/../maps/cache-flush.xml'); + } + + private function executes(string $statementID, string $cacheID): bool + { + $cache = $this->manager->getCacheModel($cacheID); + $cache->set('key', 'value'); + $this->manager->getMappedStatement($statementID)->onExecuteQuery(''); + return $cache->get('key') === null; + } + + public function testWriteStatementWithCacheModelFlushesCache() + { + $this->assertTrue($this->executes('InsertCached', 'shared-cache')); + $this->assertTrue($this->executes('UpdateCached', 'shared-cache')); + $this->assertTrue($this->executes('DeleteCached', 'shared-cache')); + } + + public function testSelectWithCacheModelUsesCache() + { + $cache = $this->manager->getCacheModel('shared-cache'); + $this->assertSame($cache, $this->manager->getMappedStatement('SelectCached')->getStatement()->getCache()); + } + + public function testStatementWithoutCacheModelDoesNotFlushCache() + { + $this->assertFalse($this->executes('UpdateUncached', 'shared-cache')); + } + + public function testFlushOnExecuteFlushesCache() + { + $this->assertTrue($this->executes('UpdateListed', 'listed-cache')); + $this->assertFalse($this->executes('UpdateCached', 'listed-cache')); + } +} diff --git a/tests/unit/Data/SqlMap/DataMapper/TPropertyAccessTest.php b/tests/unit/Data/SqlMap/DataMapper/TPropertyAccessTest.php index 087a8da..cd42436 100644 --- a/tests/unit/Data/SqlMap/DataMapper/TPropertyAccessTest.php +++ b/tests/unit/Data/SqlMap/DataMapper/TPropertyAccessTest.php @@ -204,6 +204,28 @@ public function testArrayAccess() self::expectException(Prado\Data\SqlMap\DataMapper\TInvalidPropertyException::class); TPropertyAccess::get($testobj, 'a.d.C'); } + + public function testArrayAccessObject() + { + $map = new \Prado\Collections\TMap(['a' => 'foo', 'n' => null, 'c' => ['d' => 'bar']]); + + self::assertEquals('foo', TPropertyAccess::get($map, 'a')); + self::assertNull(TPropertyAccess::get($map, 'n')); + self::assertEquals('bar', TPropertyAccess::get($map, 'c.d')); + self::assertTrue(TPropertyAccess::has($map, 'a')); + self::assertTrue(TPropertyAccess::has($map, 'c.d')); + self::assertFalse(TPropertyAccess::has($map, 'z')); + self::assertFalse(TPropertyAccess::has($map, 'c.z')); + + $testobj = new _PropertyAccessTestHelperPublicVar(); + TPropertyAccess::set($testobj, 'a', $map); + self::assertEquals('foo', TPropertyAccess::get($testobj, 'a.a')); + TPropertyAccess::set($testobj, 'a.b', 'baz'); + self::assertEquals('baz', $map['b']); + + self::expectException(Prado\Data\SqlMap\DataMapper\TInvalidPropertyException::class); + TPropertyAccess::get($map, 'z'); + } } diff --git a/tests/unit/Data/SqlMap/Statements/TMappedStatementGroupByTest.php b/tests/unit/Data/SqlMap/Statements/TMappedStatementGroupByTest.php new file mode 100644 index 0000000..031cd49 --- /dev/null +++ b/tests/unit/Data/SqlMap/Statements/TMappedStatementGroupByTest.php @@ -0,0 +1,93 @@ +rows; + } +} + +class TMappedStatementGroupByTest extends PHPUnit\Framework\TestCase +{ + private function query(array $rows): array + { + $manager = new TSqlMapManager(); + $manager->configureXml(__DIR__ . '/../maps/group-by.xml'); + $statement = $manager->getMappedStatement('GetUsersWithRoles'); + return $statement->runQueryForList( + new TMappedStatementGroupByConnection(), + null, + new TMappedStatementGroupByCommand($rows), + null + ); + } + + private function roleNames(TMappedStatementGroupByUser $user): array + { + return array_map(fn ($role) => $role->Name, $user->Roles); + } + + public function testLeftJoinWithoutMatchGivesEmptyCollection() + { + $users = $this->query([ + ['user_id' => 1, 'user_name' => 'ann', 'role_name' => 'admin'], + ['user_id' => 1, 'user_name' => 'ann', 'role_name' => 'user'], + ['user_id' => 2, 'user_name' => 'bob', 'role_name' => null], + ['user_id' => 3, 'user_name' => 'cy', 'role_name' => 'user'], + ]); + + $this->assertSame(['ann', 'bob', 'cy'], array_map(fn ($user) => $user->Name, $users)); + $this->assertSame(['admin', 'user'], $this->roleNames($users[0])); + $this->assertSame([], $users[1]->Roles); + $this->assertSame(['user'], $this->roleNames($users[2])); + } + + public function testLeftJoinWithoutAnyMatch() + { + $users = $this->query([ + ['user_id' => 1, 'user_name' => 'ann', 'role_name' => null], + ['user_id' => 2, 'user_name' => 'bob', 'role_name' => null], + ]); + + $this->assertSame(['ann', 'bob'], array_map(fn ($user) => $user->Name, $users)); + $this->assertSame([], $users[0]->Roles); + $this->assertSame([], $users[1]->Roles); + } + + public function testColumnCaseFolding() + { + $users = $this->query([ + ['USER_ID' => 1, 'USER_NAME' => 'ann', 'ROLE_NAME' => 'admin'], + ['USER_ID' => 2, 'USER_NAME' => 'bob', 'ROLE_NAME' => null], + ]); + + $this->assertSame(['admin'], $this->roleNames($users[0])); + $this->assertSame([], $users[1]->Roles); + } +} diff --git a/tests/unit/Data/SqlMap/Statements/TSqlMapObjectCollectionTreeTest.php b/tests/unit/Data/SqlMap/Statements/TSqlMapObjectCollectionTreeTest.php index de6112e..e62b286 100644 --- a/tests/unit/Data/SqlMap/Statements/TSqlMapObjectCollectionTreeTest.php +++ b/tests/unit/Data/SqlMap/Statements/TSqlMapObjectCollectionTreeTest.php @@ -43,16 +43,15 @@ public function test_zappable_empty_not_in_sleep() public function test_zappable_entries_populated_in_sleep() { - // A root-only add populates _entries but NOT _tree + // A root-only add populates _entries and _tree $this->tree->add(null, 'root', ['object' => new stdClass(), 'property' => null]); $sleepKeys = $this->tree->__sleep(); $cn = TSqlMapObjectCollectionTree::class; - // _entries is non-empty — should appear in __sleep() $this->assertContains("\0$cn\0_entries", $sleepKeys); - // _tree is still empty after root-only add — should be absent - $this->assertNotContains("\0$cn\0_tree", $sleepKeys); + $this->assertContains("\0$cn\0_tree", $sleepKeys); + $this->assertNotContains("\0$cn\0_list", $sleepKeys); } public function test_zappable_tree_populated_in_sleep() @@ -124,18 +123,38 @@ public function test_collect_with_parent_child_using_tlist() $this->assertSame(2, $parentObj->items->getCount()); } - public function test_collect_root_only_no_children_empty_result() + public function test_collect_root_without_children() { - // A root node with no children: after collect() the root-only collect - // goes through collectChildren, processes it, calls onChildNodesVisited(null, [root]) - // which returns early due to empty parent. - // So result is []. + // A root node with no children is collected with its list unchanged. $parentObj = new stdClass(); + $parentObj->items = []; $this->tree->add(null, 'p1', ['object' => $parentObj, 'property' => null]); - // No children added — _tree is empty, so collect() loop exits immediately $result = $this->tree->collect(); - $this->assertSame([], $result); + $this->assertSame([$parentObj], $result); + $this->assertSame([], $parentObj->items); + } + + public function test_collect_keeps_root_order_with_and_without_children() + { + $p1 = new stdClass(); + $p1->items = []; + $p2 = new stdClass(); + $p2->items = []; + $p3 = new stdClass(); + $p3->items = []; + $child = new stdClass(); + + $this->tree->add(null, 'p1', ['object' => $p1, 'property' => null]); + $this->tree->add(null, 'p2', ['object' => $p2, 'property' => null]); + $this->tree->add('p2', 'c1', ''); + $this->tree->add('p2', 'c1', ['object' => $child, 'property' => 'items']); + $this->tree->add(null, 'p3', ['object' => $p3, 'property' => null]); + + $this->assertSame([$p1, $p2, $p3], $this->tree->collect()); + $this->assertSame([], $p1->items); + $this->assertSame([$child], $p2->items); + $this->assertSame([], $p3->items); } public function test_serialization_round_trip() diff --git a/tests/unit/Data/SqlMap/maps/cache-flush.xml b/tests/unit/Data/SqlMap/maps/cache-flush.xml new file mode 100644 index 0000000..22efad0 --- /dev/null +++ b/tests/unit/Data/SqlMap/maps/cache-flush.xml @@ -0,0 +1,14 @@ + + + + + + + + + INSERT INTO t (a) VALUES (1) + UPDATE t SET a = 1 + DELETE FROM t + UPDATE t SET a = 2 + UPDATE t SET a = 3 + diff --git a/tests/unit/Data/SqlMap/maps/group-by.xml b/tests/unit/Data/SqlMap/maps/group-by.xml new file mode 100644 index 0000000..8d935b4 --- /dev/null +++ b/tests/unit/Data/SqlMap/maps/group-by.xml @@ -0,0 +1,17 @@ + + + + + + + + + + + + + +