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

Transactions passed in find() or findFirst() to query inside of started transactions #13226

Closed
chilimatic opened this Issue Dec 18, 2017 · 2 comments

Comments

Projects
None yet
2 participants
@chilimatic
Contributor

chilimatic commented Dec 18, 2017

Hello!

Type: new feature | enhancement
Link to issue: #12409, #12564, #13097
In raising this pull request, I confirm the following (please check boxes):

  • I have read and understood the Contributing Guidelines?
  • I have checked that another pull request for this purpose does not exist.
  • I wrote some tests for this PR.

this ticket exists because I was to incompetent to first create a Ticket before starting a PR. The target branch has changed and I need to rewrite the history with the correct ticket number :)

Small description of change:

Currently you only can use the internal Transaction behaviour without an option to use a specific transaction in find / findFirst. This should be changed

This change is to achieve the following:

In Unit-testing one usual procedure is to create a transaction for a TestCase and after the execution rollback this change.

Which means there is a "long transaction" going on and we want to check if the transaction actually is writing the correct values in the database. But the static context of model::find / model::findFirst does not get the transaction passed.

To achieve that passing a new index "transaction" containing an transaction in the $param array seams to be a viable solution.

Another use-case can be having multiple transactions and wanting to look inside.

Just to be precise -> ofc we could just get the connection and query manually inside of it, but I want to be as close to the framework as possible when testing certain services hence I want to use the model.

I took your reviews and advice from the last attempt and used the params to not break the interfaces / change method signatures.

the logic is implemented as follows:

if you pass in a transaction via param in the find/findFirst method. It will set the transaction to query object and this transaction will be used for all queries and executions within this query context.

This should not affect any standard behaviour but still allow me to finally achieve cleaner unittests for my usecase.

chilimatic added a commit to chilimatic/cphalcon that referenced this issue Dec 18, 2017

phalcon#13226 Unitests for find / findFirst
 - adds setUpTransactionManager in ModelTrait
 - adds missing cache directory to README
 - adds tests for find / findFirst

chilimatic added a commit to chilimatic/cphalcon that referenced this issue Dec 18, 2017

chilimatic added a commit to chilimatic/cphalcon that referenced this issue Dec 18, 2017

phalcon#13226 Code-examples + cleanup
- adds const to Model to avoid typos
- add code examples

chilimatic added a commit to chilimatic/cphalcon that referenced this issue Dec 18, 2017

chilimatic added a commit to chilimatic/cphalcon that referenced this issue Dec 18, 2017

phalcon#13226 simplifies get transaction logic
 - fixes check before checking multiple different database engine connections

chilimatic added a commit to chilimatic/cphalcon that referenced this issue Dec 18, 2017

chilimatic added a commit to chilimatic/cphalcon that referenced this issue Dec 18, 2017

chilimatic added a commit to chilimatic/cphalcon that referenced this issue Dec 18, 2017

chilimatic added a commit to chilimatic/cphalcon that referenced this issue Dec 18, 2017

chilimatic added a commit to chilimatic/cphalcon that referenced this issue Dec 19, 2017

phalcon#13226 Adds explicit transaction to find and findFirst
 - adds setUpTransactionManager in ModelTrait
 - adds missing cache directory to README
 - adds tests for find / findFirst
 - adds const to Model to avoid typos
 - add code examples
 - fixes check before checking multiple different database engine connections
 - unifies businesslogic for find and findFirst
 - adds missing types
 - Adds Querytest And Changelog update
 - removes annotations + phpcs cleanup

chilimatic added a commit to chilimatic/cphalcon that referenced this issue Dec 19, 2017

Adds explicit transaction to find and findFirst (phalcon#13226)
 - adds setUpTransactionManager in ModelTrait
 - adds missing cache directory to README
 - adds tests for find / findFirst
 - adds const to Model to avoid typos
 - add code examples
 - fixes check before checking multiple different database engine connections
 - unifies businesslogic for find and findFirst
 - adds missing types
 - Adds Querytest And Changelog update
 - removes annotations + phpcs cleanup

chilimatic added a commit to chilimatic/cphalcon that referenced this issue Dec 19, 2017

Adds explicit transaction to find and findFirst (phalcon#13226)
 - adds setUpTransactionManager in ModelTrait
 - adds missing cache directory to README
 - adds tests for find / findFirst
 - adds const to Model to avoid typos
 - add code examples
 - fixes check before checking multiple different database engine connections
 - unifies businesslogic for find and findFirst
 - adds missing types
 - Adds Querytest And Changelog update
 - removes annotations + phpcs cleanup

chilimatic added a commit to chilimatic/cphalcon that referenced this issue Dec 19, 2017

Adds explicit transaction to find and findFirst (phalcon#13226)
 - adds setUpTransactionManager in ModelTrait
 - adds missing cache directory to README
 - adds tests for find / findFirst
 - adds const to Model to avoid typos
 - add code examples
 - fixes check before checking multiple different database engine connections
 - unifies businesslogic for find and findFirst
 - adds missing types
 - Adds Querytest And Changelog update
 - removes annotations + phpcs cleanup

sergeyklay added a commit that referenced this issue Dec 22, 2017

Adds explicit transaction to find and findFirst (#13226)
 - adds setUpTransactionManager in ModelTrait
 - adds missing cache directory to README
 - adds tests for find / findFirst
 - adds const to Model to avoid typos
 - add code examples
 - fixes check before checking multiple different database engine connections
 - unifies businesslogic for find and findFirst
 - adds missing types
 - Adds Querytest And Changelog update
 - removes annotations + phpcs cleanup
@sergeyklay

This comment has been minimized.

Show comment
Hide comment
@sergeyklay

sergeyklay Dec 22, 2017

Member

Fixed in the 3.3.x branch. Feel free to open a new issue if the problem appears again. Thank you for contributing.

Member

sergeyklay commented Dec 22, 2017

Fixed in the 3.3.x branch. Feel free to open a new issue if the problem appears again. Thank you for contributing.

@sergeyklay sergeyklay closed this Dec 22, 2017

@chilimatic

This comment has been minimized.

Show comment
Hide comment
@chilimatic

chilimatic Dec 22, 2017

Contributor

cool thx ! :)

Contributor

chilimatic commented Dec 22, 2017

cool thx ! :)

chilimatic added a commit to chilimatic/cphalcon that referenced this issue Jan 15, 2018

Adds explicit transaction to find and findFirst (phalcon#13226)
 - adds setUpTransactionManager in ModelTrait
 - adds missing cache directory to README
 - adds tests for find / findFirst
 - adds const to Model to avoid typos
 - add code examples
 - fixes check before checking multiple different database engine connections
 - unifies businesslogic for find and findFirst
 - adds missing types
 - Adds Querytest And Changelog update
 - removes annotations + phpcs cleanup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment