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.

Sign up for GitHub

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

Do not dump query twice on SQL exception #928

Merged
merged 2 commits into from
Nov 26, 2021
Merged

Do not dump query twice on SQL exception #928

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6e8ef1f
Do not dump query twice on SQL exception
mvorisek Nov 23, 2021
ce5aeba
improve local variable names
mvorisek Nov 23, 2021
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
4 changes: 2 additions & 2 deletions docs/hooks.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,10 +244,10 @@ Save information into auditLog about failure:

Upgrade schema:

use Atk4\Data\Persistence\Sql\Exception as DsqlException;
use Atk4\Data\Persistence\Sql\Exception as SqlException;

$m->onHook(Model::HOOK_ROLLBACK, function($m, $exception) {
if ($exception instanceof DsqlException) {
if ($exception instanceof SqlException) {
$m->schema->upgrade();
$m->breakHook(false); // exception will not be thrown
}
Expand Down
22 changes: 6 additions & 16 deletions src/Persistence/Sql.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
use Atk4\Data\Model;
use Atk4\Data\Persistence;
use Atk4\Data\Persistence\Sql\Connection;
use Atk4\Data\Persistence\Sql\Exception as DsqlException;
use Atk4\Data\Persistence\Sql\Exception as SqlException;
use Atk4\Data\Persistence\Sql\Expression;
use Atk4\Data\Persistence\Sql\Query;
use Doctrine\DBAL\Platforms\AbstractPlatform;
Expand Down Expand Up @@ -529,10 +529,8 @@ public function tryLoad(Model $model, $id): ?array
->addMoreInfo('id', $noId ? null : $id);
}
$data = $this->typecastLoadRow($model, $rowsRaw[0]);
} catch (DsqlException $e) {
} catch (SqlException $e) {
throw (new Exception('Unable to load due to query error', 0, $e))
->addMoreInfo('query', $query->getDebugQuery())
->addMoreInfo('message', $e->getMessage())
->addMoreInfo('model', $model)
->addMoreInfo('scope', $model->scope()->toWords());
}
Expand Down Expand Up @@ -565,10 +563,8 @@ public function insert(Model $model, array $data): string
try {
$model->hook(self::HOOK_BEFORE_INSERT_QUERY, [$insert]);
$st = $insert->execute();
} catch (DsqlException $e) {
} catch (SqlException $e) {
throw (new Exception('Unable to execute insert query', 0, $e))
->addMoreInfo('query', $insert->getDebugQuery())
->addMoreInfo('message', $e->getMessage())
->addMoreInfo('model', $model)
->addMoreInfo('scope', $model->getModel(true)->scope()->toWords());
}
Expand Down Expand Up @@ -606,10 +602,8 @@ public function prepareIterator(Model $model): \Traversable

try {
return $export->getRowsIterator();
} catch (DsqlException $e) {
} catch (SqlException $e) {
throw (new Exception('Unable to execute iteration query', 0, $e))
->addMoreInfo('query', $export->getDebugQuery())
->addMoreInfo('message', $e->getMessage())
->addMoreInfo('model', $model)
->addMoreInfo('scope', $model->scope()->toWords());
}
Expand Down Expand Up @@ -639,10 +633,8 @@ public function update(Model $model, $id, array $data): void
if ($data) {
$st = $update->execute();
}
} catch (DsqlException $e) {
} catch (SqlException $e) {
throw (new Exception('Unable to update due to query error', 0, $e))
->addMoreInfo('query', $update->getDebugQuery())
->addMoreInfo('message', $e->getMessage())
->addMoreInfo('model', $model)
->addMoreInfo('scope', $model->getModel(true)->scope()->toWords());
}
Expand Down Expand Up @@ -684,10 +676,8 @@ public function delete(Model $model, $id): void

try {
$delete->execute();
} catch (DsqlException $e) {
} catch (SqlException $e) {
throw (new Exception('Unable to delete due to query error', 0, $e))
->addMoreInfo('query', $delete->getDebugQuery())
->addMoreInfo('message', $e->getMessage())
->addMoreInfo('model', $model)
->addMoreInfo('scope', $model->getModel(true)->scope()->toWords());
}
Expand Down
19 changes: 9 additions & 10 deletions src/Reference/HasOneSql.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

namespace Atk4\Data\Reference;

use Atk4\Data\Field;
use Atk4\Data\Field\SqlExpressionField;
use Atk4\Data\Model;

Expand All @@ -13,10 +12,10 @@ class HasOneSql extends HasOne
/**
* Creates expression which sub-selects a field inside related model.
*/
public function addField(string $ourFieldName, string $theirFieldName = null, array $defaults = []): SqlExpressionField
public function addField(string $fieldName, string $theirFieldName = null, array $defaults = []): SqlExpressionField
{
if ($theirFieldName === null) {
$theirFieldName = $ourFieldName;
$theirFieldName = $fieldName;
}

$ourModel = $this->getOurModel(null);
Expand All @@ -29,7 +28,7 @@ public function addField(string $ourFieldName, string $theirFieldName = null, ar
$defaults['caption'] ??= $refModelField->caption;
$defaults['ui'] ??= $refModelField->ui;

$fieldExpression = $ourModel->addExpression($ourFieldName, array_merge(
$fieldExpression = $ourModel->addExpression($fieldName, array_merge(
[
function (Model $ourModel) use ($theirFieldName) {
// remove order if we just select one field from hasOne model
Expand All @@ -47,17 +46,17 @@ function (Model $ourModel) use ($theirFieldName) {
));

// Will try to execute last
$this->onHookToOurModel($ourModel, Model::HOOK_BEFORE_SAVE, function (Model $ourModel) use ($ourFieldName, $theirFieldName) {
// if title field is changed, but reference ID field (our_field)
$this->onHookToOurModel($ourModel, Model::HOOK_BEFORE_SAVE, function (Model $ourModel) use ($fieldName, $theirFieldName) {
// if field is changed, but reference ID field (our_field)
// is not changed, then update reference ID field value
if ($ourModel->isDirty($ourFieldName) && !$ourModel->isDirty($this->our_field)) {
if ($ourModel->isDirty($fieldName) && !$ourModel->isDirty($this->our_field)) {
$theirModel = $this->createTheirModel();

$theirModel->addCondition($theirFieldName, $ourModel->get($ourFieldName));
$theirModel->addCondition($theirFieldName, $ourModel->get($fieldName));
$ourModel->set($this->getOurFieldName(), $theirModel->action('field', [$theirModel->id_field]));
$ourModel->_unset($ourFieldName);
$ourModel->_unset($fieldName);
}
}, [], 21);
}, [], 20);

return $fieldExpression;
}
Expand Down