EnterpriseSearchService and EnterpriseSearchServiceTest
This is where most of the effort was spent.
I've completely refactored the way we perform tests. We now mock the responses from the API, rather than mocking responses from the method as a whole.
Lots of refactoring where I've broken single methods down into separate parts to help make testing easier (and more robust).
Document creation
Other than fixing linting, and trying to remove "complexity" (EG: by switching to "exit early" strategies), I haven't changed much (or anything) about how we create DataObjectDocuments.
To be honest, it's all confusing to me... I tried to stay away from this area as much as possible.
Breaking changes
PHP
Minimum PHP requirement has been increased to PHP 8.
Because this was going to be a breaking change anyway, it seemed like a good time to start adding PHP 8 supported type hints, etc.
Environment variables
Usages of the name APP has been replaced with ENTERPRISE.
Some methods previously returned void or $this, which meant they didn't give any useful feedback from the request we had just made.
Methods like EnterpriseSearchService::configure() now return info about the response (in this case, an array of the active configuration for each index).
Methods with changed return types:
IndexingInterface::configure()
IndexingInterface::addDocument()
IndexingInterface::removeDocument()
BatchDocumentInterface::addDocuments()
BatchDocumentInterface::removeDocuments()
Pretty much every property/parameter everywhere
So, I think I've been able to keep the general API for all methods (that being, the order and purpose of params), but a lot of methods have had type declarations added, so if folks are extended our classes and overriding methods, then these would likely be breaking changes for them.
Where possible, properties have been given type hints (where they didn't previously). Exceptions are the usual properties that extend some other vendor classes that they have to match.
NaiveSearchService
Practically, I don't think anyone would have been accessing this class, but the namespace has changed to be consistent with other classes.
I don't think this will be a breaking change, as the old App Search already explicitly used Guzzle as its HTTP Client. However, the new Enterprise search uses PSR-18 "discovery". I've found this to be a bit too fragile (EG: It was breaking for me with Symfony Client), so I've added a new config to our ClientFactory to allow folks to specify what client they would like instantiated. Default is Guzzle.
SilverStripe\Core\Injector\Injector:
Elastic\EnterpriseSearch\Client:
factory: SilverStripe\SearchService\Service\EnterpriseSearch\ClientFactory
constructor:
# original configs
host: '`ENTERPRISE_SEARCH_ENDPOINT`'
token: '`ENTERPRISE_SEARCH_API_KEY`'
# new config
http_client: '%$GuzzleHttp\Client'
recordBaseClass and pageContent
We had two conflicting configs, one defined these fields in snake case (EG: record_base_class), and the other defined them in camel case (EG: recordBaseClass). I've standardised all configs to snake case, as that is consistent with other field names.
Looking through some of my own projects, we seem to use the snake case, so this might not be a breaking change.
New features
New Permission for triggering ReIndex through Service Admin
SearchAdmin_ReIndex permission has been added so that it's not longer only ADMIN who are able to trigger a full ReIndex through the Model Admin area.
Fixes #71 Fixes #62 Replaces #70
Manual Testing
Enterprise Search and PHP 8.1
https://github.com/elastic/enterprise-search-php/pull/25
Focus area
EnterpriseSearchService
andEnterpriseSearchServiceTest
This is where most of the effort was spent.
I've completely refactored the way we perform tests. We now mock the responses from the API, rather than mocking responses from the method as a whole.
Lots of refactoring where I've broken single methods down into separate parts to help make testing easier (and more robust).
Document creation
Other than fixing linting, and trying to remove "complexity" (EG: by switching to "exit early" strategies), I haven't changed much (or anything) about how we create
DataObjectDocuments
.To be honest, it's all confusing to me... I tried to stay away from this area as much as possible.
Breaking changes
PHP
Minimum PHP requirement has been increased to PHP 8.
Because this was going to be a breaking change anyway, it seemed like a good time to start adding PHP 8 supported type hints, etc.
Environment variables
Usages of the name
APP
has been replaced withENTERPRISE
.Before:
After:
Method return types
Some methods previously returned
void
or$this
, which meant they didn't give any useful feedback from the request we had just made.Methods like
EnterpriseSearchService::configure()
now return info about the response (in this case, an array of the active configuration for each index).Methods with changed return types:
IndexingInterface::configure()
IndexingInterface::addDocument()
IndexingInterface::removeDocument()
BatchDocumentInterface::addDocuments()
BatchDocumentInterface::removeDocuments()
Pretty much every property/parameter everywhere
So, I think I've been able to keep the general API for all methods (that being, the order and purpose of params), but a lot of methods have had type declarations added, so if folks are extended our classes and overriding methods, then these would likely be breaking changes for them.
Where possible, properties have been given type hints (where they didn't previously). Exceptions are the usual properties that extend some other vendor classes that they have to match.
NaiveSearchService
Practically, I don't think anyone would have been accessing this class, but the namespace has changed to be consistent with other classes.
Before:
SilverStripe\SearchService\Services\Naive\NaiveSearchService
After:SilverStripe\SearchService\Service\Naive\NaiveSearchService
Maybe breaking
Default HTTP Client
I don't think this will be a breaking change, as the old App Search already explicitly used
Guzzle
as its HTTP Client. However, the new Enterprise search uses PSR-18 "discovery". I've found this to be a bit too fragile (EG: It was breaking for me with Symfony Client), so I've added a new config to ourClientFactory
to allow folks to specify what client they would like instantiated. Default isGuzzle
.recordBaseClass
andpageContent
We had two conflicting configs, one defined these fields in snake case (EG:
record_base_class
), and the other defined them in camel case (EG:recordBaseClass
). I've standardised all configs to snake case, as that is consistent with other field names.Looking through some of my own projects, we seem to use the snake case, so this might not be a breaking change.
New features
New Permission for triggering ReIndex through Service Admin
SearchAdmin_ReIndex
permission has been added so that it's not longer only ADMIN who are able to trigger a full ReIndex through the Model Admin area.How sad that this is my only new feature...