Fix clippy warnings and add build checks

stevepryde · Nov 4, 2020 · c07e680 · c07e680
1 parent b304b99
commit c07e680
Show file tree

Hide file tree

Showing 7 changed files with 68 additions and 26 deletions.
diff --git a/.github/workflows/audit-check.yml b/.github/workflows/audit-check.yml
@@ -0,0 +1,18 @@
+name: Security audit
+on:
+  push:
+    paths:
+      - '**/Cargo.toml'
+      - '**/Cargo.lock'
+  pull_request:
+    paths:
+      - '**/Cargo.toml'
+      - '**/Cargo.lock'
+jobs:
+  security_audit:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions-rs/audit-check@v1
+        with:
+          token: ${{ secrets.GITHUB_TOKEN }}
diff --git a/.github/workflows/build-check.yml b/.github/workflows/build-check.yml
@@ -0,0 +1,18 @@
+name: build-check
+
+on: [push, pull_request]
+
+env:
+  CARGO_TERM_COLOR: always
+
+jobs:
+  build:
+
+    runs-on: ubuntu-latest
+
+    steps:
+    - uses: actions/checkout@v2
+    - name: Build
+      run: cargo build --verbose
+    - name: Run tests
+      run: cargo test --verbose
diff --git a/.github/workflows/clippy-check.yml b/.github/workflows/clippy-check.yml
@@ -0,0 +1,11 @@
+on: [push, pull_request]
+name: Clippy check
+jobs:
+  clippy_check:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - run: rustup component add clippy
+      - uses: actions-rs/clippy-check@v1
+        with:
+          token: ${{ secrets.GITHUB_TOKEN }}
diff --git a/src/nodes.rs b/src/nodes.rs
@@ -47,7 +47,7 @@ fn parse_url(url: &str) -> Option<(Scheme, Authority)> {
     match url.parse::<Uri>() {
         Ok(uri) => {
             let scheme = uri.scheme().cloned().unwrap_or(Scheme::HTTP);
-            let authority = uri.authority().cloned().unwrap_or(default_authority());
+            let authority = uri.authority().cloned().unwrap_or_else(default_authority);
             Some((scheme, authority))
         }
         Err(_) => None,

diff --git a/src/response.rs b/src/response.rs
@@ -1,4 +1,4 @@
-use hyper::{Body, StatusCode};
+use hyper::{Body, Response, StatusCode};
 use serde::Serialize;
 
 #[derive(Debug, Serialize)]
@@ -26,6 +26,18 @@ impl XenonResponse {
             _ => StatusCode::INTERNAL_SERVER_ERROR,
         }
     }
+
+    pub fn into_response(self) -> Response<Body> {
+        Response::builder()
+            .status(self.status())
+            .body(self.into())
+            .unwrap_or_else(|_| {
+                Response::builder()
+                    .status(StatusCode::INTERNAL_SERVER_ERROR)
+                    .body(Body::from("Xenon failed to serialize an error"))
+                    .unwrap()
+            })
+    }
 }
 
 impl Into<Body> for XenonResponse {

diff --git a/src/server.rs b/src/server.rs
@@ -77,10 +77,10 @@ pub async fn start_server() -> XenonResult<()> {
         let remote_addr = conn.remote_addr();
         async move {
             let state = state.clone();
-            let remote_addr = remote_addr.clone();
+            let remote_addr = remote_addr;
             Ok::<_, Infallible>(service_fn(move |req| {
                 let state = state.clone();
-                handle(req, remote_addr.clone(), state)
+                handle(req, remote_addr, state)
             }))
         }
     });
@@ -132,30 +132,13 @@ async fn handle(
         Ok(x) => Ok(x),
         Err(XenonError::RespondWith(r)) => {
             debug!("Xenon replied with error: {:#?}", r);
-            Ok(Response::builder()
-                .status(r.status())
-                .body(r.into())
-                .unwrap_or_else(|_| {
-                    Response::builder()
-                        .status(StatusCode::INTERNAL_SERVER_ERROR)
-                        .body(Body::from("Xenon failed to serialize an error"))
-                        .unwrap()
-                }))
+            Ok(r.into_response())
         }
         Err(e) => {
             // Coerce all errors into WebDriver-compatible response.
             error!("Internal Error: {:#?}", e);
             let r = XenonResponse::InternalServerError(e.to_string());
-
-            Ok(Response::builder()
-                .status(r.status())
-                .body(r.into())
-                .unwrap_or_else(|_| {
-                    Response::builder()
-                        .status(StatusCode::INTERNAL_SERVER_ERROR)
-                        .body(Body::from("Xenon failed to serialize an error"))
-                        .unwrap()
-                }))
+            Ok(r.into_response())
         }
     }
 }
@@ -635,8 +618,8 @@ async fn process_node_init(state: Arc<RwLock<XenonState>>) {
                                 }
                             };
 
-                        // Update these. This requires a write lock but only briefly.
-                        let s = state.write().await;
+                        // Update these. Read lock on state. Write lock on nodes.
+                        let s = state.read().await;
                         let rwlock_nodes = s.remote_nodes();
                         let mut nodes = rwlock_nodes.write().await;
                         if let Some(node) = nodes.get_mut(&node.id()) {

diff --git a/src/session.rs b/src/session.rs
@@ -90,7 +90,7 @@ impl Session {
             Some(p) => p,
             None => {
                 return Err(XenonError::RespondWith(
-                    XenonResponse::ErrorCreatingSession(format!("Port not recognised")),
+                    XenonResponse::ErrorCreatingSession("Port not recognised".to_string()),
                 ))
             }
         };