From 3551abc50207a1ae8f3c8f65f90d11186127d710 Mon Sep 17 00:00:00 2001 From: Alex Ioannidis Date: Mon, 18 Nov 2024 18:00:33 +0100 Subject: [PATCH] global: support loading mappings from function entrypoints --- invenio_search/ext.py | 98 ++++++++++++++++++++--------------- tests/conftest.py | 3 ++ tests/mock_module/__init__.py | 32 ++++++++++++ tests/test_cli.py | 24 ++++++--- tests/test_invenio_search.py | 53 +++++++++++-------- 5 files changed, 137 insertions(+), 73 deletions(-) diff --git a/invenio_search/ext.py b/invenio_search/ext.py index 300f9804..eb9ad76d 100644 --- a/invenio_search/ext.py +++ b/invenio_search/ext.py @@ -219,7 +219,12 @@ def _walk_dir(*parts): def load_entry_point_group_mappings(self, entry_point_group_mappings): """Load actions from an entry point group.""" for ep in iter_entry_points(group=entry_point_group_mappings): - self.register_mappings(ep.name, ep.module_name) + # Load the entry point using .load() and register the mappings + ep_value = ep.load() + if callable(ep_value): + ep_value(self) + else: + self.register_mappings(ep.name, ep.module_name) def _client_builder(self): """Build search engine (ES/OS) client.""" @@ -296,7 +301,7 @@ def _get_indices(self, tree_or_filename): def create_index( self, index, - mapping_path=None, + mapping=None, prefix=None, suffix=None, create_write_alias=True, @@ -304,39 +309,41 @@ def create_index( dry_run=False, ): """Create index with a write alias.""" - mapping_path = mapping_path or self.mappings[index] + mapping = mapping or self.mappings[index] + + if isinstance(mapping, dict): + mapping_body = mapping + # Load mapping from file + elif isinstance(mapping, str): + with open(mapping, "r") as mapping_body: + mapping_body = json.load(mapping_body) final_alias = None alias_result = None - # To prevent index init --force from creating a suffixed - # index if the current instance is running without suffixes - # make sure there is no index with the same name as the - # alias name (i.e. the index name without the suffix). - with open(mapping_path, "r") as body: - final_index = build_index_name( - index, prefix=prefix, suffix=suffix, app=self.app + final_index = build_index_name( + index, prefix=prefix, suffix=suffix, app=self.app + ) + if create_write_alias: + final_alias = build_alias_name(index, prefix=prefix, app=self.app) + index_result = ( + self.client.indices.create( + index=final_index, + body=mapping_body, + ignore=ignore, ) - if create_write_alias: - final_alias = build_alias_name(index, prefix=prefix, app=self.app) - index_result = ( - self.client.indices.create( + if not dry_run + else None + ) + if create_write_alias: + alias_result = ( + self.client.indices.put_alias( index=final_index, - body=json.load(body), + name=final_alias, ignore=ignore, ) if not dry_run else None ) - if create_write_alias: - alias_result = ( - self.client.indices.put_alias( - index=final_index, - name=final_alias, - ignore=ignore, - ) - if not dry_run - else None - ) return (final_index, index_result), (final_alias, alias_result) def create(self, ignore=None, ignore_existing=False, index_list=None): @@ -419,7 +426,16 @@ def _build(tree_or_filename, alias=None): def update_mapping(self, index, check=True): """Update mapping of the existing index.""" - mapping_path = self.mappings[index] + mapping = self.mappings[index] + if isinstance(mapping, dict): + mapping_body = mapping + # Load mapping from file + elif isinstance(mapping, str): + with open(mapping, "r") as mapping_body: + mapping_body = json.load(mapping_body) + # The Update API accepts only the mapping body, not the full index definition + mapping_body = mapping_body["mappings"] + index_alias_name = build_alias_name(index) # get api returns only dicts @@ -431,27 +447,25 @@ def update_mapping(self, index, check=True): full_index_name = index_keys[0] - old_mapping = index_dict[full_index_name]["mappings"] + old_mapping_body = index_dict[full_index_name]["mappings"] # need to initialise Index class to use the .put_mapping API wrapper method index_ = dsl.Index(full_index_name, using=self.client) - with open(mapping_path, "r") as body: - mapping = json.load(body)["mappings"] - changes = list(dictdiffer.diff(old_mapping, mapping)) + changes = list(dictdiffer.diff(old_mapping_body, mapping_body)) - # allow only additions to mappings (backwards compatibility is kept) - if not check or all([change[0] == "add" for change in changes]): - # raises 400 if the mapping cannot be updated - # (f.e. type changes or index needs to be closed) - index_.put_mapping(using=self.client, body=mapping) - else: - non_add_changes = [change for change in changes if change[0] != "add"] - raise NotAllowedMappingUpdate( - "Only additions are allowed when updating mappings to keep backwards compatibility. " - f"This mapping has {len(non_add_changes)} non addition changes.\n\n" - f"Full list of changes: {changes}" - ) + # allow only additions to mappings (backwards compatibility is kept) + if not check or all([change[0] == "add" for change in changes]): + # raises 400 if the mapping cannot be updated + # (f.e. type changes or index needs to be closed) + index_.put_mapping(using=self.client, body=mapping_body) + else: + non_add_changes = [change for change in changes if change[0] != "add"] + raise NotAllowedMappingUpdate( + "Only additions are allowed when updating mappings to keep backwards compatibility. " + f"This mapping has {len(non_add_changes)} non addition changes.\n\n" + f"Full list of changes: {changes}" + ) def _replace_prefix(self, template_path, body, enforce_prefix): """Replace index prefix in template request body.""" diff --git a/tests/conftest.py b/tests/conftest.py index f70b71b8..8187a8c2 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -38,6 +38,9 @@ def extra_entry_points(): "invenio_search.index_templates": [ "records = mock_module.index_templates", ], + "invenio_search.mappings": [ + "organizations = mock_module:mock_mapping", + ], } diff --git a/tests/mock_module/__init__.py b/tests/mock_module/__init__.py index 0513d8ef..590d77a8 100644 --- a/tests/mock_module/__init__.py +++ b/tests/mock_module/__init__.py @@ -7,3 +7,35 @@ # under the terms of the MIT License; see LICENSE file for more details. """Mock module used to test loading of ES resources.""" + +from invenio_search.api import dsl + + +class Address(dsl.InnerDoc): + """Address innder document.""" + + street = dsl.Text() + city = dsl.Text() + zip = dsl.Keyword() + + +class Organization(dsl.Document): + """Organization document.""" + + class Index: + """Index configuration.""" + + name = "organizations-organization-v1.0.0" + + title = dsl.Text() + acronym = dsl.Keyword() + address = dsl.Object(Address) + + +def mock_mapping(ext): + """Mock mapping.""" + index = Organization._index + mapping = index.to_dict() + + ext.mappings[index._name] = mapping + ext.aliases["organizations"] = {"organizations-organization": {index._name: None}} diff --git a/tests/test_cli.py b/tests/test_cli.py index 75561249..3db7126e 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -52,19 +52,20 @@ def test_init(app, template_entrypoints): "records-authorities-authority-v1.0.0", "records-bibliographic-bibliographic-v1.0.0", "records-default-v1.0.0", + "organizations-organization-v1.0.0", } - assert 3 == len(invenio_search.mappings) + assert 4 == len(invenio_search.mappings) with patch( "invenio_search.ext.iter_entry_points", return_value=template_entrypoints("invenio_search.templates"), ): assert len(invenio_search.templates.keys()) == 1 - assert "record-view-{}".format(_get_version()) in invenio_search.templates + assert f"record-view-{_get_version()}" in invenio_search.templates current_search_client.indices.delete_alias("_all", "_all", ignore=[400, 404]) current_search_client.indices.delete("*") - aliases = current_search_client.indices.get_alias() + aliases = current_search_client.indices.get_alias(expand_wildcards="open") assert 0 == len(aliases) runner = CliRunner() @@ -74,7 +75,7 @@ def test_init(app, template_entrypoints): result = runner.invoke(cmd, ["init", "--force"], obj=script_info) assert result.exit_code == 0 assert current_search_client.indices.exists_template( - "record-view-{}".format(_get_version()) + f"record-view-{_get_version()}", ) assert ( len( @@ -94,8 +95,8 @@ def test_init(app, template_entrypoints): "index_template_example" ) - aliases = current_search_client.indices.get_alias() - assert 8 == sum(len(idx.get("aliases", {})) for idx in aliases.values()) + aliases = current_search_client.indices.get_alias(expand_wildcards="open") + assert 11 == sum(len(idx.get("aliases", {})) for idx in aliases.values()) assert current_search_client.indices.exists(list(invenio_search.mappings.keys())) # Clean-up: @@ -105,7 +106,7 @@ def test_init(app, template_entrypoints): result = runner.invoke(cmd, ["destroy", "--yes-i-know"], obj=script_info) assert 0 == result.exit_code - aliases = current_search_client.indices.get_alias() + aliases = current_search_client.indices.get_alias(expand_wildcards="open") assert 0 == len(aliases) @@ -123,12 +124,19 @@ def test_list(app): result = runner.invoke(cmd, ["list", "--only-aliases"], obj=script_info) # Turn cli outputted str presentation of Python list into a list - assert set(ast.literal_eval(result.output)) == {"records", "authors"} + assert set(ast.literal_eval(result.output)) == { + "records", + "authors", + "organizations", + } result = runner.invoke(cmd, ["list"], obj=script_info) assert result.output == ( "├──authors\n" "│ └──authors-authors-v1.0.0\n" + "├──organizations\n" + "│ └──organizations-organization\n" + "│ └──organizations-organization-v1.0.0\n" "└──records *\n" " ├──records-authorities\n" " │ └──records-authorities-authority-v1.0.0\n" diff --git a/tests/test_invenio_search.py b/tests/test_invenio_search.py index 626331b1..b5f8566f 100644 --- a/tests/test_invenio_search.py +++ b/tests/test_invenio_search.py @@ -111,12 +111,16 @@ class ep(object): name = "records" module_name = "mock_module.mappings" - yield ep + def load(self): + return None - assert len(ext.mappings) == 0 + yield ep() + + # One mapping is already registered from the "mock_module:mock_mapping" entry point + assert len(ext.mappings) == 1 with patch("invenio_search.ext.iter_entry_points", mock_entry_points_mappings): ext.load_entry_point_group_mappings(entry_point_group_mappings=ep_group) - assert len(ext.mappings) == 3 + assert len(ext.mappings) == 4 with patch( "invenio_search.ext.iter_entry_points", @@ -161,7 +165,7 @@ def test_whitelisted_aliases(app, aliases_config, expected_aliases): current_search_client.indices.delete("*") list(current_search.create(ignore=None)) - aliases = current_search_client.indices.get_alias() + aliases = current_search_client.indices.get_alias(expand_wildcards="open") if expected_aliases == []: assert 0 == len(aliases) else: @@ -195,6 +199,10 @@ def test_creating_alias_existing_index( ): """Test creating new alias and index where there already exists one.""" search = app.extensions["invenio-search"] + # Clear mappings + search.mappings = {} + search.aliases = {} + search.register_mappings("authors", "mock_module.mappings") search._current_suffix = suffix current_search_client.indices.delete_alias("_all", "_all", ignore=[400, 404]) @@ -270,37 +278,36 @@ def _test_prefix_indices(app, prefix_value): search._current_suffix = suffix search.register_mappings("records", "mock_module.mappings") + auth_idx = {f"{prefix}records-authorities-authority-v1.0.0{suffix}"} + bib_idx = {f"{prefix}records-bibliographic-bibliographic-v1.0.0{suffix}"} + default_idx = {f"{prefix}records-default-v1.0.0{suffix}"} + org_idx = {f"{prefix}organizations-organization-v1.0.0{suffix}"} + record_indices = auth_idx | bib_idx | default_idx + all_indices = record_indices | org_idx + # clean-up in case something failed previously current_search_client.indices.delete("*") # create indices and test list(search.create()) - es_indices = current_search_client.indices.get_alias() - def _f(name): # formatting helper - return name.format(p=prefix, s=suffix) - - assert set(es_indices.keys()) == { - _f("{p}records-authorities-authority-v1.0.0{s}"), - _f("{p}records-bibliographic-bibliographic-v1.0.0{s}"), - _f("{p}records-default-v1.0.0{s}"), - } + es_indices = current_search_client.indices.get_alias(expand_wildcards="open") + assert set(es_indices.keys()) == all_indices # Build set of aliases es_aliases = defaultdict(set) for index, info in es_indices.items(): for alias in info.get("aliases", {}): es_aliases[alias].add(index) - auth_idx = {_f("{p}records-authorities-authority-v1.0.0{s}")} - bib_idx = {_f("{p}records-bibliographic-bibliographic-v1.0.0{s}")} - default_idx = {_f("{p}records-default-v1.0.0{s}")} - all_indices = auth_idx | bib_idx | default_idx assert es_aliases == { - _f("{p}records-authorities-authority-v1.0.0"): auth_idx, - _f("{p}records-bibliographic-bibliographic-v1.0.0"): bib_idx, - _f("{p}records-default-v1.0.0"): default_idx, - _f("{p}records-authorities"): auth_idx, - _f("{p}records-bibliographic"): bib_idx, - _f("{p}records"): all_indices, + f"{prefix}records-authorities-authority-v1.0.0": auth_idx, + f"{prefix}records-bibliographic-bibliographic-v1.0.0": bib_idx, + f"{prefix}records-default-v1.0.0": default_idx, + f"{prefix}records-authorities": auth_idx, + f"{prefix}records-bibliographic": bib_idx, + f"{prefix}records": record_indices, + f"{prefix}organizations-organization-v1.0.0": org_idx, + f"{prefix}organizations-organization": org_idx, + f"{prefix}organizations": org_idx, } # clean-up current_search_client.indices.delete("*")