When querying for the first message, don't look earlier than the user…

…'s creation

Signed-off-by: Aurélien Bompard <[email protected]>

fedbadges/cached.py

@@ -13,6 +13,8 @@
 from dogpile.cache.util import kwarg_function_key_generator
 from fedora_messaging.message import Message as FMMessage
 
+from .utils import get_fas_user
+
 
 log = logging.getLogger(__name__)
 cache = make_region()
@@ -50,7 +52,7 @@ class CachedDatanommerMessage:
     def __init__(self, message: FMMessage):
         self.msg_id = message.id
         self.topic = message.topic
-        self.timestamp = (datetime.datetime.now(tz=datetime.timezone.utc),)
+        self.timestamp = datetime.datetime.now(tz=datetime.timezone.utc)
         self.msg = message.body
         self.headers = message._properties.headers
         self.users = message.usernames
@@ -70,8 +72,9 @@ def count(self):
 
 class CachedValue:
 
-    def __init__(self):
+    def __init__(self, fasjson=None):
         self._key_generator = kwarg_function_key_generator(self.__class__.__name__, self.compute)
+        self._fasjson = fasjson
 
     def _get_key(self, **kwargs):
         return self._key_generator(**kwargs).replace(" ", "|")
@@ -157,13 +160,39 @@ def _year_split_query(self, **grep_kwargs):
     def _first_message_timestamp(self, **grep_kwargs):
         key = self._get_key(**grep_kwargs)
         key = f"{key}|first_timestamp"
+        now = datetime.datetime.now(tz=datetime.timezone.utc)
         get_first_kwargs = grep_kwargs.copy()
         # remove grep() args that are not allowed by get_first()
         for kwarg in ("defer", "rows_per_page", "page"):
             with suppress(KeyError):
                 del get_first_kwargs[kwarg]
 
+        def _get_user_creation_time(username):
+            user = cache.get_or_create(
+                f"fas_user|{username}",
+                get_fas_user,
+                # short expiration time in case the user changes something in their account
+                expiration_time=300,
+                # Don't cache on 404
+                should_cache_fn=lambda result: result is not None,
+                creator_args=[(username, self.fasjson)],
+            )
+            if user is None:
+                return None
+            return datetime.datetime.fromisoformat(user["creation"])
+
         def _get_first_timestamp(**kwargs):
+            if "users" in kwargs and "start" not in kwargs:
+                # Optimization: don't search before the user was created
+                kwargs["start"] = None
+                for username in kwargs["users"]:
+                    user_creation_time = _get_user_creation_time(username)
+                    if user_creation_time is not None:
+                        if kwargs["start"] is None or user_creation_time > kwargs["start"]:
+                            kwargs["start"] = user_creation_time
+                if kwargs["start"] is not None and "end" not in kwargs:
+                    kwargs["end"] = now
+
             log.debug("Getting first DN message for: %r", kwargs)
             first_message = Message.get_first(**kwargs)
             return first_message.timestamp if first_message is not None else None

fedbadges/rules.py

@@ -310,6 +310,12 @@ def __repr__(self):
     def matches(self, msg):
         pass
 
+    def get_top_parent(self):
+        parent = self.parent
+        while hasattr(parent, "parent") and parent.parent is not None:
+            parent = parent.parent
+        return parent
+
 
 class AbstractTopLevelComparator(AbstractComparator):
     def __init__(self, *args, **kwargs):
@@ -449,6 +455,8 @@ def __init__(self, *args, **kwargs):
 
         # Construct a condition callable for later
         self.condition = functools.partial(self.condition_callbacks[condition_key], condition_val)
+        top_parent = self.get_top_parent()
+        self.fasjson = top_parent.fasjson
 
     def _construct_query(self, msg):
         """Construct a datanommer query for this message.
@@ -487,7 +495,7 @@ def _try_cache_or_make_query(self, msg: Message):
         search_kwargs = self._construct_query(msg)
         # Try cached values
         for CachedValue in DATANOMMER_CACHED_VALUES:
-            cached_value = CachedValue()
+            cached_value = CachedValue(fasjson=self.fasjson)
             if not cached_value.is_applicable(search_kwargs, self._d):
                 log.debug(
                     "%s with kwargs %r is not applicable to %r",

fedbadges/utils.py

@@ -158,7 +158,7 @@ def notification_callback(message):
 
 def user_exists_in_fas(fasjson, user):
     """Return true if the user exists in FAS."""
-    return nick2fas(user, fasjson) is not None
+    return get_fas_user(user, fasjson) is not None
 
 
 def get_pagure_authors(authors):
@@ -189,16 +189,24 @@ def _fasjson_backoff_hdlr(details):
     max_tries=3,
     on_backoff=_fasjson_backoff_hdlr,
 )
-def nick2fas(nick, fasjson):
+def get_fas_user(username, fasjson):
     """Return the user in FAS."""
     try:
-        return fasjson.get_user(username=nick).result["username"]
+        return fasjson.get_user(username=username).result
     except fasjson_client.errors.APIError as e:
         if e.code == 404:
             return None
         raise
 
 
+def nick2fas(nick, fasjson):
+    """Return the user in FAS."""
+    fas_user = get_fas_user(nick, fasjson)
+    if fas_user is None:
+        return None
+    return fas_user["username"]
+
+
 def email2fas(email, fasjson):
     """Return the user with the specified email in FAS."""
     if email.endswith("@fedoraproject.org"):