From a1c3da9f8fe58d5be4251191d1a47354c69dbf2e Mon Sep 17 00:00:00 2001 From: Franz Liedke Date: Tue, 24 Apr 2018 23:48:37 +0200 Subject: [PATCH 01/10] Migrations: always pass a schema builder This removes the funky auto-injection capability from migration closures. While technically removing a feature, this means we do not need a fully-wired IoC container e.g. during installation. Instead, all migration closures simply receive a schema builder object (which is what most of them were already doing anyway). --- src/Database/Migration.php | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/src/Database/Migration.php b/src/Database/Migration.php index f8c08195b..79332813d 100644 --- a/src/Database/Migration.php +++ b/src/Database/Migration.php @@ -11,8 +11,7 @@ namespace Flarum\Database; -use Flarum\Settings\SettingsRepositoryInterface; -use Illuminate\Database\ConnectionInterface; +use Flarum\Settings\DatabaseSettingsRepository; use Illuminate\Database\Schema\Blueprint; use Illuminate\Database\Schema\Builder; @@ -100,12 +99,20 @@ abstract class Migration public static function addSettings(array $defaults) { return [ - 'up' => function (SettingsRepositoryInterface $settings) use ($defaults) { + 'up' => function (Builder $schema) use ($defaults) { + $settings = new DatabaseSettingsRepository( + $schema->getConnection() + ); + foreach ($defaults as $key => $value) { $settings->set($key, $value); } }, - 'down' => function (SettingsRepositoryInterface $settings) use ($defaults) { + 'down' => function (Builder $schema) use ($defaults) { + $settings = new DatabaseSettingsRepository( + $schema->getConnection() + ); + foreach (array_keys($defaults) as $key) { $settings->delete($key); } @@ -130,7 +137,9 @@ abstract class Migration } return [ - 'up' => function (ConnectionInterface $db) use ($keys) { + 'up' => function (Builder $schema) use ($keys) { + $db = $schema->getConnection(); + foreach ($keys as $key) { $instance = $db->table('permissions')->where($key)->first(); @@ -140,7 +149,9 @@ abstract class Migration } }, - 'down' => function (ConnectionInterface $db) use ($keys) { + 'down' => function (Builder $schema) use ($keys) { + $db = $schema->getConnection(); + foreach ($keys as $key) { $db->table('permissions')->where($key)->delete(); } From d301d260c10c8515af7506c4292c011b74dc35fb Mon Sep 17 00:00:00 2001 From: Franz Liedke Date: Tue, 24 Apr 2018 23:50:36 +0200 Subject: [PATCH 02/10] Simplify interface of migration-related classes Mostly, we only need a database connection, instead of one of Laravel's "connection resolvers". Again, this makes our life easier during installation, where we already instantiate a database connection. We can now use that to instantiate our own Migrator class, instead of using the IoC container to build one. --- src/Database/DatabaseMigrationRepository.php | 60 ++++--------------- src/Database/MigrationRepositoryInterface.php | 8 --- src/Database/MigrationServiceProvider.php | 2 +- src/Database/Migrator.php | 53 ++++------------ 4 files changed, 22 insertions(+), 101 deletions(-) diff --git a/src/Database/DatabaseMigrationRepository.php b/src/Database/DatabaseMigrationRepository.php index 5ba06cc09..edf669d88 100644 --- a/src/Database/DatabaseMigrationRepository.php +++ b/src/Database/DatabaseMigrationRepository.php @@ -11,16 +11,16 @@ namespace Flarum\Database; -use Illuminate\Database\ConnectionResolverInterface as Resolver; +use Illuminate\Database\ConnectionInterface; class DatabaseMigrationRepository implements MigrationRepositoryInterface { /** - * The database connection resolver instance. + * The name of the database connection to use. * - * @var \Illuminate\Database\ConnectionResolverInterface + * @var ConnectionInterface */ - protected $resolver; + protected $connection; /** * The name of the migration table. @@ -29,23 +29,16 @@ class DatabaseMigrationRepository implements MigrationRepositoryInterface */ protected $table; - /** - * The name of the database connection to use. - * - * @var string - */ - protected $connection; - /** * Create a new database migration repository instance. * - * @param \Illuminate\Database\ConnectionResolverInterface $resolver - * @param string $table + * @param \Illuminate\Database\ConnectionInterface $connection + * @param string $table */ - public function __construct(Resolver $resolver, $table) + public function __construct(ConnectionInterface $connection, $table) { + $this->connection = $connection; $this->table = $table; - $this->resolver = $resolver; } /** @@ -104,7 +97,7 @@ class DatabaseMigrationRepository implements MigrationRepositoryInterface */ public function createRepository() { - $schema = $this->getConnection()->getSchemaBuilder(); + $schema = $this->connection->getSchemaBuilder(); $schema->create($this->table, function ($table) { $table->string('migration'); @@ -119,7 +112,7 @@ class DatabaseMigrationRepository implements MigrationRepositoryInterface */ public function repositoryExists() { - $schema = $this->getConnection()->getSchemaBuilder(); + $schema = $this->connection->getSchemaBuilder(); return $schema->hasTable($this->table); } @@ -131,37 +124,6 @@ class DatabaseMigrationRepository implements MigrationRepositoryInterface */ protected function table() { - return $this->getConnection()->table($this->table); - } - - /** - * Get the connection resolver instance. - * - * @return \Illuminate\Database\ConnectionResolverInterface - */ - public function getConnectionResolver() - { - return $this->resolver; - } - - /** - * Resolve the database connection instance. - * - * @return \Illuminate\Database\Connection - */ - public function getConnection() - { - return $this->resolver->connection($this->connection); - } - - /** - * Set the information source to gather data. - * - * @param string $name - * @return void - */ - public function setSource($name) - { - $this->connection = $name; + return $this->connection->table($this->table); } } diff --git a/src/Database/MigrationRepositoryInterface.php b/src/Database/MigrationRepositoryInterface.php index 7bd9265e8..e6ae5a1dd 100644 --- a/src/Database/MigrationRepositoryInterface.php +++ b/src/Database/MigrationRepositoryInterface.php @@ -52,12 +52,4 @@ interface MigrationRepositoryInterface * @return bool */ public function repositoryExists(); - - /** - * Set the information source to gather data. - * - * @param string $name - * @return void - */ - public function setSource($name); } diff --git a/src/Database/MigrationServiceProvider.php b/src/Database/MigrationServiceProvider.php index 5a21c9ada..165cec0da 100644 --- a/src/Database/MigrationServiceProvider.php +++ b/src/Database/MigrationServiceProvider.php @@ -22,7 +22,7 @@ class MigrationServiceProvider extends AbstractServiceProvider public function register() { $this->app->singleton('Flarum\Database\MigrationRepositoryInterface', function ($app) { - return new DatabaseMigrationRepository($app['db'], 'migrations'); + return new DatabaseMigrationRepository($app['flarum.db'], 'migrations'); }); $this->app->bind(MigrationCreator::class, function (Application $app) { diff --git a/src/Database/Migrator.php b/src/Database/Migrator.php index 926d994c6..2c1272fc0 100644 --- a/src/Database/Migrator.php +++ b/src/Database/Migrator.php @@ -13,7 +13,8 @@ namespace Flarum\Database; use Exception; use Flarum\Extension\Extension; -use Illuminate\Database\ConnectionResolverInterface as Resolver; +use Illuminate\Database\ConnectionInterface; +use Illuminate\Database\Schema\Builder; use Illuminate\Filesystem\Filesystem; class Migrator @@ -33,18 +34,11 @@ class Migrator protected $files; /** - * The connection resolver instance. + * The database schema builder instance. * - * @var \Illuminate\Database\ConnectionResolverInterface + * @var Builder */ - protected $resolver; - - /** - * The name of the default connection. - * - * @var string - */ - protected $connection; + protected $schemaBuilder; /** * The notes for the current operation. @@ -57,17 +51,18 @@ class Migrator * Create a new migrator instance. * * @param MigrationRepositoryInterface $repository - * @param Resolver $resolver + * @param ConnectionInterface $connection * @param Filesystem $files */ public function __construct( MigrationRepositoryInterface $repository, - Resolver $resolver, + ConnectionInterface $connection, Filesystem $files ) { $this->files = $files; - $this->resolver = $resolver; $this->repository = $repository; + + $this->schemaBuilder = $connection->getSchemaBuilder(); } /** @@ -199,7 +194,7 @@ class Migrator protected function runClosureMigration($migration, $direction = 'up') { if (is_array($migration) && array_key_exists($direction, $migration)) { - app()->call($migration[$direction]); + call_user_func($migration[$direction], $this->schemaBuilder); } else { throw new Exception('Migration file should contain an array with up/down.'); } @@ -268,34 +263,6 @@ class Migrator return $this->notes; } - /** - * Resolve the database connection instance. - * - * @param string $connection - * @return \Illuminate\Database\Connection - */ - public function resolveConnection($connection) - { - return $this->resolver->connection($connection); - } - - /** - * Set the default connection name. - * - * @param string $name - * @return void - */ - public function setConnection($name) - { - if (! is_null($name)) { - $this->resolver->setDefaultConnection($name); - } - - $this->repository->setSource($name); - - $this->connection = $name; - } - /** * Get the migration repository instance. * From 22f2df3670bad6d16c204e8f0f8964b5f558d254 Mon Sep 17 00:00:00 2001 From: Sajjad Hashemian Date: Wed, 6 Jun 2018 09:40:29 +0430 Subject: [PATCH 03/10] rename TokenController to CreateTokenController --- .../{TokenController.php => CreateTokenController.php} | 2 +- src/Api/routes.php | 2 +- src/Forum/Controller/LogInController.php | 4 ++-- ...okenControllerTest.php => CreateTokenControllerTest.php} | 6 +++--- 4 files changed, 7 insertions(+), 7 deletions(-) rename src/Api/Controller/{TokenController.php => CreateTokenController.php} (96%) rename tests/Api/Controller/{TokenControllerTest.php => CreateTokenControllerTest.php} (84%) diff --git a/src/Api/Controller/TokenController.php b/src/Api/Controller/CreateTokenController.php similarity index 96% rename from src/Api/Controller/TokenController.php rename to src/Api/Controller/CreateTokenController.php index 268f4a598..563bd9ed1 100644 --- a/src/Api/Controller/TokenController.php +++ b/src/Api/Controller/CreateTokenController.php @@ -21,7 +21,7 @@ use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\RequestHandlerInterface; use Zend\Diactoros\Response\JsonResponse; -class TokenController implements RequestHandlerInterface +class CreateTokenController implements RequestHandlerInterface { /** * @var \Flarum\User\UserRepository diff --git a/src/Api/routes.php b/src/Api/routes.php index c532f7e03..8e3ccd2f3 100644 --- a/src/Api/routes.php +++ b/src/Api/routes.php @@ -25,7 +25,7 @@ return function (RouteCollection $map, RouteHandlerFactory $route) { $map->post( '/token', 'token', - $route->toController(Controller\TokenController::class) + $route->toController(Controller\CreateTokenController::class) ); // Send forgot password email diff --git a/src/Forum/Controller/LogInController.php b/src/Forum/Controller/LogInController.php index cf62d81e9..7e91168fd 100644 --- a/src/Forum/Controller/LogInController.php +++ b/src/Forum/Controller/LogInController.php @@ -12,7 +12,7 @@ namespace Flarum\Forum\Controller; use Flarum\Api\Client; -use Flarum\Api\Controller\TokenController; +use Flarum\Api\Controller\CreateTokenController; use Flarum\Http\AccessToken; use Flarum\Http\Rememberer; use Flarum\Http\SessionAuthenticator; @@ -67,7 +67,7 @@ class LogInController implements RequestHandlerInterface $body = $request->getParsedBody(); $params = array_only($body, ['identification', 'password']); - $response = $this->apiClient->send(TokenController::class, $actor, [], $params); + $response = $this->apiClient->send(CreateTokenController::class, $actor, [], $params); if ($response->getStatusCode() === 200) { $data = json_decode($response->getBody()); diff --git a/tests/Api/Controller/TokenControllerTest.php b/tests/Api/Controller/CreateTokenControllerTest.php similarity index 84% rename from tests/Api/Controller/TokenControllerTest.php rename to tests/Api/Controller/CreateTokenControllerTest.php index a350fa823..82d0f6de3 100644 --- a/tests/Api/Controller/TokenControllerTest.php +++ b/tests/Api/Controller/CreateTokenControllerTest.php @@ -11,15 +11,15 @@ namespace Flarum\Tests\Api\Controller; -use Flarum\Api\Controller\TokenController; +use Flarum\Api\Controller\CreateTokenController; use Flarum\Http\AccessToken; use Flarum\Tests\Test\Concerns\RetrievesAuthorizedUsers; -class TokenControllerTest extends ApiControllerTestCase +class CreateTokenControllerTest extends ApiControllerTestCase { use RetrievesAuthorizedUsers; - protected $controller = TokenController::class; + protected $controller = CreateTokenController::class; /** * @test From 305841ddd41f52d9d3d8fc9a660a6125d2c6ed60 Mon Sep 17 00:00:00 2001 From: Toby Zerner Date: Fri, 15 Jun 2018 19:17:43 +0930 Subject: [PATCH 04/10] Add Interface suffix --- src/Extend/Assets.php | 2 +- src/Extend/Compat.php | 2 +- src/Extend/{Extender.php => ExtenderInterface.php} | 2 +- src/Extend/FormatterConfiguration.php | 2 +- src/Extend/LanguagePack.php | 2 +- src/Extend/Locales.php | 2 +- src/Extend/Routes.php | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) rename src/Extend/{Extender.php => ExtenderInterface.php} (93%) diff --git a/src/Extend/Assets.php b/src/Extend/Assets.php index 6236f4a51..ad4fbd574 100644 --- a/src/Extend/Assets.php +++ b/src/Extend/Assets.php @@ -16,7 +16,7 @@ use Flarum\Frontend\Event\Rendering; use Illuminate\Contracts\Container\Container; use Illuminate\Events\Dispatcher; -class Assets implements Extender +class Assets implements ExtenderInterface { protected $appName; diff --git a/src/Extend/Compat.php b/src/Extend/Compat.php index 662f37536..a50d89191 100644 --- a/src/Extend/Compat.php +++ b/src/Extend/Compat.php @@ -23,7 +23,7 @@ use Illuminate\Contracts\Container\Container; * * @deprecated */ -class Compat implements Extender +class Compat implements ExtenderInterface { protected $callback; diff --git a/src/Extend/Extender.php b/src/Extend/ExtenderInterface.php similarity index 93% rename from src/Extend/Extender.php rename to src/Extend/ExtenderInterface.php index 138b4bbda..6f9bfc7c9 100644 --- a/src/Extend/Extender.php +++ b/src/Extend/ExtenderInterface.php @@ -14,7 +14,7 @@ namespace Flarum\Extend; use Flarum\Extension\Extension; use Illuminate\Contracts\Container\Container; -interface Extender +interface ExtenderInterface { public function __invoke(Container $container, Extension $extension = null); } diff --git a/src/Extend/FormatterConfiguration.php b/src/Extend/FormatterConfiguration.php index b82adf0b1..61a11d1a4 100644 --- a/src/Extend/FormatterConfiguration.php +++ b/src/Extend/FormatterConfiguration.php @@ -16,7 +16,7 @@ use Flarum\Formatter\Event\Configuring; use Illuminate\Contracts\Container\Container; use Illuminate\Events\Dispatcher; -class FormatterConfiguration implements Extender +class FormatterConfiguration implements ExtenderInterface { protected $callback; diff --git a/src/Extend/LanguagePack.php b/src/Extend/LanguagePack.php index 7d1ed4931..c8a7f0c09 100644 --- a/src/Extend/LanguagePack.php +++ b/src/Extend/LanguagePack.php @@ -18,7 +18,7 @@ use Illuminate\Contracts\Container\Container; use InvalidArgumentException; use RuntimeException; -class LanguagePack implements Extender +class LanguagePack implements ExtenderInterface { public function __invoke(Container $container, Extension $extension = null) { diff --git a/src/Extend/Locales.php b/src/Extend/Locales.php index 94409130c..8b2b83e97 100644 --- a/src/Extend/Locales.php +++ b/src/Extend/Locales.php @@ -16,7 +16,7 @@ use Flarum\Extension\Extension; use Flarum\Locale\LocaleManager; use Illuminate\Contracts\Container\Container; -class Locales implements Extender +class Locales implements ExtenderInterface { protected $directory; diff --git a/src/Extend/Routes.php b/src/Extend/Routes.php index 0a99895c6..c84cde797 100644 --- a/src/Extend/Routes.php +++ b/src/Extend/Routes.php @@ -15,7 +15,7 @@ use Flarum\Extension\Extension; use Flarum\Http\RouteHandlerFactory; use Illuminate\Contracts\Container\Container; -class Routes implements Extender +class Routes implements ExtenderInterface { protected $appName; From c498e68530e2190f3de56d97382ee83f7b922c1f Mon Sep 17 00:00:00 2001 From: Toby Zerner Date: Fri, 15 Jun 2018 19:18:47 +0930 Subject: [PATCH 05/10] Use imported class name --- src/Api/Controller/ShowUserController.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Api/Controller/ShowUserController.php b/src/Api/Controller/ShowUserController.php index 310643c3b..673e8920f 100644 --- a/src/Api/Controller/ShowUserController.php +++ b/src/Api/Controller/ShowUserController.php @@ -11,6 +11,7 @@ namespace Flarum\Api\Controller; +use Flarum\Api\Serializer\CurrentUserSerializer; use Flarum\Api\Serializer\UserSerializer; use Flarum\User\UserRepository; use Psr\Http\Message\ServerRequestInterface; @@ -55,7 +56,7 @@ class ShowUserController extends AbstractShowController $actor = $request->getAttribute('actor'); if ($actor->id == $id) { - $this->serializer = 'Flarum\Api\Serializer\CurrentUserSerializer'; + $this->serializer = CurrentUserSerializer::class; } return $this->users->findOrFail($id, $actor); From 569e6c9a92456ebbecd378aa54bb66d4ab10f24c Mon Sep 17 00:00:00 2001 From: Toby Zerner Date: Fri, 15 Jun 2018 19:19:43 +0930 Subject: [PATCH 06/10] Escape string used in LIKE query --- src/User/UserRepository.php | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/User/UserRepository.php b/src/User/UserRepository.php index 8619446b9..f294a63ac 100644 --- a/src/User/UserRepository.php +++ b/src/User/UserRepository.php @@ -90,6 +90,8 @@ class UserRepository */ public function getIdsForUsername($string, User $actor = null) { + $string = $this->escapeLikeString($string); + $query = User::where('username', 'like', '%'.$string.'%') ->orderByRaw('username = ? desc', [$string]) ->orderByRaw('username like ? desc', [$string.'%']); @@ -112,4 +114,15 @@ class UserRepository return $query; } + + /** + * Escape special characters that can be used as wildcards in a LIKE query. + * + * @param string $string + * @return string + */ + private function escapeLikeString($string) + { + return str_replace(['\\', '%', '_'], ['\\\\', '\%', '\_'], $string); + } } From 3b87778fbb8bf7dc131b505dea0cf395f9f1d3b9 Mon Sep 17 00:00:00 2001 From: Toby Zerner Date: Fri, 15 Jun 2018 19:24:23 +0930 Subject: [PATCH 07/10] =?UTF-8?q?Prevent=20@=20character=20used=20in=20sea?= =?UTF-8?q?rches=20from=20crashing=20MySQL=20=F0=9F=99=84?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/Discussion/Search/Gambit/FulltextGambit.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Discussion/Search/Gambit/FulltextGambit.php b/src/Discussion/Search/Gambit/FulltextGambit.php index 52307d60d..81f5e5976 100644 --- a/src/Discussion/Search/Gambit/FulltextGambit.php +++ b/src/Discussion/Search/Gambit/FulltextGambit.php @@ -29,6 +29,10 @@ class FulltextGambit implements GambitInterface throw new LogicException('This gambit can only be applied on a DiscussionSearch'); } + // The @ character crashes fulltext searches on InnoDB tables. + // See https://bugs.mysql.com/bug.php?id=74042 + $bit = str_replace('@', '*', $bit); + $search->getQuery() ->selectRaw('SUBSTRING_INDEX(GROUP_CONCAT(posts.id ORDER BY MATCH(posts.content) AGAINST (?) DESC), \',\', 1) as most_relevant_post_id', [$bit]) ->leftJoin('posts', 'posts.discussion_id', '=', 'discussions.id') From 050496a20e4f088694dd2a31873af45b91793249 Mon Sep 17 00:00:00 2001 From: Toby Zerner Date: Fri, 15 Jun 2018 19:25:15 +0930 Subject: [PATCH 08/10] Make ExtensionManager a singleton --- src/Extension/ExtensionServiceProvider.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Extension/ExtensionServiceProvider.php b/src/Extension/ExtensionServiceProvider.php index b2f5fcf92..a4d5217e9 100644 --- a/src/Extension/ExtensionServiceProvider.php +++ b/src/Extension/ExtensionServiceProvider.php @@ -21,7 +21,8 @@ class ExtensionServiceProvider extends AbstractServiceProvider */ public function register() { - $this->app->bind('flarum.extensions', ExtensionManager::class); + $this->app->singleton(ExtensionManager::class); + $this->app->alias(ExtensionManager::class, 'flarum.extensions'); $this->app->booting(function (Container $app) { $app->make('flarum.extensions')->extend($app); From 2bc7c4134a0272bb63a6f37d19fa6a9a795e4602 Mon Sep 17 00:00:00 2001 From: Toby Zerner Date: Fri, 15 Jun 2018 19:25:40 +0930 Subject: [PATCH 09/10] Add comment explaining extension boot process --- src/Extension/ExtensionServiceProvider.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Extension/ExtensionServiceProvider.php b/src/Extension/ExtensionServiceProvider.php index a4d5217e9..fa61df382 100644 --- a/src/Extension/ExtensionServiceProvider.php +++ b/src/Extension/ExtensionServiceProvider.php @@ -24,6 +24,10 @@ class ExtensionServiceProvider extends AbstractServiceProvider $this->app->singleton(ExtensionManager::class); $this->app->alias(ExtensionManager::class, 'flarum.extensions'); + // Boot extensions when the app is booting. This must be done as a boot + // listener on the app rather than in the service provider's boot method + // below, so that extensions have a chance to register things on the + // container before the core boot code runs. $this->app->booting(function (Container $app) { $app->make('flarum.extensions')->extend($app); }); From 07eda605615b20b7a9152b2eb1a78c735f8234e7 Mon Sep 17 00:00:00 2001 From: Toby Zerner Date: Sat, 16 Jun 2018 11:01:42 +0930 Subject: [PATCH 10/10] Fix discussion posts not being initialized correctly. Fixes #1455 --- js/forum/src/components/DiscussionPage.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/js/forum/src/components/DiscussionPage.js b/js/forum/src/components/DiscussionPage.js index bc73a8a46..771004b17 100644 --- a/js/forum/src/components/DiscussionPage.js +++ b/js/forum/src/components/DiscussionPage.js @@ -172,8 +172,13 @@ export default class DiscussionPage extends Page { // the 'discussion' relationship linked, then sorting and splicing. let includedPosts = []; if (discussion.payload && discussion.payload.included) { + const discussionId = discussion.id(); + includedPosts = discussion.payload.included - .filter(record => record.type === 'posts' && record.relationships && record.relationships.discussion) + .filter(record => record.type === 'posts' + && record.relationships + && record.relationships.discussion + && record.relationships.discussion.data.id === discussionId) .map(record => app.store.getById('posts', record.id)) .sort((a, b) => a.id() - b.id()) .slice(0, 20);