Skip to content

Commit 5fed5eb

Browse files
p123-stackp123-stack
committed
fixed all the tx_run and did a proper cleanup
1 parent 80d8e2c commit 5fed5eb

File tree

15 files changed

+174
-128
lines changed

15 files changed

+174
-128
lines changed

‎Dockerfile‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,6 @@ RUN apt-get update \
88
git \
99
wget \
1010
&& docker-php-ext-install -j$(nproc) bcmath sockets \
11-
&& wget https://codeclimate.com/downloads/test-reporter/test-reporter-latest-linux-amd64 \
12-
&& mv test-reporter-latest-linux-amd64 /usr/bin/cc-test-reporter \
13-
&& chmod +x /usr/bin/cc-test-reporter \
1411
&& pecl install xdebug \
1512
&& docker-php-ext-enable xdebug && \
1613
curl -sS https://getcomposer.org/installer | php -- --install-dir=/usr/local/bin --filename=composer

‎src/Authentication/BasicAuth.php‎

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313

1414
namespace Laudis\Neo4j\Authentication;
1515

16+
use Bolt\enum\Signature;
17+
use Bolt\protocol\Response;
1618
use Bolt\protocol\V4_4;
1719
use Bolt\protocol\V5;
1820
use Bolt\protocol\V5_1;
@@ -22,8 +24,8 @@
2224
use Exception;
2325
use Laudis\Neo4j\Bolt\BoltMessageFactory;
2426
use Laudis\Neo4j\Common\Neo4jLogger;
25-
use Laudis\Neo4j\Common\ResponseHelper;
2627
use Laudis\Neo4j\Contracts\AuthenticateInterface;
28+
use Laudis\Neo4j\Exception\Neo4jException;
2729
use Psr\Http\Message\UriInterface;
2830

2931
/**
@@ -51,7 +53,7 @@ public function authenticateBolt(V4_4|V5|V5_1|V5_2|V5_3|V5_4 $protocol, string $
5153
$helloMetadata = ['user_agent' => $userAgent];
5254

5355
$factory->createHelloMessage($helloMetadata)->send();
54-
$response = $protocol->getResponse();
56+
$response = self::getResponse($protocol);
5557

5658
$credentials = [
5759
'scheme' => 'basic',
@@ -60,7 +62,7 @@ public function authenticateBolt(V4_4|V5|V5_1|V5_2|V5_3|V5_4 $protocol, string $
6062
];
6163

6264
$factory->createLogonMessage($credentials)->send();
63-
$protocol->getResponse();
65+
self::getResponse($protocol);
6466

6567
/** @var array{server: string, connection_id: string, hints: list} */
6668
return $response->content;
@@ -74,9 +76,10 @@ public function authenticateBolt(V4_4|V5|V5_1|V5_2|V5_3|V5_4 $protocol, string $
7476
];
7577

7678
$factory->createHelloMessage($helloMetadata)->send();
79+
$response = self::getResponse($protocol);
7780

7881
/** @var array{server: string, connection_id: string, hints: list} */
79-
return $protocol->getResponse()->content;
82+
return $response->content;
8083
}
8184

8285
/**
@@ -86,7 +89,17 @@ public function logoff(V4_4|V5|V5_1|V5_2|V5_3|V5_4 $protocol): void
8689
{
8790
$factory = $this->createMessageFactory($protocol);
8891
$factory->createLogoffMessage()->send();
89-
$protocol->getResponse();
92+
$protocol->getResponse();
93+
}
94+
95+
public static function getResponse(V4_4|V5|V5_1|V5_2|V5_3|V5_4 $protocol): Response
96+
{
97+
$response = $protocol->getResponse();
98+
if ($response->signature === Signature::FAILURE) {
99+
throw Neo4jException::fromBoltResponse($response);
100+
}
101+
102+
return $response;
90103
}
91104

92105
public function toString(UriInterface $uri): string

‎src/Authentication/KerberosAuth.php‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313

1414
namespace Laudis\Neo4j\Authentication;
1515

16+
use Bolt\enum\Signature;
17+
use Bolt\protocol\Response;
1618
use Bolt\protocol\V4_4;
1719
use Bolt\protocol\V5;
1820
use Bolt\protocol\V5_1;
@@ -22,8 +24,8 @@
2224
use Exception;
2325
use Laudis\Neo4j\Bolt\BoltMessageFactory;
2426
use Laudis\Neo4j\Common\Neo4jLogger;
25-
use Laudis\Neo4j\Common\ResponseHelper;
2627
use Laudis\Neo4j\Contracts\AuthenticateInterface;
28+
use Laudis\Neo4j\Exception\Neo4jException;
2729
use Psr\Http\Message\RequestInterface;
2830
use Psr\Http\Message\UriInterface;
2931
use Psr\Log\LogLevel;
@@ -62,7 +64,7 @@ public function authenticateBolt(V4_4|V5|V5_1|V5_2|V5_3|V5_4 $protocol, string $
6264

6365
$factory->createHelloMessage(['user_agent' => $userAgent])->send();
6466

65-
$response = $protocol->getResponse();
67+
$response = self::getResponse($protocol);
6668

6769
$this->logger?->log(LogLevel::DEBUG, 'LOGON', ['scheme' => 'kerberos', 'principal' => '']);
6870

@@ -72,14 +74,24 @@ public function authenticateBolt(V4_4|V5|V5_1|V5_2|V5_3|V5_4 $protocol, string $
7274
'credentials' => $this->token,
7375
])->send();
7476

75-
$protocol->getResponse();
77+
self::getResponse($protocol);
7678

7779
/**
7880
* @var array{server: string, connection_id: string, hints: list}
7981
*/
8082
return $response->content;
8183
}
8284

85+
public static function getResponse(V4_4|V5|V5_1|V5_2|V5_3|V5_4 $protocol): Response
86+
{
87+
$response = $protocol->getResponse();
88+
if ($response->signature === Signature::FAILURE) {
89+
throw Neo4jException::fromBoltResponse($response);
90+
}
91+
92+
return $response;
93+
}
94+
8395
public function toString(UriInterface $uri): string
8496
{
8597
return sprintf('Kerberos %s@%s:%s', $this->token, $uri->getHost(), $uri->getPort() ?? '');

‎src/Authentication/NoAuth.php‎

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313

1414
namespace Laudis\Neo4j\Authentication;
1515

16+
use Bolt\enum\Signature;
17+
use Bolt\protocol\Response;
1618
use Bolt\protocol\V4_4;
1719
use Bolt\protocol\V5;
1820
use Bolt\protocol\V5_1;
@@ -22,8 +24,8 @@
2224
use Exception;
2325
use Laudis\Neo4j\Bolt\BoltMessageFactory;
2426
use Laudis\Neo4j\Common\Neo4jLogger;
25-
use Laudis\Neo4j\Common\ResponseHelper;
2627
use Laudis\Neo4j\Contracts\AuthenticateInterface;
28+
use Laudis\Neo4j\Exception\Neo4jException;
2729
use Psr\Http\Message\RequestInterface;
2830
use Psr\Http\Message\UriInterface;
2931
use Psr\Log\LogLevel;
@@ -57,10 +59,10 @@ public function authenticateBolt(V4_4|V5|V5_1|V5_2|V5_3|V5_4 $protocol, string $
5759
$helloMetadata = ['user_agent' => $userAgent];
5860

5961
$factory->createHelloMessage($helloMetadata)->send();
60-
$response = $protocol->getResponse();
62+
$response = self::getResponse($protocol);
6163

6264
$factory->createLogonMessage(['scheme' => 'none'])->send();
63-
$protocol->getResponse();
65+
self::getResponse($protocol);
6466

6567
/** @var array{server: string, connection_id: string, hints: list} */
6668
return $response->content;
@@ -74,14 +76,24 @@ public function authenticateBolt(V4_4|V5|V5_1|V5_2|V5_3|V5_4 $protocol, string $
7476
$factory->createHelloMessage($helloMetadata)->send();
7577

7678
/** @var array{server: string, connection_id: string, hints: list} */
77-
return $protocol->getResponse()->content;
79+
return self::getResponse($protocol)->content;
80+
}
81+
82+
public static function getResponse(V4_4|V5|V5_1|V5_2|V5_3|V5_4 $protocol): Response
83+
{
84+
$response = $protocol->getResponse();
85+
if ($response->signature === Signature::FAILURE) {
86+
throw Neo4jException::fromBoltResponse($response);
87+
}
88+
89+
return $response;
7890
}
7991

8092
public function logoff(V4_4|V5|V5_1|V5_2|V5_3|V5_4 $protocol): void
8193
{
8294
$factory = $this->createMessageFactory($protocol);
8395
$factory->createLogoffMessage()->send();
84-
$protocol->getResponse();
96+
$protocol->getResponse();
8597
}
8698

8799
public function toString(UriInterface $uri): string

‎src/Authentication/OpenIDConnectAuth.php‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
use Exception;
2323
use Laudis\Neo4j\Bolt\BoltMessageFactory;
2424
use Laudis\Neo4j\Common\Neo4jLogger;
25-
use Laudis\Neo4j\Common\ResponseHelper;
2625
use Laudis\Neo4j\Contracts\AuthenticateInterface;
2726
use Psr\Http\Message\RequestInterface;
2827
use Psr\Http\Message\UriInterface;
@@ -68,7 +67,7 @@ public function authenticateBolt(V4_4|V5|V5_1|V5_2|V5_3|V5_4 $protocol, string $
6867
'credentials' => $this->token,
6968
])->send();
7069

71-
$protocol->getResponse();
70+
$protocol->getResponse();
7271

7372
/**
7473
* @var array{server: string, connection_id: string, hints: list}

‎src/Bolt/BoltUnmanagedTransaction.php‎

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
use Laudis\Neo4j\Databags\SummarizedResult;
2222
use Laudis\Neo4j\Databags\TransactionConfiguration;
2323
use Laudis\Neo4j\Enum\TransactionState;
24-
use Laudis\Neo4j\Exception\ClientException;
24+
use Laudis\Neo4j\Exception\TransactionException;
2525
use Laudis\Neo4j\Formatter\SummarizedResultFormatter;
2626
use Laudis\Neo4j\ParameterHelper;
2727
use Laudis\Neo4j\Types\CypherList;
@@ -58,23 +58,23 @@ public function __construct(
5858
/**
5959
* @param iterable<Statement> $statements
6060
*
61-
* @throws ClientException|Throwable
61+
* @throws TransactionException|Throwable
6262
*
6363
* @return CypherList<SummarizedResult>
6464
*/
6565
public function commit(iterable $statements = []): CypherList
6666
{
6767
if ($this->isFinished()) {
6868
if ($this->state === TransactionState::TERMINATED) {
69-
throw new ClientException("Can't commit, transaction has been terminated");
69+
throw new TransactionException("Can't commit, transaction has been terminated");
7070
}
7171

7272
if ($this->state === TransactionState::COMMITTED) {
73-
throw new ClientException("Can't commit, transaction has already been committed");
73+
throw new TransactionException("Can't commit, transaction has already been committed");
7474
}
7575

7676
if ($this->state === TransactionState::ROLLED_BACK) {
77-
throw new ClientException("Can't commit, transaction has already been rolled back");
77+
throw new TransactionException("Can't commit, transaction has already been rolled back");
7878
}
7979
}
8080

@@ -93,16 +93,20 @@ public function commit(iterable $statements = []): CypherList
9393
public function rollback(): void
9494
{
9595
if ($this->isFinished()) {
96-
if ($this->state === TransactionState::TERMINATED) {
97-
throw new ClientException("Can't rollback, transaction has been terminated");
98-
}
99-
10096
if ($this->state === TransactionState::COMMITTED) {
101-
throw new ClientException("Can't rollback, transaction has already been committed");
97+
throw new TransactionException("Can't rollback, transaction has already been committed");
10298
}
10399

104100
if ($this->state === TransactionState::ROLLED_BACK) {
105-
throw new ClientException("Can't rollback, transaction has already been rolled back");
101+
// Already rolled back, throw a TransactionException to be wrapped by DriverErrorResponse
102+
throw new TransactionException('Transaction has already been rolled back');
103+
}
104+
105+
if ($this->state === TransactionState::TERMINATED) {
106+
// Transaction failed, allow rollback as a no-op
107+
$this->state = TransactionState::ROLLED_BACK;
108+
109+
return;
106110
}
107111
}
108112

@@ -115,6 +119,20 @@ public function rollback(): void
115119
*/
116120
public function run(string $statement, iterable $parameters = []): SummarizedResult
117121
{
122+
if ($this->isFinished()) {
123+
if ($this->state === TransactionState::TERMINATED) {
124+
throw new TransactionException("Can't rollback, transaction has been terminated");
125+
}
126+
127+
if ($this->state === TransactionState::COMMITTED) {
128+
throw new TransactionException("Can't rollback, transaction has already been committed");
129+
}
130+
131+
if ($this->state === TransactionState::ROLLED_BACK) {
132+
throw new TransactionException("Can't rollback, transaction has already been rolled back");
133+
}
134+
}
135+
118136
return $this->runStatement(new Statement($statement, $parameters));
119137
}
120138

‎src/Exception/TransactionException.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
*
2424
* @psalm-suppress MutableDependency
2525
*/
26-
final class ClientException extends RuntimeException
26+
final class TransactionException extends RuntimeException
2727
{
2828
public function __construct(string $message, ?Throwable $previous = null)
2929
{

‎src/Formatter/SummarizedResultFormatter.php‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -198,8 +198,7 @@ function (mixed $response) use ($connection, $statement, $runStart, $resultAvail
198198
/**
199199
* @var SummarizedResult<CypherMap<OGMTypes>>
200200
*/
201-
return new SummarizedResult($summary, (new CypherList($formattedResult))->withCacheLimit($result->getFetchSize()),
202-
$meta['fields'] ?? [] );
201+
return new SummarizedResult($summary, (new CypherList($formattedResult))->withCacheLimit($result->getFetchSize()));
203202
}
204203

205204
public function formatArgs(array $profiledPlanData): PlanArguments

‎testkit-backend/src/Backend.php‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,17 @@
2323
use const JSON_THROW_ON_ERROR;
2424

2525
use JsonException;
26+
use Laudis\Neo4j\Exception\Neo4jException;
2627
use Laudis\Neo4j\TestkitBackend\Contracts\RequestHandlerInterface;
2728
use Laudis\Neo4j\TestkitBackend\Contracts\TestkitResponseInterface;
2829
use Laudis\Neo4j\TestkitBackend\Responses\BackendErrorResponse;
30+
use Laudis\Neo4j\TestkitBackend\Responses\DriverErrorResponse;
2931

3032
use const PHP_EOL;
3133

3234
use Psr\Container\ContainerInterface;
3335
use Psr\Log\LoggerInterface;
36+
use Symfony\Component\Uid\Uuid;
3437
use Throwable;
3538
use UnexpectedValueException;
3639

@@ -88,9 +91,13 @@ public function handle(): void
8891
[$handler, $request] = $this->extractRequest($message);
8992
try {
9093
$this->properSendoff($handler->handle($request));
94+
} catch (Neo4jException $e) {
95+
$this->logger->error($e->__toString());
96+
// Convert Neo4jException to DriverErrorResponse for testkit protocol
97+
$this->properSendoff(new DriverErrorResponse(Uuid::v4(), $e));
9198
} catch (Throwable $e) {
9299
$this->logger->error($e->__toString());
93-
100+
// All other exceptions become BackendErrorResponse
94101
$this->properSendoff(new BackendErrorResponse($e->getMessage()));
95102
}
96103
}

‎testkit-backend/src/Handlers/AbstractRunner.php‎

Lines changed: 10 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313

1414
namespace Laudis\Neo4j\TestkitBackend\Handlers;
1515

16-
use Exception;
1716
use Laudis\Neo4j\Contracts\SessionInterface;
1817
use Laudis\Neo4j\Contracts\TransactionInterface;
1918
use Laudis\Neo4j\Databags\SummarizedResult;
@@ -23,7 +22,6 @@
2322
use Laudis\Neo4j\TestkitBackend\Contracts\RequestHandlerInterface;
2423
use Laudis\Neo4j\TestkitBackend\MainRepository;
2524
use Laudis\Neo4j\TestkitBackend\Requests\SessionRunRequest;
26-
use Laudis\Neo4j\TestkitBackend\Requests\TransactionRunRequest;
2725
use Laudis\Neo4j\TestkitBackend\Responses\DriverErrorResponse;
2826
use Laudis\Neo4j\TestkitBackend\Responses\ResultResponse;
2927
use Laudis\Neo4j\Types\AbstractCypherObject;
@@ -80,24 +78,18 @@ public function handle($request): ResultResponse|DriverErrorResponse
8078

8179
$this->repository->addRecords($id, $result);
8280

83-
return new ResultResponse($id, $result->keys());
84-
} catch (Neo4jException $exception) {
85-
if ($request instanceof SessionRunRequest) {
86-
return new DriverErrorResponse($request->getSessionId(), $exception);
87-
}
88-
if ($request instanceof TransactionRunRequest) {
89-
return new DriverErrorResponse($request->getTxId(), $exception);
90-
}
81+
return new ResultResponse($id, $result->isEmpty() ? [] : $result->first()->keys());
82+
} catch (Neo4jException|TransactionException $exception) {
83+
$this->logger->debug($exception->__toString());
9184

92-
throw new Exception('Unhandled neo4j exception for run request of type: '.get_class($request));
93-
} catch (TransactionException $exception) {
94-
if ($request instanceof TransactionRunRequest) {
95-
return new DriverErrorResponse($request->getTxId(), $exception);
96-
}
85+
$driverErrorResponse = new DriverErrorResponse(
86+
$this->getId($request),
87+
$exception
88+
);
89+
$this->repository->addRecords($id, $driverErrorResponse);
9790

98-
throw new Exception('Unhandled neo4j exception for run request of type: '.get_class($request));
99-
}
100-
// NOTE: all other exceptions will be caught in the Backend
91+
return $driverErrorResponse;
92+
} // NOTE: all other exceptions will be caught in the Backend
10193
}
10294

10395
/**

0 commit comments

Comments
 (0)