Skip to content

Abstract Query implementation from persistence specific ones#690

Closed
georgehristov wants to merge 88 commits intodevelopfrom
action_to_toquery_with_interfaces
Closed

Abstract Query implementation from persistence specific ones#690
georgehristov wants to merge 88 commits intodevelopfrom
action_to_toquery_with_interfaces

Conversation

@georgehristov
Copy link
Copy Markdown
Collaborator

@georgehristov georgehristov commented Aug 2, 2020

The concept unifies the persistence query objects which must extend Persistence\AbstractQuery
In 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\Query object in atk4/data irrelevant of persistence type.

The Model::load functionality 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.

  • Array_ persistence is using \ArrayIterator
  • Csv persistence is using \SplFileObject which is also an iterator

If the concept is accepted it can be further worked on:

  • introduce Model\QueryTrait to introduce direct access to query methods (insert, select, delete, etc) and use in the model object
  • introduce optional Model\QueryAggregatesTrait to introduce direct access to aggregate methods (sum, avg, etc) and use in the model object
  • optimize code

BC break:

  • Model::action renamed to Model::toQuery
  • Persistence::action renamed to Persistence::query

@georgehristov georgehristov force-pushed the action_to_toquery_with_interfaces branch 3 times, most recently from 09bb6f0 to a9109b3 Compare August 2, 2020 09:08
@georgehristov georgehristov force-pushed the action_to_toquery_with_interfaces branch from a9109b3 to 5a3a7c9 Compare August 2, 2020 09:43
# Conflicts:
#	src/Model/Scope/Condition.php
#	tests/ConditionSqlTest.php
@mvorisek mvorisek mentioned this pull request Sep 14, 2020
# Conflicts:
#	src/Action/Iterator.php
#	src/Model/Scope/Condition.php
#	src/Persistence/Array_.php
#	src/Persistence/Sql.php
@romaninsh romaninsh added this to the 2.3 milestone Sep 16, 2020
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
# 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
@georgehristov georgehristov force-pushed the action_to_toquery_with_interfaces branch from e51509e to 1e313dc Compare October 30, 2020 16:13
@georgehristov georgehristov force-pushed the action_to_toquery_with_interfaces branch from 1e313dc to b413fd1 Compare October 30, 2020 16:52
@mvorisek mvorisek mentioned this pull request Dec 1, 2020
}

// get first record
if ($row = $this->persistence->query($this->model)->select([$fieldName])->getRow()) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder why this #861 bug passed test in this refactoring...

@mvorisek mvorisek removed this from the 2.3 milestone Apr 15, 2022
@mvorisek
Copy link
Copy Markdown
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?

@mvorisek
Copy link
Copy Markdown
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.

@mvorisek
Copy link
Copy Markdown
Member

mvorisek commented Sep 7, 2023

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.

@mvorisek mvorisek closed this Sep 7, 2023
@mvorisek mvorisek deleted the action_to_toquery_with_interfaces branch September 7, 2023 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

4 participants