Merge pull request #9946 from Turbo87/github-mock

github: Use mockall to generate `MockGitHubClient` implementation

Author: Turbo87

Cargo.lock

@@ -122,6 +122,7 @@ unicode-xid = "=0.2.6"
 
 [dev-dependencies]
 bytes = "=1.8.0"
+crates_io_github = { path = "crates/crates_io_github", features = ["mock"] }
 crates_io_index = { path = "crates/crates_io_index", features = ["testing"] }
 crates_io_tarball = { path = "crates/crates_io_tarball", features = ["builder"] }
 crates_io_team_repo = { path = "crates/crates_io_team_repo", features = ["mock"] }

Cargo.toml

@@ -7,9 +7,13 @@ edition = "2021"
 [lints]
 workspace = true
 
+[features]
+mock = ["dep:mockall"]
+
 [dependencies]
 anyhow = "=1.0.93"
 async-trait = "=0.1.83"
+mockall = { version = "=0.13.0", optional = true }
 oauth2 = { version = "=4.4.2", default-features = false }
 reqwest = { version = "=0.12.9", features = ["json"] }
 serde = { version = "=1.0.215", features = ["derive"] }

crates/crates_io_github/Cargo.toml

@@ -16,6 +16,7 @@ use serde::Deserialize;
 
 type Result<T> = std::result::Result<T, GitHubError>;
 
+#[cfg_attr(feature = "mock", mockall::automock)]
 #[async_trait]
 pub trait GitHubClient: Send + Sync {
     async fn current_user(&self, auth: &AccessToken) -> Result<GithubUser>;

crates/crates_io_github/src/lib.rs

@@ -2,6 +2,7 @@ use crate::tests::util::MockRequestExt;
 use crate::tests::{RequestHelper, TestApp};
 use crate::util::token::HashedToken;
 use crate::{models::ApiToken, schema::api_tokens};
+use crates_io_github::{GitHubPublicKey, MockGitHubClient};
 use diesel::prelude::*;
 use diesel_async::RunQueryDsl;
 use googletest::prelude::*;
@@ -13,13 +14,34 @@ static URL: &str = "/api/github/secret-scanning/verify";
 // Test request and signature from https://docs.github.com/en/developers/overview/secret-scanning-partner-program#create-a-secret-alert-service
 static GITHUB_ALERT: &[u8] =
     br#"[{"token":"some_token","type":"some_type","url":"some_url","source":"some_source"}]"#;
+
 static GITHUB_PUBLIC_KEY_IDENTIFIER: &str =
     "f9525bf080f75b3506ca1ead061add62b8633a346606dc5fe544e29231c6ee0d";
+
+/// Test key from https://docs.github.com/en/developers/overview/secret-scanning-partner-program#create-a-secret-alert-service
+static GITHUB_PUBLIC_KEY: &str = "-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEsz9ugWDj5jK5ELBK42ynytbo38gP\nHzZFI03Exwz8Lh/tCfL3YxwMdLjB+bMznsanlhK0RwcGP3IDb34kQDIo3Q==\n-----END PUBLIC KEY-----";
+
 static GITHUB_PUBLIC_KEY_SIGNATURE: &str = "MEUCIFLZzeK++IhS+y276SRk2Pe5LfDrfvTXu6iwKKcFGCrvAiEAhHN2kDOhy2I6eGkOFmxNkOJ+L2y8oQ9A2T9GGJo6WJY=";
 
+fn github_mock() -> MockGitHubClient {
+    let mut mock = MockGitHubClient::new();
+
+    mock.expect_public_keys().returning(|_, _| {
+        let key = GitHubPublicKey {
+            key_identifier: GITHUB_PUBLIC_KEY_IDENTIFIER.to_string(),
+            key: GITHUB_PUBLIC_KEY.to_string(),
+            is_current: true,
+        };
+
+        Ok(vec![key])
+    });
+
+    mock
+}
+
 #[tokio::test(flavor = "multi_thread")]
 async fn github_secret_alert_revokes_token() {
-    let (app, anon, user, token) = TestApp::init().with_token();
+    let (app, anon, user, token) = TestApp::init().with_github(github_mock()).with_token();
     let mut conn = app.async_db_conn().await;
 
     // Ensure no emails were sent up to this point
@@ -77,7 +99,7 @@ async fn github_secret_alert_revokes_token() {
 
 #[tokio::test(flavor = "multi_thread")]
 async fn github_secret_alert_for_revoked_token() {
-    let (app, anon, user, token) = TestApp::init().with_token();
+    let (app, anon, user, token) = TestApp::init().with_github(github_mock()).with_token();
     let mut conn = app.async_db_conn().await;
 
     // Ensure no emails were sent up to this point
@@ -138,7 +160,7 @@ async fn github_secret_alert_for_revoked_token() {
 
 #[tokio::test(flavor = "multi_thread")]
 async fn github_secret_alert_for_unknown_token() {
-    let (app, anon, user, token) = TestApp::init().with_token();
+    let (app, anon, user, token) = TestApp::init().with_github(github_mock()).with_token();
     let mut conn = app.async_db_conn().await;
 
     // Ensure no emails were sent up to this point
@@ -180,7 +202,7 @@ async fn github_secret_alert_for_unknown_token() {
 
 #[tokio::test(flavor = "multi_thread")]
 async fn github_secret_alert_invalid_signature_fails() {
-    let (_, anon) = TestApp::init().empty();
+    let (_, anon) = TestApp::init().with_github(github_mock()).empty();
 
     // No headers or request body
     let request = anon.post_request(URL);

src/tests/github_secret_scanning.rs

@@ -1,10 +1,8 @@
 use anyhow::anyhow;
-use async_trait::async_trait;
 use crates_io_github::{
-    GitHubClient, GitHubError, GitHubOrgMembership, GitHubOrganization, GitHubPublicKey,
-    GitHubTeam, GitHubTeamMembership, GithubUser,
+    GitHubError, GitHubOrgMembership, GitHubOrganization, GitHubTeam, GitHubTeamMembership,
+    GithubUser, MockGitHubClient,
 };
-use oauth2::AccessToken;
 use std::sync::atomic::{AtomicUsize, Ordering};
 
 static NEXT_GH_ID: AtomicUsize = AtomicUsize::new(0);
@@ -51,30 +49,34 @@ pub(crate) const MOCK_GITHUB_DATA: MockData = MockData {
             email: "[email protected]",
         },
     ],
-    // Test key from https://docs.github.com/en/developers/overview/secret-scanning-partner-program#create-a-secret-alert-service
-    public_keys: &[
-        MockPublicKey {
-            key_identifier: "f9525bf080f75b3506ca1ead061add62b8633a346606dc5fe544e29231c6ee0d",
-            key: "-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEsz9ugWDj5jK5ELBK42ynytbo38gP\nHzZFI03Exwz8Lh/tCfL3YxwMdLjB+bMznsanlhK0RwcGP3IDb34kQDIo3Q==\n-----END PUBLIC KEY-----",
-            is_current: true,
-        },
-    ],
 };
 
-pub(crate) struct MockGitHubClient {
-    data: &'static MockData,
-}
+impl MockData {
+    pub fn as_mock_client(&'static self) -> MockGitHubClient {
+        let mut mock = MockGitHubClient::new();
+
+        mock.expect_current_user()
+            .returning(|_auth| self.current_user());
+
+        mock.expect_org_by_name()
+            .returning(|org_name, _auth| self.org_by_name(org_name));
 
-impl MockGitHubClient {
-    pub(crate) fn new(data: &'static MockData) -> Self {
-        Self { data }
+        mock.expect_team_by_name()
+            .returning(|org_name, team_name, _auth| self.team_by_name(org_name, team_name));
+
+        mock.expect_team_membership()
+            .returning(|org_id, team_id, username, _auth| {
+                self.team_membership(org_id, team_id, username)
+            });
+
+        mock.expect_org_membership()
+            .returning(|org_id, username, _auth| self.org_membership(org_id, username));
+
+        mock
     }
-}
 
-#[async_trait]
-impl GitHubClient for MockGitHubClient {
-    async fn current_user(&self, _auth: &AccessToken) -> Result<GithubUser, GitHubError> {
-        let user = &self.data.users[0];
+    fn current_user(&self) -> Result<GithubUser, GitHubError> {
+        let user = &self.users[0];
         Ok(GithubUser {
             id: user.id,
             login: user.login.into(),
@@ -84,13 +86,8 @@ impl GitHubClient for MockGitHubClient {
         })
     }
 
-    async fn org_by_name(
-        &self,
-        org_name: &str,
-        _auth: &AccessToken,
-    ) -> Result<GitHubOrganization, GitHubError> {
+    fn org_by_name(&self, org_name: &str) -> Result<GitHubOrganization, GitHubError> {
         let org = self
-            .data
             .orgs
             .iter()
             .find(|org| org.name == org_name.to_lowercase())
@@ -101,14 +98,8 @@ impl GitHubClient for MockGitHubClient {
         })
     }
 
-    async fn team_by_name(
-        &self,
-        org_name: &str,
-        team_name: &str,
-        auth: &AccessToken,
-    ) -> Result<GitHubTeam, GitHubError> {
+    fn team_by_name(&self, org_name: &str, team_name: &str) -> Result<GitHubTeam, GitHubError> {
         let team = self
-            .data
             .orgs
             .iter()
             .find(|org| org.name == org_name.to_lowercase())
@@ -120,19 +111,17 @@ impl GitHubClient for MockGitHubClient {
         Ok(GitHubTeam {
             id: team.id,
             name: Some(team.name.into()),
-            organization: self.org_by_name(org_name, auth).await?,
+            organization: self.org_by_name(org_name)?,
         })
     }
 
-    async fn team_membership(
+    fn team_membership(
         &self,
         org_id: i32,
         team_id: i32,
         username: &str,
-        _auth: &AccessToken,
     ) -> Result<GitHubTeamMembership, GitHubError> {
         let team = self
-            .data
             .orgs
             .iter()
             .find(|org| org.id == org_id)
@@ -150,14 +139,12 @@ impl GitHubClient for MockGitHubClient {
         }
     }
 
-    async fn org_membership(
+    fn org_membership(
         &self,
         org_id: i32,
         username: &str,
-        _auth: &AccessToken,
     ) -> Result<GitHubOrgMembership, GitHubError> {
         let org = self
-            .data
             .orgs
             .iter()
             .find(|org| org.id == org_id)
@@ -180,14 +167,6 @@ impl GitHubClient for MockGitHubClient {
             Err(not_found())
         }
     }
-
-    async fn public_keys(
-        &self,
-        _username: &str,
-        _password: &str,
-    ) -> Result<Vec<GitHubPublicKey>, GitHubError> {
-        Ok(self.data.public_keys.iter().map(Into::into).collect())
-    }
 }
 
 fn not_found() -> GitHubError {
@@ -197,7 +176,6 @@ fn not_found() -> GitHubError {
 pub(crate) struct MockData {
     orgs: &'static [MockOrg],
     users: &'static [MockUser],
-    public_keys: &'static [MockPublicKey],
 }
 
 struct MockUser {
@@ -219,19 +197,3 @@ struct MockTeam {
     name: &'static str,
     members: &'static [&'static str],
 }
-
-struct MockPublicKey {
-    key_identifier: &'static str,
-    key: &'static str,
-    is_current: bool,
-}
-
-impl From<&'static MockPublicKey> for GitHubPublicKey {
-    fn from(k: &'static MockPublicKey) -> Self {
-        Self {
-            key_identifier: k.key_identifier.to_string(),
-            key: k.key.to_string(),
-            is_current: k.is_current,
-        }
-    }
-}

src/tests/util/github.rs

@@ -9,9 +9,10 @@ use crate::rate_limiter::{LimitedAction, RateLimiterConfig};
 use crate::schema::users;
 use crate::storage::StorageConfig;
 use crate::tests::util::chaosproxy::ChaosProxy;
-use crate::tests::util::github::{MockGitHubClient, MOCK_GITHUB_DATA};
+use crate::tests::util::github::MOCK_GITHUB_DATA;
 use crate::worker::{Environment, RunnerExt};
 use crate::{App, Emails, Env};
+use crates_io_github::MockGitHubClient;
 use crates_io_index::testing::UpstreamIndex;
 use crates_io_index::{Credentials, RepositoryConfig};
 use crates_io_team_repo::MockTeamRepo;
@@ -103,6 +104,7 @@ impl TestApp {
             build_job_runner: false,
             use_chaos_proxy: false,
             team_repo: MockTeamRepo::new(),
+            github: None,
         }
     }
 
@@ -252,6 +254,7 @@ pub struct TestAppBuilder {
     build_job_runner: bool,
     use_chaos_proxy: bool,
     team_repo: MockTeamRepo,
+    github: Option<MockGitHubClient>,
 }
 
 impl TestAppBuilder {
@@ -296,7 +299,7 @@ impl TestAppBuilder {
             (primary_proxy, replica_proxy)
         };
 
-        let (app, router) = build_app(self.config);
+        let (app, router) = build_app(self.config, self.github);
 
         let runner = if self.build_job_runner {
             let index = self
@@ -397,6 +400,11 @@ impl TestAppBuilder {
         self
     }
 
+    pub fn with_github(mut self, github: MockGitHubClient) -> Self {
+        self.github = Some(github);
+        self
+    }
+
     pub fn with_team_repo(mut self, team_repo: MockTeamRepo) -> Self {
         self.team_repo = team_repo;
         self
@@ -487,14 +495,13 @@ fn simple_config() -> config::Server {
     }
 }
 
-fn build_app(config: config::Server) -> (Arc<App>, axum::Router) {
+fn build_app(config: config::Server, github: Option<MockGitHubClient>) -> (Arc<App>, axum::Router) {
     // Use the in-memory email backend for all tests, allowing tests to analyze the emails sent by
     // the application. This will also prevent cluttering the filesystem.
     let emails = Emails::new_in_memory();
 
-    // Use a custom mock for the GitHub client, allowing to define the GitHub users and
-    // organizations without actually having to create GitHub accounts.
-    let github = Box::new(MockGitHubClient::new(&MOCK_GITHUB_DATA));
+    let github = github.unwrap_or_else(|| MOCK_GITHUB_DATA.as_mock_client());
+    let github = Box::new(github);
 
     let app = App::new(config, emails, github);