From 4c9b6b674680e55f73b5f00e43d459909fb6f1cd Mon Sep 17 00:00:00 2001
From: Demian Katz <demian.katz@villanova.edu>
Date: Thu, 30 Apr 2020 09:23:27 -0400
Subject: [PATCH] Support use of multiple DOI handlers. (#1560)

---
 config/vufind/config.ini                      |  11 +-
 .../src/VuFind/AjaxHandler/DoiLookup.php      |  51 ++++-
 .../VuFind/AjaxHandler/DoiLookupFactory.php   |   6 +-
 .../VuFindTest/AjaxHandler/DoiLookupTest.php  | 214 ++++++++++++++++--
 4 files changed, 257 insertions(+), 25 deletions(-)

diff --git a/config/vufind/config.ini b/config/vufind/config.ini
index f33d6b17fe7..a330d438ed8 100644
--- a/config/vufind/config.ini
+++ b/config/vufind/config.ini
@@ -1121,10 +1121,17 @@ url = "https://api.booksite.com"
 [DOI]
 ; This setting controls whether or not DOI-based links are enabled, and which
 ; API is used to fetch the data. Currently supported options: BrowZine (requires
-; credentials to be configured in BrowZine.ini), Unpaywall or false (to disable). Disabled
-; by default.
+; credentials to be configured in BrowZine.ini), Unpaywall or false (to disable).
+; Disabled by default. You may also use a comma-separated list of resolvers if you
+; want to try multiple sources.
 ;resolver = BrowZine
 
+; If you use multiple values in the resolver setting above, you can determine how the
+; software should behave when multiple resolvers return results for the same DOI.
+; You can choose "first" (only return results from the first matching resolver --
+; the default behavior) or "merge" (merge together all results and show them all).
+;multi_resolver_mode = first
+
 ;unpaywall_api_url = "https://api.unpaywall.org/v2"
 ; Unpaywall needs an email adress, see https://unpaywall.org/products/api
 ;unpaywall_email = "your@email.org"
diff --git a/module/VuFind/src/VuFind/AjaxHandler/DoiLookup.php b/module/VuFind/src/VuFind/AjaxHandler/DoiLookup.php
index c908be0f664..cd98789b8a8 100644
--- a/module/VuFind/src/VuFind/AjaxHandler/DoiLookup.php
+++ b/module/VuFind/src/VuFind/AjaxHandler/DoiLookup.php
@@ -49,22 +49,35 @@ class DoiLookup extends AbstractBase
     protected $pluginManager;
 
     /**
-     * DOI resolver configuration value
+     * DOI resolver configuration value, exploded into an array of options
+     *
+     * @var string[]
+     */
+    protected $resolvers;
+
+    /**
+     * Behavior to use when multiple resolvers find results for the same DOI (may
+     * be 'first' -- use first match, or 'merge' -- use all results)
      *
      * @var string
      */
-    protected $resolver;
+    protected $multiMode;
 
     /**
      * Constructor
      *
      * @param PluginManager $pluginManager DOI Linker Plugin Manager
-     * @param string        $resolver      DOI resolver configuration value
+     * @param string        $resolvers     DOI resolver configuration value
+     * @param string        $multiMode     Behavior to use when multiple resolvers
+     * find results for the same DOI (may be 'first' -- use first match, or 'merge'
+     * -- use all results)
      */
-    public function __construct(PluginManager $pluginManager, $resolver)
-    {
+    public function __construct(PluginManager $pluginManager, $resolvers,
+        $multiMode = 'first'
+    ) {
         $this->pluginManager = $pluginManager;
-        $this->resolver = $resolver;
+        $this->resolvers = array_map('trim', explode(',', $resolvers));
+        $this->multiMode = trim(strtolower($multiMode));
     }
 
     /**
@@ -77,9 +90,29 @@ class DoiLookup extends AbstractBase
     public function handleRequest(Params $params)
     {
         $response = [];
-        if ($this->pluginManager->has($this->resolver)) {
-            $dois = (array)$params->fromQuery('doi', []);
-            $response = $this->pluginManager->get($this->resolver)->getLinks($dois);
+        $dois = (array)$params->fromQuery('doi', []);
+        foreach ($this->resolvers as $resolver) {
+            if ($this->pluginManager->has($resolver)) {
+                $next = $this->pluginManager->get($resolver)->getLinks($dois);
+                if (empty($response)) {
+                    $response = $next;
+                } else {
+                    foreach ($next as $doi => $data) {
+                        if (!isset($response[$doi])) {
+                            $response[$doi] = $data;
+                        } elseif ($this->multiMode == 'merge') {
+                            $response[$doi] = array_merge($response[$doi], $data);
+                        }
+                    }
+                }
+                // If all DOIs have been found and we're not in merge mode, we
+                // can short circuit out of here.
+                if ($this->multiMode !== 'merge'
+                    && count(array_diff($dois, array_keys($response))) == 0
+                ) {
+                    break;
+                }
+            }
         }
         return $this->formatResponse($response);
     }
diff --git a/module/VuFind/src/VuFind/AjaxHandler/DoiLookupFactory.php b/module/VuFind/src/VuFind/AjaxHandler/DoiLookupFactory.php
index f9261996c95..aecf4ea2090 100644
--- a/module/VuFind/src/VuFind/AjaxHandler/DoiLookupFactory.php
+++ b/module/VuFind/src/VuFind/AjaxHandler/DoiLookupFactory.php
@@ -65,6 +65,10 @@ class DoiLookupFactory implements \Laminas\ServiceManager\Factory\FactoryInterfa
         $config = $container->get(\VuFind\Config\PluginManager::class)
             ->get('config');
         $pluginManager = $container->get(\VuFind\DoiLinker\PluginManager::class);
-        return new $requestedName($pluginManager, $config->DOI->resolver ?? null);
+        return new $requestedName(
+            $pluginManager,
+            $config->DOI->resolver ?? null,
+            $config->DOI->multi_resolver_mode ?? 'first'
+        );
     }
 }
diff --git a/module/VuFind/tests/unit-tests/src/VuFindTest/AjaxHandler/DoiLookupTest.php b/module/VuFind/tests/unit-tests/src/VuFindTest/AjaxHandler/DoiLookupTest.php
index a535ca58cce..25b4de3aaf9 100644
--- a/module/VuFind/tests/unit-tests/src/VuFindTest/AjaxHandler/DoiLookupTest.php
+++ b/module/VuFind/tests/unit-tests/src/VuFindTest/AjaxHandler/DoiLookupTest.php
@@ -46,33 +46,221 @@ use VuFind\DoiLinker\PluginManager;
 class DoiLookupTest extends \VuFindTest\Unit\AjaxHandlerTest
 {
     /**
-     * Test a DOI lookup.
+     * Set up configuration for a test.
+     *
+     * @param array $config Configuration to set.
      *
      * @return void
      */
-    public function testLookup()
+    protected function setupConfig($config)
     {
-        // Set up config manager:
-        $config = new Config(['DOI' => ['resolver' => 'foo']]);
+        $config = new Config($config);
         $cm = $this->container->createMock(ConfigManager::class, ['get']);
         $cm->expects($this->once())->method('get')->with($this->equalTo('config'))
             ->will($this->returnValue($config));
         $this->container->set(ConfigManager::class, $cm);
+    }
 
-        // Set up plugin manager:
-        $pm = new PluginManager($this->container);
+    /**
+     * Create a mock plugin.
+     *
+     * @param mixed  $value    Value to return in response to DOI request.
+     * @param string $times    How many times do we expect this method to be called?
+     * @param string $doi      What DOI does this handler return data for?
+     * @param array  $expected What is the expected DOI request?
+     *
+     * @return DoiLinkerInterface
+     */
+    protected function getMockPlugin($value, $times = 'once', $doi = 'bar',
+        $expected = ['bar']
+    ) {
         $mockPlugin = $this->container
             ->createMock(DoiLinkerInterface::class, ['getLinks']);
-        $mockPlugin->expects($this->once())->method('getLinks')
-            ->with($this->equalTo(['bar']))
-            ->will($this->returnValue(['bar' => 'baz']));
-        $pm->setService('foo', $mockPlugin);
+        $mockPlugin->expects($this->$times())->method('getLinks')
+            ->with($this->equalTo($expected))
+            ->will(
+                $this->returnValue(
+                    [
+                        $doi => [['link' => 'http://' . $value, 'label' => $value]]
+                    ]
+                )
+            );
+        return $mockPlugin;
+    }
+
+    /**
+     * Set up a plugin manager for a test.
+     *
+     * @param array $plugins Plugins to insert into container.
+     *
+     * @return void
+     */
+    protected function setupPluginManager($plugins)
+    {
+        $pm = new PluginManager($this->container);
+        foreach ($plugins as $name => $plugin) {
+            $pm->setService($name, $plugin);
+        }
         $this->container->set(PluginManager::class, $pm);
+    }
 
-        // Test the handler:
+    /**
+     * After setupConfig() and setupPluginManager() have been called, run the
+     * standard default test.
+     *
+     * @param array $requested DOI(s) to test request with
+     *
+     * @return array
+     */
+    protected function getHandlerResults($requested = ['bar'])
+    {
         $factory = new DoiLookupFactory();
         $handler = $factory($this->container, DoiLookup::class);
-        $params = $this->getParamsHelper(['doi' => ['bar']]);
-        $this->assertEquals([['bar' => 'baz']], $handler->handleRequest($params));
+        $params = $this->getParamsHelper(['doi' => $requested]);
+        return $handler->handleRequest($params);
+    }
+
+    /**
+     * Test a single DOI lookup.
+     *
+     * @return void
+     */
+    public function testSingleLookup()
+    {
+        // Set up config manager:
+        $this->setupConfig(['DOI' => ['resolver' => 'foo']]);
+
+        // Set up plugin manager:
+        $this->setupPluginManager(
+            ['foo' => $this->getMockPlugin('baz')]
+        );
+
+        // Test the handler:
+        $this->assertEquals(
+            [['bar' => [['link' => 'http://baz', 'label' => 'baz']]]],
+            $this->getHandlerResults()
+        );
+    }
+
+    /**
+     * Test a DOI lookup in two handlers, with "first" mode turned on by default.
+     *
+     * @return void
+     */
+    public function testFirstDefaultLookup()
+    {
+        // Set up config manager:
+        $this->setupConfig(['DOI' => ['resolver' => 'foo,foo2']]);
+
+        // Set up plugin manager:
+        $this->setupPluginManager(
+            [
+                'foo' => $this->getMockPlugin('baz'),
+                'foo2' => $this->getMockPlugin('baz2', 'never')
+            ]
+        );
+
+        // Test the handler:
+        $this->assertEquals(
+            [['bar' => [['link' => 'http://baz', 'label' => 'baz']]]],
+            $this->getHandlerResults()
+        );
+    }
+
+    /**
+     * Test a DOI lookup in two handlers, with "first" mode turned on explicitly.
+     *
+     * @return void
+     */
+    public function testFirstExplicitLookup()
+    {
+        // Set up config manager:
+        $this->setupConfig(
+            ['DOI' => ['resolver' => 'foo,foo2', 'multi_resolver_mode' => 'first']]
+        );
+
+        // Set up plugin manager:
+        $this->setupPluginManager(
+            [
+                'foo' => $this->getMockPlugin('baz'),
+                'foo2' => $this->getMockPlugin('baz2', 'never')
+            ]
+        );
+
+        // Test the handler:
+        $this->assertEquals(
+            [['bar' => [['link' => 'http://baz', 'label' => 'baz']]]],
+            $this->getHandlerResults()
+        );
+    }
+
+    /**
+     * Test a DOI lookup in two handlers, with "first" mode turned on explicitly,
+     * where each handler returns results for a different DOI.
+     *
+     * @return void
+     */
+    public function testFirstExplicitLookupMultipleDOIs()
+    {
+        // Set up config manager:
+        $this->setupConfig(
+            ['DOI' => ['resolver' => 'foo,foo2,foo3', 'multi_resolver_mode' => 'first']]
+        );
+
+        // Set up plugin manager:
+        $request = ['bar', 'bar2'];
+        $this->setupPluginManager(
+            [
+                'foo' => $this->getMockPlugin('baz', 'once', 'bar', $request),
+                'foo2' => $this->getMockPlugin('baz2', 'once', 'bar2', $request),
+                // The previous handlers will satisfy the request, so this one will
+                // never be called; included to verify short-circuit behavior:
+                'foo3' => $this->getMockPlugin('baz', 'never', 'bar', $request),
+            ]
+        );
+
+        // Test the handler:
+        $this->assertEquals(
+            [
+                [
+                    'bar' => [['link' => 'http://baz', 'label' => 'baz']],
+                    'bar2' => [['link' => 'http://baz2', 'label' => 'baz2']],
+                ]
+            ],
+            $this->getHandlerResults($request)
+        );
+    }
+
+    /**
+     * Test a DOI lookup in two handlers, with "merge" mode turned on.
+     *
+     * @return void
+     */
+    public function testMergeLookup()
+    {
+        // Set up config manager:
+        $this->setupConfig(
+            ['DOI' => ['resolver' => 'foo,foo2', 'multi_resolver_mode' => 'merge']]
+        );
+
+        // Set up plugin manager:
+        $this->setupPluginManager(
+            [
+                'foo' => $this->getMockPlugin('baz'),
+                'foo2' => $this->getMockPlugin('baz2')
+            ]
+        );
+        // Test the handler:
+        $this->assertEquals(
+            [
+                [
+                    'bar' => [
+                        ['link' => 'http://baz', 'label' => 'baz'],
+                        ['link' => 'http://baz2', 'label' => 'baz2'],
+                    ]
+                ]
+            ],
+            $this->getHandlerResults()
+        );
     }
 }
-- 
GitLab