From 4f1d09ee63cf52f82241e430b063bd0b1e3861b2 Mon Sep 17 00:00:00 2001 From: sarayourfriend <24264157+sarayourfriend@users.noreply.github.com> Date: Thu, 15 Jun 2023 14:31:32 +1000 Subject: [PATCH 1/2] Do not query filtered index if feature flag not enabled --- api/api/utils/search_context.py | 5 +++++ api/test/unit/utils/test_search_context.py | 15 +++++++++++++-- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/api/api/utils/search_context.py b/api/api/utils/search_context.py index 89d31c856f0..09cb1a7b766 100644 --- a/api/api/utils/search_context.py +++ b/api/api/utils/search_context.py @@ -1,6 +1,8 @@ from dataclasses import asdict, dataclass from typing import Self +from django.conf import settings + from elasticsearch_dsl import Q, Search from elasticsearch_dsl.response import Hit @@ -26,6 +28,9 @@ def build(cls, results: list[Hit], origin_index: OriginIndex) -> Self: all_result_identifiers = {r.identifier for r in results} + if not settings.ENABLE_FILTERED_INDEX_QUERIES: + return cls(all_result_identifiers, set()) + filtered_index_search = Search(index=f"{origin_index}-filtered") filtered_index_search = filtered_index_search.query( # Use `identifier` rather than the document `id` due to diff --git a/api/test/unit/utils/test_search_context.py b/api/test/unit/utils/test_search_context.py index 90cae250193..f3d584c9da6 100644 --- a/api/test/unit/utils/test_search_context.py +++ b/api/test/unit/utils/test_search_context.py @@ -17,7 +17,16 @@ def test_no_results(media_type_config): (True, False), ids=lambda x: "has_sensitive_text" if x else "no_sensitive_text", ) -def test_sensitive_text(media_type_config, has_sensitive_text): +@pytest.mark.parametrize( + "setting_enabled", + (True, False), + ids=lambda x: "setting_enabled" if x else "setting_disabled", +) +def test_sensitive_text( + media_type_config, has_sensitive_text, setting_enabled, settings +): + settings.ENABLE_FILTERED_INDEX_QUERIES = setting_enabled + clear_results = media_type_config.model_factory.create_batch( # Use size 10 to force result size beyond the default ES query window size=10, @@ -43,5 +52,7 @@ def test_sensitive_text(media_type_config, has_sensitive_text): assert search_context == SearchContext( {r.identifier for r in results}, - {maybe_sensitive_text_model.identifier} if has_sensitive_text else set(), + {maybe_sensitive_text_model.identifier} + if has_sensitive_text and setting_enabled + else set(), ) From 1cd632cc45af04fc95441f9f8d061ea5472edbf5 Mon Sep 17 00:00:00 2001 From: sarayourfriend <24264157+sarayourfriend@users.noreply.github.com> Date: Thu, 15 Jun 2023 14:49:14 +1000 Subject: [PATCH 2/2] Confirm no filtered index request is made at all --- api/test/unit/utils/test_search_context.py | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/api/test/unit/utils/test_search_context.py b/api/test/unit/utils/test_search_context.py index f3d584c9da6..a1c1489c3ba 100644 --- a/api/test/unit/utils/test_search_context.py +++ b/api/test/unit/utils/test_search_context.py @@ -1,3 +1,4 @@ +import pook import pytest from api.utils.search_context import SearchContext @@ -48,7 +49,23 @@ def test_sensitive_text( results = [maybe_sensitive_text_hit] + [hit for _, hit in clear_results] - search_context = SearchContext.build(results, media_type_config.origin_index) + if not setting_enabled: + es_host = settings.ES.transport.kwargs["host"] + es_port = settings.ES.transport.kwargs["port"] + + with pook.post( + f"http://{es_host}:{es_port}/{media_type_config.filtered_index}/_search", + reply=500, + ) as mock: + search_context = SearchContext.build( + results, media_type_config.origin_index + ) + assert ( + mock.total_matches == 0 + ), "There should be zero requests to ES if the settting is disabled" + pook.off() + else: + search_context = SearchContext.build(results, media_type_config.origin_index) assert search_context == SearchContext( {r.identifier for r in results},