Audit of the #4, #5, #6 fixes: nested groupBy keys, docs, tests

CHANGELOG.md

@@ -31,4 +31,5 @@ First standalone release, planned as 1.0.0, for PRADO 4.4. SqlMap moves out of t
 - `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).
+- A nested result map with its own `groupBy` groups within its parent. The same `groupBy` value under two parents previously shared one tree key, so the second parent lost the element and the first parent's element was replaced.
 - `TPropertyAccess::get()` and `has()` read `ArrayAccess` objects through `offsetExists()` and `offsetGet()`. `array_key_exists()` threw a `TypeError` for these objects on PHP 8 (#6).

docs/configuration/cacheModel.md

@@ -2,7 +2,7 @@
 
 # cacheModel
 
-A `cacheModel` defines a named cache that [`select`](../mapping/select.md) statements reference to avoid repeated database round-trips. Results returned by a cached select are stored in the cache; subsequent calls with the same statement and parameters return the cached value without hitting the database.
+A `cacheModel` defines a named cache that [`select`](../mapping/select.md) statements reference to avoid repeated database round-trips. Results returned by a cached select are stored in the cache; subsequent calls with the same statement and parameters return the cached value without hitting the database. An `insert`, `update`, or `delete` statement that references the cache model flushes it when it executes.
 
 Cache models are declared inside a [mapping file](../mapping/README.md) and are global to the [`TSqlMapManager`](../api/TSqlMapManager.md) instance.
 
@@ -32,7 +32,7 @@ Cache models are declared inside a [mapping file](../mapping/README.md) and are
 
 | Attribute | Required | Values | Description |
 |---|---|---|---|
-| `id` | yes | string | Unique identifier for this cache model within the SqlMap. Referenced by `cacheModel` on select statements. |
+| `id` | yes | string | Unique identifier for this cache model within the SqlMap. Referenced by the `cacheModel` attribute of statements. |
 | `implementation` | no | `LRU`, `FIFO`, `Basic` | Cache strategy. Defaults to `Basic` ([`TSqlMapApplicationCache`](../api/TSqlMapApplicationCache.md)). |
 | `readOnly` | no | `true`/`false` | Reserved for future use; not currently enforced by the runtime. |
 | `serialize` | no | `true`/`false` | Reserved for future use; not currently enforced by the runtime. |
@@ -106,6 +106,18 @@ Add the `cacheModel` attribute to a `select` statement to enable caching for it.
 
 When the same statement is executed with the same parameter, the result is returned from the cache. The cache key is derived from the statement id and a hash of the parameter object.
 
+## Attaching to a write statement
+
+The `cacheModel` attribute on an `insert`, `update`, or `delete` statement flushes the named cache model when the statement executes. It has the same effect as a `flushOnExecute` element that names the statement. The cache model may be declared in any mapping file.
+
+```xml
+<update id="updateAccount"
+        parameterClass="Account"
+        cacheModel="accountCache">
+    UPDATE accounts SET account_email = #EmailAddress# WHERE account_id = #Id#
+</update>
+```
+
 ## APPLICATION cache
 
 The `Basic` implementation delegates to the PRADO application cache (`Prado::getApplication()->getCache()`). This requires that an [`ICache`](https://github.com/pradosoft/prado) component — such as `TMemCache` or `TDbCache` — is configured in the application and that [`TSqlMapApplicationCache`](../api/TSqlMapApplicationCache.md) is available.

docs/mappings/resultMap.md

@@ -136,6 +136,7 @@ A single SQL `JOIN` query can produce multiple rows per parent entity (one-to-ma
 - `groupBy` lists one or more property names whose combined values identify a unique parent row.
 - The child collection property must have `resultMapping` pointing to a separate `<resultMap>` for the child entity.
 - The collection property type must be `TList`, `array`, or another list-compatible type.
+- A child `<resultMap>` may have its own `groupBy` and nested collections. Its `groupBy` values are grouped within the parent, so the same child value under two parents gives two child objects.
 
 **Example — Account with Orders:**
 

docs/statements/delete.md

@@ -25,6 +25,7 @@ The body is the SQL text to execute. [Inline parameters](../dynamic-sql/inline-p
 | `id` | Yes | Unique identifier for the statement within its namespace. Passed to [`TSqlMapGateway::delete()`](../../src/Data/SqlMap/TSqlMapGateway.php) as the first argument. |
 | `parameterClass` | No | Fully qualified PHP class name (or primitive alias) of the input parameter. When omitted, the framework infers the type from the value passed at runtime. |
 | `parameterMap` | No | References a named [`<parameterMap>`](../mappings/parameterMap.md) that defines explicit column-to-property bindings. `parameterClass` and `parameterMap` are mutually exclusive; prefer `parameterClass` with inline parameters for simple cases. |
+| `cacheModel` | No | Name of a [`<cacheModel>`](../configuration/cacheModel.md) that is flushed when the delete executes. See [Cache invalidation](#cache-invalidation). |
 
 ---
 

docs/statements/insert.md

@@ -43,8 +43,9 @@ Full example showing both `pre` and `post` key retrieval patterns:
 | `id` | Yes | Unique identifier within the namespace. Passed to [`TSqlMapGateway::insert()`](#tsqlmapgatewayinsert) as `$statementName`. |
 | `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 [`<parameterMap>`](../mappings/parameter-map.md). Takes precedence over `parameterClass` when both are present. Used for positional `?` placeholders or explicit type/null-value control. |
+| `cacheModel` | No | Name of a [`<cacheModel>`](../configuration/cacheModel.md) that is flushed when the insert executes. |
 
-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.
+The `insert` element does not support `resultClass`, `resultMap`, or `listClass`. It returns only a generated key or `null`.
 
 ---
 

docs/statements/update.md

@@ -24,6 +24,7 @@ The SQL body is a plain SQL `UPDATE` string. Inline parameter tokens (`#property
 | `id` | Yes | Unique name for this statement within the mapping file. Used as the key passed to `TSqlMapGateway::update()`. |
 | `parameterClass` | No | Fully qualified class name (or [type alias](../configuration/README.md#typealias)) of the parameter object. When omitted, SqlMap accepts any object or associative array. |
 | `parameterMap` | No | Name of a [`parameterMap`](../mappings/README.md#parametermap) defined in this or another mapping file. Mutually exclusive with inline `#property#` tokens; use one or the other. |
+| `cacheModel` | No | Name of a [`cacheModel`](../configuration/cacheModel.md) that is flushed when the update executes. See [Cache invalidation](#cache-invalidation-via-flushonexecute). |
 
 `update` does not support `resultClass` or `resultMap` — the only return value is the affected-row count.
 

src/Data/SqlMap/DataMapper/TPropertyAccess.php

@@ -10,8 +10,6 @@
 
 namespace Prado\Data\SqlMap\DataMapper;
 
-use Prado\Exceptions\TInvalidDataValueException;
-
 /**
  * TPropertyAccess class provides dot notation stype property access and setting.
  *
@@ -38,6 +36,11 @@
  * echo TPropertyAccess::get($obj, 'property1.property2.property3');
  * ```
  *
+ * Each segment of the path is resolved by the type of the current value:
+ * - array → the key, which must exist
+ * - `\ArrayAccess` object → `offsetExists()` and `offsetGet()`
+ * - other object → the `get`/`set` method, then the public property, then `__get`/`__set`
+ *
  * @author Wei Zhuo <weizho[at]gmail[dot]com>
  * @since 3.1
  */
@@ -47,7 +50,7 @@ class TPropertyAccess
 	 * Gets the property value.
 	 * @param mixed $object object or path.
 	 * @param string $path property path.
-	 * @throws TInvalidDataValueException if property path is invalid.
+	 * @throws TInvalidPropertyException if property path is invalid.
 	 * @return mixed property value.
 	 */
 	public static function get($object, $path)
@@ -88,6 +91,7 @@ public static function get($object, $path)
 	}
 
 	/**
+	 * Checks whether every segment of the property path resolves.
 	 * @param mixed $object object or array
 	 * @param string $path property path.
 	 * @return bool true if property path is valid
@@ -134,7 +138,8 @@ public static function has($object, $path)
 	 * @param mixed $originalObject by reference, object or array
 	 * @param string $path property path.
 	 * @param mixed $value new property value.
-	 * @throws TInvalidDataValueException if property path is invalid.
+	 * @throws TInvalidPropertyException if property path is invalid or the
+	 * target is not an array or object.
 	 */
 	public static function set(&$originalObject, $path, $value)
 	{

src/Data/SqlMap/Statements/TMappedStatement.php

@@ -666,9 +666,11 @@ protected function fillResultMap($resultMapName, $row, $parentGroup = null, &$re
 	 * ResultMap with GroupBy property. Save object collection graph in a tree
 	 * 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.
+	 * The tree key of a nested element is prefixed with its parent key, so equal
+	 * `groupBy` values under different parents are separate elements.
 	 * @param TResultMap $resultMap result mapping details.
 	 * @param array $row a result set row retrieved from the database
-	 * @param mixed $parent
+	 * @param mixed $parent tree key of the parent element, null or '' for a root
 	 * @param object $resultObject the result object
 	 * @return object result object.
 	 */
@@ -679,6 +681,9 @@ protected function addResultMapGroupBy($resultMap, $row, $parent, &$resultObject
 		if (empty($parent)) {
 			$rootObject = ['object' => $resultObject, 'property' => null];
 			$this->_groupBy->add(null, $group, $rootObject);
+			$parentKey = $group;
+		} else {
+			$parentKey = $parent;
 		}
 
 		foreach ($resultMap->getColumns() as $property) {
@@ -692,7 +697,7 @@ protected function addResultMapGroupBy($resultMap, $row, $parent, &$resultObject
 				if (!$this->hasResultMapRowData($nestedMap, $row)) {
 					continue;
 				}
-				$groupKey = $this->getResultMapGroupKey($nestedMap, $row);
+				$groupKey = $parentKey . '.' . $this->getResultMapGroupKey($nestedMap, $row);
 
 				//add the node reference first
 				if (empty($parent)) {
@@ -722,6 +727,7 @@ protected function addResultMapGroupBy($resultMap, $row, $parent, &$resultObject
 	 * @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.
+	 * @since 4.4.0
 	 */
 	protected function hasResultMapRowData($resultMap, $row)
 	{

tests/unit/Data/SqlMap/CacheTest.php

@@ -92,6 +92,22 @@ public function testFlushDataCacheOnExecute()
 		$this->assertTrue($list1 !== $list3);
 	}
 
+	/**
+	 * An update with a cacheModel attribute flushes that cache when it executes.
+	 */
+	public function testFlushDataCacheOnExecuteViaCacheModelAttribute()
+	{
+		$list1 = self::$sqlmap->queryForList("GetCachedAccountsViaResultMap");
+		$list2 = self::$sqlmap->queryForList("GetCachedAccountsViaResultMap");
+
+		$this->assertTrue($list1 === $list2);
+		self::$sqlmap->update("UpdateAccountViaCacheModel", $list1[0]);
+
+		$list3 = self::$sqlmap->queryForList("GetCachedAccountsViaResultMap");
+
+		$this->assertTrue($list1 !== $list3);
+	}
+
 	/**
 	 */
 	protected function getCacheModel()

tests/unit/Data/SqlMap/Configuration/TSqlMapCacheModelFlushTest.php

@@ -9,7 +9,7 @@ class TSqlMapCacheModelFlushTest extends PHPUnit\Framework\TestCase
 	protected function setUp(): void
 	{
 		$this->manager = new TSqlMapManager();
-		$this->manager->configureXml(__DIR__ . '/../maps/cache-flush.xml');
+		$this->manager->configureXml(__DIR__ . '/../maps/cache-flush-config.xml');
 	}
 
 	private function executes(string $statementID, string $cacheID): bool
@@ -38,9 +38,25 @@ public function testStatementWithoutCacheModelDoesNotFlushCache()
 		$this->assertFalse($this->executes('UpdateUncached', 'shared-cache'));
 	}
 
+	public function testWriteStatementFlushesOnlyItsOwnCacheModel()
+	{
+		$this->assertFalse($this->executes('UpdateCached', 'listed-cache'));
+		$this->assertFalse($this->executes('UpdateListed', 'shared-cache'));
+	}
+
 	public function testFlushOnExecuteFlushesCache()
 	{
 		$this->assertTrue($this->executes('UpdateListed', 'listed-cache'));
-		$this->assertFalse($this->executes('UpdateCached', 'listed-cache'));
+	}
+
+	public function testStatementListedAndWithCacheModelFlushesCache()
+	{
+		$this->assertTrue($this->executes('UpdateListedAndCached', 'listed-cache'));
+	}
+
+	public function testCacheModelInAnotherMappingFile()
+	{
+		$this->assertTrue($this->executes('UpdateInOtherFile', 'shared-cache'));
+		$this->assertTrue($this->executes('DeleteOtherFileCache', 'other-cache'));
 	}
 }

tests/unit/Data/SqlMap/DataMapper/TPropertyAccessTest.php

@@ -213,19 +213,89 @@ public function testArrayAccessObject()
 		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, 'n'));
 		self::assertTrue(TPropertyAccess::has($map, 'c.d'));
 		self::assertFalse(TPropertyAccess::has($map, 'z'));
 		self::assertFalse(TPropertyAccess::has($map, 'c.z'));
+		self::assertFalse(TPropertyAccess::has($map, 'n.z'));
+
+		TPropertyAccess::set($map, 'e', 'qux');
+		self::assertEquals('qux', $map['e']);
 
 		$testobj = new _PropertyAccessTestHelperPublicVar();
 		TPropertyAccess::set($testobj, 'a', $map);
 		self::assertEquals('foo', TPropertyAccess::get($testobj, 'a.a'));
+		self::assertTrue(TPropertyAccess::has($testobj, 'a.c.d'));
 		TPropertyAccess::set($testobj, 'a.b', 'baz');
 		self::assertEquals('baz', $map['b']);
 
 		self::expectException(Prado\Data\SqlMap\DataMapper\TInvalidPropertyException::class);
 		TPropertyAccess::get($map, 'z');
 	}
+
+	public function testArrayAccessObjectNestedPathMissing()
+	{
+		$map = new \Prado\Collections\TMap(['c' => ['d' => 'bar']]);
+
+		self::expectException(Prado\Data\SqlMap\DataMapper\TInvalidPropertyException::class);
+		TPropertyAccess::get($map, 'c.z');
+	}
+
+	public function testArrayAccessObjectUsesOffsetMethods()
+	{
+		$object = new _PropertyAccessTestHelperArrayAccess();
+
+		self::assertEquals(['a', 'b'], TPropertyAccess::get($object, 'ab'));
+		self::assertEquals('b', TPropertyAccess::get($object, 'ab.1'));
+		self::assertTrue(TPropertyAccess::has($object, 'ab'));
+		self::assertTrue(TPropertyAccess::has($object, 'ab.1'));
+		self::assertFalse(TPropertyAccess::has($object, 'cd'));
+		self::assertSame(['ab', 'ab', 'ab', 'ab', 'cd'], $object->existsCalls);
+		self::assertSame(['ab', 'ab', 'ab', 'ab'], $object->getCalls);
+
+		self::expectException(Prado\Data\SqlMap\DataMapper\TInvalidPropertyException::class);
+		TPropertyAccess::get($object, 'cd');
+	}
+
+	public function testArrayAccessObjectInsideArray()
+	{
+		$map = new \Prado\Collections\TMap(['k' => 'v']);
+		$data = ['map' => $map];
+
+		self::assertEquals('v', TPropertyAccess::get($data, 'map.k'));
+		self::assertTrue(TPropertyAccess::has($data, 'map.k'));
+		self::assertFalse(TPropertyAccess::has($data, 'map.z'));
+	}
+}
+
+/**
+ * Accepts only keys made of the letters a and b. The value of a key is the
+ * array of its letters. Records each offset call.
+ */
+class _PropertyAccessTestHelperArrayAccess implements \ArrayAccess
+{
+	public $existsCalls = [];
+	public $getCalls = [];
+
+	public function offsetExists($offset): bool
+	{
+		$this->existsCalls[] = $offset;
+		return preg_match('/^[ab]+$/', (string) $offset) === 1;
+	}
+
+	public function offsetGet($offset): mixed
+	{
+		$this->getCalls[] = $offset;
+		return str_split((string) $offset);
+	}
+
+	public function offsetSet($offset, $value): void
+	{
+	}
+
+	public function offsetUnset($offset): void
+	{
+	}
 }
 
 

tests/unit/Data/SqlMap/GroupByTest.php

@@ -34,4 +34,22 @@ public function testAccountWithOrders()
 			$this->assertSame(2, count($account->getOrders()));
 		}
 	}
+
+	/**
+	 * Orders 1, 2 and 3 belong to accounts 1, 4 and 3. The LEFT JOIN limited to
+	 * those orders leaves accounts 2 and 5 with null in every order column.
+	 */
+	public function testAccountWithoutOrdersHasEmptyCollection()
+	{
+		$this->initScript('account-init.sql');
+		$accounts = self::$sqlmap->queryForList("getAccountWithEarlyOrders");
+		$this->assertSame(5, count($accounts));
+
+		$orders = [];
+		foreach ($accounts as $account) {
+			$orders[(int) $account->getId()] = array_map(fn ($order) => (int) $order->getId(), $account->getOrders());
+		}
+		ksort($orders);
+		$this->assertSame([1 => [1], 2 => [], 3 => [3], 4 => [2], 5 => []], $orders);
+	}
 }