Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DISCO-2028] Cache weather reports in Redis #202

Merged
merged 4 commits into from
Feb 24, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
test: add new unit tests for cached weather suggestions
  • Loading branch information
linabutler committed Feb 24, 2023
commit d62ff4cca8a87d1cca6fd2dd10886a3d08e3384e
34 changes: 34 additions & 0 deletions tests/unit/providers/weather/backends/test_accuweather.py
Original file line number Diff line number Diff line change
Expand Up @@ -787,3 +787,37 @@ async def test_get_forecast_error(
await accuweather.get_forecast(mock_client, location_key)

assert str(accuweather_error.value) == expected_error_value


@pytest.mark.parametrize(
"cache_inputs_by_location",
[
(Location(country="US", region="CA", city="San Francisco", dma=807), None),
(
Location(region="CA", city="San Francisco", dma=807, postal_code="94105"),
None,
),
(
Location(
country="US",
region="CA",
city="San Francisco",
dma=807,
postal_code="94105",
),
b"US94105",
),
],
)
@pytest.mark.asyncio
async def test_cache_inputs_for_weather_report(
accuweather: AccuweatherBackend,
cache_inputs_by_location: tuple[Location, Optional[bytes]],
) -> None:
"""Test that `cache_inputs_for_weather_report` computes the correct cache inputs when
given locations with various missing fields.
"""
cache_inputs: Optional[bytes] = accuweather.cache_inputs_for_weather_report(
cache_inputs_by_location[0]
)
assert cache_inputs == cache_inputs_by_location[1]
290 changes: 288 additions & 2 deletions tests/unit/providers/weather/test_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@

"""Unit tests for the weather provider module."""

from typing import Any
from typing import Any, cast

import pytest
from pytest import LogCaptureFixture
from pytest_mock import MockerFixture
from redis.asyncio import Redis
from redis.asyncio import Redis, RedisError

from merino.cache.redis import RedisAdapter
from merino.config import settings
Expand Down Expand Up @@ -174,3 +174,289 @@ async def test_query_error(
for record in filter_caplog(caplog.records, "merino.providers.weather.provider")
]
assert actual_log_messages == expected_log_messages


@pytest.mark.asyncio
async def test_query_cached_weather_report(
redis_mock: Any,
backend_mock: Any,
provider: Provider,
geolocation: Location,
) -> None:
"""Test that weather reports are cached in Redis after the first request to the backend."""
cache_keys: dict[str, bytes] = {}
ncloudioj marked this conversation as resolved.
Show resolved Hide resolved

async def mock_redis_get(key) -> Any:
return cache_keys.get(key, None)

redis_mock.get.side_effect = mock_redis_get

async def mock_redis_set(key, value, **kwargs):
cache_keys[key] = value

redis_mock.set.side_effect = mock_redis_set

report: WeatherReport = WeatherReport(
city_name="San Francisco",
current_conditions=CurrentConditions(
url=(
"http://www.accuweather.com/en/us/san-francisco-ca/"
"94103/current-weather/39376_pc?lang=en-us"
),
summary="Mostly cloudy",
icon_id=6,
temperature=Temperature(c=15.5, f=60.0),
),
forecast=Forecast(
url=(
"http://www.accuweather.com/en/us/san-francisco-ca/"
"94103/daily-weather-forecast/39376_pc?lang=en-us"
),
summary="Pleasant Saturday",
high=Temperature(c=21.1, f=70.0),
low=Temperature(c=13.9, f=57.0),
),
)
backend_mock.cache_inputs_for_weather_report.return_value = cast(
str, geolocation.city
).encode("utf-8") + cast(str, geolocation.postal_code).encode("utf-8")
backend_mock.get_weather_report.return_value = report

expected_suggestions: list[Suggestion] = [
Suggestion(
title="Weather for San Francisco",
url=(
"http://www.accuweather.com/en/us/san-francisco-ca/"
"94103/current-weather/39376_pc?lang=en-us"
),
provider="weather",
is_sponsored=False,
score=settings.providers.accuweather.score,
icon=None,
city_name=report.city_name,
current_conditions=report.current_conditions,
forecast=report.forecast,
)
]

uncached_suggestions: list[BaseSuggestion] = await provider.query(
SuggestionRequest(query="", geolocation=geolocation)
)
assert uncached_suggestions == expected_suggestions

cache_key = provider.cache_key_for_weather_report(geolocation)
assert cache_key is not None
redis_mock.get.assert_called_once_with(cache_key)
backend_mock.get_weather_report.assert_called_once()
redis_mock.set.assert_called_once_with(
cache_key, report.json().encode("utf-8"), ex=10
)
assert cache_keys[cache_key] is not None

redis_mock.reset_mock()
backend_mock.reset_mock()

cached_suggestions: list[BaseSuggestion] = await provider.query(
SuggestionRequest(query="", geolocation=geolocation)
)
assert cached_suggestions == expected_suggestions

redis_mock.get.assert_called_once_with(cache_key)
backend_mock.get_weather_report.assert_not_called()
redis_mock.set.assert_not_called()


@pytest.mark.asyncio
async def test_query_cached_no_weather_report(
redis_mock: Any,
backend_mock: Any,
provider: Provider,
geolocation: Location,
) -> None:
"""Test that the absence of a weather report for a location is cached in Redis, avoiding
multiple requests to the backend.
"""
cache_keys: dict[str, bytes] = {}

async def mock_redis_get(key) -> Any:
return cache_keys.get(key, None)

redis_mock.get.side_effect = mock_redis_get

async def mock_redis_set(key, value, **kwargs):
cache_keys[key] = value

redis_mock.set.side_effect = mock_redis_set

backend_mock.cache_inputs_for_weather_report.return_value = cast(
str, geolocation.city
).encode("utf-8") + cast(str, geolocation.postal_code).encode("utf-8")
backend_mock.get_weather_report.return_value = None

uncached_suggestions: list[BaseSuggestion] = await provider.query(
SuggestionRequest(query="", geolocation=geolocation)
)
assert uncached_suggestions == []

cache_key = provider.cache_key_for_weather_report(geolocation)
assert cache_key is not None
redis_mock.get.assert_called_once_with(cache_key)
backend_mock.get_weather_report.assert_called_once()
redis_mock.set.assert_called_once_with(cache_key, b"{}", ex=10)
assert cache_keys[cache_key] is not None

redis_mock.reset_mock()
backend_mock.reset_mock()

cached_suggestions: list[BaseSuggestion] = await provider.query(
SuggestionRequest(query="", geolocation=geolocation)
)
assert cached_suggestions == []

redis_mock.get.assert_called_once_with(cache_key)
backend_mock.get_weather_report.assert_not_called()
redis_mock.set.assert_not_called()


@pytest.mark.asyncio
async def test_query_with_bad_cache_entry(
redis_mock: Any,
backend_mock: Any,
provider: Provider,
geolocation: Location,
) -> None:
"""Test that a bad cache entry causes the provider to make a request to the backend."""
backend_mock.cache_inputs_for_weather_report.return_value = cast(
str, geolocation.city
).encode("utf-8") + cast(str, geolocation.postal_code).encode("utf-8")
cache_key = provider.cache_key_for_weather_report(geolocation)
assert cache_key is not None

cache_keys: dict[str, bytes] = {
cache_key: b"badjson!",
}

async def mock_redis_get(key) -> Any:
return cache_keys.get(key, None)

redis_mock.get.side_effect = mock_redis_get

async def mock_redis_set(key, value, **kwargs):
cache_keys[key] = value

redis_mock.set.side_effect = mock_redis_set

report: WeatherReport = WeatherReport(
city_name="San Francisco",
current_conditions=CurrentConditions(
url=(
"http://www.accuweather.com/en/us/san-francisco-ca/"
"94103/current-weather/39376_pc?lang=en-us"
),
summary="Mostly cloudy",
icon_id=6,
temperature=Temperature(c=15.5, f=60.0),
),
forecast=Forecast(
url=(
"http://www.accuweather.com/en/us/san-francisco-ca/"
"94103/daily-weather-forecast/39376_pc?lang=en-us"
),
summary="Pleasant Saturday",
high=Temperature(c=21.1, f=70.0),
low=Temperature(c=13.9, f=57.0),
),
)
backend_mock.get_weather_report.return_value = report

expected_suggestions: list[Suggestion] = [
Suggestion(
title="Weather for San Francisco",
url=(
"http://www.accuweather.com/en/us/san-francisco-ca/"
"94103/current-weather/39376_pc?lang=en-us"
),
provider="weather",
is_sponsored=False,
score=settings.providers.accuweather.score,
icon=None,
city_name=report.city_name,
current_conditions=report.current_conditions,
forecast=report.forecast,
)
]

suggestions: list[BaseSuggestion] = await provider.query(
SuggestionRequest(query="", geolocation=geolocation)
)
assert suggestions == expected_suggestions

redis_mock.get.assert_called_once_with(cache_key)
backend_mock.get_weather_report.assert_called_once()
redis_mock.set.assert_called_once_with(
cache_key, report.json().encode("utf-8"), ex=10
)


@pytest.mark.asyncio
async def test_query_redis_unavailable(
redis_mock: Any,
backend_mock: Any,
provider: Provider,
geolocation: Location,
) -> None:
"""Test that Redis errors don't prevent requests to the backend."""
redis_mock.get.side_effect = RedisError("mercury in retrograde")
redis_mock.set.side_effect = RedisError("synergies not aligned")
linabutler marked this conversation as resolved.
Show resolved Hide resolved

report: WeatherReport = WeatherReport(
city_name="San Francisco",
current_conditions=CurrentConditions(
url=(
"http://www.accuweather.com/en/us/san-francisco-ca/"
"94103/current-weather/39376_pc?lang=en-us"
),
summary="Mostly cloudy",
icon_id=6,
temperature=Temperature(c=15.5, f=60.0),
),
forecast=Forecast(
url=(
"http://www.accuweather.com/en/us/san-francisco-ca/"
"94103/daily-weather-forecast/39376_pc?lang=en-us"
),
summary="Pleasant Saturday",
high=Temperature(c=21.1, f=70.0),
low=Temperature(c=13.9, f=57.0),
),
)
backend_mock.cache_inputs_for_weather_report.return_value = cast(
str, geolocation.city
).encode("utf-8") + cast(str, geolocation.postal_code).encode("utf-8")
backend_mock.get_weather_report.return_value = report

expected_suggestions: list[Suggestion] = [
Suggestion(
title="Weather for San Francisco",
url=(
"http://www.accuweather.com/en/us/san-francisco-ca/"
"94103/current-weather/39376_pc?lang=en-us"
),
provider="weather",
is_sponsored=False,
score=settings.providers.accuweather.score,
icon=None,
city_name=report.city_name,
current_conditions=report.current_conditions,
forecast=report.forecast,
)
]

suggestions: list[BaseSuggestion] = await provider.query(
SuggestionRequest(query="", geolocation=geolocation)
)
assert suggestions == expected_suggestions

redis_mock.get.assert_called_once()
backend_mock.get_weather_report.assert_called_once()
redis_mock.set.assert_called_once()