From 877dba316df9cbb1e4114093c7ff0a69b1dfefd8 Mon Sep 17 00:00:00 2001
From: Demian Katz <demian.katz@villanova.edu>
Date: Mon, 27 Jul 2015 14:51:13 -0400
Subject: [PATCH] Fixed/added test for another edge case.

---
 .../src/VuFind/View/Helper/Root/OpenUrl.php   | 24 +++++++++++++++++--
 .../View/Helper/Root/OpenUrlTest.php          | 24 +++++++++++++++++--
 2 files changed, 44 insertions(+), 4 deletions(-)

diff --git a/module/VuFind/src/VuFind/View/Helper/Root/OpenUrl.php b/module/VuFind/src/VuFind/View/Helper/Root/OpenUrl.php
index 9013f4281f1..9031cf92448 100644
--- a/module/VuFind/src/VuFind/View/Helper/Root/OpenUrl.php
+++ b/module/VuFind/src/VuFind/View/Helper/Root/OpenUrl.php
@@ -267,6 +267,23 @@ class OpenUrl extends \Zend\View\Helper\AbstractHelper
         return false;
     }
 
+    /**
+     * Check if an array contains a non-empty value.
+     *
+     * @param array $in Array to check
+     *
+     * @return bool
+     */
+    protected function hasNonEmptyValue($in)
+    {
+        foreach ($in as $current) {
+            if (!empty($current)) {
+                return true;
+            }
+        }
+        return false;
+    }
+
     /**
      * Check if method rules match.
      *
@@ -287,9 +304,12 @@ class OpenUrl extends \Zend\View\Helper\AbstractHelper
                     // Strip the wildcard out of the value list; what is left
                     // is the set of values that MUST be found in the record.
                     // If we subtract the record values from the required values
-                    // and still have something left behind, then the match fails.
+                    // and still have something left behind, then the match fails
+                    // as long as SOME non-empty value was provided.
                     $requiredValues = array_diff($value, ['*']);
-                    if (!count(array_diff($requiredValues, $recordValue))) {
+                    if (!count(array_diff($requiredValues, $recordValue))
+                        && $this->hasNonEmptyValue($recordValue)
+                    ) {
                         $ruleMatchCounter++;
                     }
                 } else {
diff --git a/module/VuFind/tests/unit-tests/src/VuFindTest/View/Helper/Root/OpenUrlTest.php b/module/VuFind/tests/unit-tests/src/VuFindTest/View/Helper/Root/OpenUrlTest.php
index d7c5681ddc3..54a240e37b7 100644
--- a/module/VuFind/tests/unit-tests/src/VuFindTest/View/Helper/Root/OpenUrlTest.php
+++ b/module/VuFind/tests/unit-tests/src/VuFindTest/View/Helper/Root/OpenUrlTest.php
@@ -129,6 +129,24 @@ class OpenUrlTest extends \VuFindTest\Unit\ViewHelperTestCase
         $this->assertFalse($openUrl->isActive());
     }
 
+    /**
+     * Test checkExcludedRecordRules() with no matching rule (isActive() will return
+     * FALSE!!). Specifically we're testing the case where a method has a generic
+     * wildcard match in the rules but returns an empty value.
+     *
+     * @return void
+     */
+    public function testCheckExcludedRecordsRulesFalseDueToWildcardFailure()
+    {
+        $driver = $this->getMockDriver(
+            'fake-data', 'VuFind\RecordDriver\SolrMarc', ['Article'], false
+        );
+        $openUrl = $this
+            ->getOpenUrl($this->getFixture("rule5.json"), $this->rulesConfig)
+            ->__invoke($driver, 'results');
+        $this->assertFalse($openUrl->isActive());
+    }
+
     /**
      * Test checkSupportedRecordRules() with no matching rule (isActive() will return
      * FALSE!!)
@@ -213,19 +231,21 @@ class OpenUrlTest extends \VuFindTest\Unit\ViewHelperTestCase
      * @param string $openUrl OpenURL to return
      * @param string $class   Class to mock
      * @param array  $formats Formats to return from getFormats
+     * @param string $issn    ISSN to return from getCleanISSN
      *
      * @return \VuFind\RecordDriver\SolrDefault
      */
     protected function getMockDriver($openUrl = 'fake-data',
         $class = 'VuFind\RecordDriver\SolrDefault',
-        $formats = ['ElectronicArticle', 'Article']
+        $formats = ['ElectronicArticle', 'Article'],
+        $issn = '1234-5678'
     ) {
         $driver = $this->getMockBuilder($class)
             ->disableOriginalConstructor()->getMock();
         $driver->expects($this->any())->method('getOpenUrl')
             ->will($this->returnValue($openUrl));
         $driver->expects($this->any())->method('getCleanISSN')
-            ->will($this->returnValue('1234-5678'));
+            ->will($this->returnValue($issn));
         $driver->expects($this->any())->method('getFormats')
             ->will($this->returnValue($formats));
         return $driver;
-- 
GitLab