Skip to content
This repository has been archived by the owner on Jul 26, 2024. It is now read-only.

Commit

Permalink
feat: Add an adm_mobile_max_tiles setting (DISCO-2306, #529)
Browse files Browse the repository at this point in the history
This commit adds an option to separately control the maximum number of
tiles returned to Desktop and mobile clients.

Desktop clients use the `adm_max_tiles` setting now, and mobile clients
use `adm_mobile_max_tiles` (falling back to `adm_max_tiles`).
  • Loading branch information
linabutler authored Apr 7, 2023
1 parent 2bdf407 commit 23f5ae6
Show file tree
Hide file tree
Showing 6 changed files with 132 additions and 10 deletions.
9 changes: 8 additions & 1 deletion src/adm/tiles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -253,13 +253,20 @@ pub async fn get_tiles(
let mut filtered: Vec<Tile> = Vec::new();
let iter = response.tiles.into_iter();
let filter = state.partner_filter.read().await;
let max_tiles = usize::from(if device_info.is_mobile() {
settings
.adm_mobile_max_tiles
.unwrap_or(settings.adm_max_tiles)
} else {
settings.adm_max_tiles
});
for tile in iter {
if let Some(tile) =
filter.filter_and_process(tile, location, &device_info, tags, metrics)?
{
filtered.push(tile);
}
if filtered.len() == settings.adm_max_tiles as usize {
if filtered.len() == max_tiles {
break;
}
}
Expand Down
9 changes: 6 additions & 3 deletions src/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,12 +103,14 @@ pub struct Settings {
pub adm_sub1: Option<String>,
/// adm Endpoint URL
pub adm_endpoint_url: String,
/// max number of tiles returned to non-mobile clients (default: 3)
pub adm_max_tiles: u8,
/// Mobile versions of the above
pub adm_mobile_partner_id: Option<String>,
pub adm_mobile_sub1: Option<String>,
pub adm_mobile_endpoint_url: Option<String>,
/// max number of tiles returned to clients (default: 2)
pub adm_max_tiles: u8,
/// max number of tiles returned to mobile clients (default: 2)
pub adm_mobile_max_tiles: Option<u8>,
/// number of tiles to query from ADM (default: 10)
pub adm_query_tile_count: u8,
/// Timeout requests to the ADM server after this many seconds (default: 5)
Expand Down Expand Up @@ -167,10 +169,11 @@ impl Default for Settings {
adm_endpoint_url: "".to_owned(),
adm_partner_id: None,
adm_sub1: None,
adm_max_tiles: 3,
adm_mobile_endpoint_url: None,
adm_mobile_partner_id: None,
adm_mobile_sub1: None,
adm_max_tiles: 2,
adm_mobile_max_tiles: Some(2),
adm_query_tile_count: 10,
adm_timeout: 5,
adm_settings: "".to_owned(),
Expand Down
8 changes: 8 additions & 0 deletions src/web/mock_adm_response1.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,14 @@
"image_url": "https://cdn.example.com/805.jpg",
"advertiser_url": "https://www.lph-nm.biz/?utm_source=google&utm_campaign=quux*{keyword}_m*{match-type}_d*BARBAZ_22",
"impression_url": "https://example.net/static?id=ABAD1DEA"
},
{
"id": 907,
"name": "Lasagna Come Out Tomorrow",
"click_url": "https://example.com/ctp?version=16.0.0&key=5.5&ci=6.6&ctag=1680739594006000000",
"image_url": "https://cdn.example.com/907.jpg",
"advertiser_url": "https://www.lasagna.restaurant/?utm_source=google&utm_campaign=xyzzy*{keyword}_m*{match-type}_d*FRED_56",
"impression_url": "https://example.net/static?id=CAFED00D"
}
]
}
52 changes: 47 additions & 5 deletions src/web/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,9 @@ pub fn advertiser_filters() -> AdmAdvertiserSettings {
"Los Pollos Hermanos": {
"US": [{ "host": "www.lph-nm.biz" }],
},
"Lasagna Come Out Tomorrow": {
"US": [{ "host": "www.lasagna.restaurant" }],
}
}})
.to_string(),
)
Expand Down Expand Up @@ -440,8 +443,8 @@ async fn basic_filtered() {

let result: Value = test::read_body_json(resp).await;
let tiles = result["tiles"].as_array().expect("!tiles.is_array()");
// remember, we cap at `settings.adm_max_tiles` (currently 2)
assert_eq!(tiles.len(), 2);
// for desktop, we cap at `settings.adm_max_tiles` (currently 3)
assert_eq!(tiles.len(), 3);
// Ensure the tile order from adM is preserved
let tile1 = &tiles[0];
assert_eq!(tile1["name"], "Acme");
Expand All @@ -461,6 +464,8 @@ async fn basic_filtered2() {
},
"Los Pollos Hermanos": {
},
"Lasagna Come Out Tomorrow": {
},
}

})
Expand Down Expand Up @@ -516,7 +521,44 @@ async fn basic_default() {

let result: Value = test::read_body_json(resp).await;
let tiles = result["tiles"].as_array().expect("!tiles.is_array()");
// remember, we cap at `settings.adm_max_tiles` (currently 2)
// for desktop, we cap at `settings.adm_max_tiles` (currently 3)
assert_eq!(tiles.len(), 3);
assert!(!tiles
.iter()
.any(|tile| tile["name"].as_str().unwrap() == "Lasagna Come Out Tomorrow"));
}

#[actix_web::test]
async fn basic_mobile() {
let adm = init_mock_adm(MOCK_RESPONSE1.to_owned());

let mut settings = Settings {
adm_endpoint_url: adm.endpoint_url,
adm_settings: AdmFilter::advertisers_to_string(advertiser_filters()),
..get_test_settings()
};
let app = init_app!(settings).await;

let req = test::TestRequest::get()
.uri("/v1/tiles")
.insert_header((header::USER_AGENT, UA_IPHONE))
.to_request();
let resp = test::call_service(&app, req).await;
assert_eq!(resp.status(), StatusCode::OK);

let content_type = resp.headers().get(header::CONTENT_TYPE);
assert!(content_type.is_some());
assert_eq!(
content_type
.unwrap()
.to_str()
.expect("Couldn't parse Content-Type"),
"application/json"
);

let result: Value = test::read_body_json(resp).await;
let tiles = result["tiles"].as_array().expect("!tiles.is_array()");
// for mobile, we cap at `settings.adm_mobile_max_tiles` (currently 2)
assert_eq!(tiles.len(), 2);
assert!(!tiles
.iter()
Expand Down Expand Up @@ -809,7 +851,7 @@ async fn include_regions() {
// "Dunder Mifflin" should be filtered out
let result: Value = test::read_body_json(resp).await;
let tiles = result["tiles"].as_array().expect("!tiles.is_array()");
assert_eq!(tiles.len(), 1);
assert_eq!(tiles.len(), 2);
assert_eq!(&tiles[0]["name"], "Acme");
}

Expand Down Expand Up @@ -1014,7 +1056,7 @@ async fn cache_header() {

let result: Value = test::read_body_json(resp).await;
let tiles = result["tiles"].as_array().expect("!tiles.is_array()");
assert_eq!(tiles.len(), 2);
assert_eq!(tiles.len(), 3);
}

#[actix_web::test]
Expand Down
37 changes: 37 additions & 0 deletions test-engineering/contract-tests/volumes/client/scenarios.yml
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,13 @@ scenarios:
image_size: null
impression_url: 'https://example.com/gb_desktop_macos?id=0001'
url: 'https://www.example.com/gb_desktop_macos'
- id: 76790
name: 'Example ORG'
click_url: 'https://example.org/gb_desktop_macos?version=16.0.0&key=7.2&ci=8.9&ctag=E1DE38C8972D0281F5556659A'
image_url: 'https://example.org/gb_desktop_macos02.jpg'
image_size: null
impression_url: 'https://example.org/gb_desktop_macos?id=0002'
url: 'https://www.example.org/gb_desktop_macos'
- request:
service: contile
method: GET
Expand Down Expand Up @@ -302,6 +309,36 @@ scenarios:
image_size: null
impression_url: 'https://example.org/gb_desktop_macos?id=0002'
url: 'https://www.example.org/gb_desktop_macos'
- request:
service: contile
method: GET
path: '/v1/tiles'
headers:
# Contile maps the User-Agent Header value to os-family and form-factor parameters
# The following value will result in os-family: ios and form-factor: phone
- name: User-Agent
value: 'Mozilla/5.0 (iPhone; CPU iPhone OS 14_8_1 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) FxiOS/91.0 Mobile/15E148 Safari/605.1.15'
# The following value will result in country-code: GB and region-code: ENG
- name: X-Forwarded-For
value: '81.2.69.192'
response:
status_code: 200
content:
tiles:
- id: 32348
name: 'Example COM'
click_url: 'https://example.com/gb_phone_ios?version=16.0.0&key=22.1&ci=6.2&ctag=1612376952400200000'
image_url: 'https://example.com/gb_phone_ios01.jpg'
image_size: null
impression_url: 'https://example.com/gb_phone_ios?id=0001'
url: 'https://www.example.com/gb_phone_ios'
- id: 76792
name: 'Example ORG'
click_url: 'https://example.org/gb_phone_ios?version=16.0.0&key=7.2&ci=8.9&ctag=E1DE38C8972D0281F5556659A'
image_url: 'https://example.org/gb_phone_ios02.jpg'
image_size: null
impression_url: 'https://example.org/gb_phone_ios?id=0002'
url: 'https://www.example.org/gb_phone_ios'

- name: success_200_OK_excluded_region
description: Test that Contile returns a 200 OK with an empty tiles array for excluded regions
Expand Down
27 changes: 26 additions & 1 deletion test-engineering/contract-tests/volumes/partner/GB/responses.yml
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ desktop:
headers: []
content:
tiles:
- id: 41236
- id: 41237
name: 'DunBroch'
click_url: 'https://dunbroch.co.uk/gb_desktop_linux?version=16.0.0&key=22.1&ci=6.2&ctag=1612376952400200000'
image_url: 'https://dunbroch.co.uk/gb_desktop_linux01.jpg'
Expand All @@ -72,3 +72,28 @@ desktop:
image_url: 'https://example.org/gb_desktop_linux02.jpg'
impression_url: 'https://example.org/gb_desktop_linux?id=0002'
advertiser_url: 'https://www.example.org/gb_desktop_linux'

phone:
ios:
status_code: 200
headers: []
content:
tiles:
- id: 41236
name: 'DunBroch'
click_url: 'https://dunbroch.co.uk/gb_phone_ios?version=16.0.0&key=22.1&ci=6.2&ctag=1612376952400200000'
image_url: 'https://dunbroch.co.uk/gb_phone_ios01.jpg'
impression_url: 'https://dunbroch.co.uk/gb_phone_ios?id=0001'
advertiser_url: 'https://www.dunbroch.co.uk/gb_phone_ios/2021'
- id: 32348
name: 'Example COM'
click_url: 'https://example.com/gb_phone_ios?version=16.0.0&key=22.1&ci=6.2&ctag=1612376952400200000'
image_url: 'https://example.com/gb_phone_ios01.jpg'
impression_url: 'https://example.com/gb_phone_ios?id=0001'
advertiser_url: 'https://www.example.com/gb_phone_ios'
- id: 76792
name: 'Example ORG'
click_url: 'https://example.org/gb_phone_ios?version=16.0.0&key=7.2&ci=8.9&ctag=E1DE38C8972D0281F5556659A'
image_url: 'https://example.org/gb_phone_ios02.jpg'
impression_url: 'https://example.org/gb_phone_ios?id=0002'
advertiser_url: 'https://www.example.org/gb_phone_ios'

0 comments on commit 23f5ae6

Please sign in to comment.