Skip to content

Commit 7ebfba3

Browse files
authored
Standardize Settings parameter position (#194)
* Standardize Settings parameter position Standardize Settings as first parameter across creative module functions for consistency with Rust conventions and improved code discoverability. Resolves: #188
1 parent aa10f27 commit 7ebfba3

7 files changed

Lines changed: 85 additions & 90 deletions

File tree

crates/common/README.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,9 @@ Additional behavior:
3333

3434
Helpers:
3535

36-
- `rewrite_creative_html(markup, settings) -> String` — rewrite an HTML fragment
37-
- `rewrite_css_body(css, settings) -> String` — rewrite a CSS body (`url(...)` entries)
38-
- `rewrite_srcset(srcset, settings) -> String` — proxy absolute candidates; preserve descriptors (`1x`, `1.5x`, `100w`)
36+
- `rewrite_creative_html(settings, markup) -> String` — rewrite an HTML fragment
37+
- `rewrite_css_body(settings, css) -> String` — rewrite a CSS body (`url(...)` entries)
38+
- `rewrite_srcset(settings, srcset) -> String` — proxy absolute candidates; preserve descriptors (`1x`, `1.5x`, `100w`)
3939
- `split_srcset_candidates(srcset) -> Vec<&str>` — robust splitting for commas with/without spaces; avoids splitting the first `data:` mediatype comma
4040

4141
Static bundles (served by publisher module):

crates/common/src/creative.rs

Lines changed: 73 additions & 73 deletions
Large diffs are not rendered by default.

crates/common/src/integrations/prebid.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ impl PrebidIntegration {
229229
Err(_) => String::from_utf8(body_bytes).unwrap_or_else(|_| "".to_string()),
230230
};
231231

232-
let rewritten = creative::rewrite_creative_html(&html, settings);
232+
let rewritten = creative::rewrite_creative_html(settings, &html);
233233

234234
Ok(Response::from_status(StatusCode::OK)
235235
.with_header(header::CONTENT_TYPE, "text/html; charset=utf-8")

crates/common/src/proxy.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -150,14 +150,14 @@ fn finalize_proxied_response(
150150
if ct.contains("text/html") {
151151
// HTML: rewrite and serve as HTML (safe to read as string)
152152
let body = beresp.take_body_str();
153-
let rewritten = crate::creative::rewrite_creative_html(&body, settings);
153+
let rewritten = crate::creative::rewrite_creative_html(settings, &body);
154154
return rebuild_text_response(beresp, "text/html; charset=utf-8", rewritten);
155155
}
156156

157157
if ct.contains("text/css") {
158158
// CSS: rewrite url(...) references in stylesheets (safe to read as string)
159159
let body = beresp.take_body_str();
160-
let rewritten = crate::creative::rewrite_css_body(&body, settings);
160+
let rewritten = crate::creative::rewrite_css_body(settings, &body);
161161
return rebuild_text_response(beresp, "text/css; charset=utf-8", rewritten);
162162
}
163163

@@ -668,7 +668,7 @@ pub async fn handle_first_party_proxy_sign(
668668
.unwrap_or_else(|| "https".to_string());
669669
format!("{}:{}", default_scheme, trimmed)
670670
} else {
671-
crate::creative::to_abs(trimmed, settings).ok_or_else(|| {
671+
crate::creative::to_abs(settings, trimmed).ok_or_else(|| {
672672
Report::new(TrustedServerError::Proxy {
673673
message: "unsupported url".to_string(),
674674
})
@@ -1410,7 +1410,7 @@ mod tests {
14101410
.unwrap_or("")
14111411
.to_string();
14121412
assert!(ct_pre.contains("text/html"), "ct_pre={}", ct_pre);
1413-
let direct = creative::rewrite_creative_html(html, &settings);
1413+
let direct = creative::rewrite_creative_html(&settings, html);
14141414
assert!(direct.contains("/first-party/proxy?tsurl="), "{}", direct);
14151415
let req = Request::new(Method::GET, "https://edge.example/first-party/proxy");
14161416
let out = finalize(&settings, &req, "https://cdn.example/a.png", beresp);

crates/common/src/publisher.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,10 +76,7 @@ fn detect_request_scheme(req: &Request) -> String {
7676

7777
/// Unified tsjs static serving: `/static/tsjs=<filename>`
7878
/// Accepts: `tsjs-core(.min).js`, `tsjs-ext(.min).js`, `tsjs-creative(.min).js`
79-
pub fn handle_tsjs_dynamic(
80-
_settings: &Settings,
81-
req: Request,
82-
) -> Result<Response, Report<TrustedServerError>> {
79+
pub fn handle_tsjs_dynamic(req: Request) -> Result<Response, Report<TrustedServerError>> {
8380
const PREFIX: &str = "/static/tsjs=";
8481
let path = req.get_path();
8582
if !path.starts_with(PREFIX) {

crates/fastly/src/main.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,7 @@ async fn route_request(
5959
// Match known routes and handle them
6060
let result = match (method, path.as_str()) {
6161
// Serve the tsjs library
62-
(Method::GET, path) if path.starts_with("/static/tsjs=") => {
63-
handle_tsjs_dynamic(&settings, req)
64-
}
62+
(Method::GET, path) if path.starts_with("/static/tsjs=") => handle_tsjs_dynamic(req),
6563

6664
// Discovery endpoint for trusted-server capabilities and JWKS
6765
(Method::GET, "/.well-known/trusted-server.json") => {

docs/guide/creative-processing.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -704,7 +704,7 @@ log::debug!("creative: skipped non-network scheme {}", url);
704704

705705
```rust
706706
let original = "<img src=\"https://tracker.com/pixel.gif\">";
707-
let rewritten = rewrite_creative_html(original, &settings);
707+
let rewritten = rewrite_creative_html(&settings, original);
708708
assert!(rewritten.contains("/first-party/proxy"));
709709
```
710710

@@ -713,7 +713,7 @@ assert!(rewritten.contains("/first-party/proxy"));
713713
#[test]
714714
fn test_image_src_rewrite() {
715715
let html = r#"<img src="https://cdn.example.com/banner.jpg">"#;
716-
let result = rewrite_creative_html(html, &test_settings());
716+
let result = rewrite_creative_html(&test_settings(), html);
717717
assert!(result.contains("/first-party/proxy?tsurl="));
718718
assert!(result.contains("&tstoken="));
719719
}

0 commit comments

Comments
 (0)