From db11a7bcb0574bc516d56081d51662f17a0aa810 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Bompard?= Date: Thu, 30 May 2024 18:16:16 +0200 Subject: [PATCH] When querying for the first message, don't look earlier than the user's creation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Aurélien Bompard --- fedbadges/cached.py | 33 +++++++++++++++++++++++++++++++-- fedbadges/rules.py | 10 +++++++++- fedbadges/utils.py | 14 +++++++++++--- 3 files changed, 51 insertions(+), 6 deletions(-) diff --git a/fedbadges/cached.py b/fedbadges/cached.py index 431b8f9..89a7e56 100644 --- a/fedbadges/cached.py +++ b/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 diff --git a/fedbadges/rules.py b/fedbadges/rules.py index 38cf06f..f585ee8 100644 --- a/fedbadges/rules.py +++ b/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", diff --git a/fedbadges/utils.py b/fedbadges/utils.py index e8bf66f..48c1988 100644 --- a/fedbadges/utils.py +++ b/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"):