From ca966ebc7742da7ad710f385a6708d8921bb97b0 Mon Sep 17 00:00:00 2001
From: Demian Katz <demian.katz@villanova.edu>
Date: Thu, 19 Jul 2018 09:40:46 -0400
Subject: [PATCH] Refactor record loader to use helper classes/methods. -
Improves code clarity.
---
module/VuFind/src/VuFind/Record/Loader.php | 84 ++++++------
.../src/VuFind/Record/SourceAndIdList.php | 129 ++++++++++++++++++
2 files changed, 168 insertions(+), 45 deletions(-)
create mode 100644 module/VuFind/src/VuFind/Record/SourceAndIdList.php
diff --git a/module/VuFind/src/VuFind/Record/Loader.php b/module/VuFind/src/VuFind/Record/Loader.php
index 40ac9e504ab..557e1fd7068 100644
--- a/module/VuFind/src/VuFind/Record/Loader.php
+++ b/module/VuFind/src/VuFind/Record/Loader.php
@@ -71,9 +71,9 @@ class Loader implements \Zend\Log\LoggerAwareInterface
/**
* Constructor
*
- * @param SearchService $searchService Search service
- * @param RecordFactory $recordFactory Record loader
- * @param Cache $recordCache Record Cache
+ * @param SearchService $searchService Search service
+ * @param RecordFactory $recordFactory Record loader
+ * @param Cache $recordCache Record Cache
*/
public function __construct(SearchService $searchService,
RecordFactory $recordFactory, Cache $recordCache = null
@@ -145,25 +145,23 @@ class Loader implements \Zend\Log\LoggerAwareInterface
public function loadBatchForSource($ids, $source = DEFAULT_SEARCH_BACKEND,
$tolerateBackendExceptions = false
) {
+ $list = new Checklist($ids);
$cachedRecords = [];
if (null !== $this->recordCache && $this->recordCache->isPrimary($source)) {
// Try to load records from cache if source is cachable
$cachedRecords = $this->recordCache->lookupBatch($ids, $source);
// Check which records could not be loaded from the record cache
foreach ($cachedRecords as $cachedRecord) {
- $key = array_search($cachedRecord->getUniqueId(), $ids);
- if ($key !== false) {
- unset($ids[$key]);
- }
+ $list->check($cachedRecord->getUniqueId());
}
}
// Try to load the uncached records from the original $source
$genuineRecords = [];
- if (!empty($ids)) {
+ if ($list->hasUnchecked()) {
try {
- $genuineRecords = $this->searchService->retrieveBatch($source, $ids)
- ->getRecords();
+ $genuineRecords = $this->searchService
+ ->retrieveBatch($source, $list->getUnchecked())->getRecords();
} catch (\VuFindSearch\Backend\Exception\BackendException $e) {
if (!$tolerateBackendExceptions) {
throw $e;
@@ -175,18 +173,16 @@ class Loader implements \Zend\Log\LoggerAwareInterface
}
foreach ($genuineRecords as $genuineRecord) {
- $key = array_search($genuineRecord->getUniqueId(), $ids);
- if ($key !== false) {
- unset($ids[$key]);
- }
+ $list->check($genuineRecord->getUniqueId());
}
}
- if (!empty($ids) && null !== $this->recordCache
+ if ($list->hasUnchecked() && null !== $this->recordCache
&& $this->recordCache->isFallback($source)
) {
// Try to load missing records from cache if source is cachable
- $cachedRecords = $this->recordCache->lookupBatch($ids, $source);
+ $cachedRecords = $this->recordCache
+ ->lookupBatch($list->getUnchecked(), $source);
}
// Merge records found in cache and records loaded from original $source
@@ -198,6 +194,24 @@ class Loader implements \Zend\Log\LoggerAwareInterface
return $retVal;
}
+ /**
+ * Build a "missing record" driver.
+ *
+ * @param array $details Associative array of record details (from a
+ * SourceAndIdList)
+ *
+ * @return \VuFind\RecordDriver\Missing
+ */
+ protected function buildMissingRecord($details)
+ {
+ $fields = $details['extra_fields'] ?? [];
+ $fields['id'] = $details['id'];
+ $record = $this->recordFactory->get('Missing');
+ $record->setRawData($fields);
+ $record->setSourceIdentifier($details['source']);
+ return $record;
+ }
+
/**
* Given an array of associative arrays with id and source keys (or pipe-
* separated source|id strings), load all of the requested records in the
@@ -217,48 +231,28 @@ class Loader implements \Zend\Log\LoggerAwareInterface
*/
public function loadBatch($ids, $tolerateBackendExceptions = false)
{
- // Sort the IDs by source -- we'll create an associative array indexed by
- // source and record ID which points to the desired position of the indexed
- // record in the final return array:
- $idBySource = [];
- foreach ($ids as $i => $details) {
- // Convert source|id string to array if necessary:
- if (!is_array($details)) {
- $parts = explode('|', $details, 2);
- $ids[$i] = $details = [
- 'source' => $parts[0], 'id' => $parts[1]
- ];
- }
- $idBySource[$details['source']][$details['id']] = $i;
- }
+ // Create a SourceAndIdList object to help sort the IDs by source:
+ $list = new SourceAndIdList($ids);
// Retrieve the records and put them back in order:
$retVal = [];
- foreach ($idBySource as $source => $details) {
+ foreach ($list->getIdsBySource() as $source => $currentIds) {
$records = $this->loadBatchForSource(
- array_keys($details), $source, $tolerateBackendExceptions
+ $currentIds, $source, $tolerateBackendExceptions
);
foreach ($records as $current) {
- $id = $current->getUniqueId();
- // In theory, we should be able to assume that $details[$id] is
- // set... but in practice, we can't make that assumption. In some
- // cases, Summon IDs will change, and requests for an old ID value
- // will return a record with a different ID.
- if (isset($details[$id])) {
- $retVal[$details[$id]] = $current;
+ $position = $list->getRecordPosition($current);
+ if ($position !== false) {
+ $retVal[$position] = $current;
}
}
}
// Check for missing records and fill gaps with \VuFind\RecordDriver\Missing
// objects:
- foreach ($ids as $i => $details) {
+ foreach ($list->getAll() as $i => $details) {
if (!isset($retVal[$i]) || !is_object($retVal[$i])) {
- $fields = $details['extra_fields'] ?? [];
- $fields['id'] = $details['id'];
- $retVal[$i] = $this->recordFactory->get('Missing');
- $retVal[$i]->setRawData($fields);
- $retVal[$i]->setSourceIdentifier($details['source']);
+ $retVal[$i] = $this->buildMissingRecord($details);
}
}
diff --git a/module/VuFind/src/VuFind/Record/SourceAndIdList.php b/module/VuFind/src/VuFind/Record/SourceAndIdList.php
new file mode 100644
index 00000000000..893254c4549
--- /dev/null
+++ b/module/VuFind/src/VuFind/Record/SourceAndIdList.php
@@ -0,0 +1,129 @@
+<?php
+/**
+ * Record ID list (support class for Loader)
+ *
+ * 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 Record
+ * @author Demian Katz <demian.katz@villanova.edu>
+ * @license http://opensource.org/licenses/gpl-2.0.php GNU General Public License
+ * @link https://vufind.org Main Site
+ */
+namespace VuFind\Record;
+
+use VuFind\RecordDriver\AbstractBase as Record;
+
+/**
+ * Record ID list (support class for Loader)
+ *
+ * @category VuFind
+ * @package Record
+ * @author Demian Katz <demian.katz@villanova.edu>
+ * @license http://opensource.org/licenses/gpl-2.0.php GNU General Public License
+ * @link https://vufind.org Main Site
+ */
+class SourceAndIdList
+{
+ /**
+ * Processed ID data.
+ *
+ * @var array
+ */
+ protected $ids = [];
+
+ /**
+ * Record positions in the original list, indexed by source and ID.
+ *
+ * @var array
+ */
+ protected $bySource = [];
+
+ /**
+ * Constructor
+ *
+ * @param array $ids Array of associative arrays with id/source keys or strings
+ * in source|id format. In associative array formats, there is also an optional
+ * "extra_fields" key which can be used to pass in data formatted as if it
+ * belongs to the Solr schema; this is used to create a mock driver object if
+ * the real data source is unavailable.
+ */
+ public function __construct($ids)
+ {
+ // Sort the IDs by source -- we'll create an associative array indexed by
+ // source and record ID which points to the desired position of the indexed
+ // record in the final return array:
+ foreach ($ids as $i => $details) {
+ // Convert source|id string to array if necessary:
+ if (!is_array($details)) {
+ $parts = explode('|', $details, 2);
+ $ids[$i] = $details = [
+ 'source' => $parts[0], 'id' => $parts[1]
+ ];
+ }
+ $this->bySource[$details['source']][$details['id']] = $i;
+ }
+ $this->ids = $ids;
+ }
+
+ /**
+ * Get the full list of IDs sent to the constructor, normalized to array
+ * format.
+ *
+ * @return array
+ */
+ public function getAll()
+ {
+ return $this->ids;
+ }
+
+ /**
+ * Get an associative source => id list array.
+ *
+ * @return array
+ */
+ public function getIdsBySource()
+ {
+ return array_map('array_keys', $this->bySource);
+ }
+
+ /**
+ * If the provided record driver corresponds with an ID in the list, return
+ * the associated position in the list. Otherwise, return false.
+ *
+ * @param Record $record Record
+ *
+ * @return int|bool
+ */
+ public function getRecordPosition(Record $record)
+ {
+ $id = $record->getUniqueId();
+ $source = $record->getSourceIdentifier();
+
+ // First check if the primary ID is set; in some cases (e.g. Summon),
+ // the ID may have changed, so also check the prior ID if available.
+ if (isset($this->bySource[$source][$id])) {
+ return $this->bySource[$source][$id];
+ }
+ $oldId = $record->tryMethod('getPreviousUniqueId');
+ if ($oldId !== null && isset($this->bySource[$source][$oldId])) {
+ return $this->bySource[$source][$oldId];
+ }
+ return false;
+ }
+}
--
GitLab