From 712286151fdb3be8cf3d189e00f7b501dc97cf51 Mon Sep 17 00:00:00 2001
From: Alexander Skvortsov <38059171+askvortsov1@users.noreply.github.com>
Date: Tue, 4 May 2021 12:53:20 -0400
Subject: [PATCH] Optimize tag permissions (#126)
The new implementation generates a subquery of IDs instead of sending big arrays of data to/from the database. This massively speeds up performance.
---
extensions/tags/.github/workflows/test.yml | 78 ++++++
extensions/tags/composer.json | 28 +-
extensions/tags/src/Access/GlobalPolicy.php | 5 +-
.../src/Access/ScopeDiscussionVisibility.php | 4 +-
.../ScopeDiscussionVisibilityForAbility.php | 4 +-
.../tags/src/Access/ScopeFlagVisibility.php | 4 +-
.../tags/src/Access/ScopeTagVisibility.php | 4 +-
extensions/tags/src/Tag.php | 86 ++++--
extensions/tags/tests/fixtures/.gitkeep | 0
.../RetrievesRepresentativeTags.php | 30 ++
.../api/discussions/CreateTest.php | 265 ++++++++++++++++++
.../tests/integration/api/tags/CreateTest.php | 107 +++++++
.../integration/authorization/PolicyTest.php | 116 ++++++++
extensions/tags/tests/integration/setup.php | 16 ++
.../visibility/DiscussionVisibilityTest.php | 127 +++++++++
.../visibility/TagVisibilityTest.php | 99 +++++++
extensions/tags/tests/phpunit.integration.xml | 24 ++
extensions/tags/tests/phpunit.unit.xml | 27 ++
extensions/tags/tests/unit/.gitkeep | 0
19 files changed, 986 insertions(+), 38 deletions(-)
create mode 100644 extensions/tags/.github/workflows/test.yml
create mode 100644 extensions/tags/tests/fixtures/.gitkeep
create mode 100644 extensions/tags/tests/integration/RetrievesRepresentativeTags.php
create mode 100644 extensions/tags/tests/integration/api/discussions/CreateTest.php
create mode 100644 extensions/tags/tests/integration/api/tags/CreateTest.php
create mode 100644 extensions/tags/tests/integration/authorization/PolicyTest.php
create mode 100644 extensions/tags/tests/integration/setup.php
create mode 100644 extensions/tags/tests/integration/visibility/DiscussionVisibilityTest.php
create mode 100644 extensions/tags/tests/integration/visibility/TagVisibilityTest.php
create mode 100644 extensions/tags/tests/phpunit.integration.xml
create mode 100644 extensions/tags/tests/phpunit.unit.xml
create mode 100644 extensions/tags/tests/unit/.gitkeep
diff --git a/extensions/tags/.github/workflows/test.yml b/extensions/tags/.github/workflows/test.yml
new file mode 100644
index 000000000..d3cfc5a82
--- /dev/null
+++ b/extensions/tags/.github/workflows/test.yml
@@ -0,0 +1,78 @@
+name: Tests
+
+on: [push, pull_request]
+
+jobs:
+ test:
+ runs-on: ubuntu-latest
+
+ strategy:
+ matrix:
+ php: [7.3, 7.4, '8.0']
+ service: ['mysql:5.7', mariadb]
+ prefix: ['', flarum_]
+
+ include:
+ - service: 'mysql:5.7'
+ db: MySQL
+ - service: mariadb
+ db: MariaDB
+ - prefix: flarum_
+ prefixStr: (prefix)
+
+ exclude:
+ - php: 7.3
+ service: 'mysql:5.7'
+ prefix: flarum_
+ - php: 7.3
+ service: mariadb
+ prefix: flarum_
+ - php: 8.0
+ service: 'mysql:5.7'
+ prefix: flarum_
+ - php: 8.0
+ service: mariadb
+ prefix: flarum_
+
+ services:
+ mysql:
+ image: ${{ matrix.service }}
+ ports:
+ - 13306:3306
+
+ name: 'PHP ${{ matrix.php }} / ${{ matrix.db }} ${{ matrix.prefixStr }}'
+
+ steps:
+ - uses: actions/checkout@master
+
+ - name: Setup PHP
+ uses: shivammathur/setup-php@v2
+ with:
+ php-version: ${{ matrix.php }}
+ coverage: xdebug
+ extensions: curl, dom, gd, json, mbstring, openssl, pdo_mysql, tokenizer, zip
+ tools: phpunit, composer:v2
+
+ # The authentication alter is necessary because newer mysql versions use the `caching_sha2_password` driver,
+ # which isn't supported prior to PHP7.4
+ # When we drop support for PHP7.3, we should remove this from the setup.
+ - name: Create MySQL Database
+ run: |
+ sudo systemctl start mysql
+ mysql -uroot -proot -e 'CREATE DATABASE flarum_test;' --port 13306
+ mysql -uroot -proot -e "ALTER USER 'root'@'localhost' IDENTIFIED WITH mysql_native_password BY 'root';" --port 13306
+
+ - name: Install Composer dependencies
+ run: composer install
+
+ - name: Setup Composer tests
+ run: composer test:setup
+ env:
+ DB_PORT: 13306
+ DB_PASSWORD: root
+ DB_PREFIX: ${{ matrix.prefix }}
+
+ - name: Run Composer tests
+ run: composer test
+ env:
+ COMPOSER_PROCESS_TIMEOUT: 600
diff --git a/extensions/tags/composer.json b/extensions/tags/composer.json
index 87cc71e51..243eed013 100644
--- a/extensions/tags/composer.json
+++ b/extensions/tags/composer.json
@@ -2,7 +2,9 @@
"name": "flarum/tags",
"description": "Organize discussions into a hierarchy of tags and categories.",
"type": "flarum-extension",
- "keywords": ["discussion"],
+ "keywords": [
+ "discussion"
+ ],
"license": "MIT",
"support": {
"issues": "https://github.com/flarum/core/issues",
@@ -24,6 +26,11 @@
"Flarum\\Tags\\": "src/"
}
},
+ "autoload-dev": {
+ "psr-4": {
+ "Flarum\\Tags\\Tests\\": "tests/"
+ }
+ },
"extra": {
"branch-alias": {
"dev-master": "0.1.x-dev"
@@ -37,5 +44,24 @@
"color": "#fff"
}
}
+ },
+ "scripts": {
+ "test": [
+ "@test:unit",
+ "@test:integration"
+ ],
+ "test:unit": "phpunit -c tests/phpunit.unit.xml",
+ "test:integration": "phpunit -c tests/phpunit.integration.xml",
+ "test:setup": "@php tests/integration/setup.php"
+ },
+ "scripts-descriptions": {
+ "test": "Runs all tests.",
+ "test:unit": "Runs all unit tests.",
+ "test:integration": "Runs all integration tests.",
+ "test:setup": "Sets up a database for use with integration tests. Execute this only once."
+ },
+ "require-dev": {
+ "flarum/core": "0.1.x-dev",
+ "flarum/testing": "*@dev"
}
}
diff --git a/extensions/tags/src/Access/GlobalPolicy.php b/extensions/tags/src/Access/GlobalPolicy.php
index 1ce1a738c..94301e19d 100644
--- a/extensions/tags/src/Access/GlobalPolicy.php
+++ b/extensions/tags/src/Access/GlobalPolicy.php
@@ -37,9 +37,8 @@ class GlobalPolicy extends AbstractPolicy
if ($actor->hasPermission($ability) && $actor->hasPermission('bypassTagCounts')) {
return $this->allow();
}
-
- $enoughPrimary = count(Tag::getIdsWhereCan($actor, $ability, true, false)) >= $this->settings->get('flarum-tags.min_primary_tags');
- $enoughSecondary = count(Tag::getIdsWhereCan($actor, $ability, false, true)) >= $this->settings->get('flarum-tags.min_secondary_tags');
+ $enoughPrimary = Tag::queryIdsWhereCan(Tag::query()->getQuery(), $actor, $ability, true, false)->count() >= $this->settings->get('flarum-tags.min_primary_tags');
+ $enoughSecondary = Tag::queryIdsWhereCan(Tag::query()->getQuery(), $actor, $ability, false, true)->count() >= $this->settings->get('flarum-tags.min_secondary_tags');
if ($enoughPrimary && $enoughSecondary) {
return $this->allow();
diff --git a/extensions/tags/src/Access/ScopeDiscussionVisibility.php b/extensions/tags/src/Access/ScopeDiscussionVisibility.php
index d3bbc0765..7e004b8b4 100644
--- a/extensions/tags/src/Access/ScopeDiscussionVisibility.php
+++ b/extensions/tags/src/Access/ScopeDiscussionVisibility.php
@@ -27,7 +27,9 @@ class ScopeDiscussionVisibility
->whereNotIn('discussions.id', function ($query) use ($actor) {
return $query->select('discussion_id')
->from('discussion_tag')
- ->whereIn('tag_id', Tag::getIdsWhereCannot($actor, 'viewDiscussions'));
+ ->whereNotIn('tag_id', function ($query) use ($actor) {
+ Tag::queryIdsWhereCan($query->from('tags'), $actor, 'viewDiscussions');
+ });
})
->orWhere(function ($query) use ($actor) {
$query->whereVisibleTo($actor, 'viewDiscussionsInRestrictedTags');
diff --git a/extensions/tags/src/Access/ScopeDiscussionVisibilityForAbility.php b/extensions/tags/src/Access/ScopeDiscussionVisibilityForAbility.php
index 5bf2b6ba0..26a9310f7 100644
--- a/extensions/tags/src/Access/ScopeDiscussionVisibilityForAbility.php
+++ b/extensions/tags/src/Access/ScopeDiscussionVisibilityForAbility.php
@@ -32,7 +32,9 @@ class ScopeDiscussionVisibilityForAbility
$query->whereIn('discussions.id', function ($query) use ($actor, $ability) {
return $query->select('discussion_id')
->from('discussion_tag')
- ->whereIn('tag_id', Tag::getIdsWhereCan($actor, 'discussion.'.$ability));
+ ->whereIn('tag_id', function ($query) use ($actor, $ability) {
+ Tag::queryIdsWhereCan($query->from('tags'), $actor, 'discussion.'.$ability);
+ });
});
}
}
diff --git a/extensions/tags/src/Access/ScopeFlagVisibility.php b/extensions/tags/src/Access/ScopeFlagVisibility.php
index bcb459b6f..f9547e3e0 100644
--- a/extensions/tags/src/Access/ScopeFlagVisibility.php
+++ b/extensions/tags/src/Access/ScopeFlagVisibility.php
@@ -28,7 +28,9 @@ class ScopeFlagVisibility
->whereNotExists(function ($query) use ($actor) {
return $query->selectRaw('1')
->from('discussion_tag')
- ->whereIn('tag_id', Tag::getIdsWhereCannot($actor, 'discussion.viewFlags'))
+ ->whereNotIn('tag_id', function ($query) use ($actor) {
+ Tag::queryIdsWhereCan($query->from('tags'), $actor, 'discussion.viewFlags');
+ })
->whereColumn('discussions.id', 'discussion_id');
});
}
diff --git a/extensions/tags/src/Access/ScopeTagVisibility.php b/extensions/tags/src/Access/ScopeTagVisibility.php
index f159dc467..2802efe0b 100644
--- a/extensions/tags/src/Access/ScopeTagVisibility.php
+++ b/extensions/tags/src/Access/ScopeTagVisibility.php
@@ -21,6 +21,8 @@ class ScopeTagVisibility
*/
public function __invoke(User $actor, Builder $query)
{
- $query->whereNotIn('id', Tag::getIdsWhereCannot($actor, 'viewDiscussions'));
+ $query->whereIn('id', function ($query) use ($actor) {
+ Tag::queryIdsWhereCan($query, $actor, 'viewDiscussions');
+ });
}
}
diff --git a/extensions/tags/src/Tag.php b/extensions/tags/src/Tag.php
index ee5da4e57..cace21f16 100644
--- a/extensions/tags/src/Tag.php
+++ b/extensions/tags/src/Tag.php
@@ -15,6 +15,7 @@ use Flarum\Discussion\Discussion;
use Flarum\Group\Permission;
use Flarum\User\User;
use Illuminate\Database\Eloquent\Builder;
+use Illuminate\Database\Query\Builder as QueryBuilder;
/**
* @property int $id
@@ -190,45 +191,70 @@ class Tag extends AbstractModel
Permission::where('permission', 'like', "tag{$this->id}.%")->delete();
}
- protected static function getIdsWherePermission(User $user, string $permission, bool $condition = true, bool $includePrimary = true, bool $includeSecondary = true): array
+ protected static function buildPermissionSubquery($base, $isAdmin, $hasGlobalPermission, $tagIdsWithPermission)
{
- static $tags;
+ $base
+ ->from('tags as perm_tags')
+ ->select('perm_tags.id');
- if (! $tags) {
- $tags = static::with('parent')->get();
+ // This needs to be a special case, as `tagIdsWithPermissions`
+ // won't include admin perms (which are all perms by default).
+ if ($isAdmin) {
+ return;
}
- $ids = [];
- $hasGlobalPermission = $user->hasPermission($permission);
+ $base->where(function ($query) use ($tagIdsWithPermission) {
+ $query
+ ->where('perm_tags.is_restricted', true)
+ ->whereIn('perm_tags.id', $tagIdsWithPermission);
+ });
- $canForTag = function (self $tag) use ($user, $permission, $hasGlobalPermission) {
- return ($hasGlobalPermission && ! $tag->is_restricted) || ($tag->is_restricted && $user->hasPermission('tag'.$tag->id.'.'.$permission));
- };
-
- foreach ($tags as $tag) {
- $can = $canForTag($tag);
-
- if ($can && $tag->parent) {
- $can = $canForTag($tag->parent);
- }
-
- $isPrimary = $tag->position !== null && ! $tag->parent;
-
- if ($can === $condition && ($includePrimary && $isPrimary || $includeSecondary && ! $isPrimary)) {
- $ids[] = $tag->id;
- }
+ if ($hasGlobalPermission) {
+ $base->orWhere('perm_tags.is_restricted', false);
}
-
- return $ids;
}
- public static function getIdsWhereCan(User $user, string $permission, bool $includePrimary = true, bool $includeSecondary = true): array
+ public static function queryIdsWhereCan(QueryBuilder $base, User $user, string $currPermission, bool $includePrimary = true, bool $includeSecondary = true): QueryBuilder
{
- return static::getIdsWherePermission($user, $permission, true, $includePrimary, $includeSecondary);
- }
+ $hasGlobalPermission = $user->hasPermission($currPermission);
+ $isAdmin = $user->isAdmin();
+ $allPermissions = $user->getPermissions();
- public static function getIdsWhereCannot(User $user, string $permission, bool $includePrimary = true, bool $includeSecondary = true): array
- {
- return static::getIdsWherePermission($user, $permission, false, $includePrimary, $includeSecondary);
+ $tagIdsWithPermission = collect($allPermissions)
+ ->filter(function ($permission) use ($currPermission) {
+ return substr($permission, 0, 3) === 'tag' && strpos($permission, $currPermission) !== false;
+ })
+ ->map(function ($permission) {
+ $scopeFragment = explode('.', $permission, 2)[0];
+
+ return substr($scopeFragment, 3);
+ })
+ ->values();
+
+ $validTags = $base
+ ->from('tags as s_tags')
+ ->where(function ($query) use ($isAdmin, $hasGlobalPermission, $tagIdsWithPermission) {
+ $query
+ ->whereIn('s_tags.id', function ($query) use ($isAdmin, $hasGlobalPermission, $tagIdsWithPermission) {
+ static::buildPermissionSubquery($query, $isAdmin, $hasGlobalPermission, $tagIdsWithPermission);
+ })
+ ->where(function ($query) use ($isAdmin, $hasGlobalPermission, $tagIdsWithPermission) {
+ $query
+ ->whereIn('s_tags.parent_id', function ($query) use ($isAdmin, $hasGlobalPermission, $tagIdsWithPermission) {
+ static::buildPermissionSubquery($query, $isAdmin, $hasGlobalPermission, $tagIdsWithPermission);
+ })
+ ->orWhere('s_tags.parent_id', null);
+ });
+ })
+ ->where(function ($query) use ($includePrimary, $includeSecondary) {
+ if (! $includePrimary) {
+ $query->where('s_tags.position', '=', null);
+ }
+ if (! $includeSecondary) {
+ $query->where('s_tags.position', '!=', null);
+ }
+ });
+
+ return $validTags->select('s_tags.id');
}
}
diff --git a/extensions/tags/tests/fixtures/.gitkeep b/extensions/tags/tests/fixtures/.gitkeep
new file mode 100644
index 000000000..e69de29bb
diff --git a/extensions/tags/tests/integration/RetrievesRepresentativeTags.php b/extensions/tags/tests/integration/RetrievesRepresentativeTags.php
new file mode 100644
index 000000000..06d346f91
--- /dev/null
+++ b/extensions/tags/tests/integration/RetrievesRepresentativeTags.php
@@ -0,0 +1,30 @@
+ 1, 'name' => 'Primary 1', 'slug' => 'primary-1', 'position' => 0, 'parent_id' => null],
+ ['id' => 2, 'name' => 'Primary 2', 'slug' => 'primary-2', 'position' => 1, 'parent_id' => null],
+ ['id' => 3, 'name' => 'Primary 2 Child 1', 'slug' => 'primary-2-child-1', 'position' => 2, 'parent_id' => 2],
+ ['id' => 4, 'name' => 'Primary 2 Child 2', 'slug' => 'primary-2-child-2', 'position' => 3, 'parent_id' => 2],
+ ['id' => 5, 'name' => 'Primary 2 Child Restricted', 'slug' => 'primary-2-child-restricted', 'position' => 4, 'parent_id' => 2, 'is_restricted' => true],
+ ['id' => 6, 'name' => 'Primary Restricted', 'slug' => 'primary-restricted', 'position' => 5, 'parent_id' => null, 'is_restricted' => true],
+ ['id' => 7, 'name' => 'Primary Restricted Child 1', 'slug' => 'primary-restricted-child-1', 'position' => 6, 'parent_id' => 6],
+ ['id' => 8, 'name' => 'Primary Restricted Child Restricted', 'slug' => 'primary-restricted-child-restricted', 'position' => 7, 'parent_id' => 6, 'is_restricted' => true],
+ ['id' => 9, 'name' => 'Secondary 1', 'slug' => 'secondary-1', 'position' => null, 'parent_id' => null],
+ ['id' => 10, 'name' => 'Secondary 2', 'slug' => 'secondary-2', 'position' => null, 'parent_id' => null],
+ ['id' => 11, 'name' => 'Secondary Restricted', 'slug' => 'secondary-restricted', 'position' => null, 'parent_id' => null, 'is_restricted' => true],
+ ];
+ }
+}
diff --git a/extensions/tags/tests/integration/api/discussions/CreateTest.php b/extensions/tags/tests/integration/api/discussions/CreateTest.php
new file mode 100644
index 000000000..a131e07ed
--- /dev/null
+++ b/extensions/tags/tests/integration/api/discussions/CreateTest.php
@@ -0,0 +1,265 @@
+extension('flarum-tags');
+
+ $this->prepareDatabase([
+ 'tags' => $this->tags(),
+ 'users' => [
+ $this->normalUser(),
+ ],
+ 'group_permission' => [
+ ['group_id' => Group::MEMBER_ID, 'permission' => 'tag5.viewDiscussions'],
+ ['group_id' => Group::MEMBER_ID, 'permission' => 'tag8.viewDiscussions'],
+ ['group_id' => Group::MEMBER_ID, 'permission' => 'tag8.startDiscussion'],
+ ['group_id' => Group::MEMBER_ID, 'permission' => 'tag11.viewDiscussions'],
+ ['group_id' => Group::MEMBER_ID, 'permission' => 'tag11.startDiscussion'],
+ ]
+ ]);
+ }
+
+ /**
+ * @test
+ */
+ public function admin_can_create_discussion_without_tags()
+ {
+ $response = $this->send(
+ $this->request('POST', '/api/discussions', [
+ 'authenticatedAs' => 1,
+ 'json' => [
+ 'data' => [
+ 'attributes' => [
+ 'title' => 'test - too-obscure',
+ 'content' => 'predetermined content for automated testing - too-obscure',
+ ],
+ ]
+ ],
+ ])
+ );
+
+ $this->assertEquals(201, $response->getStatusCode());
+ }
+
+ /**
+ * @test
+ */
+ public function user_cant_create_discussion_without_tags()
+ {
+ $response = $this->send(
+ $this->request('POST', '/api/discussions', [
+ 'authenticatedAs' => 2,
+ 'json' => [
+ 'data' => [
+ 'attributes' => [
+ 'title' => 'test - too-obscure',
+ 'content' => 'predetermined content for automated testing - too-obscure',
+ ],
+ ]
+ ],
+ ])
+ );
+
+ $this->assertEquals(422, $response->getStatusCode());
+ }
+
+ /**
+ * @test
+ */
+ public function user_can_create_discussion_in_primary_tag()
+ {
+ $response = $this->send(
+ $this->request('POST', '/api/discussions', [
+ 'authenticatedAs' => 2,
+ 'json' => [
+ 'data' => [
+ 'attributes' => [
+ 'title' => 'test - too-obscure',
+ 'content' => 'predetermined content for automated testing - too-obscure',
+ ],
+ 'relationships' => [
+ 'tags' => [
+ 'data' => [
+ ['type' => 'tags', 'id' => 1]
+ ]
+ ]
+ ]
+ ],
+ ],
+ ])
+ );
+
+ $this->assertEquals(201, $response->getStatusCode());
+ }
+
+ /**
+ * @test
+ */
+ public function user_cant_create_discussion_in_primary_tag_where_can_view_but_cant_start()
+ {
+ $response = $this->send(
+ $this->request('POST', '/api/discussions', [
+ 'authenticatedAs' => 2,
+ 'json' => [
+ 'data' => [
+ 'attributes' => [
+ 'title' => 'test - too-obscure',
+ 'content' => 'predetermined content for automated testing - too-obscure',
+ ],
+ 'relationships' => [
+ 'tags' => [
+ 'data' => [
+ ['type' => 'tags', 'id' => 5]
+ ]
+ ]
+ ]
+ ],
+ ],
+ ])
+ );
+
+ $this->assertEquals(403, $response->getStatusCode());
+ }
+
+ /**
+ * @test
+ */
+ public function user_can_create_discussion_in_tag_where_can_view_and_can_start()
+ {
+ $response = $this->send(
+ $this->request('POST', '/api/discussions', [
+ 'authenticatedAs' => 2,
+ 'json' => [
+ 'data' => [
+ 'attributes' => [
+ 'title' => 'test - too-obscure',
+ 'content' => 'predetermined content for automated testing - too-obscure',
+ ],
+ 'relationships' => [
+ 'tags' => [
+ 'data' => [
+ ['type' => 'tags', 'id' => 1],
+ ['type' => 'tags', 'id' => 11]
+ ]
+ ]
+ ]
+ ],
+ ],
+ ])
+ );
+
+ $this->assertEquals(201, $response->getStatusCode());
+ }
+
+ /**
+ * @test
+ */
+ public function user_cant_create_discussion_in_child_tag_without_parent_tag()
+ {
+ $response = $this->send(
+ $this->request('POST', '/api/discussions', [
+ 'authenticatedAs' => 2,
+ 'json' => [
+ 'data' => [
+ 'attributes' => [
+ 'title' => 'test - too-obscure',
+ 'content' => 'predetermined content for automated testing - too-obscure',
+ ],
+ 'relationships' => [
+ 'tags' => [
+ 'data' => [
+ ['type' => 'tags', 'id' => 4]
+ ]
+ ]
+ ]
+ ],
+ ],
+ ])
+ );
+
+ $this->assertEquals(422, $response->getStatusCode());
+ }
+
+ /**
+ * @test
+ */
+ public function user_can_create_discussion_in_child_tag_with_parent_tag()
+ {
+ $response = $this->send(
+ $this->request('POST', '/api/discussions', [
+ 'authenticatedAs' => 2,
+ 'json' => [
+ 'data' => [
+ 'attributes' => [
+ 'title' => 'test - too-obscure',
+ 'content' => 'predetermined content for automated testing - too-obscure',
+ ],
+ 'relationships' => [
+ 'tags' => [
+ 'data' => [
+ ['type' => 'tags', 'id' => 3],
+ ['type' => 'tags', 'id' => 4]
+ ]
+ ]
+ ]
+ ],
+ ],
+ ])
+ );
+
+ $this->assertEquals(422, $response->getStatusCode());
+ }
+
+ /**
+ * @test
+ */
+ public function primary_tag_required_by_default()
+ {
+ $response = $this->send(
+ $this->request('POST', '/api/discussions', [
+ 'authenticatedAs' => 2,
+ 'json' => [
+ 'data' => [
+ 'attributes' => [
+ 'title' => 'test - too-obscure',
+ 'content' => 'predetermined content for automated testing - too-obscure',
+ ],
+ 'relationships' => [
+ 'tags' => [
+ 'data' => [
+ ['type' => 'tags', 'id' => 11]
+ ]
+ ]
+ ]
+ ],
+ ],
+ ])
+ );
+
+ $this->assertEquals(422, $response->getStatusCode());
+ }
+}
diff --git a/extensions/tags/tests/integration/api/tags/CreateTest.php b/extensions/tags/tests/integration/api/tags/CreateTest.php
new file mode 100644
index 000000000..39acb49a2
--- /dev/null
+++ b/extensions/tags/tests/integration/api/tags/CreateTest.php
@@ -0,0 +1,107 @@
+extension('flarum-tags');
+
+ $this->prepareDatabase([
+ 'users' => [
+ $this->normalUser(),
+ ],
+ ]);
+ }
+
+ /**
+ * @test
+ */
+ public function normal_user_cant_create_tag()
+ {
+ $response = $this->send(
+ $this->request(
+ 'POST',
+ '/api/tags',
+ ['authenticatedAs' => 2]
+ )
+ );
+
+ $this->assertEquals(403, $response->getStatusCode());
+ }
+
+ /**
+ * @test
+ */
+ public function admin_cannot_create_tag_without_data()
+ {
+ $response = $this->send(
+ $this->request('POST', '/api/tags', [
+ 'authenticatedAs' => 1,
+ 'json' => [],
+ ])
+ );
+
+ $this->assertEquals(422, $response->getStatusCode());
+ }
+
+ /**
+ * @test
+ */
+ public function admin_can_create_tag()
+ {
+ $response = $this->send(
+ $this->request('POST', '/api/tags', [
+ 'authenticatedAs' => 1,
+ 'json' => [
+ 'data' => [
+ 'attributes' => [
+ 'name' => 'Dev Blog',
+ 'slug' => 'dev-blog',
+ 'description' => 'Follow Flarum development!',
+ 'color' => '#123456'
+ ],
+ ],
+ ],
+ ])
+ );
+
+ $this->assertEquals(201, $response->getStatusCode());
+
+ // Verify API response body
+ $data = json_decode($response->getBody(), true);
+ $this->assertEquals('Dev Blog', Arr::get($data, 'data.attributes.name'));
+ $this->assertEquals('dev-blog', Arr::get($data, 'data.attributes.slug'));
+ $this->assertEquals('Follow Flarum development!', Arr::get($data, 'data.attributes.description'));
+ $this->assertEquals('#123456', Arr::get($data, 'data.attributes.color'));
+ $this->assertNull(Arr::get($data, 'data.attributes.icon'));
+
+ // Verify database entry
+ $tag = Tag::all()->last();
+ $this->assertEquals('Dev Blog', $tag->name);
+ $this->assertEquals('dev-blog', $tag->slug);
+ $this->assertEquals('Follow Flarum development!', $tag->description);
+ $this->assertEquals('#123456', $tag->color);
+ $this->assertNull($tag->icon);
+ }
+}
diff --git a/extensions/tags/tests/integration/authorization/PolicyTest.php b/extensions/tags/tests/integration/authorization/PolicyTest.php
new file mode 100644
index 000000000..9fc750dec
--- /dev/null
+++ b/extensions/tags/tests/integration/authorization/PolicyTest.php
@@ -0,0 +1,116 @@
+extension('flarum-tags');
+
+ $this->prepareDatabase([
+ 'tags' => $this->tags(),
+ 'users' => [
+ $this->normalUser(),
+ ]
+ ]);
+ }
+
+ /**
+ * @test
+ */
+ public function cant_start_discussion_globally_if_permission_not_granted()
+ {
+ $this->database()->table('group_permission')->where('permission', 'startDiscussion')->delete();
+
+ $this->assertFalse(User::find(2)->can('startDiscussion'));
+ }
+
+ /**
+ * @test
+ */
+ public function can_start_discussion_globally_if_allowed_in_primary_tag()
+ {
+ $this->prepareDatabase([
+ 'group_permission' => [
+ ['group_id' => Group::MEMBER_ID, 'permission' => 'tag6.startDiscussion'],
+ ]
+ ]);
+
+ $this->database()->table('group_permission')->where('permission', 'startDiscussion')->delete();
+
+ $this->assertTrue(User::find(2)->can('startDiscussion'));
+ }
+
+ /**
+ * @test
+ */
+ public function cant_start_discussion_globally_if_allowed_in_child_tag_only()
+ {
+ $this->prepareDatabase([
+ 'group_permission' => [
+ ['group_id' => Group::MEMBER_ID, 'permission' => 'tag8.startDiscussion'],
+ ]
+ ]);
+
+ $this->database()->table('group_permission')->where('permission', 'startDiscussion')->delete();
+
+ $this->assertFalse(User::find(2)->can('startDiscussion'));
+ }
+
+ /**
+ * @test
+ */
+ public function cant_start_discussion_globally_if_allowed_in_secondary_tag()
+ {
+ $this->prepareDatabase([
+ 'group_permission' => [
+ ['group_id' => Group::MEMBER_ID, 'permission' => 'tag11.startDiscussion'],
+ ]
+ ]);
+
+ $this->database()->table('group_permission')->where('permission', 'startDiscussion')->delete();
+
+ $this->assertFalse(User::find(2)->can('startDiscussion'));
+ }
+
+ /**
+ * @test
+ */
+ public function can_start_discussion_globally_if_allowed_in_secondary_tag_and_minimums_adjusted()
+ {
+ $this->prepareDatabase([
+ 'group_permission' => [
+ ['group_id' => Group::MEMBER_ID, 'permission' => 'tag11.startDiscussion'],
+ ]
+ ]);
+
+ $this->setting('flarum-tags.min_primary_tags', 0);
+ $this->setting('flarum-tags.min_secondary_tags', 1);
+
+ $this->database()->table('group_permission')->where('permission', 'startDiscussion')->delete();
+
+ $this->assertTrue(User::find(2)->can('startDiscussion'));
+ }
+}
diff --git a/extensions/tags/tests/integration/setup.php b/extensions/tags/tests/integration/setup.php
new file mode 100644
index 000000000..67039c083
--- /dev/null
+++ b/extensions/tags/tests/integration/setup.php
@@ -0,0 +1,16 @@
+run();
diff --git a/extensions/tags/tests/integration/visibility/DiscussionVisibilityTest.php b/extensions/tags/tests/integration/visibility/DiscussionVisibilityTest.php
new file mode 100644
index 000000000..77039dbbf
--- /dev/null
+++ b/extensions/tags/tests/integration/visibility/DiscussionVisibilityTest.php
@@ -0,0 +1,127 @@
+extension('flarum-tags');
+
+ $this->prepareDatabase([
+ 'discussions' => [
+ ['id' => 1, 'title' => 'no tags', 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 1, 'comment_count' => 1],
+ ['id' => 2, 'title' => 'open tags', 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 1, 'comment_count' => 1],
+ ['id' => 3, 'title' => 'open tag, restricted child tag', 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 1, 'comment_count' => 1],
+ ['id' => 4, 'title' => 'open tag, one restricted secondary tag', 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 1, 'comment_count' => 1],
+ ['id' => 5, 'title' => 'all closed', 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 1, 'comment_count' => 1],
+ ],
+ 'posts' => [
+ ['id' => 1, 'discussion_id' => 1, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 1, 'type' => 'comment', 'content' => ''],
+ ['id' => 2, 'discussion_id' => 2, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 1, 'type' => 'comment', 'content' => ''],
+ ['id' => 3, 'discussion_id' => 3, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 1, 'type' => 'comment', 'content' => ''],
+ ['id' => 4, 'discussion_id' => 4, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 1, 'type' => 'comment', 'content' => ''],
+ ['id' => 5, 'discussion_id' => 5, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 1, 'type' => 'comment', 'content' => ''],
+ ],
+ 'discussion_tag' => [
+ ['discussion_id' => 2, 'tag_id' => 1],
+ ['discussion_id' => 3, 'tag_id' => 2],
+ ['discussion_id' => 3, 'tag_id' => 5],
+ ['discussion_id' => 4, 'tag_id' => 1],
+ ['discussion_id' => 4, 'tag_id' => 11],
+ ['discussion_id' => 5, 'tag_id' => 6],
+ ['discussion_id' => 5, 'tag_id' => 7],
+ ['discussion_id' => 5, 'tag_id' => 8],
+ ],
+ 'tags' => $this->tags(),
+ 'users' => [
+ $this->normalUser(),
+ ],
+ 'group_permission' => [
+ ['group_id' => Group::MEMBER_ID, 'permission' => 'tag5.viewDiscussions'],
+ ['group_id' => Group::MEMBER_ID, 'permission' => 'tag8.viewDiscussions'],
+ ['group_id' => Group::MEMBER_ID, 'permission' => 'tag11.viewDiscussions']
+ ]
+ ]);
+ }
+
+ /**
+ * @test
+ */
+ public function admin_sees_all()
+ {
+ $response = $this->send(
+ $this->request('GET', '/api/discussions', [
+ 'authenticatedAs' => 1
+ ])
+ );
+
+ $this->assertEquals(200, $response->getStatusCode());
+
+ $data = json_decode($response->getBody()->getContents(), true)['data'];
+
+ $ids = Arr::pluck($data, 'id');
+ $this->assertEqualsCanonicalizing(['1', '2', '3', '4', '5'], $ids);
+ }
+
+ /**
+ * @test
+ */
+ public function user_sees_where_allowed()
+ {
+ $response = $this->send(
+ $this->request('GET', '/api/discussions', [
+ 'authenticatedAs' => 2
+ ])
+ );
+
+ $this->assertEquals(200, $response->getStatusCode());
+
+ $data = json_decode($response->getBody()->getContents(), true)['data'];
+
+ // 5 isnt included because parent access doesnt necessarily give child access
+ // 6, 7, 8 aren't included because child access shouldnt work unless parent
+ // access is also given.
+ $ids = Arr::pluck($data, 'id');
+ $this->assertEqualsCanonicalizing(['1', '2', '3', '4'], $ids);
+ }
+
+ /**
+ * @test
+ */
+ public function guest_cant_see_restricted_or_children_of_restricted()
+ {
+ $response = $this->send(
+ $this->request('GET', '/api/discussions')
+ );
+
+ $this->assertEquals(200, $response->getStatusCode());
+
+ $data = json_decode($response->getBody()->getContents(), true)['data'];
+
+ $ids = Arr::pluck($data, 'id');
+ $this->assertEqualsCanonicalizing(['1', '2'], $ids);
+ }
+}
diff --git a/extensions/tags/tests/integration/visibility/TagVisibilityTest.php b/extensions/tags/tests/integration/visibility/TagVisibilityTest.php
new file mode 100644
index 000000000..30705222b
--- /dev/null
+++ b/extensions/tags/tests/integration/visibility/TagVisibilityTest.php
@@ -0,0 +1,99 @@
+extension('flarum-tags');
+
+ $this->prepareDatabase([
+ 'tags' => $this->tags(),
+ 'users' => [
+ $this->normalUser(),
+ ],
+ 'group_permission' => [
+ ['group_id' => Group::MEMBER_ID, 'permission' => 'tag8.viewDiscussions'],
+ ['group_id' => Group::MEMBER_ID, 'permission' => 'tag11.viewDiscussions']
+ ]
+ ]);
+ }
+
+ /**
+ * @test
+ */
+ public function admin_sees_all()
+ {
+ $response = $this->send(
+ $this->request('GET', '/api/tags', [
+ 'authenticatedAs' => 1
+ ])
+ );
+
+ $this->assertEquals(200, $response->getStatusCode());
+
+ $data = json_decode($response->getBody()->getContents(), true)['data'];
+
+ $this->assertEquals(['1', '2', '3', '4', '5', '6', '7', '8', '9', '10', '11'], Arr::pluck($data, 'id'));
+ }
+
+ /**
+ * @test
+ */
+ public function user_sees_where_allowed()
+ {
+ $response = $this->send(
+ $this->request('GET', '/api/tags', [
+ 'authenticatedAs' => 2
+ ])
+ );
+
+ $this->assertEquals(200, $response->getStatusCode());
+
+ $data = json_decode($response->getBody()->getContents(), true)['data'];
+
+ // 5 isnt included because parent access doesnt necessarily give child access
+ // 6, 7, 8 aren't included because child access shouldnt work unless parent
+ // access is also given.
+ $this->assertEquals(['1', '2', '3', '4', '9', '10', '11'], Arr::pluck($data, 'id'));
+ }
+
+ /**
+ * @test
+ */
+ public function guest_cant_see_restricted_or_children_of_restricted()
+ {
+ $response = $this->send(
+ $this->request('GET', '/api/tags')
+ );
+
+ $this->assertEquals(200, $response->getStatusCode());
+
+ $data = json_decode($response->getBody()->getContents(), true)['data'];
+
+ // Order-independent comparison
+ $this->assertEquals(['1', '2', '3', '4', '9', '10'], Arr::pluck($data, 'id'));
+ }
+}
diff --git a/extensions/tags/tests/phpunit.integration.xml b/extensions/tags/tests/phpunit.integration.xml
new file mode 100644
index 000000000..23afc237d
--- /dev/null
+++ b/extensions/tags/tests/phpunit.integration.xml
@@ -0,0 +1,24 @@
+
+
+
+
+ ../src/
+
+
+
+
+ ./integration
+
+
+
diff --git a/extensions/tags/tests/phpunit.unit.xml b/extensions/tags/tests/phpunit.unit.xml
new file mode 100644
index 000000000..d3a4a3e3d
--- /dev/null
+++ b/extensions/tags/tests/phpunit.unit.xml
@@ -0,0 +1,27 @@
+
+
+
+
+ ../src/
+
+
+
+
+ ./unit
+
+
+
+
+
+
diff --git a/extensions/tags/tests/unit/.gitkeep b/extensions/tags/tests/unit/.gitkeep
new file mode 100644
index 000000000..e69de29bb