Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Search Improvements (OS/ES support) #298

Draft
wants to merge 59 commits into
base: master
Choose a base branch
from

Conversation

patrick-austin
Copy link
Contributor

Covers a variety of improvements to free text search functionality.
Closes #267

Changes to functionality

  • Expanded the number of fields that are indexed and searchable across various entities
  • Metadata from a search comes directly from Lucene, rather than returning a list of ids that then require a subsequent DB call.
    • In addition to performing authorisation on the main returned entity (Investigation/Dataset/Datafile), we also use the public steps/tables to safely return nested information (such as a DatafileParameter without additional time spent authorising)
  • Support for faceting of results, sorting, searching after, flexible batches of search results
  • Support for other search engines, specifically Opensearch/Elasticsearch
    • Note that a branch without the Opensearch code but all the other features exists

Changes to code

  • Various entities have had getDoc either added or expanded, alongside the addition of getDocumentFields which enforces that the search component does not return metadata that isn't allowed by the ICAT public steps/tables
  • Modified existing search functions in ICATRest and added new endpoints search/documents and facet/documents
  • Changes to EntityBeanManager to support new search calls
  • Small additions to GateKeeper to allow the publicly allowed search fields to be marked as stale and updated as public steps/tables are updated
  • Changes to PropertyHandler to support different SearchEngines as config options as well as other new config parameters
  • LuceneManager replaced with generic SearchManager which interacts with the search engine via an instance of SearchApi
  • Creation of SearchApi, and abstract class for interfacing with search engine implementations
  • LuceneApi modified extensively, and now extends SearchApi
  • Creation of OpensearchApi which extends SearchApi and supports either Opensearch or Elasticsearch clusters by building generic JSON. Also has some static functionality refactored into supporting classes:
    • OpensearchQueryBuilder
    • OpensearchScriptBuilder
  • Creation of FacetDimension and FacetLabel to represent facets of results, and replacing LuceneSearchResult with SearchResult which has facet support
  • Various small changes to rename variables from luceneAbc to searchAbc to reflect what is or isn't specific to the icat.lucene search engine
  • Updated tests

Interdependencies on other components

patrick-austin and others added 30 commits February 1, 2022 19:58
Copy link
Contributor

@VKTB VKTB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will continue reviewing the rest of the changes after I re-review the changes you have made in the icat.lucene PR.

@VKTB
Copy link
Contributor

VKTB commented Oct 11, 2022

@patrick-austin I have finished reviewing this PR so feel free to start addressing and pushing changes. Let me know when you are done and I will review your changes.

Copy link
Contributor

@VKTB VKTB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking much nicer, thanks for making the changes. Just a few docstring related comments which once address will make this PR ready to be merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Compatibility with icat.lucene upgrades
3 participants