Skip to content

Commit

Permalink
Fix inproper bypass of invalid ticket caches
Browse files Browse the repository at this point in the history
Apparently the caches were not bypassed if it hit a None object. This means, that it would mess things up and just incorrectly cache it
  • Loading branch information
No767 committed Dec 30, 2023
1 parent bccacc7 commit 3414378
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 22 deletions.
19 changes: 8 additions & 11 deletions bot/cogs/tickets.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,24 +73,21 @@ def add_status_checklist(

@lru_cache(maxsize=64)
def get_staff(self, guild: discord.Guild) -> Optional[list[discord.Member]]:
mod_role = guild.get_role(
STAFF_ROLE
) # TODO: change the STAFF_ROLE_ID to the correct one
mod_role = guild.get_role(STAFF_ROLE)
if mod_role is None:
return None
return [member for member in mod_role.members]

### Conditions for closing tickets

async def can_close_ticket(
self, ctx: RoboContext, connection: Union[asyncpg.Pool, asyncpg.Connection]
):
connection = connection or self.pool
partial_ticket = await get_partial_ticket(self.bot, ctx.author.id, connection)
async def can_close_ticket(self, ctx: RoboContext):
partial_ticket = await get_partial_ticket(self.bot, ctx.author.id)

if partial_ticket.id is None:
return False

if (
ctx.guild is None
and partial_ticket is not None
and partial_ticket.owner_id == ctx.author.id
or await self.can_admin_close_ticket(ctx)
):
Expand Down Expand Up @@ -293,7 +290,7 @@ async def close(self, ctx: RoboContext) -> None:
"""

async with self.pool.acquire() as conn:
if await self.can_close_ticket(ctx, conn):
if await self.can_close_ticket(ctx):
owner_id = ctx.author.id
if await self.can_admin_close_ticket(ctx):
owner_id = await conn.fetchval(
Expand Down Expand Up @@ -327,7 +324,7 @@ async def ticket_info(self, ctx: RoboContext) -> None:
"""Provides information about the current ticket"""
ticket = await get_cached_thread(self.bot, ctx.author.id, self.pool)
partial_ticket = await get_partial_ticket(self.bot, ctx.author.id, self.pool)
if ticket is None or partial_ticket is None:
if ticket is None or partial_ticket.id is None:
await ctx.send(
"You have no active tickets. Please send a message to Rodhaj to get started"
)
Expand Down
6 changes: 6 additions & 0 deletions bot/libs/tickets/structs.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,9 @@ def __init__(self, record: Optional[asyncpg.Record] = None):
self.thread_id = record["thread_id"]
self.owner_id = record["owner_id"]
self.location_id = record["location_id"]

# For debugging purposes
def __repr__(self):
if self.id is None:
return f"<PartialTicket id={self.id}>"
return f"<PartialTicket id={self.id} thread_id={self.thread_id} owner_id={self.owner_id} location_id={self.location_id}>"
25 changes: 16 additions & 9 deletions bot/libs/tickets/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,10 @@ async def register_user(
return True


@alru_cache
@alru_cache(maxsize=256)
async def get_partial_ticket(
bot: Rodhaj, user_id: int, connection: Optional[asyncpg.Pool] = None
) -> Optional[PartialTicket]:
bot: Rodhaj, user_id: int, pool: Optional[asyncpg.Pool] = None
) -> PartialTicket:
"""Provides an `PartialTicket` object in order to perform various actions
The `PartialTicket` represents a partial record of an ticket found in the
Expand All @@ -49,8 +49,9 @@ async def get_partial_ticket(
of it is filled.
Args:
bot (Rodhaj): An instance of `Rodhaj`
user_id (int): ID of the user
pool (asyncpg.Pool): Pool of connections from asyncpg
pool (asyncpg.Pool): Pool of connections from asyncpg. Defaults to `None`
Returns:
PartialTicket: An representation of a "partial" ticket
Expand All @@ -60,15 +61,19 @@ async def get_partial_ticket(
FROM tickets
WHERE owner_id = $1;
"""
connection = connection or bot.pool
rows = await connection.fetchrow(query, user_id)
pool = pool or bot.pool
rows = await pool.fetchrow(query, user_id)
if rows is None:
# This represents no active ticket
return None
# In order to prevent caching invalid tickets, we need to invalidate the cache.
# By invalidating the cache, we basically "ignore" the invalid
# ticket. This essentially still leaves us with the performance boosts
# of the LRU cache, while also properly invalidating invalid tickets
get_partial_ticket.cache_invalidate(bot, user_id, pool)
return PartialTicket()
return PartialTicket(rows)


@alru_cache
@alru_cache(maxsize=64)
async def get_cached_thread(
bot: Rodhaj, user_id: int, connection: Optional[asyncpg.Pool] = None
) -> Optional[ThreadWithGuild]:
Expand All @@ -80,6 +85,7 @@ async def get_cached_thread(
Args:
bot (Rodhaj): Instance of `RodHaj`
user_id (int): ID of the user
connection (Optional[asyncpg.Pool]): Pool of connections from asyncpg. Defaults to `None`
Returns:
Optional[ThreadWithGuild]: The thread with the guild the thread belongs to.
Expand All @@ -101,6 +107,7 @@ async def get_cached_thread(
if isinstance(forum_channel, discord.ForumChannel):
thread = forum_channel.get_thread(record["thread_id"])
if thread is None:
get_cached_thread.cache_invalidate(bot, user_id, connection)
return None
return ThreadWithGuild(thread, thread.guild)

Expand Down
3 changes: 1 addition & 2 deletions bot/rodhaj.py
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,7 @@ async def on_message(self, message: discord.Message) -> None:
potential_ticket = await get_partial_ticket(self, author.id, self.pool)

# Represents that there is no active ticket
if potential_ticket is None:
# This is for the tag selection system but Noelle is still working on that
if potential_ticket.id is None:
tickets_cog: Tickets = self.get_cog("Tickets") # type: ignore
default_tags = ReservedTags(
question=False, serious=False, private=False
Expand Down

0 comments on commit 3414378

Please sign in to comment.