Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/ogc 508 replace elastic search by postgres v2 #1357

Draft
wants to merge 49 commits into
base: master
Choose a base branch
from
Draft
Changes from 1 commit
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
ef937bc
Resolve merge conflict
Tschuppi81 Jun 3, 2024
2fdd50e
Adds view and templates for search views
Tschuppi81 Jun 4, 2024
d90e85a
Revert type wrong type annotations
Tschuppi81 Jun 5, 2024
22932f0
Sort search results after score, timestamp
Tschuppi81 Jun 5, 2024
0507b46
Adds a simple ranking
Tschuppi81 Jun 5, 2024
e6b510e
Merge branch 'master' into feature/ogc-508-replace-elastic-search-by-…
Tschuppi81 Jun 6, 2024
b8f3b76
Resolve merge conflicts
Tschuppi81 Jul 12, 2024
3beab01
Person title and user title and userprofile are now hybrid properties
Tschuppi81 Jul 16, 2024
da09357
Rework ranking
Tschuppi81 Jul 16, 2024
db8ae9b
Cleanup
Tschuppi81 Jul 18, 2024
f9ba776
Make tickets searchable
Tschuppi81 Jul 18, 2024
5718732
Make tickets searchable
Tschuppi81 Jul 18, 2024
737fb14
makes directory entries searchable
Tschuppi81 Jul 18, 2024
7846539
Revert "makes directory entries searchable"
Tschuppi81 Jul 18, 2024
cfcd6af
Resolve merge conflict
Tschuppi81 Jul 19, 2024
904a32c
Make mypy almost happy
Tschuppi81 Jul 19, 2024
956befa
Add fixme's as ranking does not work with extra localized text for su…
Tschuppi81 Jul 19, 2024
d1f8037
Merge branch 'master' into feature/ogc-508-replace-elastic-search-by-…
Tschuppi81 Aug 23, 2024
b25a51d
Make a generic class
Tschuppi81 Aug 23, 2024
007d8cb
Fix search results for tickets as not each ticket provides the values
Tschuppi81 Aug 26, 2024
57f1ea4
Transform directory_entry.keywords to hybrid property
Tschuppi81 Aug 27, 2024
abbf86c
Adds weighted vector to search query
Tschuppi81 Sep 9, 2024
4d04f5d
Resove merge conflicts
Tschuppi81 Sep 9, 2024
ac18d16
Fix extra localized text for tickets in search results
Tschuppi81 Sep 9, 2024
45eb359
Adds future events on top of search results (in case of hit)
Tschuppi81 Sep 9, 2024
d7a560a
Make load bach result a cached property
Tschuppi81 Sep 9, 2024
85eff7e
Make mypy happy
Tschuppi81 Sep 9, 2024
6114dad
Adds default implementation for extra localized text
Tschuppi81 Sep 9, 2024
71e61b6
Fixing org search template
Tschuppi81 Sep 9, 2024
bb16507
Adjust and add search template for org
Tschuppi81 Oct 3, 2024
1c86775
Exclude documents according their access level
Tschuppi81 Oct 3, 2024
b0a3fa5
Unaccent data while indexing and also for querying
Tschuppi81 Oct 4, 2024
35cac54
Convert es_public and access to hybrid properties; fix psql search fo…
Tschuppi81 Oct 7, 2024
f4a4d33
Fix: unaccent is only needed during index creation but not while quer…
Tschuppi81 Oct 7, 2024
ce27112
Fix condition and improvements
Tschuppi81 Oct 7, 2024
7c1142c
Fix agency memberships psql expression
Tschuppi81 Oct 8, 2024
59bfda0
Ensure suggestions are tuple of strings
Tschuppi81 Oct 9, 2024
7e93291
Extend and fix a few tests
Tschuppi81 Oct 14, 2024
0167dd6
More hybrid properties and fixes for such
Tschuppi81 Oct 15, 2024
045f8ad
Extend and fix tests for landsgemeinde
Tschuppi81 Oct 18, 2024
0b026c9
Adds sql expression for GeneralFile property access
Tschuppi81 Oct 19, 2024
d05926c
Extend tests
Tschuppi81 Oct 21, 2024
6f93953
Extend tests
Tschuppi81 Oct 21, 2024
14ff52f
Fix mypy complaints
Tschuppi81 Oct 22, 2024
6bdaa97
Performance: use scalar instead of count
Tschuppi81 Oct 22, 2024
68b311c
Adds missing sql expressions and fixes one
Tschuppi81 Oct 22, 2024
bc8e1f4
Cleanup
Tschuppi81 Oct 22, 2024
d5c5b5e
Resolves merge conflicts
Tschuppi81 Oct 22, 2024
1b0e741
Fix import
Tschuppi81 Oct 22, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
More hybrid properties and fixes for such
Tschuppi81 committed Oct 15, 2024
commit 0167dd63c0d4d8f0501fb36b62cec58aff5744fe
2 changes: 1 addition & 1 deletion src/onegov/agency/models/agency.py
Original file line number Diff line number Diff line change
@@ -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
)
2 changes: 1 addition & 1 deletion src/onegov/agency/models/person.py
Original file line number Diff line number Diff line change
@@ -61,15 +61,15 @@
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 = (

Check warning on line 66 in src/onegov/agency/models/person.py

Codecov / codecov/patch

src/onegov/agency/models/person.py#L66

Added line #L66 was not covered by tests
select([Organisation.agency_phone_internal_field,
Organisation.agency_phone_internal_digits])
.limit(1)
.scalar_subquery()
)
return func.substr(

Check warning on line 72 in src/onegov/agency/models/person.py

Codecov / codecov/patch

src/onegov/agency/models/person.py#L72

Added line #L72 was not covered by tests
func.replace(getattr(
cls, org_subquery.c.agency_phone_internal_field), ' ', ''),
-org_subquery.c.agency_phone_internal_field_digits
23 changes: 22 additions & 1 deletion src/onegov/feriennet/models/activity.py
Original file line number Diff line number Diff line change
@@ -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_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 @@

return organiser

@organiser.expression # ignore[no-redef]
def organizer(cls) -> 'ClauseElement':
return func.array([

Check warning on line 82 in src/onegov/feriennet/models/activity.py

Codecov / codecov/patch

src/onegov/feriennet/models/activity.py#L82

Added line #L82 was not covered by tests
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',
22 changes: 19 additions & 3 deletions src/onegov/fsi/models/course_attendee.py
Original file line number Diff line number Diff line change
@@ -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,14 +133,23 @@
cascade='all, delete-orphan'
)

@property
@hybrid_property
def title(self) -> str:
return ' '.join(
part
for part in (self.first_name, self.last_name)
if part
) or self.email

@title.expression
def title(cls) -> 'ClauseElement':
has_name = and_(cls.first_name != None, cls.last_name != None)
return case([

Check warning on line 147 in src/onegov/fsi/models/course_attendee.py

Codecov / codecov/patch

src/onegov/fsi/models/course_attendee.py#L146-L147

Added lines #L146 - L147 were not covered by tests
(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 @@
assert self.user is not None
return self.user.role

@property
@hybrid_property
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since I converted email to a hybrid_property the test test_add_edit_external_attendee fails with TypeError: 'email' is an invalid keyword argument for CourseAttendee.
If I revert back the test passed but the search-postgres?q=martin fails due to the missing hybrid property...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's very possible that setter has to be defined before the expression. Although I'm still not sure if the auto-generated __init__ for SQLAlchemy will contain settable hybrid properties, it might not.

But this whole thing seems to be heading in very much the same direction as your first attempt at implementing the indexing. Maybe it would be more robust to store the computed es properties like es_public in the database at the same time we store the index.

This way we don't have to add all these new potentially subtly incorrect hybrid properties just to use them within a SQL query and are closer to the original elastic search implementation. If we want more of these things to happen online down the road, we can always improve it later.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried the setter before the expression but it didn't help...

def email(self) -> str:
"""Needs a switch for external users"""
if not self.user_id:
@@ -169,6 +179,12 @@
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
24 changes: 21 additions & 3 deletions src/onegov/fsi/models/course_event.py
Original file line number Diff line number Diff line change
@@ -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 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

Check warning on line 140 in src/onegov/fsi/models/course_event.py

Codecov / codecov/patch

src/onegov/fsi/models/course_event.py#L140

Added line #L140 was not covered by tests

return select([Course.name]).where(

Check warning on line 142 in src/onegov/fsi/models/course_event.py

Codecov / codecov/patch

src/onegov/fsi/models/course_event.py#L142

Added line #L142 was not covered by tests
Course.id == cls.course_id
).as_scalar()

@property
def lead(self) -> str:
return (
@@ -141,10 +151,18 @@
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

Check warning on line 160 in src/onegov/fsi/models/course_event.py

Codecov / codecov/patch

src/onegov/fsi/models/course_event.py#L160

Added line #L160 was not covered by tests

return select([Course.description]).where(

Check warning on line 162 in src/onegov/fsi/models/course_event.py

Codecov / codecov/patch

src/onegov/fsi/models/course_event.py#L162

Added line #L162 was not covered by tests
Course.id == cls.course_id
).as_scalar()

def __str__(self) -> str:
start = to_timezone(
self.start, 'Europe/Zurich').strftime('%d.%m.%Y %H:%M')
2 changes: 1 addition & 1 deletion src/onegov/org/models/file.py
Original file line number Diff line number Diff line change
@@ -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'
)
Loading