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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed bug that the empty caches cannot be used for Model::findManyFromCache(). #5643

Merged
merged 17 commits into from
Apr 14, 2023
Merged
2 changes: 2 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -70,5 +70,7 @@ jobs:
run: export TRAVIS_BUILD_DIR=$(pwd) && bash ./.travis/setup.pgsql.sh
- name: Run Scripts Before Test
run: cp .travis/.env.example .env
- name: Print PHP Environments
run: php -i
- name: Run Test Cases
run: ./.travis/run.test.sh
1 change: 1 addition & 0 deletions CHANGELOG-3.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
## Fixed

- [#5642](https://github.com/hyperf/hyperf/pull/5642) Fixed bug that the model cache cannot created when using `find many` to get non-exists models.
- [#5643](https://github.com/hyperf/hyperf/pull/5643) Fixed bug that the empty caches cannot be used for `Model::findManyFromCache()`.

## Added

Expand Down
23 changes: 23 additions & 0 deletions src/model-cache/src/Handler/DefaultValueInterface.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?php

declare(strict_types=1);
/**
* This file is part of Hyperf.
*
* @link https://www.hyperf.io
* @document https://hyperf.wiki
* @contact group@hyperf.io
* @license https://github.com/hyperf/hyperf/blob/master/LICENSE
*/
namespace Hyperf\ModelCache\Handler;

interface DefaultValueInterface
{
public function defaultValue(mixed $primaryValue): array;

public function isDefaultValue(array $data): bool;

public function getPrimaryValue(array $data): mixed;

public function clearDefaultValue(array $data): array;
}
29 changes: 26 additions & 3 deletions src/model-cache/src/Handler/RedisHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
use Hyperf\Utils\InteractsWithTime;
use Psr\Container\ContainerInterface;

class RedisHandler implements HandlerInterface
class RedisHandler implements HandlerInterface, DefaultValueInterface
{
use InteractsWithTime;

Expand Down Expand Up @@ -69,7 +69,7 @@ public function set($key, $value, $ttl = null): bool
throw new CacheException('The value must is array.');
}

$data = array_merge($data, [$this->defaultKey => $this->defaultValue]);
$data = array_merge([$this->defaultKey => $this->defaultValue], $data);
$res = $this->redis->hMSet($key, $data);
if ($ttl) {
$seconds = $this->secondsUntil($ttl);
Expand Down Expand Up @@ -101,7 +101,6 @@ public function getMultiple($keys, $default = null): iterable
$data = $this->manager->handle(HashGetMultiple::class, (array) $keys);
$result = [];
foreach ($data as $item) {
unset($item[$this->defaultKey]);
if (! empty($item)) {
$result[] = $item;
}
Expand Down Expand Up @@ -135,4 +134,28 @@ public function incr($key, $column, $amount): bool

return is_numeric($data);
}

public function defaultValue(mixed $primaryValue): array
{
return [
$this->defaultKey => $primaryValue,
];
}

public function isDefaultValue(array $data): bool
{
$value = current($data);
return $this->defaultValue($value) === $data;
}

public function getPrimaryValue(array $data): mixed
{
return current($data);
}

public function clearDefaultValue(array $data): array
{
unset($data[$this->defaultKey]);
return $data;
}
}
27 changes: 22 additions & 5 deletions src/model-cache/src/Manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
use Hyperf\Database\Model\Collection;
use Hyperf\Database\Model\Model;
use Hyperf\DbConnection\Collector\TableCollector;
use Hyperf\ModelCache\Handler\DefaultValueInterface;
use Hyperf\ModelCache\Handler\HandlerInterface;
use Hyperf\ModelCache\Handler\RedisHandler;
use InvalidArgumentException;
Expand Down Expand Up @@ -77,14 +78,14 @@ public function findFromCache($id, string $class): ?Model
}

// Fetch it from database, because it not exists in cache handler.
if (is_null($data)) {
if ($data === null) {
$model = $instance->newQuery()->where($primaryKey, '=', $id)->first();
if ($model) {
$ttl = $this->getCacheTTL($instance, $handler);
$handler->set($key, $this->formatModel($model), $ttl);
} else {
$ttl = $handler->getConfig()->getEmptyModelTtl();
$handler->set($key, [], $ttl);
$handler->set($key, $this->defaultValue($handler, $id), $ttl);
}
return $model;
}
Expand Down Expand Up @@ -121,7 +122,15 @@ public function findManyFromCache(array $ids, string $class): Collection
$items = [];
$fetchIds = [];
foreach ($data as $item) {
if ($handler instanceof DefaultValueInterface && $handler->isDefaultValue($item)) {
$fetchIds[] = $handler->getPrimaryValue($item);
continue;
}

if (isset($item[$primaryKey])) {
if ($handler instanceof DefaultValueInterface) {
$item = $handler->clearDefaultValue($item);
}
$items[] = $item;
$fetchIds[] = $item[$primaryKey];
}
Expand All @@ -140,7 +149,7 @@ public function findManyFromCache(array $ids, string $class): Collection
if ($model = $dictionary[$id] ?? null) {
$handler->set($key, $this->formatModel($model), $ttl);
} else {
$handler->set($key, [], $emptyTtl);
$handler->set($key, $this->defaultValue($handler, $id), $emptyTtl);
}
}

Expand Down Expand Up @@ -168,9 +177,8 @@ public function findManyFromCache(array $ids, string $class): Collection

/**
* Destroy the models for the given IDs from cache.
* @param mixed $ids
*/
public function destroy($ids, string $class): bool
public function destroy(iterable $ids, string $class): bool
{
/** @var Model $instance */
$instance = new $class();
Expand Down Expand Up @@ -269,4 +277,13 @@ protected function getPrefix(string $connection): string
{
return (string) $this->container->get(ConfigInterface::class)->get('databases.' . $connection . '.prefix');
}

protected function defaultValue(mixed $handler, mixed $primaryValue): array
{
if ($handler instanceof DefaultValueInterface) {
return $handler->defaultValue($primaryValue);
}

return [];
}
}
25 changes: 24 additions & 1 deletion src/model-cache/tests/Handler/RedisHandlerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use DateInterval;
use Hyperf\Context\ApplicationContext;
use Hyperf\ModelCache\Config;
use Hyperf\ModelCache\Handler\DefaultValueInterface;
use Hyperf\ModelCache\Handler\HandlerInterface;
use Hyperf\ModelCache\Handler\RedisHandler;
use Hyperf\Redis\RedisProxy;
Expand Down Expand Up @@ -83,7 +84,29 @@ public function testGetMultiple()
$result[] = $item;
}

$this->assertSame($result, $handler->getMultiple($keys));
$data = $handler->getMultiple($keys);
if ($handler instanceof DefaultValueInterface) {
foreach ($data as $i => $value) {
$data[$i] = $handler->clearDefaultValue($value);
}
}
$this->assertSame($result, $data);
}

public function testDefaultValue()
{
$handler = $this->mockHandler();
if (! $handler instanceof DefaultValueInterface) {
$this->markTestSkipped('Don\'t implements DefaultValueInterface');
}

$data = $handler->defaultValue(1);
$this->assertSame(['HF-DATA' => 1], $data);

$this->assertTrue($handler->isDefaultValue($data));
$this->assertFalse($handler->isDefaultValue(['HF-DATA' => 1, 'id' => 1]));
$this->assertSame(3, $handler->getPrimaryValue(['HF-DATA' => 3]));
$this->assertSame([], $handler->clearDefaultValue(['HF-DATA' => 3]));
}

protected function mockHandler(): HandlerInterface
Expand Down
25 changes: 24 additions & 1 deletion src/model-cache/tests/ModelCacheTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,11 @@
namespace HyperfTest\ModelCache;

use DateInterval;
use Hyperf\Database\Events\QueryExecuted;
use Hyperf\Database\Model\Relations\Relation;
use Hyperf\DbConnection\Db;
use Hyperf\DbConnection\Listener\InitTableCollectorListener;
use Hyperf\Engine\Channel;
use Hyperf\ModelCache\EagerLoad\EagerLoader;
use Hyperf\ModelCache\InvalidCacheManager;
use Hyperf\ModelCache\Listener\EagerLoadListener;
Expand All @@ -40,6 +42,16 @@
*/
class ModelCacheTest extends TestCase
{
/**
* @var array
*/
protected $channel;

protected function setUp(): void
{
$this->channel = new Channel(999);
}

protected function tearDown(): void
{
Mockery::close();
Expand Down Expand Up @@ -169,7 +181,9 @@ public function testFindNullBeforeCreate()

public function testFindManyNullBeforeCreate()
{
$container = ContainerStub::mockContainer();
$container = ContainerStub::mockContainer(listenQueryExecuted: function (QueryExecuted $executed) {
$this->channel->push($executed);
});

$id = 207;

Expand All @@ -180,6 +194,15 @@ public function testFindManyNullBeforeCreate()
$this->assertSame(0, $models->count());

$this->assertEquals(1, $redis->del('{mc:default:m:user}:id:' . $id));
$models = UserModel::findManyFromCache([$id]);
$models = UserModel::findManyFromCache([$id]);
$models = UserModel::findManyFromCache([$id]);
$models = UserModel::findManyFromCache([$id]);
$models = UserModel::findManyFromCache([$id]);
$models = UserModel::findManyFromCache([$id]);

$this->assertLessThanOrEqual(2, $this->channel->length());

UserModel::query(true)->where('id', $id)->delete();
}

Expand Down
6 changes: 5 additions & 1 deletion src/model-cache/tests/Stub/ContainerStub.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
use Hyperf\Database\ConnectionResolverInterface;
use Hyperf\Database\Connectors\ConnectionFactory;
use Hyperf\Database\Connectors\MySqlConnector;
use Hyperf\Database\Events\QueryExecuted;
use Hyperf\Database\Events\TransactionCommitted;
use Hyperf\Database\Model\Events\Deleted;
use Hyperf\Database\Model\Events\Saved;
Expand Down Expand Up @@ -50,7 +51,7 @@

class ContainerStub
{
public static function mockContainer($ttl = 86400)
public static function mockContainer($ttl = 86400, ?callable $listenQueryExecuted = null)
{
$container = Mockery::mock(Container::class);
$container->shouldReceive('get')->with(TableCollector::class)->andReturn(new TableCollector());
Expand Down Expand Up @@ -133,6 +134,9 @@ public static function mockContainer($ttl = 86400)
$provider->on(TransactionCommitted::class, [new DeleteCacheInTransactionListener(), 'process']);
$provider->on(Saved::class, [$listener, 'process']);
$provider->on(Deleted::class, [$listener, 'process']);
if ($listenQueryExecuted) {
$provider->on(QueryExecuted::class, $listenQueryExecuted);
}
$eventDispatcher = new EventDispatcher($provider, $logger);
$container->shouldReceive('get')->with(EventDispatcherInterface::class)->andReturn($eventDispatcher);

Expand Down