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

Commit

Permalink
feat: Add support for multiple ADM endpoints (#346)
Browse files Browse the repository at this point in the history
* chore: Update to edition 2021
* feat: Add support for multiple ADM endpoints

ADM requires different endpoint data for mobile vs non-mobile tile
requests.

This patch also changes `AdmSettings` to `AdmFilterSettings` since
that's more explicit about what they are.

* Also, Firefox on iPad returns the default Mac Safari desktop UA string (per guidance by Apple for applications using the WebView on iPad.) Code has been modified to handle this.
* out-of-area tiles will receive and empty `200` list (see #290)
* `position` field has been removed from the response to the UA (#347)

Issue: #345, #347, CONSVC-1562


Co-authored-by: Raphael Pierzina <raphael@hackebrot.de>
  • Loading branch information
jrconlin and hackebrot authored Feb 8, 2022
1 parent 8d13e88 commit 80d7dca
Show file tree
Hide file tree
Showing 18 changed files with 167 additions and 153 deletions.
1 change: 0 additions & 1 deletion .cargo/audit.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,4 @@ ignore = [
"RUSTSEC-2020-0071", # Time 0.14.3 - Potential segfault in time crate.
"RUSTSEC-2020-0159", # Chrono 0.4.19 - Potential segfault in `localtime_r` invokations.
"RUSTSEC-2021-0124", # Bound by tokio restrictions
"RUSTSEC-2021-0131", # bound by actix-http 2.2.1
]
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ authors = [
"jrconlin <jr+git@mozilla.com>",
"Mozilla Services Engineering <services-engineering+code@mozilla.com>"
]
edition = "2018"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

Expand Down
2 changes: 1 addition & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
ARG APPNAME=contile

# make sure that the build and run environments are the same version
FROM rust:1.54-slim-buster as builder
FROM rust:1.58-slim-buster as builder
ARG APPNAME
ADD . /app
WORKDIR /app
Expand Down
46 changes: 22 additions & 24 deletions docs/API.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ Servers

|environment | server |
|--|--|
|dev| https://contile-dev.topsites.nonprod.cloudops.mozgcp.net/ |
|stage| https://contile-stage.topsites.nonprod.cloudops.mozgcp.net/ |
|prod| https://contile.services.mozilla.com/ |
|dev| <https://contile-dev.topsites.nonprod.cloudops.mozgcp.net/> |
|stage| <https://contile-stage.topsites.nonprod.cloudops.mozgcp.net/> |
|prod| <https://contile.services.mozilla.com/> |

## Calls

Expand Down Expand Up @@ -40,26 +40,24 @@ Returns a JSON structure containing the tile information. For example:

```json
{"tiles":[
    {
        "id":74301,
        "name":"Amazon",
        "url":"https://...",
        "click_url":"https://...",
        "image_url":"https://...",
        "image_size":200,
        "impression_url":"...",
        "position":1
    },
    {
        "id":74161,
        "name":"eBay",
        "url":"https://...",
        "click_url":"https://...",
        "image_url":"https://...",
        "image_size":200,
        "impression_url":"https://...",
        "position":2
    }
{
"id":74301,
"name":"Amazon",
"url":"https://...",
"click_url":"https://...",
"image_url":"https://...",
"image_size":200,
"impression_url":"...",
},
{
"id":74161,
"name":"eBay",
"url":"https://...",
"click_url":"https://...",
"image_url":"https://...",
"image_size":200,
"impression_url":"https://...",
}
]}
```

Expand Down Expand Up @@ -91,4 +89,4 @@ In addition, the following extended Dockerflow endpoints are enabled:
GET /__error__
```

Force an error, used to test Sentry reporting. This has an optional parameter of `with_location=true` which will include detected IP location information in the Sentry error message.
Force an error, used to test Sentry reporting. This has an optional parameter of `with_location=true` which will include detected IP location information in the Sentry error message.
2 changes: 2 additions & 0 deletions integration-tests/docker-compose.init_error.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ version: "3"
services:
contile:
environment:
CONTILE_ADM_SUB1: sub1_test
CONTILE_ADM_PARTNER_ID: partner_id_test
CONTILE_ADM_SETTINGS: /tmp/contile/adm_settings_init_error.json
client:
environment:
Expand Down
7 changes: 5 additions & 2 deletions integration-tests/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ services:
environment:
PORT: 5000
RESPONSES_DIR: /tmp/partner/
ACCEPTED_MOBILE_FORM_FACTORS: 'desktop,phone,tablet'
ACCEPTED_DESKTOP_FORM_FACTORS: 'desktop,phone,tablet'
ACCEPTED_MOBILE_FORM_FACTORS: phone,tablet
ACCEPTED_DESKTOP_FORM_FACTORS: desktop
expose:
- "5000"
volumes:
Expand All @@ -26,8 +26,11 @@ services:
environment:
CONTILE_MAXMINDDB_LOC: /tmp/mmdb/GeoLite2-City-Test.mmdb
CONTILE_ADM_ENDPOINT_URL: http://partner:5000/tilesp/desktop
CONTILE_ADM_MOBILE_ENDPOINT_URL: http://partner:5000/tilesp/mobile
CONTILE_ADM_QUERY_TILE_COUNT: 5
CONTILE_ADM_SETTINGS: /tmp/contile/adm_settings.json
CONTILE_ADM_SUB1: sub1_test
CONTILE_ADM_PARTNER_ID: partner_id_test
CONTILE_ADM_HAS_LEGACY_IMAGE: '["Example ORG","Example COM"]'
# Timeout requests to the ADM server after this many seconds (default: 5)
CONTILE_ADM_TIMEOUT: 2
Expand Down
22 changes: 2 additions & 20 deletions integration-tests/volumes/client/scenarios.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,13 @@ scenarios:
image_size: null
impression_url: 'https://example.com/desktop_windows?id=0001'
url: 'https://www.example.com/desktop_windows'
position: 1
- id: 56789
name: 'Example ORG'
click_url: 'https://example.org/desktop_windows?version=16.0.0&key=7.2&ci=8.9&ctag=E1DE38C8972D0281F5556659A'
image_url: 'https://example.org/desktop_windows02.jpg'
image_size: null
impression_url: 'https://example.org/desktop_windows?id=0002'
url: 'https://www.example.org/desktop_windows'
position: 1
- request:
method: GET
path: '/v1/tiles'
Expand All @@ -49,15 +47,13 @@ scenarios:
image_size: null
impression_url: 'https://example.com/desktop_windows?id=0001'
url: 'https://www.example.com/desktop_windows'
position: 1
- id: 56789
name: 'Example ORG'
click_url: 'https://example.org/desktop_windows?version=16.0.0&key=7.2&ci=8.9&ctag=E1DE38C8972D0281F5556659A'
image_url: 'https://example.org/desktop_windows02.jpg'
image_size: null
impression_url: 'https://example.org/desktop_windows?id=0002'
url: 'https://www.example.org/desktop_windows'
position: 1

- name: success_desktop_macos
description: Test that Contile successfully returns tiles for macOS on Desktop.
Expand All @@ -81,15 +77,13 @@ scenarios:
image_size: null
impression_url: 'https://example.com/desktop_macos?id=0001'
url: 'https://www.example.com/desktop_macos'
position: 1
- id: 56790
name: 'Example ORG'
click_url: 'https://example.org/desktop_macos?version=16.0.0&key=7.2&ci=8.9&ctag=E1DE38C8972D0281F5556659A'
image_url: 'https://example.org/desktop_macos02.jpg'
image_size: null
impression_url: 'https://example.org/desktop_macos?id=0002'
url: 'https://www.example.org/desktop_macos'
position: 1

- name: success_desktop_linux
description: Test that Contile returns tiles for Linux on Desktop.
Expand All @@ -113,15 +107,13 @@ scenarios:
image_size: null
impression_url: 'https://example.com/desktop_linux?id=0001'
url: 'https://www.example.com/desktop_linux'
position: 1
- id: 56791
name: 'Example ORG'
click_url: 'https://example.org/desktop_linux?version=16.0.0&key=7.2&ci=8.9&ctag=E1DE38C8972D0281F5556659A'
image_url: 'https://example.org/desktop_linux02.jpg'
image_size: null
impression_url: 'https://example.org/desktop_linux?id=0002'
url: 'https://www.example.org/desktop_linux'
position: 1

- name: error_phone_android_reqwest_error
description: Test that Contile correctly handles a 500 from the partner API.
Expand Down Expand Up @@ -155,7 +147,7 @@ scenarios:
- name: User-Agent
value: 'iPad; CPU iPhone OS 11_5_1 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) FxiOS/35.0 Mobile/15E148 Safari/605.1.15'
response:
status_code: 204 # No Content
status_code: 204
content: ''

- name: error_phone_ios_timeout
Expand Down Expand Up @@ -186,7 +178,7 @@ scenarios:
# Contile maps the User-Agent Header value to os-family and form-factor parameters
# The following value will result in os-family: macos and form-factor: desktop
- name: User-Agent
value: 'Mozilla/5.0 (Macintosh; Intel Mac OS X 11_4) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/14.1 Safari/605.1.15'
value: 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:10.0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/97.0.4692.71 Safari/537.36'
response:
status_code: 403 # Forbidden
content:
Expand Down Expand Up @@ -222,15 +214,13 @@ scenarios:
image_size: null
impression_url: 'https://example.com/us_wa_desktop_macos?id=0001'
url: 'https://www.example.com/us_wa_desktop_macos'
position: 1
- id: 56790
name: 'Example ORG'
click_url: 'https://example.org/us_wa_desktop_macos?version=16.0.0&key=7.2&ci=8.9&ctag=E1DE38C8972D0281F5556659A'
image_url: 'https://example.org/us_wa_desktop_macos02.jpg'
image_size: null
impression_url: 'https://example.org/us_wa_desktop_macos?id=0002'
url: 'https://www.example.org/us_wa_desktop_macos'
position: 1

- name: legacy_image_filter
description: >
Expand Down Expand Up @@ -262,15 +252,13 @@ scenarios:
image_size: null
impression_url: 'https://dunbroch.co.uk/gb_desktop_macos?id=0001'
url: 'https://www.dunbroch.co.uk/gb_desktop_macos'
position: 2
- id: 32346
name: 'Example COM'
click_url: 'https://example.com/gb_desktop_macos?version=16.0.0&key=22.1&ci=6.2&ctag=1612376952400200000'
image_url: 'https://example.com/gb_desktop_macos01.jpg'
image_size: null
impression_url: 'https://example.com/gb_desktop_macos?id=0001'
url: 'https://www.example.com/gb_desktop_macos'
position: 1
- request:
method: GET
path: '/v1/tiles'
Expand All @@ -296,15 +284,13 @@ scenarios:
image_size: null
impression_url: 'https://example.com/gb_desktop_macos?id=0001'
url: 'https://www.example.com/gb_desktop_macos'
position: 1
- 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'
position: 1

- name: success_200_OK_exluded_region
description: Test that Contile returns a 200 OK with an empty tiles array for excluded regions
Expand Down Expand Up @@ -360,15 +346,13 @@ scenarios:
image_size: null
impression_url: 'https://example.com/gb_desktop_windows?id=0001'
url: 'https://www.example.com/gb_desktop_windows'
position: 1
- id: 76789
name: 'Example ORG'
click_url: 'https://example.org/gb_desktop_windows?version=16.0.0&key=7.2&ci=8.9&ctag=E1DE38C8972D0281F5556659A'
image_url: 'https://example.org/gb_desktop_windows02.jpg'
image_size: null
impression_url: 'https://example.org/gb_desktop_windows?id=0002'
url: 'https://www.example.org/gb_desktop_windows'
position: 1

- name: advertiser_url_path_filter_exact
description: >
Expand Down Expand Up @@ -403,12 +387,10 @@ scenarios:
image_size: null
impression_url: 'https://example.com/gb_desktop_linux?id=0001'
url: 'https://www.example.com/gb_desktop_linux'
position: 1
- id: 76791
name: 'Example ORG'
click_url: 'https://example.org/gb_desktop_linux?version=16.0.0&key=7.2&ci=8.9&ctag=E1DE38C8972D0281F5556659A'
image_url: 'https://example.org/gb_desktop_linux02.jpg'
image_size: null
impression_url: 'https://example.org/gb_desktop_linux?id=0002'
url: 'https://www.example.org/gb_desktop_linux'
position: 1
3 changes: 3 additions & 0 deletions integration-tests/volumes/client/scenarios_204.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
scenarios:
- name: success_204_No_Content_exluded_region
description: Test that Contile returns a 204 No Content for excluded regions
# This test uses the Environment Flag "CONTILE_EXCLUDED_COUNTRIES_200" (specified
# in the `docker-compose.204.yml` file) and checks that Contile returns a 204 with
# no content if a request is made from an excluded country location.
steps:
- request:
method: GET
Expand Down
1 change: 0 additions & 1 deletion integration-tests/volumes/client/scenarios_init_error.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,3 @@ scenarios:
image_size: null
impression_url: 'https://dunbroch.co.uk/gb_desktop_windows?id=0001'
url: 'https://www.dunbroch.co.uk/gb_desktop_windows'
position: 2
3 changes: 3 additions & 0 deletions integration-tests/volumes/partner/responses.yml
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,9 @@ phone:

tablet:
ios:
# While this is normally a valid type and the partner should return content for it, this test
# is about dealing with ADM returning invalid content. Contile should be able to handle this
# without returning a 500.
status_code: 200
headers: []
content: "hello world"
9 changes: 3 additions & 6 deletions src/adm/filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use url::Url;

use super::{
tiles::{AdmTile, Tile},
AdmAdvertiserFilterSettings, AdmSettings, DEFAULT,
AdmAdvertiserFilterSettings, AdmFilterSettings, DEFAULT,
};
use crate::{
adm::settings::PathMatching,
Expand Down Expand Up @@ -170,7 +170,7 @@ impl AdmFilter {
/// Try to update the ADM filter data from the remote bucket.
pub async fn update(&mut self) -> HandlerResult<()> {
if let Some(bucket) = &self.source_url {
let adm_settings = AdmSettings::from_settings_bucket(bucket)
let adm_settings = AdmFilterSettings::from_settings_bucket(bucket)
.await
.map_err(|e| {
HandlerError::internal(&format!(
Expand Down Expand Up @@ -436,10 +436,7 @@ impl AdmFilter {
// Use the default.position (Option<u8>) if the filter.position (Option<u8>) isn't
// defined. In either case `None` is a valid return, but we should favor `filter` over
// `default`.
Some(Tile::from_adm_tile(
tile,
filter.position.or(default.position),
))
Some(Tile::from_adm_tile(tile))
}
None => {
if !self.ignore_list.contains(&tile.name.to_lowercase()) {
Expand Down
2 changes: 1 addition & 1 deletion src/adm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,5 @@ mod settings;
mod tiles;

pub use filter::{spawn_updater, AdmFilter};
pub(crate) use settings::{AdmAdvertiserFilterSettings, AdmSettings, DEFAULT};
pub(crate) use settings::{AdmAdvertiserFilterSettings, AdmFilterSettings, AdmPse, DEFAULT};
pub use tiles::{get_tiles, TileResponse};
Loading

0 comments on commit 80d7dca

Please sign in to comment.