diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 15ed88316f7c44cf485ac5d06bb54cca66e0da28..20d007003a866a79635ed82f5d80df74d6483f1c 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -13,10 +13,10 @@ stages: python_unit_test: extends: .base_rules - image: public.ecr.aws/bitnami/python:3.6 + image: public.ecr.aws/bitnami/python:3.8 stage: test script: - - virtualenv --python=python3.6 env + - virtualenv --python=python3.8 env - env/bin/pip install -r wdfn-server/requirements.txt -r wdfn-server/requirements-dev.txt - env/bin/coverage run --omit=wdfn-server/waterdata/tests/*.py,env/* -m pytest wdfn-server/waterdata - env/bin/coverage xml -o python_coverage.xml @@ -36,10 +36,10 @@ python_unit_test: python_lint: extends: .base_rules - image: public.ecr.aws/bitnami/python:3.6 + image: public.ecr.aws/bitnami/python:3.8 stage: test script: - - virtualenv --python=python3.6 env + - virtualenv --python=python3.8 env - env/bin/pip install -r wdfn-server/requirements.txt -r wdfn-server/requirements-dev.txt - cd wdfn-server && ../env/bin/prospector rules: diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f6edcbd2057ad1ed05f4c0b3df2cd648859769f..555022ad7b06ee2d363e1a0010f7f784d650c5e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html). ## [Unreleased](https://github.com/usgs/waterdataui/compare/waterdataui-0.49.0...master) +### Changed +- The NWIS site service and sifta service now use aiohttp and are called concurrently in the monitoring_location view. ## [0.49.0](https://github.com/usgs/waterdataui/compare/waterdataui-0.48.0...waterdataui-0.49.0) - 2021-08-03 ### Changed diff --git a/wdfn-server/requirements.txt b/wdfn-server/requirements.txt index e30a4a7d4c822c3a2d99f11db826915f5b791d72..abdc26b70a68d9d40e7e1b9839ee36a5f113d965 100644 --- a/wdfn-server/requirements.txt +++ b/wdfn-server/requirements.txt @@ -1,5 +1,7 @@ +aiohttp[speedups]==3.7.4.post0 +aioresponses==0.7.2 certifi==2021.5.30 -Flask==2.0.1 +Flask[async]==2.0.1 Markdown==3.3.4 MarkupSafe==2.0.1 pendulum==2.1.2 diff --git a/wdfn-server/waterdata/services/nwissite.py b/wdfn-server/waterdata/services/nwissite.py index 071e89c38fb2256223736dd181b37b98fac5f1bc..b89e967a49af8ea13cf1a85aaecf4775d0d56607 100644 --- a/wdfn-server/waterdata/services/nwissite.py +++ b/wdfn-server/waterdata/services/nwissite.py @@ -3,117 +3,106 @@ Class and functions for calling NWIS services and working with the returned data. """ -from requests import exceptions as request_exceptions, Session from ..utils import parse_rdb from .. import app -class SiteService: +async def get(session, params): + # pylint: disable=no-member """ - Provides access to the NWIS site service + Returns a tuple containing the request status code and a list of dictionaries that represent the contents of + RDB file + :param session - instance aiohttp.ClientSession + :param dict params: + :yields + - status_code - status code returned from the service request + - reason - string + - site_data - list of dictionaries """ + endpoint = app.config["SITE_DATA_ENDPOINT"] + app.logger.debug(f'Requesting data from {endpoint} and {params}') + default_params = { + 'format': 'rdb' + } + default_params.update(params) + response = await session.get(endpoint, params=default_params) + if response.status == 200: + lines = await response.text() + app.logger.debug(f'Received data from {endpoint} and {params}') + return 200, response.reason, list(parse_rdb(iter(lines.splitlines()))) + + return response.status, response.reason, [] + + +async def get_site_data(session, site_no, agency_cd=''): + """ + Get the metadata for site_no, agency_cd (which may be blank) using the additional query parameters, param + Note that more than one dictionary can be returned if agency_cd is empty. + :param session: instance of aiohttp.ClientSession + :param str site_no: site identifier + :param str agency_cd: identifier for the agency that owns the site + :yields: + - status - status code from response + - reason - string + - site_metadata - list of dict representing the data returned in the rdb file + """ + params = { + 'sites': site_no, + 'siteOutput': 'expanded' + } + if agency_cd: + params['agencyCd'] = agency_cd + return await get(session, params) - def __init__(self, endpoint): - """ - Constructor method. - - :param str endpoint: the scheme, host and path to the NWIS site service - """ - self.endpoint = endpoint - self.session = Session() - - def get(self, params): - # pylint: disable=no-member - """ - Returns a tuple containing the request status code and a list of dictionaries that represent the contents of - RDB file - - :param dict params: - :returns - - status_code - status code returned from the service request - - reason - string - - site_data - list of dictionaries - """ - app.logger.debug(f'Requesting data from {self.endpoint}') - default_params = { - 'format': 'rdb' - } - default_params.update(params) - try: - response = self.session.get(self.endpoint, params=default_params) - except (request_exceptions.Timeout, request_exceptions.ConnectionError) as err: - app.logger.error(repr(err)) - return 500, repr(err), None - if response.status_code == 200: - return 200, response.reason, list(parse_rdb(response.iter_lines(decode_unicode=True))) - - return response.status_code, response.reason, [] - - def get_site_data(self, site_no, agency_cd=''): - """ - Get the metadata for site_no, agency_cd (which may be blank) using the additional query parameters, param - Note that more than one dictionary can be returned if agency_cd is empty. - :param str site_no: site identifier - :param str agency_cd: identifier for the agency that owns the site - :returns: - - status - status code from response - - reason - string - - site_metadata - list of dict representing the data returned in the rdb file - """ - params = { - 'sites': site_no, - 'siteOutput': 'expanded' - } - if agency_cd: - params['agencyCd'] = agency_cd - return self.get(params) - - def get_period_of_record(self, site_no, agency_cd=''): - """ - Get the parameters measured at the site(s). - :param str site_no: site identifier - :param str agency_cd: identifier for the agency that owns the site: - :returns: - - status - status code from response - - reason - string - - periodOfRecord - list of dict representing the period of record for the data available at the site - """ - params = { - 'sites': site_no, - 'seriesCatalogOutput': True, - 'siteStatus': 'all' - } - if agency_cd: - params['agencyCd'] = agency_cd - return self.get(params) +async def get_period_of_record(session, site_no, agency_cd=''): + """ + Get the parameters measured at the site(s). + :param session: instance of aiohttp.ClientSession + :param str site_no: site identifier + :param str agency_cd: identifier for the agency that owns the site: + :returns: + - status - status code from response + - reason - string + - periodOfRecord - list of dict representing the period of record for the data available at the site + """ + params = { + 'sites': site_no, + 'seriesCatalogOutput': 'true', + 'siteStatus': 'all' + } + if agency_cd: + params['agencyCd'] = agency_cd + return await get(session, params) - def get_huc_sites(self, huc_cd): - """ - Get all sites within a hydrologic unit as identified by its - hydrologic unit code (HUC). - :param str huc_cd: hydrologic unit code - :returns: all sites in the specified HUC - - status - status code from response - - reason - string - - sites - list of dict representing the sites in huc_cd - """ - return self.get({ - 'huc': huc_cd - }) +async def get_huc_sites(session, huc_cd): + """ + Get all sites within a hydrologic unit as identified by its + hydrologic unit code (HUC). + :param session: instance of aiohttp.ClientSession + :param str huc_cd: hydrologic unit code + :returns: all sites in the specified HUC + - status - status code from response + - reason - string + - sites - list of dict representing the sites in huc_cd + """ + return await get(session, { + 'huc': huc_cd + }) - def get_county_sites(self, state_county_cd): - """ - Get all sites within a county. - :param str state_county_cd: FIPS ID for a statecounty - :returns: all sites in the specified county - - status - status code from response - - reason - string - - sites - list of dict representing the site in state_county_cd - """ - return self.get({ - 'countyCd': state_county_cd - }) +async def get_county_sites(session, state_county_cd): + """ + Get all sites within a county. + :param session: instance of aiohttp.ClientSession + :param str state_county_cd: FIPS ID for a statecounty + :returns: all sites in the specified county + - status - status code from response + - reason - string + - sites - list of dict representing the site in state_county_cd + """ + return await get(session, { + 'countyCd': state_county_cd + }) diff --git a/wdfn-server/waterdata/services/sifta.py b/wdfn-server/waterdata/services/sifta.py index e5067569f9b3a801358b9a66559b8aed0bf9d2e3..f5cf9634479482954af1cc83466eda9f27b7cace 100644 --- a/wdfn-server/waterdata/services/sifta.py +++ b/wdfn-server/waterdata/services/sifta.py @@ -1,38 +1,30 @@ """ Helpers to retrieve SIFTA cooperator data. """ -from requests import exceptions as request_exceptions, Session from .. import app -class SiftaService: - # pylint: disable=too-few-public-methods, no-member +async def get_cooperators(session, site_no): """ - Provide access to a service that returns cooperator data - """ - def __init__(self, endpoint): - self.endpoint = endpoint - self.session = Session() + Gets the cooperator data from the SIFTA service - def get_cooperators(self, site_no): - """ - Gets the cooperator data from the SIFTA service + :param session - instance aiohttp.ClientSession + :param str site_no: USGS site number + :returns Array of dict + """ + # pylint: disable=fixme + url = f'{app.config["COOPERATOR_SERVICE_ENDPOINT"]}{site_no}' + app.logger.debug(f'Requesting data from {url}') # pylint: disable=no-member - :param site_no: USGS site number - :return Array of dict - """ - url = f'{self.endpoint}{site_no}' - try: - response = self.session.get(url) - except (request_exceptions.Timeout, request_exceptions.ConnectionError) as err: - app.logger.error(repr(err)) - return [] + async with session.get(url) as response: + # TODO: Add exception handling + app.logger.debug(f'Retrieved data from {url}') # pylint: disable=no-member - if response.status_code != 200: + if response.status != 200: return [] try: - resp_json = response.json() + resp_json = await response.json() except ValueError: return [] else: diff --git a/wdfn-server/waterdata/services/timezone.py b/wdfn-server/waterdata/services/timezone.py index f237d295f8c49d7c6f7218e62220b98f75656cbf..9fb6ab2ff16a0e43a1324f9a9720bf9e022ed556 100644 --- a/wdfn-server/waterdata/services/timezone.py +++ b/wdfn-server/waterdata/services/timezone.py @@ -25,6 +25,8 @@ class TimeZoneService: :return str """ url = f'{self.endpoint}/points/{latitude},{longitude}' + + app.logger.debug('Requesting timezone') try: response = self.session.get(url) except (request_exceptions.Timeout, request_exceptions.ConnectionError) as err: @@ -33,5 +35,7 @@ class TimeZoneService: if response.status_code != 200: return None + app.logger.debug('Received timezone') + json_data = response.json() return json_data['properties'].get('timeZone', None) if 'properties' in json_data else None diff --git a/wdfn-server/waterdata/tests/services/test_nwissite.py b/wdfn-server/waterdata/tests/services/test_nwissite.py index 2a0f3c28fe25975593a36ec6a71e350fcb054e98..17969ee6dc4a57fca81de83c412f4f7acfebd81b 100644 --- a/wdfn-server/waterdata/tests/services/test_nwissite.py +++ b/wdfn-server/waterdata/tests/services/test_nwissite.py @@ -2,151 +2,178 @@ Tests for NWISWeb service calls. """ -from unittest import TestCase +import asyncio -from requests_mock import Mocker +import aiohttp +from aioresponses import aioresponses +import pytest -from ...services.nwissite import SiteService +from ... import app +from ...services.nwissite import get, get_site_data, get_period_of_record, get_county_sites, get_huc_sites from ..mock_test_data import SITE_RDB, PARAMETER_RDB - -class TestSiteService(TestCase): - - def setUp(self): - self.endpoint = 'https://www.fakesiteservice.gov/nwis' - self.site_service = SiteService(self.endpoint) - - def test_successful_get_no_query_parameters(self): - with Mocker(session=self.site_service.session) as session_mock: - session_mock.get(self.endpoint, text=SITE_RDB) - status_code, _, result = self.site_service.get({}) - self.assertEqual(session_mock.call_count, 1) - self.assertIn('format=rdb', session_mock.request_history[0].query) - self.assertEqual(status_code, 200) - self.assertEqual(len(result), 1) - self.assertEqual(result[0]['site_no'], '01630500') - - def test_successful_get_query_parameters(self): - with Mocker(session=self.site_service.session) as session_mock: - session_mock.get(self.endpoint, text=SITE_RDB) - status_code, _, _ = self.site_service.get({ - 'param1': 'this', - 'param2': 'that' - }) - self.assertEqual(status_code, 200) - self.assertIn('param1=this', session_mock.request_history[0].query) - self.assertIn('param2=that', session_mock.request_history[0].query) - self.assertIn('format=rdb', session_mock.request_history[0].query) - - def test_500_get(self): - with Mocker(session=self.site_service.session) as session_mock: - session_mock.get(self.endpoint, reason='Bad server', status_code=500) - status_code, reason, _ = self.site_service.get({}) - self.assertEqual(status_code, 500) - self.assertEqual(reason, 'Bad server') - - def test_non500_error_get(self): - with Mocker(session=self.site_service.session) as session_mock: - session_mock.get(self.endpoint, reason='Not found', status_code=404) - status_code, reason, result = self.site_service.get({}) - self.assertEqual(status_code, 404) - self.assertEqual(reason, 'Not found') - self.assertEqual(result, []) - - def test_successful_get_site_data_with_no_agency_cd(self): - with Mocker(session=self.site_service.session) as session_mock: - session_mock.get(self.endpoint, text=SITE_RDB, reason='OK') - status_code, reason, result = self.site_service.get_site_data('01630500') - self.assertIn('sites=01630500', session_mock.request_history[0].query) - self.assertIn('siteoutput=expanded', session_mock.request_history[0].query) - self.assertNotIn('agencycd', session_mock.request_history[0].query) - self.assertEqual(status_code, 200) - self.assertEqual(reason, 'OK') - self.assertEqual(len(result), 1) - - def test_successful_get_site_with_agency_cd(self): - with Mocker(session=self.site_service.session) as session_mock: - session_mock.get(self.endpoint, text=SITE_RDB, reason='OK') - status_code, reason, result = self.site_service.get_site_data('01630500', agency_cd='USGS') - self.assertIn('sites=01630500', session_mock.request_history[0].query) - self.assertIn('siteoutput=expanded', session_mock.request_history[0].query) - self.assertIn('agencycd=usgs', session_mock.request_history[0].query) - self.assertEqual(status_code, 200) - self.assertEqual(reason, 'OK') - self.assertEqual(len(result), 1) - - def test_404_get_site(self): - with Mocker(session=self.site_service.session) as session_mock: - session_mock.get(self.endpoint, text=SITE_RDB, reason='Not found', status_code=404) - status_code, reason, result = self.site_service.get_site_data('01630500', agency_cd='USGS') - self.assertEqual(status_code, 404) - self.assertEqual(reason, 'Not found') - self.assertEqual(result, []) - - def test_successful_get_period_of_record_with_no_agency_cd(self): - with Mocker(session=self.site_service.session) as session_mock: - session_mock.get(self.endpoint, text=PARAMETER_RDB, reason='OK') - status_code, reason, result = self.site_service.get_period_of_record('01630500') - self.assertIn('sites=01630500', session_mock.request_history[0].query) - self.assertIn('sitestatus=all', session_mock.request_history[0].query) - self.assertIn('seriescatalogoutput=true', session_mock.request_history[0].query) - self.assertNotIn('agencycd', session_mock.request_history[0].query) - self.assertEqual(status_code, 200) - self.assertEqual(reason, 'OK') - self.assertEqual(len(result), 8) - - def test_successful_get_period_of_record_with_agency_cd(self): - with Mocker(session=self.site_service.session) as session_mock: - session_mock.get(self.endpoint, text=PARAMETER_RDB, reason='OK') - status_code, reason, result = self.site_service.get_period_of_record('01630500', agency_cd='USGS') - self.assertIn('sites=01630500', session_mock.request_history[0].query) - self.assertIn('sitestatus=all', session_mock.request_history[0].query) - self.assertIn('seriescatalogoutput=true', session_mock.request_history[0].query) - self.assertIn('agencycd=usgs', session_mock.request_history[0].query) - self.assertEqual(status_code, 200) - self.assertEqual(reason, 'OK') - self.assertEqual(len(result), 8) - - def test_404_get_period_of_record(self): - with Mocker(session=self.site_service.session) as session_mock: - session_mock.get(self.endpoint, text=PARAMETER_RDB, reason='Not found', status_code=404) - status_code, reason, result = self.site_service.get_period_of_record('01630500', agency_cd='USGS') - self.assertEqual(status_code, 404) - self.assertEqual(reason, 'Not found') - self.assertEqual(result, []) - - def test_successful_get_huc_sites(self): - with Mocker(session=self.site_service.session) as session_mock: - session_mock.get(self.endpoint, text=SITE_RDB, reason='OK') - status_code, reason, result = self.site_service.get_huc_sites('07010101') - self.assertIn('huc=07010101', session_mock.request_history[0].query) - self.assertEqual(status_code, 200) - self.assertEqual(reason, 'OK') - self.assertEqual(len(result), 1) - - def test_unsuccessful_get_huc_sites(self): - with Mocker(session=self.site_service.session) as session_mock: - session_mock.get(self.endpoint, text=SITE_RDB, reason='Not found', status_code=404) - status_code, reason, result = self.site_service.get_huc_sites('07010101') - self.assertIn('huc=07010101', session_mock.request_history[0].query) - self.assertEqual(status_code, 404) - self.assertEqual(reason, 'Not found') - self.assertEqual(len(result), 0) - - def test_successful_get_county_sites(self): - with Mocker(session=self.site_service.session) as session_mock: - session_mock.get(self.endpoint, text=SITE_RDB, reason='OK') - status_code, reason, result = self.site_service.get_county_sites('55003') - self.assertIn('countycd=55003', session_mock.request_history[0].query) - self.assertEqual(status_code, 200) - self.assertEqual(reason, 'OK') - self.assertEqual(len(result), 1) - - def test_unsuccessful_get_county_sites(self): - with Mocker(session=self.site_service.session) as session_mock: - session_mock.get(self.endpoint, text=SITE_RDB, reason='Not found', status_code=404) - status_code, reason, result = self.site_service.get_county_sites('55003') - self.assertIn('countycd=55003', session_mock.request_history[0].query) - self.assertEqual(status_code, 404) - self.assertEqual(reason, 'Not found') - self.assertEqual(len(result), 0) +ENDPOINT = app.config['SITE_DATA_ENDPOINT'] + + +@pytest.fixture(autouse=True) +def aiohttp_session(): + session = aiohttp.ClientSession() + yield session + session.close() + +def test_successful_get_no_query_parameters(aiohttp_session): + loop = asyncio.get_event_loop() + with aioresponses() as mock: + mock.get(f'{ENDPOINT}?format=rdb', status=200, body=SITE_RDB) + status_code, _, result = loop.run_until_complete(get(aiohttp_session, {})) + + assert status_code == 200 + assert len(result) == 1 + assert result[0]['site_no'] == '01630500' + + +def test_successful_get_query_parameters(aiohttp_session): + loop = asyncio.get_event_loop() + with aioresponses() as mock: + mock.get(f'{ENDPOINT}?param1=this¶m2=that&format=rdb', status=200, body=SITE_RDB) + status_code, _, _ = loop.run_until_complete(get(aiohttp_session, { + 'param1': 'this', + 'param2': 'that' + })) + assert status_code == 200 + + +def test_500_get(aiohttp_session): + loop = asyncio.get_event_loop() + with aioresponses() as mock: + mock.get(f'{ENDPOINT}?format=rdb', status=500, reason='Bad server') + status_code, reason, _ = loop.run_until_complete(get(aiohttp_session, {})) + assert status_code == 500 + assert reason == 'Bad server' + + +def test_non500_error_get(aiohttp_session): + loop = asyncio.get_event_loop() + with aioresponses() as mock: + mock.get(f'{ENDPOINT}?format=rdb', status=404, reason='Not found') + status_code, reason, result = loop.run_until_complete(get(aiohttp_session, {})) + assert status_code == 404 + assert reason == 'Not found' + assert result == [] + + +def test_successful_get_site_data_with_no_agency_cd(aiohttp_session): + loop = asyncio.get_event_loop() + with aioresponses() as mock: + mock.get(f'{ENDPOINT}?format=rdb&sites=01630500&siteOutput=expanded', status=200, reason='OK', body=SITE_RDB) + status_code, reason, result = loop.run_until_complete(get_site_data(aiohttp_session, '01630500')) + assert status_code == 200 + assert reason == 'OK' + assert len(result) == 1 + + +def test_successful_get_site_with_agency_cd(aiohttp_session): + loop = asyncio.get_event_loop() + with aioresponses() as mock: + mock.get(f'{ENDPOINT}?format=rdb&sites=01630500&siteOutput=expanded&agencyCd=USGS', + status=200, reason='OK', body=SITE_RDB) + status_code, reason, result = loop.run_until_complete(get_site_data(aiohttp_session, '01630500', agency_cd='USGS')) + assert status_code == 200 + assert reason == 'OK' + assert len(result) == 1 + + +def test_404_get_site(aiohttp_session): + loop = asyncio.get_event_loop() + with aioresponses() as mock: + mock.get(f'{ENDPOINT}?format=rdb&sites=01630500&siteOutput=expanded&agencyCd=USGS', + status=404, reason='Not found') + status_code, reason, result = loop.run_until_complete( + get_site_data(aiohttp_session, '01630500', agency_cd='USGS')) + assert status_code == 404 + assert reason == 'Not found' + assert result == [] + + +def test_successful_get_period_of_record_with_no_agency_cd(aiohttp_session): + loop = asyncio.get_event_loop() + with aioresponses() as mock: + mock.get(f'{ENDPOINT}?format=rdb&sites=01630500&siteStatus=all&seriesCatalogOutput=true', + status=200, reason='OK', body=PARAMETER_RDB) + status_code, reason, result = loop.run_until_complete( + get_period_of_record(aiohttp_session, '01630500')) + assert status_code == 200 + assert reason == 'OK' + assert len(result) == 8 + + +def test_successful_get_period_of_record_with_agency_cd(aiohttp_session): + loop = asyncio.get_event_loop() + with aioresponses() as mock: + mock.get(f'{ENDPOINT}?format=rdb&sites=01630500&siteStatus=all&seriesCatalogOutput=true&agencyCd=USGS', + status=200, reason='OK', body=PARAMETER_RDB) + status_code, reason, result = loop.run_until_complete( + get_period_of_record(aiohttp_session, '01630500', agency_cd='USGS')) + assert status_code == 200 + assert reason == 'OK' + assert len(result) == 8 + + +def test_404_get_period_of_record(aiohttp_session): + loop = asyncio.get_event_loop() + with aioresponses() as mock: + mock.get(f'{ENDPOINT}?format=rdb&sites=01630500&siteStatus=all&seriesCatalogOutput=true&agencyCd=USGS', + status=404, reason='Not found') + status_code, reason, result = loop.run_until_complete( + get_period_of_record(aiohttp_session, '01630500', agency_cd='USGS')) + assert status_code == 404 + assert reason == 'Not found' + assert result == [] + + +def test_successful_get_huc_sites(aiohttp_session): + loop = asyncio.get_event_loop() + with aioresponses() as mock: + mock.get(f'{ENDPOINT}?format=rdb&huc=07010101', + status=200, reason='OK', body=SITE_RDB) + status_code, reason, result = loop.run_until_complete( + get_huc_sites(aiohttp_session, '07010101')) + assert status_code == 200 + assert reason == 'OK' + assert len(result) == 1 + + +def test_unsuccessful_get_huc_sites(aiohttp_session): + loop = asyncio.get_event_loop() + with aioresponses() as mock: + mock.get(f'{ENDPOINT}?format=rdb&huc=07010101', + status=404, reason='Not found') + status_code, reason, result = loop.run_until_complete( + get_huc_sites(aiohttp_session, '07010101')) + assert status_code == 404 + assert reason == 'Not found' + assert len(result) == 0 + + +def test_successful_get_county_sites(aiohttp_session): + loop = asyncio.get_event_loop() + with aioresponses() as mock: + mock.get(f'{ENDPOINT}?format=rdb&countyCd=55003', + status=200, reason='OK', body=SITE_RDB) + status_code, reason, result = loop.run_until_complete( + get_county_sites(aiohttp_session, '55003')) + assert status_code == 200 + assert reason == 'OK' + assert len(result) == 1 + + +def test_unsuccessful_get_county_sites(aiohttp_session): + loop = asyncio.get_event_loop() + with aioresponses() as mock: + mock.get(f'{ENDPOINT}?format=rdb&countyCd=55003', + status=404, reason='Not found', body=SITE_RDB) + status_code, reason, result = loop.run_until_complete( + get_county_sites(aiohttp_session, '55003')) + assert status_code == 404 + assert reason == 'Not found' + assert len(result) == 0 diff --git a/wdfn-server/waterdata/tests/services/test_sifta.py b/wdfn-server/waterdata/tests/services/test_sifta.py index 6ab5ec12e0085eef2ba9dcb41b426d9f59b33ada..6357e294216315d52b90a64743c244284db92503 100644 --- a/wdfn-server/waterdata/tests/services/test_sifta.py +++ b/wdfn-server/waterdata/tests/services/test_sifta.py @@ -2,11 +2,14 @@ Tests for the cooperator service calls. """ +import asyncio import json -from requests_mock import Mocker +import aiohttp +from aioresponses import aioresponses -from ...services.sifta import SiftaService +from ... import app +from ...services.sifta import get_cooperators MOCK_RESPONSE = """ @@ -14,24 +17,21 @@ MOCK_RESPONSE = """ """ MOCK_CUSTOMER_LIST = json.loads(MOCK_RESPONSE)['Customers'] -ENDPOINT = 'https://www.fakesifta.gov/' - def test_sifta_response(): - sifta_service = SiftaService(ENDPOINT) - with Mocker(session=sifta_service.session) as session_mock: - session_mock.get(f'{ENDPOINT}12345', text=MOCK_RESPONSE) - result = sifta_service.get_cooperators('12345') + loop = asyncio.get_event_loop() + session = aiohttp.ClientSession() + with aioresponses() as mock: + mock.get(f'{app.config["COOPERATOR_SERVICE_ENDPOINT"]}12345', status=200, body=MOCK_RESPONSE) + resp = loop.run_until_complete(get_cooperators(session, '12345')) - assert session_mock.call_count == 1 - assert result == MOCK_CUSTOMER_LIST, 'Expected response' + assert resp == MOCK_CUSTOMER_LIST, 'Expected response' def test_sifta_handling_bad_status_code(): - sifta_service = SiftaService(ENDPOINT) - with Mocker(session=sifta_service.session) as session_mock: - session_mock.get(f'{ENDPOINT}12345', status_code=500) - result = sifta_service.get_cooperators('12345') - - assert session_mock.call_count == 1 - assert result == [] + loop = asyncio.get_event_loop() + session = aiohttp.ClientSession() + with aioresponses() as mock: + mock.get(f'{app.config["COOPERATOR_SERVICE_ENDPOINT"]}12345', status=500) + resp = loop.run_until_complete(get_cooperators(session, '12345')) + assert resp == [] diff --git a/wdfn-server/waterdata/tests/test_utils.py b/wdfn-server/waterdata/tests/test_utils.py index 3ca2f185467bdd13204336a65ccdb2a3d2610c6e..8f354a87b88b15b92031088e8c696c8e197a9af9 100644 --- a/wdfn-server/waterdata/tests/test_utils.py +++ b/wdfn-server/waterdata/tests/test_utils.py @@ -10,7 +10,7 @@ import requests as r from .. import app -from ..utils import construct_url, defined_when, execute_get_request, parse_rdb, set_cookie_for_banner_message,\ +from ..utils import construct_url, execute_get_request, parse_rdb, set_cookie_for_banner_message,\ create_message @@ -19,7 +19,7 @@ class TestConstructUrl(TestCase): def setUp(self): self.test_netloc = 'https://fakeurl.gov' self.test_path = '/blah1/blah2' - self.test_params_dict = { 'param1': 'value1', 'param2': 'value2'} + self.test_params_dict = {'param1': 'value1', 'param2': 'value2'} self.test_params_sequence = (('param1', 'value1'), ('param2', 'value2')) def test_with_params_as_dict(self): @@ -174,29 +174,6 @@ class TestGetWaterServicesData(TestCase): self.assertEqual(result.text, '') -class TestDefinedWhen(TestCase): - def setUp(self): - pass - - def test_true(self): - @defined_when(True, lambda: 'fallback') - def decorated(): - return 'called' - self.assertEqual(decorated(), 'called') - - def test_false(self): - @defined_when(False, lambda: 'fallback') - def decorated(): - return 'called' - self.assertEqual(decorated(), 'fallback') - - def test_arg_passing(self): - @defined_when(True, lambda: 'fallback') - def decorated(*args, **kwargs): - return ','.join([*args, *kwargs.keys(), *kwargs.values()]) - self.assertEqual(decorated('1', '2', kw1='3', kw2='4'), '1,2,kw1,kw2,3,4') - - class TestParseRdb(TestCase): def setUp(self): diff --git a/wdfn-server/waterdata/tests/test_views.py b/wdfn-server/waterdata/tests/test_views.py index 5ed0dcf96f5bb0ef8cb5fe020dc45bf94c9c4664..a49799ae5f224c47df3dd91061a891d5c1062510 100644 --- a/wdfn-server/waterdata/tests/test_views.py +++ b/wdfn-server/waterdata/tests/test_views.py @@ -94,8 +94,8 @@ class TestMonitoringLocationView(TestCase): self.test_rdb_lines = self.test_rdb_text.split('\n') self.headers = {'Accept': 'application/ld+json'} - @mock.patch('waterdata.views.SiteService.get_period_of_record') - @mock.patch('waterdata.views.SiteService.get_site_data') + @mock.patch('waterdata.views.get_period_of_record') + @mock.patch('waterdata.views.get_site_data') def test_everything_okay(self, site_mock, param_mock): site_mock.return_value = (200, '', [datum for datum in parse_rdb(iter(SITE_RDB.split('\n')))]) @@ -118,7 +118,7 @@ class TestMonitoringLocationView(TestCase): self.assertEqual(json_ld_response.status_code, 200) self.assertIsInstance(json.loads(json_ld_response.data), dict) - @mock.patch('waterdata.views.SiteService.get_site_data') + @mock.patch('waterdata.views.get_site_data') def test_4xx_from_water_services(self, site_mock): site_mock.return_value = (400, 'Site number is invalid.', []) @@ -134,7 +134,7 @@ class TestMonitoringLocationView(TestCase): self.assertEqual(json_ld_response.status_code, 200) self.assertIsNone(json.loads(json_ld_response.data)) - @mock.patch('waterdata.views.SiteService.get_site_data') + @mock.patch('waterdata.views.get_site_data') def test_5xx_from_water_services(self, site_mock): site_mock.return_value = (500, '', None) @@ -149,44 +149,44 @@ class TestMonitoringLocationView(TestCase): self.assertEqual(json_ld_response.status_code, 503) self.assertIsNone(json.loads(json_ld_response.data)) - @mock.patch('waterdata.views.SiteService.get_site_data') + @mock.patch('waterdata.views.get_site_data') def test_agency_cd(self, site_mock): site_mock.return_value = (500, '', None) response = self.app_client.get('/monitoring-location/{0}/?agency_cd=USGS'.format(self.test_site_number)) - site_mock.assert_called_with(self.test_site_number, 'USGS') self.assertEqual(response.status_code, 503) -class TestHydrologicalUnitView: - # pylint: disable=R0201 - - @pytest.fixture(autouse=True) - def mock_site_call(self): - """Return the same mock site list for each call to the site service""" - with requests_mock.mock() as req: - url = re.compile('{host}.*'.format(host=app.config['SITE_DATA_ENDPOINT'])) - req.get(url, text=PARAMETER_RDB) - yield +class TestHydrologicalUnitView(TestCase): + def setUp(self): + self.app_client = app.test_client() - def test_huc2(self, client): - response = client.get('/hydrological-unit/') + @mock.patch('waterdata.views.get_huc_sites') + def test_huc2(self, site_mock): + site_mock.return_value = (200, '', [datum for datum in parse_rdb(iter(SITE_RDB.split('\n')))]) + response = self.app_client.get('/hydrological-unit/') assert response.status_code == 200 - def test_some_exist(self, client): + @mock.patch('waterdata.views.get_huc_sites') + def test_some_exist(self, site_mock): + site_mock.return_value = (200, '', [datum for datum in parse_rdb(iter(SITE_RDB.split('\n')))]) for huc_cd in list(app.config['HUC_LOOKUP']['hucs'].keys())[:20]: - response = client.get('/hydrological-unit/{}/'.format(huc_cd)) + response = self.app_client.get('/hydrological-unit/{}/'.format(huc_cd)) assert response.status_code == 200 - def test_404s(self, client): - response = client.get('/hydrological-unit/1/') + @mock.patch('waterdata.views.get_huc_sites') + def test_404s(self, site_mock): + site_mock.return_value = (200, '', [datum for datum in parse_rdb(iter(SITE_RDB.split('\n')))]) + response = self.app_client.get('/hydrological-unit/1/') assert response.status_code == 404 - def test_locations_list(self, client): - response = client.get('/hydrological-unit/01010001/monitoring-locations/') + @mock.patch('waterdata.views.get_huc_sites') + def test_locations_list(self, site_mock): + site_mock.return_value = (200, '', [datum for datum in parse_rdb(iter(SITE_RDB.split('\n')))]) + response = self.app_client.get('/hydrological-unit/01010001/monitoring-locations/') assert response.status_code == 200 text = response.data.decode('utf-8') # There are eight instances of this site in MOCK_SITE_LIST_2. - assert text.count('01630500') == 16, 'Expected site 01630500 in output' + assert text.count('01630500') == 2, 'Expected site 01630500 in output' class TestNetworkView(TestCase): @@ -220,39 +220,42 @@ class TestNetworkView(TestCase): assert response.status_code == 404 -class TestCountryStateCountyView: - # pylint: disable=R0201 - - @pytest.fixture(autouse=True) - def mock_site_call(self): - """Return the same mock site list for each call to the site service""" - with requests_mock.mock() as req: - url = re.compile('{host}.*'.format(host=app.config['SITE_DATA_ENDPOINT'])) - req.get(url, text=PARAMETER_RDB) - yield +class TestCountryStateCountyView(TestCase): + def setUp(self): + self.app_client = app.test_client() - def test_country_level_search_us(self, client): - response = client.get('/states/') + @mock.patch('waterdata.views.get_county_sites') + def test_country_level_search_us(self, site_mock): + site_mock.return_value = (200, '', [datum for datum in parse_rdb(iter(SITE_RDB.split('\n')))]) + response = self.app_client.get('/states/') assert response.status_code == 200 - def test_some_counties_exist(self, client): + @mock.patch('waterdata.views.get_county_sites') + def test_some_counties_exist(self, site_mock): + site_mock.return_value = (200, '', [datum for datum in parse_rdb(iter(SITE_RDB.split('\n')))]) for state_cd in list(app.config['COUNTRY_STATE_COUNTY_LOOKUP']['US']['state_cd'].keys())[:20]: - response = client.get('/states/{}/'.format(state_cd)) + response = self.app_client.get('/states/{}/'.format(state_cd)) assert response.status_code == 200 - def test_404s_incorrect_state_code(self, client): - response = client.get('/states/1/') + @mock.patch('waterdata.views.get_county_sites') + def test_404s_incorrect_state_code(self, site_mock): + site_mock.return_value = (200, '', [datum for datum in parse_rdb(iter(SITE_RDB.split('\n')))]) + response = self.app_client.get('/states/1/') assert response.status_code == 404 - def test_404s_incorrect_county_code(self, client): - response = client.get('/states/01/counties/1/') + @mock.patch('waterdata.views.get_county_sites') + def test_404s_incorrect_county_code(self, site_mock): + site_mock.return_value = (200, '', [datum for datum in parse_rdb(iter(SITE_RDB.split('\n')))]) + response = self.app_client.get('/states/01/counties/1/') assert response.status_code == 404 - def test_locations_list(self, client): - response = client.get('/states/23/counties/003/monitoring-locations/') + @mock.patch('waterdata.views.get_county_sites') + def test_locations_list(self, site_mock): + site_mock.return_value = (200, '', [datum for datum in parse_rdb(iter(SITE_RDB.split('\n')))]) + response = self.app_client.get('/states/23/counties/003/monitoring-locations/') assert response.status_code == 200 text = response.data.decode('utf-8') - assert text.count('01630500') == 16, 'Expected site 01630500 in output' + assert text.count('01630500') == 2, 'Expected site 01630500 in output' class TestTimeSeriesComponentView: diff --git a/wdfn-server/waterdata/utils.py b/wdfn-server/waterdata/utils.py index f13fae09b003a3364e0f8e56abc455195e825bb1..c82b9bc77dfd985ea92cf17ebebcbf4b227f0f99 100644 --- a/wdfn-server/waterdata/utils.py +++ b/wdfn-server/waterdata/utils.py @@ -2,9 +2,8 @@ Utility functions """ -from functools import update_wrapper -from urllib.parse import urlencode, urljoin from email.message import EmailMessage +from urllib.parse import urlencode, urljoin from flask import request @@ -124,23 +123,3 @@ def parse_rdb(rdb_iter_lines): continue record_values = record.split('\t') yield dict(zip(headers, record_values)) - - -def defined_when(condition, fallback): - """ - Decorator that fallsback to a specified function if `condition` is False. - :param condition: bool Decorated function will be called if True, otherwise - fallback will be called - :param fallback: function to be called if condition is False - :return: Decorated function - :rtype: function - """ - def wrap(f): # pylint: disable=invalid-name - if condition: - # pylint:disable=do-not-assign-a-lambda-expression-use-a-def, unnecessary-lambda - func = lambda *args, **kwargs: f(*args, **kwargs) # flake8: noqa - else: - func = fallback - return update_wrapper(func, f) - - return wrap diff --git a/wdfn-server/waterdata/views.py b/wdfn-server/waterdata/views.py index fa3b7c8eb0468defbc034a1c7fe2f101e5563a6e..b1b780c1d8bcee891792f60042f68ae5162eed61 100644 --- a/wdfn-server/waterdata/views.py +++ b/wdfn-server/waterdata/views.py @@ -1,10 +1,12 @@ """ Main application views. """ +import asyncio import datetime import json import smtplib +import aiohttp from ua_parser import user_agent_parser from flask import abort, render_template, redirect, request, Markup, make_response, url_for @@ -14,21 +16,19 @@ from markdown import markdown from . import app, __version__ from .location_utils import build_linked_data, get_disambiguated_values, rollup_dataseries, \ get_period_of_record_by_parm_cd, get_default_parameter_code -from .utils import defined_when, set_cookie_for_banner_message, create_message +from .utils import set_cookie_for_banner_message, create_message from .services.camera import get_monitoring_location_camera_details -from .services.nwissite import SiteService +from .services.nwissite import get_county_sites, get_huc_sites, get_site_data, get_period_of_record from .services.ogc import MonitoringLocationNetworkService -from .services.sifta import SiftaService +from .services.sifta import get_cooperators from .services.timezone import TimeZoneService # Station Fields Mapping to Descriptions from .constants import STATION_FIELDS_D -site_service = SiteService(app.config['SITE_DATA_ENDPOINT']) monitoring_location_network_service = \ MonitoringLocationNetworkService(app.config['MONITORING_LOCATIONS_OBSERVATIONS_ENDPOINT']) time_zone_service = TimeZoneService(app.config['WEATHER_SERVICE_ENDPOINT']) -sifta_service = SiftaService(app.config['COOPERATOR_SERVICE_ENDPOINT']) @app.context_processor @@ -102,23 +102,29 @@ def iv_data_availability(): @app.route('/monitoring-location/<site_no>/', methods=['GET']) -def monitoring_location(site_no): +async def monitoring_location(site_no): # pylint: disable=too-many-locals # TODO: refactor to use more utility functions so that this doesn't have too many locals # pylint: disable=fixme """ Monitoring Location view - :param site_no: USGS site number - """ agency_cd = request.args.get('agency_cd', '') - site_status, site_status_reason, site_data = site_service.get_site_data(site_no, agency_cd) + + async with aiohttp.ClientSession() as session: + site_data_task = asyncio.create_task(get_site_data(session, site_no, agency_cd)) + period_of_record_task = asyncio.create_task(get_period_of_record(session, site_no, agency_cd)) + cooperators_task = asyncio.create_task(get_cooperators(session, site_no)) + + site_data_resp, period_of_record_resp, cooperators = \ + await asyncio.gather(site_data_task, period_of_record_task, cooperators_task) + site_status, site_status_reason, site_data = site_data_resp json_ld = None if site_status == 200: template = 'monitoring_location.html' context = { - 'status_code': site_status, + 'status_code': 200, 'stations': site_data, 'STATION_FIELDS_D': STATION_FIELDS_D } @@ -126,7 +132,7 @@ def monitoring_location(site_no): if len(site_data) == 1: unique_site = site_data[0] - _, _, period_of_record = site_service.get_period_of_record(site_no, agency_cd) + _, _, period_of_record = period_of_record_resp iv_period_of_record = get_period_of_record_by_parm_cd(period_of_record, 'uv') gw_period_of_record = get_period_of_record_by_parm_cd(period_of_record, 'gw') if app.config[ 'GROUNDWATER_LEVELS_ENABLED'] else {} @@ -166,8 +172,6 @@ def monitoring_location(site_no): except KeyError: site_owner_state = None - cooperators = sifta_service.get_cooperators(site_no) - if site_owner_state is not None: email_for_data_questions = \ app.config['EMAIL_TARGET']['contact'].format(state_district_code=site_owner_state.lower()) @@ -229,8 +233,7 @@ def return_404(): @app.route('/hydrological-unit/', defaults={'huc_cd': None}, methods=['GET']) @app.route('/hydrological-unit/<huc_cd>/', methods=['GET']) -@defined_when(app.config['HYDROLOGIC_PAGES_ENABLED'], return_404) -def hydrological_unit(huc_cd, show_locations=False): +async def hydrological_unit(huc_cd, show_locations=False): """ Hydrological unit view :param str huc_cd: ID for this unit @@ -238,12 +241,18 @@ def hydrological_unit(huc_cd, show_locations=False): """ # Get the data corresponding to this HUC + if not app.config['HYDROLOGIC_PAGES_ENABLED']: + return return_404() + monitoring_locations = [] if huc_cd: huc = app.config['HUC_LOOKUP']['hucs'].get(huc_cd, None) # If this is a HUC8 site, get the monitoring locations within it. if huc and show_locations: - _, _, monitoring_locations = site_service.get_huc_sites(huc_cd) + async with aiohttp.ClientSession() as session: + huc_task = asyncio.create_task(get_huc_sites(session, huc_cd)) + huc_resp = await asyncio.gather(huc_task) + _, _, monitoring_locations = huc_resp[0] # If we don't have a HUC, display all the root HUC2 units as children. else: @@ -264,12 +273,11 @@ def hydrological_unit(huc_cd, show_locations=False): @app.route('/hydrological-unit/<huc_cd>/monitoring-locations/', methods=['GET']) -@defined_when(app.config['HYDROLOGIC_PAGES_ENABLED'], return_404) -def hydrological_unit_locations(huc_cd): +async def hydrological_unit_locations(huc_cd): """ Returns a HUC page with a list of monitoring locations included. """ - return hydrological_unit(huc_cd, show_locations=True) + return await hydrological_unit(huc_cd, show_locations=True) @app.route('/networks/', defaults={'network_cd': ''}, methods=['GET']) @@ -315,8 +323,7 @@ def networks(network_cd): @app.route('/states/', defaults={'state_cd': None, 'county_cd': None}, methods=['GET']) @app.route('/states/<state_cd>/', defaults={'county_cd': None}, methods=['GET']) @app.route('/states/<state_cd>/counties/<county_cd>/', methods=['GET']) -@defined_when(app.config['STATE_COUNTY_PAGES_ENABLED'], return_404) -def states_counties(state_cd, county_cd, show_locations=False): +async def states_counties(state_cd, county_cd, show_locations=False): """ State unit view @@ -324,6 +331,8 @@ def states_counties(state_cd, county_cd, show_locations=False): :param county_cd: ID for this political unit - 'county' :param bool show_locations: """ + if not app.config['STATE_COUNTY_PAGES_ENABLED']: + return return_404() monitoring_locations = [] political_unit = {} @@ -333,7 +342,10 @@ def states_counties(state_cd, county_cd, show_locations=False): political_unit = app.config['COUNTRY_STATE_COUNTY_LOOKUP']['US']['state_cd'].get(state_cd, None)['county_cd']\ .get(county_cd, None) if show_locations: - _, _, monitoring_locations = site_service.get_county_sites(state_county_cd) + async with aiohttp.ClientSession() as session: + county_task = asyncio.create_task(get_county_sites(session, state_county_cd)) + county_resp = await asyncio.gather(county_task) + _, _, monitoring_locations = county_resp[0] # Get the data corresponding to this state elif state_cd and not county_cd: @@ -360,18 +372,18 @@ def states_counties(state_cd, county_cd, show_locations=False): @app.route('/states/<state_cd>/counties/<county_cd>/monitoring-locations/', methods=['GET']) -@defined_when(app.config['STATE_COUNTY_PAGES_ENABLED'], return_404) -def county_station_locations(state_cd, county_cd): +async def county_station_locations(state_cd, county_cd): """ Returns a page listing monitoring locations within a county. """ - return states_counties(state_cd, county_cd, show_locations=True) + return await states_counties(state_cd, county_cd, show_locations=True) @app.route('/components/time-series/<site_no>/', methods=['GET']) -@defined_when(app.config['EMBED_IMAGE_FEATURE_ENABLED'], return_404) def time_series_component(site_no): """ Returns an unadorned page with the time series component for a site. """ + if not app.config['EMBED_IMAGE_FEATURE_ENABLED']: + return_404() return render_template('monitoring_location_embed.html', site_no=site_no)