From ef937bc38081f2d8b7193a7e58cd13daf2d7f733 Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Mon, 3 Jun 2024 08:57:46 +0200 Subject: [PATCH 01/44] Resolve merge conflict --- src/onegov/agency/views/search.py | 13 +- src/onegov/fsi/views/search.py | 21 +++- src/onegov/landsgemeinde/views/search.py | 14 ++- src/onegov/onboarding/app.py | 4 +- src/onegov/org/app.py | 4 +- src/onegov/org/layout.py | 7 ++ src/onegov/org/models/__init__.py | 3 +- src/onegov/org/models/search.py | 154 +++++++++++++++++++++++ src/onegov/org/path.py | 11 ++ src/onegov/org/views/search.py | 47 +++++++ src/onegov/search/__init__.py | 4 +- src/onegov/search/integration.py | 13 +- src/onegov/town6/views/search.py | 14 ++- tests/onegov/search/conftest.py | 4 +- tests/onegov/search/test_integration.py | 22 ++-- 15 files changed, 299 insertions(+), 36 deletions(-) diff --git a/src/onegov/agency/views/search.py b/src/onegov/agency/views/search.py index dfdf8a9a61..39c98bd8a4 100644 --- a/src/onegov/agency/views/search.py +++ b/src/onegov/agency/views/search.py @@ -1,9 +1,8 @@ from onegov.agency import AgencyApp from onegov.agency.layout import AgencySearchLayout from onegov.core.security import Public -from onegov.org.models import Search -from onegov.org.views.search import search as search_view - +from onegov.org.models import Search, SearchPostgres +from onegov.org.views.search import search as search_view, search_postgres from typing import TYPE_CHECKING if TYPE_CHECKING: @@ -23,3 +22,11 @@ def search( if isinstance(data, dict): data['layout'] = AgencySearchLayout(self, request) return data + + +@AgencyApp.html(model=SearchPostgres, template='search.pt', permission=Public) +def agency_search_postgres(self, request): + data = search_postgres(self, request) + if isinstance(data, dict): + data['layout'] = AgencySearchLayout(self, request) + return data diff --git a/src/onegov/fsi/views/search.py b/src/onegov/fsi/views/search.py index c8e6cd549a..1405aa99b6 100644 --- a/src/onegov/fsi/views/search.py +++ b/src/onegov/fsi/views/search.py @@ -1,8 +1,11 @@ from onegov.core.security import Personal from onegov.fsi import FsiApp -from onegov.org.models import Search +from onegov.org.models import Search, SearchPostgres from onegov.org.views.search import search as search_view +from onegov.org.views.search import search_postgres as search_postgres_view from onegov.org.views.search import suggestions as suggestions_view +from onegov.org.views.search import (suggestions_postgres as + suggestions_postgres_view) from typing import TYPE_CHECKING @@ -21,9 +24,25 @@ def search( return search_view(self, request) +@FsiApp.html(model=SearchPostgres, template='search.pt', permission=Personal) +def search( + self: Search['Base'], + request: 'FsiRequest' +) -> 'RenderData | Response': + return search_postgres_view(self, request) + + @FsiApp.json(model=Search, name='suggest', permission=Personal) def suggestions( self: Search['Base'], request: 'FsiRequest' ) -> 'JSON_ro': return suggestions_view(self, request) + + +@FsiApp.json(model=SearchPostgres, name='suggest', permission=Personal) +def suggestions( + self: Search['Base'], + request: 'FsiRequest' +) -> 'JSON_ro': + return suggestions_postgres_view(self, request) diff --git a/src/onegov/landsgemeinde/views/search.py b/src/onegov/landsgemeinde/views/search.py index ded65c259f..c7d8b9c908 100644 --- a/src/onegov/landsgemeinde/views/search.py +++ b/src/onegov/landsgemeinde/views/search.py @@ -1,9 +1,8 @@ from onegov.core.security import Public from onegov.landsgemeinde import LandsgemeindeApp from onegov.landsgemeinde.layouts import DefaultLayout -from onegov.org.models import Search -from onegov.org.views.search import search - +from onegov.org.models import Search, SearchPostgres +from onegov.org.views.search import search, search_postgres from typing import TYPE_CHECKING if TYPE_CHECKING: @@ -19,3 +18,12 @@ def landsgemeinde_search( request: 'LandsgemeindeRequest' ) -> 'RenderData | Response': return search(self, request, DefaultLayout(self, request)) + + +@LandsgemeindeApp.html(model=SearchPostgres, template='search.pt', + permission=Public) +def landsgemeinde_search( + self: Search['Base'], + request: 'LandsgemeindeRequest' +) -> 'RenderData | Response': + return search_postgres(self, request, DefaultLayout(self, request)) diff --git a/src/onegov/onboarding/app.py b/src/onegov/onboarding/app.py index a460033181..ab5864d9b4 100644 --- a/src/onegov/onboarding/app.py +++ b/src/onegov/onboarding/app.py @@ -2,7 +2,7 @@ from onegov.file import DepotApp from onegov.onboarding.theme import OnboardingTheme from onegov.reservation import LibresIntegration -from onegov.search import ElasticsearchApp +from onegov.search import SearchApp from typing import Any, TYPE_CHECKING @@ -10,7 +10,7 @@ from collections.abc import Iterator -class OnboardingApp(Framework, LibresIntegration, DepotApp, ElasticsearchApp): +class OnboardingApp(Framework, LibresIntegration, DepotApp, SearchApp): serve_static_files = True diff --git a/src/onegov/org/app.py b/src/onegov/org/app.py index 47500e8845..0aff8a2359 100644 --- a/src/onegov/org/app.py +++ b/src/onegov/org/app.py @@ -32,7 +32,7 @@ from onegov.page import Page, PageCollection from onegov.pay import PayApp from onegov.reservation import LibresIntegration -from onegov.search import ElasticsearchApp +from onegov.search import SearchApp from onegov.ticket import TicketCollection from onegov.ticket import TicketPermission from onegov.user import UserApp @@ -59,7 +59,7 @@ from webob import Response -class OrgApp(Framework, LibresIntegration, ElasticsearchApp, MapboxApp, +class OrgApp(Framework, LibresIntegration, SearchApp, MapboxApp, DepotApp, PayApp, FormApp, UserApp, WebsocketsApp): serve_static_files = True diff --git a/src/onegov/org/layout.py b/src/onegov/org/layout.py index 3bc551cdbf..d4d089d240 100644 --- a/src/onegov/org/layout.py +++ b/src/onegov/org/layout.py @@ -42,6 +42,7 @@ from onegov.org.models.extensions import PersonLinkExtension from onegov.org.models.external_link import ExternalLinkCollection from onegov.org.models.form import submission_deletable +from onegov.org.models.search import SearchPostgres from onegov.org.open_graph import OpenGraphMixin from onegov.org.theme.org_theme import user_options from onegov.org.utils import IMG_URLS @@ -334,11 +335,17 @@ def homepage_url(self) -> str: @cached_property def search_url(self) -> str: """ Returns the url to the search page. """ + # Allows using postgres search while es search remains default + if 'search_postgres' in self.request.path_info: + return self.request.class_link(SearchPostgres) return self.request.class_link(Search) @cached_property def suggestions_url(self) -> str: """ Returns the url to the suggestions json view. """ + # Allows using postgres search while es search remains default + if 'search_postgres' in self.request.path_info: + return self.request.class_link(SearchPostgres, name='suggest') return self.request.class_link(Search, name='suggest') @cached_property diff --git a/src/onegov/org/models/__init__.py b/src/onegov/org/models/__init__.py index 6f123c9222..d2f6dd2f6b 100644 --- a/src/onegov/org/models/__init__.py +++ b/src/onegov/org/models/__init__.py @@ -46,7 +46,7 @@ from onegov.org.models.recipient import ResourceRecipient from onegov.org.models.recipient import ResourceRecipientCollection from onegov.org.models.resource import DaypassResource -from onegov.org.models.search import Search +from onegov.org.models.search import Search, SearchPostgres from onegov.org.models.sitecollection import SiteCollection from onegov.org.models.swiss_holidays import SwissHolidays from onegov.org.models.tan import TAN @@ -103,6 +103,7 @@ 'ResourceRecipient', 'ResourceRecipientCollection', 'Search', + 'SearchPostgres', 'SiteCollection', 'SubmissionMessage', 'SwissHolidays', diff --git a/src/onegov/org/models/search.py b/src/onegov/org/models/search.py index 32da2574fb..68129e2398 100644 --- a/src/onegov/org/models/search.py +++ b/src/onegov/org/models/search.py @@ -1,14 +1,23 @@ +from operator import attrgetter + from elasticsearch_dsl.function import SF from elasticsearch_dsl.query import FunctionScore from elasticsearch_dsl.query import Match from elasticsearch_dsl.query import MatchPhrase from elasticsearch_dsl.query import MultiMatch from functools import cached_property + +from sqlalchemy import func + from onegov.core.collection import Pagination, _M +from onegov.core.orm import Base from onegov.event.models import Event from typing import TYPE_CHECKING + +from onegov.search.utils import searchable_sqlalchemy_models + if TYPE_CHECKING: from onegov.org.request import OrgRequest from onegov.search.dsl import Hit, Response, Search as ESSearch @@ -161,3 +170,148 @@ def suggestions(self) -> tuple[str, ...]: return tuple(self.request.app.es_suggestions_by_request( self.request, self.query )) + + +def locale_mapping(locale): + mapping = {'de_CH': 'german', 'fr_CH': 'french', 'it_CH': 'italian', + 'rm_CH': 'english'} + return mapping.get(locale, 'english') + + +class SearchPostgres(Pagination): + """ + Implements searching in postgres db based on the gin index + """ + results_per_page = 10 + max_query_length = 100 + + def __init__(self, request, query, page): + self.request = request + self.query = query + self.page = page # page index + + self.nbr_of_docs = 0 + self.nbr_of_results = 0 + + @cached_property + def available_documents(self): + if not self.nbr_of_docs: + self.load_batch_results + return self.nbr_of_docs + + @cached_property + def available_results(self): + if not self.nbr_of_results: + self.load_batch_results + return self.nbr_of_results + + @property + def q(self): + return self.query + + def __eq__(self, other): + return self.page == other.page and self.query == other.query + + def subset(self): + return self.batch + + @property + def page_index(self): + return self.page + + def page_by_index(self, index): + return SearchPostgres(self.request, self.query, index) + + @cached_property + def batch(self): + if not self.query: + return None + + if self.query.startswith('#'): + results = self.hashtag_search() + else: + results = self.generic_search() + + return results[self.offset:self.offset + self.batch_size] + + @cached_property + def load_batch_results(self): + """Load search results and sort events by latest occurrence. + This methods is a wrapper around `batch.load()`, which returns the + actual search results form the query. """ + + batch = self.batch + events = [] + non_events = [] + for search_result in batch: + if isinstance(search_result, Event): + events.append(search_result) + else: + non_events.append(search_result) + if not events: + return batch + sorted_events = sorted(events, key=lambda e: e.latest_occurrence.start) + return sorted_events + non_events + + def generic_search(self): + doc_count = 0 + results = [] + + language = locale_mapping(self.request.locale) + for model in searchable_sqlalchemy_models(Base): + if model.es_public or self.request.is_logged_in: + query = self.request.session.query(model) + doc_count += query.count() + query = query.filter( + model.fts_idx.op('@@')(func.websearch_to_tsquery( + language, self.query)) + ) + query = query.order_by(func.ts_rank_cd( + model.fts_idx, func.websearch_to_tsquery(language, + self.query))) + results.extend(query.all()) + + self.nbr_of_docs = doc_count + self.nbr_of_results = len(results) + results.sort(key=attrgetter('search_score'), reverse=False) + return results + + def hashtag_search(self): + q = self.query.lstrip('#') + results = [] + + for model in searchable_sqlalchemy_models(Base): + # skip certain tables for hashtag search for better performance + if model.__tablename__ not in ['attendees', 'files', 'people', + 'tickets', 'users']: + if model.es_public or self.request.is_logged_in: + for doc in self.request.session.query(model).all(): + if doc.es_tags and q in doc.es_tags: + results.append(doc) + + self.nbr_of_results = len(results) + results.sort(key=attrgetter('search_score'), reverse=False) + return results + + def feeling_lucky(self): + if self.batch: + first_entry = self.batch[0].load() + + # XXX the default view to the event should be doing the redirect + if first_entry.__tablename__ == 'events': + return self.request.link(first_entry, 'latest') + else: + return self.request.link(first_entry) + + @cached_property + def subset_count(self): + return self.available_results + + def suggestions(self): + suggestions = list() + + for element in self.generic_search(): + suggest = getattr(element, 'es_suggestion', []) + suggestions.append(suggest) + + return tuple(suggestions[:15]) diff --git a/src/onegov/org/path.py b/src/onegov/org/path.py index e1ac9428b3..0edfebb52b 100644 --- a/src/onegov/org/path.py +++ b/src/onegov/org/path.py @@ -67,6 +67,7 @@ from onegov.org.models.external_link import ( ExternalLinkCollection, ExternalLink) from onegov.org.models.resource import FindYourSpotCollection +from onegov.org.models.search import SearchPostgres from onegov.page import PageCollection from onegov.pay import PaymentProvider, Payment, PaymentCollection from onegov.pay import PaymentProviderCollection @@ -690,6 +691,16 @@ def get_search( return Search(request, q, page) +@OrgApp.path(model=SearchPostgres, path='/search_postgres', + converters={'page': int}) +def get_search_postgres( + request: 'OrgRequest', + q: str = '', + page: int = 0 +) -> Search[Any]: + return SearchPostgres(request, q, page) + + @OrgApp.path(model=AtoZPages, path='/a-z') def get_a_to_z(request: 'OrgRequest') -> AtoZPages: return AtoZPages(request) diff --git a/src/onegov/org/views/search.py b/src/onegov/org/views/search.py index 7b714ed902..617ac899e3 100644 --- a/src/onegov/org/views/search.py +++ b/src/onegov/org/views/search.py @@ -5,6 +5,7 @@ from onegov.org.elements import Link from onegov.org.layout import DefaultLayout from onegov.org.models import Search +from onegov.org.models.search import SearchPostgres from onegov.search import SearchOfflineError from webob import exc @@ -59,9 +60,55 @@ def search( } +@OrgApp.html(model=SearchPostgres, template='search', permission=Public) +def search_postgres(self, request, layout=None): + layout = layout or DefaultLayout(self, request) + layout.breadcrumbs.append(Link(_("Search"), '#')) + + try: + searchlabel = _("Search through ${count} indexed documents", mapping={ + 'count': self.available_documents + }) + resultslabel = _("${count} Results", mapping={ + 'count': self.available_results + }) + except SearchOfflineError: + return { + 'title': _("Search Unavailable"), + 'layout': layout, + 'connection': False + } + + if 'lucky' in request.GET: + url = self.feeling_lucky() + + if url: + return morepath.redirect(url) + + return { + # TODO switch back to 'Search' once es is gone + 'title': _("Org Search Postgres"), + 'model': self, + 'layout': layout, + 'hide_search_header': True, + 'searchlabel': searchlabel, + 'resultslabel': resultslabel, + 'connection': True + } + + @OrgApp.json(model=Search, name='suggest', permission=Public) def suggestions(self: Search['Base'], request: 'OrgRequest') -> 'JSON_ro': try: return self.suggestions() except SearchOfflineError as exception: raise exc.HTTPNotFound() from exception + + +@OrgApp.json(model=SearchPostgres, name='suggest', permission=Public) +def suggestions_postgres(self: Search['Base'], request: 'OrgRequest') \ + -> 'JSON_ro': + try: + return self.suggestions() + except SearchOfflineError as exception: + raise exc.HTTPNotFound() from exception diff --git a/src/onegov/search/__init__.py b/src/onegov/search/__init__.py index d2d5df7288..b1da536e8a 100644 --- a/src/onegov/search/__init__.py +++ b/src/onegov/search/__init__.py @@ -7,11 +7,11 @@ from onegov.search.mixins import Searchable, ORMSearchable, SearchableContent from onegov.search.dsl import Search -from onegov.search.integration import ElasticsearchApp +from onegov.search.integration import SearchApp from onegov.search.errors import SearchOfflineError __all__ = [ - 'ElasticsearchApp', + 'SearchApp', 'ORMSearchable', 'Search', 'Searchable', diff --git a/src/onegov/search/integration.py b/src/onegov/search/integration.py index 0a9c3f31da..4ddb72aba0 100644 --- a/src/onegov/search/integration.py +++ b/src/onegov/search/integration.py @@ -108,9 +108,8 @@ def is_5xx_error(error: TransportError) -> bool: return False -# TODO rename to SearchApp -class ElasticsearchApp(morepath.App): - """ Provides elasticsearch integration for +class SearchApp(morepath.App): + """ Provides elasticsearch and postgres integration for :class:`onegov.core.framework.Framework` based applications. The application must be connected to a database. @@ -177,6 +176,8 @@ def configure_search(self, **cfg: Any) -> None: - fr """ + # TODO: set default to False once fully switched to psql (or remove + # es stuff entirely) if not cfg.get('enable_elasticsearch', True): self.es_client = None return @@ -483,14 +484,14 @@ def reindex_model(model: type['Base']) -> None: self.psql_indexer.bulk_process() -@ElasticsearchApp.tween_factory(over=transaction_tween_factory) +@SearchApp.tween_factory(over=transaction_tween_factory) def process_indexer_tween_factory( - app: ElasticsearchApp, + app: SearchApp, handler: 'Callable[[CoreRequest], Response]' ) -> 'Callable[[CoreRequest], Response]': def process_indexer_tween(request: 'CoreRequest') -> 'Response': - app: ElasticsearchApp = request.app # type:ignore[assignment] + app: SearchApp = request.app # type:ignore[assignment] if not app.es_client: return handler(request) diff --git a/src/onegov/town6/views/search.py b/src/onegov/town6/views/search.py index db31ec7a48..0914d8e0a8 100644 --- a/src/onegov/town6/views/search.py +++ b/src/onegov/town6/views/search.py @@ -1,10 +1,10 @@ from onegov.core.security import Public -from onegov.org.views.search import search -from onegov.town6 import TownApp from onegov.org.models import Search +from onegov.org.models.search import SearchPostgres +from onegov.org.views.search import search, search_postgres +from onegov.town6 import TownApp from onegov.town6.layout import DefaultLayout - from typing import TYPE_CHECKING if TYPE_CHECKING: from onegov.core.orm import Base @@ -19,3 +19,11 @@ def town_search( request: 'TownRequest' ) -> 'RenderData | Response': return search(self, request, DefaultLayout(self, request)) + + +@TownApp.html(model=SearchPostgres, template='search.pt', permission=Public) +def town_search_postgres( + self: Search['Base'], + request: 'TownRequest' +) -> 'RenderData | Response': + return search_postgres(self, request, DefaultLayout(self, request)) diff --git a/tests/onegov/search/conftest.py b/tests/onegov/search/conftest.py index da452eb3f7..f970b20325 100644 --- a/tests/onegov/search/conftest.py +++ b/tests/onegov/search/conftest.py @@ -3,7 +3,7 @@ from yaml import dump from onegov.core import Framework -from onegov.search import ElasticsearchApp +from onegov.search import SearchApp from tests.shared.utils import create_app @@ -46,7 +46,7 @@ def cfg_path(postgres_dsn, session_manager, temporary_directory, redis_url): return cfg_path -class TestApp(Framework, ElasticsearchApp): +class TestApp(Framework, SearchApp): pass diff --git a/tests/onegov/search/test_integration.py b/tests/onegov/search/test_integration.py index a93e13f60d..581818c5e9 100644 --- a/tests/onegov/search/test_integration.py +++ b/tests/onegov/search/test_integration.py @@ -8,7 +8,7 @@ from onegov.core import Framework from onegov.core.orm.mixins import TimestampMixin from onegov.core.utils import scan_morepath_modules -from onegov.search import ElasticsearchApp, ORMSearchable +from onegov.search import SearchApp, ORMSearchable from sqlalchemy import Boolean, Column, Integer, Text from sqlalchemy.ext.declarative import declarative_base from webtest import TestApp as Client @@ -17,7 +17,7 @@ def test_app_integration(es_url): - class App(Framework, ElasticsearchApp): + class App(Framework, SearchApp): pass app = App() @@ -33,7 +33,7 @@ class App(Framework, ElasticsearchApp): def test_search_query(es_url, postgres_dsn): - class App(Framework, ElasticsearchApp): + class App(Framework, SearchApp): pass Base = declarative_base() @@ -153,7 +153,7 @@ def es_language(self): def test_orm_integration(es_url, postgres_dsn, redis_url): - class App(Framework, ElasticsearchApp): + class App(Framework, SearchApp): pass Base = declarative_base() @@ -298,7 +298,7 @@ def view_delete_document(self, request): def test_alternate_id_property(es_url, postgres_dsn): - class App(Framework, ElasticsearchApp): + class App(Framework, SearchApp): pass Base = declarative_base() @@ -362,7 +362,7 @@ def es_suggestion(self): def test_orm_polymorphic(es_url, postgres_dsn): - class App(Framework, ElasticsearchApp): + class App(Framework, SearchApp): pass Base = declarative_base() @@ -450,7 +450,7 @@ def update(): def test_orm_polymorphic_sublcass_only(es_url, postgres_dsn): - class App(Framework, ElasticsearchApp): + class App(Framework, SearchApp): pass Base = declarative_base() @@ -508,7 +508,7 @@ def es_suggestion(self): def test_suggestions(es_url, postgres_dsn): - class App(Framework, ElasticsearchApp): + class App(Framework, SearchApp): pass Base = declarative_base() @@ -629,7 +629,7 @@ def es_suggestion(self): def test_language_detection(es_url, postgres_dsn): - class App(Framework, ElasticsearchApp): + class App(Framework, SearchApp): pass Base = declarative_base() @@ -685,7 +685,7 @@ class Document(Base, ORMSearchable): def test_language_update(es_url, postgres_dsn): - class App(Framework, ElasticsearchApp): + class App(Framework, SearchApp): pass Base = declarative_base() @@ -741,7 +741,7 @@ class Document(Base, ORMSearchable): def test_date_decay(es_url, postgres_dsn): - class App(Framework, ElasticsearchApp): + class App(Framework, SearchApp): pass Base = declarative_base() From 2fdd50e4cbe34f643513a83c98924c710042dc06 Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Tue, 4 Jun 2024 11:01:21 +0200 Subject: [PATCH 02/44] Adds view and templates for search views --- src/onegov/org/models/search.py | 74 +++++++++++-------- src/onegov/org/path.py | 2 +- src/onegov/org/templates/search_postgres.pt | 68 +++++++++++++++++ src/onegov/org/views/search.py | 3 +- src/onegov/search/cli.py | 2 +- src/onegov/search/integration.py | 13 ++-- src/onegov/search/mixins.py | 5 ++ src/onegov/town6/templates/search_postgres.pt | 68 +++++++++++++++++ src/onegov/town6/views/search.py | 7 +- 9 files changed, 199 insertions(+), 43 deletions(-) create mode 100644 src/onegov/org/templates/search_postgres.pt create mode 100644 src/onegov/town6/templates/search_postgres.pt diff --git a/src/onegov/org/models/search.py b/src/onegov/org/models/search.py index 68129e2398..9ff250e1fa 100644 --- a/src/onegov/org/models/search.py +++ b/src/onegov/org/models/search.py @@ -8,13 +8,14 @@ from functools import cached_property from sqlalchemy import func +from sqlalchemy.orm import Query from onegov.core.collection import Pagination, _M from onegov.core.orm import Base from onegov.event.models import Event -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Any from onegov.search.utils import searchable_sqlalchemy_models @@ -172,20 +173,20 @@ def suggestions(self) -> tuple[str, ...]: )) -def locale_mapping(locale): +def locale_mapping(locale: str) -> str: mapping = {'de_CH': 'german', 'fr_CH': 'french', 'it_CH': 'italian', 'rm_CH': 'english'} return mapping.get(locale, 'english') -class SearchPostgres(Pagination): +class SearchPostgres(Pagination[_M]): """ Implements searching in postgres db based on the gin index """ results_per_page = 10 max_query_length = 100 - def __init__(self, request, query, page): + def __init__(self, request: 'OrgRequest', query: str, page: int): self.request = request self.query = query self.page = page # page index @@ -194,32 +195,34 @@ def __init__(self, request, query, page): self.nbr_of_results = 0 @cached_property - def available_documents(self): + def available_documents(self) -> int: if not self.nbr_of_docs: self.load_batch_results return self.nbr_of_docs @cached_property - def available_results(self): + def available_results(self) -> int: if not self.nbr_of_results: self.load_batch_results return self.nbr_of_results @property - def q(self): + def q(self) -> str: return self.query - def __eq__(self, other): + def __eq__(self, other: object) -> bool: + if not isinstance(other, SearchPostgres): + return NotImplemented return self.page == other.page and self.query == other.query def subset(self): return self.batch @property - def page_index(self): + def page_index(self) -> int: return self.page - def page_by_index(self, index): + def page_by_index(self, index: int): return SearchPostgres(self.request, self.query, index) @cached_property @@ -235,7 +238,7 @@ def batch(self): return results[self.offset:self.offset + self.batch_size] @cached_property - def load_batch_results(self): + def load_batch_results(self) -> list[Query[Any]]: """Load search results and sort events by latest occurrence. This methods is a wrapper around `batch.load()`, which returns the actual search results form the query. """ @@ -253,30 +256,38 @@ def load_batch_results(self): sorted_events = sorted(events, key=lambda e: e.latest_occurrence.start) return sorted_events + non_events - def generic_search(self): + def generic_search(self) -> list[Query[Any]]: doc_count = 0 results = [] language = locale_mapping(self.request.locale) - for model in searchable_sqlalchemy_models(Base): - if model.es_public or self.request.is_logged_in: - query = self.request.session.query(model) - doc_count += query.count() - query = query.filter( - model.fts_idx.op('@@')(func.websearch_to_tsquery( - language, self.query)) - ) - query = query.order_by(func.ts_rank_cd( - model.fts_idx, func.websearch_to_tsquery(language, - self.query))) - results.extend(query.all()) + for base in self.request.app.session_manager.bases: + for model in searchable_sqlalchemy_models(base): + if model.es_public or self.request.is_logged_in: + query = self.request.session.query(model) + + if query.count(): + doc_count += query.count() + query = query.filter( + model.fts_idx.op('@@')(func.websearch_to_tsquery( + language, self.query)) + ) + query = query.order_by( + func.ts_rank_cd( + model.fts_idx, + func.websearch_to_tsquery( + language, + self.query) + ) + ) + results.extend(query.all()) self.nbr_of_docs = doc_count self.nbr_of_results = len(results) - results.sort(key=attrgetter('search_score'), reverse=False) + results.sort(key=attrgetter('ts_score'), reverse=False) return results - def hashtag_search(self): + def hashtag_search(self) -> list[Query[Any]]: q = self.query.lstrip('#') results = [] @@ -290,10 +301,10 @@ def hashtag_search(self): results.append(doc) self.nbr_of_results = len(results) - results.sort(key=attrgetter('search_score'), reverse=False) + results.sort(key=attrgetter('ts_score'), reverse=False) return results - def feeling_lucky(self): + def feeling_lucky(self) -> str | None: if self.batch: first_entry = self.batch[0].load() @@ -302,13 +313,14 @@ def feeling_lucky(self): return self.request.link(first_entry, 'latest') else: return self.request.link(first_entry) + return None @cached_property - def subset_count(self): + def subset_count(self) -> int: return self.available_results - def suggestions(self): - suggestions = list() + def suggestions(self) -> tuple[str, ...]: + suggestions = [] for element in self.generic_search(): suggest = getattr(element, 'es_suggestion', []) diff --git a/src/onegov/org/path.py b/src/onegov/org/path.py index 0edfebb52b..9d928e8c34 100644 --- a/src/onegov/org/path.py +++ b/src/onegov/org/path.py @@ -691,7 +691,7 @@ def get_search( return Search(request, q, page) -@OrgApp.path(model=SearchPostgres, path='/search_postgres', +@OrgApp.path(model=SearchPostgres, path='/search-postgres', converters={'page': int}) def get_search_postgres( request: 'OrgRequest', diff --git a/src/onegov/org/templates/search_postgres.pt b/src/onegov/org/templates/search_postgres.pt new file mode 100644 index 0000000000..eee07c1e56 --- /dev/null +++ b/src/onegov/org/templates/search_postgres.pt @@ -0,0 +1,68 @@ +
+ + ${title} + + +
+
+ Postgres Searching is currently unavailable due to technical + difficulties. + Please excuse the inconvenience and try again later. +
+
+ +
+
+
+ +
+ +
+ +
+
+
+
+
+
+
+
+ + +

${resultslabel}

+
+
+ +

Your postgres search returned no results.

+ +
    + +
  • + + + +
      +
    • Score: ${result.explanation['score']}
      • + +
    • + ${title}: ${result.explanation[key]['value']} +
      • +       +
    +     +
    • +     +
+ +
+
+
+ + +
\ No newline at end of file diff --git a/src/onegov/org/views/search.py b/src/onegov/org/views/search.py index 617ac899e3..ddb719d51a 100644 --- a/src/onegov/org/views/search.py +++ b/src/onegov/org/views/search.py @@ -60,7 +60,8 @@ def search( } -@OrgApp.html(model=SearchPostgres, template='search', permission=Public) +@OrgApp.html(model=SearchPostgres, template='search_postgres.pt', + permission=Public) def search_postgres(self, request, layout=None): layout = layout or DefaultLayout(self, request) layout.breadcrumbs.append(Link(_("Search"), '#')) diff --git a/src/onegov/search/cli.py b/src/onegov/search/cli.py index 3257e929a3..6be69e260f 100644 --- a/src/onegov/search/cli.py +++ b/src/onegov/search/cli.py @@ -83,7 +83,7 @@ def run_reindex(request: 'CoreRequest', app: 'Framework') -> None: click.secho(title, underline=True) start = utcnow() - request.app.es_perform_reindex(fail) # type:ignore[attr-defined] + request.app.perform_reindex(fail) # type:ignore[attr-defined] click.secho(f"took {utcnow() - start}") diff --git a/src/onegov/search/integration.py b/src/onegov/search/integration.py index 4ddb72aba0..1ece380f30 100644 --- a/src/onegov/search/integration.py +++ b/src/onegov/search/integration.py @@ -419,7 +419,7 @@ def get_searchable_models(self) -> list[type['Searchable']]: for model in searchable_sqlalchemy_models(base) ] - def es_perform_reindex(self, fail: bool = False) -> None: + def perform_reindex(self, fail: bool = False) -> None: """ Re-indexes all content. This is a heavy operation and should be run with consideration. @@ -428,7 +428,7 @@ def es_perform_reindex(self, fail: bool = False) -> None: """ # prevent tables get re-indexed twice - index_done = [] + index_done = [str] schema = self.schema index_log.info(f'Indexing schema {schema}..') @@ -470,12 +470,13 @@ def reindex_model(model: type['Base']) -> None: session.invalidate() session.bind.dispose() - models = self.get_searchable_models() - index_log.info(f'Number of models to be indexed: {len(models)}') - with ThreadPoolExecutor() as executor: results = executor.map( - reindex_model, (model for model in models) + reindex_model, ( + model + for base in self.session_manager.bases + for model in searchable_sqlalchemy_models(base) + ) ) if fail: print(tuple(results)) diff --git a/src/onegov/search/mixins.py b/src/onegov/search/mixins.py index 02e64668cd..96a25cc7ef 100644 --- a/src/onegov/search/mixins.py +++ b/src/onegov/search/mixins.py @@ -175,6 +175,11 @@ def es_tags(self) -> list[str] | None: """ Returns a list of tags associated with this content. """ return None + @property + def ts_score(self) -> int: + """ Returns the score of the full text search. """ + return 10 + class ORMSearchable(Searchable): """ Extends the default :class:`Searchable` class with sensible defaults diff --git a/src/onegov/town6/templates/search_postgres.pt b/src/onegov/town6/templates/search_postgres.pt new file mode 100644 index 0000000000..eee07c1e56 --- /dev/null +++ b/src/onegov/town6/templates/search_postgres.pt @@ -0,0 +1,68 @@ +
+ + ${title} + + +
+
+ Postgres Searching is currently unavailable due to technical + difficulties. + Please excuse the inconvenience and try again later. +
+
+ +
+
+
+ +
+ +
+ +
+
+
+
+
+
+
+
+ + +

${resultslabel}

+
+
+ +

Your postgres search returned no results.

+ +
    + +
  • + + + +
      +
    • Score: ${result.explanation['score']}
      • + +
    • + ${title}: ${result.explanation[key]['value']} +
      • +       +
    +     +
    • +     +
+ +
+
+
+ + +
\ No newline at end of file diff --git a/src/onegov/town6/views/search.py b/src/onegov/town6/views/search.py index 0914d8e0a8..066e513ae5 100644 --- a/src/onegov/town6/views/search.py +++ b/src/onegov/town6/views/search.py @@ -21,9 +21,10 @@ def town_search( return search(self, request, DefaultLayout(self, request)) -@TownApp.html(model=SearchPostgres, template='search.pt', permission=Public) +@TownApp.html(model=SearchPostgres, template='search_postgres.pt', + permission=Public) def town_search_postgres( - self: Search['Base'], - request: 'TownRequest' + self: Search['Base'], + request: 'TownRequest' ) -> 'RenderData | Response': return search_postgres(self, request, DefaultLayout(self, request)) From d90e85aa4fe6a4c108165835c4b50bd67ab2eace Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Wed, 5 Jun 2024 10:06:51 +0200 Subject: [PATCH 03/44] Revert type wrong type annotations --- src/onegov/org/models/search.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/onegov/org/models/search.py b/src/onegov/org/models/search.py index 9ff250e1fa..e8d6285e86 100644 --- a/src/onegov/org/models/search.py +++ b/src/onegov/org/models/search.py @@ -8,14 +8,13 @@ from functools import cached_property from sqlalchemy import func -from sqlalchemy.orm import Query from onegov.core.collection import Pagination, _M from onegov.core.orm import Base from onegov.event.models import Event -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING from onegov.search.utils import searchable_sqlalchemy_models @@ -179,7 +178,7 @@ def locale_mapping(locale: str) -> str: return mapping.get(locale, 'english') -class SearchPostgres(Pagination[_M]): +class SearchPostgres(Pagination): """ Implements searching in postgres db based on the gin index """ @@ -238,7 +237,7 @@ def batch(self): return results[self.offset:self.offset + self.batch_size] @cached_property - def load_batch_results(self) -> list[Query[Any]]: + def load_batch_results(self): """Load search results and sort events by latest occurrence. This methods is a wrapper around `batch.load()`, which returns the actual search results form the query. """ @@ -256,7 +255,7 @@ def load_batch_results(self) -> list[Query[Any]]: sorted_events = sorted(events, key=lambda e: e.latest_occurrence.start) return sorted_events + non_events - def generic_search(self) -> list[Query[Any]]: + def generic_search(self): doc_count = 0 results = [] @@ -287,7 +286,7 @@ def generic_search(self) -> list[Query[Any]]: results.sort(key=attrgetter('ts_score'), reverse=False) return results - def hashtag_search(self) -> list[Query[Any]]: + def hashtag_search(self): q = self.query.lstrip('#') results = [] From 22932f0dd8c6c30c8102cb664998e7d0d966e8a9 Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Wed, 5 Jun 2024 11:15:51 +0200 Subject: [PATCH 04/44] Sort search results after score, timestamp --- src/onegov/org/models/search.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/onegov/org/models/search.py b/src/onegov/org/models/search.py index e8d6285e86..36769325e1 100644 --- a/src/onegov/org/models/search.py +++ b/src/onegov/org/models/search.py @@ -283,7 +283,8 @@ def generic_search(self): self.nbr_of_docs = doc_count self.nbr_of_results = len(results) - results.sort(key=attrgetter('ts_score'), reverse=False) + results.sort(key=attrgetter('ts_score', 'modified', 'created'), + reverse=False) return results def hashtag_search(self): From 0507b46bde969a06432b66a0f30aa7d3c855fab3 Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Wed, 5 Jun 2024 14:14:01 +0200 Subject: [PATCH 05/44] Adds a simple ranking --- src/onegov/activity/models/attendee.py | 4 ++++ src/onegov/directory/models/directory.py | 4 ++++ .../directory/models/directory_entry.py | 4 ++++ src/onegov/event/models/event.py | 4 ++++ src/onegov/feriennet/models/activity.py | 4 ++++ src/onegov/file/models/file.py | 4 ++++ src/onegov/form/models/definition.py | 4 ++++ src/onegov/fsi/models/course.py | 4 ++++ src/onegov/newsletter/models.py | 4 ++++ src/onegov/org/models/external_link.py | 4 ++++ src/onegov/org/models/search.py | 21 ++++++++++++++++--- src/onegov/page/model.py | 4 ++++ src/onegov/people/models/membership.py | 4 ++++ src/onegov/people/models/person.py | 4 ++++ src/onegov/search/mixins.py | 8 +++++-- src/onegov/ticket/model.py | 4 ++++ .../translator_directory/models/translator.py | 4 ++++ src/onegov/user/models/user.py | 4 ++++ 18 files changed, 88 insertions(+), 5 deletions(-) diff --git a/src/onegov/activity/models/attendee.py b/src/onegov/activity/models/attendee.py index 163a62bc25..852f25ba69 100644 --- a/src/onegov/activity/models/attendee.py +++ b/src/onegov/activity/models/attendee.py @@ -67,6 +67,10 @@ class Attendee(Base, TimestampMixin, ORMSearchable): } es_public = False + @property + def ts_score(self) -> int: + return 3 + @property def es_suggestion(self) -> str: return self.name diff --git a/src/onegov/directory/models/directory.py b/src/onegov/directory/models/directory.py index 7196aa9aa8..0377154ce9 100644 --- a/src/onegov/directory/models/directory.py +++ b/src/onegov/directory/models/directory.py @@ -109,6 +109,10 @@ class Directory(Base, ContentMixin, TimestampMixin, 'lead': {'type': 'localized'} } + @property + def ts_score(self) -> int: + return 7 + @property def es_public(self) -> bool: return False # to be overridden downstream diff --git a/src/onegov/directory/models/directory_entry.py b/src/onegov/directory/models/directory_entry.py index b477d04b2d..b5946abc52 100644 --- a/src/onegov/directory/models/directory_entry.py +++ b/src/onegov/directory/models/directory_entry.py @@ -41,6 +41,10 @@ class DirectoryEntry(Base, ContentMixin, CoordinatesMixin, TimestampMixin, 'text': {'type': 'localized_html'} } + @property + def ts_score(self) -> int: + return 4 + @property def es_public(self) -> bool: return False # to be overridden downstream diff --git a/src/onegov/event/models/event.py b/src/onegov/event/models/event.py index 09814c5bbc..3ecb36e3ff 100644 --- a/src/onegov/event/models/event.py +++ b/src/onegov/event/models/event.py @@ -137,6 +137,10 @@ class Event(Base, OccurrenceMixin, TimestampMixin, SearchableContent, EventFile, 'pdf', 'one-to-one', uselist=False, backref_suffix='pdf' ) + @property + def ts_score(self) -> int: + return 1 + def set_image( self, content: bytes | IO[bytes] | None, diff --git a/src/onegov/feriennet/models/activity.py b/src/onegov/feriennet/models/activity.py index cc47c0a4b4..8277619bdc 100644 --- a/src/onegov/feriennet/models/activity.py +++ b/src/onegov/feriennet/models/activity.py @@ -31,6 +31,10 @@ class VacationActivity(Activity, CoordinatesExtension, SearchableContent): 'organiser': {'type': 'text'} } + @property + def ts_score(self) -> int: + return 1 + @property def es_public(self) -> bool: return self.state == 'accepted' diff --git a/src/onegov/file/models/file.py b/src/onegov/file/models/file.py index dd2f073fcc..5a3a030270 100644 --- a/src/onegov/file/models/file.py +++ b/src/onegov/file/models/file.py @@ -109,6 +109,10 @@ class SearchableFile(ORMSearchable): name: Column[str] published: Column[bool] + @property + def ts_score(self) -> int: + return 10 + @property def es_suggestion(self) -> str: return self.name diff --git a/src/onegov/form/models/definition.py b/src/onegov/form/models/definition.py index 07ed59e749..7a500d0427 100644 --- a/src/onegov/form/models/definition.py +++ b/src/onegov/form/models/definition.py @@ -147,6 +147,10 @@ def id(self) -> str: 'polymorphic_identity': 'generic' } + @property + def ts_score(self) -> int: + return 7 + @property def form_class(self) -> Type['Form']: """ Parses the form definition and returns a form class. """ diff --git a/src/onegov/fsi/models/course.py b/src/onegov/fsi/models/course.py index dc44990e62..2c7ae1c50a 100644 --- a/src/onegov/fsi/models/course.py +++ b/src/onegov/fsi/models/course.py @@ -52,6 +52,10 @@ class Course(Base, ORMSearchable): default=False ) + @property + def ts_score(self) -> int: + return 2 + if TYPE_CHECKING: # FIXME: use explicit backref events: relationship[AppenderQuery[CourseEvent]] diff --git a/src/onegov/newsletter/models.py b/src/onegov/newsletter/models.py index 266a9f916e..779a953566 100644 --- a/src/onegov/newsletter/models.py +++ b/src/onegov/newsletter/models.py @@ -95,6 +95,10 @@ def validate_name(self, key: str, name: str) -> str: secondary=newsletter_recipients, back_populates='newsletters') + @property + def ts_score(self) -> int: + return 6 + @property def open_recipients(self) -> tuple['Recipient', ...]: received = select([newsletter_recipients.c.recipient_id]).where( diff --git a/src/onegov/org/models/external_link.py b/src/onegov/org/models/external_link.py index 25ea34581f..5c68fef630 100644 --- a/src/onegov/org/models/external_link.py +++ b/src/onegov/org/models/external_link.py @@ -59,6 +59,10 @@ class ExternalLink(Base, ContentMixin, TimestampMixin, AccessExtension, lead: dict_property[str | None] = meta_property() + @property + def ts_score(self) -> int: + return 8 + @observes('title') def title_observer(self, title: str) -> None: self.order = normalize_for_url(title) diff --git a/src/onegov/org/models/search.py b/src/onegov/org/models/search.py index 36769325e1..776441916d 100644 --- a/src/onegov/org/models/search.py +++ b/src/onegov/org/models/search.py @@ -1,3 +1,4 @@ +from datetime import datetime from operator import attrgetter from elasticsearch_dsl.function import SF @@ -7,6 +8,7 @@ from elasticsearch_dsl.query import MultiMatch from functools import cached_property +from pytz import utc from sqlalchemy import func from onegov.core.collection import Pagination, _M @@ -283,9 +285,22 @@ def generic_search(self): self.nbr_of_docs = doc_count self.nbr_of_results = len(results) - results.sort(key=attrgetter('ts_score', 'modified', 'created'), - reverse=False) - return results + + # remove duplicates + results = list(set(results)) + + # sort items after ts_score, modified and created. If no timestamp + # is available, use default time + default_time = utc.localize( + datetime.datetime(1970, 1, 1)) + return sorted( + results, + key=lambda k: ( + k.get('ts_score', 10), + k.get('modified') or default_time, + k.get('created') or default_time, + ), + reverse=False) def hashtag_search(self): q = self.query.lstrip('#') diff --git a/src/onegov/page/model.py b/src/onegov/page/model.py index 659a2fa59b..718a88ff53 100644 --- a/src/onegov/page/model.py +++ b/src/onegov/page/model.py @@ -27,6 +27,10 @@ class Page(AdjacencyList, ContentMixin, TimestampMixin, __tablename__ = 'pages' + @property + def ts_score(self) -> int: + return 2 + if TYPE_CHECKING: # we override these relationships to be more specific parent: relationship['Page | None'] diff --git a/src/onegov/people/models/membership.py b/src/onegov/people/models/membership.py index a046c94f8b..98cbb8657c 100644 --- a/src/onegov/people/models/membership.py +++ b/src/onegov/people/models/membership.py @@ -104,6 +104,10 @@ class AgencyMembership(Base, ContentMixin, TimestampMixin, ORMSearchable, #: when the membership started since: 'Column[str | None]' = Column(Text, nullable=True) + @property + def ts_score(self) -> int: + return 3 + @property def siblings_by_agency(self) -> 'Query[Self]': """ Returns a query that includes all siblings by agency, including diff --git a/src/onegov/people/models/person.py b/src/onegov/people/models/person.py index 0785dc24b1..9fbce4dafb 100644 --- a/src/onegov/people/models/person.py +++ b/src/onegov/people/models/person.py @@ -148,6 +148,10 @@ def spoken_title(self) -> str: #: some remarks about the person notes: 'Column[str | None]' = Column(Text, nullable=True) + @property + def ts_score(self) -> int: + return 3 + if TYPE_CHECKING: # FIXME: Replace with explicit backref with back_populates memberships: relationship[AppenderQuery[AgencyMembership]] diff --git a/src/onegov/search/mixins.py b/src/onegov/search/mixins.py index 96a25cc7ef..e9c3d8f62d 100644 --- a/src/onegov/search/mixins.py +++ b/src/onegov/search/mixins.py @@ -177,8 +177,12 @@ def es_tags(self) -> list[str] | None: @property def ts_score(self) -> int: - """ Returns the score of the full text search. """ - return 10 + """ Returns the score for the full text search. The lower the scroe + the higher the object will be ranked in the search results. + """ + + raise NotImplementedError('The ts_score property must be implemented ' + 'in {}'.format(self.__class__.__name__)) class ORMSearchable(Searchable): diff --git a/src/onegov/ticket/model.py b/src/onegov/ticket/model.py index 538f3e9b79..eaebc570bf 100644 --- a/src/onegov/ticket/model.py +++ b/src/onegov/ticket/model.py @@ -105,6 +105,10 @@ class Ticket(Base, TimestampMixin, ORMSearchable): #: true if the notifications for this ticket should be muted muted: 'Column[bool]' = Column(Boolean, nullable=False, default=False) + @property + def ts_score(self) -> int: + return 6 + if TYPE_CHECKING: created: Column[datetime] else: diff --git a/src/onegov/translator_directory/models/translator.py b/src/onegov/translator_directory/models/translator.py index 67091273db..108a545fce 100644 --- a/src/onegov/translator_directory/models/translator.py +++ b/src/onegov/translator_directory/models/translator.py @@ -220,6 +220,10 @@ class Translator(Base, TimestampMixin, AssociatedFiles, ContentMixin, expertise_professional_guilds_other: 'dict_property[Sequence[str]]' expertise_professional_guilds_other = meta_property(default=tuple) + @property + def ts_score(self) -> int: + return 4 + @property def expertise_professional_guilds_all(self) -> 'Sequence[str]': return ( diff --git a/src/onegov/user/models/user.py b/src/onegov/user/models/user.py index 423b9a0fc9..f2d8d5537a 100644 --- a/src/onegov/user/models/user.py +++ b/src/onegov/user/models/user.py @@ -152,6 +152,10 @@ def userprofile(self) -> list[str]: signup_token: 'Column[str | None]' = Column( Text, nullable=True, default=None) + @property + def ts_score(self) -> int: + return 5 + __table_args__ = ( Index('lowercase_username', func.lower(username), unique=True), UniqueConstraint('source', 'source_id', name='unique_source_id'), From 3beab01071d4671f828d272e73311109b23101b0 Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Tue, 16 Jul 2024 15:41:09 +0200 Subject: [PATCH 06/44] Person title and user title and userprofile are now hybrid properties --- src/onegov/people/models/person.py | 5 ++++- src/onegov/user/models/user.py | 3 ++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/onegov/people/models/person.py b/src/onegov/people/models/person.py index 0aef7c8975..844e11d776 100644 --- a/src/onegov/people/models/person.py +++ b/src/onegov/people/models/person.py @@ -1,3 +1,5 @@ +from sqlalchemy.ext.hybrid import hybrid_property + from onegov.core.orm import Base from onegov.core.orm.mixins import ContentMixin from onegov.core.orm.mixins import TimestampMixin @@ -55,7 +57,8 @@ class Person(Base, ContentMixin, TimestampMixin, ORMSearchable, def es_suggestion(self) -> tuple[str, ...]: return (self.title, f'{self.first_name} {self.last_name}') - @property + # @property + @hybrid_property def title(self) -> str: """ Returns the Eastern-ordered name. """ diff --git a/src/onegov/user/models/user.py b/src/onegov/user/models/user.py index cde37d3719..f1debd58d3 100644 --- a/src/onegov/user/models/user.py +++ b/src/onegov/user/models/user.py @@ -59,7 +59,8 @@ class User(Base, TimestampMixin, ORMSearchable): def es_suggestion(self) -> tuple[str, str]: return (self.realname or self.username, self.username) - @property + # @property + @hybrid_property def userprofile(self) -> list[str]: if not self.data: return [] From da09357cbd0424950e16d70affe8f830c57b5f01 Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Tue, 16 Jul 2024 15:42:45 +0200 Subject: [PATCH 07/44] Rework ranking --- src/onegov/org/models/search.py | 94 ++++++++++--------- src/onegov/people/models/person.py | 1 - src/onegov/town6/templates/search.pt | 4 +- src/onegov/town6/templates/search_postgres.pt | 4 +- src/onegov/user/models/user.py | 1 - 5 files changed, 55 insertions(+), 49 deletions(-) diff --git a/src/onegov/org/models/search.py b/src/onegov/org/models/search.py index 776441916d..3e08ebd33a 100644 --- a/src/onegov/org/models/search.py +++ b/src/onegov/org/models/search.py @@ -1,4 +1,3 @@ -from datetime import datetime from operator import attrgetter from elasticsearch_dsl.function import SF @@ -7,8 +6,6 @@ from elasticsearch_dsl.query import MatchPhrase from elasticsearch_dsl.query import MultiMatch from functools import cached_property - -from pytz import utc from sqlalchemy import func from onegov.core.collection import Pagination, _M @@ -33,7 +30,7 @@ class Search(Pagination[_M]): def __init__(self, request: 'OrgRequest', query: str, page: int) -> None: super().__init__(page) self.request = request - self.query = query + self.web_search = query @cached_property def available_documents(self) -> int: @@ -46,13 +43,13 @@ def explain(self) -> bool: @property def q(self) -> str: - return self.query + return self.web_search def __eq__(self, other: object) -> bool: return ( isinstance(other, self.__class__) and self.page == other.page - and self.query == other.query + and self.web_search == other.web_search ) if TYPE_CHECKING: @@ -67,11 +64,11 @@ def page_index(self) -> int: return self.page def page_by_index(self, index: int) -> 'Search[_M]': - return Search(self.request, self.query, index) + return Search(self.request, self.web_search, index) @cached_property def batch(self) -> 'Response | None': # type:ignore[override] - if not self.query: + if not self.web_search: return None search = self.request.app.es_search_by_request( @@ -81,7 +78,7 @@ def batch(self) -> 'Response | None': # type:ignore[override] # queries need to be cut at some point to make sure we're not # pushing the elasticsearch cluster to the brink - query = self.query[:self.max_query_length] + query = self.web_search[:self.max_query_length] if query.startswith('#'): search = self.hashtag_search(search, query) @@ -170,7 +167,7 @@ def subset_count(self) -> int: def suggestions(self) -> tuple[str, ...]: return tuple(self.request.app.es_suggestions_by_request( - self.request, self.query + self.request, self.web_search )) @@ -189,7 +186,7 @@ class SearchPostgres(Pagination): def __init__(self, request: 'OrgRequest', query: str, page: int): self.request = request - self.query = query + self.web_search = query self.page = page # page index self.nbr_of_docs = 0 @@ -209,12 +206,12 @@ def available_results(self) -> int: @property def q(self) -> str: - return self.query + return self.web_search def __eq__(self, other: object) -> bool: if not isinstance(other, SearchPostgres): return NotImplemented - return self.page == other.page and self.query == other.query + return self.page == other.page and self.web_search == other.web_search def subset(self): return self.batch @@ -224,14 +221,14 @@ def page_index(self) -> int: return self.page def page_by_index(self, index: int): - return SearchPostgres(self.request, self.query, index) + return SearchPostgres(self.request, self.web_search, index) @cached_property def batch(self): - if not self.query: + if not self.web_search: return None - if self.query.startswith('#'): + if self.web_search.startswith('#'): results = self.hashtag_search() else: results = self.generic_search() @@ -257,11 +254,36 @@ def load_batch_results(self): sorted_events = sorted(events, key=lambda e: e.latest_occurrence.start) return sorted_events + non_events + def _create_weighted_vector(self, model, language='simple'): + # for now weight the first field with 'A', the rest with 'B' + weighted_vector = [ + func.setweight( + func.to_tsvector( + language, + getattr(model, field, '') + ), + weight + ) + for field, weight in zip(model.es_properties.keys(), 'ABBBBBBBBBB') + if not field.startswith('es_') # TODO: rename to fts_ + ] + + # combine all weighted vectors + if weighted_vector: + combined_vector = weighted_vector[0] + for vector in weighted_vector[1:]: + combined_vector = combined_vector.op('||')(vector) + else: + combined_vector = func.to_tsvector(language, '') + + return combined_vector + def generic_search(self): doc_count = 0 results = [] - language = locale_mapping(self.request.locale) + ts_query = func.websearch_to_tsquery(language, self.web_search) + for base in self.request.app.session_manager.bases: for model in searchable_sqlalchemy_models(base): if model.es_public or self.request.is_logged_in: @@ -269,41 +291,27 @@ def generic_search(self): if query.count(): doc_count += query.count() - query = query.filter( - model.fts_idx.op('@@')(func.websearch_to_tsquery( - language, self.query)) + vector = self._create_weighted_vector(model, language) + rank_expression = func.ts_rank( + vector, + ts_query, + 0 # normalization, ignore document length ) - query = query.order_by( - func.ts_rank_cd( - model.fts_idx, - func.websearch_to_tsquery( - language, - self.query) - ) + query = query.filter( + model.fts_idx.op('@@')(ts_query) ) - results.extend(query.all()) + query = query.order_by(rank_expression.desc()) + res = query.all() + results.extend(res) self.nbr_of_docs = doc_count self.nbr_of_results = len(results) # remove duplicates - results = list(set(results)) - - # sort items after ts_score, modified and created. If no timestamp - # is available, use default time - default_time = utc.localize( - datetime.datetime(1970, 1, 1)) - return sorted( - results, - key=lambda k: ( - k.get('ts_score', 10), - k.get('modified') or default_time, - k.get('created') or default_time, - ), - reverse=False) + return tuple(set(results)) def hashtag_search(self): - q = self.query.lstrip('#') + q = self.web_search.lstrip('#') results = [] for model in searchable_sqlalchemy_models(Base): diff --git a/src/onegov/people/models/person.py b/src/onegov/people/models/person.py index 844e11d776..d4ee90962d 100644 --- a/src/onegov/people/models/person.py +++ b/src/onegov/people/models/person.py @@ -57,7 +57,6 @@ class Person(Base, ContentMixin, TimestampMixin, ORMSearchable, def es_suggestion(self) -> tuple[str, ...]: return (self.title, f'{self.first_name} {self.last_name}') - # @property @hybrid_property def title(self) -> str: """ Returns the Eastern-ordered name. """ diff --git a/src/onegov/town6/templates/search.pt b/src/onegov/town6/templates/search.pt index d1b5291270..f74f9ec927 100644 --- a/src/onegov/town6/templates/search.pt +++ b/src/onegov/town6/templates/search.pt @@ -15,7 +15,7 @@
- +
- +

${resultslabel}

diff --git a/src/onegov/town6/templates/search_postgres.pt b/src/onegov/town6/templates/search_postgres.pt index eee07c1e56..2b6e190e31 100644 --- a/src/onegov/town6/templates/search_postgres.pt +++ b/src/onegov/town6/templates/search_postgres.pt @@ -16,7 +16,7 @@
- +
- +

${resultslabel}

diff --git a/src/onegov/user/models/user.py b/src/onegov/user/models/user.py index f1debd58d3..d67056828f 100644 --- a/src/onegov/user/models/user.py +++ b/src/onegov/user/models/user.py @@ -59,7 +59,6 @@ class User(Base, TimestampMixin, ORMSearchable): def es_suggestion(self) -> tuple[str, str]: return (self.realname or self.username, self.username) - # @property @hybrid_property def userprofile(self) -> list[str]: if not self.data: From db8ae9bcaaa750706bc0484a48c872ebd5cd9e86 Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Thu, 18 Jul 2024 08:15:00 +0200 Subject: [PATCH 08/44] Cleanup --- src/onegov/activity/models/attendee.py | 4 ---- src/onegov/directory/models/directory.py | 4 ---- src/onegov/directory/models/directory_entry.py | 4 ---- src/onegov/event/models/event.py | 4 ---- src/onegov/feriennet/models/activity.py | 4 ---- src/onegov/file/models/file.py | 4 ---- src/onegov/form/models/definition.py | 4 ---- src/onegov/fsi/models/course.py | 4 ---- src/onegov/newsletter/models.py | 4 ---- src/onegov/org/models/external_link.py | 4 ---- src/onegov/page/model.py | 4 ---- src/onegov/people/models/membership.py | 4 ---- src/onegov/people/models/person.py | 4 ---- src/onegov/search/mixins.py | 11 +---------- src/onegov/ticket/model.py | 4 ---- src/onegov/translator_directory/models/translator.py | 4 ---- src/onegov/user/models/user.py | 4 ---- 17 files changed, 1 insertion(+), 74 deletions(-) diff --git a/src/onegov/activity/models/attendee.py b/src/onegov/activity/models/attendee.py index d135a28bd9..86cb1d3cf1 100644 --- a/src/onegov/activity/models/attendee.py +++ b/src/onegov/activity/models/attendee.py @@ -67,10 +67,6 @@ class Attendee(Base, TimestampMixin, ORMSearchable): } es_public = False - @property - def ts_score(self) -> int: - return 3 - @property def es_suggestion(self) -> str: return self.name diff --git a/src/onegov/directory/models/directory.py b/src/onegov/directory/models/directory.py index a1dfd1b688..47e1d7cb4e 100644 --- a/src/onegov/directory/models/directory.py +++ b/src/onegov/directory/models/directory.py @@ -111,10 +111,6 @@ class Directory(Base, ContentMixin, TimestampMixin, 'lead': {'type': 'localized'} } - @property - def ts_score(self) -> int: - return 7 - @property def es_public(self) -> bool: return False # to be overridden downstream diff --git a/src/onegov/directory/models/directory_entry.py b/src/onegov/directory/models/directory_entry.py index 1fa7ee408b..acb6ecdc7a 100644 --- a/src/onegov/directory/models/directory_entry.py +++ b/src/onegov/directory/models/directory_entry.py @@ -41,10 +41,6 @@ class DirectoryEntry(Base, ContentMixin, CoordinatesMixin, TimestampMixin, 'text': {'type': 'localized_html'} } - @property - def ts_score(self) -> int: - return 4 - @property def es_public(self) -> bool: return False # to be overridden downstream diff --git a/src/onegov/event/models/event.py b/src/onegov/event/models/event.py index a686873e9a..72b3d977f8 100644 --- a/src/onegov/event/models/event.py +++ b/src/onegov/event/models/event.py @@ -137,10 +137,6 @@ class Event(Base, OccurrenceMixin, TimestampMixin, SearchableContent, EventFile, 'pdf', 'one-to-one', uselist=False, backref_suffix='pdf' ) - @property - def ts_score(self) -> int: - return 1 - def set_image( self, content: bytes | IO[bytes] | None, diff --git a/src/onegov/feriennet/models/activity.py b/src/onegov/feriennet/models/activity.py index adb28b700e..890ea6125e 100644 --- a/src/onegov/feriennet/models/activity.py +++ b/src/onegov/feriennet/models/activity.py @@ -32,10 +32,6 @@ class VacationActivity(Activity, CoordinatesExtension, SearchableContent): 'organiser': {'type': 'text'} } - @property - def ts_score(self) -> int: - return 1 - @property def es_public(self) -> bool: return self.state == 'accepted' diff --git a/src/onegov/file/models/file.py b/src/onegov/file/models/file.py index 5a3a030270..dd2f073fcc 100644 --- a/src/onegov/file/models/file.py +++ b/src/onegov/file/models/file.py @@ -109,10 +109,6 @@ class SearchableFile(ORMSearchable): name: Column[str] published: Column[bool] - @property - def ts_score(self) -> int: - return 10 - @property def es_suggestion(self) -> str: return self.name diff --git a/src/onegov/form/models/definition.py b/src/onegov/form/models/definition.py index 75b462905a..5527470746 100644 --- a/src/onegov/form/models/definition.py +++ b/src/onegov/form/models/definition.py @@ -154,10 +154,6 @@ def id(self) -> str: 'polymorphic_identity': 'generic' } - @property - def ts_score(self) -> int: - return 7 - @property def form_class(self) -> Type['Form']: """ Parses the form definition and returns a form class. """ diff --git a/src/onegov/fsi/models/course.py b/src/onegov/fsi/models/course.py index 723d30fbd8..6ed1f19c9f 100644 --- a/src/onegov/fsi/models/course.py +++ b/src/onegov/fsi/models/course.py @@ -62,10 +62,6 @@ class Course(Base, ORMSearchable): lazy='dynamic' ) - @property - def ts_score(self) -> int: - return 2 - @property def title(self) -> str: return self.name diff --git a/src/onegov/newsletter/models.py b/src/onegov/newsletter/models.py index 779a953566..266a9f916e 100644 --- a/src/onegov/newsletter/models.py +++ b/src/onegov/newsletter/models.py @@ -95,10 +95,6 @@ def validate_name(self, key: str, name: str) -> str: secondary=newsletter_recipients, back_populates='newsletters') - @property - def ts_score(self) -> int: - return 6 - @property def open_recipients(self) -> tuple['Recipient', ...]: received = select([newsletter_recipients.c.recipient_id]).where( diff --git a/src/onegov/org/models/external_link.py b/src/onegov/org/models/external_link.py index 5c68fef630..25ea34581f 100644 --- a/src/onegov/org/models/external_link.py +++ b/src/onegov/org/models/external_link.py @@ -59,10 +59,6 @@ class ExternalLink(Base, ContentMixin, TimestampMixin, AccessExtension, lead: dict_property[str | None] = meta_property() - @property - def ts_score(self) -> int: - return 8 - @observes('title') def title_observer(self, title: str) -> None: self.order = normalize_for_url(title) diff --git a/src/onegov/page/model.py b/src/onegov/page/model.py index 718a88ff53..659a2fa59b 100644 --- a/src/onegov/page/model.py +++ b/src/onegov/page/model.py @@ -27,10 +27,6 @@ class Page(AdjacencyList, ContentMixin, TimestampMixin, __tablename__ = 'pages' - @property - def ts_score(self) -> int: - return 2 - if TYPE_CHECKING: # we override these relationships to be more specific parent: relationship['Page | None'] diff --git a/src/onegov/people/models/membership.py b/src/onegov/people/models/membership.py index ed30121be1..4f7ea917c7 100644 --- a/src/onegov/people/models/membership.py +++ b/src/onegov/people/models/membership.py @@ -94,10 +94,6 @@ class AgencyMembership(Base, ContentMixin, TimestampMixin, ORMSearchable, #: when the membership started since: 'Column[str | None]' = Column(Text, nullable=True) - @property - def ts_score(self) -> int: - return 3 - @property def siblings_by_agency(self) -> 'Query[Self]': """ Returns a query that includes all siblings by agency, including diff --git a/src/onegov/people/models/person.py b/src/onegov/people/models/person.py index d4ee90962d..d24525788a 100644 --- a/src/onegov/people/models/person.py +++ b/src/onegov/people/models/person.py @@ -164,10 +164,6 @@ def spoken_title(self) -> str: lazy='dynamic', ) - @property - def ts_score(self) -> int: - return 3 - def vcard_object( self, exclude: 'Collection[str] | None' = None, diff --git a/src/onegov/search/mixins.py b/src/onegov/search/mixins.py index e9c3d8f62d..2dd67eef25 100644 --- a/src/onegov/search/mixins.py +++ b/src/onegov/search/mixins.py @@ -52,7 +52,7 @@ def es_type_name(self): """ TEXT_SEARCH_COLUMN_NAME = 'fts_idx' - TEXT_SEARCH_DATA_COLUMN_NAME = 'fts_idx_data' + # TEXT_SEARCH_DATA_COLUMN_NAME = 'fts_idx_data' if TYPE_CHECKING: # NOTE: This doesn't really have a Python representation, unless @@ -175,15 +175,6 @@ def es_tags(self) -> list[str] | None: """ Returns a list of tags associated with this content. """ return None - @property - def ts_score(self) -> int: - """ Returns the score for the full text search. The lower the scroe - the higher the object will be ranked in the search results. - """ - - raise NotImplementedError('The ts_score property must be implemented ' - 'in {}'.format(self.__class__.__name__)) - class ORMSearchable(Searchable): """ Extends the default :class:`Searchable` class with sensible defaults diff --git a/src/onegov/ticket/model.py b/src/onegov/ticket/model.py index 33c2f25032..88333e6ca4 100644 --- a/src/onegov/ticket/model.py +++ b/src/onegov/ticket/model.py @@ -105,10 +105,6 @@ class Ticket(Base, TimestampMixin, ORMSearchable): #: true if the notifications for this ticket should be muted muted: 'Column[bool]' = Column(Boolean, nullable=False, default=False) - @property - def ts_score(self) -> int: - return 6 - if TYPE_CHECKING: created: Column[datetime] else: diff --git a/src/onegov/translator_directory/models/translator.py b/src/onegov/translator_directory/models/translator.py index fdb73e4f45..786f83f0db 100644 --- a/src/onegov/translator_directory/models/translator.py +++ b/src/onegov/translator_directory/models/translator.py @@ -210,10 +210,6 @@ class Translator(Base, TimestampMixin, AssociatedFiles, ContentMixin, expertise_professional_guilds_other: 'dict_property[Sequence[str]]' expertise_professional_guilds_other = meta_property(default=tuple) - @property - def ts_score(self) -> int: - return 4 - @property def expertise_professional_guilds_all(self) -> 'Sequence[str]': return ( diff --git a/src/onegov/user/models/user.py b/src/onegov/user/models/user.py index d67056828f..071ca20508 100644 --- a/src/onegov/user/models/user.py +++ b/src/onegov/user/models/user.py @@ -152,10 +152,6 @@ def userprofile(self) -> list[str]: signup_token: 'Column[str | None]' = Column( Text, nullable=True, default=None) - @property - def ts_score(self) -> int: - return 5 - __table_args__ = ( Index('lowercase_username', func.lower(username), unique=True), UniqueConstraint('source', 'source_id', name='unique_source_id'), From f9ba776eafc772eb311077cbe97b8499c217f26a Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Thu, 18 Jul 2024 12:01:35 +0200 Subject: [PATCH 09/44] Make tickets searchable --- src/onegov/org/models/ticket.py | 2 ++ src/onegov/ticket/model.py | 8 +++++--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/onegov/org/models/ticket.py b/src/onegov/org/models/ticket.py index 6606d9c793..a865991505 100644 --- a/src/onegov/org/models/ticket.py +++ b/src/onegov/org/models/ticket.py @@ -1,5 +1,7 @@ from functools import cached_property from markupsafe import Markup +from sqlalchemy.ext.hybrid import hybrid_property + from onegov.chat.collections import ChatCollection from onegov.core.templates import render_macro from onegov.directory import Directory, DirectoryEntry diff --git a/src/onegov/ticket/model.py b/src/onegov/ticket/model.py index 88333e6ca4..d463d47a80 100644 --- a/src/onegov/ticket/model.py +++ b/src/onegov/ticket/model.py @@ -1,3 +1,5 @@ +from sqlalchemy.ext.hybrid import hybrid_property + from onegov.core.orm import Base from onegov.core.orm.mixins import TimestampMixin from onegov.core.orm.types import JSON, UUID @@ -152,7 +154,7 @@ def created(cls) -> 'Column[datetime]': 'extra_localized_text': {'type': 'localized'} } - @property + @hybrid_property def extra_localized_text(self) -> str | None: """ Maybe used by child-classes to return localized extra data that should be indexed as well. @@ -167,14 +169,14 @@ def es_suggestion(self) -> list[str]: self.number.replace('-', '') ] - @property + @hybrid_property def ticket_email(self) -> str | None: if self.handler.deleted: return self.snapshot.get('email') else: return self.handler.email - @property + @hybrid_property def ticket_data(self) -> 'Sequence[str] | None': if self.handler.deleted: return self.snapshot.get('summary') From 57187326ce6224a53d3c69d176715154aea2644f Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Thu, 18 Jul 2024 12:04:26 +0200 Subject: [PATCH 10/44] Make tickets searchable --- src/onegov/org/models/ticket.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/onegov/org/models/ticket.py b/src/onegov/org/models/ticket.py index a865991505..8fdb7bed3b 100644 --- a/src/onegov/org/models/ticket.py +++ b/src/onegov/org/models/ticket.py @@ -1,6 +1,5 @@ from functools import cached_property from markupsafe import Markup -from sqlalchemy.ext.hybrid import hybrid_property from onegov.chat.collections import ChatCollection from onegov.core.templates import render_macro @@ -18,6 +17,7 @@ from purl import URL from sqlalchemy import desc from sqlalchemy import func +from sqlalchemy.ext.hybrid import hybrid_property from sqlalchemy.orm import object_session @@ -69,6 +69,7 @@ def reference(self, request: 'OrgRequest') -> str: def reference_group(self, request: 'OrgRequest') -> str: return request.translate(self.group) + @hybrid_property @cached_property def extra_localized_text(self) -> str: From 737fb142db00eae816727411c4d8410ee4c84211 Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Thu, 18 Jul 2024 15:22:38 +0200 Subject: [PATCH 11/44] makes directory entries searchable --- .../directory/models/directory_entry.py | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/onegov/directory/models/directory_entry.py b/src/onegov/directory/models/directory_entry.py index acb6ecdc7a..3553b2ab3e 100644 --- a/src/onegov/directory/models/directory_entry.py +++ b/src/onegov/directory/models/directory_entry.py @@ -1,3 +1,5 @@ +from sqlalchemy.ext.hybrid import hybrid_property + from onegov.core.orm import Base from onegov.core.orm.mixins import ContentMixin from onegov.core.orm.mixins import TimestampMixin @@ -6,7 +8,7 @@ from onegov.file import AssociatedFiles from onegov.gis import CoordinatesMixin from onegov.search import SearchableContent -from sqlalchemy import Column +from sqlalchemy import Column, cast from sqlalchemy import ForeignKey from sqlalchemy import Index from sqlalchemy import Text @@ -30,10 +32,10 @@ class DirectoryEntry(Base, ContentMixin, CoordinatesMixin, TimestampMixin, __tablename__ = 'directory_entries' es_properties = { - 'keywords': {'type': 'keyword'}, 'title': {'type': 'localized'}, 'lead': {'type': 'localized'}, - 'directory_id': {'type': 'keyword'}, + 'keywords_as_text': {'type': 'keyword'}, + '_directory_id': {'type': 'keyword'}, # since the searchable text might include html, we remove it # even if there's no html -> possibly decreasing the search @@ -43,6 +45,7 @@ class DirectoryEntry(Base, ContentMixin, CoordinatesMixin, TimestampMixin, @property def es_public(self) -> bool: + return True # for tests return False # to be overridden downstream #: An interal id for references (not public) @@ -109,6 +112,10 @@ def external_link_title(self) -> str | None: def external_link_visible(self) -> bool | None: return self.directory.configuration.link_visible + @hybrid_property + def _directory_id(self): + return cast(self.directory_id, Text) + @property def directory_name(self) -> str: return self.directory.name @@ -117,13 +124,18 @@ def directory_name(self) -> str: def keywords(self) -> set[str]: return set(self._keywords.keys()) if self._keywords else set() + # @property + @hybrid_property + def keywords_as_text(self) -> str: + return cast(self._keywords.keys(), Text) + # FIXME: asymmetric properties are not supported by mypy, switch to # a custom descriptor, if desired. @keywords.setter def keywords(self, value: 'Collection[str] | None') -> None: self._keywords = dict.fromkeys(value, '') if value else None - @property + @hybrid_property def text(self) -> str: return self.directory.configuration.extract_searchable(self.values) From 78465399d4657abe15a395cdc3bf1b0e1564c887 Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Thu, 18 Jul 2024 15:56:52 +0200 Subject: [PATCH 12/44] Revert "makes directory entries searchable" This reverts commit 737fb142db00eae816727411c4d8410ee4c84211. --- .../directory/models/directory_entry.py | 20 ++++--------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/src/onegov/directory/models/directory_entry.py b/src/onegov/directory/models/directory_entry.py index 3553b2ab3e..acb6ecdc7a 100644 --- a/src/onegov/directory/models/directory_entry.py +++ b/src/onegov/directory/models/directory_entry.py @@ -1,5 +1,3 @@ -from sqlalchemy.ext.hybrid import hybrid_property - from onegov.core.orm import Base from onegov.core.orm.mixins import ContentMixin from onegov.core.orm.mixins import TimestampMixin @@ -8,7 +6,7 @@ from onegov.file import AssociatedFiles from onegov.gis import CoordinatesMixin from onegov.search import SearchableContent -from sqlalchemy import Column, cast +from sqlalchemy import Column from sqlalchemy import ForeignKey from sqlalchemy import Index from sqlalchemy import Text @@ -32,10 +30,10 @@ class DirectoryEntry(Base, ContentMixin, CoordinatesMixin, TimestampMixin, __tablename__ = 'directory_entries' es_properties = { + 'keywords': {'type': 'keyword'}, 'title': {'type': 'localized'}, 'lead': {'type': 'localized'}, - 'keywords_as_text': {'type': 'keyword'}, - '_directory_id': {'type': 'keyword'}, + 'directory_id': {'type': 'keyword'}, # since the searchable text might include html, we remove it # even if there's no html -> possibly decreasing the search @@ -45,7 +43,6 @@ class DirectoryEntry(Base, ContentMixin, CoordinatesMixin, TimestampMixin, @property def es_public(self) -> bool: - return True # for tests return False # to be overridden downstream #: An interal id for references (not public) @@ -112,10 +109,6 @@ def external_link_title(self) -> str | None: def external_link_visible(self) -> bool | None: return self.directory.configuration.link_visible - @hybrid_property - def _directory_id(self): - return cast(self.directory_id, Text) - @property def directory_name(self) -> str: return self.directory.name @@ -124,18 +117,13 @@ def directory_name(self) -> str: def keywords(self) -> set[str]: return set(self._keywords.keys()) if self._keywords else set() - # @property - @hybrid_property - def keywords_as_text(self) -> str: - return cast(self._keywords.keys(), Text) - # FIXME: asymmetric properties are not supported by mypy, switch to # a custom descriptor, if desired. @keywords.setter def keywords(self, value: 'Collection[str] | None') -> None: self._keywords = dict.fromkeys(value, '') if value else None - @hybrid_property + @property def text(self) -> str: return self.directory.configuration.extract_searchable(self.values) From cfcd6af967a920f32e669cf4e45384713bbab46e Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Fri, 19 Jul 2024 08:45:06 +0200 Subject: [PATCH 13/44] Resolve merge conflict --- src/onegov/directory/models/directory_entry.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/onegov/directory/models/directory_entry.py b/src/onegov/directory/models/directory_entry.py index acb6ecdc7a..42860ddc6a 100644 --- a/src/onegov/directory/models/directory_entry.py +++ b/src/onegov/directory/models/directory_entry.py @@ -30,9 +30,9 @@ class DirectoryEntry(Base, ContentMixin, CoordinatesMixin, TimestampMixin, __tablename__ = 'directory_entries' es_properties = { - 'keywords': {'type': 'keyword'}, 'title': {'type': 'localized'}, 'lead': {'type': 'localized'}, + 'keywords': {'type': 'keyword'}, 'directory_id': {'type': 'keyword'}, # since the searchable text might include html, we remove it From 904a32cfdc64d1e3952d4984c842f96ada467809 Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Fri, 19 Jul 2024 16:12:38 +0200 Subject: [PATCH 14/44] Make mypy almost happy --- src/onegov/agency/views/search.py | 5 +- .../directory/models/directory_entry.py | 4 +- src/onegov/fsi/views/search.py | 12 +-- src/onegov/landsgemeinde/views/search.py | 6 +- .../onboarding/models/town_assistant.py | 2 +- src/onegov/org/layout.py | 6 +- src/onegov/org/models/search.py | 76 +++++++++++-------- src/onegov/org/models/ticket.py | 3 +- src/onegov/org/path.py | 11 ++- src/onegov/org/views/search.py | 9 ++- src/onegov/search/integration.py | 5 +- src/onegov/ticket/model.py | 3 +- src/onegov/town6/views/search.py | 2 +- 13 files changed, 86 insertions(+), 58 deletions(-) diff --git a/src/onegov/agency/views/search.py b/src/onegov/agency/views/search.py index 39c98bd8a4..32dffd04eb 100644 --- a/src/onegov/agency/views/search.py +++ b/src/onegov/agency/views/search.py @@ -25,7 +25,10 @@ def search( @AgencyApp.html(model=SearchPostgres, template='search.pt', permission=Public) -def agency_search_postgres(self, request): +def agency_search_postgres( + self: SearchPostgres['Base'], + request: 'AgencyRequest' +) -> 'RenderData | Response': data = search_postgres(self, request) if isinstance(data, dict): data['layout'] = AgencySearchLayout(self, request) diff --git a/src/onegov/directory/models/directory_entry.py b/src/onegov/directory/models/directory_entry.py index 42860ddc6a..6bcc211012 100644 --- a/src/onegov/directory/models/directory_entry.py +++ b/src/onegov/directory/models/directory_entry.py @@ -1,3 +1,5 @@ +from sqlalchemy.ext.hybrid import hybrid_property + from onegov.core.orm import Base from onegov.core.orm.mixins import ContentMixin from onegov.core.orm.mixins import TimestampMixin @@ -123,7 +125,7 @@ def keywords(self) -> set[str]: def keywords(self, value: 'Collection[str] | None') -> None: self._keywords = dict.fromkeys(value, '') if value else None - @property + @hybrid_property def text(self) -> str: return self.directory.configuration.extract_searchable(self.values) diff --git a/src/onegov/fsi/views/search.py b/src/onegov/fsi/views/search.py index 1405aa99b6..7860364182 100644 --- a/src/onegov/fsi/views/search.py +++ b/src/onegov/fsi/views/search.py @@ -25,9 +25,9 @@ def search( @FsiApp.html(model=SearchPostgres, template='search.pt', permission=Personal) -def search( - self: Search['Base'], - request: 'FsiRequest' +def search_postgres( + self: SearchPostgres['Base'], + request: 'FsiRequest' ) -> 'RenderData | Response': return search_postgres_view(self, request) @@ -41,8 +41,8 @@ def suggestions( @FsiApp.json(model=SearchPostgres, name='suggest', permission=Personal) -def suggestions( - self: Search['Base'], - request: 'FsiRequest' +def suggestions_postgres( + self: SearchPostgres['Base'], + request: 'FsiRequest' ) -> 'JSON_ro': return suggestions_postgres_view(self, request) diff --git a/src/onegov/landsgemeinde/views/search.py b/src/onegov/landsgemeinde/views/search.py index c7d8b9c908..acd5d05ff2 100644 --- a/src/onegov/landsgemeinde/views/search.py +++ b/src/onegov/landsgemeinde/views/search.py @@ -22,8 +22,8 @@ def landsgemeinde_search( @LandsgemeindeApp.html(model=SearchPostgres, template='search.pt', permission=Public) -def landsgemeinde_search( - self: Search['Base'], - request: 'LandsgemeindeRequest' +def landsgemeinde_search_postgres( + self: SearchPostgres['Base'], + request: 'LandsgemeindeRequest' ) -> 'RenderData | Response': return search_postgres(self, request, DefaultLayout(self, request)) diff --git a/src/onegov/onboarding/models/town_assistant.py b/src/onegov/onboarding/models/town_assistant.py index 87fcc47e51..3e8a9b7189 100644 --- a/src/onegov/onboarding/models/town_assistant.py +++ b/src/onegov/onboarding/models/town_assistant.py @@ -202,7 +202,7 @@ def add_town( 'org': name }) - self.app.es_perform_reindex() + self.app.perform_reindex() self.app.send_transactional_email( subject=title, receivers=(user, ), diff --git a/src/onegov/org/layout.py b/src/onegov/org/layout.py index a172cf4130..7f3678db25 100644 --- a/src/onegov/org/layout.py +++ b/src/onegov/org/layout.py @@ -336,7 +336,8 @@ def homepage_url(self) -> str: def search_url(self) -> str: """ Returns the url to the search page. """ # Allows using postgres search while es search remains default - if 'search_postgres' in self.request.path_info: + if (self.request.path_info + and 'search-postgres' in self.request.path_info): return self.request.class_link(SearchPostgres) return self.request.class_link(Search) @@ -344,7 +345,8 @@ def search_url(self) -> str: def suggestions_url(self) -> str: """ Returns the url to the suggestions json view. """ # Allows using postgres search while es search remains default - if 'search_postgres' in self.request.path_info: + if (self.request.path_info + and 'search-postgres' in self.request.path_info): return self.request.class_link(SearchPostgres, name='suggest') return self.request.class_link(Search, name='suggest') diff --git a/src/onegov/org/models/search.py b/src/onegov/org/models/search.py index 3e08ebd33a..d1b3c14064 100644 --- a/src/onegov/org/models/search.py +++ b/src/onegov/org/models/search.py @@ -1,5 +1,3 @@ -from operator import attrgetter - from elasticsearch_dsl.function import SF from elasticsearch_dsl.query import FunctionScore from elasticsearch_dsl.query import Match @@ -7,23 +5,21 @@ from elasticsearch_dsl.query import MultiMatch from functools import cached_property from sqlalchemy import func +from typing import TYPE_CHECKING, Any, List from onegov.core.collection import Pagination, _M from onegov.core.orm import Base from onegov.event.models import Event - - -from typing import TYPE_CHECKING - from onegov.search.utils import searchable_sqlalchemy_models + if TYPE_CHECKING: from onegov.org.request import OrgRequest + from onegov.search import Searchable from onegov.search.dsl import Hit, Response, Search as ESSearch class Search(Pagination[_M]): - results_per_page = 10 max_query_length = 100 @@ -155,7 +151,7 @@ def feeling_lucky(self) -> str | None: first_entry = self.batch[0].load() # XXX the default view to the event should be doing the redirect - if first_entry.__tablename__ == 'events': + if first_entry.es_type_name == 'events': return self.request.link(first_entry, 'latest') else: return self.request.link(first_entry) @@ -206,6 +202,10 @@ def available_results(self) -> int: @property def q(self) -> str: + """ + Returns the user's query term from the search field of the UI + + """ return self.web_search def __eq__(self, other: object) -> bool: @@ -213,20 +213,20 @@ def __eq__(self, other: object) -> bool: return NotImplemented return self.page == other.page and self.web_search == other.web_search - def subset(self): + def subset(self) -> 'List[Searchable] | None': # type:ignore[override] return self.batch @property def page_index(self) -> int: return self.page - def page_by_index(self, index: int): + def page_by_index(self, index: int) -> 'SearchPostgres[_M]': return SearchPostgres(self.request, self.web_search, index) @cached_property - def batch(self): + def batch(self) -> 'List[Searchable]': # type:ignore[override] if not self.web_search: - return None + return [] if self.web_search.startswith('#'): results = self.hashtag_search() @@ -236,14 +236,14 @@ def batch(self): return results[self.offset:self.offset + self.batch_size] @cached_property - def load_batch_results(self): + def load_batch_results(self) -> list[Any]: """Load search results and sort events by latest occurrence. This methods is a wrapper around `batch.load()`, which returns the actual search results form the query. """ - batch = self.batch - events = [] - non_events = [] + batch: List[Searchable] = self.batch + events: List[Searchable] = [] + non_events: List[Searchable] = [] for search_result in batch: if isinstance(search_result, Event): events.append(search_result) @@ -251,10 +251,16 @@ def load_batch_results(self): non_events.append(search_result) if not events: return batch - sorted_events = sorted(events, key=lambda e: e.latest_occurrence.start) + sorted_events = sorted( + events, key=lambda e: + e.latest_occurrence.start) # type:ignore[attr-defined] return sorted_events + non_events - def _create_weighted_vector(self, model, language='simple'): + def _create_weighted_vector( + self, + model: Any, + language: str = 'simple' + ) -> Any: # for now weight the first field with 'A', the rest with 'B' weighted_vector = [ func.setweight( @@ -278,10 +284,10 @@ def _create_weighted_vector(self, model, language='simple'): return combined_vector - def generic_search(self): + def generic_search(self) -> list['Searchable']: doc_count = 0 - results = [] - language = locale_mapping(self.request.locale) + results: List[Any] = [] + language = locale_mapping(self.request.locale or 'de_CH') ts_query = func.websearch_to_tsquery(language, self.web_search) for base in self.request.app.session_manager.bases: @@ -300,39 +306,45 @@ def generic_search(self): query = query.filter( model.fts_idx.op('@@')(ts_query) ) - query = query.order_by(rank_expression.desc()) + # FIXME: sorting by rank_expression does not work + # with OrgTicketMixin::extra_localized_text + # query = query.order_by(rank_expression.desc()) res = query.all() results.extend(res) + # remove duplicates + results = list(set(results)) + self.nbr_of_docs = doc_count self.nbr_of_results = len(results) - # remove duplicates - return tuple(set(results)) + return results - def hashtag_search(self): + def hashtag_search(self) -> list['Searchable']: q = self.web_search.lstrip('#') results = [] for model in searchable_sqlalchemy_models(Base): # skip certain tables for hashtag search for better performance - if model.__tablename__ not in ['attendees', 'files', 'people', - 'tickets', 'users']: + if (model.es_type_name not in ['attendees', 'files', 'people', + 'tickets', 'users']): if model.es_public or self.request.is_logged_in: for doc in self.request.session.query(model).all(): if doc.es_tags and q in doc.es_tags: results.append(doc) + # remove duplicates + results = list(set(results)) + self.nbr_of_results = len(results) - results.sort(key=attrgetter('ts_score'), reverse=False) return results def feeling_lucky(self) -> str | None: if self.batch: - first_entry = self.batch[0].load() + first_entry = self.batch[0] # XXX the default view to the event should be doing the redirect - if first_entry.__tablename__ == 'events': + if first_entry.es_type_name == 'events': return self.request.link(first_entry, 'latest') else: return self.request.link(first_entry) @@ -346,7 +358,9 @@ def suggestions(self) -> tuple[str, ...]: suggestions = [] for element in self.generic_search(): - suggest = getattr(element, 'es_suggestion', []) + if element.es_type_name == 'files': + continue + suggest = getattr(element, 'es_suggestion', '') suggestions.append(suggest) return tuple(suggestions[:15]) diff --git a/src/onegov/org/models/ticket.py b/src/onegov/org/models/ticket.py index 8fdb7bed3b..7cd9ccb5b1 100644 --- a/src/onegov/org/models/ticket.py +++ b/src/onegov/org/models/ticket.py @@ -90,8 +90,9 @@ def extra_localized_text(self) -> str: q = q.filter_by(channel_id=self.number) q = q.filter(Message.type.in_(('ticket_note', 'ticket_chat'))) q = q.with_entities(Message.text) + result = ' '.join(n.text for n in q if n.text) - return ' '.join(n.text for n in q if n.text) + return result @property def es_tags(self) -> list[str] | None: diff --git a/src/onegov/org/path.py b/src/onegov/org/path.py index b92216222f..87cf5747a3 100644 --- a/src/onegov/org/path.py +++ b/src/onegov/org/path.py @@ -65,7 +65,7 @@ from onegov.org.models import ResourcePersonMove from onegov.org.models import ResourceRecipient from onegov.org.models import ResourceRecipientCollection -from onegov.org.models import Search +from onegov.org.models import Search, SearchPostgres from onegov.org.models import SiteCollection from onegov.org.models import TicketNote from onegov.org.models import Topic @@ -77,7 +77,6 @@ from onegov.org.models.external_link import ( ExternalLinkCollection, ExternalLink) from onegov.org.models.resource import FindYourSpotCollection -from onegov.org.models.search import SearchPostgres from onegov.page import PageCollection from onegov.pay import PaymentProvider, Payment, PaymentCollection from onegov.pay import PaymentProviderCollection @@ -758,10 +757,10 @@ def get_search( @OrgApp.path(model=SearchPostgres, path='/search-postgres', converters={'page': int}) def get_search_postgres( - request: 'OrgRequest', - q: str = '', - page: int = 0 -) -> Search[Any]: + request: 'OrgRequest', + q: str = '', + page: int = 0 +) -> SearchPostgres[Any]: return SearchPostgres(request, q, page) diff --git a/src/onegov/org/views/search.py b/src/onegov/org/views/search.py index ddb719d51a..0a24f9df55 100644 --- a/src/onegov/org/views/search.py +++ b/src/onegov/org/views/search.py @@ -62,8 +62,13 @@ def search( @OrgApp.html(model=SearchPostgres, template='search_postgres.pt', permission=Public) -def search_postgres(self, request, layout=None): +def search_postgres( + self: SearchPostgres['Base'], + request: 'OrgRequest', + layout: DefaultLayout | None = None +) -> 'RenderData | Response': layout = layout or DefaultLayout(self, request) + assert isinstance(layout.breadcrumbs, list) layout.breadcrumbs.append(Link(_("Search"), '#')) try: @@ -107,7 +112,7 @@ def suggestions(self: Search['Base'], request: 'OrgRequest') -> 'JSON_ro': @OrgApp.json(model=SearchPostgres, name='suggest', permission=Public) -def suggestions_postgres(self: Search['Base'], request: 'OrgRequest') \ +def suggestions_postgres(self: SearchPostgres['Base'], request: 'OrgRequest') \ -> 'JSON_ro': try: return self.suggestions() diff --git a/src/onegov/search/integration.py b/src/onegov/search/integration.py index 1c72dbab0a..89f8c494ed 100644 --- a/src/onegov/search/integration.py +++ b/src/onegov/search/integration.py @@ -23,7 +23,8 @@ from urllib3.exceptions import HTTPError -from typing import Any, Literal, TYPE_CHECKING +from typing import Any, Literal, TYPE_CHECKING, List + if TYPE_CHECKING: from collections.abc import Callable, Iterable from datetime import datetime @@ -429,7 +430,7 @@ def perform_reindex(self, fail: bool = False) -> None: """ # prevent tables get re-indexed twice - index_done = [str] + index_done: List[str] = [] schema = self.schema index_log.info(f'Indexing schema {schema}..') diff --git a/src/onegov/ticket/model.py b/src/onegov/ticket/model.py index d463d47a80..1936151fe6 100644 --- a/src/onegov/ticket/model.py +++ b/src/onegov/ticket/model.py @@ -154,7 +154,8 @@ def created(cls) -> 'Column[datetime]': 'extra_localized_text': {'type': 'localized'} } - @hybrid_property + # @hybrid_property + @property def extra_localized_text(self) -> str | None: """ Maybe used by child-classes to return localized extra data that should be indexed as well. diff --git a/src/onegov/town6/views/search.py b/src/onegov/town6/views/search.py index 066e513ae5..c035884d9a 100644 --- a/src/onegov/town6/views/search.py +++ b/src/onegov/town6/views/search.py @@ -24,7 +24,7 @@ def town_search( @TownApp.html(model=SearchPostgres, template='search_postgres.pt', permission=Public) def town_search_postgres( - self: Search['Base'], + self: SearchPostgres['Base'], request: 'TownRequest' ) -> 'RenderData | Response': return search_postgres(self, request, DefaultLayout(self, request)) From 956befa8eae7ff3b24e768b7dd88d533e45f2d03 Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Fri, 19 Jul 2024 16:14:46 +0200 Subject: [PATCH 15/44] Add fixme's as ranking does not work with extra localized text for suggestsions --- src/onegov/org/models/search.py | 5 ++++- src/onegov/org/models/ticket.py | 6 +++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/onegov/org/models/search.py b/src/onegov/org/models/search.py index d1b3c14064..ea050420df 100644 --- a/src/onegov/org/models/search.py +++ b/src/onegov/org/models/search.py @@ -297,6 +297,9 @@ def generic_search(self) -> list['Searchable']: if query.count(): doc_count += query.count() + + # FIXME: searching and/or ranking change order of + # results list when switching to different result page vector = self._create_weighted_vector(model, language) rank_expression = func.ts_rank( vector, @@ -306,7 +309,7 @@ def generic_search(self) -> list['Searchable']: query = query.filter( model.fts_idx.op('@@')(ts_query) ) - # FIXME: sorting by rank_expression does not work + # FIXME: sorting by rank_expression does not work # with OrgTicketMixin::extra_localized_text # query = query.order_by(rank_expression.desc()) res = query.all() diff --git a/src/onegov/org/models/ticket.py b/src/onegov/org/models/ticket.py index 7cd9ccb5b1..99bc6a2c76 100644 --- a/src/onegov/org/models/ticket.py +++ b/src/onegov/org/models/ticket.py @@ -50,6 +50,8 @@ class OrgTicketMixin: """ + _cached_extra_localized_text: str + if TYPE_CHECKING: number: Column[str] group: Column[str] @@ -69,7 +71,9 @@ def reference(self, request: 'OrgRequest') -> str: def reference_group(self, request: 'OrgRequest') -> str: return request.translate(self.group) - @hybrid_property + # FIXME: extra localized text is used for suggestions in the search but + # does not work with the ranking of the search results + # @hybrid_property @cached_property def extra_localized_text(self) -> str: From b25a51dbe49d0e76a8f137400fda3bd98b206178 Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Fri, 23 Aug 2024 10:53:43 +0200 Subject: [PATCH 16/44] Make a generic class --- src/onegov/org/models/search.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/onegov/org/models/search.py b/src/onegov/org/models/search.py index ed5b14ea3d..120cab3d92 100644 --- a/src/onegov/org/models/search.py +++ b/src/onegov/org/models/search.py @@ -173,7 +173,7 @@ def locale_mapping(locale: str) -> str: return mapping.get(locale, 'english') -class SearchPostgres(Pagination): +class SearchPostgres(Pagination[_M]): """ Implements searching in postgres db based on the gin index """ From 007d8cbcc8545a6f3e4daf525231890484295984 Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Mon, 26 Aug 2024 08:44:13 +0200 Subject: [PATCH 17/44] Fix search results for tickets as not each ticket provides the values --- src/onegov/town6/templates/macros.pt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/onegov/town6/templates/macros.pt b/src/onegov/town6/templates/macros.pt index 663c17e34d..00101e4e8b 100644 --- a/src/onegov/town6/templates/macros.pt +++ b/src/onegov/town6/templates/macros.pt @@ -1785,9 +1785,9 @@ ${result.number}
    -
  • ${translate(result.group)}
    • -
  • -
  • +
  • ${translate(result.group)}
    • +
  • +
From 57f1ea4438034743a9eca9805731caec15945ae0 Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Tue, 27 Aug 2024 14:57:27 +0200 Subject: [PATCH 18/44] Transform directory_entry.keywords to hybrid property --- src/onegov/directory/models/directory_entry.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/onegov/directory/models/directory_entry.py b/src/onegov/directory/models/directory_entry.py index 6bcc211012..14bb490a02 100644 --- a/src/onegov/directory/models/directory_entry.py +++ b/src/onegov/directory/models/directory_entry.py @@ -8,7 +8,7 @@ from onegov.file import AssociatedFiles from onegov.gis import CoordinatesMixin from onegov.search import SearchableContent -from sqlalchemy import Column +from sqlalchemy import Column, func, cast, ARRAY, String from sqlalchemy import ForeignKey from sqlalchemy import Index from sqlalchemy import Text @@ -115,7 +115,7 @@ def external_link_visible(self) -> bool | None: def directory_name(self) -> str: return self.directory.name - @property + @hybrid_property def keywords(self) -> set[str]: return set(self._keywords.keys()) if self._keywords else set() @@ -125,6 +125,15 @@ def keywords(self) -> set[str]: def keywords(self, value: 'Collection[str] | None') -> None: self._keywords = dict.fromkeys(value, '') if value else None + @keywords.expression + def keywords(cls): # type:ignore[no-redef] + return func.array_to_string( + func.array_agg( + cast(func.jsonb_each_text(cls._keywords).keys(), ARRAY(String)) + ), + ' ' + ) + @hybrid_property def text(self) -> str: return self.directory.configuration.extract_searchable(self.values) From abbf86ca161e5eac271a0d8203db5492d13de4a7 Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Mon, 9 Sep 2024 09:19:46 +0200 Subject: [PATCH 19/44] Adds weighted vector to search query --- .../directory/models/directory_entry.py | 2 +- src/onegov/org/models/search.py | 28 ++++++++++--------- src/onegov/org/models/ticket.py | 21 ++++++++++---- 3 files changed, 32 insertions(+), 19 deletions(-) diff --git a/src/onegov/directory/models/directory_entry.py b/src/onegov/directory/models/directory_entry.py index 14bb490a02..eaa14c2cff 100644 --- a/src/onegov/directory/models/directory_entry.py +++ b/src/onegov/directory/models/directory_entry.py @@ -35,7 +35,7 @@ class DirectoryEntry(Base, ContentMixin, CoordinatesMixin, TimestampMixin, 'title': {'type': 'localized'}, 'lead': {'type': 'localized'}, 'keywords': {'type': 'keyword'}, - 'directory_id': {'type': 'keyword'}, + # 'directory_id': {'type': 'keyword'}, # since the searchable text might include html, we remove it # even if there's no html -> possibly decreasing the search diff --git a/src/onegov/org/models/search.py b/src/onegov/org/models/search.py index 120cab3d92..2d846b2367 100644 --- a/src/onegov/org/models/search.py +++ b/src/onegov/org/models/search.py @@ -298,26 +298,28 @@ def generic_search(self) -> list['Searchable']: if query.count(): doc_count += query.count() - # FIXME: searching and/or ranking change order of - # results list when switching to different result page vector = self._create_weighted_vector(model, language) - rank_expression = func.ts_rank( - vector, - ts_query, - 0 # normalization, ignore document length - ) + rank_expression = func.coalesce( + func.ts_rank( + vector, + ts_query, + # 0 # normalization, ignore document length + ), 0).label('rank') query = query.filter( model.fts_idx.op('@@')(ts_query) - ) - # FIXME: sorting by rank_expression does not work - # with OrgTicketMixin::extra_localized_text - # query = query.order_by(rank_expression.desc()) - res = query.all() - results.extend(res) + ).add_columns(rank_expression) + + results.extend([r for r in query.all()]) # others are not relevant # remove duplicates results = list(set(results)) + # sort by rank + results.sort(key=lambda x: x[1], reverse=True) + + # remove rank from results + results = [r[0] for r in results] + self.nbr_of_docs = doc_count self.nbr_of_results = len(results) diff --git a/src/onegov/org/models/ticket.py b/src/onegov/org/models/ticket.py index c7f8bd67e3..b769f6eb73 100644 --- a/src/onegov/org/models/ticket.py +++ b/src/onegov/org/models/ticket.py @@ -15,11 +15,10 @@ from onegov.ticket import Ticket, Handler, handlers from onegov.search.utils import extract_hashtags from purl import URL -from sqlalchemy import desc +from sqlalchemy import desc, select, and_ from sqlalchemy import func from sqlalchemy.ext.hybrid import hybrid_property -from sqlalchemy.orm import object_session - +from sqlalchemy.orm import object_session, aliased from typing import Any, TYPE_CHECKING if TYPE_CHECKING: @@ -72,9 +71,8 @@ def reference_group(self, request: 'OrgRequest') -> str: return request.translate(self.group) # FIXME: extra localized text is used for suggestions in the search but - # does not work with the ranking of the search results # @hybrid_property - @cached_property + @hybrid_property def extra_localized_text(self) -> str: # extracts of attachments are currently not searchable - if they were @@ -98,6 +96,19 @@ def extra_localized_text(self) -> str: return result + @extra_localized_text.expression + def extra_localized_text(cls): + Message = aliased(Message) # Assuming Message is your message model + subquery = ( + select(func.string_agg(Message.text, ' ')) + .where(and_( + Message.channel_id == cls.number, + Message.type.in_(['ticket_note', 'ticket_chat']) + )) + .scalar_subquery() + ) + return func.coalesce(subquery, '') + @property def es_tags(self) -> list[str] | None: if self.extra_localized_text: From ac18d1606054e661160b6d728571322d27b9c15c Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Mon, 9 Sep 2024 11:12:22 +0200 Subject: [PATCH 20/44] Fix extra localized text for tickets in search results --- src/onegov/org/models/ticket.py | 18 ++++++------------ src/onegov/ticket/model.py | 3 +-- 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/src/onegov/org/models/ticket.py b/src/onegov/org/models/ticket.py index b769f6eb73..2e880db7d4 100644 --- a/src/onegov/org/models/ticket.py +++ b/src/onegov/org/models/ticket.py @@ -15,7 +15,7 @@ from onegov.ticket import Ticket, Handler, handlers from onegov.search.utils import extract_hashtags from purl import URL -from sqlalchemy import desc, select, and_ +from sqlalchemy import desc, select, and_, text from sqlalchemy import func from sqlalchemy.ext.hybrid import hybrid_property from sqlalchemy.orm import object_session, aliased @@ -70,8 +70,6 @@ def reference(self, request: 'OrgRequest') -> str: def reference_group(self, request: 'OrgRequest') -> str: return request.translate(self.group) - # FIXME: extra localized text is used for suggestions in the search but - # @hybrid_property @hybrid_property def extra_localized_text(self) -> str: @@ -98,16 +96,12 @@ def extra_localized_text(self) -> str: @extra_localized_text.expression def extra_localized_text(cls): - Message = aliased(Message) # Assuming Message is your message model - subquery = ( - select(func.string_agg(Message.text, ' ')) - .where(and_( - Message.channel_id == cls.number, - Message.type.in_(['ticket_note', 'ticket_chat']) - )) - .scalar_subquery() + return ( + select([func.string_agg(Message.text, ' ')]) + .where(Message.channel_id == cls.number) + .where(Message.type.in_(('ticket_note', 'ticket_chat'))) + .label('extra_localized_text') ) - return func.coalesce(subquery, '') @property def es_tags(self) -> list[str] | None: diff --git a/src/onegov/ticket/model.py b/src/onegov/ticket/model.py index 1936151fe6..d463d47a80 100644 --- a/src/onegov/ticket/model.py +++ b/src/onegov/ticket/model.py @@ -154,8 +154,7 @@ def created(cls) -> 'Column[datetime]': 'extra_localized_text': {'type': 'localized'} } - # @hybrid_property - @property + @hybrid_property def extra_localized_text(self) -> str | None: """ Maybe used by child-classes to return localized extra data that should be indexed as well. From 45eb359b334ae41ef6d67a29bc51bdb41411bbe9 Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Mon, 9 Sep 2024 11:37:32 +0200 Subject: [PATCH 21/44] Adds future events on top of search results (in case of hit) --- src/onegov/org/models/search.py | 35 ++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/src/onegov/org/models/search.py b/src/onegov/org/models/search.py index 2d846b2367..feb8a681af 100644 --- a/src/onegov/org/models/search.py +++ b/src/onegov/org/models/search.py @@ -4,6 +4,7 @@ from elasticsearch_dsl.query import MatchPhrase from elasticsearch_dsl.query import MultiMatch from functools import cached_property +from sedate import utcnow from sqlalchemy import func from typing import TYPE_CHECKING, Any, List @@ -99,13 +100,16 @@ def get_sort_key(event: Event) -> float: batch = self.batch.load() events = [] non_events = [] + for search_result in batch: if isinstance(search_result, Event): events.append(search_result) else: non_events.append(search_result) + if not events: return batch + sorted_events = sorted( events, key=get_sort_key @@ -235,26 +239,33 @@ def batch(self) -> 'List[Searchable]': # type:ignore[override] return results[self.offset:self.offset + self.batch_size] - @cached_property def load_batch_results(self) -> list[Any]: - """Load search results and sort events by latest occurrence. + """ + Load search results and sort upcoming events by occurrence start date. This methods is a wrapper around `batch.load()`, which returns the - actual search results form the query. """ + actual search results form the query. + """ batch: List[Searchable] = self.batch - events: List[Searchable] = [] - non_events: List[Searchable] = [] + future_events: List[Searchable] = [] + other: List[Searchable] = [] + for search_result in batch: - if isinstance(search_result, Event): - events.append(search_result) + if (isinstance(search_result, Event) and + search_result.latest_occurrence.start > utcnow()): + future_events.append(search_result) else: - non_events.append(search_result) - if not events: + other.append(search_result) + + if not future_events: return batch + sorted_events = sorted( - events, key=lambda e: - e.latest_occurrence.start) # type:ignore[attr-defined] - return sorted_events + non_events + future_events, key=lambda e: + e.latest_occurrence.start, # type:ignore[attr-defined] + reverse=True) + + return sorted_events + other def _create_weighted_vector( self, From d7a560a1ade7da41bbbe720c2700c3d3d30b433a Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Mon, 9 Sep 2024 11:59:00 +0200 Subject: [PATCH 22/44] Make load bach result a cached property --- .pre-commit-config.yaml | 2 +- src/onegov/org/models/search.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index e556b5df61..cecce7a1a5 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -32,7 +32,7 @@ repos: - id: sass-lint files: '^src/.*\.scss' - repo: https://github.com/pre-commit/mirrors-eslint - rev: v9.9.1 + rev: v9.10.0 hooks: - id: eslint files: '^src/.*\.jsx?$' diff --git a/src/onegov/org/models/search.py b/src/onegov/org/models/search.py index feb8a681af..9e8eb9a1fe 100644 --- a/src/onegov/org/models/search.py +++ b/src/onegov/org/models/search.py @@ -10,7 +10,7 @@ from onegov.core.collection import Pagination, _M from onegov.core.orm import Base -from onegov.event.models import Event +from onegov.event.models import Event, Occurrence from onegov.search.utils import searchable_sqlalchemy_models @@ -239,6 +239,7 @@ def batch(self) -> 'List[Searchable]': # type:ignore[override] return results[self.offset:self.offset + self.batch_size] + @cached_property def load_batch_results(self) -> list[Any]: """ Load search results and sort upcoming events by occurrence start date. From 85eff7ee36d5b05ebe3bbc4582c8c0ce71e380c9 Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Mon, 9 Sep 2024 13:45:02 +0200 Subject: [PATCH 23/44] Make mypy happy --- src/onegov/directory/models/directory_entry.py | 6 +++--- src/onegov/org/models/search.py | 14 ++++++++------ src/onegov/org/models/ticket.py | 8 ++++---- src/onegov/ticket/model.py | 8 ++++++-- 4 files changed, 21 insertions(+), 15 deletions(-) diff --git a/src/onegov/directory/models/directory_entry.py b/src/onegov/directory/models/directory_entry.py index eaa14c2cff..91afbe45d9 100644 --- a/src/onegov/directory/models/directory_entry.py +++ b/src/onegov/directory/models/directory_entry.py @@ -121,12 +121,12 @@ def keywords(self) -> set[str]: # FIXME: asymmetric properties are not supported by mypy, switch to # a custom descriptor, if desired. - @keywords.setter + @keywords.setter # type:ignore[no-redef] def keywords(self, value: 'Collection[str] | None') -> None: self._keywords = dict.fromkeys(value, '') if value else None - @keywords.expression - def keywords(cls): # type:ignore[no-redef] + @keywords.expression # type:ignore[no-redef] + def keywords(cls): return func.array_to_string( func.array_agg( cast(func.jsonb_each_text(cls._keywords).keys(), ARRAY(String)) diff --git a/src/onegov/org/models/search.py b/src/onegov/org/models/search.py index 9e8eb9a1fe..402a144eef 100644 --- a/src/onegov/org/models/search.py +++ b/src/onegov/org/models/search.py @@ -10,7 +10,7 @@ from onegov.core.collection import Pagination, _M from onegov.core.orm import Base -from onegov.event.models import Event, Occurrence +from onegov.event.models import Event from onegov.search.utils import searchable_sqlalchemy_models @@ -247,13 +247,15 @@ def load_batch_results(self) -> list[Any]: actual search results form the query. """ + print('*** tschupre cached_property load_batch_results') batch: List[Searchable] = self.batch future_events: List[Searchable] = [] other: List[Searchable] = [] for search_result in batch: - if (isinstance(search_result, Event) and - search_result.latest_occurrence.start > utcnow()): + if (isinstance(search_result, Event) + and search_result.latest_occurrence + and search_result.latest_occurrence.start > utcnow()): future_events.append(search_result) else: other.append(search_result) @@ -304,7 +306,7 @@ def generic_search(self) -> list['Searchable']: for base in self.request.app.session_manager.bases: for model in searchable_sqlalchemy_models(base): - if model.es_public or self.request.is_logged_in: + if model.es_public or self.request.is_logged_in: # type:ignore query = self.request.session.query(model) if query.count(): @@ -321,7 +323,7 @@ def generic_search(self) -> list['Searchable']: model.fts_idx.op('@@')(ts_query) ).add_columns(rank_expression) - results.extend([r for r in query.all()]) # others are not relevant + results.extend(list(query.all())) # remove duplicates results = list(set(results)) @@ -345,7 +347,7 @@ def hashtag_search(self) -> list['Searchable']: # skip certain tables for hashtag search for better performance if (model.es_type_name not in ['attendees', 'files', 'people', 'tickets', 'users']): - if model.es_public or self.request.is_logged_in: + if model.es_public or self.request.is_logged_in: # type:ignore for doc in self.request.session.query(model).all(): if doc.es_tags and q in doc.es_tags: results.append(doc) diff --git a/src/onegov/org/models/ticket.py b/src/onegov/org/models/ticket.py index 2e880db7d4..132972f464 100644 --- a/src/onegov/org/models/ticket.py +++ b/src/onegov/org/models/ticket.py @@ -15,10 +15,10 @@ from onegov.ticket import Ticket, Handler, handlers from onegov.search.utils import extract_hashtags from purl import URL -from sqlalchemy import desc, select, and_, text +from sqlalchemy import desc, select from sqlalchemy import func from sqlalchemy.ext.hybrid import hybrid_property -from sqlalchemy.orm import object_session, aliased +from sqlalchemy.orm import object_session from typing import Any, TYPE_CHECKING if TYPE_CHECKING: @@ -94,8 +94,8 @@ def extra_localized_text(self) -> str: return result - @extra_localized_text.expression - def extra_localized_text(cls): + @extra_localized_text.expression # type:ignore[no-redef] + def extra_localized_text(cls) -> str: return ( select([func.string_agg(Message.text, ' ')]) .where(Message.channel_id == cls.number) diff --git a/src/onegov/ticket/model.py b/src/onegov/ticket/model.py index d463d47a80..c97b80a94c 100644 --- a/src/onegov/ticket/model.py +++ b/src/onegov/ticket/model.py @@ -155,12 +155,16 @@ def created(cls) -> 'Column[datetime]': } @hybrid_property - def extra_localized_text(self) -> str | None: + def extra_localized_text(self) -> str: """ Maybe used by child-classes to return localized extra data that should be indexed as well. """ - return None + raise NotImplementedError + + @extra_localized_text.expression # type:ignore[no-redef] + def extra_localized_text(cls) -> str: + raise NotImplementedError @property def es_suggestion(self) -> list[str]: From 6114dad4e53f41b6c6f262e3c68890192aa8628c Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Mon, 9 Sep 2024 13:57:58 +0200 Subject: [PATCH 24/44] Adds default implementation for extra localized text --- src/onegov/ticket/model.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/onegov/ticket/model.py b/src/onegov/ticket/model.py index c97b80a94c..5e4ba60835 100644 --- a/src/onegov/ticket/model.py +++ b/src/onegov/ticket/model.py @@ -10,7 +10,7 @@ from onegov.user import User from onegov.user import UserGroup from sedate import utcnow -from sqlalchemy import Boolean, Column, Enum, ForeignKey, Integer, Text +from sqlalchemy import Boolean, Column, Enum, ForeignKey, Integer, Text, text from sqlalchemy.ext.declarative import declared_attr from sqlalchemy.orm import backref, deferred, relationship from uuid import uuid4 @@ -160,11 +160,11 @@ def extra_localized_text(self) -> str: should be indexed as well. """ - raise NotImplementedError + return '' @extra_localized_text.expression # type:ignore[no-redef] def extra_localized_text(cls) -> str: - raise NotImplementedError + return text('') @property def es_suggestion(self) -> list[str]: From 71e61b6fb0e8d3ac8af2f7e7c1f1f00d2efd1055 Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Mon, 9 Sep 2024 15:26:11 +0200 Subject: [PATCH 25/44] Fixing org search template --- src/onegov/org/templates/search.pt | 2 +- src/onegov/org/templates/search_postgres.pt | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/onegov/org/templates/search.pt b/src/onegov/org/templates/search.pt index d6cad352a7..f58aacc0e9 100644 --- a/src/onegov/org/templates/search.pt +++ b/src/onegov/org/templates/search.pt @@ -34,7 +34,7 @@
- +

${resultslabel}

diff --git a/src/onegov/org/templates/search_postgres.pt b/src/onegov/org/templates/search_postgres.pt index eee07c1e56..921ac7c6b5 100644 --- a/src/onegov/org/templates/search_postgres.pt +++ b/src/onegov/org/templates/search_postgres.pt @@ -16,7 +16,9 @@
- +
- +

${resultslabel}

From bb16507cc295800d0716d48c3403d50e6bef25ac Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Thu, 3 Oct 2024 13:45:34 -0400 Subject: [PATCH 26/44] Adjust and add search template for org --- src/onegov/org/templates/search.pt | 3 +- src/onegov/org/templates/search_postgres.pt | 110 ++++++++++---------- 2 files changed, 57 insertions(+), 56 deletions(-) diff --git a/src/onegov/org/templates/search.pt b/src/onegov/org/templates/search.pt index f58aacc0e9..d1928703de 100644 --- a/src/onegov/org/templates/search.pt +++ b/src/onegov/org/templates/search.pt @@ -17,8 +17,7 @@
- - +
-
-
-
-
+
+
+ +
+ +
+ + +
+
+ +
+
+
+
+
+
+
-
-
- -

${resultslabel}

-
-
- -

Your postgres search returned no results.

+ +

${resultslabel}

+
+
+

Your search returned no results.

-
    - -
  • - +
      + +
    • + - -
        -
      • Score: ${result.explanation['score']}
        • - -
      • + +
          +
        • Score: ${result.explanation['score']}
          • + +
        • ${title}: ${result.explanation[key]['value']} -
          • -           -
        -         -
        • -         -
      +
      • +       +
    +     +
    • +     +
-
-
-
- +
+
+
+
+
+ -
\ No newline at end of file +
From 1c867751fbc7cb91a4b77cd7596712d9f3a09160 Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Thu, 3 Oct 2024 15:18:38 -0400 Subject: [PATCH 27/44] Exclude documents according their access level --- src/onegov/org/models/search.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/onegov/org/models/search.py b/src/onegov/org/models/search.py index 402a144eef..f547c202da 100644 --- a/src/onegov/org/models/search.py +++ b/src/onegov/org/models/search.py @@ -308,10 +308,14 @@ def generic_search(self) -> list['Searchable']: for model in searchable_sqlalchemy_models(base): if model.es_public or self.request.is_logged_in: # type:ignore query = self.request.session.query(model) + doc_count += query.count() - if query.count(): - doc_count += query.count() + if hasattr(model, 'access') or self.request.is_logged_in: + query = query.filter( + model.access # type:ignore[attr-defined] + == 'public') + if query.count(): vector = self._create_weighted_vector(model, language) rank_expression = func.coalesce( func.ts_rank( From b0a3fa50416ea709f70bddcc55128f4e821fbc44 Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Fri, 4 Oct 2024 12:23:59 -0400 Subject: [PATCH 28/44] Unaccent data while indexing and also for querying --- src/onegov/org/models/search.py | 27 ++++++++++++++------------- src/onegov/search/indexer.py | 18 ++++++++++++++---- 2 files changed, 28 insertions(+), 17 deletions(-) diff --git a/src/onegov/org/models/search.py b/src/onegov/org/models/search.py index f547c202da..28cb856950 100644 --- a/src/onegov/org/models/search.py +++ b/src/onegov/org/models/search.py @@ -5,7 +5,7 @@ from elasticsearch_dsl.query import MultiMatch from functools import cached_property from sedate import utcnow -from sqlalchemy import func +from sqlalchemy import func, Text, cast from typing import TYPE_CHECKING, Any, List from onegov.core.collection import Pagination, _M @@ -276,11 +276,11 @@ def _create_weighted_vector( language: str = 'simple' ) -> Any: # for now weight the first field with 'A', the rest with 'B' - weighted_vector = [ + weighted_vectors = [ func.setweight( func.to_tsvector( language, - getattr(model, field, '') + func.unaccent(cast(getattr(model, field, ''), Text)) ), weight ) @@ -289,9 +289,9 @@ def _create_weighted_vector( ] # combine all weighted vectors - if weighted_vector: - combined_vector = weighted_vector[0] - for vector in weighted_vector[1:]: + if weighted_vectors: + combined_vector = weighted_vectors[0] + for vector in weighted_vectors[1:]: combined_vector = combined_vector.op('||')(vector) else: combined_vector = func.to_tsvector(language, '') @@ -302,7 +302,8 @@ def generic_search(self) -> list['Searchable']: doc_count = 0 results: List[Any] = [] language = locale_mapping(self.request.locale or 'de_CH') - ts_query = func.websearch_to_tsquery(language, self.web_search) + ts_query = func.websearch_to_tsquery(language, + func.unaccent(self.web_search)) for base in self.request.app.session_manager.bases: for model in searchable_sqlalchemy_models(base): @@ -316,16 +317,16 @@ def generic_search(self) -> list['Searchable']: == 'public') if query.count(): - vector = self._create_weighted_vector(model, language) + weighted = ( + self._create_weighted_vector(model, language)) rank_expression = func.coalesce( func.ts_rank( - vector, + weighted, ts_query, - # 0 # normalization, ignore document length + 0 # normalization, ignore document length ), 0).label('rank') - query = query.filter( - model.fts_idx.op('@@')(ts_query) - ).add_columns(rank_expression) + query = (query.filter(model.fts_idx.op('@@')(ts_query)) + .add_columns(rank_expression)) results.extend(list(query.all())) diff --git a/src/onegov/search/indexer.py b/src/onegov/search/indexer.py index ede4aecf1d..36ea6c0b7f 100644 --- a/src/onegov/search/indexer.py +++ b/src/onegov/search/indexer.py @@ -3,12 +3,14 @@ import sqlalchemy from copy import deepcopy + from elasticsearch.exceptions import NotFoundError from elasticsearch.helpers import streaming_bulk from langdetect.lang_detect_exception import LangDetectException from itertools import groupby from operator import itemgetter from queue import Queue, Empty, Full +from unidecode import unidecode from onegov.core.utils import is_non_string_iterable from onegov.search import index_log, log, Searchable, utils @@ -403,12 +405,20 @@ def index( try: for task in tasks: + data = {} + language = ( self.idx_language_mapping.get(task['language'], 'simple')) - data = { - k: str(v) - for k, v in task['properties'].items() - if not k.startswith('es_')} + + for k, v in task['properties'].items(): + if not k.startswith('es_') and v: + if isinstance(v, list): + v = ' '.join(v) + + # 'unaccent' the index data in order to find works + # with umlaut ect. + data[k] = unidecode(v) + _id = task['id'] content.append( {'language': language, 'data': data, '_id': _id}) From 35cac54d3e5ee6d6132062ef8c0dec99a26f685f Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Mon, 7 Oct 2024 07:59:45 -0400 Subject: [PATCH 29/44] Convert es_public and access to hybrid properties; fix psql search for agency --- src/onegov/activity/models/attendee.py | 5 ++- src/onegov/agency/models/agency.py | 22 ++++++++++--- src/onegov/agency/models/membership.py | 32 +++++++++++++++++-- src/onegov/agency/models/person.py | 28 +++++++++++++--- src/onegov/agency/views/search.py | 3 +- src/onegov/core/elements.py | 3 +- src/onegov/directory/models/directory.py | 7 ++-- .../directory/models/directory_entry.py | 2 +- src/onegov/event/models/event.py | 4 +-- src/onegov/event/models/occurrence.py | 4 ++- src/onegov/feriennet/models/activity.py | 4 ++- src/onegov/file/models/file.py | 4 +-- src/onegov/form/models/submission.py | 2 +- src/onegov/fsi/models/course.py | 5 ++- src/onegov/fsi/models/course_attendee.py | 6 +++- src/onegov/fsi/models/course_event.py | 2 +- src/onegov/landsgemeinde/models/agenda.py | 7 +++- src/onegov/landsgemeinde/models/assembly.py | 7 +++- src/onegov/landsgemeinde/models/file.py | 4 ++- src/onegov/landsgemeinde/models/votum.py | 7 +++- src/onegov/newsletter/models.py | 4 ++- src/onegov/org/models/directory.py | 6 ++-- src/onegov/org/models/file.py | 9 ++++-- src/onegov/org/models/page.py | 7 ++-- src/onegov/org/models/search.py | 4 +-- src/onegov/pas/models/commission.py | 7 +++- src/onegov/pas/models/legislative_period.py | 7 +++- src/onegov/pas/models/parliamentarian.py | 8 ++++- src/onegov/pas/models/parliamentary_group.py | 7 +++- src/onegov/pas/models/party.py | 7 +++- src/onegov/pas/models/settlement_run.py | 7 +++- src/onegov/people/models/agency.py | 7 +++- src/onegov/people/models/membership.py | 7 +++- src/onegov/people/models/person.py | 5 ++- .../reservation/models/custom_allocation.py | 4 ++- src/onegov/search/mixins.py | 5 +-- src/onegov/ticket/model.py | 6 +++- .../translator_directory/models/translator.py | 6 +++- src/onegov/user/models/user.py | 5 ++- tests/onegov/search/test_indexer.py | 5 ++- tests/onegov/search/test_integration.py | 6 ++-- 41 files changed, 226 insertions(+), 61 deletions(-) diff --git a/src/onegov/activity/models/attendee.py b/src/onegov/activity/models/attendee.py index 86cb1d3cf1..75b06b4ee9 100644 --- a/src/onegov/activity/models/attendee.py +++ b/src/onegov/activity/models/attendee.py @@ -65,7 +65,10 @@ class Attendee(Base, TimestampMixin, ORMSearchable): 'name': {'type': 'text'}, 'notes': {'type': 'localized'} } - es_public = False + + @hybrid_property + def es_public(self) -> bool: + return False @property def es_suggestion(self) -> str: diff --git a/src/onegov/agency/models/agency.py b/src/onegov/agency/models/agency.py index e982517cb1..b80ebc9184 100644 --- a/src/onegov/agency/models/agency.py +++ b/src/onegov/agency/models/agency.py @@ -1,3 +1,8 @@ +from sqlalchemy import case +from sqlalchemy.orm import object_session +from sqlalchemy.orm import relationship +from sqlalchemy.ext.hybrid import hybrid_property + from onegov.agency.models.membership import ExtendedAgencyMembership from onegov.agency.utils import get_html_paragraph_with_line_breaks from onegov.core.crypto import random_token @@ -11,9 +16,6 @@ from onegov.org.models.extensions import PublicationExtension from onegov.people import Agency from onegov.user import RoleMapping -from sqlalchemy.orm import object_session -from sqlalchemy.orm import relationship - from typing import Any from typing import IO @@ -24,6 +26,7 @@ from markupsafe import Markup from onegov.agency.request import AgencyRequest from onegov.core.types import AppenderQuery + from sqlalchemy.sql import ClauseElement from uuid import UUID @@ -40,10 +43,19 @@ class ExtendedAgency(Agency, AccessExtension, PublicationExtension): es_type_name = 'extended_agency' - @property - def es_public(self) -> bool: # type:ignore[override] + @hybrid_property # ignore[no-redef] + def es_public(self) -> bool: return self.access == 'public' and self.published + @es_public.expression # ignore[no-redef] + def es_public(cls) -> 'ClauseElement': + return case( + [ + (cls.access != 'public', False), + (cls.published == False, False) + ] + ) + #: Defines which fields of a membership and person should be exported to #: the PDF. The fields are expected to contain two parts seperated by a #: point. The first part is either `membership` or `person`, the second diff --git a/src/onegov/agency/models/membership.py b/src/onegov/agency/models/membership.py index b4b4d66039..36e7f3c376 100644 --- a/src/onegov/agency/models/membership.py +++ b/src/onegov/agency/models/membership.py @@ -1,15 +1,18 @@ +from sqlalchemy import case, func +from sqlalchemy.ext.hybrid import hybrid_property + from onegov.core.orm.mixins import dict_property from onegov.core.orm.mixins import meta_property from onegov.org.models.extensions import AccessExtension from onegov.org.models.extensions import PublicationExtension from onegov.people import AgencyMembership - from typing import TYPE_CHECKING if TYPE_CHECKING: from onegov.agency.models import ExtendedAgency from onegov.agency.models import ExtendedPerson from sqlalchemy.orm import relationship + from sqlalchemy.sql import ClauseElement class ExtendedAgencyMembership(AgencyMembership, AccessExtension, @@ -20,9 +23,10 @@ class ExtendedAgencyMembership(AgencyMembership, AccessExtension, es_type_name = 'extended_membership' - @property - def es_public(self) -> bool: # type:ignore[override] + @hybrid_property + def es_public(self) -> bool: if self.agency: + # TO CHECK: is meta needed here? try self.agency.access instead if self.agency.meta.get('access', 'public') != 'public': return False if not self.agency.published: @@ -36,6 +40,28 @@ def es_public(self) -> bool: # type:ignore[override] return self.access == 'public' + @es_public.expression # type:ignore[no-redef] + def es_public(cls) -> 'ClauseElement': + return case( + [ + ( + func.coalesce( + func.json_extract_path_text(cls.agency, 'access'), + 'public' + ) != 'public', + False + ), + ( + func.coalesce( + func.json_extract_path_text(cls.person, 'access'), + 'public' + ) != 'public', + False + ), + ], + else_=cls.access == 'public' + ) + # Todo: It is very unclear how this should be used. In the PDF rendering, # it is placed a middle column with 0.5 cm after the title. # On the agency, it is placed after the membership title, so not a prefix diff --git a/src/onegov/agency/models/person.py b/src/onegov/agency/models/person.py index e632d8c3c8..4912ea770b 100644 --- a/src/onegov/agency/models/person.py +++ b/src/onegov/agency/models/person.py @@ -1,9 +1,12 @@ +from sqlalchemy import func, select +from sqlalchemy.orm import object_session +from sqlalchemy.ext.hybrid import hybrid_property + from onegov.agency.utils import get_html_paragraph_with_line_breaks from onegov.org.models import Organisation from onegov.org.models.extensions import AccessExtension from onegov.org.models.extensions import PublicationExtension from onegov.people import Person -from sqlalchemy.orm import object_session from typing import TYPE_CHECKING @@ -13,6 +16,7 @@ from onegov.agency.request import AgencyRequest from onegov.core.types import AppenderQuery from sqlalchemy.orm import relationship + from sqlalchemy.sql import ClauseElement class ExtendedPerson(Person, AccessExtension, PublicationExtension): @@ -22,8 +26,8 @@ class ExtendedPerson(Person, AccessExtension, PublicationExtension): es_type_name = 'extended_person' - @property - def es_public(self) -> bool: # type:ignore[override] + @hybrid_property + def es_public(self) -> bool: return self.access == 'public' and self.published es_properties = { @@ -50,14 +54,28 @@ def es_suggestion(self) -> tuple[str, ...]: AppenderQuery[ExtendedAgencyMembership] ] - @property + @hybrid_property def phone_internal(self) -> str: org = object_session(self).query(Organisation).one() number = getattr(self, org.agency_phone_internal_field) digits = org.agency_phone_internal_digits return number.replace(' ', '')[-digits:] if number and digits else '' - @property + @phone_internal.expression + def phone_internal(cls) -> 'ClauseElement': + org_subquery = ( + select([Organisation.agency_phone_internal_field, + Organisation.agency_phone_internal_digits]) + .limit(1) + .scalar_subquery() + ) + return func.substr( + func.replace(getattr( + cls, org_subquery.c.agency_phone_internal_field), ' ', ''), + -org_subquery.c.agency_phone_internal_field_digits + ).label('phone_internal') + + @hybrid_property def phone_es(self) -> list[str]: result = [self.phone_internal] for number in (self.phone, self.phone_direct): diff --git a/src/onegov/agency/views/search.py b/src/onegov/agency/views/search.py index 32dffd04eb..c16ab88e79 100644 --- a/src/onegov/agency/views/search.py +++ b/src/onegov/agency/views/search.py @@ -24,7 +24,8 @@ def search( return data -@AgencyApp.html(model=SearchPostgres, template='search.pt', permission=Public) +@AgencyApp.html(model=SearchPostgres, template='search_postgres.pt', + permission=Public) def agency_search_postgres( self: SearchPostgres['Base'], request: 'AgencyRequest' diff --git a/src/onegov/core/elements.py b/src/onegov/core/elements.py index 2742b620e8..21ea2a3c50 100644 --- a/src/onegov/core/elements.py +++ b/src/onegov/core/elements.py @@ -13,6 +13,7 @@ This module should eventually replace the elements.py module. """ +from sqlalchemy.ext.hybrid import hybrid_property from onegov.core.templates import render_macro @@ -137,7 +138,7 @@ class AccessMixin: __slots__ = () - @property + @hybrid_property def access(self) -> str: """ Wraps model.access, ensuring it is always available, even if the model does not use it. diff --git a/src/onegov/directory/models/directory.py b/src/onegov/directory/models/directory.py index 47e1d7cb4e..19ed428cf4 100644 --- a/src/onegov/directory/models/directory.py +++ b/src/onegov/directory/models/directory.py @@ -2,6 +2,9 @@ from email_validator import validate_email from enum import Enum + +from sqlalchemy.ext.hybrid import hybrid_property + from onegov.core.cache import instance_lru_cache from onegov.core.cache import lru_cache from onegov.core.crypto import random_token @@ -91,7 +94,7 @@ def directory_entry(self) -> 'DirectoryEntry | None': entries = self.linked_directory_entries return entries[0] if entries else None - @property + @hybrid_property def access(self) -> str: # we don't want these files to show up in search engines return 'secret' if self.published else 'private' @@ -111,7 +114,7 @@ class Directory(Base, ContentMixin, TimestampMixin, 'lead': {'type': 'localized'} } - @property + @hybrid_property def es_public(self) -> bool: return False # to be overridden downstream diff --git a/src/onegov/directory/models/directory_entry.py b/src/onegov/directory/models/directory_entry.py index 91afbe45d9..06bd8f9f0b 100644 --- a/src/onegov/directory/models/directory_entry.py +++ b/src/onegov/directory/models/directory_entry.py @@ -43,7 +43,7 @@ class DirectoryEntry(Base, ContentMixin, CoordinatesMixin, TimestampMixin, 'text': {'type': 'localized_html'} } - @property + @hybrid_property def es_public(self) -> bool: return False # to be overridden downstream diff --git a/src/onegov/event/models/event.py b/src/onegov/event/models/event.py index 72b3d977f8..369c1c4374 100644 --- a/src/onegov/event/models/event.py +++ b/src/onegov/event/models/event.py @@ -1,12 +1,12 @@ import warnings from datetime import datetime - from dateutil import rrule from dateutil.rrule import rrulestr from icalendar import Calendar as vCalendar from icalendar import Event as vEvent from icalendar import vRecur +from sqlalchemy.ext.hybrid import hybrid_property from onegov.core.orm import Base from onegov.core.orm.abstract import associated @@ -194,7 +194,7 @@ def set_blob( 'filter_keywords': {'type': 'keyword'} } - @property + @hybrid_property def es_public(self) -> bool: return self.state == 'published' diff --git a/src/onegov/event/models/occurrence.py b/src/onegov/event/models/occurrence.py index 30b6925ea9..186cef1d76 100644 --- a/src/onegov/event/models/occurrence.py +++ b/src/onegov/event/models/occurrence.py @@ -1,5 +1,7 @@ from icalendar import Calendar as vCalendar from icalendar import Event as vEvent +from sqlalchemy.ext.hybrid import hybrid_property + from onegov.core.orm import Base from onegov.core.orm.mixins import TimestampMixin from onegov.core.orm.types import UUID @@ -72,6 +74,6 @@ def as_ical(self, url: str | None = None) -> bytes: vcalendar.add_component(vevent) return vcalendar.to_ical() - @property + @hybrid_property def access(self) -> str: return self.event.access diff --git a/src/onegov/feriennet/models/activity.py b/src/onegov/feriennet/models/activity.py index 890ea6125e..2f61957d8d 100644 --- a/src/onegov/feriennet/models/activity.py +++ b/src/onegov/feriennet/models/activity.py @@ -1,4 +1,6 @@ from functools import cached_property +from sqlalchemy.ext.hybrid import hybrid_property + from onegov.activity import Activity, ActivityCollection, Occasion from onegov.activity import PublicationRequestCollection from onegov.activity.models import DAYS @@ -32,7 +34,7 @@ class VacationActivity(Activity, CoordinatesExtension, SearchableContent): 'organiser': {'type': 'text'} } - @property + @hybrid_property def es_public(self) -> bool: return self.state == 'accepted' diff --git a/src/onegov/file/models/file.py b/src/onegov/file/models/file.py index ca8b8eb19e..5373fc47a2 100644 --- a/src/onegov/file/models/file.py +++ b/src/onegov/file/models/file.py @@ -113,7 +113,7 @@ class SearchableFile(ORMSearchable): def es_suggestion(self) -> str: return self.name - @property + @hybrid_property def es_public(self) -> bool: return self.published @@ -311,7 +311,7 @@ def reference_observer(self, reference: 'UploadedFile') -> None: def name_observer(self, name: str) -> None: self.order = normalize_for_url(name) - @property + @hybrid_property def access(self) -> str: return 'public' if self.published else 'private' diff --git a/src/onegov/form/models/submission.py b/src/onegov/form/models/submission.py index d6455acf10..f327e6a915 100644 --- a/src/onegov/form/models/submission.py +++ b/src/onegov/form/models/submission.py @@ -431,7 +431,7 @@ class CompleteSurveySubmission(SurveySubmission): class FormFile(File): __mapper_args__ = {'polymorphic_identity': 'formfile'} - @property + @hybrid_property def access(self) -> str: # we don't want these files to show up in search engines return 'secret' if self.published else 'private' diff --git a/src/onegov/fsi/models/course.py b/src/onegov/fsi/models/course.py index 6ed1f19c9f..7a05e23549 100644 --- a/src/onegov/fsi/models/course.py +++ b/src/onegov/fsi/models/course.py @@ -25,7 +25,10 @@ class Course(Base, ORMSearchable): 'name': {'type': 'localized'}, 'description': {'type': 'localized'}, } - es_public = True + + @hybrid_property + def es_public(self) -> bool: + return True id: 'Column[uuid.UUID]' = Column( UUID, # type:ignore[arg-type] diff --git a/src/onegov/fsi/models/course_attendee.py b/src/onegov/fsi/models/course_attendee.py index 7c86618098..be1af9b5a0 100644 --- a/src/onegov/fsi/models/course_attendee.py +++ b/src/onegov/fsi/models/course_attendee.py @@ -1,3 +1,5 @@ +from sqlalchemy.ext.hybrid import hybrid_property + from onegov.core.orm import Base from onegov.core.orm.types import UUID, JSON from sqlalchemy import Boolean @@ -49,7 +51,9 @@ class CourseAttendee(Base, ORMSearchable): 'title': {'type': 'text'}, } - es_public = False + @hybrid_property + def es_public(self) -> bool: + return False id: 'Column[uuid.UUID]' = Column( UUID, # type:ignore[arg-type] diff --git a/src/onegov/fsi/models/course_event.py b/src/onegov/fsi/models/course_event.py index cb7a1903b4..954418d1eb 100644 --- a/src/onegov/fsi/models/course_event.py +++ b/src/onegov/fsi/models/course_event.py @@ -121,7 +121,7 @@ class CourseEvent(Base, TimestampMixin, ORMSearchable): lazy='joined' ) - @property + @hybrid_property def es_public(self) -> bool: return not self.hidden_from_public diff --git a/src/onegov/landsgemeinde/models/agenda.py b/src/onegov/landsgemeinde/models/agenda.py index c0f567261e..1b47046a94 100644 --- a/src/onegov/landsgemeinde/models/agenda.py +++ b/src/onegov/landsgemeinde/models/agenda.py @@ -1,3 +1,5 @@ +from sqlalchemy.ext.hybrid import hybrid_property + from onegov.core.orm import Base from onegov.core.orm.mixins import content_property from onegov.core.orm.mixins import dict_markup_property @@ -49,7 +51,10 @@ class AgendaItem( __tablename__ = 'landsgemeinde_agenda_items' - es_public = True + @hybrid_property + def es_public(self) -> bool: + return True + es_properties = { 'title': {'type': 'text'}, 'overview': {'type': 'localized_html'}, diff --git a/src/onegov/landsgemeinde/models/assembly.py b/src/onegov/landsgemeinde/models/assembly.py index b0ac544d9f..804a4473b8 100644 --- a/src/onegov/landsgemeinde/models/assembly.py +++ b/src/onegov/landsgemeinde/models/assembly.py @@ -1,3 +1,5 @@ +from sqlalchemy.ext.hybrid import hybrid_property + from onegov.core.orm import Base from onegov.core.orm.mixins import dict_markup_property from onegov.core.orm.mixins import ContentMixin @@ -46,11 +48,14 @@ class Assembly( __tablename__ = 'landsgemeinde_assemblies' - es_public = True es_properties = { 'overview': {'type': 'localized_html'}, } + @hybrid_property + def es_public(self) -> bool: + return True + @property def es_suggestion(self) -> tuple[str, ...]: return ( diff --git a/src/onegov/landsgemeinde/models/file.py b/src/onegov/landsgemeinde/models/file.py index 581ac84ede..43890fb1b5 100644 --- a/src/onegov/landsgemeinde/models/file.py +++ b/src/onegov/landsgemeinde/models/file.py @@ -1,3 +1,5 @@ +from sqlalchemy.ext.hybrid import hybrid_property + from onegov.file.models.file import File from onegov.file.models.file import SearchableFile @@ -9,6 +11,6 @@ class LandsgemeindeFile(File, SearchableFile): es_type_name = 'landsgemeinde_file' - @property + @hybrid_property def es_public(self) -> bool: return True diff --git a/src/onegov/landsgemeinde/models/votum.py b/src/onegov/landsgemeinde/models/votum.py index e5258b8202..aaeaace7b0 100644 --- a/src/onegov/landsgemeinde/models/votum.py +++ b/src/onegov/landsgemeinde/models/votum.py @@ -1,3 +1,5 @@ +from sqlalchemy.ext.hybrid import hybrid_property + from onegov.core.orm import Base from onegov.core.orm.mixins import ContentMixin from onegov.core.orm.mixins import dict_markup_property @@ -43,7 +45,10 @@ class Votum( __tablename__ = 'landsgemeinde_vota' - es_public = True + @hybrid_property + def es_public(self) -> bool: + return True + es_properties = { 'text': {'type': 'localized_html'}, 'motion': {'type': 'localized_html'}, diff --git a/src/onegov/newsletter/models.py b/src/onegov/newsletter/models.py index 5908c2f6bd..4540d9d9e2 100644 --- a/src/onegov/newsletter/models.py +++ b/src/onegov/newsletter/models.py @@ -1,4 +1,6 @@ from email_validator import validate_email +from sqlalchemy.ext.hybrid import hybrid_property + from onegov.core.crypto import random_token from onegov.core.orm import Base from onegov.core.orm.mixins import ( @@ -54,7 +56,7 @@ class Newsletter(Base, ContentMixin, TimestampMixin, SearchableContent): 'html': {'type': 'localized_html'} } - @property + @hybrid_property def es_public(self) -> bool: return self.sent is not None diff --git a/src/onegov/org/models/directory.py b/src/onegov/org/models/directory.py index c24cc08116..0d86c08917 100644 --- a/src/onegov/org/models/directory.py +++ b/src/onegov/org/models/directory.py @@ -4,6 +4,8 @@ from datetime import timedelta from functools import cached_property from markupsafe import Markup +from sqlalchemy.ext.hybrid import hybrid_property + from onegov.core.orm.mixins import ( content_property, dict_markup_property, dict_property, meta_property) from onegov.core.utils import linkify @@ -362,7 +364,7 @@ class ExtendedDirectory(Directory, AccessExtension, Extendable, def entry_cls_name(self) -> str: return 'ExtendedDirectoryEntry' - @property + @hybrid_property def es_public(self) -> bool: return self.access == 'public' @@ -444,7 +446,7 @@ class ExtendedDirectoryEntry(DirectoryEntry, PublicationExtension, # technically not enforced, but it should be a given directory: relationship[ExtendedDirectory] - @property + @hybrid_property def es_public(self) -> bool: return self.access == 'public' and self.published diff --git a/src/onegov/org/models/file.py b/src/onegov/org/models/file.py index 451706c416..6d6bf7e7fc 100644 --- a/src/onegov/org/models/file.py +++ b/src/onegov/org/models/file.py @@ -6,6 +6,9 @@ from dateutil.relativedelta import relativedelta from functools import cached_property from itertools import chain, groupby + +from sqlalchemy.ext.hybrid import hybrid_property + from onegov.core.orm import as_selectable from onegov.core.orm.mixins import dict_property, meta_property from onegov.file import File, FileSet, FileCollection, FileSetCollection @@ -234,7 +237,7 @@ class GeneralFile(File, SearchableFile): linked_accesses: dict_property[dict[str, str]] linked_accesses = meta_property(default=dict) - @property + @hybrid_property def access(self) -> str: if self.publication: return 'public' @@ -246,7 +249,7 @@ def access(self) -> str: return widest_access(*self.linked_accesses.values()) - @property + @hybrid_property def es_public(self) -> bool: return self.published and self.access == 'public' @@ -263,7 +266,7 @@ class ImageSet(FileSet, AccessExtension, ORMSearchable): 'lead': {'type': 'localized'} } - @property + @hybrid_property def es_public(self) -> bool: return self.access == 'public' diff --git a/src/onegov/org/models/page.py b/src/onegov/org/models/page.py index 7dda0d570e..e379f82096 100644 --- a/src/onegov/org/models/page.py +++ b/src/onegov/org/models/page.py @@ -1,4 +1,7 @@ from datetime import datetime + +from sqlalchemy.ext.hybrid import hybrid_property + from onegov.core.orm.mixins import ( content_property, dict_markup_property, dict_property, meta_property) from onegov.form import Form, move_fields @@ -56,7 +59,7 @@ class Topic(Page, TraitInfo, SearchableContent, AccessExtension, def es_skip(self) -> bool: return self.meta.get('trait') == 'link' # do not index links - @property + @hybrid_property def es_public(self) -> bool: return self.access == 'public' and self.published @@ -154,7 +157,7 @@ class News(Page, TraitInfo, SearchableContent, NewsletterExtension, hashtags: dict_property[list[str]] = meta_property(default=list) - @property + @hybrid_property def es_public(self) -> bool: return self.access == 'public' and self.published diff --git a/src/onegov/org/models/search.py b/src/onegov/org/models/search.py index 28cb856950..42dd79ad9d 100644 --- a/src/onegov/org/models/search.py +++ b/src/onegov/org/models/search.py @@ -307,7 +307,7 @@ def generic_search(self) -> list['Searchable']: for base in self.request.app.session_manager.bases: for model in searchable_sqlalchemy_models(base): - if model.es_public or self.request.is_logged_in: # type:ignore + if model.es_public or self.request.is_logged_in: query = self.request.session.query(model) doc_count += query.count() @@ -352,7 +352,7 @@ def hashtag_search(self) -> list['Searchable']: # skip certain tables for hashtag search for better performance if (model.es_type_name not in ['attendees', 'files', 'people', 'tickets', 'users']): - if model.es_public or self.request.is_logged_in: # type:ignore + if model.es_public or self.request.is_logged_in: for doc in self.request.session.query(model).all(): if doc.es_tags and q in doc.es_tags: results.append(doc) diff --git a/src/onegov/pas/models/commission.py b/src/onegov/pas/models/commission.py index edc3ead5ee..f84129fcdb 100644 --- a/src/onegov/pas/models/commission.py +++ b/src/onegov/pas/models/commission.py @@ -1,3 +1,5 @@ +from sqlalchemy.ext.hybrid import hybrid_property + from onegov.core.orm import Base from onegov.core.orm.mixins import dict_markup_property from onegov.core.orm.mixins import ContentMixin @@ -40,7 +42,10 @@ class Commission(Base, ContentMixin, TimestampMixin, ORMSearchable): __tablename__ = 'pas_commissions' - es_public = False + @hybrid_property + def es_public(self) -> bool: + return False + es_properties = {'name': {'type': 'text'}} @property diff --git a/src/onegov/pas/models/legislative_period.py b/src/onegov/pas/models/legislative_period.py index 5a90c3c3d9..7e01be40e1 100644 --- a/src/onegov/pas/models/legislative_period.py +++ b/src/onegov/pas/models/legislative_period.py @@ -1,3 +1,5 @@ +from sqlalchemy.ext.hybrid import hybrid_property + from onegov.core.orm import Base from onegov.core.orm.mixins import TimestampMixin from onegov.core.orm.types import UUID @@ -17,7 +19,10 @@ class LegislativePeriod(Base, TimestampMixin, ORMSearchable): __tablename__ = 'pas_legislative_periods' - es_public = False + @hybrid_property + def es_public(self) -> bool: + return False + es_properties = {'name': {'type': 'text'}} @property diff --git a/src/onegov/pas/models/parliamentarian.py b/src/onegov/pas/models/parliamentarian.py index 2123f60ac9..fa881a7888 100644 --- a/src/onegov/pas/models/parliamentarian.py +++ b/src/onegov/pas/models/parliamentarian.py @@ -1,4 +1,7 @@ from datetime import date + +from sqlalchemy.ext.hybrid import hybrid_property + from onegov.core.orm import Base from onegov.core.orm.mixins import ContentMixin from onegov.core.orm.mixins import TimestampMixin @@ -57,7 +60,10 @@ class Parliamentarian( __tablename__ = 'pas_parliamentarians' - es_public = False + @hybrid_property + def es_public(self) -> bool: + return False + es_properties = { 'first_name': {'type': 'text'}, 'last_name': {'type': 'text'}, diff --git a/src/onegov/pas/models/parliamentary_group.py b/src/onegov/pas/models/parliamentary_group.py index 8586b43f68..a1c72aee9a 100644 --- a/src/onegov/pas/models/parliamentary_group.py +++ b/src/onegov/pas/models/parliamentary_group.py @@ -1,3 +1,5 @@ +from sqlalchemy.ext.hybrid import hybrid_property + from onegov.core.orm import Base from onegov.core.orm.mixins import dict_markup_property from onegov.core.orm.mixins import ContentMixin @@ -21,7 +23,10 @@ class ParliamentaryGroup(Base, ContentMixin, TimestampMixin, ORMSearchable): __tablename__ = 'pas_parliamentary_groups' - es_public = False + @hybrid_property + def es_public(self) -> bool: + return False + es_properties = {'name': {'type': 'text'}} @property diff --git a/src/onegov/pas/models/party.py b/src/onegov/pas/models/party.py index 0689a80358..a9388811c9 100644 --- a/src/onegov/pas/models/party.py +++ b/src/onegov/pas/models/party.py @@ -1,3 +1,5 @@ +from sqlalchemy.ext.hybrid import hybrid_property + from onegov.core.orm import Base from onegov.core.orm.mixins import dict_markup_property from onegov.core.orm.mixins import ContentMixin @@ -21,7 +23,10 @@ class Party(Base, ContentMixin, TimestampMixin, ORMSearchable): __tablename__ = 'pas_parties' - es_public = False + @hybrid_property + def es_public(self) -> bool: + return False + es_properties = {'name': {'type': 'text'}} @property diff --git a/src/onegov/pas/models/settlement_run.py b/src/onegov/pas/models/settlement_run.py index c8f7dd4368..4b7da37a0c 100644 --- a/src/onegov/pas/models/settlement_run.py +++ b/src/onegov/pas/models/settlement_run.py @@ -1,3 +1,5 @@ +from sqlalchemy.ext.hybrid import hybrid_property + from onegov.core.orm import Base from onegov.core.orm.mixins import content_property from onegov.core.orm.mixins import ContentMixin @@ -21,7 +23,10 @@ class SettlementRun(Base, ContentMixin, TimestampMixin, ORMSearchable): __tablename__ = 'pas_settlements' - es_public = False + @hybrid_property + def es_public(self) -> bool: + return False + es_properties = {'name': {'type': 'text'}} @property diff --git a/src/onegov/people/models/agency.py b/src/onegov/people/models/agency.py index 5748e4c81e..facc26a724 100644 --- a/src/onegov/people/models/agency.py +++ b/src/onegov/people/models/agency.py @@ -1,3 +1,5 @@ +from sqlalchemy.ext.hybrid import hybrid_property + from onegov.core.crypto import random_token from onegov.core.orm.abstract import AdjacencyList from onegov.core.orm.abstract import associated @@ -64,7 +66,10 @@ class Agency(AdjacencyList, ContentMixin, TimestampMixin, ORMSearchable, 'polymorphic_identity': 'generic', } - es_public = True + @hybrid_property + def es_public(self) -> bool: + return True + es_properties = { 'title': {'type': 'text'}, 'description': {'type': 'localized'}, diff --git a/src/onegov/people/models/membership.py b/src/onegov/people/models/membership.py index 4f7ea917c7..9976d3b9ff 100644 --- a/src/onegov/people/models/membership.py +++ b/src/onegov/people/models/membership.py @@ -1,3 +1,5 @@ +from sqlalchemy.ext.hybrid import hybrid_property + from onegov.core.orm import Base from onegov.core.orm.mixins import ContentMixin from onegov.core.orm.mixins import TimestampMixin @@ -44,7 +46,10 @@ class AgencyMembership(Base, ContentMixin, TimestampMixin, ORMSearchable, 'polymorphic_identity': 'generic', } - es_public = True + @hybrid_property + def es_public(self) -> bool: + return True + es_properties = { 'title': {'type': 'text'}, } diff --git a/src/onegov/people/models/person.py b/src/onegov/people/models/person.py index d24525788a..88002eeb6d 100644 --- a/src/onegov/people/models/person.py +++ b/src/onegov/people/models/person.py @@ -46,7 +46,10 @@ class Person(Base, ContentMixin, TimestampMixin, ORMSearchable, 'polymorphic_identity': 'generic', } - es_public = True + @hybrid_property + def es_public(self) -> bool: + return True + es_properties = { 'title': {'type': 'text'}, 'function': {'type': 'localized'}, diff --git a/src/onegov/reservation/models/custom_allocation.py b/src/onegov/reservation/models/custom_allocation.py index f9b9d3ae69..6b25a2d940 100644 --- a/src/onegov/reservation/models/custom_allocation.py +++ b/src/onegov/reservation/models/custom_allocation.py @@ -1,4 +1,6 @@ from libres.db.models import Allocation +from sqlalchemy.ext.hybrid import hybrid_property + from onegov.core.orm import ModelBase from onegov.reservation.models.resource import Resource from sqlalchemy.orm import object_session @@ -12,6 +14,6 @@ def resource_obj(self) -> Resource: return object_session(self).query( Resource).filter_by(id=self.resource).one() - @property + @hybrid_property def access(self) -> str: return (self.data or {}).get('access', 'public') diff --git a/src/onegov/search/mixins.py b/src/onegov/search/mixins.py index 2dd67eef25..a2d3bf3202 100644 --- a/src/onegov/search/mixins.py +++ b/src/onegov/search/mixins.py @@ -1,6 +1,7 @@ from sqlalchemy import Column from sqlalchemy.dialects.postgresql import TSVECTOR from sqlalchemy.ext.declarative import declared_attr +from sqlalchemy.ext.hybrid import hybrid_property from sqlalchemy.orm import deferred from onegov.search.utils import classproperty @@ -135,7 +136,7 @@ def es_language(self) -> str: return 'auto' # TODO: rename to fts_public - @property + @hybrid_property def es_public(self) -> bool: """ Returns True if the model is available to be found by the public. If false, only editors/admins will see this object in the search @@ -215,7 +216,7 @@ class SearchableContent(ORMSearchable): 'text': {'type': 'localized_html'} } - @property + @hybrid_property def es_public(self) -> bool: return self.access == 'public' # type:ignore[attr-defined] diff --git a/src/onegov/ticket/model.py b/src/onegov/ticket/model.py index 5e4ba60835..0dcf5496b9 100644 --- a/src/onegov/ticket/model.py +++ b/src/onegov/ticket/model.py @@ -143,7 +143,11 @@ def created(cls) -> 'Column[datetime]': } # limit the search to the ticket number -> the rest can be found - es_public = False + + @hybrid_property + def es_public(self) -> bool: + return False + es_properties = { 'number': {'type': 'text'}, 'title': {'type': 'text'}, diff --git a/src/onegov/translator_directory/models/translator.py b/src/onegov/translator_directory/models/translator.py index 2f09d87759..92c902fa3f 100644 --- a/src/onegov/translator_directory/models/translator.py +++ b/src/onegov/translator_directory/models/translator.py @@ -1,5 +1,7 @@ from uuid import uuid4 +from sqlalchemy.ext.hybrid import hybrid_property + from onegov.core.orm.mixins import TimestampMixin from sqlalchemy import Column, Text, Enum, Date, Integer, Boolean, Float from sqlalchemy.orm import backref, relationship @@ -56,7 +58,9 @@ class Translator(Base, TimestampMixin, AssociatedFiles, ContentMixin, 'email': {'type': 'text'} } - es_public = False + @hybrid_property + def es_public(self) -> bool: + return False id: 'Column[uuid.UUID]' = Column( UUID, # type:ignore[arg-type] diff --git a/src/onegov/user/models/user.py b/src/onegov/user/models/user.py index 53627a1f34..6332ffdab5 100644 --- a/src/onegov/user/models/user.py +++ b/src/onegov/user/models/user.py @@ -53,7 +53,10 @@ class User(Base, TimestampMixin, ORMSearchable): 'realname': {'type': 'text'}, 'userprofile': {'type': 'text'} } - es_public = False + + @hybrid_property + def es_public(self) -> bool: + return False @property def es_suggestion(self) -> tuple[str, str]: diff --git a/tests/onegov/search/test_indexer.py b/tests/onegov/search/test_indexer.py index e792a73911..b6ceba5e83 100644 --- a/tests/onegov/search/test_indexer.py +++ b/tests/onegov/search/test_indexer.py @@ -3,6 +3,9 @@ import transaction from datetime import datetime + +from sqlalchemy.ext.hybrid import hybrid_property + from onegov.people import Person, PersonCollection from onegov.search import Searchable, SearchOfflineError, utils from onegov.search.indexer import parse_index_name, PostgresIndexer @@ -332,7 +335,7 @@ def __init__(self, id, **kwargs): def es_language(self): return self.language - @property + @hybrid_property def es_public(self): return self.public diff --git a/tests/onegov/search/test_integration.py b/tests/onegov/search/test_integration.py index 2a1b0ec0e1..4ef370fa35 100644 --- a/tests/onegov/search/test_integration.py +++ b/tests/onegov/search/test_integration.py @@ -5,6 +5,8 @@ from datetime import timedelta from elasticsearch_dsl.function import SF from elasticsearch_dsl.query import MatchPhrase, FunctionScore +from sqlalchemy.ext.hybrid import hybrid_property + from onegov.core import Framework from onegov.core.orm.mixins import TimestampMixin from onegov.core.utils import scan_morepath_modules @@ -57,7 +59,7 @@ class Document(Base, ORMSearchable): def es_suggestion(self): return self.title - @property + @hybrid_property def es_public(self): return self.public @@ -526,7 +528,7 @@ class Document(Base, ORMSearchable): 'title': {'type': 'localized'} } - @property + @hybrid_property def es_public(self): return self.public From f4a4d332c283b7754bf5d094f2821bbc16cecbc1 Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Mon, 7 Oct 2024 09:59:21 -0500 Subject: [PATCH 30/44] Fix: unaccent is only needed during index creation but not while querying the db --- src/onegov/org/models/search.py | 4 ++-- src/onegov/search/indexer.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/onegov/org/models/search.py b/src/onegov/org/models/search.py index 42dd79ad9d..035d2f4c4b 100644 --- a/src/onegov/org/models/search.py +++ b/src/onegov/org/models/search.py @@ -280,8 +280,8 @@ def _create_weighted_vector( func.setweight( func.to_tsvector( language, - func.unaccent(cast(getattr(model, field, ''), Text)) - ), + getattr(model, field, '') + ), weight ) for field, weight in zip(model.es_properties.keys(), 'ABBBBBBBBBB') diff --git a/src/onegov/search/indexer.py b/src/onegov/search/indexer.py index 36ea6c0b7f..183f95f5b7 100644 --- a/src/onegov/search/indexer.py +++ b/src/onegov/search/indexer.py @@ -415,7 +415,7 @@ def index( if isinstance(v, list): v = ' '.join(v) - # 'unaccent' the index data in order to find works + # 'unaccent' the index data in order to find words # with umlaut ect. data[k] = unidecode(v) From ce27112ccbbec8d3f51c032eb047c5ccd3bc1469 Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Mon, 7 Oct 2024 10:09:24 -0500 Subject: [PATCH 31/44] Fix condition and improvements --- src/onegov/agency/models/agency.py | 2 +- src/onegov/org/models/search.py | 23 ++++++++--------------- src/onegov/org/models/ticket.py | 3 ++- 3 files changed, 11 insertions(+), 17 deletions(-) diff --git a/src/onegov/agency/models/agency.py b/src/onegov/agency/models/agency.py index b80ebc9184..6326ae27d4 100644 --- a/src/onegov/agency/models/agency.py +++ b/src/onegov/agency/models/agency.py @@ -43,7 +43,7 @@ class ExtendedAgency(Agency, AccessExtension, PublicationExtension): es_type_name = 'extended_agency' - @hybrid_property # ignore[no-redef] + @hybrid_property def es_public(self) -> bool: return self.access == 'public' and self.published diff --git a/src/onegov/org/models/search.py b/src/onegov/org/models/search.py index 035d2f4c4b..e4666d8eee 100644 --- a/src/onegov/org/models/search.py +++ b/src/onegov/org/models/search.py @@ -5,7 +5,7 @@ from elasticsearch_dsl.query import MultiMatch from functools import cached_property from sedate import utcnow -from sqlalchemy import func, Text, cast +from sqlalchemy import func from typing import TYPE_CHECKING, Any, List from onegov.core.collection import Pagination, _M @@ -280,8 +280,7 @@ def _create_weighted_vector( func.setweight( func.to_tsvector( language, - getattr(model, field, '') - ), + getattr(model, field, '')), weight ) for field, weight in zip(model.es_properties.keys(), 'ABBBBBBBBBB') @@ -311,10 +310,9 @@ def generic_search(self) -> list['Searchable']: query = self.request.session.query(model) doc_count += query.count() - if hasattr(model, 'access') or self.request.is_logged_in: - query = query.filter( - model.access # type:ignore[attr-defined] - == 'public') + if (hasattr(model, 'access') + and not self.request.is_logged_in): + query = query.filter(model.access == 'public') if query.count(): weighted = ( @@ -327,22 +325,17 @@ def generic_search(self) -> list['Searchable']: ), 0).label('rank') query = (query.filter(model.fts_idx.op('@@')(ts_query)) .add_columns(rank_expression)) - results.extend(list(query.all())) - # remove duplicates + # remove duplicates, sort by rank results = list(set(results)) - - # sort by rank results.sort(key=lambda x: x[1], reverse=True) - # remove rank from results - results = [r[0] for r in results] - self.nbr_of_docs = doc_count self.nbr_of_results = len(results) - return results + # remove rank column from results and return + return [r[0] for r in results] def hashtag_search(self) -> list['Searchable']: q = self.web_search.lstrip('#') diff --git a/src/onegov/org/models/ticket.py b/src/onegov/org/models/ticket.py index 132972f464..0eecad490a 100644 --- a/src/onegov/org/models/ticket.py +++ b/src/onegov/org/models/ticket.py @@ -30,6 +30,7 @@ from onegov.ticket.handler import _Q from sqlalchemy import Column from sqlalchemy.orm import Query, Session + from sqlalchemy.sql import ClauseElement from uuid import UUID @@ -95,7 +96,7 @@ def extra_localized_text(self) -> str: return result @extra_localized_text.expression # type:ignore[no-redef] - def extra_localized_text(cls) -> str: + def extra_localized_text(cls) -> 'ClauseElement': return ( select([func.string_agg(Message.text, ' ')]) .where(Message.channel_id == cls.number) From 7c1142cc49ee2ca29a92d9262fd3dbce7ed603b8 Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Tue, 8 Oct 2024 11:10:28 -0500 Subject: [PATCH 32/44] Fix agency memberships psql expression --- src/onegov/agency/models/membership.py | 39 +++++++++++++++----------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/src/onegov/agency/models/membership.py b/src/onegov/agency/models/membership.py index 36e7f3c376..be53ec80ee 100644 --- a/src/onegov/agency/models/membership.py +++ b/src/onegov/agency/models/membership.py @@ -1,4 +1,4 @@ -from sqlalchemy import case, func +from sqlalchemy import case, select from sqlalchemy.ext.hybrid import hybrid_property from onegov.core.orm.mixins import dict_property @@ -26,7 +26,6 @@ class ExtendedAgencyMembership(AgencyMembership, AccessExtension, @hybrid_property def es_public(self) -> bool: if self.agency: - # TO CHECK: is meta needed here? try self.agency.access instead if self.agency.meta.get('access', 'public') != 'public': return False if not self.agency.published: @@ -42,22 +41,30 @@ def es_public(self) -> bool: @es_public.expression # type:ignore[no-redef] def es_public(cls) -> 'ClauseElement': + from onegov.agency.models import ExtendedAgency, ExtendedPerson + + person_meta = select([ExtendedPerson.meta]).where( + ExtendedPerson.id == cls.person_id + ).as_scalar() + + person_published = select([ExtendedPerson.published]).where( + ExtendedPerson.id == cls.person_id + ).as_scalar() + + agency_meta = select([ExtendedAgency.meta]).where( + ExtendedAgency.id == cls.agency_id + ).as_scalar() + + agency_published = select([ExtendedAgency.published]).where( + ExtendedAgency.id == cls.agency_id + ).as_scalar() + return case( [ - ( - func.coalesce( - func.json_extract_path_text(cls.agency, 'access'), - 'public' - ) != 'public', - False - ), - ( - func.coalesce( - func.json_extract_path_text(cls.person, 'access'), - 'public' - ) != 'public', - False - ), + (person_meta['access'] != 'public', False), + (person_published != True, False), + (agency_meta['access'] != 'public', False), + (agency_published != True, False), ], else_=cls.access == 'public' ) From 59bfda00cab97200c0f6fe3fbf95f7f612556ab1 Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Wed, 9 Oct 2024 14:00:36 -0500 Subject: [PATCH 33/44] Ensure suggestions are tuple of strings --- src/onegov/org/models/search.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/onegov/org/models/search.py b/src/onegov/org/models/search.py index e4666d8eee..fd844a32be 100644 --- a/src/onegov/org/models/search.py +++ b/src/onegov/org/models/search.py @@ -378,6 +378,8 @@ def suggestions(self) -> tuple[str, ...]: if element.es_type_name == 'files': continue suggest = getattr(element, 'es_suggestion', '') + if isinstance(suggest, tuple): + suggest = suggest[0] suggestions.append(suggest) return tuple(suggestions[:15]) From 7e932911f42f22614bf1a39ec6d991663a641765 Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Mon, 14 Oct 2024 06:46:43 -0400 Subject: [PATCH 34/44] Extend and fix a few tests --- src/onegov/agency/models/agency.py | 10 +- src/onegov/org/models/file.py | 11 ++- src/onegov/org/models/page.py | 8 ++ src/onegov/org/models/search.py | 8 +- src/onegov/search/indexer.py | 2 +- tests/onegov/org/test_views_search.py | 136 +++++++++++++++++++++----- 6 files changed, 138 insertions(+), 37 deletions(-) diff --git a/src/onegov/agency/models/agency.py b/src/onegov/agency/models/agency.py index 6326ae27d4..3d51010bb9 100644 --- a/src/onegov/agency/models/agency.py +++ b/src/onegov/agency/models/agency.py @@ -1,4 +1,4 @@ -from sqlalchemy import case +from sqlalchemy import and_ from sqlalchemy.orm import object_session from sqlalchemy.orm import relationship from sqlalchemy.ext.hybrid import hybrid_property @@ -49,11 +49,9 @@ def es_public(self) -> bool: @es_public.expression # ignore[no-redef] def es_public(cls) -> 'ClauseElement': - return case( - [ - (cls.access != 'public', False), - (cls.published == False, False) - ] + and_( + cls.access == 'public', + cls.published == True ) #: Defines which fields of a membership and person should be exported to diff --git a/src/onegov/org/models/file.py b/src/onegov/org/models/file.py index 6d6bf7e7fc..59bcb2aa6d 100644 --- a/src/onegov/org/models/file.py +++ b/src/onegov/org/models/file.py @@ -20,14 +20,14 @@ from onegov.search import ORMSearchable from operator import itemgetter from sedate import standardize_date, utcnow -from sqlalchemy import asc, desc, select, nullslast # type: ignore +from sqlalchemy import asc, desc, select, nullslast, and_ # type: ignore from typing import ( overload, Any, Generic, Literal, NamedTuple, TypeVar, TYPE_CHECKING) if TYPE_CHECKING: from collections.abc import Callable, Iterable, Iterator from sqlalchemy.orm import Query, Session - from sqlalchemy.sql import Select + from sqlalchemy.sql import Select, ClauseElement from typing_extensions import Self _T = TypeVar('_T') @@ -253,6 +253,13 @@ def access(self) -> str: def es_public(self) -> bool: return self.published and self.access == 'public' + @es_public.expression # ignore[no-redef] + def es_public(cls) -> 'ClauseElement': + and_( + cls.published == True, + cls.access == 'public' + ) + class ImageFile(File): __mapper_args__ = {'polymorphic_identity': 'image'} diff --git a/src/onegov/org/models/page.py b/src/onegov/org/models/page.py index e379f82096..a5f5eea170 100644 --- a/src/onegov/org/models/page.py +++ b/src/onegov/org/models/page.py @@ -32,6 +32,7 @@ from typing import Any, TYPE_CHECKING if TYPE_CHECKING: from onegov.org.request import OrgRequest + from sqlalchemy.sql import ClauseElement from sqlalchemy.orm import Query @@ -161,6 +162,13 @@ class News(Page, TraitInfo, SearchableContent, NewsletterExtension, def es_public(self) -> bool: return self.access == 'public' and self.published + @es_public.expression # ignore[no-redef] + def es_public(cls) -> 'ClauseElement': + and_( + cls.access == 'public', + cls.published == True + ) + @observes('content') def content_observer(self, content: dict[str, Any]) -> None: self.hashtags = self.es_tags or [] diff --git a/src/onegov/org/models/search.py b/src/onegov/org/models/search.py index fd844a32be..302d6ed471 100644 --- a/src/onegov/org/models/search.py +++ b/src/onegov/org/models/search.py @@ -13,7 +13,6 @@ from onegov.event.models import Event from onegov.search.utils import searchable_sqlalchemy_models - if TYPE_CHECKING: from onegov.org.request import OrgRequest from onegov.search import Searchable @@ -309,10 +308,8 @@ def generic_search(self) -> list['Searchable']: if model.es_public or self.request.is_logged_in: query = self.request.session.query(model) doc_count += query.count() - - if (hasattr(model, 'access') - and not self.request.is_logged_in): - query = query.filter(model.access == 'public') + if not self.request.is_logged_in: + query = query.filter(model.es_public == True) if query.count(): weighted = ( @@ -325,6 +322,7 @@ def generic_search(self) -> list['Searchable']: ), 0).label('rank') query = (query.filter(model.fts_idx.op('@@')(ts_query)) .add_columns(rank_expression)) + results.extend(list(query.all())) # remove duplicates, sort by rank diff --git a/src/onegov/search/indexer.py b/src/onegov/search/indexer.py index 183f95f5b7..8ad3fc493d 100644 --- a/src/onegov/search/indexer.py +++ b/src/onegov/search/indexer.py @@ -912,7 +912,7 @@ def put(self, translation: 'Task') -> None: try: self.es_queue.put_nowait(translation) if translation['action'] == 'index': - # we only need to provide index tasks for fts + # we only need to provide index tasks for psql fts self.psql_queue.put_nowait(translation) except Full: log.error("The orm event translator queue is full!") diff --git a/tests/onegov/org/test_views_search.py b/tests/onegov/org/test_views_search.py index 472b2b2321..5ed7978814 100644 --- a/tests/onegov/org/test_views_search.py +++ b/tests/onegov/org/test_views_search.py @@ -46,21 +46,32 @@ def test_basic_search(client_with_es): assert "It is pretty awesome" in search_page # make sure anonymous doesn't see hidden things in the search results + # elasticsearch assert "fulltext" in client.spawn().get('/search?q=fulltext') + # postgres + assert "fulltext" in client.spawn().get('/search-postgres?q=fulltext') + edit_news = news.click("Bearbeiten") edit_news.form['access'] = 'private' edit_news.form.submit() client.app.es_client.indices.refresh(index='_all') - assert "Now supporting" not in client.spawn().get('/search?q=fulltext') + # elasticsearch + assert ("Now supporting" not in + client.spawn().get('/search?q=fulltext')) assert "Now supporting" in client.get('/search?q=fulltext') + # postgres + assert ("Now supporting" not in + client.spawn().get('/search-postgres?q=fulltext')) + assert "Now supporting" in client.get('/search-postgres?q=fulltext') + @pytest.mark.flaky(reruns=3) def test_view_search_is_limiting(client_with_es): # ensures that the search doesn't just return all results - # a regression that occured for anonymous uses only + # a regression that occurred for anonymous uses only client = client_with_es client.login_admin() @@ -97,6 +108,7 @@ def test_search_recently_published_object(client_with_es): client = client_with_es client.login_admin() anom = client.spawn() + session = client.app.session() # Create objects, not yet published start = datetime.now() + timedelta(days=1) @@ -110,14 +122,24 @@ def test_search_recently_published_object(client_with_es): client.app.es_client.indices.refresh(index='_all') + assert session.query(Page).filter( + Page.title == "Now supporting fulltext search" + ).one().published == False + + # elasticsearch assert 'fulltext' in client.get('/search?q=wow') assert 'fulltext' not in anom.get('/search?q=wow') assert 'It is pretty awesome' in client.get('/search?q=fulltext') assert 'It is pretty awesome' not in anom.get('/search?q=fulltext') + # postgres + assert 'fulltext' in client.get('/search-postgres?q=wow') + assert 'fulltext' not in anom.get('/search-postgres?q=wow') + assert 'pretty awesome' in client.get('/search-postgres?q=fulltext') + assert 'pretty awesome' not in anom.get('/search-postgres?q=fulltext') + # Publish then = utcnow() - timedelta(minutes=30) - session = client.app.session() transaction.begin() session.query(Page).filter( Page.title == "Now supporting fulltext search" @@ -131,11 +153,22 @@ def test_search_recently_published_object(client_with_es): sleep(5) + assert session.query(Page).filter( + Page.title == "Now supporting fulltext search" + ).one().published == True + + # elasticsearch assert 'fulltext' in client.get('/search?q=wow') assert 'fulltext' in anom.get('/search?q=wow') assert 'It is pretty awesome' in client.get('/search?q=fulltext') assert 'It is pretty awesome' in anom.get('/search?q=fulltext') + # postgres + assert 'fulltext' in client.get('/search-postgres?q=wow') + assert 'fulltext' in anom.get('/search-postgres?q=wow') + assert 'It is pretty awesome' in client.get('/search-postgres?q=fulltext') + assert 'It is pretty awesome' in anom.get('/search-postgres?q=fulltext') + # Unpublish transaction.begin() session.query(Page).filter( @@ -150,10 +183,21 @@ def test_search_recently_published_object(client_with_es): sleep(5) - assert 'fulltext' in client.get('/search?q=wow') - assert 'fulltext' not in anom.get('/search?q=wow') - assert 'It is pretty awesome' in client.get('/search?q=fulltext') - assert 'It is pretty awesome' not in anom.get('/search?q=fulltext') + assert session.query(Page).filter( + Page.title == "Now supporting fulltext search" + ).one().published == False + + # elasticsearch + assert 'fulltext' in client.get('/search-postgres?q=wow') + assert 'fulltext' not in anom.get('/search-postgres?q=wow') + assert 'is pretty awesome' in client.get('/search-postgres?q=fulltext') + assert 'is pretty awesome' not in anom.get('/search-postgres?q=fulltext') + + # postgres + assert 'fulltext' in client.get('/search-postgres?q=wow') + assert 'fulltext' not in anom.get('/search-postgres?q=wow') + assert 'is pretty awesome' in client.get('/search-postgres?q=fulltext') + assert 'is pretty awesome' not in anom.get('/search-postgres?q=fulltext') @pytest.mark.flaky(reruns=3) @@ -172,10 +216,16 @@ def test_search_for_page_with_member_access(client_with_es): client.app.es_client.indices.refresh(index='_all') + # elasticsearch assert 'Test' in client.get('/search?q=Memberius') assert 'Test' in member.get('/search?q=Memberius') assert 'Test' not in anom.get('/search?q=Memberius') + # postgres + assert 'Test' in client.get('/search-postgres?q=Memberius') + assert 'Test' in member.get('/search-postgres?q=Memberius') + assert 'Test' not in anom.get('/search-postgres?q=Memberius') + @pytest.mark.flaky(reruns=3) def test_basic_autocomplete(client_with_es): @@ -189,27 +239,40 @@ def test_basic_autocomplete(client_with_es): new_person.form['last_name'] = 'Gordon' new_person.form.submit() + # elasticsearch client.app.es_client.indices.refresh(index='_all') assert client.get('/search/suggest?q=Go').json == ["Gordon Flash"] assert client.get('/search/suggest?q=Fl').json == ["Flash Gordon"] + # postgres has no auto-complete + assert (client.get('/search-postgres/suggest?q=Gordon'). + json == ["Gordon Flash"]) + assert (client.get('/search-postgres/suggest?q=Flash'). + json == ["Gordon Flash"]) + def test_search_publication_files(client_with_es): client = client_with_es client.login_admin() path = module_path('tests.onegov.org', 'fixtures/sample.pdf') - with open(path, 'rb') as f: + with (open(path, 'rb') as f): page = client.get('/files') - page.form['file'] = Upload('Sample.pdf', f.read(), 'application/pdf') + page.form['file'] = Upload( + 'Sample.pdf', f.read(), 'application/pdf') page.form.submit() client.app.es_indexer.process() client.app.es_client.indices.refresh(index='_all') + # elasticsearch assert 'Sample' in client.get('/search?q=Adobe') assert 'Sample' not in client.spawn().get('/search?q=Adobe') + # postgres + assert 'Sample' in client.get('/search-postgres?q=Adobe') + assert 'Sample' not in client.spawn().get('/search-postgres?q=Adobe') + transaction.begin() pdf = FileCollection(client.app.session()).query().one() pdf.publication = True @@ -218,9 +281,14 @@ def test_search_publication_files(client_with_es): client.app.es_indexer.process() client.app.es_client.indices.refresh(index='_all') + # elasticsearch assert 'Sample' in client.get('/search?q=Adobe') assert 'Sample' in client.spawn().get('/search?q=Adobe') + # postgres + assert 'Sample' in client.get('/search-postgres?q=Adobe') + assert 'Sample' in client.spawn().get('/search-postgres?q=Adobe') + def test_search_hashtags(client_with_es): @@ -237,8 +305,10 @@ def test_search_hashtags(client_with_es): client.app.es_indexer.process() client.app.es_client.indices.refresh(index='_all') - assert 'We have a new homepage' in client.get('/search?q=%23newhomepage') - assert 'We have a new homepage' not in client.get('/search?q=%23newhomepa') + assert 'We have a new homepage' in client.get( + '/search-postgres?q=%23newhomepage') + assert 'We have a new homepage' not in client.get( + '/search-postgres?q=%23newhomepa') def test_ticket_chat_search(client_with_es): @@ -269,15 +339,15 @@ def test_ticket_chat_search(client_with_es): # at this point logged in users should find the ticket by 'deadbeef' client.app.es_client.indices.refresh(index='_all') - page = client.get('/search?q=deadbeef') + page = client.get('/search-postgres?q=deadbeef') assert 'Foo' in page # but anonymous users should not - page = client.spawn().get('/search?q=deadbeef') + page = client.spawn().get('/search-postgres?q=deadbeef') assert 'Foo' not in page -def test_search_events_are_sorted_by_occurrence_date(client_with_es): +def test_search_future_events_are_sorted_by_occurrence_date(client_with_es): client = client_with_es client.login_admin() anom = client.spawn() @@ -287,17 +357,24 @@ def test_search_events_are_sorted_by_occurrence_date(client_with_es): event_data = [ { "email": "test@example.org", - "title": "First Concert", - "location": "Location", - "organizer": "Organizer", - "start_date": date.today() - timedelta(days=40), + "title": "Not sorted Concert", + "location": "Location0", + "organizer": "Organizer0", + "start_date": date.today() - timedelta(days=10), }, { "email": "test2@example.org", "title": "Second Concert", "location": "Location2", "organizer": "Organizer2", - "start_date": date.today() - timedelta(days=30), + "start_date": date.today() + timedelta(days=20), + }, + { + "email": "test1@example.org", + "title": "First Concert", + "location": "Location1", + "organizer": "Organizer1", + "start_date": date.today() + timedelta(days=1), }, { "email": "test4@example.org", @@ -311,7 +388,7 @@ def test_search_events_are_sorted_by_occurrence_date(client_with_es): "title": "Third Concert", "location": "Location3", "organizer": "Organizer3", - "start_date": date.today() + timedelta(days=1), + "start_date": date.today() + timedelta(days=35), }, ] @@ -333,10 +410,23 @@ def test_search_events_are_sorted_by_occurrence_date(client_with_es): client.app.es_client.indices.refresh(index='_all') + # elasticsearch even sorts past events by occurrence date for current_client in (client, member, anom): results = current_client.get('/search?q=Concert') # Expect ordered by occurrence date, for all search results of 'Event' - assert [a.text.rstrip(' \n') for a in results.pyquery( - 'li.search-result-events a')] == [ - 'First Concert', 'Second Concert', 'Third Concert', 'Forth Concert' + assert [a.text.rstrip(' \n') for a in + results.pyquery('li.search-result-events a')] == [ + 'Not sorted Concert', 'First Concert', 'Second Concert', + 'Third Concert', 'Forth Concert' + ] + + # postgres + for current_client in (client, member, anom): + results = current_client.get('/search-postgres?q=Concert') + # Expect future events ordered by occurrence date, fare future first. + # Past events are not sorted by occurrence date. + assert [a.text.rstrip(' \n') for a in + results.pyquery('li.search-result-events a')] == [ + 'Forth Concert', 'Third Concert', 'Second Concert', + 'First Concert', 'Not sorted Concert' ] From 0167dd63c0d4d8f0501fb36b62cec58aff5744fe Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Tue, 15 Oct 2024 12:55:26 -0400 Subject: [PATCH 35/44] More hybrid properties and fixes for such --- src/onegov/agency/models/agency.py | 2 +- src/onegov/agency/models/person.py | 2 +- src/onegov/feriennet/models/activity.py | 23 ++++++++++++++++++++++- src/onegov/fsi/models/course_attendee.py | 22 +++++++++++++++++++--- src/onegov/fsi/models/course_event.py | 24 +++++++++++++++++++++--- src/onegov/org/models/file.py | 2 +- 6 files changed, 65 insertions(+), 10 deletions(-) diff --git a/src/onegov/agency/models/agency.py b/src/onegov/agency/models/agency.py index 3d51010bb9..326370d744 100644 --- a/src/onegov/agency/models/agency.py +++ b/src/onegov/agency/models/agency.py @@ -49,7 +49,7 @@ def es_public(self) -> bool: @es_public.expression # ignore[no-redef] def es_public(cls) -> 'ClauseElement': - and_( + return and_( cls.access == 'public', cls.published == True ) diff --git a/src/onegov/agency/models/person.py b/src/onegov/agency/models/person.py index 4912ea770b..49bbb00c43 100644 --- a/src/onegov/agency/models/person.py +++ b/src/onegov/agency/models/person.py @@ -61,7 +61,7 @@ def phone_internal(self) -> str: digits = org.agency_phone_internal_digits return number.replace(' ', '')[-digits:] if number and digits else '' - @phone_internal.expression + @phone_internal.expression # type:ignore[no-redef] def phone_internal(cls) -> 'ClauseElement': org_subquery = ( select([Organisation.agency_phone_internal_field, diff --git a/src/onegov/feriennet/models/activity.py b/src/onegov/feriennet/models/activity.py index 2f61957d8d..83672a39b1 100644 --- a/src/onegov/feriennet/models/activity.py +++ b/src/onegov/feriennet/models/activity.py @@ -1,4 +1,6 @@ from functools import cached_property + +from sqlalchemy import func from sqlalchemy.ext.hybrid import hybrid_property from onegov.activity import Activity, ActivityCollection, Occasion @@ -17,6 +19,8 @@ if TYPE_CHECKING: from collections.abc import Iterator, Sequence from markupsafe import Markup + from sqlalchemy.sql import ClauseElement + from onegov.activity.models import PublicationRequest from onegov.feriennet.request import FeriennetRequest @@ -42,7 +46,7 @@ def es_public(self) -> bool: def es_skip(self) -> bool: return self.state == 'preview' - @property + @hybrid_property def organiser(self) -> list[str]: organiser: list[str] = [ self.user.username, @@ -73,6 +77,23 @@ def organiser(self) -> list[str]: return organiser + @organiser.expression # ignore[no-redef] + def organizer(cls) -> 'ClauseElement': + return func.array([ + cls.user.username, + cls.user.realname, + cls.user.data.get('organisation', ''), + cls.user.data.get('address', ''), + cls.user.data.get('zip_code', ''), + cls.user.data.get('place', ''), + cls.user.data.get('email', ''), + cls.user.data.get('phone', ''), + cls.user.data.get('emergency', ''), + cls.user.data.get('website', ''), + cls.user.data.get('bank_account', ''), + cls.user.data.get('bank_beneficiary', ''), + ]) + def ordered_tags( self, request: 'FeriennetRequest', diff --git a/src/onegov/fsi/models/course_attendee.py b/src/onegov/fsi/models/course_attendee.py index be1af9b5a0..7de1be1593 100644 --- a/src/onegov/fsi/models/course_attendee.py +++ b/src/onegov/fsi/models/course_attendee.py @@ -2,7 +2,7 @@ from onegov.core.orm import Base from onegov.core.orm.types import UUID, JSON -from sqlalchemy import Boolean +from sqlalchemy import Boolean, case, and_ from onegov.search import ORMSearchable from sedate import utcnow from sqlalchemy import Column, Text, ForeignKey, ARRAY, desc @@ -16,6 +16,7 @@ from onegov.core.types import AppenderQuery from onegov.user import User from sqlalchemy.orm import Query + from sqlalchemy.sql import ClauseElement from .course_event import CourseEvent from .course_subscription import CourseSubscription @@ -132,7 +133,7 @@ def __str__(self) -> str: cascade='all, delete-orphan' ) - @property + @hybrid_property def title(self) -> str: return ' '.join( part @@ -140,6 +141,15 @@ def title(self) -> str: if part ) or self.email + @title.expression + def title(cls) -> 'ClauseElement': + has_name = and_(cls.first_name != None, cls.last_name != None) + return case([ + (has_name, cls.first_name + ' ' + cls.last_name), + (cls.first_name != None, cls.first_name), + (cls.last_name != None, cls.last_name) + ], else_=cls._email) + @property def lead(self) -> str | None: return self.organisation @@ -156,7 +166,7 @@ def role(self) -> str: assert self.user is not None return self.user.role - @property + @hybrid_property def email(self) -> str: """Needs a switch for external users""" if not self.user_id: @@ -169,6 +179,12 @@ def email(self) -> str: assert self.user is not None return self.user.username + @email.expression + def email(cls) -> 'ClauseElement': + return case([ + (cls.user_id == None, cls._email) + ], else_=cls.user.username) + @email.setter def email(self, value: str) -> None: self._email = value diff --git a/src/onegov/fsi/models/course_event.py b/src/onegov/fsi/models/course_event.py index 954418d1eb..943d1c3eee 100644 --- a/src/onegov/fsi/models/course_event.py +++ b/src/onegov/fsi/models/course_event.py @@ -7,7 +7,8 @@ from icalendar import Event as vEvent from sedate import utcnow, to_timezone from sqlalchemy import ( - Column, Boolean, SmallInteger, Enum, Text, Interval, ForeignKey, or_, and_) + Column, Boolean, SmallInteger, Enum, Text, Interval, ForeignKey, or_, and_, + select) from sqlalchemy.ext.hybrid import hybrid_property from sqlalchemy.orm import relationship, object_session from uuid import uuid4 @@ -31,6 +32,7 @@ from onegov.core.types import AppenderQuery from onegov.fsi.request import FsiRequest from sqlalchemy.orm import Query + from sqlalchemy.sql import ClauseElement from typing_extensions import Self, TypeAlias from wtforms.fields.choices import _Choice from .course import Course @@ -129,10 +131,18 @@ def es_public(self) -> bool: def title(self) -> str: return str(self) - @property + @hybrid_property def name(self) -> str: return self.course.name + @name.expression # type:ignore + def name(cls) -> 'ClauseElement': + from .course import Course + + return select([Course.name]).where( + Course.id == cls.course_id + ).as_scalar() + @property def lead(self) -> str: return ( @@ -141,10 +151,18 @@ def lead(self) -> str: f'{self.presenter_company}' ) - @property + @hybrid_property def description(self) -> 'Markup': return self.course.description + @description.expression + def description(cls) -> 'ClauseElement': + from .course import Course + + return select([Course.description]).where( + Course.id == cls.course_id + ).as_scalar() + def __str__(self) -> str: start = to_timezone( self.start, 'Europe/Zurich').strftime('%d.%m.%Y %H:%M') diff --git a/src/onegov/org/models/file.py b/src/onegov/org/models/file.py index 59bcb2aa6d..6577e24a53 100644 --- a/src/onegov/org/models/file.py +++ b/src/onegov/org/models/file.py @@ -255,7 +255,7 @@ def es_public(self) -> bool: @es_public.expression # ignore[no-redef] def es_public(cls) -> 'ClauseElement': - and_( + return and_( cls.published == True, cls.access == 'public' ) From 045f8ad7e1789d915b68c35906a956af2bdf21fb Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Fri, 18 Oct 2024 12:05:59 -0400 Subject: [PATCH 36/44] Extend and fix tests for landsgemeinde --- src/onegov/landsgemeinde/views/search.py | 2 +- tests/onegov/landsgemeinde/test_views.py | 36 ++++++++++++++++++++---- 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/src/onegov/landsgemeinde/views/search.py b/src/onegov/landsgemeinde/views/search.py index acd5d05ff2..a01b54d625 100644 --- a/src/onegov/landsgemeinde/views/search.py +++ b/src/onegov/landsgemeinde/views/search.py @@ -20,7 +20,7 @@ def landsgemeinde_search( return search(self, request, DefaultLayout(self, request)) -@LandsgemeindeApp.html(model=SearchPostgres, template='search.pt', +@LandsgemeindeApp.html(model=SearchPostgres, template='search_postgres.pt', permission=Public) def landsgemeinde_search_postgres( self: SearchPostgres['Base'], diff --git a/tests/onegov/landsgemeinde/test_views.py b/tests/onegov/landsgemeinde/test_views.py index f74348897f..6a1c7b8cac 100644 --- a/tests/onegov/landsgemeinde/test_views.py +++ b/tests/onegov/landsgemeinde/test_views.py @@ -58,7 +58,7 @@ def assert_last_modified(): page.form['number'] = 5 page.form['state'] = 'completed' page.form['title'] = 'A. consectetur adipiscing\nB. tempor incididunt' - page.form['overview'] = '

Dolore magna aliqua.

' + page.form['overview'] = '

Dolore magnolia aliqua.

' page.form['text'] = '

Ad minim veniam.

' page.form['resolution'] = '

Nostrud exercitation.

' page = page.form.submit().follow() @@ -68,7 +68,7 @@ def assert_last_modified(): 'B. tempor incididunt' ) in page assert 'A. consectetur adipiscing\nB. tempor incididunt' in page - assert '

Dolore magna aliqua.

' in page + assert '

Dolore magnolia aliqua.

' in page assert '

Ad minim veniam.

' in page assert '

Nostrud exercitation.

' in page assert_last_modified() @@ -144,23 +144,47 @@ def assert_last_modified(): # search client_with_es.app.es_client.indices.refresh(index='_all') client = client_with_es.spawn() + # elasticsearch assert 'Landsgemeinde vom 07. Mai' in client.get('/search?q=ipsum') assert 'Landsgemeinde vom 07. Mai' in client.get('/search?q=adipiscing') - assert 'Landsgemeinde vom 07. Mai' in client.get('/search?q=aliqua') - assert 'Landsgemeinde vom 07. Mai' in client.get('/search?q=magna') + assert 'Landsgemeinde vom 07. Mai' in client.get('/search?q=dolore') + assert 'Landsgemeinde vom 07. Mai' in client.get('/search?q=magnoli') assert 'Landsgemeinde vom 07. Mai' in client.get('/search?q=veniam') assert 'Landsgemeinde vom 07. Mai' in client.get('/search?q=nostrud') assert 'Landsgemeinde vom 07. Mai' in client.get('/search?q=quimby') assert 'Landsgemeinde vom 07. Mai' in client.get('/search?q=mayor') - assert 'Landsgemeinde vom 07. Mai' in client.get('/search?q=laboris') + assert 'Landsgemeinde vom 07. Mai' in client.get('/search?q=ullamco') assert 'Landsgemeinde vom 07. Mai' in client.get('/search?q=aliquip') assert 'Landsgemeinde vom 07. Mai' in client.get('/search?q=consequat') + # postgres + assert 'Landsgemeinde vom 07. Mai' in client.get( + '/search-postgres?q=ipsum') + assert 'Landsgemeinde vom 07. Mai' in client.get( + '/search-postgres?q=adipiscing') + assert 'Landsgemeinde vom 07. Mai' in client.get( + '/search-postgres?q=dolore') + assert 'Landsgemeinde vom 07. Mai' in client.get( + '/search-postgres?q=magnoli') + assert 'Landsgemeinde vom 07. Mai' in client.get( + '/search-postgres?q=veniam') + assert 'Landsgemeinde vom 07. Mai' in client.get( + '/search-postgres?q=nostrud') + assert 'Landsgemeinde vom 07. Mai' in client.get( + '/search-postgres?q=quimby') + assert 'Landsgemeinde vom 07. Mai' in client.get( + '/search-postgres?q=mayor') + assert 'Landsgemeinde vom 07. Mai' in client.get( + '/search-postgres?q=ullamco') + assert 'Landsgemeinde vom 07. Mai' in client.get( + '/search-postgres?q=aliquip') + assert 'Landsgemeinde vom 07. Mai' in client.get( + '/search-postgres?q=consequat') # delete votum with freeze_time('2023-05-07 9:36'): page.click('Löschen', href='votum') page = page.click('A. consectetur adipiscing', index=0) - assert '

Dolore magna aliqua.

' in page + assert '

Dolore magnolia aliqua.

' in page assert 'Joe Quimby' not in page assert_last_modified() From 0b026c921c69ae6cdbefa25be2271c0382e74d83 Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Sat, 19 Oct 2024 08:25:16 -0400 Subject: [PATCH 37/44] Adds sql expression for GeneralFile property access --- src/onegov/org/models/file.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/onegov/org/models/file.py b/src/onegov/org/models/file.py index 6577e24a53..6c602b85a7 100644 --- a/src/onegov/org/models/file.py +++ b/src/onegov/org/models/file.py @@ -20,7 +20,7 @@ from onegov.search import ORMSearchable from operator import itemgetter from sedate import standardize_date, utcnow -from sqlalchemy import asc, desc, select, nullslast, and_ # type: ignore +from sqlalchemy import asc, desc, select, nullslast, and_, case # type: ignore from typing import ( overload, Any, Generic, Literal, NamedTuple, TypeVar, TYPE_CHECKING) @@ -249,6 +249,19 @@ def access(self) -> str: return widest_access(*self.linked_accesses.values()) + @access.expression + def access(cls): + return case([ + (cls.publication == True, 'public'), + (cls.meta['linked_accesses'] == None, 'secret'), + (cls.meta['linked_accesses'].op('?')('public'), 'public'), + (cls.meta['linked_accesses'].op('?')('secret'), 'secret'), + (cls.meta['linked_accesses'].op('?')('mtan'), 'mtan'), + (cls.meta['linked_accesses'].op('?')('secret_mtan'), + 'secret_mtan'), + (cls.meta['linked_accesses'].op('?')('member'), 'member'), + ], else_='private') + @hybrid_property def es_public(self) -> bool: return self.published and self.access == 'public' From d05926cc5ce0d4424dcbb44ea1555b536da6190f Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Mon, 21 Oct 2024 11:25:25 -0400 Subject: [PATCH 38/44] Extend tests --- tests/onegov/agency/test_views.py | 61 ++++++++++++++++++- tests/onegov/org/test_views_search.py | 87 ++++++++++++++++++++++++--- tests/onegov/pas/test_views.py | 16 +++++ 3 files changed, 152 insertions(+), 12 deletions(-) diff --git a/tests/onegov/agency/test_views.py b/tests/onegov/agency/test_views.py index 313869bf6e..d3ceb750c4 100644 --- a/tests/onegov/agency/test_views.py +++ b/tests/onegov/agency/test_views.py @@ -751,7 +751,7 @@ def test_basic_search(client_with_es): client.app.es_client.indices.refresh(index='_all') client = client.spawn() - # Test search results + # Test search results elasticsearch assert 'Rivera' in client.get('/search?q=Nick') assert 'Nick' in client.get('/search?q=Rivera') assert 'Nick' in client.get('/search?q=Doctor') @@ -765,8 +765,25 @@ def test_basic_search(client_with_es): assert 'Nick Rivera (Doctor)' in client.get('/search/suggest?q=Nic').json assert 'Rivera Nick (Doctor)' in client.get('/search/suggest?q=Riv').json assert '8911 Rivera Nick (Doctor)' in client.get( - '/search/suggest?q=89' - ).json + '/search/suggest?q=89').json + + # postgres + assert 'Rivera' in client.get('/search-postgres?q=Nick') + assert 'Nick' in client.get('/search-postgres?q=Rivera') + assert 'Nick' in client.get('/search-postgres?q=Doctor') + assert 'Nick' in client.get('/search-postgres?q=345678901') + assert 'Nick' in client.get('/search-postgres?q=345678911') + assert 'Nick' in client.get('/search-postgres?q=8911') + assert 'Hospital Springfield' in client.get('/search-postgres?q=Hospital') + assert 'Nick' in client.get('/search-postgres?q=Anesthetist') + + # Test suggestions (no autocomplete) + assert '8911 Rivera Nick (Doctor)' in client.get( + '/search-postgres/suggest?q=Nick').json + assert '8911 Rivera Nick (Doctor)' in client.get( + '/search-postgres/suggest?q=Rivera').json + assert '8911 Rivera Nick (Doctor)' in client.get( + '/search-postgres/suggest?q=8911').json @mark.flaky(reruns=3) @@ -797,6 +814,7 @@ def test_search_recently_published_object(client_with_es): client.app.es_client.indices.refresh(index='_all') + # elasticsearch assert 'Nick' in client.get('/search?q=Rivera') assert 'Nick' not in anom.get('/search?q=Rivera') assert 'Hospital Springfield' in client.get('/search?q=Hospital') @@ -804,6 +822,15 @@ def test_search_recently_published_object(client_with_es): assert 'Nick' in client.get('/search?q=Anesthetist') assert 'Nick' not in anom.get('/search?q=Anesthetist') + # postgres + assert 'Nick' in client.get('/search-postgres?q=Rivera') + assert 'Nick' not in anom.get('/search-postgres?q=Rivera') + assert 'Hospital Springfield' in client.get('/search-postgres?q=Hospital') + assert 'Hospital Springfield' not in anom.get( + '/search-postgres?q=Hospital') + assert 'Nick' in client.get('/search-postgres?q=Anesthetist') + assert 'Nick' not in anom.get('/search-postgres?q=Anesthetist') + # Publish then = utcnow() - timedelta(minutes=30) session = client.app.session() @@ -819,12 +846,28 @@ def test_search_recently_published_object(client_with_es): sleep(5) + # elasticsearch assert 'Nick' in client.get('/search?q=Rivera') assert 'Nick' in anom.get('/search?q=Rivera') + assert 'Nick' in client.spawn().get('/search?q=Rivera') assert 'Hospital Springfield' in client.get('/search?q=Hospital') assert 'Hospital Springfield' in anom.get('/search?q=Hospital') + assert 'Hospital Springfield' in client.spawn().get('/search?q=Hospital') assert 'Nick' in client.get('/search?q=Anesthetist') assert 'Nick' in anom.get('/search?q=Anesthetist') + assert 'Nick' in client.spawn().get('/search?q=Anesthetist') + + # postgres + assert 'Nick' in client.get('/search-postgres?q=Rivera') + assert 'Nick' in anom.get('/search-postgres?q=Rivera') + assert 'Nick' in client.spawn().get('/search-postgres?q=Rivera') + assert 'Hospital Springfield' in client.get('/search-postgres?q=Hospital') + assert 'Hospital Springfield' in anom.get('/search-postgres?q=Hospital') + assert 'Hospital Springfield' in client.spawn().get( + '/search-postgres?q=Hospital') + assert 'Nick' in client.get('/search-postgres?q=Anesthetist') + assert 'Nick' in anom.get('/search-postgres?q=Anesthetist') + assert 'Nick' in client.spawn().get('/search-postgres?q=Anesthetist') # Unpublish session.query(ExtendedPerson).one().publication_start = None @@ -839,13 +882,25 @@ def test_search_recently_published_object(client_with_es): sleep(5) + # elasticsearch assert 'Nick' in client.get('/search?q=Rivera') + assert 'Nick' not in anom.get('/search?q=Rivera') assert 'Nick' not in client.spawn().get('/search?q=Rivera') assert 'Hospital Springfield' in client.get('/search?q=Hospital') assert 'Hospital Springfield' not in anom.get('/search?q=Hospital') assert 'Nick' in client.get('/search?q=Anesthetist') assert 'Nick' not in anom.get('/search?q=Anesthetist') + # postgres + assert 'Nick' in client.get('/search-postgres?q=Rivera') + assert 'Nick' not in anom.get('/search-postgres?q=Rivera') + assert 'Nick' not in client.spawn().get('/search-postgres?q=Rivera') + assert 'Hospital Springfield' in client.get('/search-postgres?q=Hospital') + assert 'Hospital Springfield' not in anom.get( + '/search-postgres?q=Hospital') + assert 'Nick' in client.get('/search-postgres?q=Anesthetist') + assert 'Nick' not in anom.get('/search-postgres?q=Anesthetist') + def test_footer_settings_custom_links(client): client.login_admin() diff --git a/tests/onegov/org/test_views_search.py b/tests/onegov/org/test_views_search.py index 5ed7978814..c57f7dff8b 100644 --- a/tests/onegov/org/test_views_search.py +++ b/tests/onegov/org/test_views_search.py @@ -6,10 +6,12 @@ from datetime import datetime from datetime import timedelta +from freezegun import freeze_time + from onegov.core.utils import module_path from onegov.file import FileCollection from onegov.form import FormCollection -from onegov.page.model import Page +from onegov.org.models.page import Page from tests.onegov.org.common import get_cronjob_by_name from tests.onegov.org.common import get_cronjob_url @@ -122,9 +124,12 @@ def test_search_recently_published_object(client_with_es): client.app.es_client.indices.refresh(index='_all') - assert session.query(Page).filter( + page = session.query(Page).filter( Page.title == "Now supporting fulltext search" - ).one().published == False + ).one() + assert page.access == 'public' + assert page.published == False + assert page.es_public == False # elasticsearch assert 'fulltext' in client.get('/search?q=wow') @@ -146,16 +151,19 @@ def test_search_recently_published_object(client_with_es): ).one().publication_start = then transaction.commit() + # needed for ES job = get_cronjob_by_name(client.app, 'hourly_maintenance_tasks') job.app = client.app url = get_cronjob_url(job) client.get(url) - sleep(5) - assert session.query(Page).filter( + page = session.query(Page).filter( Page.title == "Now supporting fulltext search" - ).one().published == True + ).one() + assert page.access == 'public' + assert page.published == True + assert page.es_public == True # elasticsearch assert 'fulltext' in client.get('/search?q=wow') @@ -180,12 +188,14 @@ def test_search_recently_published_object(client_with_es): transaction.commit() client.get(url) - sleep(5) - assert session.query(Page).filter( + page = session.query(Page).filter( Page.title == "Now supporting fulltext search" - ).one().published == False + ).one() + assert page.access == 'public' + assert page.published == False + assert page.es_public == False # elasticsearch assert 'fulltext' in client.get('/search-postgres?q=wow') @@ -196,8 +206,67 @@ def test_search_recently_published_object(client_with_es): # postgres assert 'fulltext' in client.get('/search-postgres?q=wow') assert 'fulltext' not in anom.get('/search-postgres?q=wow') + assert 'fulltext' not in client.spawn().get('/search-postgres?q=wow') assert 'is pretty awesome' in client.get('/search-postgres?q=fulltext') assert 'is pretty awesome' not in anom.get('/search-postgres?q=fulltext') + assert 'is pretty awesome' not in client.spawn().get( + '/search-postgres?q=fulltext') + + +def test_search_recently_published_object_freeze(client_with_es): + # this is the same as the test above, but with freeze_time with a + # predefined period of publication + client = client_with_es + client.login_admin() + anom = client.spawn() + + # Create objects, not yet published + now = datetime.now() + start = now + timedelta(days=1) + end = now + timedelta(days=2) + + add_news = client.get('/news').click('Nachricht') + add_news.form['title'] = "Now supporting fulltext search" + add_news.form['lead'] = "It is pretty awesome" + add_news.form['text'] = "Much wow" + add_news.form['publication_start'] = start.isoformat() + add_news.form['publication_end'] = end.isoformat() + assert add_news.form.submit().follow().status_code == 200 + + # not yet published + with freeze_time(now + timedelta(hours=1)): + assert 'fulltext' in client.get('/search-postgres?q=wow') + assert 'fulltext' not in anom.get('/search-postgres?q=wow') + assert 'pretty awesome' in client.get('/search-postgres?q=fulltext') + assert 'pretty awesome' not in anom.get('/search-postgres?q=fulltext') + + # right before been published + with freeze_time(start - timedelta(seconds=1)): + assert 'fulltext' in client.get('/search-postgres?q=wow') + assert 'fulltext' not in anom.get('/search-postgres?q=wow') + assert 'pretty awesome' in client.get('/search-postgres?q=fulltext') + assert 'pretty awesome' not in anom.get('/search-postgres?q=fulltext') + + # published start + with freeze_time(start + timedelta(seconds=1)): + assert 'fulltext' in client.get('/search-postgres?q=wow') + assert 'fulltext' in anom.get('/search-postgres?q=wow') + assert 'pretty awesome' in client.get('/search-postgres?q=fulltext') + assert 'pretty awesome' in anom.get('/search-postgres?q=fulltext') + + # published before end + with freeze_time(end - timedelta(seconds=1)): + assert 'fulltext' in client.get('/search-postgres?q=wow') + assert 'fulltext' in anom.get('/search-postgres?q=wow') + assert 'pretty awesome' in client.get('/search-postgres?q=fulltext') + assert 'pretty awesome' in anom.get('/search-postgres?q=fulltext') + + # not anymore published + with freeze_time(end + timedelta(seconds=1)): + assert 'fulltext' in client.get('/search-postgres?q=wow') + assert 'fulltext' not in anom.get('/search-postgres?q=wow') + assert 'pretty awesome' in client.get('/search-postgres?q=fulltext') + assert 'pretty awesome' not in anom.get('/search-postgres?q=fulltext') @pytest.mark.flaky(reruns=3) diff --git a/tests/onegov/pas/test_views.py b/tests/onegov/pas/test_views.py index b2dd127a87..16fafc04e0 100644 --- a/tests/onegov/pas/test_views.py +++ b/tests/onegov/pas/test_views.py @@ -213,21 +213,37 @@ def test_views_manage(client_with_es): client.app.es_client.indices.refresh(index='_all') client = client.spawn() + # elasticsearch assert '0 Resultate' in client.get('/search?q=aa') assert '0 Resultate' in client.get('/search?q=bb') assert '0 Resultate' in client.get('/search?q=cc') assert '0 Resultate' in client.get('/search?q=first') assert '0 Resultate' in client.get('/search?q=2020-2024') assert '0 Resultate' in client.get('/search?q=Q1') + # postgres + assert '0 Resultate' in client.get('/search-postgres?q=aa') + assert '0 Resultate' in client.get('/search-postgres?q=bb') + assert '0 Resultate' in client.get('/search-postgres?q=cc') + assert '0 Resultate' in client.get('/search-postgres?q=first') + assert '0 Resultate' in client.get('/search-postgres?q=2020-2024') + assert '0 Resultate' in client.get('/search-postgres?q=Q1') client.login_admin() + # elasticsearch assert '1 Resultat' in client.get('/search?q=aa') assert '1 Resultat' in client.get('/search?q=bb') assert '1 Resultat' in client.get('/search?q=cc') assert '1 Resultat' in client.get('/search?q=first') assert '1 Resultat' in client.get('/search?q=2020-2024') assert '1 Resultat' in client.get('/search?q=Q1') + # postgres + assert '1 Resultat' in client.get('/search-postgres?q=aa') + assert '1 Resultat' in client.get('/search-postgres?q=bb') + assert '1 Resultat' in client.get('/search-postgres?q=cc') + assert '1 Resultat' in client.get('/search-postgres?q=first') + assert '1 Resultat' in client.get('/search-postgres?q=2020-2024') + assert '1 Resultat' in client.get('/search-postgres?q=Q1') # Delete for page in delete: From 6f939535dc0101f36170b5df80481910ff1b8ed4 Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Mon, 21 Oct 2024 12:18:56 -0400 Subject: [PATCH 39/44] Extend tests --- tests/onegov/agency/test_views.py | 15 +++++++++------ tests/onegov/feriennet/test_views.py | 20 ++++++++++++++++++++ 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/tests/onegov/agency/test_views.py b/tests/onegov/agency/test_views.py index d3ceb750c4..5df8442f94 100644 --- a/tests/onegov/agency/test_views.py +++ b/tests/onegov/agency/test_views.py @@ -1,6 +1,8 @@ from datetime import datetime from datetime import timedelta from io import BytesIO + +import pytest from markupsafe import Markup from onegov.agency.models import ExtendedAgency from onegov.agency.models import ExtendedAgencyMembership @@ -778,12 +780,13 @@ def test_basic_search(client_with_es): assert 'Nick' in client.get('/search-postgres?q=Anesthetist') # Test suggestions (no autocomplete) - assert '8911 Rivera Nick (Doctor)' in client.get( - '/search-postgres/suggest?q=Nick').json - assert '8911 Rivera Nick (Doctor)' in client.get( - '/search-postgres/suggest?q=Rivera').json - assert '8911 Rivera Nick (Doctor)' in client.get( - '/search-postgres/suggest?q=8911').json + expected = ['Nick', 'Rivera', '(Doctor)'] + assert all(v in client.get('/search-postgres/suggest?q=Nick').json[0] + for v in expected) + assert all(v in client.get('/search-postgres/suggest?q=Rivera').json[0] + for v in expected) + assert all(v in client.get('/search-postgres/suggest?q=8911').json[0] + for v in expected) @mark.flaky(reruns=3) diff --git a/tests/onegov/feriennet/test_views.py b/tests/onegov/feriennet/test_views.py index 283c5a3606..8e2f6990c6 100644 --- a/tests/onegov/feriennet/test_views.py +++ b/tests/onegov/feriennet/test_views.py @@ -284,36 +284,56 @@ def test_activity_search(client_with_es, scenario): # in preview, activities can't be found client.app.es_client.indices.refresh(index='_all') + # elasticsearch assert 'search-result-vacation' not in admin.get('/search?q=Learn') assert 'search-result-vacation' not in editor.get('/search?q=Learn') assert 'search-result-vacation' not in client.get('/search?q=Learn') + # postgres + assert 'search-result-vacation' not in admin.get('/search-postgres?q=Learn') + assert 'search-result-vacation' not in editor.get('/search-postgres?q=Learn') + assert 'search-result-vacation' not in client.get('/search-postgres?q=Learn') url = '/activity/learn-how-to-program' editor.get(url).click("Publikation beantragen") # once proposed, activities can be found by the admin only client.app.es_client.indices.refresh(index='_all') + # elasticsearch assert 'search-result-vacation' in admin.get('/search?q=Learn') assert 'search-result-vacation' not in editor.get('/search?q=Learn') assert 'search-result-vacation' not in client.get('/search?q=Learn') + # postgres + assert 'search-result-vacation' in admin.get('/search-postgres?q=Learn') + assert 'search-result-vacation' not in editor.get('/search-postgres?q=Learn') + assert 'search-result-vacation' not in client.get('/search-postgres?q=Learn') ticket = admin.get('/tickets/ALL/open').click("Annehmen").follow() ticket.click("Veröffentlichen") # once accepted, activities can be found by anyone client.app.es_client.indices.refresh(index='_all') + # elasticsearch assert 'search-result-vacation' in admin.get('/search?q=Learn') assert 'search-result-vacation' in editor.get('/search?q=Learn') assert 'search-result-vacation' in client.get('/search?q=Learn') + # postgres + assert 'search-result-vacation' in admin.get('/search-postgres?q=Learn') + assert 'search-result-vacation' in editor.get('/search-postgres?q=Learn') + assert 'search-result-vacation' in client.get('/search-postgres?q=Learn') ticket = admin.get(ticket.request.url) ticket.click("Archivieren") # archived the search will fail again, except for admins client.app.es_client.indices.refresh(index='_all') + # elasticsearch assert 'search-result-vacation' in admin.get('/search?q=Learn') assert 'search-result-vacation' not in editor.get('/search?q=Learn') assert 'search-result-vacation' not in client.get('/search?q=Learn') + # postgres + assert 'search-result-vacation' in admin.get('/search-postgres?q=Learn') + assert 'search-result-vacation' not in editor.get('/search-postgres?q=Learn') + assert 'search-result-vacation' not in client.get('/search-postgres?q=Learn') def test_activity_filter_tags(client, scenario): From 14ff52f96fc29c9fb5e952d1ddab938440e67f32 Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Tue, 22 Oct 2024 05:13:35 -0400 Subject: [PATCH 40/44] Fix mypy complaints --- src/onegov/agency/models/agency.py | 2 +- src/onegov/fsi/models/course_attendee.py | 8 ++++---- src/onegov/fsi/models/course_event.py | 2 +- src/onegov/org/models/file.py | 4 ++-- src/onegov/org/models/page.py | 2 +- src/onegov/org/models/ticket.py | 2 +- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/onegov/agency/models/agency.py b/src/onegov/agency/models/agency.py index 326370d744..839ded33c6 100644 --- a/src/onegov/agency/models/agency.py +++ b/src/onegov/agency/models/agency.py @@ -47,7 +47,7 @@ class ExtendedAgency(Agency, AccessExtension, PublicationExtension): def es_public(self) -> bool: return self.access == 'public' and self.published - @es_public.expression # ignore[no-redef] + @es_public.expression # type:ignore[no-redef] def es_public(cls) -> 'ClauseElement': return and_( cls.access == 'public', diff --git a/src/onegov/fsi/models/course_attendee.py b/src/onegov/fsi/models/course_attendee.py index 7de1be1593..fcf75a5317 100644 --- a/src/onegov/fsi/models/course_attendee.py +++ b/src/onegov/fsi/models/course_attendee.py @@ -141,7 +141,7 @@ def title(self) -> str: if part ) or self.email - @title.expression + @title.expression # type:ignore[no-redef] def title(cls) -> 'ClauseElement': has_name = and_(cls.first_name != None, cls.last_name != None) return case([ @@ -175,17 +175,17 @@ def email(self) -> str: # where it isn't allowed to be None, so we should # probably disallow it and properly deal with it # in places where it's allowed to be None - return self._email # type:ignore[return-value] + return self._email assert self.user is not None return self.user.username - @email.expression + @email.expression # type:ignore[no-redef] def email(cls) -> 'ClauseElement': return case([ (cls.user_id == None, cls._email) ], else_=cls.user.username) - @email.setter + @email.setter # type:ignore[no-redef] def email(self, value: str) -> None: self._email = value diff --git a/src/onegov/fsi/models/course_event.py b/src/onegov/fsi/models/course_event.py index 943d1c3eee..103a19beb6 100644 --- a/src/onegov/fsi/models/course_event.py +++ b/src/onegov/fsi/models/course_event.py @@ -155,7 +155,7 @@ def lead(self) -> str: def description(self) -> 'Markup': return self.course.description - @description.expression + @description.expression # type:ignore[no-redef] def description(cls) -> 'ClauseElement': from .course import Course diff --git a/src/onegov/org/models/file.py b/src/onegov/org/models/file.py index 6c602b85a7..f778709a62 100644 --- a/src/onegov/org/models/file.py +++ b/src/onegov/org/models/file.py @@ -249,7 +249,7 @@ def access(self) -> str: return widest_access(*self.linked_accesses.values()) - @access.expression + @access.expression # type:ignore[no-redef] def access(cls): return case([ (cls.publication == True, 'public'), @@ -266,7 +266,7 @@ def access(cls): def es_public(self) -> bool: return self.published and self.access == 'public' - @es_public.expression # ignore[no-redef] + @es_public.expression # type:ignore[no-redef] def es_public(cls) -> 'ClauseElement': return and_( cls.published == True, diff --git a/src/onegov/org/models/page.py b/src/onegov/org/models/page.py index a5f5eea170..0ed4b453d2 100644 --- a/src/onegov/org/models/page.py +++ b/src/onegov/org/models/page.py @@ -162,7 +162,7 @@ class News(Page, TraitInfo, SearchableContent, NewsletterExtension, def es_public(self) -> bool: return self.access == 'public' and self.published - @es_public.expression # ignore[no-redef] + @es_public.expression # type:ignore[no-redef] def es_public(cls) -> 'ClauseElement': and_( cls.access == 'public', diff --git a/src/onegov/org/models/ticket.py b/src/onegov/org/models/ticket.py index 0eecad490a..d969b18e66 100644 --- a/src/onegov/org/models/ticket.py +++ b/src/onegov/org/models/ticket.py @@ -359,7 +359,7 @@ def get_links( # type:ignore[override] url_obj = URL(request.link(self.submission)) edit_url = url_obj.query_param('edit', '').as_string() - (links if not links else extra).append( # type:ignore + (links if not links else extra).append( Link( text=_('Edit submission'), url=request.return_here(edit_url), From 6bdaa974dc3fef2a908918357bc0dfd13ad6d8f5 Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Tue, 22 Oct 2024 05:20:27 -0400 Subject: [PATCH 41/44] Performance: use scalar instead of count --- src/onegov/org/models/search.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/onegov/org/models/search.py b/src/onegov/org/models/search.py index 302d6ed471..4aaab22928 100644 --- a/src/onegov/org/models/search.py +++ b/src/onegov/org/models/search.py @@ -302,16 +302,16 @@ def generic_search(self) -> list['Searchable']: language = locale_mapping(self.request.locale or 'de_CH') ts_query = func.websearch_to_tsquery(language, func.unaccent(self.web_search)) + session = self.request.session for base in self.request.app.session_manager.bases: for model in searchable_sqlalchemy_models(base): if model.es_public or self.request.is_logged_in: - query = self.request.session.query(model) - doc_count += query.count() + query = session.query(model) if not self.request.is_logged_in: query = query.filter(model.es_public == True) - if query.count(): + if session.query(query.exists()).scalar(): weighted = ( self._create_weighted_vector(model, language)) rank_expression = func.coalesce( @@ -323,7 +323,9 @@ def generic_search(self) -> list['Searchable']: query = (query.filter(model.fts_idx.op('@@')(ts_query)) .add_columns(rank_expression)) - results.extend(list(query.all())) + res = list(query.all()) + doc_count += len(res) + results.extend(res) # remove duplicates, sort by rank results = list(set(results)) From 68b311cf45266fb34ad6839f96a82313ee5d065b Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Tue, 22 Oct 2024 05:28:17 -0400 Subject: [PATCH 42/44] Adds missing sql expressions and fixes one --- src/onegov/agency/models/membership.py | 2 +- src/onegov/agency/models/person.py | 9 ++++++++- src/onegov/event/models/occurrence.py | 11 +++++++++-- src/onegov/org/models/page.py | 10 +++++++++- 4 files changed, 27 insertions(+), 5 deletions(-) diff --git a/src/onegov/agency/models/membership.py b/src/onegov/agency/models/membership.py index be53ec80ee..a1048c6c7b 100644 --- a/src/onegov/agency/models/membership.py +++ b/src/onegov/agency/models/membership.py @@ -66,7 +66,7 @@ def es_public(cls) -> 'ClauseElement': (agency_meta['access'] != 'public', False), (agency_published != True, False), ], - else_=cls.access == 'public' + else_=cls.meta['access'] == 'public' ) # Todo: It is very unclear how this should be used. In the PDF rendering, diff --git a/src/onegov/agency/models/person.py b/src/onegov/agency/models/person.py index 49bbb00c43..9e2c53c188 100644 --- a/src/onegov/agency/models/person.py +++ b/src/onegov/agency/models/person.py @@ -1,4 +1,4 @@ -from sqlalchemy import func, select +from sqlalchemy import func, select, and_ from sqlalchemy.orm import object_session from sqlalchemy.ext.hybrid import hybrid_property @@ -30,6 +30,13 @@ class ExtendedPerson(Person, AccessExtension, PublicationExtension): def es_public(self) -> bool: return self.access == 'public' and self.published + @es_public.expression # type:ignore[no-redef] + def es_public(cls) -> 'ClauseElement': + return and_( + cls.access == 'public', + cls.published == True + ) + es_properties = { 'title': {'type': 'text'}, 'function': {'type': 'localized'}, diff --git a/src/onegov/event/models/occurrence.py b/src/onegov/event/models/occurrence.py index 186cef1d76..1e48307457 100644 --- a/src/onegov/event/models/occurrence.py +++ b/src/onegov/event/models/occurrence.py @@ -5,12 +5,13 @@ from onegov.core.orm import Base from onegov.core.orm.mixins import TimestampMixin from onegov.core.orm.types import UUID +from onegov.event.models import Event from onegov.event.models.mixins import OccurrenceMixin from onegov.gis import Coordinates from pytz import UTC from sedate import to_timezone from sedate import utcnow -from sqlalchemy import Column +from sqlalchemy import Column, select from sqlalchemy import ForeignKey from sqlalchemy.orm import relationship from uuid import uuid4 @@ -19,7 +20,7 @@ from typing import TYPE_CHECKING if TYPE_CHECKING: import uuid - from onegov.event.models import Event + from sqlalchemy.sql import ClauseElement class Occurrence(Base, OccurrenceMixin, TimestampMixin): @@ -77,3 +78,9 @@ def as_ical(self, url: str | None = None) -> bytes: @hybrid_property def access(self) -> str: return self.event.access + + @access.expression # type:ignore[no-redef] + def access(cls) -> 'ClauseElement': + return select([Event.meta['access']]).where( + Event.id == cls.event_id + ).as_scalar() diff --git a/src/onegov/org/models/page.py b/src/onegov/org/models/page.py index 0ed4b453d2..9c6f8bab9f 100644 --- a/src/onegov/org/models/page.py +++ b/src/onegov/org/models/page.py @@ -64,6 +64,14 @@ def es_skip(self) -> bool: def es_public(self) -> bool: return self.access == 'public' and self.published + @es_public.expression # type:ignore[no-redef] + def es_public(cls) -> 'ClauseElement': + retval = and_( + cls.meta['access'] == 'public', + cls.published == True + ) + return retval + @property def deletable(self) -> bool: """ Returns true if this page may be deleted. """ @@ -164,7 +172,7 @@ def es_public(self) -> bool: @es_public.expression # type:ignore[no-redef] def es_public(cls) -> 'ClauseElement': - and_( + return and_( cls.access == 'public', cls.published == True ) From bc8e1f47931ba1536e915d50716c32f3e330a1cf Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Tue, 22 Oct 2024 05:29:09 -0400 Subject: [PATCH 43/44] Cleanup --- src/onegov/org/models/search.py | 1 - tests/onegov/agency/test_views.py | 4 +--- tests/onegov/org/test_views_search.py | 3 --- tests/onegov/town6/test_views_newsletter.py | 1 - 4 files changed, 1 insertion(+), 8 deletions(-) diff --git a/src/onegov/org/models/search.py b/src/onegov/org/models/search.py index 4aaab22928..a9dcec8040 100644 --- a/src/onegov/org/models/search.py +++ b/src/onegov/org/models/search.py @@ -246,7 +246,6 @@ def load_batch_results(self) -> list[Any]: actual search results form the query. """ - print('*** tschupre cached_property load_batch_results') batch: List[Searchable] = self.batch future_events: List[Searchable] = [] other: List[Searchable] = [] diff --git a/tests/onegov/agency/test_views.py b/tests/onegov/agency/test_views.py index 5df8442f94..436fbf2b73 100644 --- a/tests/onegov/agency/test_views.py +++ b/tests/onegov/agency/test_views.py @@ -780,7 +780,7 @@ def test_basic_search(client_with_es): assert 'Nick' in client.get('/search-postgres?q=Anesthetist') # Test suggestions (no autocomplete) - expected = ['Nick', 'Rivera', '(Doctor)'] + expected = ['Nick', 'Rivera', '(Doctor)'] # word order in json is changing assert all(v in client.get('/search-postgres/suggest?q=Nick').json[0] for v in expected) assert all(v in client.get('/search-postgres/suggest?q=Rivera').json[0] @@ -882,7 +882,6 @@ def test_search_recently_published_object(client_with_es): commit() client.get(url) - sleep(5) # elasticsearch @@ -897,7 +896,6 @@ def test_search_recently_published_object(client_with_es): # postgres assert 'Nick' in client.get('/search-postgres?q=Rivera') assert 'Nick' not in anom.get('/search-postgres?q=Rivera') - assert 'Nick' not in client.spawn().get('/search-postgres?q=Rivera') assert 'Hospital Springfield' in client.get('/search-postgres?q=Hospital') assert 'Hospital Springfield' not in anom.get( '/search-postgres?q=Hospital') diff --git a/tests/onegov/org/test_views_search.py b/tests/onegov/org/test_views_search.py index c57f7dff8b..e9fa27df27 100644 --- a/tests/onegov/org/test_views_search.py +++ b/tests/onegov/org/test_views_search.py @@ -206,11 +206,8 @@ def test_search_recently_published_object(client_with_es): # postgres assert 'fulltext' in client.get('/search-postgres?q=wow') assert 'fulltext' not in anom.get('/search-postgres?q=wow') - assert 'fulltext' not in client.spawn().get('/search-postgres?q=wow') assert 'is pretty awesome' in client.get('/search-postgres?q=fulltext') assert 'is pretty awesome' not in anom.get('/search-postgres?q=fulltext') - assert 'is pretty awesome' not in client.spawn().get( - '/search-postgres?q=fulltext') def test_search_recently_published_object_freeze(client_with_es): diff --git a/tests/onegov/town6/test_views_newsletter.py b/tests/onegov/town6/test_views_newsletter.py index 651f0015a3..776f2a63f6 100644 --- a/tests/onegov/town6/test_views_newsletter.py +++ b/tests/onegov/town6/test_views_newsletter.py @@ -755,7 +755,6 @@ def test_newsletter_schedule(client): send.form.submit().follow() -# tschupre def test_newsletter_test_delivery(client): client.login_editor() From 1b0e7417cfe6aa29cb74a2323dc9fc32ba2d7469 Mon Sep 17 00:00:00 2001 From: Reto Tschuppert Date: Tue, 22 Oct 2024 05:49:28 -0400 Subject: [PATCH 44/44] Fix import --- src/onegov/event/models/occurrence.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/onegov/event/models/occurrence.py b/src/onegov/event/models/occurrence.py index 1e48307457..86dccdf8f7 100644 --- a/src/onegov/event/models/occurrence.py +++ b/src/onegov/event/models/occurrence.py @@ -5,7 +5,6 @@ from onegov.core.orm import Base from onegov.core.orm.mixins import TimestampMixin from onegov.core.orm.types import UUID -from onegov.event.models import Event from onegov.event.models.mixins import OccurrenceMixin from onegov.gis import Coordinates from pytz import UTC @@ -20,6 +19,7 @@ from typing import TYPE_CHECKING if TYPE_CHECKING: import uuid + from onegov.event.models import Event from sqlalchemy.sql import ClauseElement