fixed issues concerning API Cache refresh

README.md

@@ -12,7 +12,7 @@
 
 ## ⚠️ Disclaimer concerning career pages ⚠️
 
-Players statistics are cached for performance purposes, as Blizzard pages take ~2-3 seconds to load. Since the pages are back, I noticed it's very unstable on their side, we often have a "504 Gateway Time-out" error, either on players search or career pages, sometimes a "404 Page Not Found" error even if the player exists and its profile is public.
+Players statistics are cached for performance purposes, as Blizzard pages take ~2-3 seconds to load. Since the pages are back, I noticed it's very unstable on their side, we often have a "504 Gateway Time-out" error, either on players search or career pages, sometimes a "404 Page Not Found" error even if the player exists and its profile is public, resulting in a "404 Player Not Found" response from the API.
 
 As a consequence, I configured my cache system in order to prevent (in most cases) any issue regarding pages load :
 - Career data is cached for ~2 hours

overfastapi/commands/check_and_update_cache.py

@@ -1,6 +1,8 @@
 """Command used in order to check and update Redis API Cache depending on
 the expired cache refresh limit configuration. It can be run in the background.
 """
+import re
+
 from overfastapi.common.cache_manager import CacheManager
 from overfastapi.common.enums import HeroKey
 from overfastapi.common.logging import logger
@@ -14,6 +16,8 @@
 from overfastapi.handlers.list_heroes_request_handler import ListHeroesRequestHandler
 from overfastapi.handlers.list_roles_request_handler import ListRolesRequestHandler
 
+# Mapping of cache_key prefixes to the associated
+# request handler used for cache refresh
 PREFIXES_HANDLERS_MAPPING = {
     "/heroes": ListHeroesRequestHandler,
     "/heroes/roles": ListRolesRequestHandler,
@@ -22,18 +26,32 @@
     "/players": GetPlayerCareerRequestHandler,
 }
 
+# Regular expressions for keys we don't want to refresh the cache explicitely
+# from here (either will be done in another process or not at all because not
+# relevant)
+EXCEPTION_KEYS_REGEX = {
+    r"^\/players\/[^\/]+\/(summary|stats)$",  # players summary or search
+    r"^\/players$",  # players search
+}
+
 
 def get_soon_expired_cache_keys() -> set[str]:
     """Get a set of URIs for values in API Cache which will expire soon
     without taking subroutes and query parameters"""
     cache_manager = CacheManager()
-    return {
+
+    expiring_keys = set()
+    for key in cache_manager.get_soon_expired_api_cache_keys():
         # api-cache:/heroes?role=damage => /heroes?role=damage => /heroes
-        key.split(":")[1].split("?")[0]
-        for key in cache_manager.get_soon_expired_api_cache_keys()
-        # Avoid players subroutes
-        if key.split("/")[-1].split("?")[0] not in ("summary", "stats")
-    }
+        cache_key = key.split(":")[1].split("?")[0]
+        # Avoid keys we don't want to refresh from here
+        if any(
+            re.match(exception_key, cache_key) for exception_key in EXCEPTION_KEYS_REGEX
+        ):
+            continue
+        # Add the key to the set
+        expiring_keys.add(cache_key)
+    return expiring_keys
 
 
 def get_request_handler_class_and_kwargs(cache_key: str) -> tuple[type, dict]:

overfastapi/handlers/api_request_handler.py

@@ -136,6 +136,12 @@ def update_all_api_cache(self, parsers: list[APIParser], **kwargs) -> None:
                 # page and use the kwargs to generate the appropriate URL
                 try:
                     parser = parser_class(**kwargs)
+                except ParserInitError as error:
+                    logger.error(
+                        "Failed to instanciate Parser when refreshing : {}",
+                        error.message,
+                    )
+                    return
                 except HTTPException:
                     return
 

overfastapi/main.py

@@ -24,7 +24,7 @@
 
 ## ⚠️ Disclaimer concerning career pages ⚠️
 
-Players statistics are cached for performance purposes, as Blizzard pages take ~2-3 seconds to load. Since the pages are back, I noticed it's very unstable on their side, we often have a "504 Gateway Time-out" error, either on players search or career pages, sometimes a "404 Page Not Found" error even if the player exists and its profile is public.
+Players statistics are cached for performance purposes, as Blizzard pages take ~2-3 seconds to load. Since the pages are back, I noticed it's very unstable on their side, we often have a "504 Gateway Time-out" error, either on players search or career pages, sometimes a "404 Page Not Found" error even if the player exists and its profile is public, resulting in a "404 Player Not Found" response from the API.
 
 As a consequence, I configured my cache system in order to prevent (in most cases) any issue regarding pages load :
 - Career data is cached for ~2 hours

overfastapi/parsers/api_parser.py

@@ -78,7 +78,6 @@ def parse(self) -> None:
             self.cache_key, self.hash
         )
         if parser_cache_data:
-
             # Parser cache is already valid, no need to do anything
             logger.info("Parser Cache found !")
             self.data = json.loads(parser_cache_data)

overfastapi/routers/players.py

@@ -37,7 +37,10 @@
 async def get_player_common_parameters(
     player_id: str = Path(
         title="Player unique name",
-        description='Identifier of the player : BattleTag (with "#" replaced by "-")',
+        description=(
+            'Identifier of the player : BattleTag (with "#" replaced by "-"). '
+            "Be careful, letter case (capital/non-capital letters) is important !"
+        ),
         example="TeKrop-2217",
     ),
 ):

pyproject.toml

@@ -1,6 +1,6 @@
 [tool.poetry]
 name = "overfast-api"
-version = "2.3.1"
+version = "2.3.2"
 description = "Overwatch API giving data about heroes, maps, and players statistics."
 authors = ["TeKrop <tekrop@gmail.com>"]
 license = "MIT"

tests/commands/test_check_and_update_cache.py

@@ -288,7 +288,7 @@ def test_check_and_update_specific_player_to_update(
     )
 
 
-def test_check_error_from_blizzard(cache_manager: CacheManager):
+def test_check_internal_error_from_blizzard(cache_manager: CacheManager):
     # Add some data (to update and not to update)
     cache_manager.update_api_cache("/heroes/ana", "{}", EXPIRED_CACHE_REFRESH_LIMIT - 5)
 
@@ -326,7 +326,7 @@ def test_check_timeout_from_blizzard(cache_manager: CacheManager):
 
 
 @pytest.mark.parametrize("player_html_data", ["TeKrop-2217"], indirect=True)
-def test_check_parsing_error(cache_manager: CacheManager, player_html_data: str):
+def test_check_parser_parsing_error(cache_manager: CacheManager, player_html_data: str):
     # Add some data (to update and not to update)
     cache_manager.update_api_cache(
         "/players/TeKrop-2217", "{}", EXPIRED_CACHE_REFRESH_LIMIT - 5
@@ -348,3 +348,32 @@ def test_check_parsing_error(cache_manager: CacheManager, player_html_data: str)
         "https://overwatch.blizzard.com/en-us/career/TeKrop-2217/",
         "AttributeError(\"'NoneType' object has no attribute 'find'\")",
     )
+
+
+@pytest.mark.parametrize("player_html_data", ["Unknown-1234"], indirect=True)
+def test_check_parser_init_error(cache_manager: CacheManager, player_html_data: str):
+    # Add some data (to update and not to update)
+    cache_manager.update_api_cache(
+        "/players/TeKrop-2217", "{}", EXPIRED_CACHE_REFRESH_LIMIT - 5
+    )
+
+    logger_error_mock = Mock()
+    with patch(
+        "requests.get",
+        return_value=Mock(status_code=200, text=player_html_data),
+    ), patch("overfastapi.common.logging.logger.error", logger_error_mock):
+        check_and_update_cache_main()
+
+    logger_error_mock.assert_any_call(
+        "Failed to instanciate Parser when refreshing : {}", "Player not found"
+    )
+
+
+def test_check_players_search_never_to_refresh(cache_manager: CacheManager):
+    # The key will expire soon but shouldn't be in the list of keys to refresh
+    cache_manager.update_api_cache(
+        "/players?name=TeKrop", "{}", EXPIRED_CACHE_REFRESH_LIMIT - 5
+    )
+    # Another one that should be
+    cache_manager.update_api_cache("/heroes", "{}", EXPIRED_CACHE_REFRESH_LIMIT - 5)
+    assert get_soon_expired_cache_keys() == {"/heroes"}