refactor: api job starter waits until api is ready

josecelano · josecelano · commit 15aa8313fde6 · 2022-11-25T14:04:23.000Z
There are two main changes:

- The API server does not send the message when is ready. The job
  starter waits until the API server is running. This change is less
  radical becuase we keep the `start_job` return type as the other job
  starters. We did not want to send a real message from the API. We only
  wanted to know that the API thread is up and running.
- The job starter waits until the API job is running even in production
  code. In the previous version we did that only for the e2e tests.
diff --git a/src/api/server.rs b/src/api/server.rs
@@ -5,11 +5,9 @@ use std::sync::Arc;
 use std::time::Duration;
 
 use serde::{Deserialize, Serialize};
-use tokio::sync::oneshot::Sender;
 use warp::{filters, reply, serve, Filter};
 
 use super::resources::auth_key_resource::AuthKeyResource;
-use crate::jobs::tracker_api::ApiReady;
 use crate::peer::TorrentPeer;
 use crate::protocol::common::*;
 use crate::tracker::TorrentTracker;
@@ -90,11 +88,7 @@ fn authenticate(tokens: HashMap<String, String>) -> impl Filter<Extract = (), Er
         .untuple_one()
 }
 
-pub fn start(
-    socket_addr: SocketAddr,
-    tracker: Arc<TorrentTracker>,
-    messenger_to_initiator: Sender<ApiReady>,
-) -> impl warp::Future<Output = ()> {
+pub fn start(socket_addr: SocketAddr, tracker: Arc<TorrentTracker>) -> impl warp::Future<Output = ()> {
     // GET /api/torrents?offset=:u32&limit=:u32
     // View torrent list
     let api_torrents = tracker.clone();
@@ -349,11 +343,6 @@ pub fn start(
 
     let server = api_routes.and(authenticate(tracker.config.http_api.access_tokens.clone()));
 
-    // Send a message to the initiator to notify the API is ready to accept requests
-    if messenger_to_initiator.send(ApiReady()).is_err() {
-        panic!("the receiver dropped");
-    }
-
     let (_addr, api_server) = serve(server).bind_with_graceful_shutdown(socket_addr, async move {
         tokio::signal::ctrl_c().await.expect("Failed to listen to shutdown signal.");
     });
diff --git a/src/jobs/tracker_api.rs b/src/jobs/tracker_api.rs
@@ -1,30 +1,40 @@
 use std::sync::Arc;
 
 use log::info;
-use tokio::sync::oneshot::{self, Receiver};
+use tokio::sync::oneshot;
 use tokio::task::JoinHandle;
 
 use crate::api::server;
 use crate::tracker::TorrentTracker;
 use crate::Configuration;
 
 #[derive(Debug)]
-pub struct ApiReady();
+pub struct ApiServerJobStarted();
 
-pub fn start_job(config: &Configuration, tracker: Arc<TorrentTracker>) -> (JoinHandle<()>, Receiver<ApiReady>) {
+pub async fn start_job(config: &Configuration, tracker: Arc<TorrentTracker>) -> JoinHandle<()> {
     let bind_addr = config
         .http_api
         .bind_address
         .parse::<std::net::SocketAddr>()
         .expect("Tracker API bind_address invalid.");
 
-    let (tx, rx) = oneshot::channel::<ApiReady>();
-
     info!("Starting Torrust API server on: {}", bind_addr);
 
+    let (tx, rx) = oneshot::channel::<ApiServerJobStarted>();
+
+    // Run the API server
     let join_handle = tokio::spawn(async move {
-        server::start(bind_addr, tracker, tx).await;
+        if tx.send(ApiServerJobStarted()).is_err() {
+            panic!("the start job dropped");
+        }
+        server::start(bind_addr, tracker).await;
     });
 
-    (join_handle, rx)
+    // Wait until the API server job is running
+    match rx.await {
+        Ok(_msg) => info!("Torrust API server started"),
+        Err(_) => panic!("the api server dropped"),
+    }
+
+    join_handle
 }
diff --git a/src/setup.rs b/src/setup.rs
@@ -49,8 +49,7 @@ pub async fn setup(config: &Configuration, tracker: Arc<TorrentTracker>) -> Vec<
 
     // Start HTTP API server
     if config.http_api.enabled {
-        let (join_handle, _receiver) = tracker_api::start_job(config, tracker.clone());
-        jobs.push(join_handle);
+        jobs.push(tracker_api::start_job(config, tracker.clone()).await);
     }
 
     // Remove torrents without peers, every interval
diff --git a/tests/api.rs b/tests/api.rs
@@ -152,16 +152,9 @@ mod tracker_api {
                 logging::setup_logging(&configuration);
 
                 // Start the HTTP API job
-                let (join_handle, api_receiver) = tracker_api::start_job(&configuration, tracker.clone());
-                self.job = Some(join_handle);
+                self.job = Some(tracker_api::start_job(&configuration, tracker).await);
 
                 self.started.store(true, Ordering::Relaxed);
-
-                // Wait until the API is ready
-                match api_receiver.await {
-                    Ok(msg) => println!("Message received from API server: {:?}", msg),
-                    Err(_) => panic!("the api server dropped"),
-                }
             }
         }
     }

Original file line number	Diff line number	Diff line change
`@@ -49,8 +49,7 @@ pub async fn setup(config: &Configuration, tracker: Arc<TorrentTracker>) -> Vec<`
`49`	`49`
`50`	`50`	`// Start HTTP API server`
`51`	`51`	`if config.http_api.enabled {`
`52`		`- let (join_handle, _receiver) = tracker_api::start_job(config, tracker.clone());`
`53`		`- jobs.push(join_handle);`
	`52`	`+ jobs.push(tracker_api::start_job(config, tracker.clone()).await);`
`54`	`53`	`}`
`55`	`54`
`56`	`55`	`// Remove torrents without peers, every interval`
Original file line number	Diff line number	Diff line change
`@@ -152,16 +152,9 @@ mod tracker_api {`
`152`	`152`	`logging::setup_logging(&configuration);`
`153`	`153`
`154`	`154`	`// Start the HTTP API job`
`155`		`- let (join_handle, api_receiver) = tracker_api::start_job(&configuration, tracker.clone());`
`156`		`- self.job = Some(join_handle);`
	`155`	`+ self.job = Some(tracker_api::start_job(&configuration, tracker).await);`
`157`	`156`
`158`	`157`	`self.started.store(true, Ordering::Relaxed);`
`159`		`-`
`160`		`- // Wait until the API is ready`
`161`		`- match api_receiver.await {`
`162`		`- Ok(msg) => println!("Message received from API server: {:?}", msg),`
`163`		`- Err(_) => panic!("the api server dropped"),`
`164`		`- }`
`165`	`158`	`}`
`166`	`159`	`}`
`167`	`160`	`}`