Mirrors/koel
2
0
Fork 0
mirror of https://github.com/koel/koel synced 2024-11-27 22:40:26 +00:00
Code Issues Projects Releases Packages Wiki Activity

feat(smart-playlist): validate smart playlist request (#1366)

Browse source
This commit is contained in:
Phan An 2021-10-10 20:05:51 +02:00 committed by GitHub
parent e2484e7791
commit 30f4878ec3
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 321 additions and 50 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
app/Casts/SmartPlaylistRulesCast.php
View file

@ -12,7 +12,7 @@ class SmartPlaylistRulesCast implements CastsAttributes
public function get($model, string $key, $value, array $attributes): Collection
{
return collect(json_decode($value, true) ?: [])->map(static function (array $group): ?SmartPlaylistRuleGroup {
return SmartPlaylistRuleGroup::create($group);
return SmartPlaylistRuleGroup::tryCreate($group);
});
}

4
app/Http/Controllers/API/PlaylistController.php
View file

@ -4,13 +4,13 @@ namespace App\Http\Controllers\API;
use App\Http\Requests\API\PlaylistStoreRequest;
use App\Http\Requests\API\PlaylistSyncRequest;
use App\Http\Requests\API\PlaylistUpdateRequest;
use App\Models\Playlist;
use App\Models\User;
use App\Repositories\PlaylistRepository;
use App\Services\PlaylistService;
use App\Services\SmartPlaylistService;
use Illuminate\Contracts\Auth\Authenticatable;
use Illuminate\Http\Request;
class PlaylistController extends Controller
{
@ -52,7 +52,7 @@ class PlaylistController extends Controller
return response()->json($playlist);
}
public function update(Request $request, Playlist $playlist)
public function update(PlaylistUpdateRequest $request, Playlist $playlist)
{
$this->authorize('owner', $playlist);

6
app/Http/Requests/API/PlaylistStoreRequest.php
View file

@ -2,6 +2,10 @@
namespace App\Http\Requests\API;
use App\Models\Song;
use App\Rules\ValidSmartPlaylistRulePayload;
use Illuminate\Validation\Rule;
/**
* @property array<string> $songs
* @property string $name
@ -15,6 +19,8 @@ class PlaylistStoreRequest extends Request
return [
'name' => 'required',
'songs' => 'array',
'songs.*' => [Rule::exists(Song::class, 'id')],
'rules' => ['array', 'nullable', new ValidSmartPlaylistRulePayload()],
];
}
}

4
app/Http/Requests/API/PlaylistSyncRequest.php
View file

@ -2,6 +2,9 @@
namespace App\Http\Requests\API;
use App\Models\Song;
use Illuminate\Validation\Rule;
/**
* @property array<string> $songs
*/
@ -12,6 +15,7 @@ class PlaylistSyncRequest extends Request
{
return [
'songs' => 'present|array',
'songs.*' => [Rule::exists(Song::class, 'id')],
];
}
}

17
app/Http/Requests/API/PlaylistUpdateRequest.php Normal file
View file

@ -0,0 +1,17 @@
<?php
namespace App\Http\Requests\API;
use App\Rules\ValidSmartPlaylistRulePayload;
class PlaylistUpdateRequest extends Request
{
/** @return array<mixed> */
public function rules(): array
{
return [
'name' => 'required',
'rules' => ['array', 'nullable', new ValidSmartPlaylistRulePayload()],
];
}
}

31
app/Rules/ValidSmartPlaylistRulePayload.php Normal file
View file

@ -0,0 +1,31 @@
<?php
namespace App\Rules;
use App\Values\SmartPlaylistRule;
use Illuminate\Contracts\Validation\Rule;
use Throwable;
class ValidSmartPlaylistRulePayload implements Rule
{
/** @param array $value */
public function passes($attribute, $value): bool
{
try {
foreach ((array) $value as $ruleGroupConfig) {
foreach ($ruleGroupConfig['rules'] as $rule) {
SmartPlaylistRule::assertConfig($rule, false);
}
}
return true;
} catch (Throwable $e) {
return false;
}
}
public function message(): string
{
return 'Invalid smart playlist rules';
}
}

36
app/Values/SmartPlaylistRule.php
View file

@ -33,6 +33,27 @@ final class SmartPlaylistRule implements Arrayable
self::OPERATOR_NOT_IN_LAST,
];
public const MODEL_TITLE = 'title';
public const MODEL_ALBUM_NAME = 'album.name';
public const MODEL_ARTIST_NAME = 'artist.name';
public const MODEL_PLAY_COUNT = 'interactions.play_count';
public const MODEL_LAST_PLAYED = 'interactions.updated_at';
public const MODEL_USER_ID = 'interactions.user_id';
public const MODEL_LENGTH = 'length';
public const MODEL_DATE_ADDED = 'created_at';
public const MODEL_DATE_MODIFIED = 'updated_at';
public const VALID_MODELS = [
self::MODEL_TITLE,
self::MODEL_ALBUM_NAME,
self::MODEL_ARTIST_NAME,
self::MODEL_PLAY_COUNT,
self::MODEL_LAST_PLAYED,
self::MODEL_LENGTH,
self::MODEL_DATE_ADDED,
self::MODEL_DATE_MODIFIED,
];
public ?int $id;
public string $operator;
public array $value;
@ -40,7 +61,7 @@ final class SmartPlaylistRule implements Arrayable
private function __construct(array $config)
{
Assert::oneOf($config['operator'], self::VALID_OPERATORS);
self::assertConfig($config);
$this->id = $config['id'] ?? null;
$this->value = $config['value'];
@ -48,9 +69,20 @@ final class SmartPlaylistRule implements Arrayable
$this->operator = $config['operator'];
}
public static function assertConfig(array $config, bool $allowUserIdModel = true): void
{
Assert::oneOf($config['operator'], self::VALID_OPERATORS);
Assert::oneOf(
$config['model'],
$allowUserIdModel ? array_prepend(self::VALID_MODELS, self::MODEL_USER_ID) : self::VALID_MODELS
);
Assert::isArray($config['value']);
Assert::countBetween($config['value'], 1, 2);
}
public static function create(array $config): self
{
return new static($config);
return new self($config);
}
/** @return array<mixed> */

24
app/Values/SmartPlaylistRuleGroup.php
View file

@ -13,23 +13,27 @@ final class SmartPlaylistRuleGroup implements Arrayable
/** @var Collection|array<SmartPlaylistRule> */
public Collection $rules;
public static function create(array $jsonArray): ?self
public static function tryCreate(array $jsonArray): ?self
{
$group = new self();
try {
$group->id = $jsonArray['id'] ?? null;
$group->rules = collect(array_map(static function (array $rawRuleConfig) {
return SmartPlaylistRule::create($rawRuleConfig);
}, $jsonArray['rules']));
return $group;
return self::create($jsonArray);
} catch (Throwable $exception) {
return null;
}
}
public static function create(array $jsonArray): self
{
$group = new self();
$group->id = $jsonArray['id'] ?? null;
$group->rules = collect(array_map(static function (array $rawRuleConfig) {
return SmartPlaylistRule::create($rawRuleConfig);
}, $jsonArray['rules']));
return $group;
}
/** @return array<mixed> */
public function toArray(): array
{

31
tests/Feature/PlaylistTest.php
View file

@ -74,20 +74,22 @@ class PlaylistTest extends TestCase
public function testCreatingSmartPlaylistIgnoresSongs(): void
{
/** @var User $user */
$user = User::factory()->create();
$this->postAsUser('api/playlist', [
'name' => 'Smart Foo Bar',
'rules' => [
SmartPlaylistRule::create([
'model' => 'artist.name',
'operator' => SmartPlaylistRule::OPERATOR_IS,
'value' => ['Bob Dylan'],
])->toArray(),
[
'id' => 12345,
'rules' => [
SmartPlaylistRule::create([
'model' => 'artist.name',
'operator' => SmartPlaylistRule::OPERATOR_IS,
'value' => ['Bob Dylan'],
])->toArray(),
],
],
],
'songs' => Song::orderBy('id')->take(3)->get()->pluck('id')->all(),
], $user);
]);
/** @var Playlist $playlist */
$playlist = Playlist::orderBy('id', 'desc')->first();
@ -96,6 +98,17 @@ class PlaylistTest extends TestCase
self::assertEmpty($playlist->songs);
}
public function testCreatingPlaylistWithNonExistentSongsFails(): void
{
$response = $this->postAsUser('api/playlist', [
'name' => 'Foo Bar',
'rules' => [],
'songs' => ['foo'],
]);
$response->assertUnprocessable();
}
public function testUpdatePlaylistName(): void
{
/** @var User $user */

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

@ -32,67 +32,67 @@ class SmartPlaylistServiceTest extends TestCase
public function provideRuleConfigs(): array
{
return [
[
'is' => [
$this->readFixtureFile('is.json'),
'select * from "songs" where ("title" = ?)',
['Foo'],
],
[
'is not' => [
$this->readFixtureFile('isNot.json'),
'select * from "songs" where ("title" <> ?)',
['Foo'],
],
[
'contains' => [
$this->readFixtureFile('contains.json'),
'select * from "songs" where ("title" LIKE ?)',
['%Foo%'],
],
[
'does not contain' => [
$this->readFixtureFile('doesNotContain.json'),
'select * from "songs" where ("title" NOT LIKE ?)',
['%Foo%'],
],
[
'begins with' => [
$this->readFixtureFile('beginsWith.json'),
'select * from "songs" where ("title" LIKE ?)',
['Foo%'],
],
[
'ends with' => [
$this->readFixtureFile('endsWith.json'),
'select * from "songs" where ("title" LIKE ?)',
['%Foo'],
],
[
'is between' => [
$this->readFixtureFile('isBetween.json'),
'select * from "songs" where ("bit_rate" between ? and ?)',
['192', '256'],
'select * from "songs" where ("created_at" between ? and ?)',
['2021.10.01', '2021.11.01'],
],
[
'in last' => [
$this->readFixtureFile('inLast.json'),
'select * from "songs" where ("created_at" >= ?)',
['2018-07-08 00:00:00'],
],
[
'not in last' => [
$this->readFixtureFile('notInLast.json'),
'select * from "songs" where ("created_at" < ?)',
['2018-07-08 00:00:00'],
],
[
'is less than' => [
$this->readFixtureFile('isLessThan.json'),
'select * from "songs" where ("length" < ?)',
['300'],
],
[
'is and is not' => [
$this->readFixtureFile('is and isNot.json'),
'select * from "songs" where ("title" = ? and exists (select * from "artists" where "songs"."artist_id" = "artists"."id" and "name" <> ?))', // @phpcs-ignore-line
['Foo', 'Bar'],
],
[
'(is and is not) or (is and is greater than)' => [
$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" > ?)', // @phpcs-ignore-line
['Foo', 'Bar', 'Metal', '128'],
'select * from "songs" where ("title" = ? and exists (select * from "albums" where "songs"."album_id" = "albums"."id" and "name" <> ?)) or ("title" = ? and "created_at" > ?)', // @phpcs-ignore-line
['Foo', 'Bar', 'Baz', '2021.10.01'],
],
[
'is or is' => [
$this->readFixtureFile('is or is.json'),
'select * from "songs" where ("title" = ?) or (exists (select * from "artists" where "songs"."artist_id" = "artists"."id" and "name" = ?))', // @phpcs-ignore-line
['Foo', 'Bar'],
@ -104,7 +104,7 @@ class SmartPlaylistServiceTest extends TestCase
public function testBuildQueryForRules(array $rawRules, string $sql, array $bindings): void
{
$ruleGroups = collect($rawRules)->map(static function (array $group): SmartPlaylistRuleGroup {
return SmartPlaylistRuleGroup::create($group);
return SmartPlaylistRuleGroup::tryCreate($group);
});
$query = $this->service->buildQueryFromRules($ruleGroups);
@ -123,7 +123,7 @@ class SmartPlaylistServiceTest extends TestCase
{
$ruleGroups = collect($this->readFixtureFile('requiresUser.json'))->map(
static function (array $group): SmartPlaylistRuleGroup {
return SmartPlaylistRuleGroup::create($group);
return SmartPlaylistRuleGroup::tryCreate($group);
}
);

164
tests/Unit/Rules/ValidSmartPlaylistRulePayloadTest.php Normal file
View file

@ -0,0 +1,164 @@
<?php
namespace Tests\Unit\Rules;
use App\Rules\ValidSmartPlaylistRulePayload;
use Tests\TestCase;
class ValidSmartPlaylistRulePayloadTest extends TestCase
{
/** @return array<mixed> */
public function provideInvalidPayloads(): array
{
return [
'invalid format' => ['foo'],
'invalid model' => [
[
[
'rules' => [
[
'model' => 'foo',
'operator' => 'like',
'value' => ['bar'],
],
],
],
],
],
'invalid operator' => [
[
[
'rules' => [
[
'model' => 'artist.name',
'operator' => '<script>',
'value' => ['bar'],
],
],
],
],
],
'values are not an array' => [
[
[
'rules' => [
[
'model' => 'artist.name',
'operator' => 'is',
'value' => 'bar',
],
],
],
],
],
'values are empty' => [
[
[
'rules' => [
[
'model' => 'artist.name',
'operator' => 'is',
'value' => [],
],
],
],
],
],
'values item account exceeds 2' => [
[
[
'rules' => [
[
'model' => 'artist.name',
'operator' => 'is',
'value' => ['bar', 'baz', 'qux'],
],
],
],
],
],
];
}
/** @dataProvider provideInvalidPayloads */
public function testInvalidCases($value): void
{
self::assertFalse((new ValidSmartPlaylistRulePayload())->passes('rules', $value));
}
/** @return array<mixed> */
public function provideValidPayloads(): array
{
return [
'one rule' => [
[
[
'rules' => [
[
'model' => 'artist.name',
'operator' => 'is',
'value' => ['bar'],
],
],
],
],
],
'multiple rules' => [
[
[
'rules' => [
[
'model' => 'artist.name',
'operator' => 'is',
'value' => ['bar'],
],
[
'model' => 'interactions.play_count',
'operator' => 'isGreaterThan',
'value' => [50],
],
],
],
],
],
'multiple groups' => [
[
[
'rules' => [
[
'model' => 'artist.name',
'operator' => 'is',
'value' => ['bar'],
],
[
'model' => 'interactions.play_count',
'operator' => 'isGreaterThan',
'value' => [50],
],
],
],
[
'rules' => [
[
'model' => 'album.name',
'operator' => 'contains',
'value' => ['bar'],
],
[
'model' => 'interactions.play_count',
'operator' => 'isBetween',
'value' => [10, 100],
],
],
],
],
],
];
}
/** @dataProvider provideValidPayloads */
public function testValidCases($value): void
{
self::assertTrue((new ValidSmartPlaylistRulePayload())->passes('rules', $value));
}
}

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

@ -25,18 +25,18 @@
"rules": [
{
"id": 1542573972485,
"model": "genre",
"model": "title",
"operator": "is",
"value": [
"Metal"
"Baz"
]
},
{
"id": 1542573978609,
"model": "bit_rate",
"model": "created_at",
"operator": "isGreaterThan",
"value": [
"128"
"2021.10.01"
]
}
]

6
tests/blobs/rules/isBetween.json
View file

@ -4,11 +4,11 @@
"rules": [
{
"id": 1541356313181,
"model": "bit_rate",
"model": "created_at",
"operator": "isBetween",
"value": [
"192",
"256"
"2021.10.01",
"2021.11.01"
]
}
]