From 786500332341c8e39f7e3e82e493e567ecaa70e4 Mon Sep 17 00:00:00 2001 From: Tobias Krischer Date: Sun, 16 Oct 2022 19:46:15 +0200 Subject: Use status code 204 No Content for empty responses --- src/api/admin/api_server.rs | 6 ++-- src/api/admin/cluster.rs | 6 ++-- src/api/k2v/batch.rs | 2 +- src/api/k2v/item.rs | 2 +- src/garage/tests/k2v/batch.rs | 22 +++++++-------- src/garage/tests/k2v/errorcodes.rs | 20 ++++++------- src/garage/tests/k2v/item.rs | 58 +++++++++++++++++++------------------- src/garage/tests/k2v/poll.rs | 10 +++---- src/garage/tests/k2v/simple.rs | 6 ++-- src/garage/tests/s3/website.rs | 20 ++++++------- 10 files changed, 76 insertions(+), 76 deletions(-) (limited to 'src') diff --git a/src/api/admin/api_server.rs b/src/api/admin/api_server.rs index 0816bda1..2896d058 100644 --- a/src/api/admin/api_server.rs +++ b/src/api/admin/api_server.rs @@ -5,7 +5,7 @@ use async_trait::async_trait; use futures::future::Future; use http::header::{ACCESS_CONTROL_ALLOW_METHODS, ACCESS_CONTROL_ALLOW_ORIGIN, ALLOW}; -use hyper::{Body, Request, Response}; +use hyper::{Body, Request, Response, StatusCode}; use opentelemetry::trace::SpanRef; @@ -69,7 +69,7 @@ impl AdminApiServer { fn handle_options(&self, _req: &Request) -> Result, Error> { Ok(Response::builder() - .status(204) + .status(StatusCode::NO_CONTENT) .header(ALLOW, "OPTIONS, GET, POST") .header(ACCESS_CONTROL_ALLOW_METHODS, "OPTIONS, GET, POST") .header(ACCESS_CONTROL_ALLOW_ORIGIN, "*") @@ -94,7 +94,7 @@ impl AdminApiServer { .ok_or_internal_error("Could not serialize metrics")?; Ok(Response::builder() - .status(200) + .status(StatusCode::OK) .header(http::header::CONTENT_TYPE, encoder.format_type()) .body(Body::from(buffer))?) } diff --git a/src/api/admin/cluster.rs b/src/api/admin/cluster.rs index 99c6e332..706db727 100644 --- a/src/api/admin/cluster.rs +++ b/src/api/admin/cluster.rs @@ -151,7 +151,7 @@ pub async fn handle_update_cluster_layout( garage.system.update_cluster_layout(&layout).await?; Ok(Response::builder() - .status(StatusCode::OK) + .status(StatusCode::NO_CONTENT) .body(Body::empty())?) } @@ -166,7 +166,7 @@ pub async fn handle_apply_cluster_layout( garage.system.update_cluster_layout(&layout).await?; Ok(Response::builder() - .status(StatusCode::OK) + .status(StatusCode::NO_CONTENT) .body(Body::empty())?) } @@ -181,7 +181,7 @@ pub async fn handle_revert_cluster_layout( garage.system.update_cluster_layout(&layout).await?; Ok(Response::builder() - .status(StatusCode::OK) + .status(StatusCode::NO_CONTENT) .body(Body::empty())?) } diff --git a/src/api/k2v/batch.rs b/src/api/k2v/batch.rs index db9901cf..78035362 100644 --- a/src/api/k2v/batch.rs +++ b/src/api/k2v/batch.rs @@ -42,7 +42,7 @@ pub async fn handle_insert_batch( garage.k2v.rpc.insert_batch(bucket_id, items2).await?; Ok(Response::builder() - .status(StatusCode::OK) + .status(StatusCode::NO_CONTENT) .body(Body::empty())?) } diff --git a/src/api/k2v/item.rs b/src/api/k2v/item.rs index 836d386f..f85138c7 100644 --- a/src/api/k2v/item.rs +++ b/src/api/k2v/item.rs @@ -153,7 +153,7 @@ pub async fn handle_insert_item( .await?; Ok(Response::builder() - .status(StatusCode::OK) + .status(StatusCode::NO_CONTENT) .body(Body::empty())?) } diff --git a/src/garage/tests/k2v/batch.rs b/src/garage/tests/k2v/batch.rs index acae1910..6abba1c5 100644 --- a/src/garage/tests/k2v/batch.rs +++ b/src/garage/tests/k2v/batch.rs @@ -6,7 +6,7 @@ use assert_json_diff::assert_json_eq; use serde_json::json; use super::json_body; -use hyper::Method; +use hyper::{Method, StatusCode}; #[tokio::test] async fn test_batch() { @@ -49,7 +49,7 @@ async fn test_batch() { .send() .await .unwrap(); - assert_eq!(res.status(), 200); + assert_eq!(res.status(), StatusCode::NO_CONTENT); for sk in ["a", "b", "c", "d.1", "d.2", "e"] { let res = ctx @@ -62,7 +62,7 @@ async fn test_batch() { .send() .await .unwrap(); - assert_eq!(res.status(), 200); + assert_eq!(res.status(), StatusCode::OK); assert_eq!( res.headers().get("content-type").unwrap().to_str().unwrap(), "application/octet-stream" @@ -104,7 +104,7 @@ async fn test_batch() { .send() .await .unwrap(); - assert_eq!(res.status(), 200); + assert_eq!(res.status(), StatusCode::OK); let json_res = json_body(res).await; assert_json_eq!( json_res, @@ -266,7 +266,7 @@ async fn test_batch() { .send() .await .unwrap(); - assert_eq!(res.status(), 200); + assert_eq!(res.status(), StatusCode::NO_CONTENT); for sk in ["b", "c", "d.1", "d.2"] { let res = ctx @@ -280,9 +280,9 @@ async fn test_batch() { .await .unwrap(); if sk == "b" { - assert_eq!(res.status(), 204); + assert_eq!(res.status(), StatusCode::NO_CONTENT); } else { - assert_eq!(res.status(), 200); + assert_eq!(res.status(), StatusCode::OK); } ct.insert( sk, @@ -317,7 +317,7 @@ async fn test_batch() { .send() .await .unwrap(); - assert_eq!(res.status(), 200); + assert_eq!(res.status(), StatusCode::OK); let json_res = json_body(res).await; assert_json_eq!( json_res, @@ -478,7 +478,7 @@ async fn test_batch() { .send() .await .unwrap(); - assert_eq!(res.status(), 200); + assert_eq!(res.status(), StatusCode::OK); let json_res = json_body(res).await; assert_json_eq!( json_res, @@ -514,7 +514,7 @@ async fn test_batch() { .send() .await .unwrap(); - assert_eq!(res.status(), 204); + assert_eq!(res.status(), StatusCode::NO_CONTENT); assert_eq!( res.headers().get("content-type").unwrap().to_str().unwrap(), "application/octet-stream" @@ -547,7 +547,7 @@ async fn test_batch() { .send() .await .unwrap(); - assert_eq!(res.status(), 200); + assert_eq!(res.status(), StatusCode::OK); let json_res = json_body(res).await; assert_json_eq!( json_res, diff --git a/src/garage/tests/k2v/errorcodes.rs b/src/garage/tests/k2v/errorcodes.rs index 2fcc45bc..ecb84729 100644 --- a/src/garage/tests/k2v/errorcodes.rs +++ b/src/garage/tests/k2v/errorcodes.rs @@ -1,13 +1,13 @@ use crate::common; -use hyper::Method; +use hyper::{Method, StatusCode}; #[tokio::test] async fn test_error_codes() { let ctx = common::context(); let bucket = ctx.create_bucket("test-k2v-error-codes"); - // Regular insert should work (code 200) + // Regular insert should work (code 204) let res = ctx .k2v .request @@ -19,7 +19,7 @@ async fn test_error_codes() { .send() .await .unwrap(); - assert_eq!(res.status(), 200); + assert_eq!(res.status(), StatusCode::NO_CONTENT); // Insert with trash causality token: invalid request let res = ctx @@ -34,7 +34,7 @@ async fn test_error_codes() { .send() .await .unwrap(); - assert_eq!(res.status(), 400); + assert_eq!(res.status(), StatusCode::BAD_REQUEST); // Search without partition key: invalid request let res = ctx @@ -52,7 +52,7 @@ async fn test_error_codes() { .send() .await .unwrap(); - assert_eq!(res.status(), 400); + assert_eq!(res.status(), StatusCode::BAD_REQUEST); // Search with start that is not in prefix: invalid request let res = ctx @@ -70,7 +70,7 @@ async fn test_error_codes() { .send() .await .unwrap(); - assert_eq!(res.status(), 400); + assert_eq!(res.status(), StatusCode::BAD_REQUEST); // Search with invalid json: 400 let res = ctx @@ -88,7 +88,7 @@ async fn test_error_codes() { .send() .await .unwrap(); - assert_eq!(res.status(), 400); + assert_eq!(res.status(), StatusCode::BAD_REQUEST); // Batch insert with invalid causality token: 400 let res = ctx @@ -105,7 +105,7 @@ async fn test_error_codes() { .send() .await .unwrap(); - assert_eq!(res.status(), 400); + assert_eq!(res.status(), StatusCode::BAD_REQUEST); // Batch insert with invalid data: 400 let res = ctx @@ -122,7 +122,7 @@ async fn test_error_codes() { .send() .await .unwrap(); - assert_eq!(res.status(), 400); + assert_eq!(res.status(), StatusCode::BAD_REQUEST); // Poll with invalid causality token: 400 let res = ctx @@ -137,5 +137,5 @@ async fn test_error_codes() { .send() .await .unwrap(); - assert_eq!(res.status(), 400); + assert_eq!(res.status(), StatusCode::BAD_REQUEST); } diff --git a/src/garage/tests/k2v/item.rs b/src/garage/tests/k2v/item.rs index 32537336..2641386f 100644 --- a/src/garage/tests/k2v/item.rs +++ b/src/garage/tests/k2v/item.rs @@ -6,7 +6,7 @@ use assert_json_diff::assert_json_eq; use serde_json::json; use super::json_body; -use hyper::Method; +use hyper::{Method, StatusCode}; #[tokio::test] async fn test_items_and_indices() { @@ -56,7 +56,7 @@ async fn test_items_and_indices() { .send() .await .unwrap(); - assert_eq!(res.status(), 200); + assert_eq!(res.status(), StatusCode::NO_CONTENT); // Get value back let res = ctx @@ -69,7 +69,7 @@ async fn test_items_and_indices() { .send() .await .unwrap(); - assert_eq!(res.status(), 200); + assert_eq!(res.status(), StatusCode::OK); assert_eq!( res.headers().get("content-type").unwrap().to_str().unwrap(), "application/octet-stream" @@ -132,7 +132,7 @@ async fn test_items_and_indices() { .send() .await .unwrap(); - assert_eq!(res.status(), 200); + assert_eq!(res.status(), StatusCode::NO_CONTENT); // Get value back let res = ctx @@ -145,7 +145,7 @@ async fn test_items_and_indices() { .send() .await .unwrap(); - assert_eq!(res.status(), 200); + assert_eq!(res.status(), StatusCode::OK); assert_eq!( res.headers().get("content-type").unwrap().to_str().unwrap(), "application/octet-stream" @@ -201,7 +201,7 @@ async fn test_items_and_indices() { .send() .await .unwrap(); - assert_eq!(res.status(), 200); + assert_eq!(res.status(), StatusCode::NO_CONTENT); // Get value back let res = ctx @@ -214,7 +214,7 @@ async fn test_items_and_indices() { .send() .await .unwrap(); - assert_eq!(res.status(), 200); + assert_eq!(res.status(), StatusCode::OK); assert_eq!( res.headers().get("content-type").unwrap().to_str().unwrap(), "application/json" @@ -271,7 +271,7 @@ async fn test_items_and_indices() { .send() .await .unwrap(); - assert_eq!(res.status(), 200); + assert_eq!(res.status(), StatusCode::OK); let ct = res .headers() .get("x-garage-causality-token") @@ -292,7 +292,7 @@ async fn test_items_and_indices() { .send() .await .unwrap(); - assert_eq!(res.status(), 204); + assert_eq!(res.status(), StatusCode::NO_CONTENT); // ReadIndex -- now there should be some stuff tokio::time::sleep(Duration::from_secs(1)).await; @@ -364,7 +364,7 @@ async fn test_item_return_format() { .send() .await .unwrap(); - assert_eq!(res.status(), 200); + assert_eq!(res.status(), StatusCode::NO_CONTENT); // f0: either let res = ctx @@ -377,7 +377,7 @@ async fn test_item_return_format() { .send() .await .unwrap(); - assert_eq!(res.status(), 200); + assert_eq!(res.status(), StatusCode::OK); assert_eq!( res.headers().get("content-type").unwrap().to_str().unwrap(), "application/octet-stream" @@ -405,7 +405,7 @@ async fn test_item_return_format() { .send() .await .unwrap(); - assert_eq!(res.status(), 200); + assert_eq!(res.status(), StatusCode::OK); assert_eq!( res.headers().get("content-type").unwrap().to_str().unwrap(), "application/json" @@ -424,7 +424,7 @@ async fn test_item_return_format() { .send() .await .unwrap(); - assert_eq!(res.status(), 200); + assert_eq!(res.status(), StatusCode::OK); assert_eq!( res.headers().get("content-type").unwrap().to_str().unwrap(), "application/octet-stream" @@ -446,7 +446,7 @@ async fn test_item_return_format() { .send() .await .unwrap(); - assert_eq!(res.status(), 200); + assert_eq!(res.status(), StatusCode::OK); assert_eq!( res.headers().get("content-type").unwrap().to_str().unwrap(), "application/json" @@ -466,7 +466,7 @@ async fn test_item_return_format() { .send() .await .unwrap(); - assert_eq!(res.status(), 200); + assert_eq!(res.status(), StatusCode::NO_CONTENT); // f0: either let res = ctx @@ -479,7 +479,7 @@ async fn test_item_return_format() { .send() .await .unwrap(); - assert_eq!(res.status(), 200); + assert_eq!(res.status(), StatusCode::OK); assert_eq!( res.headers().get("content-type").unwrap().to_str().unwrap(), "application/json" @@ -503,7 +503,7 @@ async fn test_item_return_format() { .send() .await .unwrap(); - assert_eq!(res.status(), 200); + assert_eq!(res.status(), StatusCode::OK); assert_eq!( res.headers().get("content-type").unwrap().to_str().unwrap(), "application/json" @@ -528,7 +528,7 @@ async fn test_item_return_format() { .send() .await .unwrap(); - assert_eq!(res.status(), 409); // CONFLICT + assert_eq!(res.status(), StatusCode::CONFLICT); // CONFLICT // f3: json let res = ctx @@ -541,7 +541,7 @@ async fn test_item_return_format() { .send() .await .unwrap(); - assert_eq!(res.status(), 200); + assert_eq!(res.status(), StatusCode::OK); assert_eq!( res.headers().get("content-type").unwrap().to_str().unwrap(), "application/json" @@ -568,7 +568,7 @@ async fn test_item_return_format() { .send() .await .unwrap(); - assert_eq!(res.status(), 204); + assert_eq!(res.status(), StatusCode::NO_CONTENT); // f0: either let res = ctx @@ -581,7 +581,7 @@ async fn test_item_return_format() { .send() .await .unwrap(); - assert_eq!(res.status(), 200); + assert_eq!(res.status(), StatusCode::OK); assert_eq!( res.headers().get("content-type").unwrap().to_str().unwrap(), "application/json" @@ -599,7 +599,7 @@ async fn test_item_return_format() { .send() .await .unwrap(); - assert_eq!(res.status(), 200); + assert_eq!(res.status(), StatusCode::OK); assert_eq!( res.headers().get("content-type").unwrap().to_str().unwrap(), "application/json" @@ -625,7 +625,7 @@ async fn test_item_return_format() { .send() .await .unwrap(); - assert_eq!(res.status(), 409); // CONFLICT + assert_eq!(res.status(), StatusCode::CONFLICT); // CONFLICT // f3: json let res = ctx @@ -638,7 +638,7 @@ async fn test_item_return_format() { .send() .await .unwrap(); - assert_eq!(res.status(), 200); + assert_eq!(res.status(), StatusCode::OK); assert_eq!( res.headers().get("content-type").unwrap().to_str().unwrap(), "application/json" @@ -658,7 +658,7 @@ async fn test_item_return_format() { .send() .await .unwrap(); - assert_eq!(res.status(), 204); + assert_eq!(res.status(), StatusCode::NO_CONTENT); // f0: either let res = ctx @@ -671,7 +671,7 @@ async fn test_item_return_format() { .send() .await .unwrap(); - assert_eq!(res.status(), 204); // NO CONTENT + assert_eq!(res.status(), StatusCode::NO_CONTENT); // NO CONTENT // f1: not specified let res = ctx @@ -683,7 +683,7 @@ async fn test_item_return_format() { .send() .await .unwrap(); - assert_eq!(res.status(), 200); + assert_eq!(res.status(), StatusCode::OK); assert_eq!( res.headers().get("content-type").unwrap().to_str().unwrap(), "application/json" @@ -702,7 +702,7 @@ async fn test_item_return_format() { .send() .await .unwrap(); - assert_eq!(res.status(), 204); // NO CONTENT + assert_eq!(res.status(), StatusCode::NO_CONTENT); // NO CONTENT // f3: json let res = ctx @@ -715,7 +715,7 @@ async fn test_item_return_format() { .send() .await .unwrap(); - assert_eq!(res.status(), 200); + assert_eq!(res.status(), StatusCode::OK); assert_eq!( res.headers().get("content-type").unwrap().to_str().unwrap(), "application/json" diff --git a/src/garage/tests/k2v/poll.rs b/src/garage/tests/k2v/poll.rs index 70dc0410..e56705ae 100644 --- a/src/garage/tests/k2v/poll.rs +++ b/src/garage/tests/k2v/poll.rs @@ -1,4 +1,4 @@ -use hyper::Method; +use hyper::{Method, StatusCode}; use std::time::Duration; use crate::common; @@ -20,7 +20,7 @@ async fn test_poll() { .send() .await .unwrap(); - assert_eq!(res.status(), 200); + assert_eq!(res.status(), StatusCode::NO_CONTENT); // Retrieve initial value to get its causality token let res2 = ctx @@ -33,7 +33,7 @@ async fn test_poll() { .send() .await .unwrap(); - assert_eq!(res2.status(), 200); + assert_eq!(res2.status(), StatusCode::OK); let ct = res2 .headers() .get("x-garage-causality-token") @@ -80,7 +80,7 @@ async fn test_poll() { .send() .await .unwrap(); - assert_eq!(res.status(), 200); + assert_eq!(res.status(), StatusCode::NO_CONTENT); // Check poll finishes with correct value let poll_res = tokio::select! { @@ -88,7 +88,7 @@ async fn test_poll() { res = poll => res.unwrap().unwrap(), }; - assert_eq!(poll_res.status(), 200); + assert_eq!(poll_res.status(), StatusCode::OK); let poll_res_body = hyper::body::to_bytes(poll_res.into_body()) .await diff --git a/src/garage/tests/k2v/simple.rs b/src/garage/tests/k2v/simple.rs index ae9a8674..465fc24d 100644 --- a/src/garage/tests/k2v/simple.rs +++ b/src/garage/tests/k2v/simple.rs @@ -1,6 +1,6 @@ use crate::common; -use hyper::Method; +use hyper::{Method, StatusCode}; #[tokio::test] async fn test_simple() { @@ -18,7 +18,7 @@ async fn test_simple() { .send() .await .unwrap(); - assert_eq!(res.status(), 200); + assert_eq!(res.status(), StatusCode::NO_CONTENT); let res2 = ctx .k2v @@ -30,7 +30,7 @@ async fn test_simple() { .send() .await .unwrap(); - assert_eq!(res2.status(), 200); + assert_eq!(res2.status(), StatusCode::OK); let res2_body = hyper::body::to_bytes(res2.into_body()) .await diff --git a/src/garage/tests/s3/website.rs b/src/garage/tests/s3/website.rs index 0570ac6a..244a2fa0 100644 --- a/src/garage/tests/s3/website.rs +++ b/src/garage/tests/s3/website.rs @@ -4,7 +4,7 @@ use aws_sdk_s3::{ model::{CorsConfiguration, CorsRule, ErrorDocument, IndexDocument, WebsiteConfiguration}, types::ByteStream, }; -use http::Request; +use http::{Request, StatusCode}; use hyper::{ body::{to_bytes, Body}, Client, @@ -43,7 +43,7 @@ async fn test_website() { let mut resp = client.request(req()).await.unwrap(); - assert_eq!(resp.status(), 404); + assert_eq!(resp.status(), StatusCode::NOT_FOUND); assert_ne!( to_bytes(resp.body_mut()).await.unwrap().as_ref(), BODY.as_ref() @@ -56,7 +56,7 @@ async fn test_website() { .expect_success_status("Could not allow website on bucket"); resp = client.request(req()).await.unwrap(); - assert_eq!(resp.status(), 200); + assert_eq!(resp.status(), StatusCode::OK); assert_eq!( to_bytes(resp.body_mut()).await.unwrap().as_ref(), BODY.as_ref() @@ -69,7 +69,7 @@ async fn test_website() { .expect_success_status("Could not deny website on bucket"); resp = client.request(req()).await.unwrap(); - assert_eq!(resp.status(), 404); + assert_eq!(resp.status(), StatusCode::NOT_FOUND); assert_ne!( to_bytes(resp.body_mut()).await.unwrap().as_ref(), BODY.as_ref() @@ -175,7 +175,7 @@ async fn test_website_s3_api() { let mut resp = client.request(req).await.unwrap(); - assert_eq!(resp.status(), 200); + assert_eq!(resp.status(), StatusCode::OK); assert_eq!( resp.headers().get("access-control-allow-origin").unwrap(), "*" @@ -200,7 +200,7 @@ async fn test_website_s3_api() { let mut resp = client.request(req).await.unwrap(); - assert_eq!(resp.status(), 404); + assert_eq!(resp.status(), StatusCode::NOT_FOUND); assert_eq!( to_bytes(resp.body_mut()).await.unwrap().as_ref(), BODY_ERR.as_ref() @@ -220,7 +220,7 @@ async fn test_website_s3_api() { let mut resp = client.request(req).await.unwrap(); - assert_eq!(resp.status(), 200); + assert_eq!(resp.status(), StatusCode::OK); assert_eq!( resp.headers().get("access-control-allow-origin").unwrap(), "*" @@ -244,7 +244,7 @@ async fn test_website_s3_api() { let mut resp = client.request(req).await.unwrap(); - assert_eq!(resp.status(), 403); + assert_eq!(resp.status(), StatusCode::FORBIDDEN); assert_ne!( to_bytes(resp.body_mut()).await.unwrap().as_ref(), BODY.as_ref() @@ -285,7 +285,7 @@ async fn test_website_s3_api() { let mut resp = client.request(req).await.unwrap(); - assert_eq!(resp.status(), 403); + assert_eq!(resp.status(), StatusCode::FORBIDDEN); assert_ne!( to_bytes(resp.body_mut()).await.unwrap().as_ref(), BODY.as_ref() @@ -311,7 +311,7 @@ async fn test_website_s3_api() { let mut resp = client.request(req).await.unwrap(); - assert_eq!(resp.status(), 404); + assert_eq!(resp.status(), StatusCode::NOT_FOUND); assert_ne!( to_bytes(resp.body_mut()).await.unwrap().as_ref(), BODY_ERR.as_ref() -- cgit v1.2.3