From 12d3a06d5d470a10927623354a98eede0fd5b544 Mon Sep 17 00:00:00 2001 From: Mackenzie Grimes - NOAA Affiliate Date: Tue, 31 Dec 2024 12:22:35 -0700 Subject: [PATCH 1/2] tweak POST to suppoort status: existing, not just assume status: new --- python/nwsc_proxy/ncp_web_service.py | 63 ++++++++++++++++---------- python/nwsc_proxy/src/profile_store.py | 13 ++++-- 2 files changed, 48 insertions(+), 28 deletions(-) diff --git a/python/nwsc_proxy/ncp_web_service.py b/python/nwsc_proxy/ncp_web_service.py index bb337ef..381ba83 100644 --- a/python/nwsc_proxy/ncp_web_service.py +++ b/python/nwsc_proxy/ncp_web_service.py @@ -13,22 +13,15 @@ from datetime import datetime, UTC from argparse import ArgumentParser, Namespace -from flask import Flask, current_app, request, jsonify +from flask import Flask, Response, current_app, request, jsonify + +from idsse.common.utils import to_iso from src.profile_store import ProfileStore # constants GSL_KEY = '8209c979-e3de-402e-a1f5-556d650ab889' - -def to_iso(date_time: datetime) -> str: - """Format a datetime instance to an ISO string. Borrowed from idsse.commons.utils for now""" - return (f'{date_time.strftime("%Y-%m-%dT%H:%M")}:' - f'{(date_time.second + date_time.microsecond / 1e6):06.3f}' - 'Z' if date_time.tzname() in [None, str(UTC)] - else date_time.strftime("%Z")[3:]) - - # pylint: disable=too-few-public-methods class HealthRoute: """Handle requests to /health endpoint""" @@ -49,29 +42,17 @@ class EventsRoute: def __init__(self, base_dir: str): self.profile_store = ProfileStore(base_dir) - # pylint: disable=too-many-return-statements def handler(self): - """Logic for requests to /all-events""" + """Logic for requests to /all-events.""" # check that this request has proper key to get or add data if request.headers.get('X-Api-Key') != current_app.config['GSL_KEY']: return jsonify({'message': 'ERROR: Unauthorized'}), 401 if request.method == 'POST': - # request is saving new Support Profile event - request_body: dict = request.json - profile_id = self.profile_store.save(request_body) - if not profile_id: - return jsonify({'message': f'Profile {request_body.get("id")} already exists'} - ), 400 - - return jsonify({'message': f'Profile {profile_id} saved'}), 201 + return self._handle_create() if request.method == 'DELETE': - profile_id = request.args.get('uuid', default=None, type=str) - is_deleted = self.profile_store.delete(profile_id) - if not is_deleted: - return jsonify({'message': f'Profile {profile_id} not found'}), 404 - return jsonify({'message': f'Profile {profile_id} deleted'}), 204 + return self._handle_delete() # otherwise, must be 'GET' operation data_source = request.args.get('dataSource', None, type=str) @@ -98,6 +79,38 @@ def handler(self): return jsonify({'profiles': profiles, 'errors': []}), 200 + def _handle_delete(self) -> Response: + """Logic for DELETE requests to /all-events. Returns Response with status_code: 204 on + success, 404 otherwise.""" + profile_id = request.args.get('uuid', default=None, type=str) + is_deleted = self.profile_store.delete(profile_id) + if not is_deleted: + return jsonify({'message': f'Profile {profile_id} not found'}), 404 + return jsonify({'message': f'Profile {profile_id} deleted'}), 204 + + def _handle_create(self) -> Response: + """Logic for POST requests to /all-events. Returns Response with status_code: 201 on + success, 400 otherwise.""" + request_body: dict = request.json + + profile_data: dict | None = request_body.get('data') + status: str | None = request_body.get('status') + if not profile_data or not status: + return jsonify({'message': 'Missing one of required attributes: [data, status]'}), 400 + + if status == 'new': + is_new = True + elif status == 'existing': + is_new = False + else: + return jsonify({'message': 'Status must be one of [new, existing]'}), 400 + + profile_id = self.profile_store.save(profile_data, is_new) + if not profile_id: + return jsonify({'message': f'Profile {profile_data.get("id")} already exists'}), 400 + + return jsonify({'message': f'Profile {profile_id} saved'}), 201 + class AppWrapper: """Web server class wrapping Flask operations""" diff --git a/python/nwsc_proxy/src/profile_store.py b/python/nwsc_proxy/src/profile_store.py index a0eb4fd..316d0ba 100644 --- a/python/nwsc_proxy/src/profile_store.py +++ b/python/nwsc_proxy/src/profile_store.py @@ -88,9 +88,15 @@ def get_all(self, filter_new_profiles = False) -> list[dict]: return [cached_profile.data for cached_profile in self.profile_cache if cached_profile.is_new == filter_new_profiles] - def save(self, profile: dict) -> str | None: + def save(self, profile: dict, is_new = True) -> str | None: """Persist a new Support Profile Profile to this API + Args: + profile (dict): the JSON data of the Support Profile to store. + is_new (optional, bool): whether to store the Profile as "new" or "existing". This + will only control whether this SupportProfile will be returned to calls to the + `get_all()` method (if it is classified as new vs. existing). Defaults to True. + Returns: str | None: UUID of saved Support Profile on success, otherwise None """ @@ -103,10 +109,11 @@ def save(self, profile: dict) -> str | None: logger.warning('Cannot save profile; already exists %s', existing_profile.id) return None - cached_profile = CachedProfile(profile, is_new=True) + cached_profile = CachedProfile(profile, is_new=is_new) # save Profile JSON to filesystem - filepath = os.path.join(self._new_dir, f'{cached_profile.id}.json') + file_dir = self._new_dir if is_new else self._existing_dir + filepath = os.path.join(file_dir, f'{cached_profile.id}.json') logger.info('Now saving profile to path: %s', filepath) with open(filepath, 'w', encoding='utf-8') as file: json.dump(profile, file) From 39b4b0adddd6d783145352ce8b21ff94ceb8fedf Mon Sep 17 00:00:00 2001 From: Mackenzie Grimes - NOAA Affiliate Date: Tue, 31 Dec 2024 17:32:06 -0700 Subject: [PATCH 2/2] add unit test coverage --- .../nwsc_proxy/test/test_ncp_web_service.py | 31 +++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/python/nwsc_proxy/test/test_ncp_web_service.py b/python/nwsc_proxy/test/test_ncp_web_service.py index 1f7bbf7..fee947b 100644 --- a/python/nwsc_proxy/test/test_ncp_web_service.py +++ b/python/nwsc_proxy/test/test_ncp_web_service.py @@ -160,14 +160,41 @@ def test_get_new_profiles(wrapper: AppWrapper, mock_request: Mock, mock_profile_ assert mark_existing_call_args[0][1][0] == example_profile['id'] -def test_create_profile_success(wrapper: AppWrapper, mock_request: Mock, mock_profile_store: Mock): +def test_create_profile_new(wrapper: AppWrapper, mock_request: Mock, mock_profile_store: Mock): mock_request.method = 'POST' - mock_request.json = {'id': EXAMPLE_UUID, 'name': 'My Profile'} + example_profile = {'id': EXAMPLE_UUID, 'name': 'My Profile'} + mock_request.json = {'status': 'new', 'data': example_profile} mock_profile_store.return_value.save.return_value = EXAMPLE_UUID # save() success result: tuple[Response, int] = wrapper.app.view_functions['events']() assert result[1] == 201 + # should have saved profile with is_new: True + mock_profile_store.return_value.save.assert_called_once_with(example_profile, True) + + +def test_create_profile_existing(wrapper: AppWrapper, mock_request: Mock, mock_profile_store: Mock): + mock_request.method = 'POST' + example_profile = {'id': EXAMPLE_UUID, 'name': 'My Profile'} + mock_request.json = {'status': 'existing', 'data': example_profile} + mock_profile_store.return_value.save.return_value = EXAMPLE_UUID # save() success + + result: tuple[Response, int] = wrapper.app.view_functions['events']() + + assert result[1] == 201 + # should have saved profile with is_new: False + mock_profile_store.return_value.save.assert_called_once_with(example_profile, False) + + +def test_create_profile_invalid(wrapper: AppWrapper, mock_request: Mock, mock_profile_store: Mock): + mock_request.method = 'POST' + example_profile = {'id': EXAMPLE_UUID, 'name': 'My Profile'} + mock_request.json = {'status': 'foobar', 'data': example_profile} + + result: tuple[Response, int] = wrapper.app.view_functions['events']() + + assert result[1] == 400 + mock_profile_store.return_value.save.assert_not_called() def test_create_previous_profile_failure(wrapper: AppWrapper,