Fix removal of documents from index when they no longer exist in Silverstripe

silverstripe / silverstripe-search-service

A service-agnostic search module for Silverstripe CMS

BSD 3-Clause "New" or "Revised" License

5 stars 18 forks source link

Fix removal of documents from index when they no longer exist in Silverstripe #55

Closed madmatt closed 3 years ago

madmatt commented 3 years ago

Fixes #36.

API changes:

Add new BatchDocumentRemovalInterface interface and removeAllDocuments method
Implement removeAllDocuments for AppSearchService, ServiceFake, and NaiveSearchService

Fixes:

Fix ClearIndexJob so that it runs in a single job run (provided it can finish within limits), and retries up to 5 times
Add messaging to ClearIndexJob so we know whether it succeeded or not

Tests:

Add removeAllDocuments to ServiceFake
Add new and fix existing ClearIndexJob tests

madmatt commented 3 years ago

I can't seem to see codecov stats at all - the links in this PR just go straight to the CodeCov homepage, and logging in doesn't show me anything under the silverstripe namespace. I presume this is because the new removeAllDocuments method isn't covered but can anyone else access CodeCov to confirm if there's anything else missing tests now?

madmatt commented 3 years ago

No that's fair, I'll jump on that tonight. Thanks @unclecheese!

madmatt commented 3 years ago

I'll fix up the test failure tomorrow - I know what is causing it, just need to remember my PHPUnit mocking-fu

madmatt commented 3 years ago

Alright I've fixed this up now @unclecheese, hopefully this is what you were after :-)

unclecheese commented 3 years ago

Almost there. Just removed the duck typing, and we should be good to go if that passes.

madmatt commented 3 years ago

Nice, thanks @unclecheese!