Abstract Query implementation from persistence specific ones#690
Closed
georgehristov wants to merge 88 commits intodevelopfrom
Closed
Abstract Query implementation from persistence specific ones#690georgehristov wants to merge 88 commits intodevelopfrom
georgehristov wants to merge 88 commits intodevelopfrom
Conversation
… action_to_toquery
09bb6f0 to
a9109b3
Compare
mvorisek
reviewed
Aug 2, 2020
mvorisek
reviewed
Aug 2, 2020
mvorisek
reviewed
Aug 2, 2020
a9109b3 to
5a3a7c9
Compare
# Conflicts: # src/Model/Scope/Condition.php # tests/ConditionSqlTest.php
# Conflicts: # src/Persistence/Sql.php
Merged
# Conflicts: # src/Action/Iterator.php # src/Model/Scope/Condition.php # src/Persistence/Array_.php # src/Persistence/Sql.php
mvorisek
added a commit
that referenced
this pull request
Sep 18, 2020
* reimplemented as in #690 * fix id getter Co-authored-by: Michael Voříšek <mvorisek@mvorisek.cz>
# Conflicts: # docs/advanced.rst # src/Action/Iterator.php # src/Model.php # src/Persistence/Array_.php # src/Persistence/Sql.php # src/Reference/ContainsMany.php # src/Reference/ContainsOne.php # tests/PersistentArrayOfStringsTest.php # tests/PersistentArrayTest.php
4993719 to
b948796
Compare
# Conflicts: # .github/workflows/unit-tests.yml # src-schema/Migration.php # src-schema/PhpunitTestCase.php # src/Action/Iterator.php # src/Field.php # src/FieldSqlExpression.php # src/Model/Join.php # src/Persistence.php # src/Persistence/Sql.php # src/Persistence/Sql/Join.php # tests-schema/ModelTest.php # tests/DeepCopyTest.php
# Conflicts: # tests/ExpressionSqlTest.php
e51509e to
1e313dc
Compare
1e313dc to
b413fd1
Compare
Merged
mvorisek
reviewed
Apr 23, 2021
| } | ||
|
|
||
| // get first record | ||
| if ($row = $this->persistence->query($this->model)->select([$fieldName])->getRow()) { |
Member
There was a problem hiding this comment.
I wonder why this #861 bug passed test in this refactoring...
Member
|
Hi @georgehristov, the ship sailed already too much to be able to resolve the conflicts easily. Can you please rebase/redo this PR on top of the current develop branch or should this PR be closed? |
Member
|
@georgehristov I am gonna to close this PR soon, if there is any value outside the rename of "action" to "query", I would happily integrate these improvements as a separate PR. |
Member
|
Closing due inactivity. The persistence changes might be welcomed, but should be submit without the action->query refactoring, any big refactoring must be submit as a separate PR. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The concept unifies the persistence query objects which must extend
Persistence\AbstractQueryIn the case of Persistence\Sql\Query the calls are forwarded to underlying DSQL object.
This abstracts the link to the actual query engine and unifies the use of
Persistence\Queryobject in atk4/data irrelevant of persistence type.The
Model::loadfunctionality has been moved to the Model object strictly leaving Persistence object to care only about raw data CRUD.The Persistence CRUD methods have been moved to the generic Persistence class which relies on storage specific Query objects to perform the interaction with the data storage engine (Sql, Array (memory), Csv).
Array and Csv Query engines extend
Persistence\IteratorQuery.\ArrayIterator\SplFileObjectwhich is also an iteratorIf the concept is accepted it can be further worked on:
Model\QueryTraitto introduce direct access to query methods (insert, select, delete, etc) and use in the model objectModel\QueryAggregatesTraitto introduce direct access to aggregate methods (sum, avg, etc) and use in the model objectBC break:
Model::actionrenamed toModel::toQueryPersistence::actionrenamed toPersistence::query