Skip to content

Commit

Permalink
Ensure add/removeDocuments returns string for IDs
Browse files Browse the repository at this point in the history
  • Loading branch information
Chris Penny committed Aug 10, 2022
1 parent b48b133 commit 1de946d
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 7 deletions.
14 changes: 9 additions & 5 deletions src/Service/EnterpriseSearch/EnterpriseSearchService.php
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public function getExternalURL(): ?string

public function getExternalURLDescription(): ?string
{
return 'Elastic App Search Dashboard';
return 'Elastic Enterprise Search Dashboard';
}

public function getDocumentationURL(): ?string
Expand Down Expand Up @@ -107,10 +107,12 @@ public function addDocuments(array $documents): array

$this->handleError($response);

$processedIds += array_column($response, 'id');
// Grab all the ID values, and also cast them to string
$processedIds += array_map('strval', array_column($response, 'id'));
}

return $processedIds;
// One document could have existed in multiple indexes, we only care to track it once
return array_unique($processedIds);
}

/**
Expand Down Expand Up @@ -162,10 +164,12 @@ public function removeDocuments(array $items): array

// Results here can be marked as deleted true or false. false would indicate that no document with that ID
// exists in Elastic - which, is the same result, really
$processedIds += array_column($response, 'id');
// Grab all the ID values, and also cast them to string
$processedIds += array_map('strval', array_column($response, 'id'));
}

return $processedIds;
// One document could have existed in multiple indexes, we only care to track it once
return array_unique($processedIds);
}

/**
Expand Down
15 changes: 13 additions & 2 deletions tests/Service/EnterpriseSearch/EnterpriseSearchServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
use SilverStripe\SearchService\Service\EnterpriseSearch\EnterpriseSearchService;
use SilverStripe\SearchService\Service\IndexConfiguration;
use SilverStripe\SearchService\Tests\Fake\DataObjectFake;
use SilverStripe\SearchService\Tests\Fake\DataObjectFakePrivate;
use SilverStripe\SearchService\Tests\Fake\DataObjectFakeVersioned;
use SilverStripe\SearchService\Tests\Fake\ImageFake;
use SilverStripe\SearchService\Tests\Fake\TagFake;
Expand All @@ -34,6 +35,7 @@ class EnterpriseSearchServiceTest extends SearchServiceTest
*/
protected static $extra_dataobjects = [
DataObjectFake::class,
DataObjectFakePrivate::class,
DataObjectFakeVersioned::class,
TagFake::class,
ImageFake::class,
Expand Down Expand Up @@ -1207,7 +1209,11 @@ public function testAddDocuments(): void
'errors' => [],
],
[
'id' => 'doc-321',
'id' => 321, // We'll check that this is cast to string
'errors' => [],
],
[
'id' => '321', // Should be removed as a duplicate of the above
'errors' => [],
],
]);
Expand All @@ -1217,7 +1223,7 @@ public function testAddDocuments(): void

$expectedIds = [
'doc-123',
'doc-321',
'321',
];

$resultIds = $this->searchService->addDocuments($documents);
Expand Down Expand Up @@ -1353,6 +1359,10 @@ public function testRemoveDocuments(): void
'id' => sprintf('silverstripe_searchservice_tests_fake_dataobjectfake_%s', $documentThree->ID),
'deleted' => true,
],
[
'id' => 123, // Test that int is cast to string
'deleted' => true,
],
]);

// Append this mock response to our stack
Expand All @@ -1362,6 +1372,7 @@ public function testRemoveDocuments(): void
sprintf('silverstripe_searchservice_tests_fake_dataobjectfake_%s', $documentOne->ID),
sprintf('silverstripe_searchservice_tests_fake_dataobjectfake_%s', $documentTwo->ID),
sprintf('silverstripe_searchservice_tests_fake_dataobjectfake_%s', $documentThree->ID),
'123',
];

$resultIds = $this->searchService->addDocuments($documents);
Expand Down

0 comments on commit 1de946d

Please sign in to comment.