Radish alpha
r
Radicle CI broker
Radicle
Git (anonymous pull)
Log in to clone via SSH
refactor: can't create a Broker without setting default adapter
Lars Wirzenius committed 1 year ago
commit 2a7ddc303af6684fa5fddcb0ca70b22556d27101
parent 6e52f1da50d3f7b461a92c78f5781993555e4bae
2 files changed +23 -60
modified src/bin/cib.rs
@@ -173,14 +173,14 @@ impl QueuedCmd {
    fn run(&self, args: &Args, config: &Config) -> Result<(), CibError> {
        let profile = Profile::load().map_err(CibError::profile)?;

-
        let mut broker =
-
            Broker::new(config.db(), config.max_run_time()).map_err(CibError::new_broker)?;
        let spec = config.default_adapter()?;
        let adapter = Adapter::new(&spec.command)
            .with_environment(spec.envs())
            .with_sensitive_environment(spec.sensitive_envs());
        logger::adapter_config(config);
-
        broker.set_default_adapter(&adapter);
+

+
        let broker = Broker::new(config.db(), config.max_run_time(), &adapter)
+
            .map_err(CibError::new_broker)?;

        let mut event_notifications = NotificationChannel::new_event();
        event_notifications
@@ -264,14 +264,14 @@ impl ProcessEventsCmd {
            false,
        );

-
        let mut broker =
-
            Broker::new(config.db(), config.max_run_time()).map_err(CibError::new_broker)?;
        let spec = config.default_adapter()?;
        let adapter = Adapter::new(&spec.command)
            .with_environment(spec.envs())
            .with_sensitive_environment(spec.sensitive_envs());
        logger::adapter_config(config);
-
        broker.set_default_adapter(&adapter);
+

+
        let broker = Broker::new(config.db(), config.max_run_time(), &adapter)
+
            .map_err(CibError::new_broker)?;

        let processor = QueueProcessorBuilder::default()
            .events_rx(events_notification.rx().map_err(CibError::notification)?)
modified src/broker.rs
@@ -28,17 +28,21 @@ use crate::{
/// node, and executes the appropriate adapter to run CI on the
/// repository.
pub struct Broker {
-
    default_adapter: Option<Adapter>,
+
    default_adapter: Adapter,
    max_run_time: Duration,
    db: Db,
}

impl Broker {
    #[allow(clippy::result_large_err)]
-
    pub fn new(db_filename: &Path, max_run_time: Duration) -> Result<Self, BrokerError> {
+
    pub fn new(
+
        db_filename: &Path,
+
        max_run_time: Duration,
+
        default_adapter: &Adapter,
+
    ) -> Result<Self, BrokerError> {
        logger::broker_db(db_filename);
        Ok(Self {
-
            default_adapter: None,
+
            default_adapter: default_adapter.clone(),
            max_run_time,
            db: Db::new(db_filename)?,
        })
@@ -49,12 +53,8 @@ impl Broker {
        Ok(self.db.get_all_runs()?)
    }

-
    pub fn set_default_adapter(&mut self, adapter: &Adapter) {
-
        self.default_adapter = Some(adapter.clone());
-
    }
-

-
    pub fn default_adapter(&self) -> Option<&Adapter> {
-
        self.default_adapter.as_ref()
+
    pub fn default_adapter(&self) -> &Adapter {
+
        &self.default_adapter
    }

    #[allow(clippy::result_large_err)]
@@ -65,8 +65,7 @@ impl Broker {
    ) -> Result<Run, BrokerError> {
        let broker_run_id = RunId::default();
        let span = span!(Level::TRACE, "execute_ci_run", %broker_run_id,).entered();
-
        let rid = trigger.repo();
-
        let adapter = self.default_adapter().ok_or(BrokerError::NoAdapter(rid))?;
+
        let adapter = self.default_adapter();
        let run = span
            .in_scope(|| self.execute_helper(adapter, broker_run_id, trigger, run_notification))?;
        Ok(run)
@@ -202,51 +201,17 @@ mod test {
        test::{mock_adapter, trigger_request, TestResult},
    };

-
    fn broker(filename: &Path) -> anyhow::Result<Broker> {
-
        Ok(Broker::new(filename, Duration::from_secs(1))?)
+
    fn broker(filename: &Path, adapter: &Adapter) -> anyhow::Result<Broker> {
+
        Ok(Broker::new(filename, Duration::from_secs(1), adapter)?)
    }

    #[test]
-
    fn has_no_adapters_initially() -> TestResult<()> {
+
    fn has_adapter() -> TestResult<()> {
        let tmp = tempdir()?;
        let db = tmp.path().join("db.db");
-
        let broker = broker(&db)?;
-
        assert_eq!(broker.default_adapter(), None);
-
        Ok(())
-
    }
-

-
    #[test]
-
    fn does_not_have_a_default_adapter_initially() -> TestResult<()> {
-
        let tmp = tempdir()?;
-
        let db = tmp.path().join("db.db");
-
        let broker = broker(&db)?;
-

-
        assert_eq!(broker.default_adapter(), None);
-
        Ok(())
-
    }
-

-
    #[test]
-
    fn sets_a_default_adapter_initially() -> TestResult<()> {
-
        let tmp = tempdir()?;
-
        let db = tmp.path().join("db.db");
-
        let mut broker = broker(&db)?;
-

-
        let adapter = Adapter::default();
-
        broker.set_default_adapter(&adapter);
-
        assert_eq!(broker.default_adapter(), Some(&adapter));
-
        Ok(())
-
    }
-

-
    #[test]
-
    fn finds_default_adapter_for_unknown_repo() -> TestResult<()> {
-
        let tmp = tempdir()?;
-
        let db = tmp.path().join("db.db");
-
        let mut broker = broker(&db)?;
-

        let adapter = Adapter::default();
-
        broker.set_default_adapter(&adapter);
-

-
        assert_eq!(broker.default_adapter(), Some(&adapter));
+
        let broker = broker(&db, &adapter)?;
+
        assert_eq!(broker.default_adapter(), &adapter);
        Ok(())
    }

@@ -264,8 +229,7 @@ echo '{"response":"finished","result":"success"}'

        let tmp = tempdir()?;
        let db = tmp.path().join("db.db");
-
        let mut broker = broker(&db)?;
-
        broker.set_default_adapter(&adapter);
+
        let mut broker = broker(&db, &adapter)?;

        let trigger = trigger_request()?;

@@ -297,8 +261,7 @@ exit 1

        let tmp = tempdir()?;
        let db = tmp.path().join("db.db");
-
        let mut broker = broker(&db)?;
-
        broker.set_default_adapter(&adapter);
+
        let mut broker = broker(&db, &adapter)?;

        let trigger = trigger_request()?;