Skip to content

Commit

Permalink
Avoid boxing RequestTracker parameters (#9806)
Browse files Browse the repository at this point in the history
We only need to box these types when they are consumed.

This is related to https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rf-smart
and makes the APIs more flexible most of the time.

We can change back if these APIs will take ownership of the requests.

Co-authored-by: pyamada (Remote Dev Environment) <pyamada@atlassian.com>
  • Loading branch information
yamadapc and pyamada-atlassian committed Jun 18, 2024
1 parent 07dfb42 commit 747c3a3
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 12 deletions.
3 changes: 1 addition & 2 deletions crates/parcel/src/request_tracker/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,7 @@ impl<'a, T: Clone> RunRequestContext<'a, T> {
// TODO
}

// TODO: Why is this boxed?
pub fn run_request(&mut self, request: Box<&dyn Request<T>>) -> anyhow::Result<T> {
pub fn run_request(&mut self, request: &impl Request<T>) -> anyhow::Result<T> {
self
.request_tracker
.run_child_request(request, self.parent_request_hash)
Expand Down
4 changes: 2 additions & 2 deletions crates/parcel/src/request_tracker/request_tracker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,13 @@ impl<T: Clone> RequestTracker<T> {
}
}

pub fn run_request(&mut self, request: Box<&dyn Request<T>>) -> anyhow::Result<T> {
pub fn run_request(&mut self, request: &impl Request<T>) -> anyhow::Result<T> {
self.run_child_request(request, None)
}

pub fn run_child_request(
&mut self,
request: Box<&dyn Request<T>>,
request: &impl Request<T>,
parent_request_hash: Option<u64>,
) -> anyhow::Result<T> {
let request_id = request.id();
Expand Down
16 changes: 8 additions & 8 deletions crates/parcel/src/request_tracker/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ fn should_run_request() {
let request_b = TestRequest::new("B", &[&request_c]);
let request_a = TestRequest::new("A", &[&request_b]);

let result = rt.run_request(Box::new(&request_a)).unwrap();
let result = rt.run_request(&request_a).unwrap();

assert_eq!(result[0], "A");
assert_eq!(result[1], "B");
Expand All @@ -28,13 +28,13 @@ fn should_reuse_previously_run_request() {
let request_b = TestRequest::new("B", &[&request_c]);
let request_a = TestRequest::new("A", &[&request_b]);

let result = rt.run_request(Box::new(&request_a)).unwrap();
let result = rt.run_request(&request_a).unwrap();

assert_eq!(result[0], "A");
assert_eq!(result[1], "B");
assert_eq!(result[2], "C");

let result = rt.run_request(Box::new(&request_a)).unwrap();
let result = rt.run_request(&request_a).unwrap();
assert_eq!(result[0], "A");
assert_eq!(result[1], "B");
assert_eq!(result[2], "C");
Expand All @@ -46,12 +46,12 @@ fn should_run_request_once() {

let request_a = TestRequest::new("A", &[]);

let result = rt.run_request(Box::new(&request_a)).unwrap();
let result = rt.run_request(&request_a).unwrap();

assert_eq!(result[0], "A");
assert_eq!(request_a.run_count(), 1);

let result = rt.run_request(Box::new(&request_a)).unwrap();
let result = rt.run_request(&request_a).unwrap();
assert_eq!(result[0], "A");
assert_eq!(request_a.run_count(), 1);
}
Expand All @@ -63,14 +63,14 @@ fn should_run_request_once_2() {
let request_b = TestRequest::new("B", &[]);
let request_a = TestRequest::new("A", &[&request_b]);

let result = rt.run_request(Box::new(&request_a)).unwrap();
let result = rt.run_request(&request_a).unwrap();

assert_eq!(result[0], "A");
assert_eq!(result[1], "B");
assert_eq!(request_a.run_count(), 1);
assert_eq!(request_b.run_count(), 1);

let result = rt.run_request(Box::new(&request_a)).unwrap();
let result = rt.run_request(&request_a).unwrap();
assert_eq!(result[0], "A");
assert_eq!(result[1], "B");
assert_eq!(request_a.run_count(), 1);
Expand Down Expand Up @@ -127,7 +127,7 @@ impl<'a> Request<Vec<String>> for TestRequest<'a> {

while let Some(subrequest) = subrequets.pop() {
let req = subrequest.clone();
let subrequest_result = context.run_request(Box::new(&req))?;
let subrequest_result = context.run_request(&req)?;
result.extend(subrequest_result);
}

Expand Down

0 comments on commit 747c3a3

Please sign in to comment.