From c5aa5f292a8cb8ae7c801be108b71e61262c50a8 Mon Sep 17 00:00:00 2001
From: Demian Katz <demian.katz@villanova.edu>
Date: Tue, 24 Apr 2018 13:02:05 -0400
Subject: [PATCH] Standardize code for dealing with facet limits. (#1167)

---
 .../VuFind/Search/Params/FacetLimitTrait.php  | 109 ++++++++++++++++++
 .../VuFind/src/VuFind/Search/Solr/Params.php  |  63 ++--------
 .../src/VuFind/Search/Summon/Params.php       |  76 ++++++++++--
 3 files changed, 181 insertions(+), 67 deletions(-)
 create mode 100644 module/VuFind/src/VuFind/Search/Params/FacetLimitTrait.php

diff --git a/module/VuFind/src/VuFind/Search/Params/FacetLimitTrait.php b/module/VuFind/src/VuFind/Search/Params/FacetLimitTrait.php
new file mode 100644
index 00000000000..e432d14469d
--- /dev/null
+++ b/module/VuFind/src/VuFind/Search/Params/FacetLimitTrait.php
@@ -0,0 +1,109 @@
+<?php
+/**
+ * Trait to add facet limiting settings to a Params object.
+ *
+ * PHP version 7
+ *
+ * Copyright (C) Villanova University 2018.
+ *
+ * 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   Demian Katz <demian.katz@villanova.edu>
+ * @license  http://opensource.org/licenses/gpl-2.0.php GNU General Public License
+ * @link     https://vufind.org/wiki/development:plugins:record_drivers Wiki
+ */
+namespace VuFind\Search\Params;
+
+use Zend\Config\Config;
+
+/**
+ * Trait to add facet limiting settings to a Params object.
+ *
+ * @category VuFind
+ * @package  Search
+ * @author   Demian Katz <demian.katz@villanova.edu>
+ * @license  http://opensource.org/licenses/gpl-2.0.php GNU General Public License
+ * @link     https://vufind.org/wiki/development:plugins:record_drivers Wiki
+ */
+trait FacetLimitTrait
+{
+    /**
+     * Default facet result limit
+     *
+     * @var int
+     */
+    protected $facetLimit = 30;
+
+    /**
+     * Per-field facet result limit
+     *
+     * @var array
+     */
+    protected $facetLimitByField = [];
+
+    /**
+     * Initialize facet limit from a Config object.
+     *
+     * @param Config $config Configuration
+     *
+     * @return void
+     */
+    protected function initFacetLimitsFromConfig(Config $config = null)
+    {
+        if (is_numeric($config->facet_limit ?? null)) {
+            $this->setFacetLimit($config->facet_limit);
+        }
+        foreach ($config->facet_limit_by_field ?? [] as $k => $v) {
+            $this->facetLimitByField[$k] = $v;
+        }
+    }
+
+    /**
+     * Set Facet Limit
+     *
+     * @param int $l the new limit value
+     *
+     * @return void
+     */
+    public function setFacetLimit($l)
+    {
+        $this->facetLimit = $l;
+    }
+
+    /**
+     * Set Facet Limit by Field
+     *
+     * @param array $new Associative array of $field name => $limit
+     *
+     * @return void
+     */
+    public function setFacetLimitByField(array $new)
+    {
+        $this->facetLimitByField = $new;
+    }
+
+    /**
+     * Get the facet limit for the specified field.
+     *
+     * @param string $field Field to look up
+     *
+     * @return int
+     */
+    protected function getFacetLimitForField($field)
+    {
+        return $this->facetLimitByField[$field] ?? $this->facetLimit;
+    }
+}
diff --git a/module/VuFind/src/VuFind/Search/Solr/Params.php b/module/VuFind/src/VuFind/Search/Solr/Params.php
index 87efc2b6cef..5bd76b4487e 100644
--- a/module/VuFind/src/VuFind/Search/Solr/Params.php
+++ b/module/VuFind/src/VuFind/Search/Solr/Params.php
@@ -40,19 +40,7 @@ use VuFindSearch\ParamBag;
  */
 class Params extends \VuFind\Search\Base\Params
 {
-    /**
-     * Default facet result limit
-     *
-     * @var int
-     */
-    protected $facetLimit = 30;
-
-    /**
-     * Per-field facet result limit
-     *
-     * @var array
-     */
-    protected $facetLimitByField = [];
+    use \VuFind\Search\Params\FacetLimitTrait;
 
     /**
      * Offset for facet results
@@ -111,22 +99,13 @@ class Params extends \VuFind\Search\Base\Params
 
         // Use basic facet limit by default, if set:
         $config = $configLoader->get($options->getFacetsIni());
-        if (isset($config->Results_Settings->facet_limit)
-            && is_numeric($config->Results_Settings->facet_limit)
-        ) {
-            $this->setFacetLimit($config->Results_Settings->facet_limit);
-        }
+        $this->initFacetLimitsFromConfig($config->Results_Settings ?? null);
         if (isset($config->LegacyFields)) {
             $this->facetAliases = $config->LegacyFields->toArray();
         }
         if (isset($config->ExtraFacetLabels)) {
             $this->extraFacetLabels = $config->ExtraFacetLabels->toArray();
         }
-        if (isset($config->Results_Settings->facet_limit_by_field)) {
-            foreach ($config->Results_Settings->facet_limit_by_field as $k => $v) {
-                $this->facetLimitByField[$k] = $v;
-            }
-        }
         if (isset($config->Results_Settings->sorted_by_index)
             && count($config->Results_Settings->sorted_by_index) > 0
         ) {
@@ -194,9 +173,9 @@ class Params extends \VuFind\Search\Base\Params
         if (!empty($this->facetConfig)) {
             $facetSet['limit'] = $this->facetLimit;
             foreach (array_keys($this->facetConfig) as $facetField) {
-                if (isset($this->facetLimitByField[$facetField])) {
-                    $facetSet["f.{$facetField}.facet.limit"]
-                        = $this->facetLimitByField[$facetField];
+                $fieldLimit = $this->getFacetLimitForField($facetField);
+                if ($fieldLimit != $this->facetLimit) {
+                    $facetSet["f.{$facetField}.facet.limit"] = $fieldLimit;
                 }
                 if ($this->getFacetOperator($facetField) == 'OR') {
                     $facetField = '{!ex=' . $facetField . '_filter}' . $facetField;
@@ -239,30 +218,6 @@ class Params extends \VuFind\Search\Base\Params
         }
     }
 
-    /**
-     * Set Facet Limit
-     *
-     * @param int $l the new limit value
-     *
-     * @return void
-     */
-    public function setFacetLimit($l)
-    {
-        $this->facetLimit = $l;
-    }
-
-    /**
-     * Set Facet Limit by Field
-     *
-     * @param array $new Associative array of $field name => $limit
-     *
-     * @return void
-     */
-    public function setFacetLimitByField(array $new)
-    {
-        $this->facetLimitByField = $new;
-    }
-
     /**
      * Set Facet Offset
      *
@@ -322,12 +277,8 @@ class Params extends \VuFind\Search\Base\Params
      */
     protected function initFacetList($facetList, $facetSettings, $cfgFile = 'facets')
     {
-        $config = $this->configLoader->get('facets');
-        if (isset($config->$facetSettings->facet_limit)
-            && is_numeric($config->$facetSettings->facet_limit)
-        ) {
-            $this->setFacetLimit($config->$facetSettings->facet_limit);
-        }
+        $config = $this->configLoader->get($cfgFile);
+        $this->initFacetLimitsFromConfig($config->$facetSettings ?? null);
         return parent::initFacetList($facetList, $facetSettings, $cfgFile);
     }
 
diff --git a/module/VuFind/src/VuFind/Search/Summon/Params.php b/module/VuFind/src/VuFind/Search/Summon/Params.php
index b90242bf8a5..69cdcf0e5b2 100644
--- a/module/VuFind/src/VuFind/Search/Summon/Params.php
+++ b/module/VuFind/src/VuFind/Search/Summon/Params.php
@@ -42,6 +42,8 @@ use VuFindSearch\ParamBag;
  */
 class Params extends \VuFind\Search\Base\Params
 {
+    use \VuFind\Search\Params\FacetLimitTrait;
+
     /**
      * Settings for all the facets
      *
@@ -56,6 +58,19 @@ class Params extends \VuFind\Search\Base\Params
      */
     protected $dateFacetSettings = [];
 
+    /**
+     * Constructor
+     *
+     * @param \VuFind\Search\Base\Options  $options      Options to use
+     * @param \VuFind\Config\PluginManager $configLoader Config loader
+     */
+    public function __construct($options, \VuFind\Config\PluginManager $configLoader)
+    {
+        parent::__construct($options, $configLoader);
+        $config = $configLoader->get($options->getFacetsIni());
+        $this->initFacetLimitsFromConfig($config->Facet_Settings ?? null);
+    }
+
     /**
      * Add a field to facet on.
      *
@@ -213,24 +228,16 @@ class Params extends \VuFind\Search\Base\Params
      */
     protected function getBackendFacetParameters()
     {
-        $config = $this->configLoader->get('Summon');
-        $defaultFacetLimit = isset($config->Facet_Settings->facet_limit)
-            ? $config->Facet_Settings->facet_limit : 30;
-        $fieldSpecificLimits = isset($config->Facet_Settings->facet_limit_by_field)
-            ? $config->Facet_Settings->facet_limit_by_field : null;
-
         $finalFacets = [];
         foreach ($this->getFullFacetSettings() as $facet) {
             // See if parameters are included as part of the facet name;
             // if not, override them with defaults.
             $parts = explode(',', $facet);
             $facetName = $parts[0];
-            $bestDefaultFacetLimit
-                = $fieldSpecificLimits->$facetName ?? $defaultFacetLimit;
             $defaultMode = ($this->getFacetOperator($facet) == 'OR') ? 'or' : 'and';
             $facetMode = $parts[1] ?? $defaultMode;
             $facetPage = $parts[2] ?? 1;
-            $facetLimit = $parts[3] ?? $bestDefaultFacetLimit;
+            $facetLimit = $parts[3] ?? $this->getFacetLimitForField($facetName);
             $facetParams = "{$facetMode},{$facetPage},{$facetLimit}";
             $finalFacets[] = "{$facetName},{$facetParams}";
         }
@@ -343,6 +350,46 @@ class Params extends \VuFind\Search\Base\Params
         return $filter;
     }
 
+    /**
+     * Initialize facet settings for the specified configuration sections.
+     *
+     * @param string $facetList     Config section containing fields to activate
+     * @param string $facetSettings Config section containing related settings
+     * @param string $cfgFile       Name of configuration to load
+     *
+     * @return bool                 True if facets set, false if no settings found
+     */
+    protected function initFacetList($facetList, $facetSettings, $cfgFile = 'facets')
+    {
+        $config = $this->configLoader->get($cfgFile);
+        // Special case -- when most settings are in Results_Settings, the limits
+        // can be found in Facet_Settings.
+        $limitSection = ($facetSettings === 'Results_Settings')
+            ? 'Facet_Settings' : $facetSettings;
+        $this->initFacetLimitsFromConfig($config->$limitSection ?? null);
+        return parent::initFacetList($facetList, $facetSettings, $cfgFile);
+    }
+
+    /**
+     * Initialize facet settings for the advanced search screen.
+     *
+     * @return void
+     */
+    public function initAdvancedFacets()
+    {
+        $this->initFacetList('Advanced_Facets', 'Advanced_Facet_Settings', 'Summon');
+    }
+
+    /**
+     * Initialize facet settings for the standard search screen.
+     *
+     * @return void
+     */
+    public function initBasicFacets()
+    {
+        $this->initFacetList('Facets', 'Results_Settings', 'Summon');
+    }
+
     /**
      * Load all available facet settings.  This is mainly useful for showing
      * appropriate labels when an existing search has multiple filters associated
@@ -356,8 +403,15 @@ class Params extends \VuFind\Search\Base\Params
      */
     public function activateAllFacets($preferredSection = false)
     {
-        $this->initFacetList('Facets', 'Results_Settings', 'Summon');
-        $this->initFacetList('Advanced_Facets', 'Advanced_Facet_Settings', 'Summon');
+        // Based on preference, change the order of initialization to make sure
+        // that preferred facet labels come in last.
+        if ($preferredSection == 'Advanced') {
+            $this->initBasicFacets();
+            $this->initAdvancedFacets();
+        } else {
+            $this->initAdvancedFacets();
+            $this->initBasicFacets();
+        }
         $this->initCheckboxFacets('CheckboxFacets', 'Summon');
     }
 }
-- 
GitLab