EBernhardson has uploaded a new change for review.
https://gerrit.wikimedia.org/r/318023
Change subject: Refactor repositories out of QueriesController
......................................................................
Refactor repositories out of QueriesController
This class had quite a few repositories, and i was about to add one
more. In a (perhaps misguided) attempt at the single responsibility
principle pull all the repositories out of QueriesController into a
class with less responsibilities, QueriesManager. Now QueriesController
can handle the forms/templating/etc end of things, and QueriesManager
can handle shuffling data back and forth from the database.
Change-Id: Ia5fb71aedf3d13f5c9376394555d6ae9a2bf0ddc
---
M src/RelevanceScoring/Controller/QueriesController.php
A src/RelevanceScoring/QueriesManager.php
M src/RelevanceScoring/RelevanceScoringProvider.php
3 files changed, 147 insertions(+), 85 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/wikimedia/discovery/discernatron
refs/changes/23/318023/1
diff --git a/src/RelevanceScoring/Controller/QueriesController.php
b/src/RelevanceScoring/Controller/QueriesController.php
index fd2b822..172f4cb 100644
--- a/src/RelevanceScoring/Controller/QueriesController.php
+++ b/src/RelevanceScoring/Controller/QueriesController.php
@@ -8,11 +8,7 @@
use WikiMedia\OAuth\User;
use WikiMedia\RelevanceScoring\Application;
use WikiMedia\RelevanceScoring\Assert\MinimumSubmitted;
-use WikiMedia\RelevanceScoring\Repository\QueriesRepository;
-use WikiMedia\RelevanceScoring\Repository\ResultsRepository;
-use WikiMedia\RelevanceScoring\Repository\ScoresRepository;
-use WikiMedia\RelevanceScoring\Repository\ScoringQueueRepository;
-use WikiMedia\RelevanceScoring\Repository\UsersRepository;
+use WikiMedia\RelevanceScoring\QueriesManager;
class QueriesController
{
@@ -24,14 +20,8 @@
private $twig;
/** @var FormFactory */
private $formFactory;
- /** @var QueriesRepository */
- private $queriesRepo;
- /** @var ResultsRepository */
- private $resultsRepo;
- /** @var ScoresRepository */
- private $scoresRepo;
- /** @var UsersRepository */
- private $userRepo;
+ /** @var QueriesManager */
+ private $queriesManager;
/** @var string[] */
private $wikis;
@@ -40,22 +30,14 @@
User $user,
Twig_Environment $twig,
FormFactory $formFactory,
- QueriesRepository $queriesRepo,
- ResultsRepository $resultsRepo,
- ScoresRepository $scoresRepo,
- ScoringQueueRepository $scoringQueueRepo,
- UsersRepository $userRepo,
+ QueriesManager $queriesManager,
array $wikis
) {
$this->app = $app;
$this->user = $user;
$this->twig = $twig;
$this->formFactory = $formFactory;
- $this->queriesRepo = $queriesRepo;
- $this->resultsRepo = $resultsRepo;
- $this->scoresRepo = $scoresRepo;
- $this->scoringQueueRepo = $scoringQueueRepo;
- $this->userRepo = $userRepo;
+ $this->queriesManager = $queriesManager;
$this->wikis = $wikis;
}
@@ -66,7 +48,7 @@
public function nextQuery(Request $request)
{
- $maybeId = $this->scoringQueueRepo->pop($this->user);
+ $maybeId = $this->queriesManager->nextQueryId();
$params = [];
if ($request->query->get('saved')) {
$params['saved'] = 1;
@@ -82,7 +64,7 @@
public function skipQueryById(Request $request, $queryId)
{
- $maybeQuery = $this->queriesRepo->getQuery($queryId);
+ $maybeQuery = $this->queriesManager->getQuery($queryId);
if ($maybeQuery->isEmpty()) {
// @todo 404
throw new \Exception('Query not found');
@@ -95,8 +77,7 @@
// look into adding session based notifications to make it easier to
// tell users about this.
if ($form->isValid()) {
- $this->queriesRepo->markQuerySkipped($this->user, $queryId);
- $this->scoringQueueRepo->unassignUser($this->user);
+ $this->queriesManager->skipQuery($queryId);
}
return $this->app->redirect($this->app->path('next_query'));
@@ -104,7 +85,7 @@
public function queryById(Request $request, $queryId)
{
- $maybeQuery = $this->queriesRepo->getQuery($queryId);
+ $maybeQuery = $this->queriesManager->getQuery($queryId);
if ($maybeQuery->isEmpty()) {
// @todo 404
throw new \Exception('Query not found');
@@ -116,20 +97,15 @@
]);
}
- $maybeResults = $this->resultsRepo->getQueryResults($queryId);
+ $maybeResults = $this->queriesManager->getQueryResults($queryId);
if ($maybeResults->isEmpty()) {
throw new \Exception('No results found for query');
}
- $results = $this->shufflePreserveKeys(
- $maybeResults->get(),
- // user id is used to give each user a different order,
- // but the same user gets same order each time.
- $this->user->uid
- );
// When encoded to json we will lose the ordering, so
// add a key to identify the order
$position = 0;
+ $results = $maybeResults->get();
foreach (array_keys($results) as $id) {
$results[$id]['order'] = $position++;
}
@@ -138,8 +114,7 @@
$form->handleRequest($request);
if ($form->isValid() && !$request->request->has('cards')) {
- $this->scoresRepo->storeQueryScores($this->user, $queryId,
$form->getData());
- $this->scoringQueueRepo->markScored($this->user, $queryId);
+ $this->queriesManager->saveScores($queryId, $form->getData());
return $this->app->redirect($this->app->path('next_query',
['saved' => 1]));
}
@@ -207,48 +182,6 @@
return $builder->getForm();
}
- /**
- * PHP's shuffle function loses the keys. So sort the keys
- * and make a new array based on the order of sorted keys.
- * Additionally php's shuffle is automatically seeded so we
- * can't get the same order across requests. Fix that by using
- * a local fisher yates implementation.
- *
- * @param array $array
- *
- * @return array
- */
- private function shufflePreserveKeys(array $array, $seed)
- {
- $keys = $this->fisherYatesShuffle(array_keys($array), $seed);
- $result = array();
- foreach ($keys as $key) {
- $result[$key] = $array[$key];
- }
-
- return $result;
- }
-
- /**
- * @param array $array Must be numerically indexed starting
- * from 0 with no gaps.
- * @param int $seed
- *
- * @return array
- */
- private function fisherYatesShuffle(array $array, $seed)
- {
- mt_srand($seed);
- for ($i = count($array) - 1; $i > 0; --$i) {
- $j = mt_rand(0, $i);
- $tmp = $array[$i];
- $array[$i] = $array[$j];
- $array[$j] = $tmp;
- }
-
- return $array;
- }
-
private function chooseScoringTemplate(Request $request)
{
$fromQuery = $request->query->get('cards', null);
@@ -263,7 +196,7 @@
: 'classic';
if ($interface !== $this->user->extra['scoringInterface']) {
$this->user->extra['scoringInterface'] = $interface;
- $this->userRepo->updateUser($this->user);
+ $this->queriesManager->updateUserStorage();
}
}
diff --git a/src/RelevanceScoring/QueriesManager.php
b/src/RelevanceScoring/QueriesManager.php
new file mode 100644
index 0000000..4a642a7
--- /dev/null
+++ b/src/RelevanceScoring/QueriesManager.php
@@ -0,0 +1,122 @@
+<?php
+
+namespace WikiMedia\RelevanceScoring;
+
+use PlasmaConduit\option\Some;
+use WikiMedia\OAuth\User;
+use WikiMedia\RelevanceScoring\Repository\ResultsRepository;
+use WikiMedia\RelevanceScoring\Repository\ScoresRepository;
+use WikiMedia\RelevanceScoring\Repository\ScoringQueueRepository;
+use WikiMedia\RelevanceScoring\Repository\QueriesRepository;
+use WikiMedia\RelevanceScoring\Repository\UsersRepository;
+
+
+/**
+ * Handles the data read/write patterns used by the QueriesController
+ */
+class QueriesManager {
+ /** @var User */
+ private $user;
+ /** @var ResultsRepository */
+ private $resultsRepository;
+ /** @var ScoresRepository */
+ private $scoresRepo;
+ /** @var ScoringQueueRepository */
+ private $scoringQueueRepo;
+ /** @var QueriesRepository */
+ private $queriesRepo;
+ /** @var UsersRepository */
+ private $usersRepo;
+
+ public function __construct(
+ User $user,
+ QueriesRepository $queriesRepo,
+ ResultsRepository $resultsRepo,
+ ScoresRepository $scoresRepo,
+ ScoringQueueRepository $scoringQueueRepo,
+ UsersRepository $usersRepo
+ ) {
+ $this->user = $user;
+ $this->resultsRepo = $resultsRepo;
+ $this->scoresRepo = $scoresRepo;
+ $this->scoringQueueRepo = $scoringQueueRepo;
+ $this->queriesRepo = $queriesRepo;
+ $this->usersRepo = $usersRepo;
+ }
+
+ public function nextQueryId() {
+ return $this->scoringQueueRepo->pop($this->user);
+ }
+
+ public function getQuery($queryId) {
+ return $this->queriesRepo->getQuery($queryId);
+ }
+
+ public function getQueryResults($queryId) {
+ $maybeResults = $this->resultsRepo->getQueryResults($queryId);
+ if ( $maybeResults->isEmpty()) {
+ return $maybeResults;
+ }
+
+ $results = $this->shufflePreserveKeys(
+ $maybeResults->get(),
+ $this->user->uid
+ );
+ return new Some($results);
+ }
+
+ public function skipQuery($queryId) {
+ $this->queriesRepo->markQuerySkipped($this->user, $queryId);
+ $this->scoringQueueRepo->unassignUser($this->user);
+ }
+
+ public function saveScores($queryId, array $scores) {
+ $this->scoresRepo->storeQueryScores($this->user, $queryId, $scores);
+ $this->scoringQueueRepo->markScored($this->user, $queryId);
+ }
+
+ public function updateUserStorage() {
+ $this->userRepo->updateUser($this->user);
+ }
+ /**
+ * PHP's shuffle function loses the keys. So sort the keys
+ * and make a new array based on the order of sorted keys.
+ * Additionally php's shuffle is automatically seeded so we
+ * can't get the same order across requests. Fix that by using
+ * a local fisher yates implementation.
+ *
+ * @param array $array
+ *
+ * @return array
+ */
+ private function shufflePreserveKeys(array $array, $seed)
+ {
+ $keys = $this->fisherYatesShuffle(array_keys($array), $seed);
+ $result = array();
+ foreach ($keys as $key) {
+ $result[$key] = $array[$key];
+ }
+
+ return $result;
+ }
+
+ /**
+ * @param array $array Must be numerically indexed starting
+ * from 0 with no gaps.
+ * @param int $seed
+ *
+ * @return array
+ */
+ private function fisherYatesShuffle(array $array, $seed)
+ {
+ mt_srand($seed);
+ for ($i = count($array) - 1; $i > 0; --$i) {
+ $j = mt_rand(0, $i);
+ $tmp = $array[$i];
+ $array[$i] = $array[$j];
+ $array[$j] = $tmp;
+ }
+
+ return $array;
+ }
+}
diff --git a/src/RelevanceScoring/RelevanceScoringProvider.php
b/src/RelevanceScoring/RelevanceScoringProvider.php
index b477b23..5421111 100644
--- a/src/RelevanceScoring/RelevanceScoringProvider.php
+++ b/src/RelevanceScoring/RelevanceScoringProvider.php
@@ -245,17 +245,24 @@
private function registerControllers(Application $app)
{
+ // helper for queries controller
+ $app['search.queries_manager'] = function () use ($app) {
+ return new QueriesManager(
+ $app['session']->get('user'),
+ $app['search.repository.queries'],
+ $app['search.repository.results'],
+ $app['search.repository.scores'],
+ $app['search.repository.scoring_queue'],
+ $app['search.repository.users']
+ );
+ };
$app['search.controller.queries'] = function () use ($app) {
return new QueriesController(
$app,
$app['session']->get('user'),
$app['twig'],
$app['form.factory'],
- $app['search.repository.queries'],
- $app['search.repository.results'],
- $app['search.repository.scores'],
- $app['search.repository.scoring_queue'],
- $app['search.repository.users'],
+ $app['search.queries_manager'],
$app['search.wikis']
);
};
--
To view, visit https://gerrit.wikimedia.org/r/318023
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia5fb71aedf3d13f5c9376394555d6ae9a2bf0ddc
Gerrit-PatchSet: 1
Gerrit-Project: wikimedia/discovery/discernatron
Gerrit-Branch: master
Gerrit-Owner: EBernhardson <ebernhard...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
