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.

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

refactor: Flatten autoendpoint server module #182

Merged
merged 1 commit into from
Jul 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion autoendpoint/src/error.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! Error types and transformations

use crate::server::{RouterError, VapidError};
use crate::headers::vapid::VapidError;
use crate::routers::RouterError;
AzureMarker marked this conversation as resolved.
Show resolved Hide resolved
use actix_web::{
dev::{HttpResponseBuilder, ServiceResponse},
error::{PayloadError, ResponseError},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::error::{ApiError, ApiErrorKind};
use crate::server::extractors::notification_headers::NotificationHeaders;
use crate::server::extractors::subscription::Subscription;
use crate::extractors::notification_headers::NotificationHeaders;
use crate::extractors::subscription::Subscription;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

optional, could probably collapse these:

use crate::extractors::{
    notification_headers:::NotificationHeaders,
    subscription::Subscription,
};

if you do, just need to be consistent with other, similar includes. Could save a bit of typing for any future moves like this.

Copy link
Contributor Author

@AzureMarker AzureMarker Jul 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can make rustfmt handle this automatically:
https://rust-lang.github.io/rustfmt/?version=master&search=#merge_imports

Unfortunately this currently requires nightly, until rustfmt 2.0 is released. The reason is that it is an unstable style feature because it has the potential to break code.

Alternatively, do we really want this? I agree it's more consistent and saves typing, but I've also seen the argument that it is more prone to merge conflicts.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it's the clippy battle. I think clippy really wanted folks to collapse when possible, probably because of tons of one line uses.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, is Clippy complaining about this? I don't see any relevant lints in https://rust-lang.github.io/rust-clippy/stable/index.html

use crate::server::ServerState;
use actix_web::dev::{Payload, PayloadStream};
use actix_web::web::Data;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::error::{ApiError, ApiErrorKind, ApiResult};
use crate::server::headers::crypto_key::CryptoKeyHeader;
use crate::server::headers::util::{get_header, get_owned_header};
use crate::headers::crypto_key::CryptoKeyHeader;
use crate::headers::util::{get_header, get_owned_header};
use actix_web::HttpRequest;
use autopush_common::util::InsertOpt;
use lazy_static::lazy_static;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::server::routers::fcm::router::FcmRouter;
use crate::server::routers::webpush::WebPushRouter;
use crate::server::routers::Router;
use crate::routers::fcm::router::FcmRouter;
use crate::routers::webpush::WebPushRouter;
use crate::routers::Router;
use crate::server::ServerState;
use actix_web::dev::{Payload, PayloadStream};
use actix_web::web::Data;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
use crate::error::{ApiError, ApiErrorKind, ApiResult};
use crate::server::extractors::routers::RouterType;
use crate::server::extractors::token_info::{ApiVersion, TokenInfo};
use crate::server::extractors::user::validate_user;
use crate::server::headers::crypto_key::CryptoKeyHeader;
use crate::server::headers::vapid::{VapidHeader, VapidHeaderWithKey, VapidVersionData};
use crate::server::{ServerState, VapidError};
use crate::extractors::routers::RouterType;
use crate::extractors::token_info::{ApiVersion, TokenInfo};
use crate::extractors::user::validate_user;
use crate::headers::crypto_key::CryptoKeyHeader;
use crate::headers::vapid::{VapidError, VapidHeader, VapidHeaderWithKey, VapidVersionData};
use crate::server::ServerState;
use actix_http::{Payload, PayloadStream};
use actix_web::web::Data;
use actix_web::{FromRequest, HttpRequest};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::error::{ApiError, ApiErrorKind};
use crate::server::headers::util::get_owned_header;
use crate::headers::util::get_owned_header;
use actix_http::{Payload, PayloadStream};
use actix_web::{FromRequest, HttpRequest};
use futures::future;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! User validations

use crate::error::{ApiErrorKind, ApiResult};
use crate::server::extractors::routers::RouterType;
use crate::extractors::routers::RouterType;
use crate::server::ServerState;
use autopush_common::db::{DynamoDbUser, DynamoStorage};
use cadence::{Counted, StatsdClient};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::server::headers::util::split_key_value;
use crate::headers::util::split_key_value;
use std::collections::HashMap;

/// Parses the Crypto-Key header (and similar headers) described by
Expand Down
File renamed without changes.
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::server::headers::util::split_key_value;
use crate::headers::util::split_key_value;
use std::collections::HashMap;
use thiserror::Error;

Expand Down
4 changes: 4 additions & 0 deletions autoendpoint/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,12 @@
extern crate slog_scope;

mod error;
mod extractors;
mod headers;
mod logging;
mod metrics;
mod routers;
mod routes;
mod server;
mod settings;
mod tags;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use crate::server::routers::fcm::error::FcmError;
use crate::server::routers::fcm::settings::FcmCredential;
use crate::server::FcmSettings;
use crate::routers::fcm::error::FcmError;
use crate::routers::fcm::settings::{FcmCredential, FcmSettings};
use reqwest::StatusCode;
use serde::Deserialize;
use std::collections::HashMap;
Expand Down Expand Up @@ -118,10 +117,9 @@ struct FcmErrorResponse {

#[cfg(test)]
pub mod tests {
use crate::server::routers::fcm::client::FcmClient;
use crate::server::routers::fcm::error::FcmError;
use crate::server::routers::fcm::settings::FcmCredential;
use crate::server::FcmSettings;
use crate::routers::fcm::client::FcmClient;
use crate::routers::fcm::error::FcmError;
use crate::routers::fcm::settings::{FcmCredential, FcmSettings};
use std::collections::HashMap;
use std::io::Write;
use std::path::PathBuf;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::error::ApiErrorKind;
use crate::server::RouterError;
use crate::routers::RouterError;
use actix_web::http::StatusCode;

/// Errors that may occur in the Firebase Cloud Messaging router
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
use crate::error::{ApiError, ApiResult};
use crate::server::extractors::notification::Notification;
use crate::server::routers::fcm::client::FcmClient;
use crate::server::routers::fcm::error::FcmError;
use crate::server::routers::fcm::settings::FcmCredential;
use crate::server::routers::{Router, RouterResponse};
use crate::server::FcmSettings;
use crate::extractors::notification::Notification;
use crate::routers::fcm::client::FcmClient;
use crate::routers::fcm::error::FcmError;
use crate::routers::fcm::settings::{FcmCredential, FcmSettings};
use crate::routers::{Router, RouterResponse};
use async_trait::async_trait;
use autopush_common::util::InsertOpt;
use cadence::{Counted, StatsdClient};
Expand Down Expand Up @@ -207,18 +206,18 @@ impl Router for FcmRouter {
#[cfg(test)]
mod tests {
use crate::error::ApiErrorKind;
use crate::server::extractors::notification::Notification;
use crate::server::extractors::notification_headers::NotificationHeaders;
use crate::server::extractors::routers::RouterType;
use crate::server::extractors::subscription::Subscription;
use crate::server::routers::fcm::client::tests::{
use crate::extractors::notification::Notification;
use crate::extractors::notification_headers::NotificationHeaders;
use crate::extractors::routers::RouterType;
use crate::extractors::subscription::Subscription;
use crate::routers::fcm::client::tests::{
make_service_file, mock_fcm_endpoint_builder, mock_token_endpoint, PROJECT_ID,
};
use crate::server::routers::fcm::error::FcmError;
use crate::server::routers::fcm::router::FcmRouter;
use crate::server::routers::RouterError;
use crate::server::routers::{Router, RouterResponse};
use crate::server::FcmSettings;
use crate::routers::fcm::error::FcmError;
use crate::routers::fcm::router::FcmRouter;
use crate::routers::fcm::settings::FcmSettings;
use crate::routers::RouterError;
use crate::routers::{Router, RouterResponse};
use autopush_common::db::DynamoDbUser;
use cadence::StatsdClient;
use std::collections::HashMap;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
//! Routers route notifications to user agents

use crate::error::ApiResult;
use crate::server::extractors::notification::Notification;
use crate::server::routers::fcm::error::FcmError;
use crate::extractors::notification::Notification;
use crate::routers::fcm::error::FcmError;
use actix_web::http::StatusCode;
use actix_web::HttpResponse;
use async_trait::async_trait;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use crate::error::{ApiErrorKind, ApiResult};
use crate::server::extractors::notification::Notification;
use crate::server::routers::{Router, RouterResponse};
use crate::server::RouterError;
use crate::extractors::notification::Notification;
use crate::routers::{Router, RouterError, RouterResponse};
use async_trait::async_trait;
use autopush_common::db::{DynamoDbUser, DynamoStorage};
use cadence::{Counted, StatsdClient};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,5 +32,5 @@ pub fn version_route() -> HttpResponse {
// and stored in the docker root
HttpResponse::Ok()
.content_type("application/json")
.body(include_str!("../../../../version.json"))
.body(include_str!("../../../version.json"))
}
File renamed without changes.
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::error::ApiResult;
use crate::server::extractors::notification::Notification;
use crate::server::extractors::routers::Routers;
use crate::extractors::notification::Notification;
use crate::extractors::routers::Routers;
use actix_web::HttpResponse;

/// Handle the `/wpush/{api_version}/{token}` and `/wpush/{token}` routes
Expand Down
17 changes: 3 additions & 14 deletions autoendpoint/src/server/mod.rs → autoendpoint/src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,9 @@

use crate::error::{ApiError, ApiErrorKind, ApiResult};
use crate::metrics;
use crate::server::routers::fcm::router::FcmRouter;
use crate::server::routes::health::{
health_route, lb_heartbeat_route, status_route, version_route,
};
use crate::server::routes::webpush::webpush_route;
use crate::routers::fcm::router::FcmRouter;
use crate::routes::health::{health_route, lb_heartbeat_route, status_route, version_route};
use crate::routes::webpush::webpush_route;
use crate::settings::Settings;
use actix_cors::Cors;
use actix_web::{
Expand All @@ -17,15 +15,6 @@ use cadence::StatsdClient;
use fernet::MultiFernet;
use std::sync::Arc;

mod extractors;
mod headers;
mod routers;
mod routes;

pub use headers::vapid::VapidError;
pub use routers::fcm::settings::FcmSettings;
pub use routers::RouterError;

#[derive(Clone)]
pub struct ServerState {
/// Server Data
Expand Down
2 changes: 1 addition & 1 deletion autoendpoint/src/settings.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! Application settings

use crate::server::FcmSettings;
use crate::routers::fcm::settings::FcmSettings;
use config::{Config, ConfigError, Environment, File};
use fernet::{Fernet, MultiFernet};
use serde::Deserialize;
Expand Down