Skip to content

Commit

Permalink
Remove the OCI8Connection::getExecuteMode() method
Browse files Browse the repository at this point in the history
The existing relationship between the connection and its statement violate the ISP principle. Instead of having access only to the execution mode of the connection, statements have access to the entire connection API.

Having a method which is not defined in the driver connection interface makes it impossible to mark the method `final` (doctrine#3590).
  • Loading branch information
morozov committed Jun 25, 2020
1 parent 92ba48c commit 2da86a3
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 26 deletions.
31 changes: 31 additions & 0 deletions src/Driver/OCI8/ExecutionMode.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?php

declare(strict_types=1);

namespace Doctrine\DBAL\Driver\OCI8;

/**
* Encapsulates the execution mode that is shared between the connection and its statements.
*
* @internal This class is not covered by the backward compatibility promise
*/
final class ExecutionMode
{
/** @var bool */
private $isAutoCommitEnabled = true;

public function enableAutoCommit(): void
{
$this->isAutoCommitEnabled = true;
}

public function disableAutoCommit(): void
{
$this->isAutoCommitEnabled = false;
}

public function isAutoCommitEnabled(): bool
{
return $this->isAutoCommitEnabled;
}
}
26 changes: 8 additions & 18 deletions src/Driver/OCI8/OCI8Connection.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
use function sprintf;
use function str_replace;

use const OCI_COMMIT_ON_SUCCESS;
use const OCI_NO_AUTO_COMMIT;

/**
Expand All @@ -36,8 +35,8 @@ class OCI8Connection implements ConnectionInterface, ServerInfoAwareConnection
/** @var resource */
protected $dbh;

/** @var int */
protected $executeMode = OCI_COMMIT_ON_SUCCESS;
/** @var ExecutionMode */
private $executionMode;

/**
* Creates a Connection to an Oracle Database using oci8 extension.
Expand Down Expand Up @@ -67,7 +66,8 @@ public function __construct(
throw OCI8Exception::fromErrorInfo(oci_error());
}

$this->dbh = $dbh;
$this->dbh = $dbh;
$this->executionMode = new ExecutionMode();
}

/**
Expand Down Expand Up @@ -107,7 +107,7 @@ public function requiresQueryForServerVersion()

public function prepare(string $sql): DriverStatement
{
return new Statement($this->dbh, $sql, $this);
return new Statement($this->dbh, $sql, $this->executionMode);
}

public function query(string $sql): ResultInterface
Expand Down Expand Up @@ -154,22 +154,12 @@ public function lastInsertId($name = null)
return (int) $result;
}

/**
* Returns the current execution mode.
*
* @return int
*/
public function getExecuteMode()
{
return $this->executeMode;
}

/**
* {@inheritdoc}
*/
public function beginTransaction()
{
$this->executeMode = OCI_NO_AUTO_COMMIT;
$this->executionMode->disableAutoCommit();

return true;
}
Expand All @@ -183,7 +173,7 @@ public function commit()
throw OCI8Exception::fromErrorInfo(oci_error($this->dbh));
}

$this->executeMode = OCI_COMMIT_ON_SUCCESS;
$this->executionMode->enableAutoCommit();

return true;
}
Expand All @@ -197,7 +187,7 @@ public function rollBack()
throw OCI8Exception::fromErrorInfo(oci_error($this->dbh));
}

$this->executeMode = OCI_COMMIT_ON_SUCCESS;
$this->executionMode->enableAutoCommit();

return true;
}
Expand Down
24 changes: 16 additions & 8 deletions src/Driver/OCI8/OCI8Statement.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@

use const OCI_B_BIN;
use const OCI_B_BLOB;
use const OCI_COMMIT_ON_SUCCESS;
use const OCI_D_LOB;
use const OCI_NO_AUTO_COMMIT;
use const OCI_TEMP_BLOB;
use const PREG_OFFSET_CAPTURE;
use const SQLT_CHR;
Expand All @@ -42,8 +44,8 @@ class OCI8Statement implements StatementInterface
/** @var resource */
protected $_sth;

/** @var OCI8Connection */
protected $_conn;
/** @var ExecutionMode */
private $executionMode;

/** @var string[] */
protected $_paramMap = [];
Expand All @@ -63,17 +65,17 @@ class OCI8Statement implements StatementInterface
* @param resource $dbh The connection handle.
* @param string $query The SQL query.
*/
public function __construct($dbh, $query, OCI8Connection $conn)
public function __construct($dbh, $query, ExecutionMode $executionMode)
{
[$query, $paramMap] = self::convertPositionalToNamedPlaceholders($query);

$stmt = oci_parse($dbh, $query);
assert(is_resource($stmt));

$this->_sth = $stmt;
$this->_dbh = $dbh;
$this->_paramMap = $paramMap;
$this->_conn = $conn;
$this->_sth = $stmt;
$this->_dbh = $dbh;
$this->_paramMap = $paramMap;
$this->executionMode = $executionMode;
}

/**
Expand Down Expand Up @@ -299,7 +301,13 @@ public function execute($params = null): ResultInterface
}
}

$ret = @oci_execute($this->_sth, $this->_conn->getExecuteMode());
if ($this->executionMode->isAutoCommitEnabled()) {
$mode = OCI_COMMIT_ON_SUCCESS;
} else {
$mode = OCI_NO_AUTO_COMMIT;
}

$ret = @oci_execute($this->_sth, $mode);
if (! $ret) {
throw OCI8Exception::fromErrorInfo(oci_error($this->_sth));
}
Expand Down
33 changes: 33 additions & 0 deletions tests/Driver/OCI8/ExecutionModeTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<?php

declare(strict_types=1);

namespace Doctrine\DBAL\Tests\Driver\OCI8;

use Doctrine\DBAL\Driver\OCI8\ExecutionMode;
use PHPStan\Testing\TestCase;

final class ExecutionModeTest extends TestCase
{
/** @var ExecutionMode */
private $mode;

protected function setUp(): void
{
$this->mode = new ExecutionMode();
}

public function testDefaultAutoCommitStatus(): void
{
self::assertTrue($this->mode->isAutoCommitEnabled());
}

public function testChangeAutoCommitStatus(): void
{
$this->mode->disableAutoCommit();
self::assertFalse($this->mode->isAutoCommitEnabled());

$this->mode->enableAutoCommit();
self::assertTrue($this->mode->isAutoCommitEnabled());
}
}

0 comments on commit 2da86a3

Please sign in to comment.