Copy query cache from yii\db\Command to yii\db\Query::createCommand() and yii\db\ActiveQuery::createCommand().

[hubeiwei]

Q	A
Is bugfix?	no
New feature?	yes
Breaks BC?	no
Tests pass?	yes
Fixed issues	-

[yii-bot]

Thank you for putting effort in the improvement of the Yii framework.
We have reviewed your pull request.

Unfortunately a use case is missing. It is required to get a better understanding of the pull request and helps us to determine the necessity and applicability of the suggested change to the framework.

Could you supply us with a use case please? Please be as detailed as possible and show some code!

Thanks!

This is an automated comment, triggered by adding the label pr:missing usecase.

[hubeiwei]

Command will check the cache in the query, Query should be able to set the cache

(new Query())->cache(7200)->all();
User::find()->cache(7200)->all();

[samdark]

I think http://www.yiiframework.com/doc-2.0/guide-caching-data.html#query-caching is enough as a way to achieve it.

@yiisoft/reviewers, @yiisoft/core-developers need opinions.

[hubeiwei]

Yes,I think you can write less code in this way.

[rob006]

@samdark Is there any reason to not implementing it? IMO it may be useful and will improve consistency.

[samdark]

Two reasons:

1. Another way to do the same thing.
2. We have to support it.

[hubeiwei]

yii\db\Connection::enableQueryCache defaults to true, yii\db\Command will first check the cache before query, even if you don't use it, I think it is meaningful to do so.

[rob006]

@samdark yii\db\Command already has such method - it is quite weird that low-level DB component (Command) has such shortcut and high-level DB component (Query) does not.

[samdark]

@rob006 the idea of having it only at connection level was essentially that it can wrap anything and there's no need for more ways to handle it.

[rob006]

the idea of having it only at connection level

The idea seems to be dropped, since we have:

yii2/framework/db/Command.php

Line 123 in e9bec0d

public function cache($duration = null, $dependency = null)

[samdark]

While I personally don't like the idea much, if you think it's fine I'm OK merging it.

@cebe, @SilverFire, @klimov-paul thoughts?

[SilverFire]

We already have 2 ways to cache a query:

$db->cache(function($db) {
    return User::find()->all();
});

$cache->getOrSet(['meaningful-key'], function () {
    return User::find()->all();
})

But the suggested one can be a shorter and more convenient.

I'd suggest to move properties queryCacheDuration and queryCacheDependency together with cache(), noCache() and hasCache() methods to the CacheableQueryTrait, that can be used in Query and Command.

[SilverFire]

See comment above. Otherwise good to go

[hubeiwei]

@SilverFire Command::cache() have default cache duration, through $this->db->queryCacheDuration, but Query no $db this member variables. if want to use CacheableQueryTrait, I think we should give it up.

[rob006]

@SilverFire Why did you merge this without any tests? I'm pretty sure that this will not work as expected: MyModel::find()->cache()->one($myCustomDb). Default queryCacheDuration will be taken from Yii::$app->db instead of $myCustomDb.

Also hasCache() is misleading, since it does not say whether the query has cache, it only says that cache is enabled. I'm also not sure why it is public or whether implementation is correct (does it make sense do enable cache with queryCacheDuration === null but queryCacheDependency !== null?).

[SilverFire]

It was accidentally, I'm now working on fix

[SilverFire]

See 5bd6ed5

[rob006]

I would keep cache() API consistent with Command::cache() and use $duration === null as "use Connection::$queryCacheDuration". We could just add private Query::$isCacheEnabled and pass $queryCacheDuration and $queryCacheDependency directly to command - it will simplify the code and API.