Mirrors/koel
2
0
Fork 0
mirror of https://github.com/koel/koel synced 2024-11-24 05:03:05 +00:00
Code Issues Projects Releases Packages Wiki Activity

Revise the smart playlist rule

Browse source
This commit is contained in:
Phan An 2018-11-18 22:50:03 +01:00
parent 26caca38b9
commit d45948e1bd
17 changed files with 367 additions and 331 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

36
app/Models/Rule.php
View file

@ -8,14 +8,6 @@ use InvalidArgumentException;
class Rule
{
private const LOGIC_OR = 'or';
private const LOGIC_AND = 'and';
private const VALID_LOGICS = [
self::LOGIC_AND,
self::LOGIC_OR,
];
public const OPERATOR_IS = 'is';
public const OPERATOR_IS_NOT = 'isNot';
public const OPERATOR_CONTAINS = 'contains';
@ -43,17 +35,14 @@ class Rule
];
private $operator;
private $logic;
private $value;
private $model;
private $parameterFactory;
private function __construct(array $config)
{
$this->validateLogic($config['logic']);
$this->validateOperator($config['operator']);
$this->logic = $config['logic'];
$this->value = $config['value'];
$this->model = $config['model'];
$this->operator = $config['operator'];
@ -83,31 +72,18 @@ class Rule
// If the model is something like 'artist.name' or 'interactions.play_count', we have a subquery to deal with.
// We handle such a case with a recursive call which, in theory, should work with an unlimited level of nesting,
// though in practice we only have one level max.
$subQueryLogic = self::LOGIC_AND ? 'whereHas' : 'orWhereHas';
return $query->$subQueryLogic($fragments[0], function (Builder $subQuery) use ($fragments): Builder {
return $query->whereHas($fragments[0], function (Builder $subQuery) use ($fragments): Builder {
return $this->build($subQuery, $fragments[1]);
});
}
/**
* Resolve the logic of a (sub)query base on the configured operator.
* Basically, if the operator is "between," we use "whereBetween." Otherwise, it's "where." Simple.
*/
private function resolveLogic(): string
{
if ($this->operator === self::OPERATOR_IS_BETWEEN) {
return $this->logic === self::LOGIC_AND ? 'whereBetween' : 'orWhereBetween';
}
return $this->logic === self::LOGIC_AND ? 'where' : 'orWhere';
}
private function validateLogic(string $logic): void
{
if (!in_array($logic, self::VALID_LOGICS, true)) {
throw new InvalidArgumentException(
sprintf(
'%s is not a valid value for logic. Valid values are: %s', $logic, implode(', ', self::VALID_LOGICS)
)
);
}
return $this->operator === self::OPERATOR_IS_BETWEEN ? 'whereBetween' : 'where';
}
private function validateOperator(string $operator): void

39
app/Services/SmartPlaylistService.php
View file

@ -30,18 +30,22 @@ class SmartPlaylistService
$rules = $this->addRequiresUserRules($playlist->rules, $playlist->user);
return $this->buildQueryForRules($rules)->get();
return $this->buildQueryFromRules($rules)->get();
}
public function buildQueryForRules(array $rules): Builder
public function buildQueryFromRules(array $rules): Builder
{
return tap(Song::query(), static function (Builder $query) use ($rules): Builder {
foreach ($rules as $config) {
$query = Rule::create($config)->build($query);
}
$query = Song::query();
return $query;
collect($rules)->each(static function (array $ruleGroup) use ($query): void {
$query->orWhere(static function (Builder $subQuery) use ($ruleGroup): void {
foreach ($ruleGroup['rules'] as $config) {
Rule::create($config)->build($subQuery);
}
});
});
return $query;
}
/**
@ -51,26 +55,29 @@ class SmartPlaylistService
*
* @param string[] $rules
*/
private function addRequiresUserRules(array $rules, User $user): array
public function addRequiresUserRules(array $rules, User $user): array
{
$additionalRules = [];
foreach ($rules as &$ruleGroup) {
$additionalRules = [];
foreach ($rules as $rule) {
foreach (self::RULE_REQUIRES_USER_PREFIXES as $modelPrefix) {
if (starts_with($rule['model'], $modelPrefix)) {
$additionalRules[] = $this->createRequireUserRule($user, $modelPrefix);
foreach ($ruleGroup['rules'] as &$config) {
foreach (self::RULE_REQUIRES_USER_PREFIXES as $modelPrefix) {
if (starts_with($config['model'], $modelPrefix)) {
$additionalRules[] = $this->createRequireUserRule($user, $modelPrefix);
}
}
}
// make sure all those additional rules are unique.
$ruleGroup['rules'] = array_merge($ruleGroup['rules'], collect($additionalRules)->unique('model')->all());
}
// make sure all those additional rules are unique.
return array_merge($rules, collect($additionalRules)->unique('model')->all());
return $rules;
}
private function createRequireUserRule(User $user, string $modelPrefix): array
{
return [
'logic' => 'and',
'model' => $modelPrefix.'user_id',
'operator' => 'is',
'value' => [$user->id],

362
tests/Integration/Services/SmartPlaylistServiceTest.php
View file

@ -32,176 +32,78 @@ class SmartPlaylistServiceTest extends TestCase
parent::tearDown();
}
private function readFixtureFile(string $fileName): array
{
return json_decode(file_get_contents(__DIR__ . '/../../blobs/rules/' . $fileName), true);
}
public function provideRules(): array
{
return [
[
[
[
'logic' => 'and',
'model' => 'title',
'operator' => 'beginsWith',
'value' => ['Foo'],
],
],
'select * from "songs" where "title" LIKE ?',
$this->readFixtureFile('is.json'),
'select * from "songs" where ("title" = ?)',
['Foo'],
],
[
$this->readFixtureFile('isNot.json'),
'select * from "songs" where ("title" <> ?)',
['Foo'],
],
[
$this->readFixtureFile('contains.json'),
'select * from "songs" where ("title" LIKE ?)',
['%Foo%'],
],
[
$this->readFixtureFile('doesNotContain.json'),
'select * from "songs" where ("title" NOT LIKE ?)',
['%Foo%'],
],
[
$this->readFixtureFile('beginsWith.json'),
'select * from "songs" where ("title" LIKE ?)',
['Foo%'],
],
[
[
[
'logic' => 'and',
'model' => 'title',
'operator' => 'endsWith',
'value' => ['Foo'],
],
],
'select * from "songs" where "title" LIKE ?',
$this->readFixtureFile('endsWith.json'),
'select * from "songs" where ("title" LIKE ?)',
['%Foo'],
],
[
[
[
'logic' => 'or',
'model' => 'title',
'operator' => 'is',
'value' => ['Foo'],
],
],
'select * from "songs" where "title" = ?',
['Foo'],
$this->readFixtureFile('isBetween.json'),
'select * from "songs" where ("bit_rate" between ? and ?)',
['192', '256'],
],
[
[
[
'logic' => 'or',
'model' => 'title',
'operator' => 'isNot',
'value' => ['Foo'],
],
],
'select * from "songs" where "title" <> ?',
['Foo'],
],
[
[
[
'logic' => 'and',
'model' => 'title',
'operator' => 'contains',
'value' => ['Foo'],
],
],
'select * from "songs" where "title" LIKE ?',
['%Foo%'],
],
[
[
[
'logic' => 'and',
'model' => 'title',
'operator' => 'notContain',
'value' => ['Foo'],
],
],
'select * from "songs" where "title" NOT LIKE ?',
['%Foo%'],
],
[
[
[
'logic' => 'and',
'model' => 'length',
'operator' => 'isGreaterThan',
'value' => [100],
],
],
'select * from "songs" where "length" > ?',
[100],
],
[
[
[
'logic' => 'and',
'model' => 'length',
'operator' => 'isLessThan',
'value' => [100],
],
],
'select * from "songs" where "length" < ?',
[100],
],
[
[
[
'logic' => 'and',
'model' => 'length',
'operator' => 'isBetween',
'value' => [100, 200],
],
],
'select * from "songs" where "length" between ? and ?',
[100, 200],
],
[
[
[
'logic' => 'and',
'model' => 'created_at',
'operator' => 'inLast',
'value' => [7],
],
],
'select * from "songs" where "created_at" >= ?',
$this->readFixtureFile('inLast.json'),
'select * from "songs" where ("created_at" >= ?)',
['2018-07-08 00:00:00'],
],
[
[
[
'logic' => 'and',
'model' => 'created_at',
'operator' => 'notInLast',
'value' => [7],
],
],
'select * from "songs" where "created_at" < ?',
$this->readFixtureFile('notInLast.json'),
'select * from "songs" where ("created_at" < ?)',
['2018-07-08 00:00:00'],
],
[
[
[
'logic' => 'and',
'model' => 'created_at',
'operator' => 'notInLast',
'value' => [7],
],
[
'logic' => 'or',
'model' => 'length',
'operator' => 'isBetween',
'value' => [100, 200],
],
],
'select * from "songs" where "created_at" < ? or "length" between ? and ?',
['2018-07-08 00:00:00', 100, 200],
$this->readFixtureFile('isLessThan.json'),
'select * from "songs" where ("length" < ?)',
['300'],
],
[
[
[
'logic' => 'or',
'model' => 'artist.name',
'operator' => 'isNot',
'value' => ['Foo'],
],
[
'logic' => 'and',
'model' => 'created_at',
'operator' => 'notInLast',
'value' => [7],
],
],
'select * from "songs" where exists (select * from "artists" where "songs"."artist_id" = "artists"."id" or ("name" <> ?)) and "created_at" < ?',
['Foo', '2018-07-08 00:00:00', 100, 200],
$this->readFixtureFile('is and isNot.json'),
'select * from "songs" where ("title" = ? and exists (select * from "artists" where "songs"."artist_id" = "artists"."id" and "name" <> ?))',
['Foo', 'Bar'],
],
[
$this->readFixtureFile('(is and isNot) or (is and isGreaterThan).json'),
'select * from "songs" where ("title" = ? and exists (select * from "albums" where "songs"."album_id" = "albums"."id" and "name" <> ?)) or ("genre" = ? and "bit_rate" > ?)',
['Foo', 'Bar', 'Metal', '128'],
],
[
$this->readFixtureFile('is or is.json'),
'select * from "songs" where ("title" = ?) or (exists (select * from "artists" where "songs"."artist_id" = "artists"."id" and "name" = ?))',
['Foo', 'Bar'],
],
];
}
@ -214,7 +116,7 @@ class SmartPlaylistServiceTest extends TestCase
*/
public function testBuildQueryForRules(array $rules, string $sql, array $bindings): void
{
$query = $this->service->buildQueryForRules($rules);
$query = $this->service->buildQueryFromRules($rules);
$this->assertSame($sql, $query->toSql());
$queryBinding = $query->getBindings();
@ -226,146 +128,36 @@ class SmartPlaylistServiceTest extends TestCase
}
}
public function testAddRequiresUserRules(): void
{
$rules = $this->readFixtureFile('requiresUser.json');
/** @var User $user */
$user = factory(User::class)->create();
$this->assertEquals([
'model' => 'interactions.user_id',
'operator' => 'is',
'value' => [$user->id],
], $this->service->addRequiresUserRules($rules, $user)[0]['rules'][1]);
}
public function testAllOperatorsAreCovered(): void
{
$operators = collect($this->provideRules())->map(static function (array $providedRules): string {
return $providedRules[0][0]['operator'];
$rules = collect($this->provideRules())->map(static function (array $providedRule): array {
return $providedRule[0];
});
$this->assertSame(count(Rule::VALID_OPERATORS), $operators->unique()->count());
}
$operators = [];
public function testRuleWithoutSubQueries(): void
{
/** @var User $user */
$user = factory(User::class)->create();
factory(Song::class, 2)->create([
'title' => static function (): string {
return 'Unique Foo '.uniqid();
},
]);
factory(Song::class, 5)->create([
'title' => 'Has nothing to do with the rule',
]);
$playlist = factory(Playlist::class)->create([
'user_id' => $user->id,
'rules' => [
[
'logic' => 'and',
'model' => 'title',
'operator' => 'beginsWith',
'value' => ['Unique Foo'],
],
],
]);
$songs = $this->service->getSongs($playlist);
$this->assertCount(2, $songs);
$songs->each(function (Song $song): void {
$this->assertStringStartsWith('Unique Foo', $song->title);
});
}
public function testRulesWithSubQuery(): void
{
/** @var User $user */
$user = factory(User::class)->create();
/** @var Album[] $albums */
$albums = factory(Album::class, 3)->create([
'artist_id' => static function (): int {
return factory(Artist::class)->create([
'name' => static function (): string {
return 'Foo Artist '.uniqid();
},
])->id;
},
]);
foreach ($albums as $album) {
factory(Song::class, 2)->create([
'album_id' => $album->id,
'artist_id' => $album->artist->id,
]);
foreach ($rules as $rule) {
foreach ($rule as $ruleGroup) {
foreach ($ruleGroup['rules'] as $config) {
$operators[] = $config['operator'];
}
}
}
/** @var Album $inApplicableAlbum */
$inApplicableAlbum = factory(Album::class)->create([
'artist_id' => factory(Artist::class)->create(['name' => 'Nothing to do with the rule']),
]);
factory(Song::class, 1)->create([
'album_id' => $inApplicableAlbum->id,
]);
$playlist = factory(Playlist::class)->create([
'user_id' => $user->id,
'rules' => [
[
'logic' => 'and',
'model' => 'artist.name',
'operator' => 'beginsWith',
'value' => ['Foo Artist'],
],
[
'logic' => 'or',
'model' => 'title',
'operator' => 'beginsWith',
'value' => ['There should not be anything like this'],
],
],
]);
$songs = $this->service->getSongs($playlist);
$this->assertCount(6, $songs);
$songs->each(function (Song $song): void {
$this->assertStringStartsWith('Foo Artist', $song->artist->name);
});
}
public function testRulesWithUser(): void
{
/** @var User $bob */
$bob = factory(User::class)->create();
/** @var User $alice */
$alice = factory(User::class)->create();
$bobSong = factory(Song::class)->create([
'title' => 'Song for Bob',
]);
factory(Interaction::class)->create([
'user_id' => $bob->id,
'song_id' => $bobSong->id,
'play_count' => 10,
]);
$aliceSong = factory(Song::class)->create([
'title' => 'Song for Alice',
]);
factory(Interaction::class)->create([
'user_id' => $alice->id,
'song_id' => $aliceSong->id,
'play_count' => 12,
]);
$playlist = factory(Playlist::class)->create([
'user_id' => $bob->id,
'rules' => [
[
'logic' => 'and',
'model' => 'interactions.play_count',
'operator' => 'isGreaterThan',
'value' => [5],
],
],
]);
/** @var Song[]|Collection $songs */
$songs = $this->service->getSongs($playlist);
$this->assertCount(1, $songs);
$this->assertSame('Song for Bob', $songs[0]->title);
$this->assertSame(count(Rule::VALID_OPERATORS), count(array_unique($operators)));
}
}

44
tests/blobs/rules/(is and isNot) or (is and isGreaterThan).json Normal file
View file

@ -0,0 +1,44 @@
[
{
"id": 1541356285385,
"rules": [
{
"id": 1541356313181,
"model": "title",
"operator": "is",
"value": [
"Foo"
]
},
{
"id": 1542573964152,
"model": "album.name",
"operator": "isNot",
"value": [
"Bar"
]
}
]
},
{
"id": 1542573971423,
"rules": [
{
"id": 1542573972485,
"model": "genre",
"operator": "is",
"value": [
"Metal"
]
},
{
"id": 1542573978609,
"model": "bit_rate",
"operator": "isGreaterThan",
"value": [
"128"
]
}
]
}
]

15
tests/blobs/rules/beginsWith.json Normal file
View file

@ -0,0 +1,15 @@
[
{
"id": 1541356285385,
"rules": [
{
"id": 1541356313181,
"model": "title",
"operator": "beginsWith",
"value": [
"Foo"
]
}
]
}
]

15
tests/blobs/rules/contains.json Normal file
View file

@ -0,0 +1,15 @@
[
{
"id": 1541356285385,
"rules": [
{
"id": 1541356313181,
"model": "title",
"operator": "contains",
"value": [
"Foo"
]
}
]
}
]

15
tests/blobs/rules/doesNotContain.json Normal file
View file

@ -0,0 +1,15 @@
[
{
"id": 1541356285385,
"rules": [
{
"id": 1541356313181,
"model": "title",
"operator": "notContain",
"value": [
"Foo"
]
}
]
}
]

15
tests/blobs/rules/endsWith.json Normal file
View file

@ -0,0 +1,15 @@
[
{
"id": 1541356285385,
"rules": [
{
"id": 1541356313181,
"model": "title",
"operator": "endsWith",
"value": [
"Foo"
]
}
]
}
]

15
tests/blobs/rules/inLast.json Normal file
View file

@ -0,0 +1,15 @@
[
{
"id": 1541356285385,
"rules": [
{
"id": 1541356313181,
"model": "created_at",
"operator": "inLast",
"value": [
"7"
]
}
]
}
]

23
tests/blobs/rules/is and isNot.json Normal file
View file

@ -0,0 +1,23 @@
[
{
"id": 1541356285385,
"rules": [
{
"id": 1541356313181,
"model": "title",
"operator": "is",
"value": [
"Foo"
]
},
{
"id": 1542572962736,
"model": "artist.name",
"operator": "isNot",
"value": [
"Bar"
]
}
]
}
]

28
tests/blobs/rules/is or is.json Normal file
View file

@ -0,0 +1,28 @@
[
{
"id": 1541356285385,
"rules": [
{
"id": 1541356313181,
"model": "title",
"operator": "is",
"value": [
"Foo"
]
}
]
},
{
"id": 1542573097361,
"rules": [
{
"id": 1542573098295,
"model": "artist.name",
"operator": "is",
"value": [
"Bar"
]
}
]
}
]

15
tests/blobs/rules/is.json Normal file
View file

@ -0,0 +1,15 @@
[
{
"id": 1541356285385,
"rules": [
{
"id": 1541356313181,
"model": "title",
"operator": "is",
"value": [
"Foo"
]
}
]
}
]

16
tests/blobs/rules/isBetween.json Normal file
View file

@ -0,0 +1,16 @@
[
{
"id": 1541356285385,
"rules": [
{
"id": 1541356313181,
"model": "bit_rate",
"operator": "isBetween",
"value": [
"192",
"256"
]
}
]
}
]

15
tests/blobs/rules/isLessThan.json Normal file
View file

@ -0,0 +1,15 @@
[
{
"id": 1541356285385,
"rules": [
{
"id": 1541356313181,
"model": "length",
"operator": "isLessThan",
"value": [
"300"
]
}
]
}
]

15
tests/blobs/rules/isNot.json Normal file
View file

@ -0,0 +1,15 @@
[
{
"id": 1541356285385,
"rules": [
{
"id": 1541356313181,
"model": "title",
"operator": "isNot",
"value": [
"Foo"
]
}
]
}
]

15
tests/blobs/rules/notInLast.json Normal file
View file

@ -0,0 +1,15 @@
[
{
"id": 1541356285385,
"rules": [
{
"id": 1541356313181,
"model": "created_at",
"operator": "notInLast",
"value": [
"7"
]
}
]
}
]

15
tests/blobs/rules/requiresUser.json Normal file
View file

@ -0,0 +1,15 @@
[
{
"id": 1541356285385,
"rules": [
{
"id": 1541356313181,
"model": "interactions.play_count",
"operator": "is",
"value": [
"42"
]
}
]
}
]