Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issues with Oracle server #901

Merged
merged 11 commits into from
Oct 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/test-unit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ jobs:
if [ "${{ matrix.type }}" != "StaticAnalysis" ]; then composer remove --no-interaction --no-update phpstan/phpstan --dev; fi
composer update --ansi --prefer-dist --no-interaction --no-progress --optimize-autoloader
if [ "${{ matrix.type }}" == "Phpunit Lowest" ]; then composer update --ansi --prefer-dist --prefer-lowest --prefer-stable --no-interaction --no-progress --optimize-autoloader; fi
if [ "${{ matrix.type }}" == "Phpunit Burn" ]; then sed -i 's~ *public function runBare(): void~public function runBare(): void { gc_collect_cycles(); gc_collect_cycles(); $memDiffs = array_fill(0, '"$(if [ \"$GITHUB_EVENT_NAME\" == \"schedule\" ]; then echo 64; else echo 16; fi)"', 0); for ($i = -1; $i < count($memDiffs); ++$i) { $this->_runBare(); gc_collect_cycles(); gc_collect_cycles(); $mem = memory_get_usage(); if ($i !== -1) { $memDiffs[$i] = $mem - $memPrev; } $memPrev = $mem; rsort($memDiffs); if (array_sum($memDiffs) >= 4096 * 1024 || $memDiffs[2] > 0) { $this->onNotSuccessfulTest(new AssertionFailedError( "Memory leak detected! (" . implode(" + ", array_map(fn ($v) => number_format($v / 1024, 3, ".", " "), array_filter($memDiffs))) . " KB, " . ($i + 2) . " iterations)" )); } } } private function _runBare(): void~' vendor/phpunit/phpunit/src/Framework/TestCase.php && cat vendor/phpunit/phpunit/src/Framework/TestCase.php | grep '_runBare('; fi
if [ "${{ matrix.type }}" == "Phpunit Burn" ]; then sed -i 's~ *public function runBare(): void~public function runBare(): void { gc_collect_cycles(); gc_collect_cycles(); $memDiffs = array_fill(0, '"$(if [ \"$GITHUB_EVENT_NAME\" == \"schedule\" ]; then echo 64; else echo 4; fi)"', 0); for ($i = -1; $i < count($memDiffs); ++$i) { $this->_runBare(); gc_collect_cycles(); gc_collect_cycles(); $mem = memory_get_usage(); if ($i !== -1) { $memDiffs[$i] = $mem - $memPrev; } $memPrev = $mem; rsort($memDiffs); if (array_sum($memDiffs) >= 4096 * 1024 || $memDiffs[2] > 0) { $this->onNotSuccessfulTest(new AssertionFailedError( "Memory leak detected! (" . implode(" + ", array_map(fn ($v) => number_format($v / 1024, 3, ".", " "), array_filter($memDiffs))) . " KB, " . ($i + 2) . " iterations)" )); } } } private function _runBare(): void~' vendor/phpunit/phpunit/src/Framework/TestCase.php && cat vendor/phpunit/phpunit/src/Framework/TestCase.php | grep '_runBare('; fi

- name: Init
run: |
Expand Down
6 changes: 3 additions & 3 deletions phpunit.xml.dist
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
<phpunit bootstrap="vendor/autoload.php" printerClass="Atk4\Core\Phpunit\ResultPrinter" colors="true">
<php>
<var name="DB_DSN" value="sqlite::memory:" />
<var name="DB_USER" value="" />
<var name="DB_PASSWD" value="" />
<env name="DB_DSN" value="sqlite::memory:" />
<env name="DB_USER" value="" />
<env name="DB_PASSWD" value="" />
</php>
<testsuites>
<testsuite name="tests">
Expand Down
2 changes: 1 addition & 1 deletion src/Field.php
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,7 @@ public function isHidden(): bool
*/
public function getCaption(): string
{
return $this->caption ?? $this->ui['caption'] ?? $this->readableCaption(preg_replace('~^atk_fp_\w+?__~', '', $this->short_name));
return $this->caption ?? $this->ui['caption'] ?? $this->readableCaption($this->short_name);
}

// }}}
Expand Down
2 changes: 1 addition & 1 deletion src/Model/ReferencesTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public function addRef(string $link, array $defaults): Reference
/**
* Add hasOne reference.
*
* @return Reference\HasOne
* @return Reference\HasOne|Reference\HasOneSql
*/
public function hasOne(string $link, array $defaults = []) //: Reference
{
Expand Down
6 changes: 3 additions & 3 deletions src/Persistence/Sql.php
Original file line number Diff line number Diff line change
Expand Up @@ -740,7 +740,7 @@ private function getIdSequenceName(Model $model): ?string
// PostgreSQL uses sequence internally for PK autoincrement,
// use default name if not set explicitly
if ($this->connection instanceof \Atk4\Data\Persistence\Sql\Postgresql\Connection) {
$sequenceName = $model->table . '_' . $model->id_field . '_seq';
$sequenceName = $model->table . '_' . $model->getField($model->id_field)->getPersistenceName() . '_seq';
}
}

Expand All @@ -757,7 +757,7 @@ public function lastInsertId(Model $model): string
}

$query = $this->connection->dsql()->table($model->table);
$query->field($query->expr('max({id_col})', ['id_col' => $model->id_field]), 'max_id');
$query->field($query->expr('max({id_col})', ['id_col' => $model->getField($model->id_field)->getPersistenceName()]), 'max_id');

return $query->getOne();
}
Expand All @@ -771,7 +771,7 @@ protected function syncIdSequence(Model $model): void
if ($this->connection instanceof \Atk4\Data\Persistence\Sql\Postgresql\Connection) {
$this->connection->expr(
'select setval([], coalesce(max({}), 0) + 1, false) from {}',
[$this->getIdSequenceName($model), $model->id_field, $model->table]
[$this->getIdSequenceName($model), $model->getField($model->id_field)->getPersistenceName(), $model->table]
)->execute();
}
}
Expand Down
20 changes: 18 additions & 2 deletions src/Persistence/Sql/Connection.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,11 @@
namespace Atk4\Data\Persistence\Sql;

use Atk4\Core\DiContainerTrait;
use Doctrine\Common\EventManager;
use Doctrine\DBAL\Connection as DbalConnection;
use Doctrine\DBAL\DriverManager;
use Doctrine\DBAL\Event\ConnectionEventArgs;
use Doctrine\DBAL\Events;
use Doctrine\DBAL\Platforms\AbstractPlatform;
use Doctrine\DBAL\Platforms\OraclePlatform;
use Doctrine\DBAL\Platforms\PostgreSQL94Platform;
Expand Down Expand Up @@ -194,6 +197,11 @@ final public static function isComposerDbal2x(): bool
return !class_exists(DbalResult::class);
}

protected static function createDbalEventManager(): EventManager
{
return new EventManager();
}

/**
* Establishes connection based on a $dsn.
*
Expand All @@ -215,7 +223,7 @@ protected static function connectDbalConnection(array $dsn)
if (self::isComposerDbal2x()) {
$dbalConnection = DriverManager::getConnection([
'pdo' => $pdo,
]);
], null, (static::class)::createDbalEventManager());
} else {
$pdoConnection = (new \ReflectionClass(\Doctrine\DBAL\Driver\PDO\Connection::class))
->newInstanceWithoutConstructor();
Expand All @@ -225,12 +233,20 @@ protected static function connectDbalConnection(array $dsn)
}, null, \Doctrine\DBAL\Driver\PDO\Connection::class)();
$dbalConnection = DriverManager::getConnection([
'driver' => 'pdo_' . $pdo->getAttribute(\PDO::ATTR_DRIVER_NAME),
]);
], null, (static::class)::createDbalEventManager());
\Closure::bind(function () use ($dbalConnection, $pdoConnection): void {
$dbalConnection->_conn = $pdoConnection;
}, null, \Doctrine\DBAL\Connection::class)();
}

// postConnect event is not dispatched when PDO is passed, dispatch it manually
if ($dbalConnection->getEventManager()->hasListeners(Events::postConnect)) {
$dbalConnection->getEventManager()->dispatchEvent(
Events::postConnect,
new ConnectionEventArgs($dbalConnection)
);
}

// DBAL 3.x removed some old platforms, to support instanceof reliably,
// make sure that DBAL 2.x platform is always supported in DBAL 3.x, see:
// https://github.com/doctrine/dbal/pull/3912
Expand Down
27 changes: 18 additions & 9 deletions src/Persistence/Sql/Oracle/Connection.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,30 @@
namespace Atk4\Data\Persistence\Sql\Oracle;

use Atk4\Data\Persistence\Sql\Connection as BaseConnection;
use Doctrine\Common\EventManager;
use Doctrine\DBAL\Event\Listeners\OracleSessionInit;

class Connection extends BaseConnection
{
protected $query_class = Query::class;

public function __construct($properties = [])
protected static function createDbalEventManager(): EventManager
{
parent::__construct($properties);

// date and datetime format should be like this for Agile Data to correctly pick it up and typecast
$this->expr('ALTER SESSION SET NLS_TIMESTAMP_FORMAT={datetime_format} NLS_DATE_FORMAT={date_format} NLS_NUMERIC_CHARACTERS={dec_char}', [
'datetime_format' => 'YYYY-MM-DD HH24:MI:SS', // datetime format
'date_format' => 'YYYY-MM-DD', // date format
'dec_char' => '. ', // decimal separator, no thousands separator
])->execute();
$evm = new EventManager();

// setup connection globalization to use standard datetime format incl. microseconds support
$dateFormat = 'YYYY-MM-DD';
$timeFormat = 'HH24:MI:SS.FF6';
$tzFormat = 'TZH:TZM';
$evm->addEventSubscriber(new OracleSessionInit([
'NLS_DATE_FORMAT' => $dateFormat,
'NLS_TIME_FORMAT' => $timeFormat,
'NLS_TIMESTAMP_FORMAT' => $dateFormat . ' ' . $timeFormat,
'NLS_TIME_TZ_FORMAT' => $timeFormat . ' ' . $tzFormat,
'NLS_TIMESTAMP_TZ_FORMAT' => $dateFormat . ' ' . $timeFormat . ' ' . $tzFormat,
]));

return $evm;
}

// {{{ fix for too many connections for CI testing
Expand Down
19 changes: 0 additions & 19 deletions src/Persistence/Sql/Oracle/Version12/Connection.php

This file was deleted.

28 changes: 0 additions & 28 deletions src/Persistence/Sql/Oracle/Version12/Query.php

This file was deleted.

26 changes: 21 additions & 5 deletions src/Schema/Migration.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use Doctrine\DBAL\Platforms\OraclePlatform;
use Doctrine\DBAL\Platforms\SqlitePlatform;
use Doctrine\DBAL\Schema\AbstractSchemaManager;
use Doctrine\DBAL\Schema\Column;
use Doctrine\DBAL\Schema\Table;

class Migration
Expand Down Expand Up @@ -73,11 +74,21 @@ public function table(string $tableName): self
return $this;
}

private function getPrimaryKeyColumn(): ?Column
{
if ($this->table->getPrimaryKey() === null) {
return null;
}

return $this->table->getColumn($this->table->getPrimaryKey()->getColumns()[0]);
}

public function create(): self
{
$this->getSchemaManager()->createTable($this->table);

if ($this->getDatabasePlatform() instanceof OraclePlatform) {
$pkColumn = $this->getPrimaryKeyColumn();
if ($this->getDatabasePlatform() instanceof OraclePlatform && $pkColumn !== null) {
$this->connection->expr(
<<<'EOT'
begin
Expand All @@ -90,17 +101,18 @@ public function create(): self
create or replace trigger {table_ai_trigger_before}
before insert on {table}
for each row
when (new."id" is null)
when (new.{id_column} is null)
declare
last_id {table}."id"%type;
last_id {table}.{id_column}%type;
begin
select nvl(max("id"), 0) into last_id from {table};
:new."id" := last_id + 1;
select nvl(max({id_column}), 0) into last_id from {table};
:new.{id_column} := last_id + 1;
end;
EOT,
[
'table' => $this->table->getName(),
'table_ai_trigger_before' => $this->table->getName() . '__aitb',
'id_column' => $pkColumn->getName(),
]
)->render(),
]
Expand Down Expand Up @@ -154,6 +166,10 @@ public function dropIfExists(): self

public function field(string $fieldName, array $options = []): self
{
if (($options['type'] ?? null) === 'time' && $this->getDatabasePlatform() instanceof OraclePlatform) {
$options['type'] = 'string';
}

$refType = $options['ref_type'] ?? self::REF_TYPE_NONE;
unset($options['ref_type']);

Expand Down
7 changes: 1 addition & 6 deletions src/Schema/TestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,7 @@ protected function setUp(): void
{
parent::setUp();

// establish connection
$dsn = $GLOBALS['DB_DSN'] ?? 'sqlite::memory:';
$user = $GLOBALS['DB_USER'] ?? null;
$pass = $GLOBALS['DB_PASSWD'] ?? null;

$this->db = Persistence::connect($dsn, $user, $pass);
$this->db = Persistence::connect($_ENV['DB_DSN'], $_ENV['DB_USER'], $_ENV['DB_PASSWD']);

// reset DB autoincrement to 1, tests rely on it
if ($this->getDatabasePlatform() instanceof MySQLPlatform) {
Expand Down
3 changes: 0 additions & 3 deletions tests/Persistence/Sql/QueryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1205,9 +1205,6 @@ public function testGroupConcat(): void
$q = new Oracle\Query();
$this->assertSame('listagg("foo", :a) within group (order by "foo")', $q->groupConcat('foo', '-')->render());

$q = new Oracle\Version12\Query();
$this->assertSame('listagg("foo", :a) within group (order by "foo")', $q->groupConcat('foo', '-')->render());

$q = new Postgresql\Query();
$this->assertSame('string_agg("foo", :a)', $q->groupConcat('foo', '-')->render());

Expand Down
2 changes: 1 addition & 1 deletion tests/Persistence/Sql/WithDb/ConnectionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class ConnectionTest extends TestCase
{
public function testServerConnection(): void
{
$c = Connection::connect($GLOBALS['DB_DSN'], $GLOBALS['DB_USER'], $GLOBALS['DB_PASSWD']);
$c = Connection::connect($_ENV['DB_DSN'], $_ENV['DB_USER'], $_ENV['DB_PASSWD']);

$this->assertSame('1', $c->expr('SELECT 1' . ($c->getDatabasePlatform() instanceof OraclePlatform ? ' FROM DUAL' : ''))->getOne());
}
Expand Down
2 changes: 1 addition & 1 deletion tests/Persistence/Sql/WithDb/SelectTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ private function dropDbIfExists(): void

protected function setUp(): void
{
$this->c = Connection::connect($GLOBALS['DB_DSN'], $GLOBALS['DB_USER'], $GLOBALS['DB_PASSWD']);
$this->c = Connection::connect($_ENV['DB_DSN'], $_ENV['DB_USER'], $_ENV['DB_PASSWD']);

$this->dropDbIfExists();

Expand Down
2 changes: 1 addition & 1 deletion tests/Persistence/Sql/WithDb/TransactionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ private function dropDbIfExists(): void

protected function setUp(): void
{
$this->c = Connection::connect($GLOBALS['DB_DSN'], $GLOBALS['DB_USER'], $GLOBALS['DB_PASSWD']);
$this->c = Connection::connect($_ENV['DB_DSN'], $_ENV['DB_USER'], $_ENV['DB_PASSWD']);

$this->dropDbIfExists();

Expand Down
2 changes: 1 addition & 1 deletion tests/SmboTransferTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ public function testRef(): void
/*
public function testBasicEntities(): void
{
$db = Persistence::connect($GLOBALS['DB_DSN'], $GLOBALS['DB_USER'], $GLOBALS['DB_PASSWD']);
$db = Persistence::connect($_ENV['DB_DSN'], $_ENV['DB_USER'], $_ENV['DB_PASSWD']);

// Create a new company
$company = new Company($db);
Expand Down