Merge branch 'ISSUE-198' of github.com:neo4j-php/neo4j-php-client int…

…o ISSUE-198

.github/workflows/integration-test-aura.yml

@@ -25,14 +25,14 @@ jobs:
       - uses: php-actions/composer@v6
         with:
           progress: yes
-          php_version: 8.0
+          php_version: 8.1
           version: 2
       - name: clean database
         run: CONNECTION=$CONNECTION php tests/clean-database.php
       - uses: php-actions/phpunit@v3
         with:
           configuration: phpunit.xml.dist
-          php_version: 8.0
+          php_version: 8.1
           memory_limit: 1024M
           version: composer
           testsuite: Integration

.github/workflows/integration-test-cluster-neo4j-4.yml

@@ -13,7 +13,7 @@ jobs:
     runs-on: ubuntu-latest
     env:
       CONNECTION: neo4j://neo4j:testtest@localhost:7688
-    name: "Running on PHP 8.0 in a Neo4j 4.4 cluster"
+    name: "Running on PHP 8.1 in a Neo4j 4.4 cluster"
 
     steps:
       - uses: actions/checkout@v2
@@ -25,12 +25,12 @@ jobs:
       - uses: php-actions/composer@v6
         with:
           progress: yes
-          php_version: 8.0
+          php_version: 8.1
           version: 2
       - uses: php-actions/phpunit@v3
         with:
           configuration: phpunit.xml.dist
-          php_version: 8.0
+          php_version: 8.1
           memory_limit: 1024M
           version: composer
           testsuite: Integration

.github/workflows/integration-test-cluster-neo4j-5.yml

@@ -13,7 +13,7 @@ jobs:
     runs-on: ubuntu-latest
     env:
       CONNECTION: neo4j://neo4j:testtest@localhost:7687
-    name: "Running on PHP 8.0 with a Neo4j 5.10-enterprise cluster"
+    name: "Running on PHP 8.1 with a Neo4j 5.10-enterprise cluster"
 
     steps:
       - uses: actions/checkout@v2
@@ -25,12 +25,12 @@ jobs:
       - uses: php-actions/composer@v6
         with:
           progress: yes
-          php_version: 8.0
+          php_version: 8.1
           version: 2
       - uses: php-actions/phpunit@v3
         with:
           configuration: phpunit.xml.dist
-          php_version: 8.0
+          php_version: 8.1
           memory_limit: 1024M
           version: composer
           testsuite: Integration

composer.json

@@ -28,7 +28,7 @@
         "psr/http-factory": "^1.0",
         "psr/http-client": "^1.0",
         "php-http/message": "^1.0",
-        "stefanak-michal/bolt": "^7.0",
+        "stefanak-michal/bolt": "^7.0.1",
         "symfony/polyfill-php80": "^1.2",
         "psr/simple-cache": ">=2.0",
         "ext-json": "*",

src/Bolt/BoltConnection.php

@@ -26,6 +26,7 @@
 use Laudis\Neo4j\Contracts\FormatterInterface;
 use Laudis\Neo4j\Databags\BookmarkHolder;
 use Laudis\Neo4j\Databags\DatabaseInfo;
+use Laudis\Neo4j\Databags\Neo4jError;
 use Laudis\Neo4j\Enum\AccessMode;
 use Laudis\Neo4j\Enum\ConnectionProtocol;
 use Laudis\Neo4j\Exception\Neo4jException;
@@ -166,11 +167,10 @@ public function consumeResults(): void
      */
     public function reset(): void
     {
-        $response = $this->protocol()->reset()
+        $response = $this->protocol()
+            ->reset()
             ->getResponse();
-
         $this->assertNoFailure($response);
-
         $this->subscribedResults = [];
     }
 
@@ -182,12 +182,15 @@ public function reset(): void
     public function begin(?string $database, ?float $timeout, BookmarkHolder $holder): void
     {
         $this->consumeResults();
-        $extra = $this->buildRunExtra($database, $timeout, $holder, AccessMode::WRITE());
 
+        if ($this->protocol()->serverState !== ServerState::READY) {
+            throw new Neo4jException([Neo4jError::fromMessageAndCode('Neo.ClientError.Request.Invalid', 'Message \'BEGIN\' cannot be handled by a session which isn\'t in the READY state.')]);
+        }
+
+        $extra = $this->buildRunExtra($database, $timeout, $holder, AccessMode::WRITE());
         $response = $this->protocol()
             ->begin($extra)
             ->getResponse();
-
         $this->assertNoFailure($response);
     }
 
@@ -198,12 +201,14 @@ public function begin(?string $database, ?float $timeout, BookmarkHolder $holder
      */
     public function discard(?int $qid): void
     {
-        $extra = $this->buildResultExtra(null, $qid);
-        $bolt = $this->protocol();
+        if (!in_array($this->protocol()->serverState, [ServerState::STREAMING, ServerState::TX_STREAMING], true)) {
+            throw new Neo4jException([Neo4jError::fromMessageAndCode('Neo.ClientError.Request.Invalid', 'Message \'DISCARD\' cannot be handled by a session which isn\'t in the STREAMING|TX_STREAMING state.')]);
+        }
 
-        $response = $bolt->discard($extra)
+        $extra = $this->buildResultExtra(null, $qid);
+        $response = $this->protocol()
+            ->discard($extra)
             ->getResponse();
-
         $this->assertNoFailure($response);
     }
 
@@ -216,12 +221,15 @@ public function discard(?int $qid): void
      */
     public function run(string $text, array $parameters, ?string $database, ?float $timeout, BookmarkHolder $holder, ?AccessMode $mode): array
     {
+        if (!in_array($this->protocol()->serverState, [ServerState::READY, ServerState::TX_READY, ServerState::TX_STREAMING], true)) {
+            throw new Neo4jException([Neo4jError::fromMessageAndCode('Neo.ClientError.Request.Invalid', 'Message \'RUN\' cannot be handled by a session which isn\'t in the READY|TX_READY|TX_STREAMING state.')]);
+        }
+
         $extra = $this->buildRunExtra($database, $timeout, $holder, $mode);
-        $response = $this->protocol()->run($text, $parameters, $extra)
+        $response = $this->protocol()
+            ->run($text, $parameters, $extra)
             ->getResponse();
-
         $this->assertNoFailure($response);
-
         /** @var BoltMeta */
         return $response->content;
     }
@@ -234,10 +242,14 @@ public function run(string $text, array $parameters, ?string $database, ?float $
     public function commit(): void
     {
         $this->consumeResults();
+
+        if ($this->protocol()->serverState !== ServerState::TX_READY) {
+            throw new Neo4jException([Neo4jError::fromMessageAndCode('Neo.ClientError.Request.Invalid', 'Message \'COMMIT\' cannot be handled by a session which isn\'t in the TX_READY state.')]);
+        }
+
         $response = $this->protocol()
             ->commit()
             ->getResponse();
-
         $this->assertNoFailure($response);
     }
 
@@ -249,10 +261,14 @@ public function commit(): void
     public function rollback(): void
     {
         $this->consumeResults();
+
+        if ($this->protocol()->serverState !== ServerState::TX_READY) {
+            throw new Neo4jException([Neo4jError::fromMessageAndCode('Neo.ClientError.Request.Invalid', 'Message \'ROLLBACK\' cannot be handled by a session which isn\'t in the TX_READY state.')]);
+        }
+
         $response = $this->protocol()
             ->rollback()
             ->getResponse();
-
         $this->assertNoFailure($response);
     }
 
@@ -270,13 +286,16 @@ public function protocol(): V4_4|V5|V5_3|V5_4
      */
     public function pull(?int $qid, ?int $fetchSize): array
     {
+        if (!in_array($this->protocol()->serverState, [ServerState::STREAMING, ServerState::TX_STREAMING], true)) {
+            throw new Neo4jException([Neo4jError::fromMessageAndCode('Neo.ClientError.Request.Invalid', 'Message \'PULL\' cannot be handled by a session which isn\'t in the STREAMING|TX_STREAMING state.')]);
+        }
+
         $extra = $this->buildResultExtra($fetchSize, $qid);
 
         $tbr = [];
         /** @var Response $response */
         foreach ($this->protocol()->pull($extra)->getResponses() as $response) {
             $this->assertNoFailure($response);
-
             $tbr[] = $response->content;
         }
 
@@ -353,6 +372,7 @@ public function getUserAgent(): string
     private function assertNoFailure(Response $response): void
     {
         if ($response->signature === Signature::FAILURE) {
+            $this->protocol()->reset()->getResponse(); //what if the reset fails? what should be expected behaviour?
             throw Neo4jException::fromBoltResponse($response);
         }
     }

tests/Integration/ClientIntegrationTest.php

@@ -87,13 +87,8 @@ public function testValidRun(): void
 
     public function testInvalidRun(): void
     {
-        $exception = false;
-        try {
-            $this->getSession()->transaction(static fn (TransactionInterface $tsx) => $tsx->run('MERGE (x:Tes0342hdm21.())', ['test' => 'a', 'otherTest' => 'b']));
-        } catch (Neo4jException) {
-            $exception = true;
-        }
-        self::assertTrue($exception);
+        $this->expectException(Neo4jException::class);
+        $this->getSession()->transaction(static fn (TransactionInterface $tsx) => $tsx->run('MERGE (x:Tes0342hdm21.())', ['test' => 'a', 'otherTest' => 'b']));
     }
 
     public function testInvalidRunRetry(): void
@@ -106,7 +101,8 @@ public function testInvalidRunRetry(): void
         }
         self::assertTrue($exception);
 
-        $this->getSession()->run('RETURN 1 AS one');
+        $response = $this->getSession()->run('RETURN 1 AS one');
+        $this->assertEquals(1, $response->first()->get('one'));
     }
 
     public function testValidStatement(): void
@@ -142,7 +138,7 @@ public function testStatements(): void
         $response = $this->getSession()->runStatements([
             Statement::create('MERGE (x:TestNode {test: $test})', $params),
             Statement::create('MERGE (x:OtherTestNode {test: $otherTest})', $params),
-            Statement::create('RETURN 1 AS x', []),
+            Statement::create('RETURN 1 AS x'),
         ]);
 
         self::assertEquals(3, $response->count());

tests/Integration/ComplexQueryTest.php

@@ -198,10 +198,14 @@ public function testPeriodicCommit(): void
             self::markTestSkipped('Only local environment has access to local files');
         }
 
+        $this->getSession()->run('MATCH (n:File) DELETE n');
+
         $this->getSession()->run(<<<CYPHER
-USING PERIODIC COMMIT 10
 LOAD CSV FROM 'file:///csv-example.csv' AS line
-MERGE (n:File {name: line[0]});
+CALL {
+    WITH line
+    MERGE (n:File {name: line[0]})
+} IN TRANSACTIONS OF 10 ROWS;
 CYPHER, []);
 
         $result = $this->getSession()->run('MATCH (n:File) RETURN count(n) AS count');
@@ -218,9 +222,11 @@ public function testPeriodicCommitFail(): void
 
         $tsx = $this->getSession(['neo4j', 'bolt'])->beginTransaction([]);
         $tsx->run(<<<CYPHER
-USING PERIODIC COMMIT 10
 LOAD CSV FROM 'file:///csv-example.csv' AS line
-MERGE (n:File {name: line[0]});
+CALL {
+    WITH line
+    MERGE (n:File {name: line[0]})
+} IN  TRANSACTIONS OF 10 ROWS;
 CYPHER
         );
         $tsx->commit();

tests/Integration/OGMFormatterIntegrationTest.php

@@ -365,7 +365,7 @@ public function testPath(): void
     public function testPath2(): void
     {
         $results = $this->getSession()->transaction(static fn (TransactionInterface $tsx) => $tsx->run(<<<'CYPHER'
-CREATE path = ((a:Node {x:$x}) - [b:HasNode {attribute: $xy}] -> (c:Node {y:$y}) - [d:HasNode {attribute: $yz}] -> (e:Node {z:$z}))
+CREATE path = (a:Node {x:$x}) - [b:HasNode {attribute: $xy}] -> (c:Node {y:$y}) - [d:HasNode {attribute: $yz}] -> (e:Node {z:$z})
 RETURN path
 CYPHER, ['x' => 'x', 'xy' => 'xy', 'y' => 'y', 'yz' => 'yz', 'z' => 'z']));
 

tests/Integration/TransactionIntegrationTest.php

@@ -13,13 +13,9 @@
 
 namespace Laudis\Neo4j\Tests\Integration;
 
-use Laudis\Neo4j\Bolt\BoltDriver;
-use Laudis\Neo4j\Bolt\Connection;
-use Laudis\Neo4j\Bolt\ConnectionPool;
 use Laudis\Neo4j\Databags\Statement;
 use Laudis\Neo4j\Exception\Neo4jException;
-use ReflectionClass;
-use Throwable;
+use PHPUnit\Framework\Attributes\DoesNotPerformAssertions;
 
 final class TransactionIntegrationTest extends EnvironmentAwareIntegrationTest
 {
@@ -224,17 +220,8 @@ public function testCommitInvalid(): void
         self::assertFalse($tsx->isRolledBack());
         self::assertTrue($tsx->isCommitted());
 
-        $exception = false;
-        try {
-            $tsx->commit();
-        } catch (Throwable) {
-            $exception = true;
-        }
-        self::assertTrue($exception);
-
-        self::assertTrue($tsx->isFinished());
-        self::assertFalse($tsx->isRolledBack());
-        self::assertTrue($tsx->isCommitted());
+        $this->expectException(Neo4jException::class);
+        $tsx->commit();
     }
 
     public function testRollbackValid(): void
@@ -256,17 +243,8 @@ public function testRollbackInvalid(): void
         self::assertTrue($tsx->isRolledBack());
         self::assertFalse($tsx->isCommitted());
 
-        $exception = false;
-        try {
-            $tsx->rollback();
-        } catch (Throwable) {
-            $exception = true;
-        }
-        self::assertTrue($exception);
-
-        self::assertTrue($tsx->isFinished());
-        self::assertTrue($tsx->isRolledBack());
-        self::assertFalse($tsx->isCommitted());
+        $this->expectException(Neo4jException::class);
+        $tsx->rollback();
     }
 
 //    /**
@@ -304,9 +282,7 @@ public function testRollbackInvalid(): void
 //        self::assertCount(3, $cache[$key]);
 //    }
 
-    /**
-     * @doesNotPerformAssertions
-     */
+    #[DoesNotPerformAssertions]
     public function testTransactionRunNoConsumeResult(): void
     {
         $tsx = $this->getSession()->beginTransaction([]);
@@ -315,9 +291,7 @@ public function testTransactionRunNoConsumeResult(): void
         $tsx->commit();
     }
 
-    /**
-     * @doesNotPerformAssertions
-     */
+    #[DoesNotPerformAssertions]
     public function testTransactionRunNoConsumeButSaveResult(): void
     {
         $tsx = $this->getSession()->beginTransaction([]);