Problem/Motivation

A misspelling (analyser vs analyzer) in \Drupal\elasticsearch_connector\Event\SynonymsSubscriber::onAlterSettings() means that the querytime_synonyms analyzer doesn't work.

The same misspelling in \Drupal\elasticsearch_connector\SearchAPI\BackendClient::updateSettings() means that analyzers cannot be assigned to fields properly.

Proposed resolution

Change $settings['analysis']['analyser']['querytime_synonyms'] to $settings['analysis']['analyzer']['querytime_synonyms'] in \Drupal\elasticsearch_connector\Event\SynonymsSubscriber::onAlterSettings(). - note this was fixed in #3575982: Elasticsearch property misspelled in SynonymsSubscriber::onAlterSettings.

Change $field_definition['analyser'] to $field_definition['analyzer'] and $field_definition['analyser_settings'] to $field_definition['analyzer_settings'] in \Drupal\elasticsearch_connector\SearchAPI\BackendClient::updateSettings().

Remaining tasks

  1. Add a merge request for 8.0.xx - merge request !175 added by @batkor in #3
  2. Add a merge request for 9.0.x - merge request !193 added by @mparker17 in #5
  3. Add regression tests - done by @mparker17 by #5
  4. Community review - skipped by @mparker17 in #7
  5. Maintainer review - done by @mparker17 in #4
  6. Commit to 9.0.x - merged by @mparker17 in #8
  7. Commit to 8.0.x - merged by @mparker17 in #10
  8. Release 9.0.x - released in 9.0.0-alpha3 by @mparker17
  9. Release 8.0.x - released in 8.0.0-alpha7 by @mparker17

User interface changes

None.

Introduced terminology

None.

API changes

Only API additions.

Data model changes

None.

Release notes snippet

Fixed a misspelling in SynonymsSubscriber and BackendClient which prevented custom analyzers from being defined and associated with fields.

Original report by @batkor

Analyzer "querytime_synonyms" not working of the typo error.

Issue fork elasticsearch_connector-3570041

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

batkor created an issue. See original summary.

batkor’s picture

batkor
Location Irkutsk
commented

See doc. https://www.elastic.co/docs/manage-data/data-store/text-analysis/create-...

batkor opened merge request !175

mparker17’s picture

mparker17
he/him
Primary language English
Location UTC-5
commented

@batkor, thank you for your contribution, and I apologize for the time it has taken me to review this issue!

I can accept most of the changes to SynonymsSubscriber, SynonymsSubscriberTest, BackendClient, and BackendClientFactory; and you uncovered a part of the module that has been untested until now! So I've written tests (and an example implementation), and I would be happy to credit you on this issue! But, respectfully, some of the changes in your merge request change the class names, namespace, and machine name of the plugin, which will break other sites using this part of the module (my client's site included).

This particular misspelling (Analyser) comes from #3271337: Add native support for n-gram and edge n-gram in the Search API OpenSearch module. As a bit of background, when I was brought on as a maintainer of Elasticsearch Connector, the other maintainers wanted the code in this module to echo search_api_opensearch as much as possible for two reasons: first, it would make it easier for site builders to switch between Elasticsearch and OpenSearch with minimal code changes; and second, it would ease the maintenance burden on maintainers in both projects when code can be easily shared between them.

mparker17 opened merge request !193

mparker17 changed the visibility of the branch 3570041-analyzer-querytimesynonyms-not to hidden.

mparker17’s picture

mparker17
he/him
Primary language English
Location UTC-5
commented
Version: 8.0.x-dev » 9.0.x-dev
Assigned: batkor » Unassigned
Issue summary: View changes
Status: Active » Reviewed & tested by the community
Issue tags: +needs backport to 8.0.x

Filled in the issue summary as documentation for future users. I will merge to 9.0.x first; then I will merge to 8.0.x.

  • mparker17 committed ea083962 on 9.0.x 
    fix: #3570041 Analyzer querytime_synonyms not working

By: batkor
By:...
mparker17’s picture

mparker17
he/him
Primary language English
Location UTC-5
commented
Version: 9.0.x-dev » 8.0.x-dev
Issue summary: View changes
Status: Reviewed & tested by the community » Patch (to be ported)
Issue tags: -needs backport to 8.0.x

Merged to 9.0.x; will merge to 8.0.x shortly.

  • mparker17 committed 66fcd36b on 8.0.x authored by batkor 
    fix: #3570041 Analyzer querytime_synonyms not working

By: batkor
By:...
mparker17’s picture

mparker17
he/him
Primary language English
Location UTC-5
commented
Issue summary: View changes
Status: Patch (to be ported) » Fixed

Merged to 8.0.x. I will update this issue when I have released these changes.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

mparker17’s picture

mparker17
he/him
Primary language English
Location UTC-5
commented
Issue summary: View changes

The changes in this issue were released in elasticsearch_connector-9.0.0-alpha3, and elasticsearch_connector-8.0.0-alpha7

batkor’s picture

batkor
Location Irkutsk
commented

Thanks

mparker17’s picture

mparker17
he/him
Primary language English
Location UTC-5
commented

@batkor, thank you again for bringing this to my attention!

I noticed that search_api_opensearch has an upcoming 3.x version with no releases, i.e.: where they can make API-breaking changes, so I've filed #3587756: Deprecate "Analyser" in 2.x, use "Analyzer" in 3.x to fix the issue upstream, and I have asked them to credit you when they accept the changes.

When search_api_opensearch accepts the changes in #3587756, then I intend to incorporate more of your original patch to this issue into elasticsearch_connector.

Given how long elasticsearch_connector has spent in alpha, and given that I'm already writing a backwards-compatibility layer in #3587756, I will probably write a similar backwards-compatibility layer for elasticsearch_connector (even if one isn't conventionally necessary), so that I don't break anyone's site. But, hopefully this misspelling will be gone soon!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.