From a106c2330289abc3ceb32e4ee5f6c7ca41579ae0 Mon Sep 17 00:00:00 2001
From: Demian Katz <demian.katz@villanova.edu>
Date: Thu, 6 Feb 2020 09:16:17 -0500
Subject: [PATCH] Add getIds method to search service. - Provides a faster way
 to retrieve lists of identifiers without extra baggage. - Fails over to proxy
 the regular search method when unsupported by the backend.

---
 .../Search/Solr/DeduplicationListener.php     |  4 +-
 .../src/VuFindSearch/Backend/Solr/Backend.php | 32 +++++++++-
 .../VuFindSearch/Feature/GetIdsInterface.php  | 59 ++++++++++++++++++
 .../VuFindSearch/src/VuFindSearch/Service.php | 39 +++++++++++-
 .../VuFindTest/Backend/Solr/BackendTest.php   | 39 ++++++++++++
 .../src/VuFindTest/SearchServiceTest.php      | 62 +++++++++++++++++++
 6 files changed, 230 insertions(+), 5 deletions(-)
 create mode 100644 module/VuFindSearch/src/VuFindSearch/Feature/GetIdsInterface.php

diff --git a/module/VuFind/src/VuFind/Search/Solr/DeduplicationListener.php b/module/VuFind/src/VuFind/Search/Solr/DeduplicationListener.php
index 4c987a65edf..a4e66f76dcc 100644
--- a/module/VuFind/src/VuFind/Search/Solr/DeduplicationListener.php
+++ b/module/VuFind/src/VuFind/Search/Solr/DeduplicationListener.php
@@ -134,10 +134,10 @@ class DeduplicationListener
         if ($backend === $this->backend) {
             $params = $event->getParam('params');
             $context = $event->getParam('context');
-            if (($context == 'search' || $context == 'similar') && $params) {
+            if ($params && in_array($context, ['search', 'similar', 'getids'])) {
                 // If deduplication is enabled, filter out merged child records,
                 // otherwise filter out dedup records.
-                if ($this->enabled) {
+                if ($this->enabled && 'getids' !== $context) {
                     $fq = '-merged_child_boolean:true';
                     if ($context == 'similar' && $id = $event->getParam('id')) {
                         $fq .= ' AND -local_ids_str_mv:"'
diff --git a/module/VuFindSearch/src/VuFindSearch/Backend/Solr/Backend.php b/module/VuFindSearch/src/VuFindSearch/Backend/Solr/Backend.php
index 1c7414c5f2e..f44a03e2d89 100644
--- a/module/VuFindSearch/src/VuFindSearch/Backend/Solr/Backend.php
+++ b/module/VuFindSearch/src/VuFindSearch/Backend/Solr/Backend.php
@@ -34,7 +34,7 @@ use VuFindSearch\Backend\Exception\RemoteErrorException;
 
 use VuFindSearch\Backend\Solr\Response\Json\Terms;
 use VuFindSearch\Exception\InvalidArgumentException;
-
+use VuFindSearch\Feature\GetIdsInterface;
 use VuFindSearch\Feature\RandomInterface;
 
 use VuFindSearch\Feature\RetrieveBatchInterface;
@@ -57,7 +57,8 @@ use VuFindSearch\Response\RecordCollectionInterface;
  * @link     https://vufind.org
  */
 class Backend extends AbstractBackend
-    implements SimilarInterface, RetrieveBatchInterface, RandomInterface
+    implements SimilarInterface, RetrieveBatchInterface, RandomInterface,
+    GetIdsInterface
 {
     /**
      * Connector.
@@ -119,6 +120,33 @@ class Backend extends AbstractBackend
         return $collection;
     }
 
+    /**
+     * Perform a search and return record collection of only record identifiers.
+     *
+     * @param AbstractQuery $query  Search query
+     * @param int           $offset Search offset
+     * @param int           $limit  Search limit
+     * @param ParamBag      $params Search backend parameters
+     *
+     * @return RecordCollectionInterface
+     */
+    public function getIds(AbstractQuery $query, $offset, $limit,
+        ParamBag $params = null
+    ) {
+        $params = $params ?: new ParamBag();
+        $this->injectResponseWriter($params);
+
+        $params->set('rows', $limit);
+        $params->set('start', $offset);
+        $params->set('fl', $this->getConnector()->getUniqueKey());
+        $params->mergeWith($this->getQueryBuilder()->build($query));
+        $response   = $this->connector->search($params);
+        $collection = $this->createRecordCollection($response);
+        $this->injectSourceIdentifier($collection);
+
+        return $collection;
+    }
+
     /**
      * Get Random records
      *
diff --git a/module/VuFindSearch/src/VuFindSearch/Feature/GetIdsInterface.php b/module/VuFindSearch/src/VuFindSearch/Feature/GetIdsInterface.php
new file mode 100644
index 00000000000..80d44d04df6
--- /dev/null
+++ b/module/VuFindSearch/src/VuFindSearch/Feature/GetIdsInterface.php
@@ -0,0 +1,59 @@
+<?php
+
+/**
+ * Optional backend feature: Get identifiers of records.
+ *
+ * PHP version 7
+ *
+ * Copyright (C) Villanova University 2010.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ * @category VuFind
+ * @package  Search
+ * @author   David Maus <maus@hab.de>
+ * @author   Ere Maijala <ere.maijala@helsinki.fi>
+ * @license  http://opensource.org/licenses/gpl-2.0.php GNU General Public License
+ * @link     https://vufind.org
+ */
+namespace VuFindSearch\Feature;
+
+use VuFindSearch\ParamBag;
+use VuFindSearch\Query\AbstractQuery;
+
+/**
+ * Optional backend feature: Get identifiers of records.
+ *
+ * @category VuFind
+ * @package  Search
+ * @author   Ere Maijala <ere.maijala@helsinki.fi>
+ * @license  http://opensource.org/licenses/gpl-2.0.php GNU General Public License
+ * @link     https://vufind.org
+ */
+interface GetIdsInterface
+{
+    /**
+     * Perform a search and return record collection of only record identifiers.
+     *
+     * @param AbstractQuery $query  Search query
+     * @param int           $offset Search offset
+     * @param int           $limit  Search limit
+     * @param ParamBag      $params Search backend parameters
+     *
+     * @return \VuFindSearch\Response\RecordCollectionInterface
+     */
+    public function getIds(AbstractQuery $query, $offset, $limit,
+        ParamBag $params = null
+    );
+}
diff --git a/module/VuFindSearch/src/VuFindSearch/Service.php b/module/VuFindSearch/src/VuFindSearch/Service.php
index 3e8a79269b6..075fd8d688d 100644
--- a/module/VuFindSearch/src/VuFindSearch/Service.php
+++ b/module/VuFindSearch/src/VuFindSearch/Service.php
@@ -30,6 +30,7 @@ namespace VuFindSearch;
 
 use VuFindSearch\Backend\BackendInterface;
 use VuFindSearch\Backend\Exception\BackendException;
+use VuFindSearch\Feature\GetIdsInterface;
 use VuFindSearch\Feature\RandomInterface;
 use VuFindSearch\Feature\RetrieveBatchInterface;
 use VuFindSearch\Response\RecordCollectionInterface;
@@ -101,8 +102,8 @@ class Service
     public function search($backend, Query\AbstractQuery $query, $offset = 0,
         $limit = 20, ParamBag $params = null
     ) {
-        $params  = $params ?: new ParamBag();
         $context = __FUNCTION__;
+        $params  = $params ?: new ParamBag();
         $args = compact('backend', 'query', 'offset', 'limit', 'params', 'context');
         $backend  = $this->resolve($backend, $args);
         $args['backend_instance'] = $backend;
@@ -118,6 +119,42 @@ class Service
         return $response;
     }
 
+    /**
+     * Perform a search that returns record IDs and return a wrapped response.
+     *
+     * @param string              $backend Search backend identifier
+     * @param Query\AbstractQuery $query   Search query
+     * @param int                 $offset  Search offset
+     * @param int                 $limit   Search limit
+     * @param ParamBag            $params  Search backend parameters
+     *
+     * @return RecordCollectionInterface
+     */
+    public function getIds($backend, Query\AbstractQuery $query, $offset = 0,
+        $limit = 20, ParamBag $params = null
+    ) {
+        $context = strtolower(__FUNCTION__);
+
+        $params  = $params ?: new ParamBag();
+        $args = compact('backend', 'query', 'offset', 'limit', 'params', 'context');
+        $backend  = $this->resolve($backend, $args);
+        $args['backend_instance'] = $backend;
+
+        $this->triggerPre($backend, $args);
+        try {
+            if ($backend instanceof GetIdsInterface) {
+                $response = $backend->getIds($query, $offset, $limit, $params);
+            } else {
+                $response = $backend->search($query, $offset, $limit, $params);
+            }
+        } catch (BackendException $e) {
+            $this->triggerError($e, $args);
+            throw $e;
+        }
+        $this->triggerPost($response, $args);
+        return $response;
+    }
+
     /**
      * Retrieve a single record.
      *
diff --git a/module/VuFindSearch/tests/unit-tests/src/VuFindTest/Backend/Solr/BackendTest.php b/module/VuFindSearch/tests/unit-tests/src/VuFindTest/Backend/Solr/BackendTest.php
index e3b3ffd8c48..6518657b158 100644
--- a/module/VuFindSearch/tests/unit-tests/src/VuFindTest/Backend/Solr/BackendTest.php
+++ b/module/VuFindSearch/tests/unit-tests/src/VuFindTest/Backend/Solr/BackendTest.php
@@ -33,6 +33,7 @@ use PHPUnit\Framework\TestCase;
 use VuFindSearch\Backend\Exception\RemoteErrorException;
 use VuFindSearch\Backend\Solr\Backend;
 use VuFindSearch\Backend\Solr\HandlerMap;
+use VuFindSearch\Backend\Solr\Response\Json\RecordCollection;
 
 use VuFindSearch\ParamBag;
 use VuFindSearch\Query\Query;
@@ -239,6 +240,44 @@ class BackendTest extends TestCase
         $this->assertEquals('foo', $back->getIdentifier());
     }
 
+    /**
+     * Test getting multiple IDs.
+     *
+     * @return void
+     */
+    public function testGetIds()
+    {
+        $paramBagChecker = function (ParamBag $params) {
+            $expected = [
+                'wt' => ['json'],
+                'json.nl' => ['arrarr'],
+                'fl' => ['id'],
+                'rows' => [10],
+                'start' => [0],
+                'q' => ['foo'],
+            ];
+            $paramsArr = $params->getArrayCopy();
+            foreach ($expected as $key => $vals) {
+                if (count(array_diff($vals, $paramsArr[$key] ?? [])) !== 0) {
+                    return false;
+                }
+            }
+            return true;
+        };
+        // TODO: currently this test is concerned with ensuring that the right
+        // parameters are sent to Solr; it may be worth adding a more realistic
+        // return value to better test processing of retrieved records.
+        $conn = $this->getConnectorMock(['search']);
+        $conn->expects($this->once())->method('search')
+            ->with($this->callback($paramBagChecker))
+            ->will($this->returnValue(json_encode([])));
+        $back = new Backend($conn);
+        $query = new Query('foo');
+        $result = $back->getIds($query, 0, 10);
+        $this->assertTrue($result instanceof RecordCollection);
+        $this->assertEquals(0, count($result));
+    }
+
     /**
      * Test refining an alphabrowse exception (string 1).
      *
diff --git a/module/VuFindSearch/tests/unit-tests/src/VuFindTest/SearchServiceTest.php b/module/VuFindSearch/tests/unit-tests/src/VuFindTest/SearchServiceTest.php
index 956bd0377b2..c3db870f687 100644
--- a/module/VuFindSearch/tests/unit-tests/src/VuFindTest/SearchServiceTest.php
+++ b/module/VuFindSearch/tests/unit-tests/src/VuFindTest/SearchServiceTest.php
@@ -31,6 +31,7 @@ namespace VuFindTest;
 use PHPUnit\Framework\TestCase;
 use VuFindSearch\Backend\BackendInterface;
 use VuFindSearch\Backend\Exception\BackendException;
+use VuFindSearch\Feature\GetIdsInterface;
 use VuFindSearch\Feature\RandomInterface;
 use VuFindSearch\Feature\RetrieveBatchInterface;
 use VuFindSearch\Feature\SimilarInterface;
@@ -128,6 +129,59 @@ class SearchServiceTest extends TestCase
         $service->search('foo', new Query('test'));
     }
 
+    /**
+     * Test that when a backend doesn't implement the "get IDs" feature
+     * interface, the getIds method of the search service simply proxies search.
+     * We'll test this by mimicing the testSearchException test above.
+     *
+     * @return void
+     *
+     * @expectedException        VuFindSearch\Backend\Exception\BackendException
+     * @expectedExceptionMessage test
+     */
+    public function testGetIdsProxyingSearchException()
+    {
+        $service = $this->getService();
+        $backend = $this->getBackend();
+        $exception = new BackendException('test');
+        $backend->expects($this->once())->method('search')
+            ->will($this->throwException($exception));
+        $em = $service->getEventManager();
+        $em->expects($this->at(0))->method('trigger')
+            ->with($this->equalTo('pre'), $this->equalTo($backend));
+        $em->expects($this->at(1))->method('trigger')
+            ->with($this->equalTo('error'), $this->equalTo($exception));
+        $service->getIds('foo', new Query('test'));
+    }
+
+    /**
+     * Test that when a backend DOES implement the "get IDs" feature
+     * interface, the appropriate method gets called.
+     * We'll test this by mimicing the testSearchException test above.
+     *
+     * @return void
+     *
+     * @expectedException        VuFindSearch\Backend\Exception\BackendException
+     * @expectedExceptionMessage test
+     */
+    public function testGetIdsException()
+    {
+        // Use a special backend for this test...
+        $this->backend = $this->createMock(\VuFindTest\TestClassForGetIdsInterface::class);
+
+        $service = $this->getService();
+        $backend = $this->getBackend();
+        $exception = new BackendException('test');
+        $backend->expects($this->once())->method('getIds')
+            ->will($this->throwException($exception));
+        $em = $service->getEventManager();
+        $em->expects($this->at(0))->method('trigger')
+            ->with($this->equalTo('pre'), $this->equalTo($backend));
+        $em->expects($this->at(1))->method('trigger')
+            ->with($this->equalTo('error'), $this->equalTo($exception));
+        $service->getIds('foo', new Query('test'));
+    }
+
     /**
      * Test batch retrieve (with RetrieveBatchInterface).
      *
@@ -705,6 +759,14 @@ class SearchServiceTest extends TestCase
     }
 }
 
+/**
+ * Stub class to test multiple interfaces.
+ */
+abstract class TestClassForGetIdsInterface
+    implements BackendInterface, GetIdsInterface
+{
+}
+
 /**
  * Stub class to test multiple interfaces.
  */
-- 
GitLab