From d69195cd0ae8443fe749eac151efd36688ecb4cb Mon Sep 17 00:00:00 2001 From: Sergei Mikhailov Date: Mon, 20 Jan 2025 16:42:49 +0100 Subject: [PATCH 1/3] fix: null-safe dereferencing of record-typed variables fields via the `` operand --- .../expressions/operators/FieldValueProcessor.php | 11 ++++++++--- .../operators/FieldValueProcessorTest.php | 15 ++++++++------- .../processing/ResponseProcessingEngineTest.php | 2 +- 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/src/qtism/runtime/expressions/operators/FieldValueProcessor.php b/src/qtism/runtime/expressions/operators/FieldValueProcessor.php index 8e61cd913..3e689d596 100644 --- a/src/qtism/runtime/expressions/operators/FieldValueProcessor.php +++ b/src/qtism/runtime/expressions/operators/FieldValueProcessor.php @@ -24,6 +24,7 @@ namespace qtism\runtime\expressions\operators; use qtism\data\expressions\operators\FieldValue; +use qtism\runtime\common\RecordContainer; /** * The FieldValueProcessor class aims at processing FieldValue expressions. @@ -45,16 +46,20 @@ class FieldValueProcessor extends OperatorProcessor #[\ReturnTypeWillChange] public function process() { - $operands = $this->getOperands(); + $operand = $this->getOperands()[0]; - if ($operands->exclusivelyRecord() === false) { + if ($operand === null) { + return null; + } + + if (!$operand instanceof RecordContainer) { $msg = 'The FieldValue operator only accepts operands with a cardinality of record.'; throw new OperatorProcessingException($msg, $this, OperatorProcessingException::WRONG_CARDINALITY); } $fieldIdentifier = $this->getExpression()->getFieldIdentifier(); - return $operands[0][$fieldIdentifier]; + return $operand[$fieldIdentifier]; } /** diff --git a/test/qtismtest/runtime/expressions/operators/FieldValueProcessorTest.php b/test/qtismtest/runtime/expressions/operators/FieldValueProcessorTest.php index dc72bbb19..edbc6730e 100644 --- a/test/qtismtest/runtime/expressions/operators/FieldValueProcessorTest.php +++ b/test/qtismtest/runtime/expressions/operators/FieldValueProcessorTest.php @@ -5,6 +5,7 @@ use qtism\common\datatypes\QtiInteger; use qtism\common\datatypes\QtiPoint; use qtism\common\enums\BaseType; +use qtism\data\expressions\Expression; use qtism\data\QtiComponent; use qtism\data\storage\xml\marshalling\MarshallerNotFoundException; use qtism\runtime\common\MultipleContainer; @@ -24,7 +25,7 @@ public function testNotEnoughOperands(): void $expression = $this->createFakeExpression(); $operands = new OperandsCollection(); $this->expectException(ExpressionProcessingException::class); - $processor = new FieldValueProcessor($expression, $operands); + new FieldValueProcessor($expression, $operands); } public function testTooMuchOperands(): void @@ -34,7 +35,7 @@ public function testTooMuchOperands(): void $operands[] = new RecordContainer(); $operands[] = new RecordContainer(); $this->expectException(ExpressionProcessingException::class); - $processor = new FieldValueProcessor($expression, $operands); + new FieldValueProcessor($expression, $operands); } public function testNullOne(): void @@ -55,9 +56,9 @@ public function testNullTwo(): void $operands = new OperandsCollection(); // null value as operand. $operands[] = null; - $this->expectException(ExpressionProcessingException::class); $processor = new FieldValueProcessor($expression, $operands); $result = $processor->process(); + $this::assertNull($result); } public function testWrongCardinalityOne(): void @@ -68,7 +69,7 @@ public function testWrongCardinalityOne(): void $operands[] = new QtiInteger(10); $processor = new FieldValueProcessor($expression, $operands); $this->expectException(ExpressionProcessingException::class); - $result = $processor->process(); + $processor->process(); } public function testWrongCardinalityTwo(): void @@ -79,7 +80,7 @@ public function testWrongCardinalityTwo(): void $operands[] = new QtiPoint(1, 2); $processor = new FieldValueProcessor($expression, $operands); $this->expectException(ExpressionProcessingException::class); - $result = $processor->process(); + $processor->process(); } public function testWrongCardinalityThree(): void @@ -91,7 +92,7 @@ public function testWrongCardinalityThree(): void // Wrong container (Multiple, Ordered) $processor = new FieldValueProcessor($expression, $operands); $this->expectException(ExpressionProcessingException::class); - $result = $processor->process(); + $processor->process(); } public function testFieldValue(): void @@ -116,7 +117,7 @@ public function testFieldValue(): void * @return QtiComponent * @throws MarshallerNotFoundException */ - public function createFakeExpression($identifier = ''): QtiComponent + public function createFakeExpression($identifier = ''): Expression { // The following XML Component creation // underlines the need of a operator... :) diff --git a/test/qtismtest/runtime/processing/ResponseProcessingEngineTest.php b/test/qtismtest/runtime/processing/ResponseProcessingEngineTest.php index a258bea63..ae6ca3d9a 100644 --- a/test/qtismtest/runtime/processing/ResponseProcessingEngineTest.php +++ b/test/qtismtest/runtime/processing/ResponseProcessingEngineTest.php @@ -221,7 +221,7 @@ public function testResponseProcessingErrorCollection(): void self::assertNotNull($exceptions); self::assertEquals( - 'The FieldValue operator only accepts operands with a cardinality of record.', + 'No variable with identifier \'SCORE\' to be set in the current state.', $exceptions->getProcessingExceptions()[0]->getMessage() ); } From 4b284204f481e08d668fda29078e681b8b5aaa63 Mon Sep 17 00:00:00 2001 From: Sergei Mikhailov Date: Mon, 20 Jan 2025 17:07:00 +0100 Subject: [PATCH 2/3] ci: add PHP 8.4 and execute the tests with coverage only on the latest version of PHP --- .github/workflows/continuous-integration.yaml | 21 +++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/.github/workflows/continuous-integration.yaml b/.github/workflows/continuous-integration.yaml index 740c52797..141249533 100644 --- a/.github/workflows/continuous-integration.yaml +++ b/.github/workflows/continuous-integration.yaml @@ -6,16 +6,23 @@ on: pull_request: branches: [ develop ] +concurrency: + group: ci-${{ github.ref }} + cancel-in-progress: true + jobs: build: - runs-on: ${{ matrix.operating-system }} + runs-on: ubuntu-latest strategy: fail-fast: false matrix: - operating-system: [ ubuntu-latest ] - php-versions: [ '7.4', '8.0', '8.1', '8.2', '8.3'] + php-version: [ '7.4', '8.0', '8.1', '8.2', '8.3' ] + coverage: [ false ] + include: + - php-version: '8.4' + coverage: true steps: - uses: actions/checkout@v3 @@ -26,7 +33,7 @@ jobs: - name: Install PHP uses: shivammathur/setup-php@v2 with: - php-version: ${{ matrix.php-versions }} + php-version: ${{ matrix.php-version }} - name: Validate composer.json and composer.lock run: composer validate --strict @@ -35,7 +42,13 @@ jobs: run: composer install --prefer-dist --no-progress - name: Run test suite + if: ${{ !matrix.coverage }} + run: vendor/bin/phpunit + + - name: Run test suite with code coverage + if: ${{ matrix.coverage }} run: php -dxdebug.mode=coverage vendor/bin/phpunit --coverage-clover coverage.xml - name: Push coverage report + if: ${{ matrix.coverage }} uses: codecov/codecov-action@v2 From 3a9dc6bf07fe579992be705ee5755f78472e14ba Mon Sep 17 00:00:00 2001 From: Sergei Mikhailov Date: Mon, 20 Jan 2025 17:22:42 +0100 Subject: [PATCH 3/3] ci: remove invalid operations on the DOMDocument from the tests --- .../common/dom/SerializableDomDocumentTest.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/qtismtest/common/dom/SerializableDomDocumentTest.php b/test/qtismtest/common/dom/SerializableDomDocumentTest.php index 8e66f0e56..e34217531 100644 --- a/test/qtismtest/common/dom/SerializableDomDocumentTest.php +++ b/test/qtismtest/common/dom/SerializableDomDocumentTest.php @@ -81,7 +81,7 @@ public function testCallingNotExistedVirtualMethods(): void $dom->$method(); } - public function testCheckThatUnsetIsWorkingSimilarToRealDomObject(): void + public function testMagicMutatorsAndAccessors(): void { $serializableDOM = $this->getSerializableDomDocument(); $coreDom = new DOMDocument($serializableDOM->xmlVersion, $serializableDOM->encoding); @@ -89,11 +89,11 @@ public function testCheckThatUnsetIsWorkingSimilarToRealDomObject(): void $this->assertEquals($coreDom->xmlVersion, $serializableDOM->version); $this->assertEquals($coreDom->encoding, $serializableDOM->encoding); - unset($coreDom->xmlVersion); - unset($coreDom->encoding); + $coreDom->xmlVersion = null; + $coreDom->encoding = 'ASCII'; - unset($serializableDOM->xmlVersion); - unset($serializableDOM->encoding); + $serializableDOM->xmlVersion = null; + $serializableDOM->encoding = 'ASCII'; $this->assertEquals($coreDom->xmlVersion, $serializableDOM->version); $this->assertEquals($coreDom->encoding, $serializableDOM->encoding);