Skip to content

Commit

Permalink
Merge #425
Browse files Browse the repository at this point in the history
425: Build-rs r=jdroenner a=ChristianBeilschmidt

I did the following things:

1. Removed `anyhow` from our build-dependencies and just unwrapped the result in the `build.rs` file.
2. Clippy wanted to add #[must_use] labels to functions that take `&self` and return `Self`. This makes sense since not using would mean you did something wrong.
3. The arrow example was not working anymore 🤷. I changed the pointer cast. Fortunately, it seems that our in-code conversions still work.
4. I updated `tokio` and could remove some Clippy lint exclusions (`// TODO: remove when tokio-rs/tokio#4245 is fixed`).
5. I updated Actix-web and they removed `AnyBody`. This means I had to wrap our error handler differently. You now have to put it into an EitherBody-enum and this takes a BoxBody. So I had to call `boxed()` here as well. Please check that the HTTP responses still work, for me, it looks fine.

Co-authored-by: Christian Beilschmidt <christian.beilschmidt@geoengine.de>
  • Loading branch information
bors[bot] and ChristianBeilschmidt committed Dec 22, 2021
2 parents a44a85f + ab520e3 commit 624e2a7
Show file tree
Hide file tree
Showing 21 changed files with 62 additions and 59 deletions.
1 change: 1 addition & 0 deletions datatypes/src/operations/image/colorizer.rs
Expand Up @@ -529,6 +529,7 @@ impl RgbaColor {
/// On debug, if factor is not in [0, 1]
///
#[allow(unstable_name_collisions)]
#[must_use]
pub fn factor_add(self, other: Self, factor: f64) -> Self {
debug_assert!((0.0..=1.0).contains(&factor));

Expand Down
2 changes: 2 additions & 0 deletions datatypes/src/plots/histogram.rs
Expand Up @@ -325,12 +325,14 @@ impl HistogramBuilder {
/// .build()
/// .unwrap();
/// ```
#[must_use]
pub fn labels(mut self, labels: Vec<String>) -> Self {
self.labels = Some(labels);
self
}

/// Add counts to the histogram
#[must_use]
pub fn counts(mut self, counts: Vec<u64>) -> Self {
self.counts = Some(counts);
self
Expand Down
1 change: 1 addition & 0 deletions datatypes/src/primitives/circle.rs
Expand Up @@ -105,6 +105,7 @@ impl Circle {
}

/// Enlarges the circle by the given delta
#[must_use]
pub fn buffer(&self, delta: f64) -> Self {
Circle {
center: self.center,
Expand Down
2 changes: 2 additions & 0 deletions datatypes/src/primitives/coordinate.rs
Expand Up @@ -42,13 +42,15 @@ impl Coordinate2D {
Self { x, y }
}

#[must_use]
pub fn min_elements(&self, other: Self) -> Self {
Coordinate2D {
x: self.x.min(other.x),
y: self.y.min(other.y),
}
}

#[must_use]
pub fn max_elements(&self, other: Self) -> Self {
Coordinate2D {
x: self.x.max(other.x),
Expand Down
1 change: 1 addition & 0 deletions datatypes/src/primitives/spatial_partition.rs
Expand Up @@ -191,6 +191,7 @@ impl SpatialPartition2D {
}

/// Align this partition by snapping bounds to the pixel borders defined by `origin` and `resolution`
#[must_use]
pub fn snap_to_grid(&self, origin: Coordinate2D, resolution: SpatialResolution) -> Self {
Self {
upper_left_coordinate: (
Expand Down
1 change: 1 addition & 0 deletions datatypes/src/primitives/time_interval.rs
Expand Up @@ -304,6 +304,7 @@ impl TimeInterval {

/// Extends a time interval with the bounds of another time interval.
/// The result has the smaller `start` and the larger `end`.
#[must_use]
pub fn extend(&self, other: &Self) -> TimeInterval {
Self {
start: self.start.min(other.start),
Expand Down
4 changes: 2 additions & 2 deletions datatypes/tests/example-arrow.rs
Expand Up @@ -676,7 +676,7 @@ fn multipoint_builder_bytes() {

let floats: &[Coordinate2D] = unsafe {
std::slice::from_raw_parts(
first_multi_point.value(0)[0] as *const u8 as *const _,
first_multi_point.value(0).as_ptr() as *const _,
first_multi_point.len(),
)
};
Expand All @@ -688,7 +688,7 @@ fn multipoint_builder_bytes() {

let floats: &[Coordinate2D] = unsafe {
std::slice::from_raw_parts(
second_multi_point.value(0)[0] as *const u8 as *const _,
second_multi_point.value(0).as_ptr() as *const _,
second_multi_point.len(),
)
};
Expand Down
2 changes: 1 addition & 1 deletion operators/Cargo.toml
Expand Up @@ -36,7 +36,7 @@ rustc-hash = { version = "1.0", default-features = false }
serde = { version = "1.0", features = ["derive"] }
serde_json = "1.0"
snafu = "0.6"
tokio = { version = "1.1", features = ["macros", "signal", "sync", "rt-multi-thread"] }
tokio = { version = "1.15", features = ["macros", "signal", "sync", "rt-multi-thread"] }
typetag = "0.1"
uuid = { version = "0.8", features = ["serde", "v4", "v5"] }

Expand Down
4 changes: 4 additions & 0 deletions operators/src/engine/result_descriptor.rs
Expand Up @@ -17,6 +17,7 @@ pub trait ResultDescriptor: Clone + Serialize {
fn spatial_reference(&self) -> SpatialReferenceOption;

/// Map one descriptor to another one
#[must_use]
fn map<F>(&self, f: F) -> Self
where
F: Fn(&Self) -> Self,
Expand All @@ -25,11 +26,13 @@ pub trait ResultDescriptor: Clone + Serialize {
}

/// Map one descriptor to another one by modifying only the spatial reference
#[must_use]
fn map_data_type<F>(&self, f: F) -> Self
where
F: Fn(&Self::DataType) -> Self::DataType;

/// Map one descriptor to another one by modifying only the data type
#[must_use]
fn map_spatial_reference<F>(&self, f: F) -> Self
where
F: Fn(&SpatialReferenceOption) -> SpatialReferenceOption;
Expand Down Expand Up @@ -90,6 +93,7 @@ pub struct VectorResultDescriptor {

impl VectorResultDescriptor {
/// Create a new `VectorResultDescriptor` by only modifying the columns
#[must_use]
pub fn map_columns<F>(&self, f: F) -> Self
where
F: Fn(&HashMap<String, FeatureDataType>) -> HashMap<String, FeatureDataType>,
Expand Down
2 changes: 1 addition & 1 deletion rust-toolchain
@@ -1,3 +1,3 @@
[toolchain]
channel = "nightly-2021-12-09"
channel = "nightly-2021-12-20"
components = ["cargo", "rustfmt", "rust-src", "clippy"]
11 changes: 5 additions & 6 deletions services/Cargo.toml
Expand Up @@ -19,12 +19,12 @@ nfdi = ["postgres", "geoengine-datatypes/postgres", "scienceobjectsdb_rust_api"]
pro = ["postgres", "geoengine-operators/pro", "geoengine-datatypes/pro"]

[dependencies]
actix-files = "=0.6.0-beta.9"
actix-http = "=3.0.0-beta.14"
actix-files = "=0.6.0-beta.10"
actix-http = "=3.0.0-beta.16"
actix-multipart = "=0.4.0-beta.9"
actix-rt = "2.5"
actix-web = "=4.0.0-beta.13"
actix-web-httpauth = "=0.6.0-beta.4"
actix-web = "=4.0.0-beta.15"
actix-web-httpauth = "=0.6.0-beta.6"
async-trait = "0.1"
base64 = "0.13"
bb8-postgres = { version = "0.7", features = ["with-uuid-0_8", "with-chrono-0_4", "with-serde_json-1"], optional = true }
Expand Down Expand Up @@ -61,7 +61,7 @@ serde_with = "1.9"
snafu = "0.6"
strum = { version = "0.23", features = ["derive"] }
time = "0.3"
tokio = { version = "1.1", features = ["macros", "fs", "signal", "sync", "rt-multi-thread"] }
tokio = { version = "1.15", features = ["macros", "fs", "signal", "sync", "rt-multi-thread"] }
tokio-util = "0.6"
tonic = { version = "0.6", features = ["tls", "tls-roots"] }
typetag = "0.1"
Expand All @@ -77,5 +77,4 @@ walkdir = "2.3"
xml-rs = "0.8.3"

[build-dependencies]
anyhow = "1.0.40"
vergen = "6"
5 changes: 2 additions & 3 deletions services/build.rs
@@ -1,10 +1,9 @@
use anyhow::Result;
use vergen::{vergen, Config, TimestampKind};

fn main() -> Result<()> {
fn main() {
let mut config = Config::default();

*config.build_mut().kind_mut() = TimestampKind::DateOnly;

vergen(config)
vergen(config).expect("Unable to generate version info");
}
4 changes: 0 additions & 4 deletions services/src/handlers/projects.rs
Expand Up @@ -271,8 +271,6 @@ mod tests {
}

#[tokio::test]
// TODO: remove when https://github.com/tokio-rs/tokio/issues/4245 is fixed
#[allow(clippy::semicolon_if_nothing_returned)]
async fn create() {
let res = create_test_helper(Method::POST).await;

Expand Down Expand Up @@ -447,8 +445,6 @@ mod tests {
}

#[tokio::test]
// TODO: remove when https://github.com/tokio-rs/tokio/issues/4245 is fixed
#[allow(clippy::semicolon_if_nothing_returned)]
async fn load() {
let res = load_test_helper(Method::GET).await;

Expand Down
2 changes: 0 additions & 2 deletions services/src/handlers/session.rs
Expand Up @@ -205,8 +205,6 @@ mod tests {
}

#[tokio::test]
// TODO: remove when https://github.com/tokio-rs/tokio/issues/4245 is fixed
#[allow(clippy::semicolon_if_nothing_returned)]
async fn anonymous() {
let res = anonymous_test_helper(Method::POST).await;

Expand Down
2 changes: 0 additions & 2 deletions services/src/handlers/workflows.rs
Expand Up @@ -505,8 +505,6 @@ mod tests {
}

#[tokio::test]
// TODO: remove when https://github.com/tokio-rs/tokio/issues/4245 is fixed
#[allow(clippy::semicolon_if_nothing_returned)]
async fn register() {
let res = register_test_helper(Method::POST).await;

Expand Down
2 changes: 0 additions & 2 deletions services/src/pro/handlers/projects.rs
Expand Up @@ -676,8 +676,6 @@ mod tests {
}

#[tokio::test]
// TODO: remove when https://github.com/tokio-rs/tokio/issues/4245 is fixed
#[allow(clippy::semicolon_if_nothing_returned)]
async fn versions() {
let res = versions_test_helper(Method::GET).await;

Expand Down
4 changes: 0 additions & 4 deletions services/src/pro/handlers/users.rs
Expand Up @@ -276,8 +276,6 @@ mod tests {
}

#[tokio::test]
// TODO: remove when https://github.com/tokio-rs/tokio/issues/4245 is fixed
#[allow(clippy::semicolon_if_nothing_returned)]
async fn register() {
let ctx = ProInMemoryContext::default();

Expand Down Expand Up @@ -424,8 +422,6 @@ mod tests {
}

#[tokio::test]
// TODO: remove when https://github.com/tokio-rs/tokio/issues/4245 is fixed
#[allow(clippy::semicolon_if_nothing_returned)]
async fn login() {
let res = login_test_helper(Method::POST, "secret123").await;

Expand Down
4 changes: 3 additions & 1 deletion services/src/pro/util/tests.rs
Expand Up @@ -127,5 +127,7 @@ where
app = app.configure(pro::handlers::drone_mapping::init_drone_mapping_routes::<C>);
}
let app = test::init_service(app).await;
test::call_service(&app, req.to_request()).await
test::call_service(&app, req.to_request())
.await
.map_into_boxed_body()
}
59 changes: 30 additions & 29 deletions services/src/server.rs
Expand Up @@ -6,7 +6,8 @@ use crate::util::config;
use crate::util::config::get_config_element;

use actix_files::Files;
use actix_web::dev::{AnyBody, ServiceResponse};
use actix_http::body::{BoxBody, EitherBody, MessageBody};
use actix_web::dev::ServiceResponse;
use actix_web::error::{InternalError, JsonPayloadError, QueryPayloadError};
use actix_web::{http, middleware, web, App, HttpResponse, HttpServer};
use log::{debug, info};
Expand Down Expand Up @@ -231,42 +232,42 @@ pub(crate) async fn show_version_handler() -> impl actix_web::Responder {

#[allow(clippy::unnecessary_wraps)]
pub(crate) fn render_404(
mut res: ServiceResponse,
) -> actix_web::Result<middleware::ErrorHandlerResponse<AnyBody>> {
res.headers_mut().insert(
mut response: ServiceResponse,
) -> actix_web::Result<middleware::ErrorHandlerResponse<BoxBody>> {
response.headers_mut().insert(
http::header::CONTENT_TYPE,
http::HeaderValue::from_static("application/json"),
http::header::HeaderValue::from_static("application/json"),
);
let res = res.map_body(|_, _| {
AnyBody::from(
serde_json::to_string(&ErrorResponse {
error: "NotFound".to_string(),
message: "Not Found".to_string(),
})
.unwrap(),
)
});
Ok(middleware::ErrorHandlerResponse::Response(res))

let response_json_string = serde_json::to_string(&ErrorResponse {
error: "NotFound".to_string(),
message: "Not Found".to_string(),
})
.expect("Serialization of fixed ErrorResponse must not fail");

let response = response.map_body(|_, _| EitherBody::new(response_json_string.boxed()));

Ok(middleware::ErrorHandlerResponse::Response(response))
}

#[allow(clippy::unnecessary_wraps)]
pub(crate) fn render_405(
mut res: ServiceResponse,
) -> actix_web::Result<middleware::ErrorHandlerResponse<AnyBody>> {
res.headers_mut().insert(
mut response: ServiceResponse,
) -> actix_web::Result<middleware::ErrorHandlerResponse<BoxBody>> {
response.headers_mut().insert(
http::header::CONTENT_TYPE,
http::HeaderValue::from_static("application/json"),
http::header::HeaderValue::from_static("application/json"),
);
let res = res.map_body(|_, _| {
AnyBody::from(
serde_json::to_string(&ErrorResponse {
error: "MethodNotAllowed".to_string(),
message: "HTTP method not allowed.".to_string(),
})
.unwrap(),
)
});
Ok(middleware::ErrorHandlerResponse::Response(res))

let response_json_string = serde_json::to_string(&ErrorResponse {
error: "MethodNotAllowed".to_string(),
message: "HTTP method not allowed.".to_string(),
})
.expect("Serialization of fixed ErrorResponse must not fail");

let response = response.map_body(|_, _| EitherBody::new(response_json_string.boxed()));

Ok(middleware::ErrorHandlerResponse::Response(response))
}

#[cfg(test)]
Expand Down
2 changes: 1 addition & 1 deletion services/src/util/parsing.rs
Expand Up @@ -135,7 +135,7 @@ mod tests {
impl Display for Test {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match &self.base_url {
Some(base_url) => f.write_str(&base_url.to_string()),
Some(base_url) => f.write_str(base_url.as_ref()),
None => f.write_str(""),
}
}
Expand Down
6 changes: 5 additions & 1 deletion services/src/util/tests.rs
Expand Up @@ -208,7 +208,9 @@ pub async fn send_test_request<C: SimpleContext>(
.configure(handlers::workflows::init_workflow_routes::<C>),
)
.await;
test::call_service(&app, req.to_request()).await
test::call_service(&app, req.to_request())
.await
.map_into_boxed_body()
}

pub async fn read_body_string(res: ServiceResponse) -> String {
Expand Down Expand Up @@ -243,8 +245,10 @@ pub fn initialize_debugging_in_test() {
}

pub trait SetMultipartBody {
#[must_use]
fn set_multipart<B: Into<Vec<u8>>>(self, parts: Vec<(&str, B)>) -> Self;

#[must_use]
fn set_multipart_files(self, file_paths: &[PathBuf]) -> Self
where
Self: Sized,
Expand Down

0 comments on commit 624e2a7

Please sign in to comment.