Skip to content

Commit

Permalink
Change PingUploader::upload to past a struct not a bunch of args
Browse files Browse the repository at this point in the history
And update some tests to check it out.
  • Loading branch information
chutten committed Feb 28, 2024
1 parent f079d38 commit 61f3b81
Show file tree
Hide file tree
Showing 8 changed files with 118 additions and 156 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* Update `glean_parser` to v13.0.0 ([release notes](https://github.com/mozilla/glean_parser/releases/tag/v13.0.0))
* Rust
* New metric type: Object ([#2489](https://github.com/mozilla/glean/pull/2489))
* BREAKING CHANGE: Support pings without `{client|ping}_info` sections ([#2756](https://github.com/mozilla/glean/pull/2756))
* Android
* Upgrade Android NDK to r26c ([#2745](https://github.com/mozilla/glean/pull/2745))

Expand Down
7 changes: 1 addition & 6 deletions glean-core/rlb/examples/long-running.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,7 @@ pub mod glean_metrics {
struct FakeUploader;

impl net::PingUploader for FakeUploader {
fn upload(
&self,
_url: String,
_body: Vec<u8>,
_headers: Vec<(String, String)>,
) -> net::UploadResult {
fn upload(&self, _upload_request: net::PingUploadRequest) -> net::UploadResult {
thread::sleep(time::Duration::from_millis(100));
net::UploadResult::http_status(200)
}
Expand Down
11 changes: 4 additions & 7 deletions glean-core/rlb/src/net/http_uploader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// License, v. 2.0. If a copy of the MPL was not distributed with this
// file, You can obtain one at https://mozilla.org/MPL/2.0/.

use crate::net::{PingUploader, UploadResult};
use crate::net::{PingUploadRequest, PingUploader, UploadResult};

/// A simple mechanism to upload pings over HTTPS.
#[derive(Debug)]
Expand All @@ -13,12 +13,9 @@ impl PingUploader for HttpUploader {
///
/// # Arguments
///
/// * `url` - the URL path to upload the data to.
/// * `body` - the serialized text data to send.
/// * `headers` - a vector of tuples containing the headers to send with
/// the request, i.e. (Name, Value).
fn upload(&self, url: String, _body: Vec<u8>, _headers: Vec<(String, String)>) -> UploadResult {
log::debug!("TODO bug 1675468: submitting to {:?}", url);
/// * `upload_request` - the requested upload.
fn upload(&self, upload_request: PingUploadRequest) -> UploadResult {
log::debug!("TODO bug 1675468: submitting to {:?}", upload_request.url);
UploadResult::http_status(200)
}
}
25 changes: 23 additions & 2 deletions glean-core/rlb/src/net/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,20 @@ use thread_state::{AtomicState, State};

mod http_uploader;

/// Everything you need to request a ping to be uploaded.
pub struct PingUploadRequest {
/// The URL the Glean SDK expects you to use to upload the ping.
pub url: String,
/// The body, already content-encoded, for upload.
pub body: Vec<u8>,
/// The HTTP headers, including any Content-Encoding.
pub headers: Vec<(String, String)>,
/// Whether the body has {client|ping}_info sections in it.
pub body_has_info_sections: bool,
/// The name (aka doctype) of the ping.
pub ping_name: String,
}

/// A description of a component used to upload pings.
pub trait PingUploader: std::fmt::Debug + Send + Sync {
/// Uploads a ping to a server.
Expand All @@ -29,7 +43,7 @@ pub trait PingUploader: std::fmt::Debug + Send + Sync {
/// * `body` - the serialized text data to send.
/// * `headers` - a vector of tuples containing the headers to send with
/// the request, i.e. (Name, Value).
fn upload(&self, url: String, body: Vec<u8>, headers: Vec<(String, String)>) -> UploadResult;
fn upload(&self, upload_request: PingUploadRequest) -> UploadResult;
}

/// The logic for uploading pings: this leaves the actual upload mechanism as
Expand Down Expand Up @@ -105,7 +119,14 @@ impl UploadManager {
let upload_url = format!("{}{}", inner.server_endpoint, request.path);
let headers: Vec<(String, String)> =
request.headers.into_iter().collect();
let result = inner.uploader.upload(upload_url, request.body, headers);
let upload_request = PingUploadRequest {
url: upload_url,
body: request.body,
headers,
body_has_info_sections: request.body_has_info_sections,
ping_name: request.ping_name,
};
let result = inner.uploader.upload(upload_request);
// Process the upload response.
match glean_core::glean_process_ping_upload_response(doc_id, result) {
UploadTaskAction::Next => (),
Expand Down
Loading

0 comments on commit 61f3b81

Please sign in to comment.