From 256ba17332e9831dc2bd998161a53a73b80ac334 Mon Sep 17 00:00:00 2001
From: Michael Cooper <mythmon@gmail.com>
Date: Wed, 24 Nov 2021 10:41:35 -0800
Subject: [PATCH] feat(adm): Make suggestion score configurable

Also changes the default from 0.2 to 0.3.

Fixes #234.
---
 merino-adm/src/remote_settings/mod.rs   | 18 ++++++++++++----
 merino-integration-tests/src/suggest.rs | 28 +++++++++++++++++++++++++
 merino-settings/src/providers.rs        |  4 ++++
 3 files changed, 46 insertions(+), 4 deletions(-)

diff --git a/merino-adm/src/remote_settings/mod.rs b/merino-adm/src/remote_settings/mod.rs
index ead6839538..e7f290e729 100644
--- a/merino-adm/src/remote_settings/mod.rs
+++ b/merino-adm/src/remote_settings/mod.rs
@@ -3,6 +3,7 @@
 mod client;
 
 use crate::remote_settings::client::RemoteSettingsClient;
+use anyhow::Context;
 use async_trait::async_trait;
 use cadence::{Histogrammed, StatsdClient};
 use deduped_dashmap::DedupedMap;
@@ -16,7 +17,7 @@ use merino_suggest::{
 };
 use serde::{Deserialize, Serialize};
 use serde_with::{serde_as, DisplayFromStr};
-use std::{collections::HashMap, sync::Arc, time::Instant};
+use std::{collections::HashMap, convert::TryInto, sync::Arc, time::Instant};
 
 lazy_static! {
     static ref NON_SPONSORED_IAB_CATEGORIES: Vec<&'static str> = vec!["5 - Education"];
@@ -56,8 +57,14 @@ impl RemoteSettingsSuggester {
                 .clone(),
         )?;
         let suggestions = Arc::new(DedupedMap::new());
+        let suggestion_score = config
+            .suggestion_score
+            .try_into()
+            .context("converting score to proportion")
+            .map_err(SetupError::InvalidConfiguration)?;
+
+        Self::sync(&mut remote_settings_client, &*suggestions, suggestion_score).await?;
 
-        Self::sync(&mut remote_settings_client, &*suggestions).await?;
         {
             let task_suggestions = Arc::clone(&suggestions);
             let task_interval = config.resync_interval;
@@ -74,7 +81,9 @@ impl RemoteSettingsSuggester {
                     timer.tick().await;
                     let loop_suggestions = &*(Arc::clone(&task_suggestions));
                     if let Some(loop_client) = Arc::get_mut(&mut task_client) {
-                        if let Err(error) = Self::sync(loop_client, loop_suggestions).await {
+                        if let Err(error) =
+                            Self::sync(loop_client, loop_suggestions, suggestion_score).await
+                        {
                             tracing::error!(
                                 ?error,
                                 "Error while syncing remote settings suggestions"
@@ -108,6 +117,7 @@ impl RemoteSettingsSuggester {
     pub async fn sync(
         remote_settings_client: &mut RemoteSettingsClient,
         suggestions: &DedupedMap<String, (), Suggestion>,
+        suggestion_score: Proportion,
     ) -> Result<(), SetupError> {
         tracing::info!(
             r#type = "adm.remote-settings.sync-start",
@@ -179,7 +189,7 @@ impl RemoteSettingsSuggester {
                     is_sponsored: !NON_SPONSORED_IAB_CATEGORIES
                         .contains(&adm_suggestion.iab_category.as_str()),
                     icon: icon_url,
-                    score: Proportion::from(0.2),
+                    score: suggestion_score,
                 };
 
                 for keyword in &adm_suggestion.keywords {
diff --git a/merino-integration-tests/src/suggest.rs b/merino-integration-tests/src/suggest.rs
index 03e84c4f9f..48ae31d4de 100644
--- a/merino-integration-tests/src/suggest.rs
+++ b/merino-integration-tests/src/suggest.rs
@@ -3,6 +3,7 @@
 
 use crate::{merino_test_macro, utils::test_tools::TestReqwestClient, TestingTools};
 use anyhow::Result;
+use lazy_static::lazy_static;
 use merino_settings::providers::{
     FixedConfig, KeywordFilterConfig, MultiplexerConfig, RemoteSettingsConfig,
     SuggestionProviderConfig,
@@ -167,6 +168,33 @@ async fn suggest_adm_rs_works_content(
     Ok(())
 }
 
+lazy_static! {
+    static ref TEST_SCORE: f64 = std::f64::consts::TAU.fract();
+}
+
+#[merino_test_macro(|settings| {
+    settings.suggestion_providers.insert(
+        "adm".to_string(),
+        SuggestionProviderConfig::RemoteSettings(RemoteSettingsConfig {
+            suggestion_score: *TEST_SCORE as f32,
+            ..RemoteSettingsConfig::default()
+        })
+    );
+    settings.remote_settings.test_changes = Some(vec!["apple".to_string()]);
+})]
+async fn suggest_adm_rs_score_is_configurable(
+    TestingTools { test_client, .. }: TestingTools,
+) -> Result<()> {
+    let response = test_client.get("/api/v1/suggest?q=apple").send().await?;
+
+    assert_eq!(response.status(), StatusCode::OK);
+    let body: serde_json::Value = response.json().await?;
+    // Check that the score is approximately correct.
+    assert!((body["suggestions"][0]["score"].as_f64().unwrap() - *TEST_SCORE).abs() < 0.001);
+
+    Ok(())
+}
+
 #[merino_test_macro(|settings| {
     // Wiki fruit is only enabled when debug is true.
     settings.debug = true;
diff --git a/merino-settings/src/providers.rs b/merino-settings/src/providers.rs
index a6648f1e19..b460eb16d1 100644
--- a/merino-settings/src/providers.rs
+++ b/merino-settings/src/providers.rs
@@ -139,6 +139,9 @@ pub struct RemoteSettingsConfig {
     #[serde_as(as = "DurationSeconds")]
     #[serde(rename = "resync_interval_sec")]
     pub resync_interval: Duration,
+
+    /// The score value to assign to suggestions. A float between 0.0 and 1.0 inclusive.
+    pub suggestion_score: f32,
 }
 
 impl Default for RemoteSettingsConfig {
@@ -147,6 +150,7 @@ impl Default for RemoteSettingsConfig {
             bucket: None,
             collection: None,
             resync_interval: Duration::from_secs(60 * 60 * 3), // 3 hours
+            suggestion_score: 0.3,
         }
     }
 }