Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 57 additions & 18 deletions MPCAutofill/cardpicker/search/search_functions.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import pycountry
from elasticsearch import Elasticsearch
from elasticsearch.exceptions import ConnectionError as ElasticConnectionError
from elasticsearch.exceptions import TransportError as ElasticTransportError
from elasticsearch_dsl.query import Bool, Match, Range, Terms

from django.conf import settings
Expand Down Expand Up @@ -88,18 +89,39 @@ def get_scaled_maximum_size(search_settings: SearchSettings) -> int:
return search_settings.filterSettings.maximumSize * 1_000_000


# fallback levels for the fuzzy retry-on-miss behaviour. level 0 is the standard match; levels 1 and 2 are
# progressively more forgiving and are only queried when every stricter level matched nothing.
FUZZY_FALLBACK_LEVELS = [1, 2]


def get_match(search_settings: SearchSettings, query_parsed: str, fallback_level: int = 0) -> Match:
if fallback_level == 1:
# forgive typos (up to elasticsearch's automatic edit distance), but still require every word to match
return Match(searchq_fuzzy={"query": query_parsed, "operator": "AND", "fuzziness": "AUTO"})
if fallback_level == 2:
# forgive typos and extra words. "2<75%" means queries of up to two words still require every word to
# match (75% of 2 rounds down to 1, which would match far too loosely); longer queries require 75%.
return Match(searchq_fuzzy={"query": query_parsed, "fuzziness": "AUTO", "minimum_should_match": "2<75%"})
if search_settings.searchTypeSettings.fuzzySearch:
return Match(searchq_fuzzy={"query": query_parsed, "operator": "AND"})
return Match(searchq_precise={"query": query_parsed, "operator": "AND"})


def get_search(
search_settings: SearchSettings,
query: str | None,
card_types: list[CardType],
expansion_code: str | None = None,
collector_number: str | None = None,
fallback_level: int = 0,
) -> CardSearch:
"""
This is the core search function for MPC Autofill - queries Elasticsearch for `self` given `search_settings`
and returns the list of corresponding `Card` identifiers.
Expects that the search index exists. Since this function is called many times, it makes sense to check this
once at the call site rather than in the body of this function.
`fallback_level` selects the match clause via `get_match` - level 0 is the standard search, and levels 1 and 2
(see `FUZZY_FALLBACK_LEVELS`) are progressively more forgiving retries for when stricter levels match nothing.
"""

# set up search - match the query and use the AND operator
Expand All @@ -124,11 +146,9 @@ def get_search(
)
if query:
query_parsed = to_searchable(query)
if search_settings.searchTypeSettings.fuzzySearch:
match = Match(searchq_fuzzy={"query": query_parsed, "operator": "AND"})
else:
match = Match(searchq_precise={"query": query_parsed, "operator": "AND"})
s = s.query(match)
s = s.query(
get_match(search_settings=search_settings, query_parsed=query_parsed, fallback_level=fallback_level)
)
if card_types:
s = s.filter(
Bool(
Expand Down Expand Up @@ -162,20 +182,37 @@ def retrieve_card_identifiers(
expansion_code: str | None = None,
collector_number: str | None = None,
) -> list[str]:
hits_iterable = (
get_search(
search_settings=search_settings,
query=query,
card_types=[card_type],
expansion_code=expansion_code,
collector_number=collector_number,
)
.sort({"priority": {"order": "desc"}})
.params(preserve_order=True)
.scan()
)
source_order = get_source_order(search_settings=search_settings)
return [result.identifier for result in sorted(hits_iterable, key=lambda result: source_order[result.source_pk])]

def execute(fallback_level: int) -> list[str]:
hits_iterable = (
get_search(
search_settings=search_settings,
query=query,
card_types=[card_type],
expansion_code=expansion_code,
collector_number=collector_number,
fallback_level=fallback_level,
)
.sort({"priority": {"order": "desc"}})
.params(preserve_order=True)
.scan()
)
return [
result.identifier for result in sorted(hits_iterable, key=lambda result: source_order[result.source_pk])
]

identifiers = execute(fallback_level=0)
if query:
for fallback_level in FUZZY_FALLBACK_LEVELS:
if identifiers:
break
try:
identifiers = execute(fallback_level=fallback_level)
except ElasticTransportError:
# the fallback is best-effort: surface the primary search's (empty) results rather than an error
break
return identifiers


def retrieve_cardback_identifiers(search_settings: SearchSettings) -> list[str]:
Expand Down Expand Up @@ -236,6 +273,8 @@ def get_new_cards_paginator(source: Source) -> Paginator[QuerySet[Card]]:
"get_elasticsearch_connection",
"ping_elasticsearch",
"elastic_connection",
"FUZZY_FALLBACK_LEVELS",
"get_match",
"get_search",
"retrieve_card_identifiers",
"retrieve_cardback_identifiers",
Expand Down
74 changes: 68 additions & 6 deletions MPCAutofill/cardpicker/tests/__snapshots__/test_views.ambr
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@
'CARD': dict({
'Brainstorm': dict({
'canonicalArtist': dict({
'name': 'Artist 102',
'name': 'Artist 0',
}),
'canonicalCard': dict({
'artist': None,
Expand Down Expand Up @@ -416,7 +416,7 @@
'cards': list([
dict({
'canonicalArtist': dict({
'name': 'Artist 107',
'name': 'Artist 0',
}),
'canonicalCard': dict({
'artist': None,
Expand Down Expand Up @@ -702,7 +702,7 @@
'cards': list([
dict({
'canonicalArtist': dict({
'name': 'Artist 109',
'name': 'Artist 0',
}),
'canonicalCard': dict({
'artist': None,
Expand Down Expand Up @@ -1783,6 +1783,17 @@
'status_code': 200,
})
# ---
# name: TestPostEditorSearchResults.test_fuzzy_fallback_respects_filters
dict({
'json': dict({
'results': dict({
'key1': list([
]),
}),
}),
'status_code': 200,
})
# ---
# name: TestPostEditorSearchResults.test_fuzzy_search
dict({
'json': dict({
Expand All @@ -1796,6 +1807,19 @@
'status_code': 200,
})
# ---
# name: TestPostEditorSearchResults.test_fuzzy_search_with_typo_falls_back
dict({
'json': dict({
'results': dict({
'key1': list([
'1UPdh7J7hScg4ZnxSPJ-EeBYHLp2s3Oz1',
'1dxSLHtw-VwwE09pZCA8OA6LbuWRZPEoU',
]),
}),
}),
'status_code': 200,
})
# ---
# name: TestPostEditorSearchResults.test_get_multiple_rows_filtered_includes_one_tag
dict({
'json': dict({
Expand Down Expand Up @@ -1965,6 +1989,31 @@
'status_code': 400,
})
# ---
# name: TestPostEditorSearchResults.test_precise_search_with_partial_name_falls_back
dict({
'json': dict({
'results': dict({
'key1': list([
'1UPdh7J7hScg4ZnxSPJ-EeBYHLp2s3Oz1',
'1dxSLHtw-VwwE09pZCA8OA6LbuWRZPEoU',
]),
}),
}),
'status_code': 200,
})
# ---
# name: TestPostEditorSearchResults.test_precise_search_with_typo_falls_back_to_fuzzy
dict({
'json': dict({
'results': dict({
'key1': list([
'1c4M-sK9gd0Xju0NXCPtqeTW_DQTldVU5',
]),
}),
}),
'status_code': 200,
})
# ---
# name: TestPostEditorSearchResults.test_priority_ordering_in_search_results
dict({
'json': dict({
Expand Down Expand Up @@ -2305,6 +2354,19 @@
'status_code': 200,
})
# ---
# name: TestPostEditorSearchResults.test_search_with_extra_words_falls_back
dict({
'json': dict({
'results': dict({
'key1': list([
'1UPdh7J7hScg4ZnxSPJ-EeBYHLp2s3Oz1',
'1dxSLHtw-VwwE09pZCA8OA6LbuWRZPEoU',
]),
}),
}),
'status_code': 200,
})
# ---
# name: TestPostExploreSearchResults.test_explore_search[filter to cardback + token]
dict({
'json': dict({
Expand Down Expand Up @@ -2399,7 +2461,7 @@
'cards': list([
dict({
'canonicalArtist': dict({
'name': 'Artist 67',
'name': 'Artist 0',
}),
'canonicalCard': dict({
'artist': None,
Expand Down Expand Up @@ -2673,7 +2735,7 @@
'cards': list([
dict({
'canonicalArtist': dict({
'name': 'Artist 66',
'name': 'Artist 0',
}),
'canonicalCard': dict({
'artist': None,
Expand Down Expand Up @@ -3025,7 +3087,7 @@
'cards': list([
dict({
'canonicalArtist': dict({
'name': 'Artist 68',
'name': 'Artist 0',
}),
'canonicalCard': dict({
'artist': None,
Expand Down
15 changes: 15 additions & 0 deletions MPCAutofill/cardpicker/tests/conftest.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import datetime as dt
import inspect
import uuid
from typing import Type

import factory as factory_boy
import pytest
from pytest_elasticsearch import factories
from testcontainers.elasticsearch import ElasticSearchContainer
Expand All @@ -12,6 +14,7 @@

from cardpicker.integrations.game.base import GameIntegration
from cardpicker.models import Card, CardTypes, DFCPair, Source, Tag
from cardpicker.tests import factories as factories_module
from cardpicker.tests.constants import Cards, DummyIntegration, Sources
from cardpicker.tests.factories import (
CanonicalCardFactory,
Expand All @@ -26,6 +29,18 @@
ELASTICSEARCH_PORT = 9300 # this is the default expected by `elasticsearch_nooproc`


@pytest.fixture(autouse=True)
def reset_factory_sequences() -> None:
"""
Snapshots embed sequence-generated values (e.g. "Artist 0"), so sequences must restart for every test -
otherwise adding or running a subset of tests shifts the numbering and breaks unrelated snapshots.
"""

for _, factory_class in inspect.getmembers(factories_module, inspect.isclass):
if issubclass(factory_class, factory_boy.django.DjangoModelFactory):
factory_class.reset_sequence(0)


@pytest.fixture(scope="session")
def postgres_container():
postgres = PostgresContainer("postgres:16.0-alpine").with_bind_ports(5432, POSTGRES_PORT)
Expand Down
2 changes: 2 additions & 0 deletions MPCAutofill/cardpicker/tests/test_integrations.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,8 @@ def test_get_meld_pairs(self):

@pytest.mark.parametrize("url", [item.value for item in Decks], ids=[item.name.lower() for item in Decks])
def test_valid_url(self, client, django_settings, snapshot, url: str):
if "moxfield" in url and not conf_settings.MOXFIELD_SECRET:
pytest.skip("MOXFIELD_SECRET is not configured (e.g. fork PRs and environments without credentials)")
decklist = MTGIntegration.query_import_site(url)
assert decklist
assert Counter(decklist.splitlines()) == snapshot
Expand Down
16 changes: 16 additions & 0 deletions MPCAutofill/cardpicker/tests/test_sources.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import datetime as dt
import json
from pathlib import Path

import freezegun
import pytest
Expand All @@ -21,6 +23,16 @@
DEFAULT_DATE = dt.datetime(2023, 1, 1)


def google_drive_credentials_available() -> bool:
# mirrors the path resolution in `cardpicker.sources.api`. fork PRs and local runs without
# credentials get a missing or unparseable client_secrets.json, so those tests must skip.
try:
with open(Path(__file__).parent.parent.parent / "client_secrets.json") as f:
return bool(json.load(f))
except (OSError, json.JSONDecodeError):
return False


class TestAPI:
# region constants

Expand Down Expand Up @@ -354,6 +366,10 @@ def test_unpack_name(
# endregion


@pytest.mark.skipif(
not google_drive_credentials_available(),
reason="Google Drive credentials (client_secrets.json) are not available",
)
class TestUpdateDatabase:
# region tests

Expand Down
Loading
Loading