From 99fb739117e4ffc52483d105d09758963b54a76c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Bompard?= Date: Mon, 13 May 2024 10:23:51 +0200 Subject: [PATCH] Add a `get_first()` method on `Message`, and don't compute the total needlessly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add a `get_first()` method on `Message` to get the first message matching a grep-like query - Don't compute the total when not necessary Signed-off-by: Aurélien Bompard --- .../datanommer/models/__init__.py | 80 ++++++++++++++++--- datanommer.models/news/+get_first.feature | 1 + datanommer.models/news/+no_total.bug | 1 + datanommer.models/tests/test_model.py | 77 +++++++++++------- 4 files changed, 117 insertions(+), 42 deletions(-) create mode 100644 datanommer.models/news/+get_first.feature create mode 100644 datanommer.models/news/+no_total.bug diff --git a/datanommer.models/datanommer/models/__init__.py b/datanommer.models/datanommer/models/__init__.py index 7dfc17c12..fcf8309f0 100644 --- a/datanommer.models/datanommer/models/__init__.py +++ b/datanommer.models/datanommer/models/__init__.py @@ -359,13 +359,10 @@ def __json__(self, request=None): return self.as_dict(request) @classmethod - def grep( + def make_query( cls, start=None, end=None, - page=1, - rows_per_page=100, - order="asc", msg_id=None, users=None, not_users=None, @@ -376,7 +373,6 @@ def grep( topics=None, not_topics=None, contains=None, - defer=False, ): """Flexible query interface for messages. @@ -404,11 +400,6 @@ def grep( (user == 'ralph') AND NOT (category == 'bodhi' OR category == 'wiki') - - ---- - - If the `defer` argument evaluates to True, the query won't actually - be executed, but a SQLAlchemy query object returned instead. """ users = users or [] @@ -468,23 +459,88 @@ def grep( if not_topics: query = query.where(not_(or_(*(Message.topic == topic for topic in not_topics)))) + return query + + @classmethod + def grep( + cls, + *, + page=1, + rows_per_page=100, + order="asc", + defer=False, + **kwargs, + ): + """Flexible query interface for messages. + + Arguments are filters. start and end should be :mod:`datetime` objs. + + Other filters should be lists of strings. They are applied in a + conjunctive-normal-form (CNF) kind of way + + for example, the following:: + + users = ['ralph', 'lmacken'] + categories = ['bodhi', 'wiki'] + + should return messages where + + (user=='ralph' OR user=='lmacken') AND + (category=='bodhi' OR category=='wiki') + + Furthermore, you can use a negative version of each argument. + + users = ['ralph'] + not_categories = ['bodhi', 'wiki'] + + should return messages where + + (user == 'ralph') AND + NOT (category == 'bodhi' OR category == 'wiki') + + ---- + + The ``jsons`` argument is a list of jsonpath filters, please refer to + `PostgreSQL's documentation + `_ + on the matter to learn how to build the jsonpath expression. + + The ``jsons_and`` argument is similar to the ``jsons`` argument, but all + the values must match for a message to be returned. + """ + query = cls.make_query(**kwargs) # Finally, tag on our pagination arguments - total = session.scalar(query.with_only_columns(func.count(Message.id))) + Message = cls + + query_total = query.with_only_columns(func.count(Message.id)) + total = None query = query.order_by(getattr(Message.timestamp, order)()) if not rows_per_page: pages = 1 else: + total = session.scalar(query_total) pages = int(math.ceil(total / float(rows_per_page))) query = query.offset(rows_per_page * (page - 1)).limit(rows_per_page) if defer: - return total, page, query + if total is None: + total = session.scalar(query_total) + return total, pages, query else: # Execute! messages = session.scalars(query).all() + if pages == 1: + total = len(messages) return total, pages, messages + @classmethod + def get_first(cls, *, order="asc", **kwargs): + """Get the first message matching the regular grep filters.""" + query = cls.make_query(**kwargs) + query = query.order_by(getattr(Message.timestamp, order)()) + return session.scalars(query).first() + class NamedSingleton: id = Column(Integer, primary_key=True, autoincrement=True) diff --git a/datanommer.models/news/+get_first.feature b/datanommer.models/news/+get_first.feature new file mode 100644 index 000000000..3059a7350 --- /dev/null +++ b/datanommer.models/news/+get_first.feature @@ -0,0 +1 @@ +Add a ``get_first()`` method on ``Message`` to get the first message matching a grep-like query \ No newline at end of file diff --git a/datanommer.models/news/+no_total.bug b/datanommer.models/news/+no_total.bug new file mode 100644 index 000000000..3c0cbc47d --- /dev/null +++ b/datanommer.models/news/+no_total.bug @@ -0,0 +1 @@ +Don't compute the total when not necessary \ No newline at end of file diff --git a/datanommer.models/tests/test_model.py b/datanommer.models/tests/test_model.py index d3b45174f..786a588e8 100644 --- a/datanommer.models/tests/test_model.py +++ b/datanommer.models/tests/test_model.py @@ -62,6 +62,15 @@ def generate_bodhi_update_complete_message(text="testing testing"): return msg +@pytest.fixture +def add_200_messages(datanommer_models): + for x in range(0, 200): + example_message = generate_message() + example_message.id = f"{x}" + dm.add(example_message) + dm.session.flush() + + def test_init_uri_and_engine(): uri = "sqlite:///db.db" engine = create_engine(uri, future=True) @@ -131,9 +140,9 @@ def test_add_missing_timestamp(datanommer_models): dbmsg = dm.session.scalar(select(dm.Message)) timediff = datetime.datetime.now() - dbmsg.timestamp - # 10 seconds between adding the message and checking + # 60 seconds between adding the message and checking # the timestamp should be more than enough. - assert timediff < datetime.timedelta(seconds=10) + assert timediff < datetime.timedelta(seconds=60) def test_add_timestamp_with_Z(datanommer_models): @@ -419,39 +428,20 @@ def test_grep_contains(datanommer_models): assert r[0].msg == example_message.body -def test_grep_rows_per_page_none(datanommer_models): - for x in range(0, 200): - example_message = generate_message() - example_message.id = f"{x}" - dm.add(example_message) - - dm.session.flush() - +def test_grep_rows_per_page(datanommer_models, add_200_messages): total, pages, messages = dm.Message.grep() assert total == 200 assert pages == 2 assert len(messages) == 100 - total, pages, messages = dm.Message.grep(rows_per_page=None) - assert total == 200 - assert pages == 1 - assert len(messages) == 200 - - -def test_grep_rows_per_page_zero(datanommer_models): - for x in range(0, 200): - example_message = generate_message() - example_message.id = f"{x}" - dm.add(example_message) - dm.session.flush() - - try: - total, pages, messages = dm.Message.grep(rows_per_page=0) - except ZeroDivisionError as e: - pytest.fail(e) - assert total == 200 - assert pages == 1 - assert len(messages) == 200 + for rows_per_page in (None, 0): + try: + total, pages, messages = dm.Message.grep(rows_per_page=rows_per_page) + except ZeroDivisionError as e: + pytest.fail(e) + assert total == 200 + assert pages == 1 + assert len(messages) == 200 def test_grep_defer(datanommer_models): @@ -466,6 +456,33 @@ def test_grep_defer(datanommer_models): assert dm.session.scalars(query).all() == dm.Message.grep()[2] +def test_grep_no_paging_and_defer(datanommer_models, add_200_messages): + total, pages, messages = dm.Message.grep(rows_per_page=0, defer=True) + assert total == 200 + assert pages == 1 + + +def test_grep_no_total_if_single_page(datanommer_models, add_200_messages, mocker): + # Assert we don't query the total of messages if we're getting them all anyway + scalar_spy = mocker.spy(dm.session, "scalar") + total, pages, messages = dm.Message.grep(rows_per_page=0) + assert total == 200 + scalar_spy.assert_not_called() + + +def test_get_first(datanommer_models): + messages = [] + for x in range(0, 200): + example_message = generate_message() + example_message.id = f"{x}" + dm.add(example_message) + messages.append(example_message) + dm.session.flush() + msg = dm.Message.get_first() + assert msg.msg_id == "0" + assert msg.msg == messages[0].body + + def test_add_duplicate(datanommer_models, caplog): example_message = generate_message() dm.add(example_message)