DISCO-3202 - Enrich the curated-recommendations endpoint with icon-urls

Author: gruberb

merino/curated_recommendations/protocol.py

@@ -85,6 +85,7 @@ class CuratedRecommendation(CorpusItem):
     __typename: TypeName = TypeName.RECOMMENDATION
     tileId: Annotated[int, Field(strict=True, ge=MIN_TILE_ID, le=MAX_TILE_ID)]
     receivedRank: int
+    icon: str | None = None
 
     @model_validator(mode="before")
     def set_tileId(cls, values):

merino/providers/manifest/provider.py

@@ -3,6 +3,9 @@
 import asyncio
 import time
 import logging
+from urllib.parse import urlparse
+
+from pydantic import HttpUrl
 
 from merino.providers.manifest.backends.filemanager import GetManifestResultCode
 from merino.utils import cron
@@ -20,6 +23,7 @@ class Provider:
     """Provide access to in-memory manifest data fetched from GCS."""
 
     manifest_data: ManifestData | None
+    lookup_table: dict[str, int]
     cron_task: asyncio.Task
     resync_interval_sec: int
     cron_interval_sec: int
@@ -39,6 +43,7 @@ def __init__(
         self.cron_interval_sec = cron_interval_sec
         self.last_fetch_at = 0.0
         self.manifest_data = ManifestData(domains=[])
+        self.lookup_table = {}
         self.data_fetched_event = asyncio.Event()
 
         super().__init__()
@@ -63,8 +68,12 @@ async def _fetch_data(self) -> None:
             result_code, data = await self.backend.fetch()
 
             match GetManifestResultCode(result_code):
-                case GetManifestResultCode.SUCCESS:
+                case GetManifestResultCode.SUCCESS if data is not None:
                     self.manifest_data = data
+                    self.lookup_table = {
+                        domain.domain: idx for idx, domain in enumerate(data.domains)
+                    }
+                    logger.info(f"Built lookup table: {self.lookup_table}")
                     self.last_fetch_at = time.time()
 
                 case GetManifestResultCode.SKIP:
@@ -86,3 +95,23 @@ def _should_fetch(self) -> bool:
     def get_manifest_data(self) -> ManifestData | None:
         """Return manifest data"""
         return self.manifest_data
+
+    def get_icon_url(self, url: str | HttpUrl) -> str | None:
+        """Get icon URL for a domain.
+
+        Args:
+            url: Full URL to extract domain from (string or HttpUrl)
+
+        Returns:
+            Icon URL if found, None otherwise
+        """
+        try:
+            url_str = str(url)
+            domain = urlparse(url_str).netloc.replace("www.", "")
+            idx = self.lookup_table.get(domain)
+            if idx is not None and self.manifest_data is not None:
+                return self.manifest_data.domains[idx].icon
+
+        except Exception as e:
+            logger.warning(f"Error getting icon for URL {url}: {e}")
+        return None

merino/web/api_v1.py

@@ -39,7 +39,7 @@
 )
 
 from merino.web.models_v1 import ProviderResponse, SuggestResponse
-from merino.providers.manifest.provider import Provider as ManifestProvider
+from merino.providers.manifest.provider import Provider
 
 logger = logging.getLogger(__name__)
 router = APIRouter()
@@ -293,6 +293,7 @@ async def providers(
 async def curated_content(
     curated_recommendations_request: CuratedRecommendationsRequest,
     provider: CuratedRecommendationsProvider = Depends(get_corpus_api_provider),
+    manifest_provider: Provider = Depends(get_manifest_provider),
 ) -> CuratedRecommendationsResponse:
     """Query Merino for curated recommendations.
 
@@ -320,7 +321,28 @@ async def curated_content(
 
     [curated-topics-doc]: https://mozilla-hub.atlassian.net/wiki/x/LQDaMg
     """
-    return await provider.fetch(curated_recommendations_request)
+    response = await provider.fetch(curated_recommendations_request)
+
+    # Add icons to recommendations
+    for rec in response.data:
+        if rec.url and (icon_url := manifest_provider.get_icon_url(rec.url)):
+            rec.icon = icon_url
+
+    # Add icons to feeds if they exist
+    if response.feeds:
+        if response.feeds.need_to_know:
+            for rec in response.feeds.need_to_know.recommendations:
+                if rec.url and (icon_url := manifest_provider.get_icon_url(rec.url)):
+                    rec.icon = icon_url
+
+        for section_name in response.feeds.model_fields_set:
+            if section := getattr(response.feeds, section_name):
+                if hasattr(section, "recommendations"):
+                    for rec in section.recommendations:
+                        if rec.url and (icon_url := manifest_provider.get_icon_url(rec.url)):
+                            rec.icon = icon_url
+
+    return response
 
 
 @router.get(
@@ -330,7 +352,7 @@ async def curated_content(
     response_model=ManifestData,
 )
 async def get_manifest(
-    request: Request, provider: ManifestProvider = Depends(get_manifest_provider)
+    request: Request, provider: Provider = Depends(get_manifest_provider)
 ) -> ORJSONResponse:
     """Query merino for manifest data."""
     logger.info("Attempting to get manifest")

tests/integration/api/conftest.py

@@ -6,11 +6,16 @@
 
 from logging import LogRecord
 from typing import Iterator, Generator
+from unittest.mock import AsyncMock
 
 import pytest
 import orjson
 from starlette.testclient import TestClient
 from aiodogstatsd import Client as AioDogstatsdClient
+
+from merino.providers.manifest import Provider
+from merino.providers.manifest.backends.manifest import ManifestBackend
+from merino.providers.manifest.backends.protocol import GetManifestResultCode, ManifestData
 from merino.utils.gcs.gcs_uploader import GcsUploader
 from contextlib import nullcontext
 from merino.curated_recommendations.fakespot_backend.protocol import (
@@ -134,3 +139,42 @@ def fakespot_feed() -> FakespotFeed:
         footerCopy=FAKESPOT_FOOTER_COPY,
         cta=FakespotCTA(ctaCopy=FAKESPOT_CTA_COPY, url=FAKESPOT_CTA_URL),
     )
+
+
+@pytest.fixture
+def mock_manifest():
+    """Mock manifest data with a known icon URL."""
+    return {
+        "domains": [
+            {
+                "rank": 1,
+                "domain": "spotify.com",
+                "categories": ["Entertainment"],
+                "serp_categories": [0],
+                "url": "https://www.spotify.com",
+                "title": "Spotify",
+                "icon": "https://test.com/spotify-favicon.ico",
+            }
+        ]
+    }
+
+
+@pytest.fixture
+def mock_manifest_backend(mock_manifest):
+    """Mock ManifestBackend that returns our test data."""
+    backend = ManifestBackend()
+    backend.fetch = AsyncMock(
+        return_value=(GetManifestResultCode.SUCCESS, ManifestData(**mock_manifest))
+    )
+    return backend
+
+
+@pytest.fixture
+def manifest_provider(mock_manifest_backend):
+    """Override the manifest provider fixture with our mocked data."""
+    provider = Provider(
+        backend=mock_manifest_backend,
+        resync_interval_sec=86400,
+        cron_interval_sec=3600,
+    )
+    return provider

tests/integration/api/v1/curated_recommendations/test_curated_recommendations.py

@@ -46,6 +46,8 @@
 )
 from merino.curated_recommendations.protocol import CuratedRecommendation
 from merino.main import app
+from merino.providers.manifest import get_provider as get_manifest_provider
+from merino.providers.manifest.backends.protocol import Domain
 from tests.integration.api.conftest import fakespot_feed
 
 
@@ -121,6 +123,16 @@ def extended_expiration_corpus_backend(
     )
 
 
+@pytest.fixture(autouse=True)
+def setup_manifest_provider(manifest_provider):
+    """Set up the manifest provider dependency"""
+    app.dependency_overrides[get_manifest_provider] = lambda: manifest_provider
+    yield
+    # Clean up after test
+    if get_manifest_provider in app.dependency_overrides:
+        del app.dependency_overrides[get_manifest_provider]
+
+
 @pytest.fixture(name="corpus_provider")
 def provider(
     corpus_backend: CorpusApiBackend,
@@ -1529,3 +1541,69 @@ def mock_post_by_days_ago(*args, **kwargs):
             else:
                 # Check that only today's items are returned if in control or not in the experiment.
                 assert set(days_ago_counter.keys()) == {0}
+
+
+@pytest.mark.asyncio
+async def test_curated_recommendations_enriched_with_icons(
+    manifest_provider,
+    corpus_http_client,
+    fixture_request_data,
+):
+    """Test the enrichment of a curated recommendation with an added icon-url."""
+    manifest_provider.manifest_data.domains = [
+        Domain(
+            rank=2,
+            title="Microsoft – AI, Cloud, Productivity, Computing, Gaming & Apps",
+            url="https://www.microsoft.com",
+            domain="microsoft.com",
+            icon="https://merino-images.services.mozilla.com/favicons/microsoft-icon.png",
+            categories=["Business", "Information Technology"],
+            serp_categories=[0],
+        )
+    ]
+
+    manifest_provider.lookup_table = {"microsoft.com": 0}
+
+    mocked_response = {
+        "data": {
+            "scheduledSurface": {
+                "items": [
+                    {
+                        "id": "scheduledSurfaceItemId-ABC",
+                        "corpusItem": {
+                            "id": "corpusItemId-XYZ",
+                            "url": "https://www.microsoft.com/some-article?utm_source=firefox-newtab-en-us",
+                            "title": "Some MS Article",
+                            "excerpt": "All about Microsoft something",
+                            "topic": "tech",
+                            "publisher": "ExamplePublisher",
+                            "isTimeSensitive": False,
+                            "imageUrl": "https://somewhere.com/test.jpg",
+                        },
+                    }
+                ]
+            }
+        }
+    }
+    corpus_http_client.post.return_value = Response(
+        status_code=200,
+        json=mocked_response,
+        request=fixture_request_data,
+    )
+
+    async with AsyncClient(app=app, base_url="http://test") as ac:
+        response = await ac.post(
+            "/api/v1/curated-recommendations",
+            json={"locale": "en-US"},
+        )
+        assert response.status_code == 200
+
+    data = response.json()
+    items = data["data"]
+    assert len(items) == 1
+
+    item = items[0]
+    assert item["url"] == "https://www.microsoft.com/some-article?utm_source=firefox-newtab-en-us"
+
+    assert "icon" in item
+    assert item["icon"] == "https://merino-images.services.mozilla.com/favicons/microsoft-icon.png"

tests/integration/api/v1/manifest/test_manifest.py

@@ -11,24 +11,6 @@
 from merino.web.api_v1 import router
 
 
-@pytest.fixture(scope="module")
-def mock_manifest_2025() -> dict:
-    """Mock manifest json for the year 2025"""
-    return {
-        "domains": [
-            {
-                "rank": 1,
-                "domain": "spotify",
-                "categories": ["Entertainment"],
-                "serp_categories": [0],
-                "url": "https://www.spotify.com",
-                "title": "Spotify",
-                "icon": "",
-            }
-        ]
-    }
-
-
 @pytest.fixture(autouse=True, name="cleanup")
 def cleanup_tasks_fixture():
     """Return a method that cleans up existing cron tasks after initialization"""
@@ -46,9 +28,7 @@ async def cleanup_tasks(provider: Provider):
 
 
 @pytest.mark.asyncio
-async def test_get_manifest_success(
-    client_with_metrics, gcp_uploader, mock_manifest_2025, cleanup
-):
+async def test_get_manifest_success(client_with_metrics, gcp_uploader, mock_manifest, cleanup):
     """Uploads a manifest to the gcs bucket and verifies that the endpoint returns the uploaded file."""
     # initialize provider on startup
     await init_provider()
@@ -57,7 +37,7 @@ async def test_get_manifest_success(
     app.include_router(router, prefix="/api/v1")
 
     # upload a manifest file to GCS test container
-    gcp_uploader.upload_content(orjson.dumps(mock_manifest_2025), "top_picks_latest.json")
+    gcp_uploader.upload_content(orjson.dumps(mock_manifest), "top_picks_latest.json")
 
     provider = get_provider()
     await provider.data_fetched_event.wait()
@@ -69,7 +49,7 @@ async def test_get_manifest_success(
 
     manifest = ManifestData(**response.json())
     assert len(manifest.domains) == 1
-    assert manifest.domains[0].domain == "spotify"
+    assert manifest.domains[0].domain == "spotify.com"
 
     assert "Cache-Control" in response.headers
 

tests/unit/conftest.py

@@ -13,6 +13,7 @@
 from pytest_mock import MockerFixture
 
 from merino.middleware.geolocation import Location
+from merino.providers.manifest.backends.protocol import ManifestData
 from merino.providers.suggest.base import SuggestionRequest
 from tests.unit.types import SuggestionRequestFixture
 from google.cloud.storage import Blob, Bucket, Client
@@ -102,3 +103,9 @@ def fixture_blob_json() -> str:
             ]
         }
     )
+
+
+@pytest.fixture(name="manifest_data")
+def fixture_manifest_data(blob_json: str) -> ManifestData:
+    """Return parsed ManifestData object for testing."""
+    return ManifestData.model_validate_json(blob_json)

tests/unit/providers/manifest/test_init_provider.py

@@ -7,8 +7,7 @@
 from unittest.mock import patch
 import pytest
 
-from merino.providers.manifest import get_provider, init_provider
-from merino.providers.manifest.provider import Provider as ManifestProvider
+from merino.providers.manifest import get_provider, init_provider, Provider
 
 
 @pytest.mark.asyncio
@@ -19,7 +18,7 @@ async def test_init_provider() -> None:
     from merino.providers.manifest import provider
 
     assert provider is not None
-    assert isinstance(provider, ManifestProvider)
+    assert isinstance(provider, Provider)
     assert provider.name == "manifest"