Add a get_first() method on Message, and don't compute the total …

…needlessly

- 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 <[email protected]>

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
+        <https://www.postgresql.org/docs/current/functions-json.html#FUNCTIONS-SQLJSON-PATH>`_
+        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)

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

datanommer.models/news/+no_total.bug

@@ -0,0 +1 @@
+Don't compute the total when not necessary

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)