From 1dd86baf1abe0f15893124a9be9cc714841d7834 Mon Sep 17 00:00:00 2001 From: Luke Anderson Date: Fri, 13 Sep 2019 19:38:40 +1000 Subject: [PATCH] Cleaning up the rest of the API functions. - Removed all unused imports - Fixed random compiler errors - Removed all of the 'sucess_response' helpers. - Enabled all of the API endpoints again, wrapping in 'into_boxfut' - Tidied up /metrics endpoint - Added a 'body_text' part to ResponseBuilder, mainly for the Prometheus /metrics endpoint - Cleaned up the unnecessary helpers::* imports, to be more explicit. --- beacon_node/rest_api/src/beacon.rs | 2 +- beacon_node/rest_api/src/error.rs | 6 +- beacon_node/rest_api/src/helpers.rs | 38 +---------- beacon_node/rest_api/src/lib.rs | 71 +++++++++++--------- beacon_node/rest_api/src/metrics.rs | 16 ++--- beacon_node/rest_api/src/network.rs | 5 +- beacon_node/rest_api/src/node.rs | 4 +- beacon_node/rest_api/src/response_builder.rs | 8 +++ beacon_node/rest_api/src/spec.rs | 21 ++---- beacon_node/rest_api/src/validator.rs | 7 +- 10 files changed, 70 insertions(+), 108 deletions(-) diff --git a/beacon_node/rest_api/src/beacon.rs b/beacon_node/rest_api/src/beacon.rs index fef3cbdf1..159337de4 100644 --- a/beacon_node/rest_api/src/beacon.rs +++ b/beacon_node/rest_api/src/beacon.rs @@ -1,6 +1,6 @@ use crate::helpers::*; use crate::response_builder::ResponseBuilder; -use crate::{ApiError, ApiResult, BoxFut, NetworkService, UrlQuery}; +use crate::{ApiError, ApiResult, UrlQuery}; use beacon_chain::{BeaconChain, BeaconChainTypes}; use hyper::{Body, Request}; use serde::Serialize; diff --git a/beacon_node/rest_api/src/error.rs b/beacon_node/rest_api/src/error.rs index 82dc73da0..26cf7ba1f 100644 --- a/beacon_node/rest_api/src/error.rs +++ b/beacon_node/rest_api/src/error.rs @@ -1,7 +1,5 @@ use crate::BoxFut; -use futures::future::IntoFuture; -use futures::Future; -use hyper::{Body, Method, Request, Response, Server, StatusCode}; +use hyper::{Body, Response, StatusCode}; use std::error::Error as StdError; #[derive(PartialEq, Debug, Clone)] @@ -71,7 +69,7 @@ impl From for ApiError { } impl StdError for ApiError { - fn cause(&self) -> Option<&StdError> { + fn cause(&self) -> Option<&dyn StdError> { None } } diff --git a/beacon_node/rest_api/src/helpers.rs b/beacon_node/rest_api/src/helpers.rs index 99a8f1dd4..3385e2017 100644 --- a/beacon_node/rest_api/src/helpers.rs +++ b/beacon_node/rest_api/src/helpers.rs @@ -1,53 +1,19 @@ -use crate::response_builder::ResponseBuilder; -use crate::{ApiError, ApiResult, BoxFut}; +use crate::{ApiError, ApiResult}; use beacon_chain::{BeaconChain, BeaconChainTypes}; use bls::PublicKey; use eth2_libp2p::{PubsubMessage, Topic}; use eth2_libp2p::{BEACON_BLOCK_TOPIC, TOPIC_ENCODING_POSTFIX, TOPIC_PREFIX}; use hex; use http::header; -use hyper::{Body, Request, Response, StatusCode}; +use hyper::{Body, Request}; use network::NetworkMessage; use parking_lot::RwLock; -use serde::Serialize; use ssz::Encode; use std::sync::Arc; use store::{iter::AncestorIter, Store}; use tokio::sync::mpsc; use types::{BeaconBlock, BeaconState, EthSpec, Hash256, RelativeEpoch, Slot}; -pub fn success_response(req: Request, item: &T) -> BoxFut { - Box::new(match ResponseBuilder::new(&req).body(item) { - Ok(resp) => futures::future::ok(resp), - Err(e) => futures::future::err(e), - }) -} - -pub fn success_response_2(req: Request, item: &T) -> ApiResult { - ResponseBuilder::new(&req).body(item) -} -pub fn success_response_2_json(req: Request, item: &T) -> ApiResult { - ResponseBuilder::new(&req).body_json(item) -} - -pub fn success_response_json(req: Request, item: &T) -> BoxFut { - if let Err(e) = check_content_type_for_json(&req) { - return Box::new(futures::future::err(e)); - } - Box::new(match ResponseBuilder::new(&req).body_json(item) { - Ok(resp) => futures::future::ok(resp), - Err(e) => futures::future::err(e), - }) -} - -pub fn success_response_old(body: Body) -> Response { - Response::builder() - .status(StatusCode::OK) - .header("content-type", "application/json") - .body(body) - .expect("We should always be able to make response from the success body.") -} - /// Parse a slot from a `0x` preixed string. /// /// E.g., `"1234"` diff --git a/beacon_node/rest_api/src/lib.rs b/beacon_node/rest_api/src/lib.rs index 3903d0ea1..35678391a 100644 --- a/beacon_node/rest_api/src/lib.rs +++ b/beacon_node/rest_api/src/lib.rs @@ -23,9 +23,8 @@ use error::{ApiError, ApiResult}; use eth2_config::Eth2Config; use futures::future::IntoFuture; use hyper::rt::Future; -use hyper::server::conn::AddrStream; -use hyper::service::{MakeService, Service}; -use hyper::{Body, Method, Request, Response, Server, StatusCode}; +use hyper::service::Service; +use hyper::{Body, Method, Request, Response, Server}; use parking_lot::RwLock; use slog::{info, o, warn}; use std::ops::Deref; @@ -37,8 +36,6 @@ use url_query::UrlQuery; pub use beacon::{BlockResponse, HeadResponse, StateResponse}; pub use config::Config as ApiConfig; -use eth2_libp2p::rpc::RequestId; -use serde::export::PhantomData; type BoxFut = Box, Error = ApiError> + Send>; @@ -107,58 +104,66 @@ impl Service for ApiService { (&Method::GET, "/network/listen_addresses") => { into_boxfut(network::get_listen_addresses::(req)) } - /* // Methods for Beacon Node - (&Method::GET, "/beacon/head") => beacon::get_head::(req), - (&Method::GET, "/beacon/block") => beacon::get_block::(req), - (&Method::GET, "/beacon/block_root") => beacon::get_block_root::(req), - (&Method::GET, "/beacon/blocks") => helpers::implementation_pending_response(req), - (&Method::GET, "/beacon/fork") => beacon::get_fork::(req), - (&Method::GET, "/beacon/attestations") => helpers::implementation_pending_response(req), + (&Method::GET, "/beacon/head") => into_boxfut(beacon::get_head::(req)), + (&Method::GET, "/beacon/block") => into_boxfut(beacon::get_block::(req)), + (&Method::GET, "/beacon/block_root") => into_boxfut(beacon::get_block_root::(req)), + (&Method::GET, "/beacon/blocks") => { + into_boxfut(helpers::implementation_pending_response(req)) + } + (&Method::GET, "/beacon/fork") => into_boxfut(beacon::get_fork::(req)), + (&Method::GET, "/beacon/attestations") => { + into_boxfut(helpers::implementation_pending_response(req)) + } (&Method::GET, "/beacon/attestations/pending") => { - helpers::implementation_pending_response(req) + into_boxfut(helpers::implementation_pending_response(req)) } - (&Method::GET, "/beacon/validators") => beacon::get_validators::(req), + (&Method::GET, "/beacon/validators") => into_boxfut(beacon::get_validators::(req)), (&Method::GET, "/beacon/validators/indicies") => { - helpers::implementation_pending_response(req) + into_boxfut(helpers::implementation_pending_response(req)) } (&Method::GET, "/beacon/validators/pubkeys") => { - helpers::implementation_pending_response(req) + into_boxfut(helpers::implementation_pending_response(req)) } // Methods for Validator - (&Method::GET, "/beacon/validator/duties") => validator::get_validator_duties::(req), - (&Method::GET, "/beacon/validator/block") => validator::get_new_beacon_block::(req), - */ + (&Method::GET, "/beacon/validator/duties") => { + into_boxfut(validator::get_validator_duties::(req)) + } + (&Method::GET, "/beacon/validator/block") => { + into_boxfut(validator::get_new_beacon_block::(req)) + } (&Method::POST, "/beacon/validator/block") => validator::publish_beacon_block::(req), - /* (&Method::GET, "/beacon/validator/attestation") => { - validator::get_new_attestation::(req) + into_boxfut(validator::get_new_attestation::(req)) } (&Method::POST, "/beacon/validator/attestation") => { - helpers::implementation_pending_response(req) + into_boxfut(helpers::implementation_pending_response(req)) } - (&Method::GET, "/beacon/state") => beacon::get_state::(req), - (&Method::GET, "/beacon/state_root") => beacon::get_state_root::(req), + (&Method::GET, "/beacon/state") => into_boxfut(beacon::get_state::(req)), + (&Method::GET, "/beacon/state_root") => into_boxfut(beacon::get_state_root::(req)), (&Method::GET, "/beacon/state/current_finalized_checkpoint") => { - beacon::get_current_finalized_checkpoint::(req) + into_boxfut(beacon::get_current_finalized_checkpoint::(req)) + } + (&Method::GET, "/beacon/state/genesis") => { + into_boxfut(beacon::get_genesis_state::(req)) } - (&Method::GET, "/beacon/state/genesis") => beacon::get_genesis_state::(req), //TODO: Add aggreggate/filtered state lookups here, e.g. /beacon/validators/balances // Methods for bootstrap and checking configuration - (&Method::GET, "/spec") => spec::get_spec::(req), - (&Method::GET, "/spec/slots_per_epoch") => spec::get_slots_per_epoch::(req), - (&Method::GET, "/spec/deposit_contract") => { - helpers::implementation_pending_response(req) + (&Method::GET, "/spec") => into_boxfut(spec::get_spec::(req)), + (&Method::GET, "/spec/slots_per_epoch") => { + into_boxfut(spec::get_slots_per_epoch::(req)) } - (&Method::GET, "/spec/eth2_config") => spec::get_eth2_config::(req), + (&Method::GET, "/spec/deposit_contract") => { + into_boxfut(helpers::implementation_pending_response(req)) + } + (&Method::GET, "/spec/eth2_config") => into_boxfut(spec::get_eth2_config::(req)), - (&Method::GET, "/metrics") => metrics::get_prometheus::(req), - */ + (&Method::GET, "/metrics") => into_boxfut(metrics::get_prometheus::(req)), _ => Box::new(futures::future::err(ApiError::NotFound( "Request path and/or method not found.".to_owned(), ))), diff --git a/beacon_node/rest_api/src/metrics.rs b/beacon_node/rest_api/src/metrics.rs index d50e4a98c..33437a534 100644 --- a/beacon_node/rest_api/src/metrics.rs +++ b/beacon_node/rest_api/src/metrics.rs @@ -1,7 +1,7 @@ +use crate::helpers::get_beacon_chain_from_request; use crate::response_builder::ResponseBuilder; -use crate::{helpers::*, ApiError, ApiResult, DBPath}; +use crate::{ApiError, ApiResult, DBPath}; use beacon_chain::BeaconChainTypes; -use http::HeaderValue; use hyper::{Body, Request}; use prometheus::{Encoder, TextEncoder}; @@ -62,14 +62,6 @@ pub fn get_prometheus(req: Request) -> ApiR .unwrap(); String::from_utf8(buffer) - .map(|string| { - let mut response = success_response_old(Body::from(string)); - // Need to change the header to text/plain for prometheus - response.headers_mut().insert( - "content-type", - HeaderValue::from_static("text/plain; charset=utf-8"), - ); - response - }) - .map_err(|e| ApiError::ServerError(format!("Failed to encode prometheus info: {:?}", e))) + .map(|string| ResponseBuilder::new(&req).body_text(string)) + .map_err(|e| ApiError::ServerError(format!("Failed to encode prometheus info: {:?}", e)))? } diff --git a/beacon_node/rest_api/src/network.rs b/beacon_node/rest_api/src/network.rs index 26e5623c2..afbddde84 100644 --- a/beacon_node/rest_api/src/network.rs +++ b/beacon_node/rest_api/src/network.rs @@ -1,9 +1,8 @@ -use crate::error::{ApiError, ApiResult}; -use crate::helpers::*; +use crate::error::ApiResult; use crate::response_builder::ResponseBuilder; use crate::NetworkService; use beacon_chain::BeaconChainTypes; -use eth2_libp2p::{Enr, Multiaddr, PeerId}; +use eth2_libp2p::{Multiaddr, PeerId}; use hyper::{Body, Request}; use std::sync::Arc; diff --git a/beacon_node/rest_api/src/node.rs b/beacon_node/rest_api/src/node.rs index 3eb8e5594..cb1b28df7 100644 --- a/beacon_node/rest_api/src/node.rs +++ b/beacon_node/rest_api/src/node.rs @@ -1,6 +1,6 @@ -use crate::helpers::*; +use crate::helpers::get_beacon_chain_from_request; use crate::response_builder::ResponseBuilder; -use crate::{ApiResult, BoxFut}; +use crate::ApiResult; use beacon_chain::BeaconChainTypes; use hyper::{Body, Request}; use version; diff --git a/beacon_node/rest_api/src/response_builder.rs b/beacon_node/rest_api/src/response_builder.rs index b48b9e41a..31f717697 100644 --- a/beacon_node/rest_api/src/response_builder.rs +++ b/beacon_node/rest_api/src/response_builder.rs @@ -61,4 +61,12 @@ impl ResponseBuilder { })?)) .map_err(|e| ApiError::ServerError(format!("Failed to build response: {:?}", e))) } + + pub fn body_text(self, text: String) -> ApiResult { + Response::builder() + .status(StatusCode::OK) + .header("content-type", "text/plain; charset=utf-8") + .body(Body::from(text)) + .map_err(|e| ApiError::ServerError(format!("Failed to build response: {:?}", e))) + } } diff --git a/beacon_node/rest_api/src/spec.rs b/beacon_node/rest_api/src/spec.rs index 5ab518636..55a139f16 100644 --- a/beacon_node/rest_api/src/spec.rs +++ b/beacon_node/rest_api/src/spec.rs @@ -1,5 +1,6 @@ use super::ApiResult; -use crate::helpers::*; +use crate::helpers::get_beacon_chain_from_request; +use crate::response_builder::ResponseBuilder; use crate::ApiError; use beacon_chain::BeaconChainTypes; use eth2_config::Eth2Config; @@ -10,11 +11,7 @@ use types::EthSpec; /// HTTP handler to return the full spec object. pub fn get_spec(req: Request) -> ApiResult { let beacon_chain = get_beacon_chain_from_request::(&req)?; - - let json: String = serde_json::to_string(&beacon_chain.spec) - .map_err(|e| ApiError::ServerError(format!("Unable to serialize spec: {:?}", e)))?; - - Ok(success_response_old(Body::from(json))) + ResponseBuilder::new(&req).body_json(&beacon_chain.spec) } /// HTTP handler to return the full Eth2Config object. @@ -24,16 +21,10 @@ pub fn get_eth2_config(req: Request) -> Api .get::>() .ok_or_else(|| ApiError::ServerError("Eth2Config extension missing".to_string()))?; - let json: String = serde_json::to_string(eth2_config.as_ref()) - .map_err(|e| ApiError::ServerError(format!("Unable to serialize Eth2Config: {:?}", e)))?; - - Ok(success_response_old(Body::from(json))) + ResponseBuilder::new(&req).body_json(eth2_config.as_ref()) } /// HTTP handler to return the full spec object. -pub fn get_slots_per_epoch(_req: Request) -> ApiResult { - let json: String = serde_json::to_string(&T::EthSpec::slots_per_epoch()) - .map_err(|e| ApiError::ServerError(format!("Unable to serialize epoch: {:?}", e)))?; - - Ok(success_response_old(Body::from(json))) +pub fn get_slots_per_epoch(req: Request) -> ApiResult { + ResponseBuilder::new(&req).body(&T::EthSpec::slots_per_epoch()) } diff --git a/beacon_node/rest_api/src/validator.rs b/beacon_node/rest_api/src/validator.rs index 05ae6b8c9..d9d55bad4 100644 --- a/beacon_node/rest_api/src/validator.rs +++ b/beacon_node/rest_api/src/validator.rs @@ -1,11 +1,14 @@ -use crate::helpers::*; +use crate::helpers::{ + check_content_type_for_json, get_beacon_chain_from_request, get_logger_from_request, + parse_pubkey, publish_beacon_block_to_network, +}; use crate::response_builder::ResponseBuilder; use crate::{ApiError, ApiResult, BoxFut, UrlQuery}; use beacon_chain::{BeaconChainTypes, BlockProcessingOutcome}; use bls::{AggregateSignature, PublicKey, Signature}; use futures::future::Future; use futures::stream::Stream; -use hyper::{Body, Error, Request}; +use hyper::{Body, Request}; use network::NetworkMessage; use parking_lot::RwLock; use serde::{Deserialize, Serialize};