From 96616cac502b1b65e0558c2c56ce3226a2ce6dab Mon Sep 17 00:00:00 2001
From: Ere Maijala <ere.maijala@helsinki.fi>
Date: Wed, 6 Nov 2019 23:04:22 +0200
Subject: [PATCH] Optimize record collection creation. (#1488)
- Avoids checking for existing records when we know there shouldn't be any duplicates.
- Moves setting source identifier of records inside the collection class to make it faster.
---
.../VuFindSearch/Backend/AbstractBackend.php | 5 +---
.../Response/RecordCollectionFactory.php | 2 +-
.../EDS/Response/RecordCollectionFactory.php | 2 +-
.../Response/XML/RecordCollectionFactory.php | 2 +-
.../Response/RecordCollectionFactory.php | 2 +-
.../Response/RecordCollectionFactory.php | 2 +-
.../Response/RecordCollectionFactory.php | 2 +-
.../Response/Json/RecordCollectionFactory.php | 2 +-
.../Response/RecordCollectionFactory.php | 2 +-
.../Response/XML/RecordCollectionFactory.php | 2 +-
.../Response/AbstractRecordCollection.php | 23 +++++++++++++++---
.../Response/Json/RecordCollectionTest.php | 24 +++++++++++++++++++
12 files changed, 54 insertions(+), 16 deletions(-)
diff --git a/module/VuFindSearch/src/VuFindSearch/Backend/AbstractBackend.php b/module/VuFindSearch/src/VuFindSearch/Backend/AbstractBackend.php
index dff9533c693..758ac6db9e8 100644
--- a/module/VuFindSearch/src/VuFindSearch/Backend/AbstractBackend.php
+++ b/module/VuFindSearch/src/VuFindSearch/Backend/AbstractBackend.php
@@ -110,14 +110,11 @@ abstract class AbstractBackend implements BackendInterface, LoggerAwareInterface
*
* @param ResponseInterface $response Response
*
- * @return void
+ * @return ResponseInterface
*/
protected function injectSourceIdentifier(RecordCollectionInterface $response)
{
$response->setSourceIdentifier($this->identifier);
- foreach ($response as $record) {
- $record->setSourceIdentifier($this->identifier);
- }
return $response;
}
}
diff --git a/module/VuFindSearch/src/VuFindSearch/Backend/BrowZine/Response/RecordCollectionFactory.php b/module/VuFindSearch/src/VuFindSearch/Backend/BrowZine/Response/RecordCollectionFactory.php
index d9a55f933b4..6de40da175c 100644
--- a/module/VuFindSearch/src/VuFindSearch/Backend/BrowZine/Response/RecordCollectionFactory.php
+++ b/module/VuFindSearch/src/VuFindSearch/Backend/BrowZine/Response/RecordCollectionFactory.php
@@ -100,7 +100,7 @@ class RecordCollectionFactory implements RecordCollectionFactoryInterface
}
$collection = new $this->collectionClass($response);
foreach ($response['data'] as $doc) {
- $collection->add(call_user_func($this->recordFactory, $doc));
+ $collection->add(call_user_func($this->recordFactory, $doc), false);
}
return $collection;
}
diff --git a/module/VuFindSearch/src/VuFindSearch/Backend/EDS/Response/RecordCollectionFactory.php b/module/VuFindSearch/src/VuFindSearch/Backend/EDS/Response/RecordCollectionFactory.php
index 577e8558e53..6fab79eebce 100644
--- a/module/VuFindSearch/src/VuFindSearch/Backend/EDS/Response/RecordCollectionFactory.php
+++ b/module/VuFindSearch/src/VuFindSearch/Backend/EDS/Response/RecordCollectionFactory.php
@@ -98,7 +98,7 @@ class RecordCollectionFactory implements RecordCollectionFactoryInterface
?? $response['Records'] ?? [];
foreach ($records as $record) {
- $collection->add(call_user_func($this->recordFactory, $record));
+ $collection->add(call_user_func($this->recordFactory, $record), false);
}
return $collection;
}
diff --git a/module/VuFindSearch/src/VuFindSearch/Backend/EIT/Response/XML/RecordCollectionFactory.php b/module/VuFindSearch/src/VuFindSearch/Backend/EIT/Response/XML/RecordCollectionFactory.php
index 9c237a60adb..170774149e2 100644
--- a/module/VuFindSearch/src/VuFindSearch/Backend/EIT/Response/XML/RecordCollectionFactory.php
+++ b/module/VuFindSearch/src/VuFindSearch/Backend/EIT/Response/XML/RecordCollectionFactory.php
@@ -96,7 +96,7 @@ class RecordCollectionFactory implements RecordCollectionFactoryInterface
}
$collection = new $this->collectionClass($response);
foreach ($response['docs'] as $doc) {
- $collection->add(call_user_func($this->recordFactory, $doc));
+ $collection->add(call_user_func($this->recordFactory, $doc), false);
}
return $collection;
}
diff --git a/module/VuFindSearch/src/VuFindSearch/Backend/LibGuides/Response/RecordCollectionFactory.php b/module/VuFindSearch/src/VuFindSearch/Backend/LibGuides/Response/RecordCollectionFactory.php
index ede139d3023..fada0c10634 100644
--- a/module/VuFindSearch/src/VuFindSearch/Backend/LibGuides/Response/RecordCollectionFactory.php
+++ b/module/VuFindSearch/src/VuFindSearch/Backend/LibGuides/Response/RecordCollectionFactory.php
@@ -100,7 +100,7 @@ class RecordCollectionFactory implements RecordCollectionFactoryInterface
}
$collection = new $this->collectionClass($response);
foreach ($response['documents'] as $doc) {
- $collection->add(call_user_func($this->recordFactory, $doc));
+ $collection->add(call_user_func($this->recordFactory, $doc), false);
}
return $collection;
}
diff --git a/module/VuFindSearch/src/VuFindSearch/Backend/Pazpar2/Response/RecordCollectionFactory.php b/module/VuFindSearch/src/VuFindSearch/Backend/Pazpar2/Response/RecordCollectionFactory.php
index c8acc2dac29..e624040d54e 100644
--- a/module/VuFindSearch/src/VuFindSearch/Backend/Pazpar2/Response/RecordCollectionFactory.php
+++ b/module/VuFindSearch/src/VuFindSearch/Backend/Pazpar2/Response/RecordCollectionFactory.php
@@ -93,7 +93,7 @@ class RecordCollectionFactory implements RecordCollectionFactoryInterface
$response['total'], $response['offset']
);
foreach ($response['records'] as $doc) {
- $collection->add(call_user_func($this->recordFactory, $doc));
+ $collection->add(call_user_func($this->recordFactory, $doc), false);
}
return $collection;
}
diff --git a/module/VuFindSearch/src/VuFindSearch/Backend/Primo/Response/RecordCollectionFactory.php b/module/VuFindSearch/src/VuFindSearch/Backend/Primo/Response/RecordCollectionFactory.php
index 210ca51b7b8..7837856a725 100644
--- a/module/VuFindSearch/src/VuFindSearch/Backend/Primo/Response/RecordCollectionFactory.php
+++ b/module/VuFindSearch/src/VuFindSearch/Backend/Primo/Response/RecordCollectionFactory.php
@@ -100,7 +100,7 @@ class RecordCollectionFactory implements RecordCollectionFactoryInterface
}
$collection = new $this->collectionClass($response);
foreach ($response['documents'] as $doc) {
- $collection->add(call_user_func($this->recordFactory, $doc));
+ $collection->add(call_user_func($this->recordFactory, $doc), false);
}
return $collection;
}
diff --git a/module/VuFindSearch/src/VuFindSearch/Backend/Solr/Response/Json/RecordCollectionFactory.php b/module/VuFindSearch/src/VuFindSearch/Backend/Solr/Response/Json/RecordCollectionFactory.php
index 6ffd4eff6c4..3618d941906 100644
--- a/module/VuFindSearch/src/VuFindSearch/Backend/Solr/Response/Json/RecordCollectionFactory.php
+++ b/module/VuFindSearch/src/VuFindSearch/Backend/Solr/Response/Json/RecordCollectionFactory.php
@@ -97,7 +97,7 @@ class RecordCollectionFactory implements RecordCollectionFactoryInterface
$collection = new $this->collectionClass($response);
if (isset($response['response']['docs'])) {
foreach ($response['response']['docs'] as $doc) {
- $collection->add(call_user_func($this->recordFactory, $doc));
+ $collection->add(call_user_func($this->recordFactory, $doc), false);
}
}
return $collection;
diff --git a/module/VuFindSearch/src/VuFindSearch/Backend/Summon/Response/RecordCollectionFactory.php b/module/VuFindSearch/src/VuFindSearch/Backend/Summon/Response/RecordCollectionFactory.php
index fbfb17f124b..aa319b2d92f 100644
--- a/module/VuFindSearch/src/VuFindSearch/Backend/Summon/Response/RecordCollectionFactory.php
+++ b/module/VuFindSearch/src/VuFindSearch/Backend/Summon/Response/RecordCollectionFactory.php
@@ -100,7 +100,7 @@ class RecordCollectionFactory implements RecordCollectionFactoryInterface
}
$collection = new $this->collectionClass($response);
foreach ($response['documents'] as $doc) {
- $collection->add(call_user_func($this->recordFactory, $doc));
+ $collection->add(call_user_func($this->recordFactory, $doc), false);
}
return $collection;
}
diff --git a/module/VuFindSearch/src/VuFindSearch/Backend/WorldCat/Response/XML/RecordCollectionFactory.php b/module/VuFindSearch/src/VuFindSearch/Backend/WorldCat/Response/XML/RecordCollectionFactory.php
index 38a35c4f428..29c727c73c1 100644
--- a/module/VuFindSearch/src/VuFindSearch/Backend/WorldCat/Response/XML/RecordCollectionFactory.php
+++ b/module/VuFindSearch/src/VuFindSearch/Backend/WorldCat/Response/XML/RecordCollectionFactory.php
@@ -100,7 +100,7 @@ class RecordCollectionFactory implements RecordCollectionFactoryInterface
}
$collection = new $this->collectionClass($response);
foreach ($response['docs'] as $doc) {
- $collection->add(call_user_func($this->recordFactory, $doc));
+ $collection->add(call_user_func($this->recordFactory, $doc), false);
}
return $collection;
}
diff --git a/module/VuFindSearch/src/VuFindSearch/Response/AbstractRecordCollection.php b/module/VuFindSearch/src/VuFindSearch/Response/AbstractRecordCollection.php
index 9320e7f226c..196744af486 100644
--- a/module/VuFindSearch/src/VuFindSearch/Response/AbstractRecordCollection.php
+++ b/module/VuFindSearch/src/VuFindSearch/Response/AbstractRecordCollection.php
@@ -127,6 +127,9 @@ abstract class AbstractRecordCollection implements RecordCollectionInterface
public function setSourceIdentifier($identifier)
{
$this->source = $identifier;
+ foreach ($this->records as $record) {
+ $record->setSourceIdentifier($identifier);
+ }
}
/**
@@ -142,18 +145,32 @@ abstract class AbstractRecordCollection implements RecordCollectionInterface
/**
* Add a record to the collection.
*
- * @param RecordInterface $record Record to add
+ * @param RecordInterface $record Record to add
+ * @param bool $checkExisting Whether to check for existing record in
+ * the collection (slower, but makes sure there are no duplicates)
*
* @return void
*/
- public function add(RecordInterface $record)
+ public function add(RecordInterface $record, $checkExisting = true)
{
- if (!in_array($record, $this->records, true)) {
+ if (!$checkExisting || !$this->has($record)) {
$this->records[$this->pointer] = $record;
$this->next();
}
}
+ /**
+ * Check if the collection contains the given record
+ *
+ * @param RecordInterface $record Record to check
+ *
+ * @return bool
+ */
+ public function has(RecordInterface $record)
+ {
+ return in_array($record, $this->records, true);
+ }
+
/**
* Replace a record in the collection.
*
diff --git a/module/VuFindSearch/tests/unit-tests/src/VuFindTest/Backend/Solr/Response/Json/RecordCollectionTest.php b/module/VuFindSearch/tests/unit-tests/src/VuFindTest/Backend/Solr/Response/Json/RecordCollectionTest.php
index d02b6d932ca..3c4357babe1 100644
--- a/module/VuFindSearch/tests/unit-tests/src/VuFindTest/Backend/Solr/Response/Json/RecordCollectionTest.php
+++ b/module/VuFindSearch/tests/unit-tests/src/VuFindTest/Backend/Solr/Response/Json/RecordCollectionTest.php
@@ -192,4 +192,28 @@ class RecordCollectionTest extends TestCase
$this->assertTrue(in_array($r2, $final));
$this->assertTrue(in_array($r3, $final));
}
+
+ /**
+ * Test that the object handles offsets properly.
+ *
+ * @return void
+ */
+ public function testAdd()
+ {
+ $coll = new RecordCollection(
+ [
+ 'response' => ['numFound' => 10, 'start' => 5]
+ ]
+ );
+ $record = $this->createMock(\VuFindSearch\Response\RecordInterface::class);
+ $coll->add($record);
+ for ($i = 0; $i < 4; $i++) {
+ $coll->add($this->createMock(\VuFindSearch\Response\RecordInterface::class));
+ }
+ $this->assertEquals(5, $coll->count());
+ $coll->add($record);
+ $this->assertEquals(5, $coll->count());
+ $coll->add($record, false);
+ $this->assertEquals(6, $coll->count());
+ }
}
--
GitLab