From 86a82e720110e2ba8b975902f3f9ed6632c6bedc Mon Sep 17 00:00:00 2001 From: Philip Gichuhi Date: Tue, 8 Nov 2022 23:29:15 +0300 Subject: [PATCH 1/2] Backing store enhancements and tests --- src/Store/BackingStore.php | 4 +- src/Store/BackingStoreFactorySingleton.php | 11 +- src/Store/BackingStoreParseNodeFactory.php | 22 +- ...ngStoreSerializationWriterProxyFactory.php | 31 +-- src/Store/InMemoryBackingStore.php | 81 +++++-- tests/Store/InMemoryBackingStoreTest.php | 203 ++++++++++++++++++ 6 files changed, 289 insertions(+), 63 deletions(-) create mode 100644 tests/Store/InMemoryBackingStoreTest.php diff --git a/src/Store/BackingStore.php b/src/Store/BackingStore.php index 4dcf9f3..aa0c648 100644 --- a/src/Store/BackingStore.php +++ b/src/Store/BackingStore.php @@ -18,7 +18,7 @@ public function get(string $key); * Will trigger subscription callbacks * * @param string $key The key to store and retrieve information. - * @param mixed|null $value The value to be $associated with the given key. + * @param mixed $value The value to be $associated with the given key. */ public function set(string $key, $value): void; @@ -37,7 +37,7 @@ public function enumerateKeysForValuesChangedToNull(): iterable; /** * Creates a subscription to any data change happening. - * @param callable $callback Callback to be invoked on data changes where the first parameter is the data key, the second the previous value and the third the new value. + * @param callable(string $key, $prevValue, $newValue):void $callback Callback to be invoked on data changes where the first parameter is the data key, the second the previous value and the third the new value. * @param string|null $subscriptionId * @return string The subscription ID to use when removing the subscription */ diff --git a/src/Store/BackingStoreFactorySingleton.php b/src/Store/BackingStoreFactorySingleton.php index baea655..75d1dbd 100644 --- a/src/Store/BackingStoreFactorySingleton.php +++ b/src/Store/BackingStoreFactorySingleton.php @@ -11,6 +11,15 @@ abstract class BackingStoreFactorySingleton */ private static ?BackingStoreFactory $instance = null; + /** + * One time initialisation of the backing store factory + * + * @param BackingStoreFactory $instance + */ + public static function setInstance(BackingStoreFactory $backingStoreFactory): void { + self::$instance = self::$instance ?? $backingStoreFactory; + } + /** * We use the getter method since PHP doesn't support instantiating an instance * outside a method. @@ -22,4 +31,4 @@ public static function getInstance(): ?BackingStoreFactory { } return self::$instance; } -} \ No newline at end of file +} diff --git a/src/Store/BackingStoreParseNodeFactory.php b/src/Store/BackingStoreParseNodeFactory.php index 0699cbd..52a8a0c 100644 --- a/src/Store/BackingStoreParseNodeFactory.php +++ b/src/Store/BackingStoreParseNodeFactory.php @@ -16,24 +16,14 @@ class BackingStoreParseNodeFactory extends ParseNodeProxyFactory{ public function __construct(ParseNodeFactory $concrete) { parent::__construct($concrete, static function ($x) { - if (is_a($x, BackedModel::class)) { - $backedModel = $x; - $backingStore = $backedModel->getBackingStore(); - - if (!is_null($backingStore)) { - $backingStore->setIsInitializationCompleted(false); - } - } + if ($x instanceof BackedModel && $x->getBackingStore()) { + $x->getBackingStore()->setIsInitializationCompleted(false); + } }, static function ($x) { - if (is_a($x, BackedModel::class)) { - $backedModel = $x; - $backingStore = $backedModel->getBackingStore(); - - if (!is_null($backingStore)) { - $backingStore->setIsInitializationCompleted(true); - } - } + if ($x instanceof BackedModel && $x->getBackingStore()) { + $x->getBackingStore()->setIsInitializationCompleted(true); + } } ); } diff --git a/src/Store/BackingStoreSerializationWriterProxyFactory.php b/src/Store/BackingStoreSerializationWriterProxyFactory.php index 7bb670d..a20455b 100644 --- a/src/Store/BackingStoreSerializationWriterProxyFactory.php +++ b/src/Store/BackingStoreSerializationWriterProxyFactory.php @@ -17,43 +17,28 @@ class BackingStoreSerializationWriterProxyFactory extends SerializationWriterPro */ public function __construct(SerializationWriterFactory $concreteSerializationWriterFactory){ $onBeforeObjectSerialization = static function (Parsable $model) { - if (is_a($model, BackedModel::class)) { - $backedModel = $model; - $backingStore = $backedModel->getBackingStore(); - if ($backingStore !== null) { - $backingStore->setReturnOnlyChangedValues(true); - } + if ($model instanceof BackedModel && $model->getBackingStore()) { + $model->getBackingStore()->setReturnOnlyChangedValues(true); } }; $onAfterObjectSerialization = static function (Parsable $model) { - if (is_a($model, BackedModel::class)) { - $backedModel = $model; - $backingStore = $backedModel->getBackingStore(); - - if ($backingStore !== null) { - $backingStore->setReturnOnlyChangedValues(false); - $backingStore->setIsInitializationCompleted(true); - } + if ($model instanceof BackedModel && $model->getBackingStore()) { + $model->getBackingStore()->setReturnOnlyChangedValues(false); + $model->getBackingStore()->setIsInitializationCompleted(true); } }; $onStartObjectSerialization = static function (Parsable $model, SerializationWriter $serializationWriter) { - if (is_a($model, BackedModel::class)) { - $backedModel = $model; - - $backingStore = $backedModel->getBackingStore(); - - if ($backingStore !== null) { - $keys = $backingStore->enumerateKeysForValuesChangedToNull(); + if ($model instanceof BackedModel && $model->getBackingStore()) { + $keys = $model->getBackingStore()->enumerateKeysForValuesChangedToNull(); foreach ($keys as $key) { $serializationWriter->writeNullValue($key); } - } } }; parent::__construct($concreteSerializationWriterFactory, $onBeforeObjectSerialization, $onAfterObjectSerialization, $onStartObjectSerialization); } -} \ No newline at end of file +} diff --git a/src/Store/InMemoryBackingStore.php b/src/Store/InMemoryBackingStore.php index cd59202..eb8cf54 100644 --- a/src/Store/InMemoryBackingStore.php +++ b/src/Store/InMemoryBackingStore.php @@ -8,21 +8,26 @@ class InMemoryBackingStore implements BackingStore { private bool $isInitializationCompleted = true; - private bool $returnOnlyChangedValues; + private bool $returnOnlyChangedValues = false; /** - * @var array> $store; + * @var array $store; */ private array $store = []; /** @var array $subscriptionStore */ private array $subscriptionStore = []; + /** * @param string $key * @return mixed */ public function get(string $key) { - $wrapper = (array)($this->store[$key] ?? null); + $this->checkCollectionSizeChanged($key); + $wrapper = $this->store[$key] ?? null; + if (is_null($wrapper)) { + return null; + } return $this->getValueFromWrapper($wrapper); } @@ -32,26 +37,40 @@ public function get(string $key) { */ public function set(string $key, $value): void { - $valueToAdd = [$this->isInitializationCompleted, $value]; - $this->store[$key] = $valueToAdd; - $oldValue = $this->store[$key]; + $oldValue = $this->store[$key] ?? null; + $valueToAdd = is_array($value) ? [$this->isInitializationCompleted, $value, count($value)] : [$this->isInitializationCompleted, $value]; + + // Dirty track changes if $value is a model and its properties change + if (!array_key_exists($key, $this->store)) { + if ($value instanceof BackedModel) { + $value->getBackingStore()->subscribe(fn ($propertyKey, $oldVal, $newVal) => $this->set($key, $value)); + } + if (is_array($value)) { + array_map(function ($item) use ($key, $value) { + if ($item instanceof BackedModel) { + $item->getBackingStore()->subscribe(fn ($propertyKey, $oldVal, $newVal) => $this->set($key, $value)); + } + }, $value); + } + } + $this->store[$key] = $valueToAdd; foreach ($this->subscriptionStore as $callback) { - $callback($key, $oldValue[1], $value); + $callback($key, $oldValue[1] ?? null, $value); } } /** - * @return array + * Enumerate the values in the store based on $returnOnlyChangedValues + * + * @return array key value pairs */ public function enumerate(): array { $result = []; foreach ($this->store as $key => $value) { - $value = (array)$value; - $val = $this->getValueFromWrapper($value); - - if ($val === null) { + $this->checkCollectionSizeChanged($key); + if (!$this->returnOnlyChangedValues || $value[0]) { $result[$key] = $value[1]; } } @@ -59,6 +78,8 @@ public function enumerate(): array { } /** + * Adds a callback to subscribe to events in the store + * * @param callable $callback * @param string|null $subscriptionId * @return string @@ -72,6 +93,8 @@ public function subscribe(callable $callback, ?string $subscriptionId = null): s } /** + * De-register the callback with the given subscriptionId + * * @param string $subscriptionId */ public function unsubscribe(string $subscriptionId): void { @@ -79,7 +102,7 @@ public function unsubscribe(string $subscriptionId): void { } /** - * + * Empties the store */ public function clear(): void { $this->store = []; @@ -114,13 +137,15 @@ public function getReturnOnlyChangedValues(): bool { } /** + * Returns a list of keys in the store that have changed to null + * * @return iterable */ public function enumerateKeysForValuesChangedToNull(): iterable { $result = []; foreach ($this->store as $key => $val) { - if (is_array($val) && $val[1] === null && $val[0]) { + if ($val[1] === null && $val[0]) { $result []= $key; } } @@ -128,17 +153,31 @@ public function enumerateKeysForValuesChangedToNull(): iterable { } /** - * @param array|null $wrapper - * @return mixed|null + * Returns value from $wrapper based on $returnOnlyChangedValues configuration + * + * @param array $wrapper + * @return mixed */ - public function getValueFromWrapper(?array $wrapper) { - if (!is_array($wrapper)) { - return null; - } + private function getValueFromWrapper(array $wrapper) { $hasChangedValue = $wrapper[0]; if (!$this->returnOnlyChangedValues || $hasChangedValue) { return $wrapper[1]; } return null; } -} \ No newline at end of file + + /** + * Checks if collection of values has changed in size. If so, dirty tracks the change by calling set() + * + * @param string $key + * @return void + */ + private function checkCollectionSizeChanged(string $key): void { + $wrapper = $this->store[$key] ?? null; + if ($wrapper && is_array($wrapper[1])) { + if (sizeof($wrapper[1]) != $wrapper[2]) { + $this->set($key, $wrapper[1]); + } + } + } +} diff --git a/tests/Store/InMemoryBackingStoreTest.php b/tests/Store/InMemoryBackingStoreTest.php new file mode 100644 index 0000000..ab080c5 --- /dev/null +++ b/tests/Store/InMemoryBackingStoreTest.php @@ -0,0 +1,203 @@ +backingStore = new InMemoryBackingStore(); + } + + public function testGetFromBackingStore(): void + { + $this->backingStore->set('key', 'value'); + $this->assertEquals('value', $this->backingStore->get('key')); + } + + public function testReturnOnlyChangedValues(): void + { + $this->backingStore->setReturnOnlyChangedValues(true); + $this->backingStore->setIsInitializationCompleted(false); + + $this->backingStore->set('key', 'value'); + $this->assertNull($this->backingStore->get('key')); + $this->assertEquals([], $this->backingStore->enumerate()); + + $this->backingStore->setReturnOnlyChangedValues(false); + $this->assertEquals('value', $this->backingStore->get('key')); + $this->assertEquals(['key' => 'value'], $this->backingStore->enumerate()); + + $this->backingStore->setIsInitializationCompleted(true); + $this->backingStore->setReturnOnlyChangedValues(true); + $this->backingStore->set('key', 'value2'); + $this->assertEquals('value2', $this->backingStore->get('key')); + $this->assertEquals(['key' => 'value2'], $this->backingStore->enumerate()); + } + + public function testEnumerateUserInitializedModelReturnsAllValues(): void + { + $model = new SampleBackedModel(10, null); + + $this->assertTrue($model->getBackingStore()->getIsInitializationCompleted()); + $this->assertFalse($model->getBackingStore()->getReturnOnlyChangedValues()); + + $storeValues = $model->getBackingStore()->enumerate(); + $this->assertEquals(2, sizeof($storeValues)); + + $changedToNull = $model->getBackingStore()->enumerateKeysForValuesChangedToNull(); + $this->assertEquals(1, sizeof($changedToNull)); + $this->assertEquals("name", $changedToNull[0]); + } + + public function testSubscription(): void + { + $callbackExecuted = false; + $callback = function ($key, $oldVal, $newVal) use (&$callbackExecuted) { + $callbackExecuted = $key === 'key' && $oldVal === 'value' && $newVal === 'value2'; + }; + $this->backingStore->set('key', 'value'); + $this->backingStore->subscribe($callback); + $this->backingStore->set('key', 'value2'); + $this->assertTrue($callbackExecuted); + + } + + public function testUnsubscribe(): void + { + $callbackExecuted = false; + $callback = function ($key, $oldVal, $newVal) use (&$callbackExecuted) { + $callbackExecuted = $key === 'key' && $oldVal === 'value' && $newVal === 'value2'; + }; + $this->backingStore->set('key', 'value'); + $subscriptionId = $this->backingStore->subscribe($callback); + $this->backingStore->set('key', 'value2'); + $this->assertTrue($callbackExecuted); + + $this->backingStore->unsubscribe($subscriptionId); + $callbackExecuted = false; + $this->assertFalse($callbackExecuted); + } + + public function testClearStore(): void + { + $this->backingStore->set('key', 'value'); + $this->backingStore->clear(); + $this->assertNull($this->backingStore->get('key')); + $this->assertEmpty($this->backingStore->enumerate()); + } + + public function testEnumerateKeysForValuesChangedToNull(): void + { + $this->backingStore->set('key', 'value'); + $this->assertEquals([], $this->backingStore->enumerateKeysForValuesChangedToNull()); + + $this->backingStore->set('key', null); + $this->assertEquals(['key'], $this->backingStore->enumerateKeysForValuesChangedToNull()); + } + + public function testChangesToCollectionSize(): void + { + $this->backingStore->setReturnOnlyChangedValues(true); + $this->backingStore->setIsInitializationCompleted(false); + + $this->backingStore->set('key', [1, 2, 3]); + $this->assertNull($this->backingStore->get('key')); + + $this->backingStore->setIsInitializationCompleted(true); + $this->backingStore->set('key', [1, 2, 3, 4]); + $this->assertEquals([1, 2, 3, 4], $this->backingStore->get('key')); + } + + public function testChangesToBackedModelValue(): void + { + $model = new SampleBackedModel(10, 'name'); + + $this->backingStore->setReturnOnlyChangedValues(true); + $this->backingStore->setIsInitializationCompleted(false); + $this->backingStore->set('key', $model); + $this->assertNull($this->backingStore->get('key')); + + $this->backingStore->setIsInitializationCompleted(true); + $model->setAge(5); + $this->assertInstanceOf(SampleBackedModel::class, $this->backingStore->get('key')); + $this->assertEquals(5, $this->backingStore->get('key')->getAge()); + } + + public function testChangesToBackedModelCollection(): void + { + $model = new SampleBackedModel(5, 'name2'); + $collection = [new SampleBackedModel(10, 'name'), $model]; + + $this->backingStore->setReturnOnlyChangedValues(true); + $this->backingStore->setIsInitializationCompleted(false); + $this->backingStore->set('key', $collection); + + $this->assertNull($this->backingStore->get('key')); + + $this->backingStore->setIsInitializationCompleted(true); + $model->setAge(250); + $this->assertIsArray($this->backingStore->get('key')); + $this->assertEquals(2, sizeof($this->backingStore->get('key'))); + $this->assertEquals(250, $this->backingStore->get('key')[1]->getAge()); + } +} + +class SampleBackedModel implements BackedModel +{ + private int $age; + private ?string $name = null; + private BackingStore $backingStore; + + public function __construct(int $age, ?string $name = null) + { + $this->backingStore = BackingStoreFactorySingleton::getInstance()->createBackingStore(); + $this->setAge($age); + $this->setName($name); + } + + /** + * @return int + */ + public function getAge(): int + { + return $this->backingStore->get("age"); + } + + /** + * @param int $age + */ + public function setAge(int $age): void + { + $this->backingStore->set("age", $age); + } + + /** + * @return string|null + */ + public function getName(): ?string + { + return $this->backingStore->get("name"); + } + + /** + * @param string|null $name + */ + public function setName(?string $name): void + { + $this->backingStore->set("name", $name); + } + + public function getBackingStore(): ?BackingStore + { + return $this->backingStore; + } +} From 7eae52577b87ad50ac7725af4e5e5fa74d21a8d9 Mon Sep 17 00:00:00 2001 From: Philip Gichuhi Date: Thu, 10 Nov 2022 15:23:55 +0300 Subject: [PATCH 2/2] Fix PHPStan issues --- src/Store/BackingStore.php | 2 +- src/Store/BackingStoreFactorySingleton.php | 2 +- src/Store/InMemoryBackingStore.php | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Store/BackingStore.php b/src/Store/BackingStore.php index aa0c648..5b8af5c 100644 --- a/src/Store/BackingStore.php +++ b/src/Store/BackingStore.php @@ -37,7 +37,7 @@ public function enumerateKeysForValuesChangedToNull(): iterable; /** * Creates a subscription to any data change happening. - * @param callable(string $key, $prevValue, $newValue):void $callback Callback to be invoked on data changes where the first parameter is the data key, the second the previous value and the third the new value. + * @param callable(string, mixed, mixed):void $callback Callback to be invoked on data changes where the first parameter is the data key, the second the previous value and the third the new value. * @param string|null $subscriptionId * @return string The subscription ID to use when removing the subscription */ diff --git a/src/Store/BackingStoreFactorySingleton.php b/src/Store/BackingStoreFactorySingleton.php index 75d1dbd..4d54e32 100644 --- a/src/Store/BackingStoreFactorySingleton.php +++ b/src/Store/BackingStoreFactorySingleton.php @@ -14,7 +14,7 @@ abstract class BackingStoreFactorySingleton /** * One time initialisation of the backing store factory * - * @param BackingStoreFactory $instance + * @param BackingStoreFactory $backingStoreFactory */ public static function setInstance(BackingStoreFactory $backingStoreFactory): void { self::$instance = self::$instance ?? $backingStoreFactory; diff --git a/src/Store/InMemoryBackingStore.php b/src/Store/InMemoryBackingStore.php index eb8cf54..b204e1c 100644 --- a/src/Store/InMemoryBackingStore.php +++ b/src/Store/InMemoryBackingStore.php @@ -11,7 +11,7 @@ class InMemoryBackingStore implements BackingStore private bool $returnOnlyChangedValues = false; /** - * @var array $store; + * @var array> $store */ private array $store = []; @@ -42,12 +42,12 @@ public function set(string $key, $value): void // Dirty track changes if $value is a model and its properties change if (!array_key_exists($key, $this->store)) { - if ($value instanceof BackedModel) { + if ($value instanceof BackedModel && $value->getBackingStore()) { $value->getBackingStore()->subscribe(fn ($propertyKey, $oldVal, $newVal) => $this->set($key, $value)); } if (is_array($value)) { array_map(function ($item) use ($key, $value) { - if ($item instanceof BackedModel) { + if ($item instanceof BackedModel && $item->getBackingStore()) { $item->getBackingStore()->subscribe(fn ($propertyKey, $oldVal, $newVal) => $this->set($key, $value)); } }, $value); @@ -155,7 +155,7 @@ public function enumerateKeysForValuesChangedToNull(): iterable { /** * Returns value from $wrapper based on $returnOnlyChangedValues configuration * - * @param array $wrapper + * @param array $wrapper * @return mixed */ private function getValueFromWrapper(array $wrapper) {