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.

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

Introduce interface for entity querybuilders #5354

Closed
NateWr opened this issue Dec 17, 2019 · 2 comments
Closed

Introduce interface for entity querybuilders #5354

NateWr opened this issue Dec 17, 2019 · 2 comments
Labels
Housekeeping:1:Todo Any dependency management or refactor that would be nice to have some day.

Comments

@NateWr
Copy link
Contributor

NateWr commented Dec 17, 2019

Query builders for entities can and should handle data retrieval whenever full objects are not required. This is in cases like getting a count of objects or retrieving all the IDs that match a set of filters.

To keep the query builders aligned, they should implement an interface with four methods. The first method, _getQueryObject, is a protected method which constructs the filter criteria for the query -- the where, leftJoin, etc conditions. It will be used by the other three methods, which offer distinct select criteria:

  • getQuery will return a query object that can be passed to the DAO's retrieveRange method to be used for getting a collection of objects.
  • count will return a count of the rows which match the query.
  • getIds will return an array of the entity ids which match the query.

This will allow more of Laravel's querybuilder syntax to be used to generate useful queries. First we define the query builder conditions (same as before):

$qb
	->filterByContexts([1])
	->orderBy('dateSubmitted');

Then we can use the same query conditions to retrieve different things:

// Get a count of matching rows
$count = $qb->count();

// Get a list of all the matching ids
$ids = $qb->getIds();

// Get iterator of full objects
$qo = $qb->getQuery();
$result = Application::get()
	->getSubmissionDAO(
		$qo->toSql(),
		$qo->getBindings(),
		new DBResultRange(20, null, 0);
	);
$queryResults = new DAOResultFactory($result, $submissionDao, '_fromRow');
$objectIterator = $queryResults->toIterator();

The query object that getQuery will return can be executed with Laravel's get() and then used alongside other features of Laravel's querybuilders to get other kinds of results. In this case, we want an array of titles:

$titles = $qb->getQuery()->get()->pluck('title');
@NateWr NateWr added the Housekeeping:1:Todo Any dependency management or refactor that would be nice to have some day. label Dec 17, 2019
NateWr added a commit to NateWr/pkp-lib that referenced this issue Jan 13, 2020
- Add interface methods for getting a query builder,
  counting rows, and getting a list of IDs.
- Add methods to EntityReadInterface to provide
  Service-level access to these queries.
- Change some instances of service class usage to
  use Services::get() rather than assigning the
  service to a variable. This is done for consistency.
NateWr added a commit to NateWr/pkp-lib that referenced this issue Jan 14, 2020
- Add interface methods for getting a query builder,
  counting rows, and getting a list of IDs.
- Add methods to EntityReadInterface to provide
  Service-level access to these queries.
- Change some instances of service class usage to
  use Services::get() rather than assigning the
  service to a variable. This is done for consistency.
NateWr added a commit to NateWr/pkp-lib that referenced this issue Jan 14, 2020
After some time, the trait did not get much use. It appears to be
an abstraction with little benefit, so I've taken the one method
and put the code inline wherever it is used.
NateWr added a commit to NateWr/ojs that referenced this issue Jan 14, 2020
- Add interface methods for getting a query builder,
counting rows, and getting a list of IDs.
- Add methods to EntityReadInterface to provide
Service-level access to these queries.
- Change some instances of service class usage to
use Services::get() rather than assigning the
service to a variable. This is done for consistency.
NateWr added a commit to NateWr/ojs that referenced this issue Jan 14, 2020
See commit message in pkp-lib: b2fc266eb1495eec38b032498031bc51f1008c9e
@NateWr
Copy link
Contributor Author

NateWr commented Jan 14, 2020

PRs:
#5389
pkp/ojs#2575
(No OMP PR required)

@asmecher can you take a look at this and let me know if this looks alright? Some further details below.

These PRs provide a new EntityQueryBuilderInterface that all query builders implement when they deal with entities. The interface provides three methods which should make it easier to use the query builders in different cases:

  • getCount() will return a count of the matching rows.
  • getIds() will return an array of ids of the matching rows.
  • getQuery() will return an instance of Laravel's \Query\Builder.

The getCount() and getIds() methods should provide a more performant database read when you don't need to get all of the object data out. There weren't many uses of this but it should still provide benefits in a few places.

The result of getQuery() can be used for lots of things. The one we use it for most is that we can pass it to our DAOs in order to retrieve full objects. But because it is an instance of \Query\Builder from Laravel, it can also be used with any of the helper methods that Laravel provides. Some examples for retrieving user data:

$qb = new \APP\Services\QueryBuilders\UserQueryBuilder();
$qb->filterByContext($contextId);

// Get all matching emails
$emails = $qb->getQuery()->pluck('u.email');

// Get the first matching result
$user = $qb->getQuery()->first();

// Get the last registered user
$user = $qb->getQuery()->max('u.date_registered');

Documentation on the Laravel 5.5 Query Builder.

Our Service classes have also been extended to expose this functionality to third-party users. The EntityReadInterface now supports getCount() and getIds() methods. And the getQueryBuilder() method is now public so that plugins can make use of it to get the query builder and execute arbitrary queries on it.

(Side note: I think naming our classes QueryBuilders is a bit confusing since that's what Laravel calls its implementation as well. It is probably worth consider a new name for our convention. In many ways these could grow to merge with our DAOs, but that's a discussion for a later date.)

@asmecher
Copy link
Member

That looks like a sensible cleanup to me, Nate, thanks! I see some room for improvement around the pagination stuff -- that's another case where I'd love to get rid of our old home-grown stuff entirely -- but let's come back to that along with more discussion about e.g. DAO vs. query builder etc.

NateWr added a commit to NateWr/pkp-lib that referenced this issue Jan 16, 2020
- Add interface methods for getting a query builder,
  counting rows, and getting a list of IDs.
- Add methods to EntityReadInterface to provide
  Service-level access to these queries.
- Change some instances of service class usage to
  use Services::get() rather than assigning the
  service to a variable. This is done for consistency.
NateWr added a commit to NateWr/pkp-lib that referenced this issue Jan 16, 2020
After some time, the trait did not get much use. It appears to be
an abstraction with little benefit, so I've taken the one method
and put the code inline wherever it is used.
NateWr added a commit to NateWr/ojs that referenced this issue Jan 16, 2020
- Add interface methods for getting a query builder,
counting rows, and getting a list of IDs.
- Add methods to EntityReadInterface to provide
Service-level access to these queries.
- Change some instances of service class usage to
use Services::get() rather than assigning the
service to a variable. This is done for consistency.
NateWr added a commit to NateWr/ojs that referenced this issue Jan 16, 2020
See commit message in pkp-lib: b2fc266eb1495eec38b032498031bc51f1008c9e
NateWr added a commit to NateWr/pkp-lib that referenced this issue Jan 16, 2020
- Add interface methods for getting a query builder,
  counting rows, and getting a list of IDs.
- Add methods to EntityReadInterface to provide
  Service-level access to these queries.
- Change some instances of service class usage to
  use Services::get() rather than assigning the
  service to a variable. This is done for consistency.
NateWr added a commit to NateWr/pkp-lib that referenced this issue Jan 16, 2020
After some time, the trait did not get much use. It appears to be
an abstraction with little benefit, so I've taken the one method
and put the code inline wherever it is used.
NateWr added a commit to NateWr/ojs that referenced this issue Jan 16, 2020
- Add interface methods for getting a query builder,
counting rows, and getting a list of IDs.
- Add methods to EntityReadInterface to provide
Service-level access to these queries.
- Change some instances of service class usage to
use Services::get() rather than assigning the
service to a variable. This is done for consistency.
NateWr added a commit to NateWr/ojs that referenced this issue Jan 16, 2020
See commit message in pkp-lib: b2fc266eb1495eec38b032498031bc51f1008c9e
NateWr added a commit to NateWr/pkp-lib that referenced this issue Jan 17, 2020
- Add interface methods for getting a query builder,
  counting rows, and getting a list of IDs.
- Add methods to EntityReadInterface to provide
  Service-level access to these queries.
- Change some instances of service class usage to
  use Services::get() rather than assigning the
  service to a variable. This is done for consistency.
NateWr added a commit to NateWr/pkp-lib that referenced this issue Jan 17, 2020
After some time, the trait did not get much use. It appears to be
an abstraction with little benefit, so I've taken the one method
and put the code inline wherever it is used.
NateWr added a commit to NateWr/ojs that referenced this issue Jan 17, 2020
- Add interface methods for getting a query builder,
counting rows, and getting a list of IDs.
- Add methods to EntityReadInterface to provide
Service-level access to these queries.
- Change some instances of service class usage to
use Services::get() rather than assigning the
service to a variable. This is done for consistency.
NateWr added a commit to NateWr/ojs that referenced this issue Jan 17, 2020
See commit message in pkp-lib: b2fc266eb1495eec38b032498031bc51f1008c9e
@NateWr NateWr closed this as completed Jan 17, 2020
ajnyga pushed a commit to ajnyga/ojs that referenced this issue Feb 4, 2020
- Add interface methods for getting a query builder,
counting rows, and getting a list of IDs.
- Add methods to EntityReadInterface to provide
Service-level access to these queries.
- Change some instances of service class usage to
use Services::get() rather than assigning the
service to a variable. This is done for consistency.
ajnyga pushed a commit to ajnyga/ojs that referenced this issue Feb 4, 2020
See commit message in pkp-lib: b2fc266eb1495eec38b032498031bc51f1008c9e
MedAhamada pushed a commit to Maanrifa/ojs that referenced this issue Apr 19, 2020
- Add interface methods for getting a query builder,
counting rows, and getting a list of IDs.
- Add methods to EntityReadInterface to provide
Service-level access to these queries.
- Change some instances of service class usage to
use Services::get() rather than assigning the
service to a variable. This is done for consistency.
MedAhamada pushed a commit to Maanrifa/ojs that referenced this issue Apr 19, 2020
See commit message in pkp-lib: b2fc266eb1495eec38b032498031bc51f1008c9e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Housekeeping:1:Todo Any dependency management or refactor that would be nice to have some day.
Projects
None yet
Development

No branches or pull requests

2 participants