Bug 18374

Summary: Respect QueryAutoTruncate syspref in Elasticsearch
Product: Koha Reporter: Nick Clemens <nick>
Component: Searching - ElasticsearchAssignee: Nick Clemens <nick>
Status: CLOSED FIXED QA Contact: Julian Maurice <julian.maurice>
Severity: major    
Priority: P3 CC: fridolin.somers, jonathan.druart, julian.maurice, katrin.fischer, kyle, tomascohen
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 19559    
Attachments: Bug 18374 - Respect QueryAutoTruncate syspref in Elasticsearch
Bug 18374 - QueryAutoTruncate unit tests
Bug 18374 - Respect QueryAutoTruncate syspref in Elasticsearch
Bug 18374 - QueryAutoTruncate unit tests
Bug 18374 - Respect QueryAutoTruncate syspref in Elasticsearch
Bug 18374 - Respect QueryAutoTruncate syspref in Elasticsearch
Bug 18374 - QueryAutoTruncate unit tests
Bug 18374 - Respect QueryAutoTruncate syspref in Elasticsearch
Bug 18374: QueryAutoTruncate unit tests
Bug 18374: Respect QueryAutoTruncate syspref in Elasticsearch
Bug 18374: (QA followup) Remove warning in Elasticsearch/Search.pm
Bug 18374: QueryAutoTruncate unit tests
Bug 18374: Respect QueryAutoTruncate syspref in Elasticsearch
Bug 18374: (QA followup) Remove warning in Elasticsearch/Search.pm
Bug 18374: (QA follow-up) Tidy up code
Bug 18374: Followup - Add support and tests for quoted strings
Bug 18374: QueryAutoTruncate unit tests
Bug 18374: Respect QueryAutoTruncate syspref in Elasticsearch
Bug 18374: (QA followup) Remove warning in Elasticsearch/Search.pm
Bug 18374: (QA follow-up) Tidy up code
Bug 18374: Followup - Add support and tests for quoted strings
Bug 18374: (QA follow-up) Simplify _truncate_terms
Bug 18374: (QA follow-up) Fix auto truncation for field:"value"

Description Nick Clemens 2017-04-03 18:30:40 UTC
Currently in Elasticsearch a search for 
'shakes' 
will not match 
'shakespeare'
but you cna force it with
'shakes*'

This patchset will obey the QueryAutoTruncate to add '*' to all search terms
Comment 1 Nick Clemens 2017-04-03 18:37:07 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2017-04-03 18:51:27 UTC Comment hidden (obsolete)
Comment 3 Nick Clemens 2017-04-06 18:55:28 UTC Comment hidden (obsolete)
Comment 4 Nick Clemens 2017-04-06 18:55:33 UTC Comment hidden (obsolete)
Comment 5 Nick Clemens 2017-04-06 19:45:26 UTC
Created attachment 61954 [details] [review]
Bug 18374 - QueryAutoTruncate unit tests

To test:
1 - Apply just this patch - should fail
2 - Apply the other patch - should pass

https://bugs.koha-community.org/show_bug.cgi?id=18347
Comment 6 Nick Clemens 2017-04-06 19:45:46 UTC Comment hidden (obsolete)
Comment 7 Nick Clemens 2017-04-08 03:44:37 UTC
Created attachment 61983 [details] [review]
Bug 18374 - Respect QueryAutoTruncate syspref in Elasticsearch

This patchset adds a subroutine '_truncate_terms' to the ES QueryParser.

If QueryAutoTruncate is enabled this function will be called for any
search to add wildcard '*' to all terms

To test:
1 - Enable Elasticsearch and have some records indexed
2 - Search for partial terms
3 - Note they fail unless '*' is appended
4 - Apply patch, leave QueryAutoTruncate disabled
5 - Note partial term searches still fail
6 - Enable QueryAutoTruncate
7 - Note partial term searches succeed
8 - Do some regular and advanced searches to make sure results are as
expected

https://bugs.koha-community.org/show_bug.cgi?id=18347
Comment 8 Jonathan Druart 2017-04-11 20:05:28 UTC
    #   Failed test at t/db_dependent/Koha_SearchEngine_Elasticsearch_Search.t line 112.
    #          got: 'donald duck'
    #     expected: 'donald* duck*'

    #   Failed test at t/db_dependent/Koha_SearchEngine_Elasticsearch_Search.t line 114.
    #          got: 'donald or duck and mickey not mouse'
    #     expected: 'donald* or duck* and mickey* not mouse*'
Comment 9 Nick Clemens 2017-07-07 10:37:51 UTC
Created attachment 64903 [details] [review]
Bug 18374 - QueryAutoTruncate unit tests

To test:
1 - Apply just this patch - should fail
2 - Apply the other patch - should pass

https://bugs.koha-community.org/show_bug.cgi?id=18347
Comment 10 Nick Clemens 2017-07-07 10:37:54 UTC
Created attachment 64904 [details] [review]
Bug 18374 - Respect QueryAutoTruncate syspref in Elasticsearch

This patchset adds a subroutine '_truncate_terms' to the ES QueryParser.

If QueryAutoTruncate is enabled this function will be called for any
search to add wildcard '*' to all terms

To test:
1 - Enable Elasticsearch and have some records indexed
2 - Search for partial terms
3 - Note they fail unless '*' is appended
4 - Apply patch, leave QueryAutoTruncate disabled
5 - Note partial term searches still fail
6 - Enable QueryAutoTruncate
7 - Note partial term searches succeed
8 - Do some regular and advanced searches to make sure results are as
expected

https://bugs.koha-community.org/show_bug.cgi?id=18347
Comment 11 Tomás Cohen Arazi 2017-09-12 15:17:41 UTC
Created attachment 67098 [details] [review]
Bug 18374: QueryAutoTruncate unit tests

To test:
1 - Apply just this patch - should fail
2 - Apply the other patch - should pass

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 12 Tomás Cohen Arazi 2017-09-12 15:17:49 UTC
Created attachment 67099 [details] [review]
Bug 18374: Respect QueryAutoTruncate syspref in Elasticsearch

This patchset adds a subroutine '_truncate_terms' to the ES QueryParser.

If QueryAutoTruncate is enabled this function will be called for any
search to add wildcard '*' to all terms

To test:
1 - Enable Elasticsearch and have some records indexed
2 - Search for partial terms
3 - Note they fail unless '*' is appended
4 - Apply patch, leave QueryAutoTruncate disabled
5 - Note partial term searches still fail
6 - Enable QueryAutoTruncate
7 - Note partial term searches succeed
8 - Do some regular and advanced searches to make sure results are as
expected

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 13 Tomás Cohen Arazi 2017-09-12 15:17:56 UTC
Created attachment 67100 [details] [review]
Bug 18374: (QA followup) Remove warning in Elasticsearch/Search.pm

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 14 Julian Maurice 2017-09-29 14:20:57 UTC
Created attachment 67467 [details] [review]
Bug 18374: QueryAutoTruncate unit tests

To test:
1 - Apply just this patch - should fail
2 - Apply the other patch - should pass

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 15 Julian Maurice 2017-09-29 14:21:02 UTC
Created attachment 67468 [details] [review]
Bug 18374: Respect QueryAutoTruncate syspref in Elasticsearch

This patchset adds a subroutine '_truncate_terms' to the ES QueryParser.

If QueryAutoTruncate is enabled this function will be called for any
search to add wildcard '*' to all terms

To test:
1 - Enable Elasticsearch and have some records indexed
2 - Search for partial terms
3 - Note they fail unless '*' is appended
4 - Apply patch, leave QueryAutoTruncate disabled
5 - Note partial term searches still fail
6 - Enable QueryAutoTruncate
7 - Note partial term searches succeed
8 - Do some regular and advanced searches to make sure results are as
expected

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 16 Julian Maurice 2017-09-29 14:21:06 UTC
Created attachment 67469 [details] [review]
Bug 18374: (QA followup) Remove warning in Elasticsearch/Search.pm

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 17 Julian Maurice 2017-09-29 14:21:10 UTC
Created attachment 67470 [details] [review]
Bug 18374: (QA follow-up) Tidy up code

Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Comment 18 Jonathan Druart 2017-09-29 15:35:33 UTC
Please add more tests:
  - donald duck and the mouse
  - *
  - "donald duck" and "the mouse"
  - field:value

I am sure you will see that it needs more work :)
Comment 19 Nick Clemens 2017-10-06 15:39:21 UTC
Created attachment 67727 [details] [review]
Bug 18374: Followup - Add support and tests for quoted strings

To test:
1 - prove t/db_dependent/Koha_SearchEngine_Elasticsearch_Search.t
2 - do some searches in staff client and test results
Comment 20 Nick Clemens 2017-10-06 15:40:13 UTC
(In reply to Jonathan Druart from comment #18)
> Please add more tests:
>   - donald duck and the mouse
>   - *
>   - "donald duck" and "the mouse"
>   - field:value
> 
> I am sure you will see that it needs more work :)

Thanks Jonathan, followup provided, let me know if you think of more cases
Comment 21 Julian Maurice 2017-10-13 10:08:25 UTC
Created attachment 68062 [details] [review]
Bug 18374: QueryAutoTruncate unit tests

To test:
1 - Apply just this patch - should fail
2 - Apply the other patch - should pass

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Comment 22 Julian Maurice 2017-10-13 10:08:28 UTC
Created attachment 68063 [details] [review]
Bug 18374: Respect QueryAutoTruncate syspref in Elasticsearch

This patchset adds a subroutine '_truncate_terms' to the ES QueryParser.

If QueryAutoTruncate is enabled this function will be called for any
search to add wildcard '*' to all terms

To test:
1 - Enable Elasticsearch and have some records indexed
2 - Search for partial terms
3 - Note they fail unless '*' is appended
4 - Apply patch, leave QueryAutoTruncate disabled
5 - Note partial term searches still fail
6 - Enable QueryAutoTruncate
7 - Note partial term searches succeed
8 - Do some regular and advanced searches to make sure results are as
expected

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Comment 23 Julian Maurice 2017-10-13 10:08:31 UTC
Created attachment 68064 [details] [review]
Bug 18374: (QA followup) Remove warning in Elasticsearch/Search.pm

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Comment 24 Julian Maurice 2017-10-13 10:08:34 UTC
Created attachment 68065 [details] [review]
Bug 18374: (QA follow-up) Tidy up code

Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Comment 25 Julian Maurice 2017-10-13 10:08:38 UTC
Created attachment 68066 [details] [review]
Bug 18374: Followup - Add support and tests for quoted strings

To test:
1 - prove t/db_dependent/Koha_SearchEngine_Elasticsearch_Search.t
2 - do some searches in staff client and test results

Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Comment 26 Julian Maurice 2017-10-13 10:08:41 UTC
Created attachment 68067 [details] [review]
Bug 18374: (QA follow-up) Simplify _truncate_terms

By using a different split regex, we can simplify a bit the process of
appending '*' to every word of the query

Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Comment 27 Julian Maurice 2017-10-13 10:08:45 UTC
Created attachment 68068 [details] [review]
Bug 18374: (QA follow-up) Fix auto truncation for field:"value"

Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Comment 28 Jonathan Druart 2017-10-13 16:59:49 UTC
Pushed to master for 17.11, thanks to everybody involved!
Comment 29 Fridolin Somers 2017-10-25 08:01:48 UTC
Pushed to 17.05.x, will be in 17.05.06.
Comment 30 Katrin Fischer 2017-10-29 18:09:35 UTC
Missing too many Elasticsearch patches in 16.11.x by now - skipping.