Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update HTTP verbs for DAP-13. #3469

Merged
merged 3 commits into from
Nov 13, 2024
Merged

Update HTTP verbs for DAP-13. #3469

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dccd082
Update HTTP verbs for DAP-13.
branlwyd Nov 8, 2024
8dc2e51
Disable divviup-ts tests.
branlwyd Nov 12, 2024
1ee6984
PR review.
branlwyd Nov 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 13 additions & 17 deletions aggregator/src/aggregator/collection_job_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ use std::{collections::HashSet, sync::Arc};
use trillium::{Handler, KnownHeaderName, Status};
use trillium_testing::{
assert_headers,
prelude::{post, put},
prelude::{get, put},
TestConn,
};

Expand Down Expand Up @@ -95,29 +95,25 @@ impl CollectionJobTestCase {
.await
}

pub(super) async fn post_collection_job_with_auth_token(
pub(super) async fn get_collection_job_with_auth_token(
&self,
collection_job_id: &CollectionJobId,
auth_token: Option<&AuthenticationToken>,
) -> TestConn {
let mut test_conn = post(
self.task
.collection_job_uri(collection_job_id)
.unwrap()
.path(),
);
let mut test_conn = get(self
.task
.collection_job_uri(collection_job_id)
.unwrap()
.path());
if let Some(auth) = auth_token {
let (header, value) = auth.request_authentication();
test_conn = test_conn.with_request_header(header, value);
}
test_conn.run_async(&self.handler).await
}

pub(super) async fn post_collection_job(
&self,
collection_job_id: &CollectionJobId,
) -> TestConn {
self.post_collection_job_with_auth_token(
pub(super) async fn get_collection_job(&self, collection_job_id: &CollectionJobId) -> TestConn {
self.get_collection_job_with_auth_token(
collection_job_id,
Some(self.task.collector_auth_token()),
)
Expand Down Expand Up @@ -343,7 +339,7 @@ async fn collection_job_success_fixed_size() {
.await;
assert_eq!(test_conn.status(), Some(Status::Created));

let test_conn = test_case.post_collection_job(&collection_job_id).await;
let test_conn = test_case.get_collection_job(&collection_job_id).await;
assert_eq!(test_conn.status(), Some(Status::Accepted));

// Update the collection job with the aggregate shares. collection job should now be complete.
Expand Down Expand Up @@ -408,7 +404,7 @@ async fn collection_job_success_fixed_size() {
panic!("unexpected batch ID");
}

let mut test_conn = test_case.post_collection_job(&collection_job_id).await;
let mut test_conn = test_case.get_collection_job(&collection_job_id).await;
assert_headers!(&test_conn, "content-type" => (Collection::<FixedSize>::MEDIA_TYPE));

let collect_resp: Collection<FixedSize> = decode_response_body(&mut test_conn).await;
Expand Down Expand Up @@ -817,7 +813,7 @@ async fn collection_job_put_idempotence_fixed_size_current_batch_no_extra_report
assert_eq!(response.status(), Some(Status::Created));

// Fetch the first collection job, to advance the current batch.
let response = test_case.post_collection_job(&collection_job_id_1).await;
let response = test_case.get_collection_job(&collection_job_id_1).await;
assert_eq!(response.status(), Some(Status::Accepted));

// Create the second collection job.
Expand All @@ -828,7 +824,7 @@ async fn collection_job_put_idempotence_fixed_size_current_batch_no_extra_report

// Fetch the second collection job, to advance the current batch. There are now no outstanding
// batches left.
let response = test_case.post_collection_job(&collection_job_id_2).await;
let response = test_case.get_collection_job(&collection_job_id_2).await;
assert_eq!(response.status(), Some(Status::Accepted));

// Re-send the collection job creation requests to confirm they are still idempotent.
Expand Down
16 changes: 8 additions & 8 deletions aggregator/src/aggregator/http_handlers.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ where
"hpke_config",
hpke_config_cors_preflight,
)
.put("tasks/:task_id/reports", instrumented(api(upload::<C>)))
.post("tasks/:task_id/reports", instrumented(api(upload::<C>)))
.with_route(
trillium::Method::Options,
"tasks/:task_id/reports",
Expand Down Expand Up @@ -397,9 +397,9 @@ where
COLLECTION_JOB_ROUTE,
instrumented(api(collection_jobs_put::<C>)),
)
.post(
.get(
COLLECTION_JOB_ROUTE,
instrumented(api(collection_jobs_post::<C>)),
instrumented(api(collection_jobs_get::<C>)),
)
.delete(
COLLECTION_JOB_ROUTE,
Expand Down Expand Up @@ -491,7 +491,7 @@ async fn hpke_config_cors_preflight(mut conn: Conn) -> Conn {
conn
}

/// API handler for the "/tasks/.../reports" PUT endpoint.
/// API handler for the "/tasks/.../reports" POST endpoint.
async fn upload<C: Clock>(
conn: &mut Conn,
(State(aggregator), BodyBytes(body)): (State<Arc<Aggregator<C>>>, BodyBytes),
Expand All @@ -517,12 +517,12 @@ async fn upload<C: Clock>(
/// Handler for CORS preflight requests to "/tasks/.../reports".
async fn upload_cors_preflight(mut conn: Conn) -> Conn {
conn.response_headers_mut()
.insert(KnownHeaderName::Allow, "PUT");
.insert(KnownHeaderName::Allow, "POST");
if let Some(origin) = conn.request_headers().get(KnownHeaderName::Origin) {
let origin = origin.clone();
let request_headers = conn.response_headers_mut();
request_headers.insert(KnownHeaderName::AccessControlAllowOrigin, origin);
request_headers.insert(KnownHeaderName::AccessControlAllowMethods, "PUT");
request_headers.insert(KnownHeaderName::AccessControlAllowMethods, "POST");
request_headers.insert(KnownHeaderName::AccessControlAllowHeaders, "content-type");
request_headers.insert(
KnownHeaderName::AccessControlMaxAge,
Expand Down Expand Up @@ -629,8 +629,8 @@ async fn collection_jobs_put<C: Clock>(
Ok(Status::Created)
}

/// API handler for the "/tasks/.../collection_jobs/..." POST endpoint.
async fn collection_jobs_post<C: Clock>(
/// API handler for the "/tasks/.../collection_jobs/..." GET endpoint.
async fn collection_jobs_get<C: Clock>(
conn: &mut Conn,
State(aggregator): State<Arc<Aggregator<C>>>,
) -> Result<(), Error> {
Expand Down
20 changes: 10 additions & 10 deletions aggregator/src/aggregator/http_handlers/tests/collection_job.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use serde_json::json;
use trillium::{KnownHeaderName, Status};
use trillium_testing::{
assert_headers,
prelude::{delete, post, put},
prelude::{delete, get, put},
};

#[tokio::test]
Expand Down Expand Up @@ -262,7 +262,7 @@ async fn collection_job_put_request_unauthenticated() {
}

#[tokio::test]
async fn collection_job_post_request_unauthenticated_collection_jobs() {
async fn collection_job_get_request_unauthenticated_collection_jobs() {
let test_case = setup_collection_job_test_case(Role::Leader, QueryType::TimeInterval).await;
test_case
.setup_time_interval_batch(Time::from_seconds_since_epoch(0))
Expand All @@ -288,7 +288,7 @@ async fn collection_job_post_request_unauthenticated_collection_jobs() {

// Incorrect authentication token.
let mut test_conn = test_case
.post_collection_job_with_auth_token(&collection_job_id, Some(&random()))
branlwyd marked this conversation as resolved.
Show resolved Hide resolved
.get_collection_job_with_auth_token(&collection_job_id, Some(&random()))
.await;

let want_status = u16::from(Status::Forbidden);
Expand All @@ -305,7 +305,7 @@ async fn collection_job_post_request_unauthenticated_collection_jobs() {

// Aggregator authentication token.
let mut test_conn = test_case
.post_collection_job_with_auth_token(
.get_collection_job_with_auth_token(
&collection_job_id,
Some(test_case.task.aggregator_auth_token()),
)
Expand All @@ -325,7 +325,7 @@ async fn collection_job_post_request_unauthenticated_collection_jobs() {

// Missing authentication token.
let mut test_conn = test_case
.post_collection_job_with_auth_token(&collection_job_id, None)
.get_collection_job_with_auth_token(&collection_job_id, None)
.await;

let want_status = u16::from(Status::Forbidden);
Expand Down Expand Up @@ -398,7 +398,7 @@ async fn collection_job_success_time_interval() {

assert_eq!(test_conn.status(), Some(Status::Created));

let test_conn = test_case.post_collection_job(&collection_job_id).await;
let test_conn = test_case.get_collection_job(&collection_job_id).await;
assert_eq!(test_conn.status(), Some(Status::Accepted));

// Update the collection job with the aggregate shares and some aggregation jobs. collection
Expand Down Expand Up @@ -452,7 +452,7 @@ async fn collection_job_success_time_interval() {
.await
.unwrap();

let mut test_conn = test_case.post_collection_job(&collection_job_id).await;
let mut test_conn = test_case.get_collection_job(&collection_job_id).await;

assert_eq!(test_conn.status(), Some(Status::Ok));
assert_headers!(
Expand Down Expand Up @@ -502,7 +502,7 @@ async fn collection_job_success_time_interval() {
}

#[tokio::test]
async fn collection_job_post_request_no_such_collection_job() {
async fn collection_job_get_request_no_such_collection_job() {
let test_case = setup_collection_job_test_case(Role::Leader, QueryType::TimeInterval).await;
test_case
.setup_time_interval_batch(Time::from_seconds_since_epoch(0))
Expand All @@ -514,7 +514,7 @@ async fn collection_job_post_request_no_such_collection_job() {
.task
.collector_auth_token()
.request_authentication();
let test_conn = post(format!(
let test_conn = get(format!(
"/tasks/{}/collection_jobs/{no_such_collection_job_id}",
test_case.task.id()
))
Expand Down Expand Up @@ -659,6 +659,6 @@ async fn delete_collection_job() {
assert_eq!(test_conn.status(), Some(Status::NoContent));

// Get the job again
let test_conn = test_case.post_collection_job(&collection_job_id).await;
let test_conn = test_case.get_collection_job(&collection_job_id).await;
assert_eq!(test_conn.status(), Some(Status::NoContent));
}
Loading
Loading