From ddf91a38dff0c2ae916794457542c3aade3cf3db Mon Sep 17 00:00:00 2001 From: Mackenzie Grimes - NOAA Affiliate Date: Tue, 27 Jan 2026 14:16:34 -0700 Subject: [PATCH 1/4] accept PUT method (overwrite old Profile) instead of PATCH (attempts partial update) --- .pre-commit-config.yaml | 2 +- python/nwsc_proxy/ncp_web_service.py | 6 +- python/nwsc_proxy/src/profile_store.py | 60 ++++++++++--------- .../nwsc_proxy/test/test_ncp_web_service.py | 14 ++--- python/nwsc_proxy/test/test_profile_store.py | 29 ++++++--- 5 files changed, 66 insertions(+), 45 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index e7efb58..6686c23 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,7 +1,7 @@ repos: # Using this mirror lets us use mypyc-compiled black, which is about 2x faster - repo: https://github.com/psf/black-pre-commit-mirror - rev: 25.1.0 + rev: 26.1.0 hooks: - id: black # It is recommended to specify the latest version of Python diff --git a/python/nwsc_proxy/ncp_web_service.py b/python/nwsc_proxy/ncp_web_service.py index 2402aa1..92f974c 100644 --- a/python/nwsc_proxy/ncp_web_service.py +++ b/python/nwsc_proxy/ncp_web_service.py @@ -66,7 +66,7 @@ def handler(self): if request.method == "DELETE": return self._handle_delete() - if request.method == "PATCH": + if request.method == "PUT": return self._handle_update() # otherwise, must be 'GET' operation @@ -126,7 +126,7 @@ def _handle_create(self) -> Response: def _handle_update(self) -> Response: if not request.data: - return jsonify({"message": "PATCH requires request body"}), 400 + return jsonify({"message": "PUT requires request body"}), 400 request_body: dict = request.json profile_id = request.args.get("id", request.args.get("uuid")) @@ -161,7 +161,7 @@ def __init__(self, base_dir: str): "/all-events", "events", view_func=events_route.handler, - methods=["GET", "POST", "PATCH", "DELETE"], + methods=["GET", "POST", "PUT", "DELETE"], ) def run(self, **kwargs): diff --git a/python/nwsc_proxy/src/profile_store.py b/python/nwsc_proxy/src/profile_store.py index 21184ed..b71566b 100644 --- a/python/nwsc_proxy/src/profile_store.py +++ b/python/nwsc_proxy/src/profile_store.py @@ -12,14 +12,15 @@ import os import json import logging -from dataclasses import dataclass + +# from dataclasses import dataclass from datetime import datetime, UTC from glob import glob from math import inf from dateutil.parser import parse as dt_parse -from src.utils import deep_update +# from src.utils import deep_update # constants controlling the subdirectory where new vs. existing Profiles are saved NEW_SUBDIR = "new" @@ -29,7 +30,6 @@ logger = logging.getLogger(__name__) -@dataclass class CachedProfile: """Data class to hold Support Profile's data and metadata ("new" vs "existing" status), as well as some derived properties extracted from the `data` JSON (e.g. @@ -41,31 +41,31 @@ class CachedProfile: is_new (bool): track if Support Profile has ever been processed. Ought to start as True """ - # pylint: disable=invalid-name - data: dict - is_new: bool + def __init__(self, data: dict, is_new: bool): + self.is_new = is_new + self._data = data @property def id(self) -> str: """The Support Profile UUID""" - return self.data.get("id") + return self._data.get("id") @property def name(self) -> str: """The Support Profile name""" - return self.data.get("name") + return self._data.get("name") @property def is_active(self) -> bool: """The Support Profile's active state (can be marked as inactive to halt processing)""" - return self.data.get("isLive") + return self._data.get("isLive") @property def start_timestamp(self) -> float: """The Support Profile event's start in Unix time (milliseconds since the epoch). math.inf if Support Profile is never-ending """ - profile_start: str | None = self.data["setting"]["timing"].get("start") + profile_start: str | None = self._data["setting"]["timing"].get("start") return dt_parse(profile_start).timestamp() if profile_start else inf @property @@ -75,7 +75,7 @@ def end_timestamp(self) -> float: """ if self.start_timestamp == inf: return inf # infinite start time, so infinite end time as well - timing: dict[str, int] = self.data["setting"]["timing"] + timing: dict[str, int] = self._data["setting"]["timing"] # look up durationInMinutes, or deprecated duration, or None profile_duration = timing.get("durationInMinutes", timing.get("duration", inf)) return self.start_timestamp + profile_duration * 60 * 1000 # convert mins to ms @@ -87,12 +87,20 @@ def data_sources(self) -> list[str]: return [ # treat any profiles with empty string dataSource as default 'NBM' _map["dataSource"] if _map["dataSource"] != "" else DEFAULT_DATA_SOURCE - for phrase in self.data["nonImpactThresholds"]["phrasesForAllSeverities"].values() + for phrase in self._data["nonImpactThresholds"]["phrasesForAllSeverities"].values() for _map in phrase["map"].values() ] except KeyError: return [DEFAULT_DATA_SOURCE] # couldn't lookup dataSources, so just default to NBM + def save_to_file(self, parent_dir: str) -> str | None: + """Commit the CachedProfile object to a local file in the given parent directory. + + Returns: + (str | None): written filepath, if successful, or None on error + """ + raise NotImplementedError + def __str__(self): return ( f"{self.__class__.__name__}(id='{self.id}', name='{self.name}', is_new={self.is_new}, " @@ -166,9 +174,9 @@ def get_all(self, data_source="ANY", is_new=False) -> list[dict]: ] if data_source == "ANY": # all data sources requested, so do not filter by products used - return [profile.data for profile in profiles_by_status] + return [profile._data for profile in profiles_by_status] return [ - profile.data for profile in profiles_by_status if data_source in profile.data_sources + profile._data for profile in profiles_by_status if data_source in profile.data_sources ] def save(self, profile: dict, is_new=True) -> str | None: @@ -251,37 +259,35 @@ def update(self, profile_id: str, data: dict) -> dict: Args: profile_id (str): The UUID of the Support Profile to update - data (dict): The JSON attributes to apply. Can be partial Support Profile + data (dict): The JSON attributes to apply. Must be complete Support Profile Returns: - dict: the latest version of the Profile, with all attribute changes applied + dict: the latest version of the Profile, with the profile overwritten by `data`, or + `None` on error (existing profile will be unchanged) Raises: FileNotFoundError: if no Support Profile exists with the provided id """ - logger.info("Updating profile_id %s with new attributes: %s", profile_id, data) + logger.info("Updating profile_id %s with new data: %s", profile_id, data) - # find the profile data from the new_profiles cache, then apply updates and save over it + # find the profile data from the new_profiles cache, then save over it cached_profile = next( (profile for profile in self.profile_cache if profile.id == profile_id), None ) if not cached_profile: raise FileNotFoundError # Profile with this ID does not exist in cache - # Apply dict of edits on top of existing cached profile (combining any nested attributes) - new_profile_data = deep_update(cached_profile.data, data) - is_new_profile = cached_profile.is_new - # a bit hacky, but the fastest and least duplicative way to update a Profile - # in cache + disk is to delete the existing profile and re-save with modified JSON data + # in cache + disk is to delete the existing profile and re-save with new JSON data self.delete(profile_id) - update_success = self.save(new_profile_data, is_new_profile) - - if not update_success: + saved_id = self.save(data, cached_profile.is_new) + if not saved_id: logger.warning("Unable to update Profile ID %s for some reason", profile_id) + # attempt to rollback, recovering the previously deleted profile + self.save(cached_profile._data, cached_profile.is_new) return None - return new_profile_data + return data def delete(self, profile_id: str) -> bool: """Delete a Support Profile profile from storage, based on its UUID. diff --git a/python/nwsc_proxy/test/test_ncp_web_service.py b/python/nwsc_proxy/test/test_ncp_web_service.py index 0216a75..8f6db8a 100644 --- a/python/nwsc_proxy/test/test_ncp_web_service.py +++ b/python/nwsc_proxy/test/test_ncp_web_service.py @@ -233,20 +233,20 @@ def test_delete_profile_failure(wrapper: AppWrapper, mock_request: Mock, mock_pr def test_update_profile_success(wrapper: AppWrapper, mock_request: Mock, mock_profile_store: Mock): - mock_request.method = "PATCH" + mock_request.method = "PUT" mock_request.args = MultiDict({"id": EXAMPLE_UUID}) - mock_request.json = {"name": "Some new name"} - updated_profile = {"id": EXAMPLE_UUID, "name": "Some new name"} + expected_data = {"id": EXAMPLE_UUID, "name": "Some new name"} + mock_request.json = expected_data + mock_profile_store.return_value.update.return_value = expected_data - mock_profile_store.return_value.update.return_value = updated_profile result: tuple[Response, int] = wrapper.app.view_functions["events"]() assert result[1] == 200 - assert result[0].json["profile"] == updated_profile + assert result[0].json["profile"] == expected_data def test_update_no_body(wrapper: AppWrapper, mock_request: Mock, mock_profile_store: Mock): - mock_request.method = "PATCH" + mock_request.method = "PUT" mock_request.args = MultiDict({"uuid": EXAMPLE_UUID}) mock_request.data = None @@ -256,7 +256,7 @@ def test_update_no_body(wrapper: AppWrapper, mock_request: Mock, mock_profile_st def test_update_profile_missing(wrapper: AppWrapper, mock_request: Mock, mock_profile_store: Mock): - mock_request.method = "PATCH" + mock_request.method = "PUT" mock_request.args = MultiDict({"uuid": EXAMPLE_UUID}) mock_profile_store.return_value.update.side_effect = FileNotFoundError diff --git a/python/nwsc_proxy/test/test_profile_store.py b/python/nwsc_proxy/test/test_profile_store.py index 1634dae..b861083 100644 --- a/python/nwsc_proxy/test/test_profile_store.py +++ b/python/nwsc_proxy/test/test_profile_store.py @@ -16,6 +16,7 @@ from copy import deepcopy from datetime import datetime, UTC from glob import glob +from unittest.mock import Mock from pytest import fixture, raises @@ -186,24 +187,38 @@ def test_update_profile_success(store: ProfileStore): profile_id = EXAMPLE_SUPPORT_PROFILE["id"] new_start_dt = "2026-01-01T12:00:00Z" new_name = "A different name" - new_profile_data = {"name": new_name, "setting": {"timing": {"start": new_start_dt}}} + new_profile = deepcopy(EXAMPLE_SUPPORT_PROFILE) + new_profile["name"] = new_name + new_profile["setting"] = {"timing": {"start": new_start_dt}} - updated_profile = store.update(profile_id, new_profile_data) + updated_profile = store.update(profile_id, new_profile) # data returned should have updated attributes assert updated_profile["name"] == new_name assert updated_profile["setting"]["timing"]["start"] == new_start_dt - # attributes at same level as any nested updated ones should not have changed - assert ( - updated_profile["setting"]["timing"].get("durationInMinutes") - == EXAMPLE_SUPPORT_PROFILE["setting"]["timing"]["durationInMinutes"] - ) # profile in cache should have indeed been changed refetched_profile = next((p for p in store.profile_cache if p.id == profile_id), None) assert refetched_profile.name == new_name assert datetime.fromtimestamp(refetched_profile.start_timestamp, UTC) == dt_parse(new_start_dt) +def test_update_profile_error_rollback(store: ProfileStore): + profile_id = EXAMPLE_SUPPORT_PROFILE["id"] + new_name = "A different name" + new_profile = deepcopy(EXAMPLE_SUPPORT_PROFILE) + new_profile["name"] = new_name + # HACK: mocks out the class under test (BAD) + store.delete = Mock(name="mock_delete") # test fails if we really delete existing Profile + store.save = Mock(name="mock_save", side_effect=[None, profile_id]) # fails first save + + updated_profile = store.update(profile_id, new_profile) + + assert updated_profile is None + # profile in cache is still there, no changes were made to data + refetched_profile = next((p for p in store.profile_cache if p.id == profile_id), None) + assert refetched_profile.name == EXAMPLE_SUPPORT_PROFILE["name"] + + def test_update_profile_not_found(store: ProfileStore): profile_id = "11111111-2222-3333-444444444444" # fake ID does not exist in ProfileStore new_profile_data = {"name": "A different name"} From 519943105584ad3c4ab0e092cffc0981a3fb4dae Mon Sep 17 00:00:00 2001 From: Mackenzie Grimes - NOAA Affiliate Date: Tue, 27 Jan 2026 16:36:07 -0700 Subject: [PATCH 2/4] refactor ProfileStore to use dictionary cache instead of list --- python/nwsc_proxy/src/profile_store.py | 135 +++++++++---------- python/nwsc_proxy/test/test_profile_store.py | 18 ++- 2 files changed, 74 insertions(+), 79 deletions(-) diff --git a/python/nwsc_proxy/src/profile_store.py b/python/nwsc_proxy/src/profile_store.py index b71566b..410cc06 100644 --- a/python/nwsc_proxy/src/profile_store.py +++ b/python/nwsc_proxy/src/profile_store.py @@ -13,15 +13,12 @@ import json import logging -# from dataclasses import dataclass from datetime import datetime, UTC from glob import glob from math import inf from dateutil.parser import parse as dt_parse -# from src.utils import deep_update - # constants controlling the subdirectory where new vs. existing Profiles are saved NEW_SUBDIR = "new" EXISTING_SUBDIR = "existing" @@ -42,30 +39,30 @@ class CachedProfile: """ def __init__(self, data: dict, is_new: bool): + self.data = data self.is_new = is_new - self._data = data @property def id(self) -> str: """The Support Profile UUID""" - return self._data.get("id") + return self.data.get("id") @property def name(self) -> str: """The Support Profile name""" - return self._data.get("name") + return self.data.get("name") @property def is_active(self) -> bool: """The Support Profile's active state (can be marked as inactive to halt processing)""" - return self._data.get("isLive") + return self.data.get("isLive") @property def start_timestamp(self) -> float: """The Support Profile event's start in Unix time (milliseconds since the epoch). math.inf if Support Profile is never-ending """ - profile_start: str | None = self._data["setting"]["timing"].get("start") + profile_start: str | None = self.data["setting"]["timing"].get("start") return dt_parse(profile_start).timestamp() if profile_start else inf @property @@ -75,7 +72,7 @@ def end_timestamp(self) -> float: """ if self.start_timestamp == inf: return inf # infinite start time, so infinite end time as well - timing: dict[str, int] = self._data["setting"]["timing"] + timing: dict[str, int] = self.data["setting"]["timing"] # look up durationInMinutes, or deprecated duration, or None profile_duration = timing.get("durationInMinutes", timing.get("duration", inf)) return self.start_timestamp + profile_duration * 60 * 1000 # convert mins to ms @@ -87,20 +84,12 @@ def data_sources(self) -> list[str]: return [ # treat any profiles with empty string dataSource as default 'NBM' _map["dataSource"] if _map["dataSource"] != "" else DEFAULT_DATA_SOURCE - for phrase in self._data["nonImpactThresholds"]["phrasesForAllSeverities"].values() + for phrase in self.data["nonImpactThresholds"]["phrasesForAllSeverities"].values() for _map in phrase["map"].values() ] except KeyError: return [DEFAULT_DATA_SOURCE] # couldn't lookup dataSources, so just default to NBM - def save_to_file(self, parent_dir: str) -> str | None: - """Commit the CachedProfile object to a local file in the given parent directory. - - Returns: - (str | None): written filepath, if successful, or None on error - """ - raise NotImplementedError - def __str__(self): return ( f"{self.__class__.__name__}(id='{self.id}', name='{self.name}', is_new={self.is_new}, " @@ -124,7 +113,7 @@ def __init__(self, base_dir: str): logger.info("Scanning base directory for raw NWS Connect API response files: %s", base_dir) for response_filename in glob("*.json", root_dir=base_dir): response_filepath = os.path.join(base_dir, response_filename) - logger.warning("Loading profiles from raw API response file: %s", response_filepath) + logger.info("Loading profiles from raw API response file: %s", response_filepath) with open(response_filepath, "r", encoding="utf-8") as infile: data: dict = json.load(infile) @@ -139,16 +128,17 @@ def __init__(self, base_dir: str): json.dump(profile, outfile) # populate cache of JSON data of all Support Profiles, marked as new vs. existing - existing_profiles = [ - CachedProfile(profile, is_new=False) + existing_profiles = { + profile["id"]: CachedProfile(profile, is_new=False) for profile in self._load_profiles_from_filesystem(self._existing_dir) - ] - new_profiles = [ - CachedProfile(profile, is_new=True) + } + new_profiles = { + profile["id"]: CachedProfile(profile, is_new=True) for profile in self._load_profiles_from_filesystem(self._new_dir) - ] + } - self.profile_cache = existing_profiles + new_profiles + # TODO: might have broken this + self.profile_cache: dict[str, CachedProfile] = {**existing_profiles, **new_profiles} def get_all(self, data_source="ANY", is_new=False) -> list[dict]: """Get all Support Profile JSONs persisted in this API, filtering by status='new' @@ -162,7 +152,7 @@ def get_all(self, data_source="ANY", is_new=False) -> list[dict]: """ profiles_by_status = [ cached_profile - for cached_profile in self.profile_cache + for cached_profile in self.profile_cache.values() if ( # is new, if client requested new profiles, or is existing cached_profile.is_new == is_new @@ -174,9 +164,9 @@ def get_all(self, data_source="ANY", is_new=False) -> list[dict]: ] if data_source == "ANY": # all data sources requested, so do not filter by products used - return [profile._data for profile in profiles_by_status] + return [profile.data for profile in profiles_by_status] return [ - profile._data for profile in profiles_by_status if data_source in profile.data_sources + profile.data for profile in profiles_by_status if data_source in profile.data_sources ] def save(self, profile: dict, is_new=True) -> str | None: @@ -194,31 +184,15 @@ def save(self, profile: dict, is_new=True) -> str | None: logger.debug("Now saving new profile: %s", profile) # if profile ID is already in the cache, reject this save - existing_profile = next( - ( - ( - cached_obj - for cached_obj in self.profile_cache - if cached_obj.id == profile.get("id") - ) - ), - None, - ) - if existing_profile: + if existing_profile := self.profile_cache.get(profile.get("id")): logger.warning("Cannot save profile; already exists %s", existing_profile.id) return None cached_profile = CachedProfile(profile, is_new=is_new) - - # save Profile JSON to filesystem - file_dir = self._new_dir if is_new else self._existing_dir - filepath = os.path.join(file_dir, f"{cached_profile.id}.json") - logger.debug("Now saving profile to path: %s", filepath) - with open(filepath, "w", encoding="utf-8") as file: - json.dump(profile, file) + filepath = self._save_profile_to_filesystem(cached_profile) # add profile to in-memory cache - self.profile_cache.append(cached_profile) + self.profile_cache[cached_profile.id] = cached_profile logger.info("Saved profile to cache, file location: %s", filepath) return cached_profile.id @@ -230,9 +204,7 @@ def mark_as_existing(self, profile_id: str) -> bool: bool: True on success. False if JSON with this profile_id not found on filesystem """ # find the profile data from the new_profiles cache and move it to existing_profiles - cached_profile = next( - (profile for profile in self.profile_cache if profile.id == profile_id), None - ) + cached_profile = self.profile_cache.get(profile_id) if not cached_profile: # profile is not in cache; it must not exist logger.warning( @@ -250,7 +222,9 @@ def mark_as_existing(self, profile_id: str) -> bool: # move the JSON file from the "new" to the "existing" directory and update cache existing_filepath = os.path.join(self._existing_dir, f"{profile_id}.json") os.rename(new_filepath, existing_filepath) + cached_profile.is_new = False + self.profile_cache[profile_id] = cached_profile return True @@ -271,23 +245,25 @@ def update(self, profile_id: str, data: dict) -> dict: logger.info("Updating profile_id %s with new data: %s", profile_id, data) # find the profile data from the new_profiles cache, then save over it - cached_profile = next( - (profile for profile in self.profile_cache if profile.id == profile_id), None - ) + cached_profile = self.profile_cache.get(profile_id) if not cached_profile: raise FileNotFoundError # Profile with this ID does not exist in cache - # a bit hacky, but the fastest and least duplicative way to update a Profile - # in cache + disk is to delete the existing profile and re-save with new JSON data - self.delete(profile_id) - saved_id = self.save(data, cached_profile.is_new) - if not saved_id: + if profile_id != data.get("id"): + raise ValueError( + f"Refusing to overwrite existing profile ID {profile_id} with mismatched data including id {data.get('id')}" + ) + + updated_profile = CachedProfile(data, cached_profile.is_new) + # update disk with latest data; if the write fails, reject update + saved_file = self._save_profile_to_filesystem(updated_profile) + if not saved_file: logger.warning("Unable to update Profile ID %s for some reason", profile_id) - # attempt to rollback, recovering the previously deleted profile - self.save(cached_profile._data, cached_profile.is_new) return None - return data + # update in-memory cache to overwrite previous profile by ID + self.profile_cache[profile_id] = updated_profile + return updated_profile.data def delete(self, profile_id: str) -> bool: """Delete a Support Profile profile from storage, based on its UUID. @@ -311,18 +287,39 @@ def delete(self, profile_id: str) -> bool: ) return False + # drop profile from disk logger.debug("Attempting to delete profile at path: %s", filepath) os.remove(filepath) - # drop profile from cache - self.profile_cache = [ - cached_profile - for cached_profile in self.profile_cache - if cached_profile.id != profile_id - ] + del self.profile_cache[profile_id] return True - def _load_profiles_from_filesystem(self, dir_: str) -> list[dict]: + def _save_profile_to_filesystem(self, profile: CachedProfile) -> str | None: + """Save CachedProfile data (dict) to filesystem so it persists through service restarts""" + profile_id = profile.data.get("id") + if not profile_id: + raise ValueError("Cannot save CachedProfile to file that has no `id` attribute") + + file_dir = self._new_dir if profile.is_new else self._existing_dir + filepath = os.path.join(file_dir, f"{profile_id}.json") + logger.debug("Now saving profile to path: %s", filepath) + try: + with open(filepath, "w", encoding="utf-8") as file: + json.dump(profile.data, file) + except (PermissionError, json.JSONDecodeError, TypeError) as exc: + logger.error( + "Failed to save Profile %s to file %s due to: (%s) %s", + profile_id, + filepath, + type(exc), + str(exc), + ) + return None + + return filepath + + @staticmethod + def _load_profiles_from_filesystem(dir_: str) -> list[dict]: """Read all JSON files from one of this ProfileStore's subdirectories, and return list of the discovered files' json data. diff --git a/python/nwsc_proxy/test/test_profile_store.py b/python/nwsc_proxy/test/test_profile_store.py index b861083..4342b3e 100644 --- a/python/nwsc_proxy/test/test_profile_store.py +++ b/python/nwsc_proxy/test/test_profile_store.py @@ -16,7 +16,6 @@ from copy import deepcopy from datetime import datetime, UTC from glob import glob -from unittest.mock import Mock from pytest import fixture, raises @@ -74,16 +73,16 @@ def store(): # tests def test_profile_store_loads_api_responses(store: ProfileStore): - assert sorted([c.id for c in store.profile_cache]) == [ + assert sorted(list(store.profile_cache.keys())) == [ "a08370c6-ab87-4808-bd51-a8597e58410d", "e1033860-f198-4c6a-a91b-beaec905132f", "fd35adec-d2a0-49a9-a320-df20a7b6d681", ] - for cache_obj in store.profile_cache: + for cache_id, cache_obj in store.profile_cache.items(): # should have loaded all profiles as status "existing", file should exist in that subdir assert not cache_obj.is_new - filepath = os.path.join(STORE_BASE_DIR, EXISTING_SUBDIR, f"{cache_obj.id}.json") + filepath = os.path.join(STORE_BASE_DIR, EXISTING_SUBDIR, f"{cache_id}.json") assert os.path.exists(filepath) # new directory should be empty to begin with @@ -197,7 +196,7 @@ def test_update_profile_success(store: ProfileStore): assert updated_profile["name"] == new_name assert updated_profile["setting"]["timing"]["start"] == new_start_dt # profile in cache should have indeed been changed - refetched_profile = next((p for p in store.profile_cache if p.id == profile_id), None) + refetched_profile = store.profile_cache.get(profile_id) assert refetched_profile.name == new_name assert datetime.fromtimestamp(refetched_profile.start_timestamp, UTC) == dt_parse(new_start_dt) @@ -207,16 +206,15 @@ def test_update_profile_error_rollback(store: ProfileStore): new_name = "A different name" new_profile = deepcopy(EXAMPLE_SUPPORT_PROFILE) new_profile["name"] = new_name - # HACK: mocks out the class under test (BAD) - store.delete = Mock(name="mock_delete") # test fails if we really delete existing Profile - store.save = Mock(name="mock_save", side_effect=[None, profile_id]) # fails first save + # inject unhashable content into JSON data, will cause save() to fail and return None + new_profile["unhashable"] = set(["foo", "bar"]) updated_profile = store.update(profile_id, new_profile) assert updated_profile is None # profile in cache is still there, no changes were made to data - refetched_profile = next((p for p in store.profile_cache if p.id == profile_id), None) - assert refetched_profile.name == EXAMPLE_SUPPORT_PROFILE["name"] + refetched_profile = store.profile_cache.get(profile_id) + assert refetched_profile.name != new_name def test_update_profile_not_found(store: ProfileStore): From 6b4efe40405dbba3417689e1d606bd0e64a6810b Mon Sep 17 00:00:00 2001 From: Mackenzie Grimes - NOAA Affiliate Date: Tue, 27 Jan 2026 16:37:14 -0700 Subject: [PATCH 3/4] cleanup TODO --- python/nwsc_proxy/src/profile_store.py | 1 - 1 file changed, 1 deletion(-) diff --git a/python/nwsc_proxy/src/profile_store.py b/python/nwsc_proxy/src/profile_store.py index 410cc06..85b77f5 100644 --- a/python/nwsc_proxy/src/profile_store.py +++ b/python/nwsc_proxy/src/profile_store.py @@ -137,7 +137,6 @@ def __init__(self, base_dir: str): for profile in self._load_profiles_from_filesystem(self._new_dir) } - # TODO: might have broken this self.profile_cache: dict[str, CachedProfile] = {**existing_profiles, **new_profiles} def get_all(self, data_source="ANY", is_new=False) -> list[dict]: From 80f35c978080ea5fea6344e20cc699b68fa68543 Mon Sep 17 00:00:00 2001 From: Mackenzie Grimes - NOAA Affiliate Date: Tue, 27 Jan 2026 16:43:20 -0700 Subject: [PATCH 4/4] fix pylint warnings --- python/nwsc_proxy/src/profile_store.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/python/nwsc_proxy/src/profile_store.py b/python/nwsc_proxy/src/profile_store.py index e09f39d..858bdb1 100644 --- a/python/nwsc_proxy/src/profile_store.py +++ b/python/nwsc_proxy/src/profile_store.py @@ -45,6 +45,7 @@ def __init__(self, data: dict, is_new: bool): @property def id(self) -> str: """The Support Profile UUID""" + # pylint: disable=invalid-name return self.data.get("id") @property @@ -250,7 +251,10 @@ def update(self, profile_id: str, data: dict) -> dict: if profile_id != data.get("id"): raise ValueError( - f"Refusing to overwrite existing profile ID {profile_id} with mismatched data including id {data.get('id')}" + ( + f"Refusing to overwrite existing profile ID {profile_id} with mismatched data " + f"including id {data.get('id')}" + ) ) updated_profile = CachedProfile(data, cached_profile.is_new)