diff --git a/Makefile b/Makefile index 256f79cd..bdec6eb9 100644 --- a/Makefile +++ b/Makefile @@ -114,7 +114,7 @@ test: _ensure_network test_api test_front test_api: @echo "🔎 Running API tests..." - ${DOCKER_COMPOSE_TEST} run --rm api pytest tests/ + ${DOCKER_COMPOSE_TEST} run --rm api pytest ${args} tests/ test_front: @echo "🔎 Running front-end tests..." diff --git a/README.md b/README.md index f6387cad..38f777a3 100644 --- a/README.md +++ b/README.md @@ -66,6 +66,16 @@ uvicorn app.api:app --reload --port=8001 --workers=4 ``` Note that it's important to use port 8001, as port 8000 will be used by the docker version of the search service. + + +To debug the backend app: +* stop API instance: `docker compose stop api` +* add a pdb.set_trace() at the point you want, +* then launch `docker compose run --rm --use-aliases api uvicorn app.api:app --proxy-headers --host 0.0.0.0 --port 8000 --reload`[^use_aliases] + +[^use_aliases]: the `--use-aliases` make it so that this container is reachable as "api" for the other containers in the compose + + ### Pre-Commit This repo uses [pre-commit](https://pre-commit.com/) to enforce code styling, etc. To use it: diff --git a/app/_types.py b/app/_types.py index 54ad45b5..af0a6c50 100644 --- a/app/_types.py +++ b/app/_types.py @@ -7,6 +7,29 @@ JSONType = dict[str, Any] +class FacetItem(BaseModel): + """Describes an entry of a facet""" + + key: str + name: str + count: int + """The number of elements for this value""" + + +class FacetInfo(BaseModel): + """Search result for a facet""" + + name: str + """The display name of the facet""" + items: list[FacetItem] + """Items in the facets""" + count_error_margin: int | None = None + + +FacetsInfos = dict[str, FacetInfo] +"""Information about facets for a search result""" + + class SearchResponseDebug(BaseModel): query: JSONType @@ -26,7 +49,8 @@ def is_success(self): class SuccessSearchResponse(BaseModel): hits: list[JSONType] - aggregations: JSONType + aggregations: JSONType | None = None + facets: FacetsInfos | None = None page: int page_size: int page_count: int diff --git a/app/api.py b/app/api.py index 8e3375e4..d3879ae2 100644 --- a/app/api.py +++ b/app/api.py @@ -8,44 +8,18 @@ from fastapi.responses import HTMLResponse, PlainTextResponse from fastapi.templating import Jinja2Templates +import app.search as app_search from app import config from app._types import SearchResponse, SuccessSearchResponse from app.config import check_config_is_defined, settings -from app.postprocessing import ( - BaseResultProcessor, - load_result_processor, - process_taxonomy_completion_response, -) -from app.query import ( - build_completion_query, - build_elasticsearch_query_builder, - build_search_query, - execute_query, -) +from app.facets import check_all_facets_fields_are_agg +from app.postprocessing import process_taxonomy_completion_response +from app.query import build_completion_query from app.utils import connection, get_logger, init_sentry logger = get_logger() -if config.CONFIG is None: - # We want to be able to import api.py (for tests for example) without - # failure, but we add a warning message as it's not expected in a - # production settings - logger.warning("Main configuration is not set, use CONFIG_PATH envvar") - FILTER_QUERY_BUILDERS = {} - RESULT_PROCESSORS = {} -else: - # we cache query builder and result processor here for faster processing - FILTER_QUERY_BUILDERS = { - index_id: build_elasticsearch_query_builder(index_config) - for index_id, index_config in config.CONFIG.indices.items() - } - RESULT_PROCESSORS = { - index_id: load_result_processor(index_config) - for index_id, index_config in config.CONFIG.indices.items() - } - - app = FastAPI( title="search-a-licious API", contact={ @@ -116,6 +90,13 @@ def get_document( return product +def check_facets_are_valid(index_id: str | None, facets: list[str] | None) -> None: + """Check that the facets are valid.""" + errors = check_all_facets_fields_are_agg(index_id, facets) + if errors: + raise HTTPException(status_code=400, detail=json.dumps(errors)) + + @app.get("/search") def search( q: Annotated[ @@ -130,7 +111,7 @@ def search( filter clause for categories and brands and look for "strawberry" in multiple fields. - The query is optional, but `sort_by` value must then be provided.""" +The query is optional, but `sort_by` value must then be provided.""" ), ] = None, langs: Annotated[ @@ -163,59 +144,44 @@ def search( and be sortable. If it is not provided, results are sorted by descending relevance score.""" ), ] = None, + facets: Annotated[ + str | None, + Query( + description="""Name of facets to return in the response as a comma-separated value. + If None (default) no facets are returned.""" + ), + ] = None, index_id: Annotated[ str | None, INDEX_ID_QUERY_PARAM, ] = None, ) -> SearchResponse: + # check and preprocess parameters check_config_is_defined() global_config = cast(config.Config, config.CONFIG) check_index_id_is_defined(index_id, global_config) - index_id, index_config = global_config.get_index_config(index_id) - result_processor = cast(BaseResultProcessor, RESULT_PROCESSORS[index_id]) + facets_list = facets.split(",") if facets else None + check_facets_are_valid(index_id, facets_list) if q is None and sort_by is None: raise HTTPException( status_code=400, detail="`sort_by` must be provided when `q` is missing" ) - - langs_set = set(langs.split(",") if langs else ["en"]) - logger.debug( - "Received search query: q='%s', langs='%s', page=%d, " - "page_size=%d, fields='%s', sort_by='%s'", - q, - langs_set, - page, - page_size, - fields, - sort_by, - ) - if page * page_size > 10_000: raise HTTPException( status_code=400, detail=f"Maximum number of returned results is 10 000 (here: page * page_size = {page * page_size})", ) - - query = build_search_query( + langs_list = langs.split(",") if langs else ["en"] + # search + return app_search.search( q=q, - langs=langs_set, - size=page_size, + langs=langs_list, + page_size=page_size, page=page, - config=index_config, + fields=fields.split(",") if fields else None, sort_by=sort_by, - # filter query builder is generated from elasticsearch mapping and - # takes ~40ms to generate, build-it before hand to avoid this delay - filter_query_builder=FILTER_QUERY_BUILDERS[index_id], - ) - logger.debug("Elasticsearch query: %s", query.to_dict()) - - projection = set(fields.split(",")) if fields else None - return execute_query( - query, - result_processor, - page=page, - page_size=page_size, - projection=projection, + facets=facets_list, + index_id=index_id, ) diff --git a/app/config.py b/app/config.py index f98a490e..cb55edeb 100644 --- a/app/config.py +++ b/app/config.py @@ -1,4 +1,5 @@ import json +import logging from enum import StrEnum, auto from pathlib import Path from typing import Annotated @@ -8,6 +9,8 @@ from pydantic.json_schema import GenerateJsonSchema from pydantic_settings import BaseSettings +log = logging.getLogger(__name__) + class LoggingLevel(StrEnum): NOTSET = "NOTSET" @@ -232,6 +235,12 @@ class ESIndexConfig(BaseModel): class TaxonomyIndexConfig(BaseModel): + """We have an index storing multiple taxonomies + + It enables functions like auto-completion, or field suggestions + as well as enrichment of requests with synonyms + """ + name: Annotated[ str, Field(description="name of the taxonomy index alias to use"), @@ -245,6 +254,12 @@ class TaxonomyIndexConfig(BaseModel): class TaxonomyConfig(BaseModel): + """Configuration of taxonomies, + that is collections of entries with synonyms in multiple languages + + Field may be linked to taxonomies. + """ + sources: Annotated[ list[TaxonomySourceConfig], Field(description="configurations of used taxonomies"), @@ -256,7 +271,7 @@ class TaxonomyConfig(BaseModel): "to be always exported during indexing. During indexing, we use the taxonomy " "to translate every taxonomized field in a language-specific subfield. The list " "of language depends on the value defined here and on the optional " - "`taxonomy_langs` field that can be defined in each document." + "`taxonomy_langs` field that can be defined in each document.", ), ] index: Annotated[ @@ -268,6 +283,11 @@ class TaxonomyConfig(BaseModel): class IndexConfig(BaseModel): + """Inside the config file we can have several indexes defined. + + This object gives configuration for one index. + """ + index: Annotated[ ESIndexConfig, Field(description="configuration of the Elasticsearch index") ] @@ -413,8 +433,19 @@ def get_taxonomy_langs(self) -> set[str]: # langs will be stored in a unique `other` subfield return (set(self.taxonomy.exported_langs)) & set(ANALYZER_LANG_MAPPING) + def get_fields_with_bucket_agg(self): + return [ + field_name for field_name, field in self.fields.items() if field.bucket_agg + ] + class Config(BaseModel): + """This is the global config object that reflects + the yaml configuration file. + + Validations will be performed while we load it. + """ + indices: dict[str, IndexConfig] = Field( description="configuration of indices. " "The key is the ID of the index that can be referenced at query time. " diff --git a/app/facets.py b/app/facets.py new file mode 100644 index 00000000..68bb679c --- /dev/null +++ b/app/facets.py @@ -0,0 +1,99 @@ +""" +A module to help building facets from aggregations +""" + +from . import config +from ._types import FacetInfo, FacetItem, FacetsInfos, SearchResponse + + +def safe_get_index_config( + index: str | None = None, configuration: config.Config | None = None +) -> config.IndexConfig | None: + """Safely get config""" + if configuration is None: + configuration = config.CONFIG + if configuration is None: + return None + _, index_config = configuration.get_index_config(index) + return index_config + + +def check_all_facets_fields_are_agg( + index_id: str | None, facets: list[str] | None +) -> list[str]: + """Check all facets are valid, + that is, correspond to a field with aggregation""" + errors: list[str] = [] + if facets is None: + return errors + config = safe_get_index_config(index_id) + if config is None: + raise ValueError(f"Cannot get index config for index_id {index_id}") + for field_name in facets: + if field_name not in config.fields: + errors.append(f"Unknown field name in facets: {field_name}") + elif not config.fields[field_name].bucket_agg: + errors.append(f"Non aggregation field name in facets: {field_name}") + return errors + + +def build_facets( + search_result: SearchResponse, + index_config: config.IndexConfig, + facets_names: list[str] | None, +) -> FacetsInfos: + """Given a search result with aggregations, + build a list of facets for API response + """ + aggregations = search_result.aggregations + if facets_names is None or aggregations is None: + return {} + facets: FacetsInfos = {} + for field_name in facets_names: + if field_name not in aggregations: + pass + agg_data = aggregations[field_name] + agg_buckets = agg_data.get("buckets", []) + # best values + facet_items = [ + FacetItem( + key=bucket["key"], + # TODO: compute name in target language if there is a taxonomy + name=bucket["key"], + count=bucket["doc_count"], + # TODO: add if selected + ) + for bucket in agg_buckets + ] + # add other values + if agg_data["sum_other_doc_count"]: + facet_items.append( + FacetItem( + key="--other--", + # TODO: translate in target language ? + name="Other", + count=agg_data["sum_other_doc_count"], + ) + ) + items_count = sum(item.count for item in facet_items) + # and no values + if (not search_result.is_count_exact) and search_result.count > items_count: + facet_items.append( + FacetItem( + key="--none--", + # TODO: translate in target language ? + name="None", + # Note: this depends on search_result.is_count_exact, + # but we leave it to user to verify + count=search_result.count - items_count, + ) + ) + # append our facet information + facets[field_name] = FacetInfo( + # FIXME translate + name=field_name, + items=facet_items, + count_error_margin=agg_data["doc_count_error_upper_bound"], + ) + + return facets diff --git a/app/query.py b/app/query.py index b1512910..721b593a 100644 --- a/app/query.py +++ b/app/query.py @@ -196,14 +196,20 @@ def parse_sort_by_parameter(sort_by: str | None, config: IndexConfig) -> str | N return sort_by -def create_aggregation_clauses(config: IndexConfig) -> dict[str, Agg]: - """Create term bucket aggregation clauses for all relevant fields as - defined in the config. +def create_aggregation_clauses( + config: IndexConfig, facets: list[str] | None +) -> dict[str, Agg]: + """Create term bucket aggregation clauses + for all fields corresponding to facets, + as defined in the config """ clauses = {} - for field in config.fields.values(): - if field.bucket_agg: - clauses[field.name] = A("terms", field=field.name) + if facets is not None: + for field_name in facets: + field = config.fields[field_name] + if field.bucket_agg: + # TODO - aggregation might depend on agg type or field type + clauses[field.name] = A("terms", field=field.name) return clauses @@ -214,6 +220,7 @@ def build_search_query( page: int, config: IndexConfig, filter_query_builder: ElasticsearchQueryBuilder, + facets: list[str] | None = None, sort_by: str | None = None, ) -> Search: """Build an elasticsearch_dsl Query. @@ -247,7 +254,7 @@ def build_search_query( if filter_query: query = query.query("bool", filter=filter_query) - for agg_name, agg in create_aggregation_clauses(config).items(): + for agg_name, agg in create_aggregation_clauses(config, facets).items(): query.aggs.bucket(agg_name, agg) sort_by = parse_sort_by_parameter(sort_by, config) diff --git a/app/search.py b/app/search.py new file mode 100644 index 00000000..143eebbd --- /dev/null +++ b/app/search.py @@ -0,0 +1,86 @@ +import logging +from typing import cast + +from app import config +from app._types import SearchResponse +from app.facets import build_facets +from app.postprocessing import BaseResultProcessor, load_result_processor +from app.query import ( + build_elasticsearch_query_builder, + build_search_query, + execute_query, +) + +logger = logging.getLogger(__name__) + +if config.CONFIG is None: + # We want to be able to import api.py (for tests for example) without + # failure, but we add a warning message as it's not expected in a + # production settings + logger.warning("Main configuration is not set, use CONFIG_PATH envvar") + FILTER_QUERY_BUILDERS = {} + RESULT_PROCESSORS = {} +else: + # we cache query builder and result processor here for faster processing + FILTER_QUERY_BUILDERS = { + index_id: build_elasticsearch_query_builder(index_config) + for index_id, index_config in config.CONFIG.indices.items() + } + RESULT_PROCESSORS = { + index_id: load_result_processor(index_config) + for index_id, index_config in config.CONFIG.indices.items() + } + + +def search( + index_id: str | None, + q: str | None, + sort_by: str | None, + page: int, + page_size: int, + fields: list[str] | None, + langs: list[str], + facets: list[str] | None, +) -> SearchResponse: + """Run a search""" + global_config = cast(config.Config, config.CONFIG) + index_id, index_config = global_config.get_index_config(index_id) + result_processor = cast(BaseResultProcessor, RESULT_PROCESSORS[index_id]) + langs_set = set(langs) + logger.debug( + "Received search query: q='%s', langs='%s', page=%d, " + "page_size=%d, fields='%s', sort_by='%s'", + q, + langs_set, + page, + page_size, + fields, + sort_by, + ) + + query = build_search_query( + q=q, + langs=langs_set, + size=page_size, + page=page, + config=index_config, + sort_by=sort_by, + # filter query builder is generated from elasticsearch mapping and + # takes ~40ms to generate, build-it before hand to avoid this delay + filter_query_builder=FILTER_QUERY_BUILDERS[index_id], + facets=facets, + ) + logger.debug("Elasticsearch query: %s", query.to_dict()) + + projection = set(fields) if fields else None + search_result = execute_query( + query, + result_processor, + page=page, + page_size=page_size, + projection=projection, + ) + search_result.facets = build_facets(search_result, index_config, facets) + # remove aggregations to avoid sending too much information + search_result.aggregations = None + return search_result diff --git a/data/config/openfoodfacts.yml b/data/config/openfoodfacts.yml index a7eeccb9..41cc8f25 100644 --- a/data/config/openfoodfacts.yml +++ b/data/config/openfoodfacts.yml @@ -123,6 +123,16 @@ indices: type: integer completeness: type: float + facets: + # include all fields + default: + order: + - categories_tags + - brands_tags + - labels_tags + - ecoscore_grade + - nova_groups + - ecoscore_grade document_denylist: - '8901552007122' lang_separator: _ diff --git a/frontend/package.json b/frontend/package.json index b34a41f7..82197acc 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -41,9 +41,7 @@ "author": "Open Food Facts", "license": "BSD-3-Clause", "dependencies": { - "@types/lodash-es": "^4.17.0", - "lit": "^3.0.0", - "lodash-es": "^4.17.21" + "lit": "^3.0.0" }, "devDependencies": { "@custom-elements-manifest/analyzer": "^0.6.3", diff --git a/frontend/public/off.html b/frontend/public/off.html index ba2f88c6..430fa731 100644 --- a/frontend/public/off.html +++ b/frontend/public/off.html @@ -251,7 +251,12 @@
- **TODO** Facets + + + + + +
@@ -274,6 +279,7 @@
@@ -283,10 +289,12 @@
diff --git a/frontend/src/event-listener-setup.ts b/frontend/src/event-listener-setup.ts index 0066a3dd..5a160f4d 100644 --- a/frontend/src/event-listener-setup.ts +++ b/frontend/src/event-listener-setup.ts @@ -6,7 +6,7 @@ // eslint-disable-next-line @typescript-eslint/no-explicit-any type Constructor = new (...args: any[]) => T; -export declare class EventRegistrationInterface { +export interface EventRegistrationInterface { /** * Calls window.addEventListener if not remove before next AnimationFrame * @param event - event name diff --git a/frontend/src/events.ts b/frontend/src/events.ts index 6e90f8bb..0cc7a208 100644 --- a/frontend/src/events.ts +++ b/frontend/src/events.ts @@ -14,6 +14,7 @@ export interface SearchResultDetail extends BaseSearchDetail { pageCount: number; pageSize: number; currentPage: number; + facets: Object; // FIXME: we could be more precise } /** diff --git a/frontend/src/search-a-licious.ts b/frontend/src/search-a-licious.ts index 5812a026..bc9cdc46 100644 --- a/frontend/src/search-a-licious.ts +++ b/frontend/src/search-a-licious.ts @@ -1,4 +1,5 @@ export {SearchaliciousBar} from './search-bar'; export {SearchaliciousButton} from './search-button'; export {SearchaliciousPages} from './search-pages'; +export {SearchaliciousFacets} from './search-facets'; export {SearchaliciousResults} from './search-results'; diff --git a/frontend/src/search-ctl.ts b/frontend/src/search-ctl.ts index 57f617cf..34488e0a 100644 --- a/frontend/src/search-ctl.ts +++ b/frontend/src/search-ctl.ts @@ -1,6 +1,9 @@ import {LitElement} from 'lit'; import {property, state} from 'lit/decorators.js'; -import {EventRegistrationMixin} from './event-listener-setup'; +import { + EventRegistrationInterface, + EventRegistrationMixin, +} from './event-listener-setup'; import {SearchaliciousEvents} from './enums'; import { ChangePageEvent, @@ -8,11 +11,13 @@ import { SearchResultEvent, SearchResultDetail, } from './events'; +import {SearchaliciousFacets} from './search-facets'; // eslint-disable-next-line @typescript-eslint/no-explicit-any type Constructor = new (...args: any[]) => T; -export declare class SearchaliciousSearchInterface { +export interface SearchaliciousSearchInterface + extends EventRegistrationInterface { query: string; name: string; baseUrl: string; @@ -92,10 +97,54 @@ export const SearchaliciousSearchMixin = >( @state() _count?: number; - // TODO: this should be on results element instead + /** + * @returns all searchalicious-facets elements linked to this search ctl + */ + _facetsNodes(): SearchaliciousFacets[] { + const allNodes: SearchaliciousFacets[] = []; + // search facets elements, we can't filter on search-name because of default value… + const facetsElements = document.querySelectorAll('searchalicious-facets'); + facetsElements.forEach((item) => { + const facetElement = item as SearchaliciousFacets; + if (facetElement.searchName == this.name) { + allNodes.push(facetElement); + } + }); + return allNodes; + } + + /** + * Get the list of facets we want to request + */ + _facets(): string[] { + const names = this._facetsNodes() + .map((facets) => facets.getFacetsNames()) + .flat(); + return [...new Set(names)]; + } + + /** + * Get the filter linked to facets + * @returns an expression to be added to query + */ + _facetsFilters(): string { + const allFilters: string[] = this._facetsNodes() + .map((facets) => facets.getSearchFilters()) + .flat(); + return allFilters.join(' AND '); + } + _searchUrl(page?: number) { // remove trailing slash const baseUrl = this.baseUrl.replace(/\/+$/, ''); + const queryParts = []; + if (this.query) { + queryParts.push(this.query); + } + const facetsFilters = this._facetsFilters(); + if (facetsFilters) { + queryParts.push(facetsFilters); + } // build parameters const params: { q: string; @@ -103,8 +152,9 @@ export const SearchaliciousSearchMixin = >( page_size: string; page?: string; index?: string; + facets?: string; } = { - q: this.query, + q: queryParts.join(' '), langs: this.langs, page_size: this.pageSize.toString(), index: this.index, @@ -112,6 +162,10 @@ export const SearchaliciousSearchMixin = >( if (page) { params.page = page.toString(); } + const facets = this._facets(); + if (facets && facets.length > 0) { + params.facets = facets.join(','); + } const queryStr = Object.entries(params) .filter( ([_, value]) => value != null // null or undefined @@ -172,11 +226,13 @@ export const SearchaliciousSearchMixin = >( this.search(detail.page); } } + /** * Launching search */ async search(page?: number) { const response = await fetch(this._searchUrl(page)); + // FIXME data should be typed… const data = await response.json(); this._results = data.hits; this._count = data.count; @@ -191,6 +247,7 @@ export const SearchaliciousSearchMixin = >( pageCount: this._pageCount!, currentPage: this._currentPage!, pageSize: this.pageSize, + facets: data.facets, }; this.dispatchEvent( new CustomEvent(SearchaliciousEvents.NEW_RESULT, { diff --git a/frontend/src/search-facets.ts b/frontend/src/search-facets.ts new file mode 100644 index 00000000..33421f39 --- /dev/null +++ b/frontend/src/search-facets.ts @@ -0,0 +1,213 @@ +import {LitElement, html, css} from 'lit'; +import {customElement, property, queryAssignedNodes} from 'lit/decorators.js'; +import {repeat} from 'lit/directives/repeat.js'; + +import {SearchaliciousResultCtlMixin} from './search-results-ctl'; +import {SearchResultEvent} from './events'; + +interface FacetsInfos { + [key: string]: FacetInfo; +} + +interface FacetInfo { + name: string; + // TODO: add other types if needed + items: FacetItem[]; +} + +interface FacetItem { + key: string; + name: string; +} + +interface FacetTerm extends FacetItem { + count: number; +} + +interface PresenceInfo { + [index: string]: boolean; +} + +function stringGuard(s: string | undefined): s is string { + return s != null; +} + +/** + * Parent Component to display a side search filter (aka facets). + * + * It must contains a SearchaliciousFacet component for each facet we want to display. + */ +@customElement('searchalicious-facets') +export class SearchaliciousFacets extends SearchaliciousResultCtlMixin( + LitElement +) { + // the last search facets + @property({attribute: false}) + // eslint-disable-next-line @typescript-eslint/no-explicit-any + facets?: FacetsInfos; + + @queryAssignedNodes({flatten: true}) + slotNodes!: Array; + + _facetNodes(): SearchaliciousFacet[] { + return this.slotNodes.filter( + (node) => node instanceof SearchaliciousFacet + ) as SearchaliciousFacet[]; + } + + /** + * Names of facets we need to query, + * this is the names of contained facetNodes. + * + * This is used by search-ctl to build the query + */ + getFacetsNames(): string[] { + return this._facetNodes().map((node) => node.name); + } + + getSearchFilters(): string[] { + return this._facetNodes() + .map((node) => node.searchFilter()) + .filter(stringGuard); + } + + /** + * As a result is received, we dispatch facets values to facetNodes + * @param event search result event + */ + override handleResults(event: SearchResultEvent) { + this.facets = event.detail.facets as FacetsInfos; + if (this.facets) { + // dispatch to children + this._facetNodes().forEach((node) => { + node.infos = this.facets![node.name]; + }); + } + } + + override render() { + // we always want to render slot, baceauso we use queryAssignedNodes + // but we may not want to display them + const display = this.facets ? '' : 'display: none'; + return html`
`; + } +} + +/** + * Base Component to display a side search filter (aka facets) + * + * This is a base class, implementations are specific based on facet type + */ +export class SearchaliciousFacet extends LitElement { + /** + * The name of the facet we display + */ + @property() + name = ''; + + // the last search infor for my facet + @property({attribute: false}) + // eslint-disable-next-line @typescript-eslint/no-explicit-any + infos?: FacetInfo; + + renderFacet() { + throw new Error('renderFacet not implemented: implement in sub class'); + } + + searchFilter(): string | undefined { + throw new Error('renderFacet not implemented: implement in sub class'); + } + + override render() { + if (this.infos) { + return this.renderFacet(); + } else { + return html``; + } + } +} + +/** + * This is a "terms" facet, this must be within a searchalicious-facets element + */ +@customElement('searchalicious-facet-terms') +export class SearchaliciousTermsFacet extends SearchaliciousFacet { + static override styles = css` + .term-wrapper { + display: block; + } + `; + + @property({attribute: false}) + selectedTerms: PresenceInfo = {}; + + /** + * Set wether a term is selected or not + */ + setTermSelected(e: Event) { + const element = e.target as HTMLInputElement; + const name = element.name; + if (element.checked) { + this.selectedTerms[name] = true; + } else { + delete this.selectedTerms[name]; + } + } + + /** + * Create the search term based upon the selected terms + */ + override searchFilter(): string | undefined { + const values = Object.keys(this.selectedTerms); + if (values.length === 0) { + return undefined; + } + let orValues = values.join(' OR '); + if (values.length > 1) { + orValues = `(${orValues})`; + } + return `${this.name}:${orValues}`; + } + + /** + * Renders a single term + */ + renderTerm(term: FacetTerm) { + return html` +
+ +
+ `; + } + + /** + * Renders the facet content + */ + override renderFacet() { + return html` +
+ + ${this.name} + ${repeat( + (this.infos!.items || []) as FacetTerm[], + (item: FacetTerm) => `${item.key}-${item.count}`, + (item: FacetTerm) => this.renderTerm(item) + )} +
+ `; + } +} + +declare global { + interface HTMLElementTagNameMap { + 'searchalicious-facets': SearchaliciousFacets; + 'searchalicious-facet-terms': SearchaliciousTermsFacet; + } +} diff --git a/frontend/src/search-pages.ts b/frontend/src/search-pages.ts index 1e15182b..3eb38b43 100644 --- a/frontend/src/search-pages.ts +++ b/frontend/src/search-pages.ts @@ -1,17 +1,26 @@ import {LitElement, html, css} from 'lit'; import {customElement, property, state} from 'lit/decorators.js'; import {repeat} from 'lit/directives/repeat.js'; -import range from 'lodash-es/range'; -import {EventRegistrationMixin} from './event-listener-setup'; import {SearchaliciousEvents} from './enums'; import {SearchResultEvent} from './events'; +import {SearchaliciousResultCtlMixin} from './search-results-ctl'; + +// small utility to have a range +// inspired from https://stackoverflow.com/a/44957114/2886726 +function _range(start: number, end: number): number[] { + return Array(end - start) + .fill(1) + .map((_, idx) => start + idx); +} /** * A component to display pagination for search results. */ @customElement('searchalicious-pages') -export class SearchaliciousPages extends EventRegistrationMixin(LitElement) { +export class SearchaliciousPages extends SearchaliciousResultCtlMixin( + LitElement +) { static override styles = css` ul { list-style-type: none; @@ -25,13 +34,6 @@ export class SearchaliciousPages extends EventRegistrationMixin(LitElement) { } `; - /** - * the search we display result for, - * this corresponds to `name` attribute of corresponding search-bar - */ - @property({attribute: 'search-name'}) - searchName = 'searchalicious'; - /** * Wether or not we should add the jump to first shortcuts */ @@ -50,12 +52,6 @@ export class SearchaliciousPages extends EventRegistrationMixin(LitElement) { @property({attribute: 'displayed-pages', type: Number}) displayedPages = 5; - /** - * Do we have an already launched search - */ - @state() - searchLaunched = false; - /** * Number of pages in current search */ @@ -97,18 +93,16 @@ export class SearchaliciousPages extends EventRegistrationMixin(LitElement) { /** * Update state on search received */ - _updatePages(event: Event) { - const detail = (event as SearchResultEvent).detail; - if (detail.searchName === this.searchName) { - this.searchLaunched = true; - this._pageCount = detail.pageCount; - this._currentPage = detail.currentPage; - [this._startRange, this._endRange] = this._computeRange( - this._currentPage, - this._pageCount - ); - this.requestUpdate(); - } + override handleResults(event: SearchResultEvent) { + const detail = event.detail; + this.searchLaunched = true; + this._pageCount = detail.pageCount; + this._currentPage = detail.currentPage; + [this._startRange, this._endRange] = this._computeRange( + this._currentPage, + this._pageCount + ); + this.requestUpdate(); } /** @@ -153,7 +147,7 @@ export class SearchaliciousPages extends EventRegistrationMixin(LitElement) { if (!this._startRange || !this._endRange) { return []; } - return range(this._startRange, this._endRange + 1); + return _range(this._startRange, this._endRange + 1); } override render() { @@ -253,23 +247,6 @@ export class SearchaliciousPages extends EventRegistrationMixin(LitElement) { ); } } - - /** - * Connect search event handlers. - */ - override connectedCallback() { - super.connectedCallback(); - this.addEventHandler(SearchaliciousEvents.NEW_RESULT, (event) => - this._updatePages(event) - ); - } - // connect to our specific events - override disconnectedCallback() { - super.disconnectedCallback(); - this.removeEventHandler(SearchaliciousEvents.NEW_RESULT, (event) => - this._updatePages(event) - ); - } } declare global { diff --git a/frontend/src/search-results-ctl.ts b/frontend/src/search-results-ctl.ts new file mode 100644 index 00000000..3ff217d2 --- /dev/null +++ b/frontend/src/search-results-ctl.ts @@ -0,0 +1,91 @@ +import {LitElement} from 'lit'; +import {property, state} from 'lit/decorators.js'; + +import { + EventRegistrationInterface, + EventRegistrationMixin, +} from './event-listener-setup'; +import {SearchaliciousEvents} from './enums'; +import {SearchResultEvent} from './events'; + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +type Constructor = new (...args: any[]) => T; + +export interface SearchaliciousResultsCtlInterface + extends EventRegistrationInterface { + searchName: string; + searchLaunched: Boolean; + + // sub class must override this one to dislpay results + handleResults(event: SearchResultEvent): void; + // this is the registered handler + _handleResults(event: Event): void; +} + +/** + * Common Mixin for elements that react upon a search result + * It can be to display results, facets or pagination. + */ +export const SearchaliciousResultCtlMixin = >( + superClass: T +) => { + /** + * The search mixin, encapsulate the logic of dialog with server + */ + class SearchaliciousResultCtlMixinClass extends EventRegistrationMixin( + superClass + ) { + /** + * the search we display result for, + * this corresponds to `name` attribute of corresponding search-bar + */ + @property({attribute: 'search-name'}) + searchName = 'searchalicious'; + + /** + * this will be true if we already launched a search + */ + @state() + searchLaunched = false; + + handleResults(_: SearchResultEvent) { + throw Error( + 'You must provide a handleResults function in you implementation' + ); + } + + /** + * event handler for NEW_RESULT events + */ + _handleResults(event: Event) { + // check if event is for our search + const resultEvent = event as SearchResultEvent; + const detail = resultEvent.detail; + if (detail.searchName === this.searchName) { + this.searchLaunched = true; + // call the real method + this.handleResults(resultEvent); + } + } + + /** + * Connect search event handlers. + */ + override connectedCallback() { + super.connectedCallback(); + this.addEventHandler(SearchaliciousEvents.NEW_RESULT, (event) => + this._handleResults(event) + ); + } + // connect to our specific events + override disconnectedCallback() { + super.disconnectedCallback(); + this.removeEventHandler(SearchaliciousEvents.NEW_RESULT, (event) => + this._handleResults(event) + ); + } + } + + return SearchaliciousResultCtlMixinClass as Constructor & + T; +}; diff --git a/frontend/src/search-results.ts b/frontend/src/search-results.ts index 48959aa0..79e09c71 100644 --- a/frontend/src/search-results.ts +++ b/frontend/src/search-results.ts @@ -2,9 +2,8 @@ import {LitElement, html} from 'lit'; import {customElement, property, state} from 'lit/decorators.js'; import {repeat} from 'lit/directives/repeat.js'; -import {EventRegistrationMixin} from './event-listener-setup'; +import {SearchaliciousResultCtlMixin} from './search-results-ctl'; import {SearchResultEvent} from './events'; -import {SearchaliciousEvents} from './enums'; import { MissingResultTemplateError, MultipleResultTemplateError, @@ -22,18 +21,9 @@ type htmlType = typeof html; * It reacts to the `searchalicious-result` event fired by the search controller. */ @customElement('searchalicious-results') -export class SearchaliciousResults extends EventRegistrationMixin(LitElement) { - /** - * the search we display result for, - * this corresponds to `name` attribute of corresponding search-bar - */ - @property({attribute: 'search-name'}) - searchName = 'searchalicious'; - - // did we already launched a search - @state() - searchLaunched = false; - +export class SearchaliciousResults extends SearchaliciousResultCtlMixin( + LitElement +) { // the last search results @state() // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -143,18 +133,12 @@ export class SearchaliciousResults extends EventRegistrationMixin(LitElement) { /** * event handler for NEW_RESULT events */ - _displayResults(event: Event) { - // check if event is for our search - const detail = (event as SearchResultEvent).detail; - if (detail.searchName === this.searchName) { - this.searchLaunched = true; - this.results = detail.results; // it's reactive, should trigger rendering - } + override handleResults(event: SearchResultEvent) { + this.results = event.detail.results; // it's reactive, should trigger rendering } /** * Create our resultRenderer, using last opportunity to read innerHTML before shadowRoot creation. - * Also connect search event handlers. */ override connectedCallback() { // we do this before calling super, to be sure we still have our innerHTML intact @@ -162,16 +146,6 @@ export class SearchaliciousResults extends EventRegistrationMixin(LitElement) { // as _buildResultRenderer needs the slot named result to already exists this.resultRenderer = this._buildResultRenderer(); super.connectedCallback(); - this.addEventHandler(SearchaliciousEvents.NEW_RESULT, (event) => - this._displayResults(event) - ); - } - // connect to our specific events - override disconnectedCallback() { - super.disconnectedCallback(); - this.removeEventHandler(SearchaliciousEvents.NEW_RESULT, (event) => - this._displayResults(event) - ); } } diff --git a/frontend/src/test/search-pages_test.ts b/frontend/src/test/search-pages_test.ts index 120ec565..09dd784a 100644 --- a/frontend/src/test/search-pages_test.ts +++ b/frontend/src/test/search-pages_test.ts @@ -19,8 +19,9 @@ suite('searchalicious-pages', () => { pageCount: pageCount, currentPage: currentPage, pageSize: 10, + facets: {}, }; - el._updatePages( + el._handleResults( new CustomEvent(SearchaliciousEvents.NEW_RESULT, { bubbles: true, composed: true, diff --git a/frontend/src/test/search-results_test.ts b/frontend/src/test/search-results_test.ts index 93e5a303..8e3ea4a9 100644 --- a/frontend/src/test/search-results_test.ts +++ b/frontend/src/test/search-results_test.ts @@ -22,8 +22,9 @@ suite('searchalicious-results', () => { pageCount: 1, currentPage: 1, pageSize: 10, + facets: {}, }; - el._displayResults( + el._handleResults( new CustomEvent(SearchaliciousEvents.NEW_RESULT, { bubbles: true, composed: true, diff --git a/tests/unit/data/complex_query.json b/tests/unit/data/complex_query.json index 78cb85ed..c8136960 100644 --- a/tests/unit/data/complex_query.json +++ b/tests/unit/data/complex_query.json @@ -113,58 +113,6 @@ "minimum_should_match": 1 } }, - "aggs": { - "brands_tags": { - "terms": { - "field": "brands_tags" - } - }, - "lang": { - "terms": { - "field": "lang" - } - }, - "owner": { - "terms": { - "field": "owner" - } - }, - "categories_tags": { - "terms": { - "field": "categories_tags" - } - }, - "labels_tags": { - "terms": { - "field": "labels_tags" - } - }, - "countries_tags": { - "terms": { - "field": "countries_tags" - } - }, - "states_tags": { - "terms": { - "field": "states_tags" - } - }, - "nutrition_grades": { - "terms": { - "field": "nutrition_grades" - } - }, - "ecoscore_grade": { - "terms": { - "field": "ecoscore_grade" - } - }, - "nova_groups": { - "terms": { - "field": "nova_groups" - } - } - }, "size": 25, "from": 25 } \ No newline at end of file diff --git a/tests/unit/data/empty_query_with_sort_by.json b/tests/unit/data/empty_query_with_sort_by.json index fd15ab84..93d6f769 100644 --- a/tests/unit/data/empty_query_with_sort_by.json +++ b/tests/unit/data/empty_query_with_sort_by.json @@ -1,56 +1,4 @@ { - "aggs": { - "brands_tags": { - "terms": { - "field": "brands_tags" - } - }, - "lang": { - "terms": { - "field": "lang" - } - }, - "owner": { - "terms": { - "field": "owner" - } - }, - "categories_tags": { - "terms": { - "field": "categories_tags" - } - }, - "labels_tags": { - "terms": { - "field": "labels_tags" - } - }, - "countries_tags": { - "terms": { - "field": "countries_tags" - } - }, - "states_tags": { - "terms": { - "field": "states_tags" - } - }, - "nutrition_grades": { - "terms": { - "field": "nutrition_grades" - } - }, - "ecoscore_grade": { - "terms": { - "field": "ecoscore_grade" - } - }, - "nova_groups": { - "terms": { - "field": "nova_groups" - } - } - }, "sort": [ "unique_scans_n" ], diff --git a/tests/unit/data/empty_query_with_sort_by_and_facets.json b/tests/unit/data/empty_query_with_sort_by_and_facets.json new file mode 100644 index 00000000..9aa2c6d6 --- /dev/null +++ b/tests/unit/data/empty_query_with_sort_by_and_facets.json @@ -0,0 +1,29 @@ +{ + "aggs": { + "brands_tags": { + "terms": { + "field": "brands_tags" + } + }, + "categories_tags": { + "terms": { + "field": "categories_tags" + } + }, + "nutrition_grades": { + "terms": { + "field": "nutrition_grades" + } + }, + "lang": { + "terms": { + "field": "lang" + } + } + }, + "sort": [ + "unique_scans_n" + ], + "size": 25, + "from": 25 +} \ No newline at end of file diff --git a/tests/unit/data/non_existing_filter_field.json b/tests/unit/data/non_existing_filter_field.json index 56c4c84c..28b7ecff 100644 --- a/tests/unit/data/non_existing_filter_field.json +++ b/tests/unit/data/non_existing_filter_field.json @@ -13,58 +13,6 @@ ] } }, - "aggs": { - "brands_tags": { - "terms": { - "field": "brands_tags" - } - }, - "lang": { - "terms": { - "field": "lang" - } - }, - "owner": { - "terms": { - "field": "owner" - } - }, - "categories_tags": { - "terms": { - "field": "categories_tags" - } - }, - "labels_tags": { - "terms": { - "field": "labels_tags" - } - }, - "countries_tags": { - "terms": { - "field": "countries_tags" - } - }, - "states_tags": { - "terms": { - "field": "states_tags" - } - }, - "nutrition_grades": { - "terms": { - "field": "nutrition_grades" - } - }, - "ecoscore_grade": { - "terms": { - "field": "ecoscore_grade" - } - }, - "nova_groups": { - "terms": { - "field": "nova_groups" - } - } - }, "size": 25, "from": 25 } \ No newline at end of file diff --git a/tests/unit/data/simple_filter_query.json b/tests/unit/data/simple_filter_query.json index 5a0265ba..fad711fe 100644 --- a/tests/unit/data/simple_filter_query.json +++ b/tests/unit/data/simple_filter_query.json @@ -12,58 +12,6 @@ ] } }, - "aggs": { - "brands_tags": { - "terms": { - "field": "brands_tags" - } - }, - "lang": { - "terms": { - "field": "lang" - } - }, - "owner": { - "terms": { - "field": "owner" - } - }, - "categories_tags": { - "terms": { - "field": "categories_tags" - } - }, - "labels_tags": { - "terms": { - "field": "labels_tags" - } - }, - "countries_tags": { - "terms": { - "field": "countries_tags" - } - }, - "states_tags": { - "terms": { - "field": "states_tags" - } - }, - "nutrition_grades": { - "terms": { - "field": "nutrition_grades" - } - }, - "ecoscore_grade": { - "terms": { - "field": "ecoscore_grade" - } - }, - "nova_groups": { - "terms": { - "field": "nova_groups" - } - } - }, "size": 25, "from": 25 } \ No newline at end of file diff --git a/tests/unit/data/simple_full_text_query.json b/tests/unit/data/simple_full_text_query.json index 8535d559..baa76cc1 100644 --- a/tests/unit/data/simple_full_text_query.json +++ b/tests/unit/data/simple_full_text_query.json @@ -57,58 +57,6 @@ ] } }, - "aggs": { - "brands_tags": { - "terms": { - "field": "brands_tags" - } - }, - "lang": { - "terms": { - "field": "lang" - } - }, - "owner": { - "terms": { - "field": "owner" - } - }, - "categories_tags": { - "terms": { - "field": "categories_tags" - } - }, - "labels_tags": { - "terms": { - "field": "labels_tags" - } - }, - "countries_tags": { - "terms": { - "field": "countries_tags" - } - }, - "states_tags": { - "terms": { - "field": "states_tags" - } - }, - "nutrition_grades": { - "terms": { - "field": "nutrition_grades" - } - }, - "ecoscore_grade": { - "terms": { - "field": "ecoscore_grade" - } - }, - "nova_groups": { - "terms": { - "field": "nova_groups" - } - } - }, "size": 10, "from": 0 } \ No newline at end of file diff --git a/tests/unit/data/simple_full_text_query_facets.json b/tests/unit/data/simple_full_text_query_facets.json new file mode 100644 index 00000000..34906eb9 --- /dev/null +++ b/tests/unit/data/simple_full_text_query_facets.json @@ -0,0 +1,84 @@ +{ + "query": { + "bool": { + "should": [ + { + "match_phrase": { + "product_name.fr": { + "query": "flocons d'avoine", + "boost": 2.0 + } + } + }, + { + "match_phrase": { + "generic_name.fr": { + "query": "flocons d'avoine", + "boost": 2.0 + } + } + }, + { + "match_phrase": { + "categories.fr": { + "query": "flocons d'avoine", + "boost": 2.0 + } + } + }, + { + "match_phrase": { + "labels.fr": { + "query": "flocons d'avoine", + "boost": 2.0 + } + } + }, + { + "match_phrase": { + "brands": { + "query": "flocons d'avoine", + "boost": 2.0 + } + } + }, + { + "multi_match": { + "query": "flocons d'avoine", + "fields": [ + "product_name.fr", + "generic_name.fr", + "categories.fr", + "labels.fr", + "brands" + ] + } + } + ] + } + }, + "aggs": { + "brands_tags": { + "terms": { + "field": "brands_tags" + } + }, + "labels_tags": { + "terms": { + "field": "labels_tags" + } + }, + "nutrition_grades": { + "terms": { + "field": "nutrition_grades" + } + }, + "owner": { + "terms": { + "field": "owner" + } + } + }, + "size": 10, + "from": 0 +} \ No newline at end of file diff --git a/tests/unit/data/sort_by_query.json b/tests/unit/data/sort_by_query.json index e7cf95fb..7bee1935 100644 --- a/tests/unit/data/sort_by_query.json +++ b/tests/unit/data/sort_by_query.json @@ -57,58 +57,6 @@ ] } }, - "aggs": { - "brands_tags": { - "terms": { - "field": "brands_tags" - } - }, - "lang": { - "terms": { - "field": "lang" - } - }, - "owner": { - "terms": { - "field": "owner" - } - }, - "categories_tags": { - "terms": { - "field": "categories_tags" - } - }, - "labels_tags": { - "terms": { - "field": "labels_tags" - } - }, - "countries_tags": { - "terms": { - "field": "countries_tags" - } - }, - "states_tags": { - "terms": { - "field": "states_tags" - } - }, - "nutrition_grades": { - "terms": { - "field": "nutrition_grades" - } - }, - "ecoscore_grade": { - "terms": { - "field": "ecoscore_grade" - } - }, - "nova_groups": { - "terms": { - "field": "nova_groups" - } - } - }, "sort": [ { "unique_scans_n": { diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py new file mode 100644 index 00000000..07436d23 --- /dev/null +++ b/tests/unit/test_config.py @@ -0,0 +1,66 @@ +import app.config + +BASE_CONFIG = """ +indices: + test: + index: + number_of_replicas: 1 + number_of_shards: 1 + name: test_index + id_field_name: mykey + last_modified_field_name: last_modified + fields: + mykey: + required: true + type: keyword + mybool: + required: true + type: bool + last_modified: + required: true + type: date + mytext: + full_text_search: true + type: text_lang + myagg: + required: true + type: keyword + bucket_agg: true + # more fields + supported_langs: ["en", "fr"] + taxonomy: + sources: [] + exported_langs: [] + index: + number_of_replicas: 1 + number_of_shards: 1 + name: test_taxonomy + document_fetcher: app._import.BaseDocumentFetcher +default_index: test +""" +AGGS_FIELDS = """ + other_agg: + type: keyword + bucket_agg: true + more_agg: + type: keyword + bucket_agg: true + somuch_agg: + type: keyword + bucket_agg: true +""" + + +def _config_with_aggs(tmpdir, facets=""): + my_config = tmpdir / "config.yaml" + conf_content = BASE_CONFIG.replace(" # more fields\n", AGGS_FIELDS) + conf_content = conf_content.replace(" # facets\n", facets + "\n") + open(my_config, "w").write(conf_content) + return my_config + + +def test_loading_config(tmpdir): + conf_file = _config_with_aggs(tmpdir) + # just test it loads for now + app.config.Config.from_yaml(conf_file) + # TODO add asserts on config diff --git a/tests/unit/test_query.py b/tests/unit/test_query.py index 5a471585..4ede37e8 100644 --- a/tests/unit/test_query.py +++ b/tests/unit/test_query.py @@ -116,13 +116,22 @@ def test_parse_lucene_dsl_query( @pytest.mark.parametrize( - "id_,q,langs,size,page,sort_by", + "id_,q,langs,size,page,sort_by,facets", [ - ("simple_full_text_query", "flocons d'avoine", {"fr"}, 10, 1, None), + ("simple_full_text_query", "flocons d'avoine", {"fr"}, 10, 1, None, None), + ( + "simple_full_text_query_facets", + "flocons d'avoine", + {"fr"}, + 10, + 1, + None, + ["brands_tags", "labels_tags", "nutrition_grades", "owner"], + ), # sort by descending number of scan count - ("sort_by_query", "flocons d'avoine", {"fr"}, 10, 1, "-unique_scans_n"), + ("sort_by_query", "flocons d'avoine", {"fr"}, 10, 1, "-unique_scans_n", None), # we change number of results (25 instead of 10) and request page 2 - ("simple_filter_query", 'countries_tags:"en:italy"', {"en"}, 25, 2, None), + ("simple_filter_query", 'countries_tags:"en:italy"', {"en"}, 25, 2, None, None), ( "complex_query", 'bacon de boeuf (countries_tags:"en:italy" AND (categories_tags:"en:beef" AND ' @@ -131,6 +140,7 @@ def test_parse_lucene_dsl_query( 25, 2, None, + None, ), ( "non_existing_filter_field", @@ -139,6 +149,7 @@ def test_parse_lucene_dsl_query( 25, 2, None, + None, ), ( "empty_query_with_sort_by", @@ -147,6 +158,16 @@ def test_parse_lucene_dsl_query( 25, 2, "unique_scans_n", + None, + ), + ( + "empty_query_with_sort_by_and_facets", + None, + {"en"}, + 25, + 2, + "unique_scans_n", + ["brands_tags", "categories_tags", "nutrition_grades", "lang"], ), ], ) @@ -157,6 +178,7 @@ def test_build_search_query( size: int, page: int, sort_by: str | None, + facets: list[str] | None, update_results: bool, default_config: IndexConfig, default_filter_query_builder: ElasticsearchQueryBuilder, @@ -169,6 +191,7 @@ def test_build_search_query( config=default_config, filter_query_builder=default_filter_query_builder, sort_by=sort_by, + facets=facets, ) if update_results: