From 078521b0b26865097b589dc0bdf56a76f9edee1d Mon Sep 17 00:00:00 2001 From: sadeesh-sdet Date: Sat, 16 Sep 2023 17:01:59 +0530 Subject: [PATCH 01/11] MDTT-38: Fix for CI failure. --- .github/workflows/tests.yml | 4 ++-- src/Test/DefaultTest.php | 34 +++++++++++++++++++++++++++++----- 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 95c0ca1..795910f 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -33,7 +33,7 @@ jobs: run: composer install --no-progress --no-suggest --prefer-dist --optimize-autoloader - name: Security check installed dependencies - uses: symfonycorp/security-checker-action@v2 + uses: symfonycorp/security-checker-action@v4 - name: Check PSR2 code style (PHP_CodeSniffer) run: composer php-cs @@ -47,4 +47,4 @@ jobs: - name: Send code coverage report to Codecov.io uses: codecov/codecov-action@v2 with: - token: ${{ secrets.CODECOV_TOKEN }} + token: ${{ secrets.CODECOV_TOKEN }} \ No newline at end of file diff --git a/src/Test/DefaultTest.php b/src/Test/DefaultTest.php index 9a8e5c7..b8a5596 100644 --- a/src/Test/DefaultTest.php +++ b/src/Test/DefaultTest.php @@ -9,7 +9,6 @@ class DefaultTest extends Test { - /** * @inheritDoc */ @@ -53,20 +52,45 @@ public function execute(array $sourceData, array $destinationData): bool } /** - * @param array>|numeric-string $data - * @param array $fields + * Recursively checks if an offset exists in a nested array. + * + * This function is designed to handle nested arrays and verify the existence of + * a specified offset (string key) within them. It performs recursive checks on + * nested arrays to ensure that the offset exists at each level. + * + * @param array>|numeric-string $data + * The data array to check for the offset in. + * @param array $fields + * An array of string keys representing the path to the desired offset. * * @return bool + * Returns `true` if the specified offset exists in the nested array, + * `false` otherwise. + * + * Used a combination of type hinting and explicit checks to assist PHPStan + * in understanding the types involved and resolving static analysis errors. */ private function issetField(mixed $data, array $fields): bool { + // Check if $data is an array + if (!is_array($data)) { + return false; + } + + // Get the next key to check $key = array_shift($fields); + + // If there are no more keys to check, the offset exists if ($key === null) { return true; } - $test = isset($data[$key]); + // Check if the key exists in the data array + if (!array_key_exists($key, $data)) { + return false; + } - return $test && $this->issetField($data[$key], $fields); + // Recursively check the next level + return $this->issetField($data[$key], $fields); } } From 00d0429869b20b53f22086d41abefee063c5785e Mon Sep 17 00:00:00 2001 From: sadeesh-sdet Date: Tue, 3 Oct 2023 20:04:22 +0530 Subject: [PATCH 02/11] Address reviewer feedback: disable code coverage and update issetField() --- .github/workflows/tests.yml | 16 +++++++++------- src/Test/DefaultTest.php | 12 ++---------- 2 files changed, 11 insertions(+), 17 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 795910f..4c3a818 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -41,10 +41,12 @@ jobs: - name: Analyse PHP Code (PHPStan) run: composer phpstan - - name: Validate code quality - run: composer phpunit-with-coverage - - - name: Send code coverage report to Codecov.io - uses: codecov/codecov-action@v2 - with: - token: ${{ secrets.CODECOV_TOKEN }} \ No newline at end of file + # As codecov throwing lot of errors, disabling the respective check temporarily + # Reference discussion: https://github.com/axelerant/mdtt/pull/24#pullrequestreview-1632092133 + # - name: Validate code quality + # run: composer phpunit-with-coverage + + # - name: Send code coverage report to Codecov.io + # uses: codecov/codecov-action@v2 + # with: + # token: ${{ secrets.CODECOV_TOKEN }} \ No newline at end of file diff --git a/src/Test/DefaultTest.php b/src/Test/DefaultTest.php index b8a5596..b3687c6 100644 --- a/src/Test/DefaultTest.php +++ b/src/Test/DefaultTest.php @@ -58,7 +58,7 @@ public function execute(array $sourceData, array $destinationData): bool * a specified offset (string key) within them. It performs recursive checks on * nested arrays to ensure that the offset exists at each level. * - * @param array>|numeric-string $data + * @param array> $data * The data array to check for the offset in. * @param array $fields * An array of string keys representing the path to the desired offset. @@ -66,17 +66,9 @@ public function execute(array $sourceData, array $destinationData): bool * @return bool * Returns `true` if the specified offset exists in the nested array, * `false` otherwise. - * - * Used a combination of type hinting and explicit checks to assist PHPStan - * in understanding the types involved and resolving static analysis errors. */ - private function issetField(mixed $data, array $fields): bool + private function issetField(array $data, array $fields): bool { - // Check if $data is an array - if (!is_array($data)) { - return false; - } - // Get the next key to check $key = array_shift($fields); From 3b5ab9a6b7c4ad9c480e55bce14bcd81d11a886b Mon Sep 17 00:00:00 2001 From: sadeesh-sdet Date: Tue, 3 Oct 2023 20:37:11 +0530 Subject: [PATCH 03/11] \[Fix\] issetField\(\): Handle the case where $data\[$key\] is not an array --- src/Test/DefaultTest.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/Test/DefaultTest.php b/src/Test/DefaultTest.php index b3687c6..bb49209 100644 --- a/src/Test/DefaultTest.php +++ b/src/Test/DefaultTest.php @@ -58,7 +58,7 @@ public function execute(array $sourceData, array $destinationData): bool * a specified offset (string key) within them. It performs recursive checks on * nested arrays to ensure that the offset exists at each level. * - * @param array> $data + * @param array> $data * The data array to check for the offset in. * @param array $fields * An array of string keys representing the path to the desired offset. @@ -82,6 +82,11 @@ private function issetField(array $data, array $fields): bool return false; } + // Make sure that the key value is an array of strings + if (!is_array($data[$key]) || !array_is_list($data[$key])) { + return false; + } + // Recursively check the next level return $this->issetField($data[$key], $fields); } From 86424acfb3600954133356332c387493deb6f24e Mon Sep 17 00:00:00 2001 From: sadeesh-sdet Date: Tue, 3 Oct 2023 21:01:37 +0530 Subject: [PATCH 04/11] \[Improve\] issetField\(\): Handle the case where $data\[$key\] is not an array of strings --- src/Test/DefaultTest.php | 36 +++++++++++++++++++++++++++++++++--- 1 file changed, 33 insertions(+), 3 deletions(-) diff --git a/src/Test/DefaultTest.php b/src/Test/DefaultTest.php index bb49209..11a23ea 100644 --- a/src/Test/DefaultTest.php +++ b/src/Test/DefaultTest.php @@ -58,7 +58,7 @@ public function execute(array $sourceData, array $destinationData): bool * a specified offset (string key) within them. It performs recursive checks on * nested arrays to ensure that the offset exists at each level. * - * @param array> $data + * @param array> $data * The data array to check for the offset in. * @param array $fields * An array of string keys representing the path to the desired offset. @@ -66,6 +66,9 @@ public function execute(array $sourceData, array $destinationData): bool * @return bool * Returns `true` if the specified offset exists in the nested array, * `false` otherwise. + * + * @throws InvalidArgumentException + * If the input data structure is not as expected. */ private function issetField(array $data, array $fields): bool { @@ -83,11 +86,38 @@ private function issetField(array $data, array $fields): bool } // Make sure that the key value is an array of strings - if (!is_array($data[$key]) || !array_is_list($data[$key])) { - return false; + if (!is_array($data[$key]) || !is_list_of_strings($data[$key])) { + throw new InvalidArgumentException("Data structure is not as expected."); } // Recursively check the next level return $this->issetField($data[$key], $fields); } + + /** + * Checks if an array contains a list of strings. + * + * @param array $value + * The array to check. + * + * @return bool + * Returns `true` if the array contains a list of strings, `false` otherwise. + * + * @throws InvalidArgumentException + * If the input value is not an array. + */ + private function is_list_of_strings(array $value): bool + { + if (!is_array($value)) { + throw new InvalidArgumentException("Input must be an array."); + } + + foreach ($value as $item) { + if (!is_string($item)) { + return false; + } + } + + return true; + } } From 40e261a4471b869b53a4becd195b04b5fa1c5253 Mon Sep 17 00:00:00 2001 From: sadeesh-sdet Date: Tue, 3 Oct 2023 21:09:13 +0530 Subject: [PATCH 05/11] \[Improve\] Handle non-array values in issetField() and update method name. --- src/Test/DefaultTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Test/DefaultTest.php b/src/Test/DefaultTest.php index 11a23ea..b0b1229 100644 --- a/src/Test/DefaultTest.php +++ b/src/Test/DefaultTest.php @@ -86,7 +86,7 @@ private function issetField(array $data, array $fields): bool } // Make sure that the key value is an array of strings - if (!is_array($data[$key]) || !is_list_of_strings($data[$key])) { + if (!is_array($data[$key]) || !isListOfStrings($data[$key])) { throw new InvalidArgumentException("Data structure is not as expected."); } @@ -106,7 +106,7 @@ private function issetField(array $data, array $fields): bool * @throws InvalidArgumentException * If the input value is not an array. */ - private function is_list_of_strings(array $value): bool + private function isListOfStrings(array $value): bool { if (!is_array($value)) { throw new InvalidArgumentException("Input must be an array."); From 88896c98b8a74aa9d5caeb4baf6b96815baa2ba8 Mon Sep 17 00:00:00 2001 From: sadeesh-sdet Date: Tue, 3 Oct 2023 21:25:24 +0530 Subject: [PATCH 06/11] \[Refactor\] Improve `issetField()` and add `isListOfStrings()` method --- src/Test/DefaultTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Test/DefaultTest.php b/src/Test/DefaultTest.php index b0b1229..9a839d0 100644 --- a/src/Test/DefaultTest.php +++ b/src/Test/DefaultTest.php @@ -6,7 +6,7 @@ use Mdtt\Exception\ExecutionException; use PHPUnit\Framework\Assert; - +use Mdtt\Test\InvalidArgumentException; class DefaultTest extends Test { /** @@ -106,7 +106,7 @@ private function issetField(array $data, array $fields): bool * @throws InvalidArgumentException * If the input value is not an array. */ - private function isListOfStrings(array $value): bool + public function isListOfStrings(array $value): bool { if (!is_array($value)) { throw new InvalidArgumentException("Input must be an array."); From 1eb3d89d17f39a163d31497baa93967a4c11f590 Mon Sep 17 00:00:00 2001 From: sadeesh-sdet Date: Tue, 3 Oct 2023 21:27:42 +0530 Subject: [PATCH 07/11] \[Refactor\] Improve `issetField()` and add `isListOfStrings()` method --- src/Test/DefaultTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Test/DefaultTest.php b/src/Test/DefaultTest.php index 9a839d0..01e5b33 100644 --- a/src/Test/DefaultTest.php +++ b/src/Test/DefaultTest.php @@ -7,6 +7,7 @@ use Mdtt\Exception\ExecutionException; use PHPUnit\Framework\Assert; use Mdtt\Test\InvalidArgumentException; + class DefaultTest extends Test { /** From 796a43562091425e61ae2aeba579783ba14f978c Mon Sep 17 00:00:00 2001 From: sadeesh-sdet Date: Tue, 3 Oct 2023 21:48:10 +0530 Subject: [PATCH 08/11] \[Refactor\] Improve `issetField()` and add `isListOfStrings()` method and add custom exception class --- src/Exception/InvalidArgumentException.php | 10 ++++++++++ src/Test/DefaultTest.php | 6 +++--- 2 files changed, 13 insertions(+), 3 deletions(-) create mode 100644 src/Exception/InvalidArgumentException.php diff --git a/src/Exception/InvalidArgumentException.php b/src/Exception/InvalidArgumentException.php new file mode 100644 index 0000000..a59111d --- /dev/null +++ b/src/Exception/InvalidArgumentException.php @@ -0,0 +1,10 @@ +isListOfStrings($data[$key])) { throw new InvalidArgumentException("Data structure is not as expected."); } @@ -107,7 +107,7 @@ private function issetField(array $data, array $fields): bool * @throws InvalidArgumentException * If the input value is not an array. */ - public function isListOfStrings(array $value): bool + private function isListOfStrings (array $value): bool { if (!is_array($value)) { throw new InvalidArgumentException("Input must be an array."); From 3056290aa93e3b6a56244271f09b3c64a5dfb211 Mon Sep 17 00:00:00 2001 From: sadeesh-sdet Date: Tue, 3 Oct 2023 21:51:41 +0530 Subject: [PATCH 09/11] \[Refactor\] Improve `issetField()` and add `isListOfStrings()` method and add custom exception class --- src/Exception/InvalidArgumentException.php | 2 +- src/Test/DefaultTest.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Exception/InvalidArgumentException.php b/src/Exception/InvalidArgumentException.php index a59111d..260b4f7 100644 --- a/src/Exception/InvalidArgumentException.php +++ b/src/Exception/InvalidArgumentException.php @@ -7,4 +7,4 @@ class InvalidArgumentException extends \Exception { -} \ No newline at end of file +} diff --git a/src/Test/DefaultTest.php b/src/Test/DefaultTest.php index 9dbd9e5..b70640e 100644 --- a/src/Test/DefaultTest.php +++ b/src/Test/DefaultTest.php @@ -107,7 +107,7 @@ private function issetField(array $data, array $fields): bool * @throws InvalidArgumentException * If the input value is not an array. */ - private function isListOfStrings (array $value): bool + private function isListOfStrings(array $value): bool { if (!is_array($value)) { throw new InvalidArgumentException("Input must be an array."); From 781d40119a4a0221dc57e230519fe727f8c7bbdf Mon Sep 17 00:00:00 2001 From: sadeesh-sdet Date: Wed, 4 Oct 2023 18:52:22 +0530 Subject: [PATCH 10/11] Refactor: Updated isListOfStrings parameter type hint to iterable. --- src/Test/DefaultTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Test/DefaultTest.php b/src/Test/DefaultTest.php index b70640e..e12766a 100644 --- a/src/Test/DefaultTest.php +++ b/src/Test/DefaultTest.php @@ -98,7 +98,7 @@ private function issetField(array $data, array $fields): bool /** * Checks if an array contains a list of strings. * - * @param array $value + * @param iterable $value * The array to check. * * @return bool @@ -107,7 +107,7 @@ private function issetField(array $data, array $fields): bool * @throws InvalidArgumentException * If the input value is not an array. */ - private function isListOfStrings(array $value): bool + private function isListOfStrings(iterable $value): bool { if (!is_array($value)) { throw new InvalidArgumentException("Input must be an array."); From 5a5b5adf3f3a732497d07ac232d10ec76a754d87 Mon Sep 17 00:00:00 2001 From: sadeesh-sdet Date: Thu, 16 Nov 2023 20:10:10 +0530 Subject: [PATCH 11/11] Refactor: Renamed isListOfStrings function to isArrayOfStrings and used built-in InvalidArgumentException class --- src/Exception/InvalidArgumentException.php | 10 ---------- src/Test/DefaultTest.php | 12 ++++++------ 2 files changed, 6 insertions(+), 16 deletions(-) delete mode 100644 src/Exception/InvalidArgumentException.php diff --git a/src/Exception/InvalidArgumentException.php b/src/Exception/InvalidArgumentException.php deleted file mode 100644 index 260b4f7..0000000 --- a/src/Exception/InvalidArgumentException.php +++ /dev/null @@ -1,10 +0,0 @@ -isListOfStrings($data[$key])) { + if (!is_array($data[$key]) || !$this->isArrayOfStrings($data[$key])) { throw new InvalidArgumentException("Data structure is not as expected."); } @@ -96,18 +96,18 @@ private function issetField(array $data, array $fields): bool } /** - * Checks if an array contains a list of strings. + * Checks if an iterable value is an array containing only strings. * * @param iterable $value * The array to check. * * @return bool - * Returns `true` if the array contains a list of strings, `false` otherwise. + * Returns `true` if the iterable value is an array containing only strings, `false` otherwise. * * @throws InvalidArgumentException - * If the input value is not an array. + * If the input value is not an array or contains non-string elements.. */ - private function isListOfStrings(iterable $value): bool + private function isArrayOfStrings(iterable $value): bool { if (!is_array($value)) { throw new InvalidArgumentException("Input must be an array.");