From 3c6fd21f350abb13b7a67c8241f3c6539bd86f1c Mon Sep 17 00:00:00 2001 From: Nathaniel Landau Date: Wed, 9 Aug 2023 21:06:21 -0400 Subject: [PATCH] refactor(character): improve character service (#32) --- src/valentina/character/traits.py | 2 +- src/valentina/cogs/characters.py | 4 +- src/valentina/models/characters.py | 374 ++++++++++++----- tests/models/test_characters.py | 635 ++++++++++++++++++++--------- 4 files changed, 718 insertions(+), 297 deletions(-) diff --git a/src/valentina/character/traits.py b/src/valentina/character/traits.py index 2ac95b57..17e76fff 100644 --- a/src/valentina/character/traits.py +++ b/src/valentina/character/traits.py @@ -46,7 +46,7 @@ async def add_trait( ) await view.wait() if view.confirmed: - ctx.bot.char_svc.add_trait( # type: ignore [attr-defined] + ctx.bot.char_svc.add_custom_trait( # type: ignore [attr-defined] character, name=trait_name, description=trait_description, diff --git a/src/valentina/cogs/characters.py b/src/valentina/cogs/characters.py index 70b60a07..2c21b7e2 100644 --- a/src/valentina/cogs/characters.py +++ b/src/valentina/cogs/characters.py @@ -199,7 +199,7 @@ async def unclaim_character( """Unclaim currently claimed character. This will allow you to claim a new character.""" if self.bot.char_svc.user_has_claim(ctx): character = self.bot.char_svc.fetch_claim(ctx) - self.bot.char_svc.remove_claim(ctx) + self.bot.char_svc.remove_claim(ctx.guild.id, ctx.author.id) await present_embed( ctx=ctx, title="Character Unclaimed", @@ -275,7 +275,7 @@ async def date_of_birth( ) @add.command(name="trait", description="Add a custom trait to a character") - async def add_trait( + async def add_custom_trait( self, ctx: discord.ApplicationContext, trait: Option(str, "The new trait to add.", required=True), diff --git a/src/valentina/models/characters.py b/src/valentina/models/characters.py index 89fa528c..2ea410d7 100644 --- a/src/valentina/models/characters.py +++ b/src/valentina/models/characters.py @@ -4,7 +4,7 @@ from discord import ApplicationContext, AutocompleteContext from loguru import logger -from peewee import DoesNotExist, ModelSelect +from peewee import DoesNotExist from valentina.models.constants import MaxTraitValue from valentina.models.db_tables import ( @@ -26,12 +26,23 @@ class CharacterService: """A service for managing the Player characters in the cache/in-memory database.""" def __init__(self) -> None: - """Initialize the CharacterService.""" - # Caches to avoid database queries - ################################## - self.characters: dict[str, Character] = {} # {char_key: Character, ...} - self.storyteller_character_cache: dict[int, list[Character]] = {} # {guild_id: Character} - self.claims: dict[str, str] = {} # {claim_key: char_key} + """Initialize the CharacterService. + + This method sets up three dictionaries that serve as caches to avoid unnecessary database queries. + + Attributes: + characters (dict[str, Character]): A dictionary mapping character keys to Character instances. + storyteller_character_cache (dict[int, list[Character]]): A dictionary mapping guild IDs to lists of Character instances. + claims (dict[str, str]): A dictionary mapping claim keys to character keys. + """ + # Initialize a dictionary to store characters, using character keys as keys and Character instances as values + self.character_cache: dict[str, Character] = {} + + # Initialize a dictionary to store storyteller characters, using guild IDs as keys and lists of Character instances as values + self.storyteller_character_cache: dict[int, list[Character]] = {} + + # Initialize a dictionary to store claims, using claim keys as keys and character keys as values + self.claim_cache: dict[str, str] = {} @staticmethod def __get_char_key(guild_id: int, char_id: int) -> str: @@ -44,34 +55,57 @@ def __get_char_key(guild_id: int, char_id: int) -> str: Returns: str: The guild and character IDs joined by an underscore. """ + # Generate a unique key for the character by joining the guild ID and character ID with an underscore return f"{guild_id}_{char_id}" @staticmethod def __get_claim_key(guild_id: int, user_id: int) -> str: """Generate a key for the claim cache. + This method generates a unique key for each claim by joining the guild ID and user ID with an underscore. + This key is used to store and retrieve claims from the cache. + Args: - guild_id (int): The guild ID - user_id (int): The user database ID + guild_id (int): The ID of the guild. + user_id (int): The ID of the user in the database. Returns: - str: The guild and user IDs joined by an underscore. + str: The generated key, consisting of the guild ID and user ID joined by an underscore. """ + # Generate a unique key for the claim by joining the guild ID and user ID with an underscore return f"{guild_id}_{user_id}" def add_claim(self, guild_id: int, char_id: int, user_id: int) -> bool: - """Claim a character for a user.""" + """Claim a character for a user. + + This method allows a user to claim a character. It first generates the character key and claim key. If the claim already exists for the user and character, it returns True. If the character is already claimed by another user, it raises a CharacterClaimedError. Otherwise, it adds the claim to the claims dictionary and returns True. + + Args: + guild_id (int): The ID of the guild. + char_id (int): The ID of the character in the database. + user_id (int): The ID of the user in the database. + + Returns: + bool: True if the claim is successfully added or already exists, False otherwise. + + Raises: + CharacterClaimedError: If the character is already claimed by another user. + """ + # Generate the character key and claim key char_key = self.__get_char_key(guild_id, char_id) claim_key = self.__get_claim_key(guild_id, user_id) - if claim_key in self.claims and self.claims[claim_key] == char_key: + # If the claim already exists for the user and character, return True + if claim_key in self.claim_cache and self.claim_cache[claim_key] == char_key: return True - if any(char_key == claim for claim in self.claims.values()): + # If the character is already claimed by another user, raise a CharacterClaimedError + if any(char_key == claim for claim in self.claim_cache.values()): logger.debug(f"CLAIM: Character {char_id} is already claimed") raise CharacterClaimedError - self.claims[claim_key] = char_key + # Add the claim to the claims dictionary + self.claim_cache[claim_key] = char_key return True def add_custom_section( @@ -80,7 +114,18 @@ def add_custom_section( section_title: str | None = None, section_description: str | None = None, ) -> bool: - """Add or update a custom section to a character.""" + """Add or update a custom section to a character. + + This method creates a new custom section with the given title and description and associates it with the specified character. + + Args: + character (Character): The character object to which the custom section will be added. + section_title (str | None): The title of the custom section. Defaults to None. + section_description (str | None): The description of the custom section. Defaults to None. + + Returns: + bool: True if the custom section was successfully added, False otherwise. + """ CustomSection.create( title=section_title, description=section_description, @@ -90,7 +135,7 @@ def add_custom_section( logger.debug(f"DATABASE: Add custom section to {character}") return True - def add_trait( + def add_custom_trait( self, character: Character, name: str, @@ -99,15 +144,15 @@ def add_trait( value: int, max_value: int = MaxTraitValue.DEFAULT.value, ) -> None: - """Create a custom trait for a specified character. + """Create and add a custom trait to a specified character. Args: - character (Character): The character to which the trait is to be added. - name (str): The name of the trait. - description (str): The description of the trait. - category (TraitCategory): The category of the trait. - value (int): The value of the trait. - max_value (int, optional): The maximum value that the trait can have. Defaults to MaxTraitValue.DEFAULT.value. + character (Character): Character receiving the trait. + name (str): Trait name. + description (str): Trait description. + category (TraitCategory): Trait category. + value (int): Trait value. + max_value (int, optional): Trait's maximum value. Defaults to MaxTraitValue.DEFAULT.value. Returns: None @@ -126,13 +171,6 @@ def add_trait( logger.debug(f"CHARACTER: Add trait '{name}' to {character}") - def is_cached_char( - self, guild_id: int | None = None, char_id: int | None = None, key: str | None = None - ) -> bool: - """Check if the user is in the cache.""" - key = self.__get_char_key(guild_id, char_id) if key is None else key - return key in self.characters - def create_character(self, ctx: ApplicationContext, **kwargs: str | int) -> Character: """Create a character in the cache and database.""" # Normalize kwargs keys to database column names @@ -154,140 +192,254 @@ def create_character(self, ctx: ApplicationContext, **kwargs: str | int) -> Char return character - def fetch_all_characters(self, guild_id: int) -> ModelSelect: - """Returns all characters for a specific guild. Checks the cache first and then the database. If characters are found in the database, they are added to the cache. + def fetch_all_characters(self, guild_id: int) -> list[Character]: + """Fetch all characters for a specific guild, checking the cache first and then the database. Args: - guild_id (int): The discord guild id to fetch characters for. + guild_id (int): The Discord guild ID to fetch characters for. Returns: - ModelSelect: A peewee ModelSelect object representing all the characters for the guild. + list[Character]: List of characters for the guild. """ - cached_ids = [] - chars_to_return = [] - - for key, character in self.characters.items(): - if key.startswith(str(guild_id)): - cached_ids.append(character.id) - chars_to_return.append(character) - logger.debug(f"CACHE: Fetch {len(chars_to_return)} characters") - + # Fetch characters from cache + cached_chars = [ + character + for key, character in self.character_cache.items() + if key.startswith(str(guild_id)) + ] + cached_ids = [character.id for character in cached_chars] + logger.debug(f"CACHE: Fetch {len(cached_chars)} characters") + + # Fetch characters from database not in cache characters = Character.select().where( - (Character.guild_id == guild_id) # grab only characters for the guild - & (Character.storyteller_character == False) # grab only player characters # noqa: E712 - & ~(Character.id.in_(cached_ids)) # grab only characters not in cache + (Character.guild_id == guild_id) + & (Character.storyteller_character == False) # noqa: E712 + & (Character.id.not_in(cached_ids)) + ) + logger.debug( + f"DATABASE: Fetch {len(characters)} characters" + if characters + else "DATABASE: No characters to fetch" ) - if len(characters) > 0: - logger.debug(f"DATABASE: Fetch {len(characters)} characters") - else: - logger.debug("DATABASE: No characters to fetch") + # Add characters from database to cache for character in characters: - self.characters[self.__get_char_key(guild_id, character.id)] = character - chars_to_return.append(character) + key = self.__get_char_key(guild_id, character.id) + self.character_cache[key] = character - return chars_to_return + return cached_chars + list(characters) def fetch_all_storyteller_characters( self, ctx: ApplicationContext | AutocompleteContext = None, guild_id: int | None = None ) -> list[Character]: - """Fetch all StoryTeller characters for a guild.""" - if guild_id is None and isinstance(ctx, ApplicationContext): - guild_id = ctx.guild.id - if guild_id is None and isinstance(ctx, AutocompleteContext): - guild_id = ctx.interaction.guild.id - - if guild_id in self.storyteller_character_cache: - logger.debug( - f"CACHE: Return {len(self.storyteller_character_cache[guild_id])} StoryTeller characters" - ) - return self.storyteller_character_cache[guild_id] + """Fetch all StoryTeller characters for a guild, checking the cache first and then the database. + + Args: + ctx (ApplicationContext | AutocompleteContext, optional): Context object containing guild information. + guild_id (int, optional): The Discord guild ID to fetch characters for. If not provided, it will be extracted from ctx. - self.storyteller_character_cache[guild_id] = [] + Returns: + list[Character]: List of StoryTeller characters for the guild. + """ + # Determine guild_id from the context if not provided + if guild_id is None: + if isinstance(ctx, ApplicationContext): + guild_id = ctx.guild.id + elif isinstance(ctx, AutocompleteContext): + guild_id = ctx.interaction.guild.id + + # Initialize cache for guild_id if not present + if guild_id not in self.storyteller_character_cache: + self.storyteller_character_cache[guild_id] = [] + + # Fetch cached characters' IDs + cached_ids = [character.id for character in self.storyteller_character_cache[guild_id]] + logger.debug(f"CACHE: Fetch {len(cached_ids)} StoryTeller characters") + + # Query the database for StoryTeller characters not in cache characters = Character.select().where( (Character.guild_id == guild_id) & (Character.storyteller_character == True) # noqa: E712 + & (Character.id.not_in(cached_ids)) ) + # Log the number of characters fetched from the database logger.debug(f"DATABASE: Fetch {len(characters)} StoryTeller characters") - self.storyteller_character_cache[guild_id] = [x for x in characters] + + # Update the cache with the fetched characters + self.storyteller_character_cache[guild_id] += list(characters) + return self.storyteller_character_cache[guild_id] def fetch_claim(self, ctx: ApplicationContext | AutocompleteContext) -> Character: - """Fetch the character claimed by a user.""" + """Fetch the character claimed by a user based on the context provided. + + This method tries to fetch the character claimed by a user from cache if available, + otherwise, it fetches the character from the database using the character ID. + + Args: + ctx (ApplicationContext | AutocompleteContext): The context which contains the author and guild information. + + Returns: + Character: The claimed character. + + Raises: + NoClaimError: If no claim is found for the given context. + """ if isinstance(ctx, ApplicationContext): - author = ctx.author - guild = ctx.guild - if isinstance(ctx, AutocompleteContext): # pragma: no cover - author = ctx.interaction.user - guild = ctx.interaction.guild + author, guild = ctx.author, ctx.guild + else: + author, guild = ctx.interaction.user, ctx.interaction.guild claim_key = self.__get_claim_key(guild.id, author.id) - if claim_key in self.claims: - char_key = self.claims[claim_key] - if self.is_cached_char(key=char_key): - logger.debug(f"CACHE: Fetch character {self.characters[char_key]}") - return self.characters[char_key] + try: + char_key = self.claim_cache[claim_key] + except KeyError as e: + raise NoClaimError from e + + if self.is_cached_character(key=char_key): + character = self.character_cache[char_key] + logger.debug(f"CACHE: Fetch character {character} for author {author} in guild {guild}") + return character - char_id = re.sub(r"[a-zA-Z0-9]+_", "", char_key) - return Character.get_by_id(int(char_id)) + char_id = re.sub(r"[a-zA-Z0-9]+_", "", char_key) + return Character.get_by_id(int(char_id)) - raise NoClaimError + def fetch_user_of_character(self, guild_id: int, char_id: int) -> int | None: + """Returns the user ID of the user who claimed a character. + + Args: + guild_id (int): The Discord guild ID to fetch the user for. + char_id (int): The character ID to fetch the user for. - def fetch_user_of_character(self, guild_id: int, char_id: int) -> int: - """Returns the user id of the user who claimed a character.""" + Returns: + int | None: The user ID of the user who claimed the character, or None if the character is not claimed. + """ + # Check if the character is claimed if self.is_char_claimed(guild_id, char_id): char_key = self.__get_char_key(guild_id, char_id) - for claim_key, claim in self.claims.items(): - if claim == char_key: - user_id = re.sub(r"[a-zA-Z0-9]+_", "", claim_key) - return int(user_id) + + # Find the claim key that matches the character key + return next( + ( + int(re.sub(r"[a-zA-Z0-9]+_", "", claim_key)) + for claim_key, claim in self.claim_cache.items() + if claim == char_key + ), + None, + ) return None + def is_cached_character( + self, guild_id: int | None = None, char_id: int | None = None, key: str | None = None + ) -> bool: + """Check if the character is in the cache using either a guild ID and character ID or a specified key. + + Args: + guild_id (int, optional): The guild ID of the character. Defaults to None. + char_id (int, optional): The character ID. Defaults to None. + key (str, optional): The key representing the character in the cache. If not provided, it will be generated using guild_id and char_id. Defaults to None. + + Returns: + bool: True if the character is in the cache, False otherwise. + """ + key = key or self.__get_char_key(guild_id, char_id) + return key in self.character_cache + def is_char_claimed(self, guild_id: int, char_id: int) -> bool: - """Check if a character is claimed by any user.""" + """Check if a character is claimed by any user. + + Args: + guild_id (int): The Discord guild ID to check the claim for. + char_id (int): The character ID to check the claim for. + + Returns: + bool: True if the character is claimed, False otherwise. + """ char_key = self.__get_char_key(guild_id, char_id) - return any(char_key == claim for claim in self.claims.values()) + return any(char_key == claim for claim in self.claim_cache.values()) def purge_cache(self, ctx: ApplicationContext | None = None, with_claims: bool = False) -> None: - """Purge all character caches. If ctx is provided, only purge the caches for that guild.""" - caches: dict[str, dict] = {"characters": self.characters} + """Purge all character caches. If ctx is provided, only purge the caches for that guild. + + Args: + ctx (ApplicationContext | None, optional): Context object containing guild information. If provided, only caches for that guild are purged. + with_claims (bool, optional): If True, also purge the claims cache. Defaults to False. + + Returns: + None + """ + # Initialize caches to purge + caches: dict[str, dict] = {"characters": self.character_cache} if with_claims: - caches["claims"] = self.claims + caches["claims"] = self.claim_cache if ctx: + # Purge caches for the specific guild self.storyteller_character_cache.pop(ctx.guild.id, None) - for _cache_name, cache in caches.items(): - for key in cache.copy(): - if key.startswith(str(ctx.guild.id)): - cache.pop(key, None) + for cache in caches.values(): + keys_to_remove = [key for key in cache if key.startswith(str(ctx.guild.id))] + for key in keys_to_remove: + cache.pop(key, None) logger.debug(f"CACHE: Purge character caches for guild {ctx.guild}") else: + # Purge all character caches self.storyteller_character_cache = {} for cache in caches.values(): cache.clear() logger.debug("CACHE: Purge all character caches") - def remove_claim(self, ctx: ApplicationContext) -> bool: - """Remove a claim from a user.""" - claim_key = self.__get_claim_key(ctx.guild.id, ctx.author.id) - if claim_key in self.claims: - logger.debug(f"CLAIM: Remove claim for user {ctx.author}") - del self.claims[claim_key] + def remove_claim(self, guild_id: int, user_id: int) -> bool: + """Remove a user's claim. + + This method removes a user's claim from the claims dictionary. If the claim exists, it deletes the claim and returns True. + If the claim doesn't exist, it returns False. + + Args: + guild_id (int): The ID of the guild. + user_id (int): The ID of the user in the database. + + Returns: + bool: True if the claim is successfully removed, False otherwise. + """ + # Generate the claim key + claim_key = self.__get_claim_key(guild_id, user_id) + + # If the claim exists, delete it from the claims dictionary and return True + if claim_key in self.claim_cache: + del self.claim_cache[claim_key] return True + + # If the claim doesn't exist, return False return False def user_has_claim(self, ctx: ApplicationContext) -> bool: - """Check if a user has a claim.""" + """Check if a user has a claim. + + Args: + ctx (ApplicationContext): Context object containing guild and author information. + + Returns: + bool: True if the user has a claim, False otherwise. + """ claim_key = self.__get_claim_key(ctx.guild.id, ctx.author.id) - return claim_key in self.claims + return claim_key in self.claim_cache def update_character( self, ctx: ApplicationContext, char_id: int, **kwargs: str | int ) -> Character: - """Update a character in the cache and database.""" + """Update a character in the cache and database. + + Args: + ctx (ApplicationContext): Context object containing guild information. + char_id (int): The character ID to update. + **kwargs (str | int): Additional keyword arguments to update the character attributes. + + Returns: + Character: The updated character object. + """ key = self.__get_char_key(ctx.guild.id, char_id) try: @@ -295,16 +447,19 @@ def update_character( except DoesNotExist as e: raise CharacterNotFoundError(e=e) from e + # Update the character in the database Character.update(modified=time_now(), **kwargs).where( Character.id == character.id ).execute() - # Clear caches - if not character.storyteller_character: - self.characters.pop(key, None) + # Re-query the character object to reflect the changes + character = Character.get_by_id(char_id) + # Clear caches based on character type if character.storyteller_character: self.storyteller_character_cache.pop(ctx.guild.id, None) + else: + self.character_cache.pop(key, None) logger.debug(f"DATABASE: Update character: {character}") return character @@ -312,16 +467,15 @@ def update_character( def update_traits_by_id( self, ctx: ApplicationContext, character: Character, trait_values_dict: dict[int, int] ) -> None: - """Update traits for a character by id. + """Update traits for a character by ID. Args: ctx (ApplicationContext): The context of the command. character (Character): The character to update. trait_values_dict (dict[int, int]): A dictionary of trait IDs and their new values. """ - key = self.__get_char_key(ctx.guild.id, character.id) # Clear character from cache but keep claims intact - self.characters.pop(key, None) + self.purge_cache(ctx) modified = time_now() diff --git a/tests/models/test_characters.py b/tests/models/test_characters.py index 2c29f8b6..a70be6ea 100644 --- a/tests/models/test_characters.py +++ b/tests/models/test_characters.py @@ -4,7 +4,14 @@ import pytest from valentina.models import CharacterService -from valentina.models.db_tables import Character, CustomSection, CustomTrait, Trait, TraitCategory +from valentina.models.db_tables import ( + Character, + CustomSection, + CustomTrait, + Trait, + TraitCategory, + TraitValue, +) from valentina.utils.errors import ( CharacterClaimedError, CharacterNotFoundError, @@ -18,101 +25,462 @@ class TestCharacterService: char_svc = CharacterService() - def test_add_claim_one(self): - """Test add_claim(). + def test_character_service_init(self): + """test_character_service_init. - Given a guild, character, and user - When a claim is added - Then the claim is added to claim cache + GIVEN a CharacterService instance. + WHEN the __init__ method is called + THEN check the dictionaries are initialized correctly. """ - assert self.char_svc.claims == {} - self.char_svc.add_claim(guild_id=1, char_id=1, user_id=1) - assert self.char_svc.claims == {"1_1": "1_1"} + # Check that the dictionaries are initialized as empty dictionaries + assert self.char_svc.character_cache == {} + assert self.char_svc.storyteller_character_cache == {} + assert self.char_svc.claim_cache == {} + + def test_get_char_key(self): + """Test get_char_key().""" + # GIVEN a guild ID and a character ID + guild_id = 123 + char_id = 456 + + # WHEN the __get_char_key method is called + # THEN check the correct key is generated + assert ( + self.char_svc._CharacterService__get_char_key(guild_id, char_id) + == f"{guild_id}_{char_id}" + ) - def test_add_claim_two(self): - """Test add_claim(). + def test_get_claim_key(self): + """Test get_claim_key().""" + # GIVEN a guild ID and a user ID + guild_id = 123 + user_id = 456 + + # WHEN the __get_claim_key method is called + # THEN check the correct key is generated + assert ( + self.char_svc._CharacterService__get_claim_key(guild_id, user_id) + == f"{guild_id}_{user_id}" + ) - Given a guild, character, and user - When a user has a claim and claims another character - Then the new claim replaces the old claim - """ - self.char_svc.add_claim(guild_id=1, char_id=1, user_id=1) - self.char_svc.add_claim(guild_id=1, char_id=2, user_id=1) - assert self.char_svc.claims == {"1_1": "1_2"} + def test_add_claim(self): + """Test add_claim().""" + # GIVEN a guild ID, a character ID, and a user ID, and an empty cache + guild_id = 123 + char_id = 456 + user_id = 789 + assert self.char_svc.claim_cache == {} + + # WHEN the add_claim method is called for the first time + # THEN check the claim is added correctly + assert self.char_svc.add_claim(guild_id, char_id, user_id) is True + assert ( + self.char_svc._CharacterService__get_claim_key(guild_id, user_id) + in self.char_svc.claim_cache + ) + assert self.char_svc.claim_cache == {f"{guild_id}_{user_id}": f"{guild_id}_{char_id}"} - def test_add_claim_three(self): - """Test add_claim(). + # GIVEN the same guild ID and user ID, but a different character ID + char_id = 999 - Given a guild, character, and user - When another has the requested character claimed - Then CharacterClaimedError is raised - """ - self.char_svc.add_claim(guild_id=1, char_id=1, user_id=1) + # WHEN the add_claim method is called again + # THEN check the method returns True and the claim still exists + assert self.char_svc.add_claim(guild_id, char_id, user_id) is True + assert ( + self.char_svc._CharacterService__get_claim_key(guild_id, user_id) + in self.char_svc.claim_cache + ) + assert self.char_svc.claim_cache == {f"{guild_id}_{user_id}": f"{guild_id}_{char_id}"} + + # GIVEN the same guild ID and character ID, but a different user ID + user_id = 999 + + # WHEN the add_claim method is called + # THEN check the method raises a CharacterClaimedError with pytest.raises(CharacterClaimedError): - self.char_svc.add_claim(guild_id=1, char_id=1, user_id=22) + self.char_svc.add_claim(guild_id, char_id, user_id) + + def test_add_custom_section(self): + """Test add_custom_section().""" + # GIVEN a character object with no custom sections + character = Character.create( + first_name="add_custom_section", + last_name="character", + nickname="testy", + char_class=1, + guild=1, + created_by=1, + clan=1, + ) + assert character.custom_sections == [] + + # WHEN the add_custom_section method is called + assert self.char_svc.add_custom_section(character, "new", "new description") is True + + # THEN check the custom section is added correctly + assert len(character.custom_sections) == 1 + assert character.custom_sections[0].title == "new" + assert character.custom_sections[0].description == "new description" + + def test_add_custom_trait(self): + """Test add_custom_trait().""" + # GIVEN a Character object and a TraitCategory object + character = Character.create( + first_name="add_custom_trait", + last_name="character", + nickname="testy", + char_class=1, + guild=1, + created_by=1, + clan=1, + ) + assert len(character.custom_traits) == 0 + category = TraitCategory.get_by_id(1) + + # WHEN the add_custom_trait method is called + self.char_svc.add_custom_trait(character, "new_trait", "new description", category, 1, 5) + + # THEN check the custom trait is added correctly + assert len(character.custom_traits) == 1 + assert character.custom_traits[0].name == "New_Trait" + assert character.custom_traits[0].description == "New Description" + assert character.custom_traits[0].category == category + assert character.custom_traits[0].value == 1 + assert character.custom_traits[0].max_value == 5 + + def test_fetch_all_characters(self, caplog): + """Test fetch_all_characters().""" + # GIVEN two characters for a guild, with one in the cache + guild_id = 123321 + character1 = Character.create( + first_name="fetch_all_characters1", + last_name="character1", + nickname="testy", + char_class=1, + guild=guild_id, + created_by=1, + clan=1, + ) + character2 = Character.create( + first_name="fetch_all_characters2", + last_name="character2", + nickname="testy", + char_class=1, + guild=guild_id, + created_by=1, + clan=1, + ) + self.char_svc.character_cache[f"{guild_id}_{character1.id}"] = character1 + + # WHEN the fetch_all_characters method is called + result = self.char_svc.fetch_all_characters(guild_id) + returned = caplog.text + + # THEN check the method returns the correct characters from the cache and the database and updates the cache + assert len(result) == 2 + assert result[0] == character1 + assert result[1] == character2 + assert "CACHE: Fetch 1 characters" in returned + assert "DATABASE: Fetch 1 characters" in returned + assert [ + character + for key, character in self.char_svc.character_cache.items() + if key.startswith(str(guild_id)) + ] == [character1, character2] + + def test_fetch_all_storyteller_characters(self, caplog): + """Test fetch_all_storyteller_characters().""" + # GIVEN two storyteller characters for a guild, with one in the cache + guild_id = 123321 + character1 = Character.create( + first_name="fetch_all_storyteller_characters1", + last_name="character1", + nickname="testy", + char_class=1, + guild=guild_id, + created_by=1, + clan=1, + storyteller_character=True, + ) + character2 = Character.create( + first_name="fetch_all_storyteller_characters2", + last_name="character2", + nickname="testy", + char_class=1, + guild=guild_id, + created_by=1, + clan=1, + storyteller_character=True, + ) + self.char_svc.storyteller_character_cache[guild_id] = [character1] - def test_add_custom_section_one(self): - """Test add_custom_section(). + # WHEN the fetch_all_storyteller_characters method is called + result = self.char_svc.fetch_all_storyteller_characters(guild_id=guild_id) + returned = caplog.text - Given a ctx object and a character - When a custom section is added - Then the custom section is added to the database - """ + # THEN check the method returns the correct characters from the cache and the database and updates the cache + assert result == [character1, character2] + assert "CACHE: Fetch 1 StoryTeller characters" in returned + assert "DATABASE: Fetch 1 StoryTeller characters" in returned + assert self.char_svc.storyteller_character_cache[guild_id] == [character1, character2] + + def test_fetch_claim(self, mock_ctx): + """Test fetch_claim().""" + # GIVEN a context object for a user with no claim + # WHEN the fetch_claim method is called + # THEN check the method raises a NoClaimError + with pytest.raises(NoClaimError): + self.char_svc.fetch_claim(mock_ctx) + + # GIVEN a character object and a context object for a user with a claim character = Character.get_by_id(1) + self.char_svc.add_claim(mock_ctx.guild.id, character.id, mock_ctx.author.id) + + # WHEN the fetch_claim method is called + # THEN check the method returns the correct character + assert self.char_svc.fetch_claim(mock_ctx) == character + + def test_fetch_user_of_character(self): + """Test fetch_user_of_character().""" + # GIVEN a guild ID, a character ID, and a claim in the cache + guild_id = 12333111222 + user_id = 1 + character = Character.create( + first_name="fetch_user_of_character", + last_name="character", + nickname="testy", + char_class=1, + guild=guild_id, + created_by=user_id, + clan=1, + ) + self.char_svc.add_claim(guild_id, character.id, user_id) + + # WHEN the fetch_user_of_character method is called with a claimed character + result = self.char_svc.fetch_user_of_character(guild_id, character.id) + + # THEN check the method returns the correct user ID + assert result == user_id + + # WHEN the fetch_user_of_character method is called with an unclaimed character + result = self.char_svc.fetch_user_of_character(guild_id, 999) + + # THEN check the method returns None + assert result is None + + def test_is_cached_character(self): + """Test is_cached_character().""" + # GIVEN a guild ID and a character ID + guild_id = 123 + char_id = 456 + + # WHEN the is_cached_character method is called + # THEN check the method returns False + assert self.char_svc.is_cached_character(guild_id, char_id) is False + + # GIVEN a guild ID, a character ID, and a character in the cache + self.char_svc.character_cache[f"{guild_id}_{char_id}"] = "test" + + # WHEN the is_cached_character method is called with a guild ID and character ID + # THEN check the method returns True + assert self.char_svc.is_cached_character(guild_id, char_id) is True + + # WHEN the is_cached_character method is called with a character_key + # THEN check the method returns True + assert self.char_svc.is_cached_character(key=f"{guild_id}_{char_id}") is True + + def test_is_char_claimed(self): + """Test is_char_claimed().""" + # GIVEN a guild id, a character id, and a claimed character + guild_id = 123 + char_id = 456 + user_id = 789 + self.char_svc.add_claim(guild_id, char_id, user_id) + + # WHEN the is_char_claimed method is called with a guild ID and character ID + # THEN check the method returns True if the character is claimed and False otherwise + assert self.char_svc.is_char_claimed(guild_id, char_id) is True + assert self.char_svc.is_char_claimed(guild_id, 999) is False + + def test_purge_cache(self, mock_ctx): + """Test purge_cache().""" + self.char_svc.character_cache = {} + self.char_svc.storyteller_character_cache = {} + self.char_svc.claim_cache = {} + + # GIVEN a guild ID, a character ID, and a character in the cache + for i in [1, 2]: + self.char_svc.character_cache[f"{i}_{i}"] = "test" + self.char_svc.storyteller_character_cache[i] = ["test1", "test2"] + self.char_svc.claim_cache[f"{i}_{i}"] = "test" + + # WHEN the purge_cache method is called with a context object + self.char_svc.purge_cache(mock_ctx) + + # THEN all caches are purged for the matching guild ID but the claims are not + assert self.char_svc.character_cache == {"2_2": "test"} + assert self.char_svc.storyteller_character_cache == {2: ["test1", "test2"]} + assert self.char_svc.claim_cache == {"1_1": "test", "2_2": "test"} + + # WHEN the purge_cache method is called with a context object and with_claims=True + self.char_svc.purge_cache(mock_ctx, with_claims=True) + + # THEN all caches are purged for the matching guild ID and the claims are purged + assert self.char_svc.character_cache == {"2_2": "test"} + assert self.char_svc.storyteller_character_cache == {2: ["test1", "test2"]} + assert self.char_svc.claim_cache == {"2_2": "test"} + + # WHEN the purge_cache method is called without a context object and with_claims=False + self.char_svc.purge_cache() - self.char_svc.add_custom_section(character, "new", "new description") - section = CustomSection.get(CustomSection.id == 2) - assert section.title == "new" - section.delete_instance() + # THEN all caches are purged + assert self.char_svc.character_cache == {} + assert self.char_svc.storyteller_character_cache == {} + assert self.char_svc.claim_cache == {"2_2": "test"} + + # WHEN the purge_cache method is called without a context object and with_claims=True + self.char_svc.purge_cache(with_claims=True) + + # THEN all caches are purged + assert self.char_svc.character_cache == {} + assert self.char_svc.storyteller_character_cache == {} + assert self.char_svc.claim_cache == {} + + def test_remove_claim(self): + """Test remove_claim().""" + # GIVEN a guild ID, a character ID, and a user ID, and a claim in the cache + guild_id = 123 + char_id = 456 + user_id = 789 + self.char_svc.add_claim(guild_id, char_id, user_id) + assert ( + self.char_svc._CharacterService__get_claim_key(guild_id, user_id) + in self.char_svc.claim_cache + ) + + # WHEN the remove_claim method is called + # THEN check the claim is removed correctly + assert self.char_svc.remove_claim(guild_id, user_id) is True + assert ( + self.char_svc._CharacterService__get_claim_key(guild_id, user_id) + not in self.char_svc.claim_cache + ) - def test_add_trait_one(self): - """Test add_trait(). + # GIVEN the same guild ID and user ID, but the claim is no longer in the cache + # WHEN the remove_claim method is called again + # THEN check the method returns False + assert self.char_svc.remove_claim(guild_id, user_id) is False - Given a ctx object and a character - When a trait is added - Then the trait is added to the database and cache - """ - self.char_svc.add_trait( - character=Character.get_by_id(1), - name="test_trait2", - value=2, - category=TraitCategory.get(name="Skills"), - description="test_description", + def test_user_has_claim(self, mock_ctx): + """Test user_has_claim().""" + # GIVEN a context object for a user with a character claim + character = Character.create( + first_name="user_has_claim", + last_name="character", + nickname="testy", + char_class=1, + guild=1, + created_by=1, + clan=1, ) + self.char_svc.claim_cache[ + f"{mock_ctx.guild.id}_{mock_ctx.author.id}" + ] = f"{mock_ctx.guild.id}_{character.id}" - # Added to database - trait = CustomTrait.get(CustomTrait.id == 2) - assert trait.name == "Test_Trait2" - - @pytest.mark.parametrize( - ( - "id", - "expected", - ), - [(1, True), (789, False)], - ) - def test_is_cached_char(self, id, expected) -> bool: - """Test is_cached_char(). - - Given a character id - When is_cached_char is called - Then the expected result is returned - """ - self.char_svc.fetch_all_characters(1) - assert self.char_svc.is_cached_char(1, id) == expected + # WHEN the user_has_claim method is called + # THEN check the method returns True + assert self.char_svc.user_has_claim(mock_ctx) is True - def test_fetch_all_characters_one(self): - """Test fetch_all_characters(). + # GIVEN a context object for a user with no character claim + # WHEN the user_has_claim method is called + # THEN check the method returns False + self.char_svc.claim_cache = {} + assert self.char_svc.user_has_claim(mock_ctx) is False - Given a guild - When fetch_all_characters is called - Then all characters associated with that guild are returned - """ - self.char_svc.purge_cache() - assert len(self.char_svc.characters) == 0 - characters = self.char_svc.fetch_all_characters(1) - assert len(self.char_svc.characters) == 2 - assert len(characters) == 2 + def test_update_character_one(self, mock_ctx): + """Test update_character().""" + # GIVEN a character object that is in the cache + character = Character.create( + first_name="update_character", + last_name="character", + nickname="testy", + char_class=1, + guild=mock_ctx.guild.id, + created_by=mock_ctx.author.id, + clan=1, + ) + self.char_svc.character_cache[f"{mock_ctx.guild.id}_{character.id}"] = character + + # WHEN the update_character method is called + updates = {"first_name": "updated", "last_name": "updated", "nickname": "updated"} + result = self.char_svc.update_character(mock_ctx, character.id, **updates) + + # THEN check the character is updated correctly + assert result.first_name == "updated" + assert result.last_name == "updated" + assert result.nickname == "updated" + assert f"{mock_ctx.guild.id}_{character.id}" not in self.char_svc.character_cache + + def test_update_character_two(self, mock_ctx): + """Test update_character().""" + # GIVEN a storyteller character object that is in the cache + character = Character.create( + first_name="update_character_two", + last_name="character", + nickname="testy", + char_class=1, + guild=mock_ctx.guild.id, + created_by=mock_ctx.author.id, + clan=1, + storyteller_character=True, + ) + self.char_svc.storyteller_character_cache[mock_ctx.guild.id] = [character] + + # WHEN the update_character method is called + updates = {"first_name": "updated", "last_name": "updated", "nickname": "updated"} + result = self.char_svc.update_character(mock_ctx, character.id, **updates) + + # THEN check the character is updated correctly + assert result.first_name == "updated" + assert result.last_name == "updated" + assert result.nickname == "updated" + assert mock_ctx.guild.id not in self.char_svc.storyteller_character_cache + + def test_update_character_three(self, mock_ctx): + """Test update_character().""" + # GIVEN a character ID that does not exist in the database + char_id = 99999912345 + + # WHEN the update_character method is called + # THEN check the method raises a CharacterNotFoundError + with pytest.raises( + CharacterNotFoundError, match="The requested character could not be found" + ): + self.char_svc.update_character(mock_ctx, char_id) + + def test_update_traits_by_id(self, mock_ctx): + """Test update_traits_by_id().""" + # GIVEN a character object and traits and a single trait value lookup + character = Character.create( + first_name="update_traits_by_id", + last_name="character", + nickname="testy", + char_class=1, + guild=mock_ctx.guild.id, + created_by=mock_ctx.author.id, + clan=1, + ) + trait1 = Trait.create(name="test_trait1", category=1) + trait2 = Trait.create(name="test_trait2", category=1) + TraitValue.create(character=character, trait=trait1, value=1) + + # WHEN the update_traits_by_id method is called + updates = {trait1.id: 2, trait2.id: 3} + self.char_svc.update_traits_by_id(mock_ctx, character, updates) + + # THEN check the trait values are updated correctly + assert character.trait_values[0].value == 2 + assert character.trait_values[1].value == 3 def test_character_traits_dict(self): """Test character.traits_dict. @@ -125,11 +493,11 @@ def test_character_traits_dict(self): assert returned == { "Physical": [Trait.get_by_id(1), Trait.get_by_id(2), Trait.get_by_id(3)], - "Skills": [CustomTrait.get_by_id(1), CustomTrait.get_by_id(2)], + "Skills": [CustomTrait.get_by_id(1)], } def test_character_traits_list(self): - """Test character.traits_list). + """Test character.traits_list. Given a character with traits When character.all_traits_list is called as a flat list @@ -141,7 +509,6 @@ def test_character_traits_list(self): Trait.get_by_id(3), Trait.get_by_id(1), CustomTrait.get_by_id(1), - CustomTrait.get_by_id(2), ] def test_character_all_trait_values(self): @@ -158,21 +525,7 @@ def test_character_all_trait_values(self): ("Dexterity", 2, 5, "●●○○○"), ("Stamina", 3, 5, "●●●○○"), ] - assert returned["Skills"] == [ - ("Test_Trait", 2, 5, "●●○○○"), - ("Test_Trait2", 2, 5, "●●○○○"), - ] - - def test_char_custom_sections(self): - """Test character.custom_sections. - - Given a character with custom sections - When character.custom_sections is called - Then all custom sections are returned - """ - returned = Character.get_by_id(1).custom_sections - assert len(returned) == 1 - assert returned[0].title == "test_section" + assert returned["Skills"] == [("Test_Trait", 2, 5, "●●○○○")] def test_character_custom_traits(self): """Test character.custom_traits. @@ -182,91 +535,5 @@ def test_character_custom_traits(self): Then all custom traits are returned """ returned = Character.get_by_id(1).custom_traits - assert len(returned) == 2 + assert len(returned) == 1 assert returned[0].name == "Test_Trait" - assert returned[1].name == "Test_Trait2" - - def test_fetch_claim(self, mock_ctx): - """Test fetch_claim(). - - Given a guild, character, and user - When fetch_claim is called - Then the claimed character is returned or NoClaimError is raised - """ - # Raise NoClaimError if no claim exists - self.char_svc.claims = {} - with pytest.raises(NoClaimError): - self.char_svc.fetch_claim(mock_ctx) - - # Return the claim if it exists in cache - self.char_svc.add_claim(guild_id=1, char_id=1, user_id=1) - returned = self.char_svc.fetch_claim(mock_ctx) - assert returned.id == 1 - assert returned.name == "Test (Testy) Character" - - # Return the claim if it exists in database - self.char_svc.purge_cache() - self.char_svc.add_claim(guild_id=1, char_id=1, user_id=1) - returned = self.char_svc.fetch_claim(mock_ctx) - assert returned.id == 1 - assert returned.name == "Test (Testy) Character" - - @pytest.mark.parametrize( - ("char_id", "expected"), - [ - (1, 1), - (2, None), - (3, None), - ], - ) - def test_fetch_user_of_character(self, char_id, expected): - """Test fetch_user_of_character(). - - Given a character id and a guild id - When fetch_user_of_character is called - Then fetch the user who claimed the character, if any - """ - self.char_svc.purge_cache() - self.char_svc.add_claim(guild_id=1, char_id=1, user_id=1) - assert self.char_svc.fetch_user_of_character(1, char_id) == expected - - def test_remove_claim(self, mock_ctx): - """Test remove_claim(). - - Given a ctx object - When remove_claim is called - Then the character is unclaimed - """ - self.char_svc.add_claim(guild_id=1, char_id=1, user_id=1) - assert self.char_svc.remove_claim(mock_ctx) is True - assert self.char_svc.remove_claim(mock_ctx) is False - - def test_user_has_claim(self, mock_ctx): - """Test user_has_claim(). - - Given a ctx object - When user_has_claim is called - Then True is returned if the user has a claim and False otherwise - """ - self.char_svc.remove_claim(mock_ctx) - assert self.char_svc.user_has_claim(mock_ctx) is False - self.char_svc.add_claim(guild_id=1, char_id=1, user_id=1) - assert self.char_svc.user_has_claim(mock_ctx) is True - - def test_update_character(self, mock_ctx): - """Test update_character(). - - Given a character id and a dict of updates - When update_character is called - Then the character is updatedin the database and purged from the cache - """ - character = Character.get_by_id(1) - assert character.experience == 0 - - self.char_svc.update_character(mock_ctx, 1, **{"experience": 5}) - character = Character.get_by_id(1) - assert character.experience == 5 - assert "1_1" not in self.char_svc.characters - - with pytest.raises(CharacterNotFoundError): - self.char_svc.update_character(mock_ctx, 12345678, **{"experience": 5})