Fix issues with search database table and entity interface (vufind-or…

…g#3757)
maccabeelevine · Jun 6, 2024 · 43c5b41 · 43c5b41
1 parent 11de475
commit 43c5b41
Show file tree

Hide file tree

Showing 15 changed files with 114 additions and 40 deletions.
diff --git a/module/VuFind/sql/migrations/pgsql/10.0/002-modify-search-user-column.sql b/module/VuFind/sql/migrations/pgsql/10.0/002-modify-search-user-column.sql
@@ -0,0 +1,11 @@
+--
+-- Modifications to user_id column in table `comments`
+--
+
+ALTER TABLE "search"
+   ALTER COLUMN user_id DROP NOT NULL;
+
+ALTER TABLE "search"
+   ALTER COLUMN user_id SET DEFAULT NULL;
+
+UPDATE "search" SET user_id=NULL WHERE user_id='0' OR user_id NOT IN (SELECT DISTINCT id FROM "user");
diff --git a/module/VuFind/sql/mysql.sql b/module/VuFind/sql/mysql.sql
@@ -133,7 +133,7 @@ CREATE TABLE `resource_tags` (
 /*!40101 SET character_set_client = utf8mb4 */;
 CREATE TABLE `search` (
   `id` bigint unsigned NOT NULL AUTO_INCREMENT,
-  `user_id` int(11) NOT NULL DEFAULT '0',
+  `user_id` int(11) DEFAULT NULL,
   `session_id` varchar(128) DEFAULT NULL,
   `created` datetime NOT NULL DEFAULT '2000-01-01 00:00:00',
   `title` varchar(20) DEFAULT NULL,

diff --git a/module/VuFind/sql/pgsql.sql b/module/VuFind/sql/pgsql.sql
@@ -90,7 +90,7 @@ DROP TABLE IF EXISTS "search";
 
 CREATE TABLE search (
 id BIGSERIAL,
-user_id int NOT NULL DEFAULT '0',
+user_id int DEFAULT NULL,
 session_id varchar(128),
 created timestamp NOT NULL DEFAULT '1970-01-01 00:00:00',
 title varchar(20) DEFAULT NULL,

diff --git a/module/VuFind/src/VuFind/Controller/AbstractSearch.php b/module/VuFind/src/VuFind/Controller/AbstractSearch.php
@@ -141,7 +141,7 @@ protected function redirectToSavedSearch($id)
 
         // If we got this far, the user is allowed to view the search, so we can
         // deminify it to a new object.
-        $minSO = $search->getSearchObject();
+        $minSO = $search->getSearchObjectOrThrowException();
         $savedSearch = $minSO->deminify($this->getResultsManager());
 
         // Now redirect to the URL associated with the saved search; this
@@ -562,7 +562,7 @@ protected function restoreAdvancedSearch($searchId)
         }
 
         // Restore the full search object:
-        $minSO = $search->getSearchObject();
+        $minSO = $search->getSearchObjectOrThrowException();
         $savedSearch = $minSO->deminify($this->getResultsManager());
 
         // Fail if this is not the right type of search:

diff --git a/module/VuFind/src/VuFind/Controller/MyResearchController.php b/module/VuFind/src/VuFind/Controller/MyResearchController.php
@@ -593,7 +593,7 @@ public function schedulesearchAction()
         // Now fetch all the results:
         $resultsManager = $this->serviceLocator
             ->get(\VuFind\Search\Results\PluginManager::class);
-        $results = $search->getSearchObject()->deminify($resultsManager);
+        $results = $search->getSearchObjectOrThrowException()->deminify($resultsManager);
 
         // Build the form.
         return $this->createViewModel(
@@ -623,7 +623,7 @@ protected function isDuplicateOfSavedSearch(
         $normalizer = $this->serviceLocator
             ->get(\VuFind\Search\SearchNormalizer::class);
         $normalized = $normalizer
-            ->normalizeMinifiedSearch($rowToCheck->getSearchObject());
+            ->normalizeMinifiedSearch($rowToCheck->getSearchObjectOrThrowException());
         $matches = $searchTable->getSearchRowsMatchingNormalizedSearch(
             $normalized,
             $sessId,

diff --git a/module/VuFind/src/VuFind/Controller/Plugin/ResultScroller.php b/module/VuFind/src/VuFind/Controller/Plugin/ResultScroller.php
@@ -666,7 +666,7 @@ protected function restoreSearch(int $searchId): ?Results
             null
         );
         if (!empty($row)) {
-            $minSO = $row->getSearchObject();
+            $minSO = $row->getSearchObjectOrThrowException();
             $search = $minSO->deminify($this->resultsManager);
             // The saved search does not remember its original limit or sort;
             // we should reapply them from the session data:

diff --git a/module/VuFind/src/VuFind/Controller/UpgradeController.php b/module/VuFind/src/VuFind/Controller/UpgradeController.php
@@ -49,6 +49,7 @@
 use VuFind\Crypt\Base62;
 use VuFind\Db\AdapterFactory;
 use VuFind\Db\Service\ResourceServiceInterface;
+use VuFind\Db\Service\SearchServiceInterface;
 use VuFind\Exception\RecordMissing as RecordMissingException;
 use VuFind\Record\ResourcePopulator;
 use VuFind\Search\Results\PluginManager as ResultsManager;
@@ -327,6 +328,20 @@ protected function fixVuFindSourceInDatabase()
         }
     }
 
+    /**
+     * Support method for fixdatabaseAction() -- clean up invalid user ID
+     * values in the search table.
+     *
+     * @return void
+     */
+    protected function fixInvalidUserIdsInSearchTable(): void
+    {
+        $count = $this->getDbService(SearchServiceInterface::class)->cleanUpInvalidUserIds();
+        if ($count) {
+            $this->session->warnings->append("Converted $count invalid user_id values in search table");
+        }
+    }
+
     /**
      * Support method for fixdatabaseAction() -- add checksums to search table rows.
      *
@@ -340,7 +355,7 @@ protected function fixSearchChecksumsInDatabase()
         $searchRows = $search->select($searchWhere);
         if (count($searchRows) > 0) {
             foreach ($searchRows as $searchRow) {
-                $searchObj = $searchRow->getSearchObject()->deminify($manager);
+                $searchObj = $searchRow->getSearchObjectOrThrowException()->deminify($manager);
                 $url = $searchObj->getUrlQuery()->getParams();
                 $checksum = crc32($url) & 0xFFFFFFF;
                 $searchRow->checksum = $checksum;
@@ -586,6 +601,9 @@ public function fixdatabaseAction()
 
             // Clean up the "VuFind" source, if necessary.
             $this->fixVuFindSourceInDatabase();
+
+            // Fix invalid user IDs in search table, if necessary.
+            $this->fixInvalidUserIdsInSearchTable();
         } catch (Exception $e) {
             $this->flashMessenger()->addMessage(
                 'Database upgrade failed: ' . $e->getMessage(),

diff --git a/module/VuFind/src/VuFind/Db/Entity/SearchEntityInterface.php b/module/VuFind/src/VuFind/Db/Entity/SearchEntityInterface.php
@@ -52,18 +52,18 @@ public function getId(): ?int;
     /**
      * Get user.
      *
-     * @return UserEntityInterface
+     * @return ?UserEntityInterface
      */
-    public function getUser(): UserEntityInterface;
+    public function getUser(): ?UserEntityInterface;
 
     /**
      * Set user.
      *
-     * @param int $user User
+     * @param ?UserEntityInterface $user User
      *
      * @return SearchEntityInterface
      */
-    public function setUser(UserEntityInterface $user): SearchEntityInterface;
+    public function setUser(?UserEntityInterface $user): SearchEntityInterface;
 
     /**
      * Get session identifier.
@@ -75,11 +75,11 @@ public function getSessionId(): ?string;
     /**
      * Set session identifier.
      *
-     * @param ?string $session_id Session id
+     * @param ?string $sessionId Session id
      *
      * @return SearchEntityInterface
      */
-    public function setSessionId(?string $session_id): SearchEntityInterface;
+    public function setSessionId(?string $sessionId): SearchEntityInterface;
 
     /**
      * Get created date.
@@ -132,18 +132,18 @@ public function setSaved(bool $saved): SearchEntityInterface;
     /**
      * Get the search object from the row.
      *
-     * @return \VuFind\Search\Minified
+     * @return ?\VuFind\Search\Minified
      */
-    public function getSearchObject(): \VuFind\Search\Minified;
+    public function getSearchObject(): ?\VuFind\Search\Minified;
 
     /**
      * Set search object.
      *
-     * @param \VuFind\Search\Minified $searchObject Search object
+     * @param ?\VuFind\Search\Minified $searchObject Search object
      *
      * @return SearchEntityInterface
      */
-    public function setSearchObject(\VuFind\Search\Minified $searchObject): SearchEntityInterface;
+    public function setSearchObject(?\VuFind\Search\Minified $searchObject): SearchEntityInterface;
 
     /**
      * Get checksum.

diff --git a/module/VuFind/src/VuFind/Db/Row/Search.php b/module/VuFind/src/VuFind/Db/Row/Search.php
@@ -97,14 +97,24 @@ protected function normalizeSearchObject()
     /**
      * Get the search object from the row.
      *
-     * @return \VuFind\Search\Minified
+     * @return ?\VuFind\Search\Minified
      */
-    public function getSearchObject(): \VuFind\Search\Minified
+    public function getSearchObject(): ?\VuFind\Search\Minified
     {
         // We need to make sure the search object is a string before unserializing:
         $this->normalizeSearchObject();
-        $result = unserialize($this->search_object);
-        if (!($result instanceof \VuFind\Search\Minified)) {
+        return $this->search_object ? unserialize($this->search_object) : null;
+    }
+
+    /**
+     * Get the search object from the row, and throw an exception if it is missing.
+     *
+     * @return \VuFind\Search\Minified
+     * @throws \Exception
+     */
+    public function getSearchObjectOrThrowException(): \VuFind\Search\Minified
+    {
+        if (!($result = $this->getSearchObject())) {
             throw new \Exception('Problem decoding saved search');
         }
         return $result;
@@ -185,23 +195,25 @@ public function getId(): ?int
     /**
      * Get user.
      *
-     * @return UserEntityInterface
+     * @return ?UserEntityInterface
      */
-    public function getUser(): UserEntityInterface
+    public function getUser(): ?UserEntityInterface
     {
-        return $this->getDbServiceManager()->get(UserServiceInterface::class)->getUserById($this->user_id);
+        return $this->user_id
+            ? $this->getDbServiceManager()->get(UserServiceInterface::class)->getUserById($this->user_id)
+            : null;
     }
 
     /**
      * Set user.
      *
-     * @param UserEntityInterface $user User
+     * @param ?UserEntityInterface $user User
      *
      * @return SearchEntityInterface
      */
-    public function setUser(UserEntityInterface $user): SearchEntityInterface
+    public function setUser(?UserEntityInterface $user): SearchEntityInterface
     {
-        $this->user_id = $user->getId();
+        $this->user_id = $user?->getId();
         return $this;
     }
 
@@ -218,13 +230,13 @@ public function getSessionId(): ?string
     /**
      * Set session identifier.
      *
-     * @param ?string $session_id Session id
+     * @param ?string $sessionId Session id
      *
      * @return SearchEntityInterface
      */
-    public function setSessionId(?string $session_id): SearchEntityInterface
+    public function setSessionId(?string $sessionId): SearchEntityInterface
     {
-        $this->session_id = $session_id;
+        $this->session_id = $sessionId;
         return $this;
     }
 
@@ -300,13 +312,13 @@ public function setSaved(bool $saved): SearchEntityInterface
     /**
      * Set search object.
      *
-     * @param \VuFind\Search\Minified $searchObject Search object
+     * @param ?\VuFind\Search\Minified $searchObject Search object
      *
      * @return SearchEntityInterface
      */
-    public function setSearchObject(\VuFind\Search\Minified $searchObject): SearchEntityInterface
+    public function setSearchObject(?\VuFind\Search\Minified $searchObject): SearchEntityInterface
     {
-        $this->search_object = serialize($searchObject);
+        $this->search_object = $searchObject ? serialize($searchObject) : null;
         return $this;
     }
 

diff --git a/module/VuFind/src/VuFind/Db/Service/SearchService.php b/module/VuFind/src/VuFind/Db/Service/SearchService.php
@@ -29,6 +29,11 @@
 
 namespace VuFind\Db\Service;
 
+use VuFind\Db\Table\DbTableAwareInterface;
+use VuFind\Db\Table\DbTableAwareTrait;
+
+use function count;
+
 /**
  * Database service for search.
  *
@@ -38,6 +43,28 @@
  * @license  http://opensource.org/licenses/gpl-2.0.php GNU General Public License
  * @link     https://vufind.org/wiki/development:plugins:database_gateways Wiki
  */
-class SearchService extends AbstractDbService implements SearchServiceInterface
+class SearchService extends AbstractDbService implements SearchServiceInterface, DbTableAwareInterface
 {
+    use DbTableAwareTrait;
+
+    /**
+     * Set invalid user_id values in the table to null; return count of affected rows.
+     *
+     * @return int
+     */
+    public function cleanUpInvalidUserIds(): int
+    {
+        $searchTable = $this->getDbTable('search');
+        $allIds = $this->getDbTable('user')->getSql()->select()->columns(['id']);
+        $searchCallback = function ($select) use ($allIds) {
+            $select->where->equalTo('user_id', '0')
+                ->OR->notIn('user_id', $allIds);
+        };
+        $badRows = $searchTable->select($searchCallback);
+        $count = count($badRows);
+        if ($count > 0) {
+            $searchTable->update(['user_id' => null], $searchCallback);
+        }
+        return $count;
+    }
 }
diff --git a/module/VuFind/src/VuFind/Db/Service/SearchServiceInterface.php b/module/VuFind/src/VuFind/Db/Service/SearchServiceInterface.php
@@ -40,4 +40,10 @@
  */
 interface SearchServiceInterface extends DbServiceInterface
 {
+    /**
+     * Set invalid user_id values in the table to null; return count of affected rows.
+     *
+     * @return int
+     */
+    public function cleanUpInvalidUserIds(): int;
 }
diff --git a/module/VuFind/src/VuFind/Db/Table/Search.php b/module/VuFind/src/VuFind/Db/Table/Search.php
@@ -251,7 +251,7 @@ public function getSearchRowsMatchingNormalizedSearch(
         };
         $results = [];
         foreach ($this->select($callback) as $match) {
-            $minified = $match->getSearchObject();
+            $minified = $match->getSearchObjectOrThrowException();
             if ($normalized->isEquivalentToMinifiedSearch($minified)) {
                 $results[] = $match;
                 if (count($results) >= $limit) {
@@ -293,7 +293,7 @@ public function saveSearch(
                 $existingRow->created = date('Y-m-d H:i:s');
                 // Keep the ID of the old search:
                 $minified = $normalized->getMinified();
-                $minified->id = $existingRow->getSearchObject()->id;
+                $minified->id = $existingRow->getSearchObjectOrThrowException()->id;
                 $existingRow->search_object = serialize($minified);
                 $existingRow->session_id = $sessionId;
                 $existingRow->save();

diff --git a/module/VuFind/src/VuFind/Search/History.php b/module/VuFind/src/VuFind/Search/History.php
@@ -119,7 +119,7 @@ public function getSearchHistory($userId = null)
         // Loop through and sort the history
         $saved = $schedule = $unsaved = [];
         foreach ($searchHistory as $current) {
-            $search = $current->getSearchObject()->deminify($this->resultsManager);
+            $search = $current->getSearchObjectOrThrowException()->deminify($this->resultsManager);
             // $current->saved may be 1 (MySQL) or true (PostgreSQL), so we should
             // avoid a strict === comparison here:
             if ($current->saved == 1) {

diff --git a/module/VuFind/src/VuFind/Search/Memory.php b/module/VuFind/src/VuFind/Search/Memory.php
@@ -304,7 +304,7 @@ protected function getSearchById(int $id): ?\VuFind\Search\Base\Results
             $search
                 = $this->searchTable->getOwnedRowById($id, $this->sessionId, null);
             if ($search) {
-                $minSO = $search->getSearchObject();
+                $minSO = $search->getSearchObjectOrThrowException();
                 $this->searchCache[$id] = $minSO->deminify($this->resultsManager);
             } else {
                 $this->searchCache[$id] = null;

diff --git a/module/VuFindConsole/src/VuFindConsole/Command/ScheduledSearch/NotifyCommand.php b/module/VuFindConsole/src/VuFindConsole/Command/ScheduledSearch/NotifyCommand.php
@@ -362,7 +362,7 @@ protected function setLanguage($userLang)
      */
     protected function getObjectForSearch($s)
     {
-        $minSO = $s->getSearchObject();
+        $minSO = $s->getSearchObjectOrThrowException();
         $searchObject = $minSO->deminify($this->resultsManager);
         if (!$searchObject->getOptions()->supportsScheduledSearch()) {
             $this->err(