-
Notifications
You must be signed in to change notification settings - Fork 52
InnerTransaction #94
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
InnerTransaction #94
Changes from all commits
8ff6db5
29bad3f
1a1448e
d0063a7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,113 @@ | ||
<?php | ||
/*************************************************************************** | ||
* Copyright (C) 2012 by Alexey S. Denisov * | ||
* * | ||
* This program is free software; you can redistribute it and/or modify * | ||
* it under the terms of the GNU Lesser General Public License as * | ||
* published by the Free Software Foundation; either version 3 of the * | ||
* License, or (at your option) any later version. * | ||
* * | ||
***************************************************************************/ | ||
|
||
/** | ||
* Utility to create transaction and not think about current nested level | ||
* | ||
* @ingroup Transaction | ||
**/ | ||
final class InnerTransaction | ||
{ | ||
/** | ||
* @var DB | ||
**/ | ||
private $db = null; | ||
private $savepointName = null; | ||
private $finished = false; | ||
|
||
/** | ||
* @param DB|GenericDAO $database | ||
* @param IsolationLevel $level | ||
* @param AccessMode $mode | ||
* @return InnerTransaction | ||
**/ | ||
public static function begin( | ||
$database, | ||
IsolationLevel $level = null, | ||
AccessMode $mode = null | ||
) | ||
{ | ||
return new self($database, $level, $mode); | ||
} | ||
|
||
/** | ||
* @param DB|GenericDAO $database | ||
* @param IsolationLevel $level | ||
* @param AccessMode $mode | ||
**/ | ||
public function __construct( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Может быть имеет смысл сделать конструктор приватным? Это бы нам дало одну точку доступа. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. У нас много где есть create который должно совпадать по аргументам с construct. Тут просто отступил от правила и назвал метод begin, для наглядности, т.к. хорошо ложиться. Но так или иначе при существующем create мы конструктор приватным не делаем. |
||
$database, | ||
IsolationLevel $level = null, | ||
AccessMode $mode = null | ||
) | ||
{ | ||
if ($database instanceof DB) { | ||
$this->db = $database; | ||
} elseif ($database instanceof GenericDAO) { | ||
$this->db = DBPool::getByDao($database); | ||
} else { | ||
throw new WrongStateException( | ||
'$database must be instance of DB or GenericDAO' | ||
); | ||
} | ||
|
||
$this->beginTransaction($level, $mode); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. А давай вынесем beginTransaction() в метод begin(), что бы при создании объекта он ничего не писал в базу, а только создавался There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. А мне текущая реализация нравится :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Мне честно говоря самому текущий вариант понравился. Он короче и нагляден и вроде бы нет причин создавать объект раньше чем надо начать трнзакцию. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Логично ))) |
||
} | ||
|
||
public function commit() | ||
{ | ||
$this->assertFinished(); | ||
$this->finished = true; | ||
if (!$this->savepointName) { | ||
$this->db->commit(); | ||
} else { | ||
$this->db->savepointRelease($this->savepointName); | ||
} | ||
} | ||
|
||
public function rollback() | ||
{ | ||
$this->assertFinished(); | ||
$this->finished = true; | ||
if (!$this->savepointName) { | ||
$this->db->rollback(); | ||
} else { | ||
$this->db->savepointRollback($this->savepointName); | ||
} | ||
} | ||
|
||
private function beginTransaction( | ||
IsolationLevel $level = null, | ||
AccessMode $mode = null | ||
) | ||
{ | ||
$this->assertFinished(); | ||
if (!$this->db->inTransaction()) { | ||
$this->db->begin($level, $mode); | ||
} else { | ||
$this->savepointName = $this->createSavepointName(); | ||
$this->db->savepointBegin($this->savepointName); | ||
} | ||
} | ||
|
||
private function assertFinished() | ||
{ | ||
if ($this->finished) | ||
throw new WrongStateException('This Transaction already finished'); | ||
} | ||
|
||
private static function createSavepointName() | ||
{ | ||
static $i = 1; | ||
return 'innerSavepoint'.($i++); | ||
} | ||
} | ||
?> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,134 @@ | ||
<?php | ||
/*************************************************************************** | ||
* Copyright (C) 2012 by Alexey S. Denisov * | ||
* * | ||
* This program is free software; you can redistribute it and/or modify * | ||
* it under the terms of the GNU Lesser General Public License as * | ||
* published by the Free Software Foundation; either version 3 of the * | ||
* License, or (at your option) any later version. * | ||
* * | ||
***************************************************************************/ | ||
|
||
/** | ||
* Utility to wrap function into transaction | ||
* | ||
* @ingroup Transaction | ||
**/ | ||
final class InnerTransactionWrapper | ||
{ | ||
/** | ||
* @var DB | ||
*/ | ||
private $db = null; | ||
/** | ||
* @var StorableDAO | ||
*/ | ||
private $dao = null; | ||
private $function = null; | ||
private $exceptionFunction = null; | ||
/** | ||
* @var IsolationLevel | ||
*/ | ||
private $level = null; | ||
/** | ||
* @var AccessMode | ||
*/ | ||
private $mode = null; | ||
|
||
/** | ||
* @return InnerTransactionWrapper | ||
*/ | ||
public static function create() | ||
{ | ||
return new self; | ||
} | ||
|
||
/** | ||
* @param DB $db | ||
* @return InnerTransactionWrapper | ||
*/ | ||
public function setDB(DB $db) | ||
{ | ||
$this->db = $db; | ||
return $this; | ||
} | ||
|
||
/** | ||
* @param StorableDAO $dao | ||
* @return InnerTransactionWrapper | ||
*/ | ||
public function setDao(StorableDAO $dao) | ||
{ | ||
$this->dao = $dao; | ||
return $this; | ||
} | ||
|
||
/** | ||
* @param collable $function | ||
* @return InnerTransactionWrapper | ||
*/ | ||
public function setFunction($function) | ||
{ | ||
Assert::isTrue(is_callable($function, false), '$function must be callable'); | ||
$this->function = $function; | ||
return $this; | ||
} | ||
|
||
/** | ||
* @param collable $function | ||
* @return InnerTransactionWrapper | ||
*/ | ||
public function setExceptionFunction($function) | ||
{ | ||
Assert::isTrue(is_callable($function, false), '$function must be callable'); | ||
$this->exceptionFunction = $function; | ||
return $this; | ||
} | ||
|
||
/** | ||
* @param IsolationLevel $level | ||
* @return InnerTransactionWrapper | ||
*/ | ||
public function setLevel(IsolationLevel $level) | ||
{ | ||
$this->level = $level; | ||
return $this; | ||
} | ||
|
||
/** | ||
* @param AccessMode $mode | ||
* @return InnerTransactionWrapper | ||
*/ | ||
public function setMode(AccessMode $mode) | ||
{ | ||
$this->mode = $mode; | ||
return $this; | ||
} | ||
|
||
public function run() | ||
{ | ||
Assert::isTrue(!is_null($this->dao) || !is_null($this->db), 'set first dao or db'); | ||
Assert::isNotNull($this->function, 'set first function'); | ||
|
||
$transaction = InnerTransaction::begin( | ||
$this->dao ?: $this->db, | ||
$this->level, | ||
$this->mode | ||
); | ||
|
||
try { | ||
$result = call_user_func_array($this->function, func_get_args()); | ||
$transaction->commit(); | ||
return $result; | ||
} catch (Exception $e) { | ||
$transaction->rollback(); | ||
if ($this->exceptionFunction) { | ||
$args = func_get_args(); | ||
array_unshift($args, $e); | ||
return call_user_func_array($this->exceptionFunction, $args); | ||
} | ||
throw $e; | ||
} | ||
} | ||
} | ||
?> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
на уровне идеи - а может сделать интерфейс, который бы реализовывали оба этих класса и ожидать именно его. DBFull, например.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Почему бы и нет, к примеру DBTransactional, также в диалекте реализовать эти методы и вроде как бы гуд должно получится :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Это сделано было для упрощения работы с классом. Попробую пояснить ход мыслей, когда именно так сделал
Реально классу нужно иметь именно DB под рукой, т.к. именного у него вызывает begin, commit, rollback. Но обычно мы работаем с объектами и у них под боком есть DAO. Из DAO можно вытащить DB, вот так:
$db = DBPool::GetByDao(TestObject::dao());
На N'ый раз уже жутко раздражает этот копипаст.Можно было бы всегда принимать только DAO и из него доставать DB, но это не хорошо, т.к. нет нет, да захочется вдруг передать именно DB и тогда жуть как сложно придется выкручиваться. Сделать два метода с чуть разными названиями и разными аргументами тоже не хочется, т.к. это вносит лишний копипаст и путаницу.
Будь в php перегрузка методов с разными аргументами - не было бы проблем. Собственно по этом и делаем в этом методе вид, что какая-то перегрузка методов есть.
Обий интерфейс придумывать и вешать на классы тоже не хочется ради какой-то новой фичи. Так классы глядишь и обрастут дясятком других интерфейсов. Поэтому я сделал так как оно есть сейчас и усложнять как-то совсем не хочется.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Не знаю что имел в виду Женя, но я понял это как некий интерейс для дтранзакционности в БД, сейчас у нас есть "BaseTransaction" и "DBTransaction" которые нигде не используются, ну собсно понятно почему )))
Вот за место них сделать интерфейс DBTransactional и дальше этот интефейс имплементировать в сам дравер и диалект )))
Лично я так подумал.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Я имел ввиду то, что описал @AlexeyDsov ;)
Ну нет, так нет, это не так и принципиально.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ну значит я не в тему описал.
Ну я за то что сейчас есть ))) вроде как оптимально пока ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Помоему имелось ввиду что-то другое. Про BaseTransaction и DBTransaction я думаю все так же - из-за них begin, commit и т.д. в базе объявлены deprecated но при этом их самих использовать неудобно + inTransaction свойство по ощущениям очень привязано к базе, а за базу отвечает именно DB класс. С другой стороны из DB сложную логику нужно выносить, т.к. вроде бы он достаточно перегружен и всякие наворотам c queue и queueFlush там не особо место, пожалуй.
P.S. пока писал ответы уже появились
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ну собсно о чем и речь, и лучше всего думаю это сделать интерфейсами.