From 763f4e7829a4f54e8627866be5609c53824db2d7 Mon Sep 17 00:00:00 2001 From: Eric Fortmeyer Date: Wed, 15 Mar 2023 05:20:12 -0500 Subject: [PATCH] fix: remove mutation bugs Removing and updating was broken. --- .pre-commit-config.yaml | 1 + acceptance-test-results.md | 12 +- ...Phpolar-CsvFileStorage-CsvFileStorage.html | 2 +- docs/graphs/classes.svg | 48 ++--- phpunit.dev.xml | 2 +- src/CsvFileStorage.php | 44 +++- tests/acceptance/StorageMutabilityTest.php | 190 ++++++++++++++++++ 7 files changed, 260 insertions(+), 39 deletions(-) create mode 100644 tests/acceptance/StorageMutabilityTest.php diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index fa69358..a84abc1 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -6,6 +6,7 @@ repos: hooks: - id: trailing-whitespace - id: end-of-file-fixer + exclude: (docs|^acceptance-test-results\.md$) - id: check-json - id: check-xml - id: check-yaml diff --git a/acceptance-test-results.md b/acceptance-test-results.md index 8f4360b..fb2a078 100644 --- a/acceptance-test-results.md +++ b/acceptance-test-results.md @@ -1,8 +1,16 @@ -# Accpetance Test Results +# Acceptance Test Results ## Memory Usage (Phpolar\CsvFileStorage\MemoryUsage) - [x] Memory usage for saving data shall be below 2000 bytes -- [x] Memory usage for saving data (with pre-loading disabled) shall be below 115000 bytes +- [x] Memory usage for saving data (with pre-loading disabled) shall be below 125000 bytes + +## Storage Mutability (Phpolar\CsvFileStorage\StorageMutability) +- [x] Shall create persistent storage +- [x] Shall add items to storage without removing existing values +- [x] Shall update objects in storage without removing other objects +- [x] Shall update items in storage without removing other values +- [x] Shall remove objects from storage without removing other objects +- [x] Shall remove items from storage without removing other values ## Project Size (Phpolar\CsvFileStorage\ProjectSize) - [x] Source code total size shall be below 4000 bytes diff --git a/docs/classes/Phpolar-CsvFileStorage-CsvFileStorage.html b/docs/classes/Phpolar-CsvFileStorage-CsvFileStorage.html index 98859f6..e0abace 100644 --- a/docs/classes/Phpolar-CsvFileStorage-CsvFileStorage.html +++ b/docs/classes/Phpolar-CsvFileStorage-CsvFileStorage.html @@ -329,7 +329,7 @@

diff --git a/docs/graphs/classes.svg b/docs/graphs/classes.svg index ebdf24e..8b64aea 100644 --- a/docs/graphs/classes.svg +++ b/docs/graphs/classes.svg @@ -1,16 +1,16 @@ -Phpolar\CsvFileStoragePhpolar\Phpolar\StorageAmbiguousUnionTypeExceptionCsvFileStorageFileNotExistsExceptionAbstractStorageRuntimeExceptionCountablePhpolar\CsvFileStoragePhpolar\Phpolar\StorageAmbiguousUnionTypeExceptionCsvFileStorageFileNotExistsExceptionAbstractStorageRuntimeExceptionCountable + +PlantUML version 1.2020.02(Sun Mar 01 04:22:07 CST 2020) +(GPL source distribution) +Java Runtime: OpenJDK Runtime Environment +JVM: OpenJDK 64-Bit Server VM +Java Version: 11.0.18+10-post-Ubuntu-0ubuntu122.04 +Operating System: Linux +Default Encoding: UTF-8 +Language: en +Country: US +--> diff --git a/phpunit.dev.xml b/phpunit.dev.xml index b7c0446..6a5711a 100644 --- a/phpunit.dev.xml +++ b/phpunit.dev.xml @@ -25,6 +25,6 @@ - + diff --git a/src/CsvFileStorage.php b/src/CsvFileStorage.php index f2ceef2..f9a8d18 100644 --- a/src/CsvFileStorage.php +++ b/src/CsvFileStorage.php @@ -49,10 +49,10 @@ final class CsvFileStorage extends AbstractStorage implements Countable private const MEMORY_STREAM = "php://memory"; - public function __construct(string $filename, private ?string $typeClassName = null) + public function __construct(private string $filename, private ?string $typeClassName = null) { - $readMode = $filename === self::MEMORY_STREAM ? "r" : "r+"; - $writeMode = $filename === self::MEMORY_STREAM ? "a+" : "r+"; + $readMode = "r"; + $writeMode = "a"; $readStream = fopen($filename, $readMode); $writeStream = fopen($filename, $writeMode); $this->closeWriteStream = $filename !== self::MEMORY_STREAM; @@ -84,11 +84,22 @@ public function __destruct() public function commit(): void { - foreach ($this->getAll() as $record) { - $this->typeClassName ??= is_object($record) === true ? get_class($record) : null; + if ($this->fileSize > 0) { + fclose($this->writeStream); + $file = fopen($this->filename, "w"); + // @codeCoverageIgnoreStart + if ($file !== false) { + $this->writeStream = $file; + } + // @codeCoverageIgnoreEnd + } + foreach ($this->getAll() as $index => $record) { switch (true) { case is_object($record): - fputcsv($this->writeStream, $this->convertObjVars($record)); + $objVars = get_object_vars($record); + $this->setUpObject($index, $record, $objVars); + $convertedVars = $this->convertObjVars($objVars); + fputcsv($this->writeStream, $convertedVars); break; case is_scalar($record): fputcsv($this->writeStream, [$record]); @@ -104,17 +115,18 @@ public function commit(): void } /** - * @return array + * @param array $objVars + * @return array */ - private function convertObjVars(object $record): array + private function convertObjVars(array $objVars): array { return array_map( static fn (mixed $item) => match (true) { - $item instanceof DateTimeImmutable => $item->format(DATE_RSS), + $item instanceof DateTimeImmutable => $item->format(DATE_RFC3339), is_scalar($item) => (string) $item, default => "", }, - get_object_vars($record), + $objVars, ); } @@ -135,7 +147,6 @@ protected function load(): void if ($this->hasEmptyHeader() === true) { throw new DomainException("Malformed CSV file"); } - if ($this->hasObjects() === false) { $this->storeLine($this->firstLine); } @@ -165,6 +176,17 @@ private function setFirstLine(): void } } + /** + * @param array $objVars + */ + private function setUpObject(int|string $index, object $record, array $objVars): void + { + if ($index === 0) { + $this->typeClassName = get_class($record); + fputcsv($this->writeStream, array_keys($objVars)); + } + } + /** * @param array $headers * @param list $line diff --git a/tests/acceptance/StorageMutabilityTest.php b/tests/acceptance/StorageMutabilityTest.php new file mode 100644 index 0000000..efe0198 --- /dev/null +++ b/tests/acceptance/StorageMutabilityTest.php @@ -0,0 +1,190 @@ +filename = tempnam(sys_get_temp_dir(), uniqid()); + } + + protected function tearDown(): void + { + file_exists($this->filename) && unlink($this->filename); + } + + #[Test] + #[TestDox("Shall create persistent storage")] + public function criterionA() + { + $sutA = new CsvFileStorage($this->filename); + $key = new ItemKey(0); + $item = new Item(PHP_INT_MAX); + $sutA->storeByKey($key, $item); + $sutA->commit(); + $this->assertFileExists($this->filename); + $sutB = new CsvFileStorage($this->filename); + $stored = $sutB->getByKey($key); + $this->assertContains((string) $item->bind(), $stored->bind()); + } + + #[Test] + #[TestDox("Shall add items to storage without removing existing values")] + public function criterionB() + { + $sutA = new CsvFileStorage($this->filename); + $key0 = new ItemKey(0); + $item0 = new Item(PHP_INT_MAX); + $sutA->storeByKey($key0, $item0); + $sutA->commit(); + $sutB = new CsvFileStorage($this->filename); + $key1 = new ItemKey(1); + $item1 = new Item(PHP_INT_MIN); + $sutB->storeByKey($key1, $item1); + $sutB->commit(); + $sutC = new CsvFileStorage($this->filename); + $stored0 = $sutC->getByKey($key0); + $stored1 = $sutC->getByKey($key1); + $this->assertContains((string) $item0->bind(), $stored0->bind()); + $this->assertContains((string) $item1->bind(), $stored1->bind()); + } + + #[Test] + #[TestDox("Shall remove items from storage without removing other values")] + public function criterionC() + { + $sutA = new CsvFileStorage($this->filename); + $key0 = new ItemKey(0); + $item0 = new Item(PHP_INT_MAX); + $sutA->storeByKey($key0, $item0); + $sutA->commit(); + unset($sutA); + $sutB = new CsvFileStorage($this->filename); + $key1 = new ItemKey(1); + $item1 = new Item(PHP_INT_MIN); + $key2 = new ItemKey(2); + $item2 = new Item(PHP_OS); + $sutB->storeByKey($key1, $item1); + $sutB->storeByKey($key2, $item2); + $sutB->commit(); + unset($sutB); + $sutC = new CsvFileStorage($this->filename); + $sutC->removeByKey($key2); + $sutC->commit(); + unset($sutC); + $sutD = new CsvFileStorage($this->filename); + $stored0 = $sutD->getByKey($key0); + $stored1 = $sutD->getByKey($key1); + $stored2 = $sutD->getByKey($key2); + $this->assertContains((string) $item0->bind(), $stored0->bind()); + $this->assertContains((string) $item1->bind(), $stored1->bind()); + $this->assertInstanceOf(ItemNotFound::class, $stored2); + } + + #[Test] + #[TestDox("Shall update items in storage without removing other values")] + public function criterionD() + { + $sutA = new CsvFileStorage($this->filename); + $key0 = new ItemKey(0); + $item0 = new Item(PHP_INT_MAX); + $sutA->storeByKey($key0, $item0); + $sutA->commit(); + $sutB = new CsvFileStorage($this->filename); + $key1 = new ItemKey(1); + $item1 = new Item(PHP_INT_MIN); + $item2 = new Item(PHP_OS); + $key2 = new ItemKey(2); + $sutB->storeByKey($key1, $item1); + $sutB->storeByKey($key2, $item2); + $sutB->commit(); + $sutC = new CsvFileStorage($this->filename); + $item2Updated = new Item(PHP_SAPI); + $sutC->replaceByKey($key2, $item2Updated); + $sutC->commit(); + $sutD = new CsvFileStorage($this->filename); + $stored0 = $sutD->getByKey($key0); + $stored1 = $sutD->getByKey($key1); + $stored2 = $sutD->getByKey($key2); + $this->assertContains((string) $item0->bind(), $stored0->bind()); + $this->assertContains((string) $item1->bind(), $stored1->bind()); + $this->assertContains((string) $item2Updated->bind(), $stored2->bind()); + } + + #[Test] + #[TestDox("Shall remove objects from storage without removing other objects")] + public function criterionE() + { + $sutA = new CsvFileStorage($this->filename, FakeValueObject::class); + $key0 = new ItemKey(0); + $item0 = new Item(new FakeValueObject("fake1")); + $sutA->storeByKey($key0, $item0); + $sutA->commit(); + unset($sutA); + $sutB = new CsvFileStorage($this->filename, FakeValueObject::class); + $key1 = new ItemKey(1); + $item1 = new Item(new FakeValueObject("fake2")); + $key2 = new ItemKey(2); + $item2 = new Item(new FakeValueObject("fake3")); + $sutB->storeByKey($key1, $item1); + $sutB->storeByKey($key2, $item2); + $sutB->commit(); + unset($sutB); + $sutC = new CsvFileStorage($this->filename, FakeValueObject::class); + $sutC->removeByKey($key2); + $sutC->commit(); + unset($sutC); + $sutD = new CsvFileStorage($this->filename, FakeValueObject::class); + $stored0 = $sutD->getByKey($key0); + $stored1 = $sutD->getByKey($key1); + $stored2 = $sutD->getByKey($key2); + $this->assertSame($item0->bind()->title, $stored0->bind()->title); + $this->assertSame($item1->bind()->title, $stored1->bind()->title); + $this->assertInstanceOf(ItemNotFound::class, $stored2); + } + + #[Test] + #[TestDox("Shall update objects in storage without removing other objects")] + public function criterionF() + { + $sutA = new CsvFileStorage($this->filename, FakeValueObject::class); + $key0 = new ItemKey(0); + $item0 = new Item(new FakeValueObject("fake1")); + $sutA->storeByKey($key0, $item0); + $sutA->commit(); + $sutB = new CsvFileStorage($this->filename, FakeValueObject::class); + $key1 = new ItemKey(1); + $item1 = new Item(new FakeValueObject("fake2")); + $item2 = new Item(new FakeValueObject("fake3")); + $key2 = new ItemKey(2); + $sutB->storeByKey($key1, $item1); + $sutB->storeByKey($key2, $item2); + $sutB->commit(); + $sutC = new CsvFileStorage($this->filename, FakeValueObject::class); + $item2Updated = new Item(new FakeValueObject("fake3 UPDATED")); + $sutC->replaceByKey($key2, $item2Updated); + $sutC->commit(); + $sutD = new CsvFileStorage($this->filename, FakeValueObject::class); + $stored0 = $sutD->getByKey($key0); + $stored1 = $sutD->getByKey($key1); + $stored2 = $sutD->getByKey($key2); + $this->assertSame($item0->bind()->title, $stored0->bind()->title); + $this->assertSame($item1->bind()->title, $stored1->bind()->title); + $this->assertSame($item2Updated->bind()->title, $stored2->bind()->title); + } +}