refactor(webserver): ensure only services could rely on tabby_db dire… (

#1407) * refactor(webserver): ensure only services could rely on tabby_db directly * update * update * update * update * update * update * update * update
TabbyML · Feb 7, 2024 · 2fd2a12 · 2fd2a12
1 parent 5158eec
commit 2fd2a12
Show file tree

Hide file tree

Showing 10 changed files with 81 additions and 30 deletions.
diff --git a/.github/workflows/ast-grep-lint.yml b/.github/workflows/ast-grep-lint.yml
@@ -0,0 +1,13 @@
+name: ast-grep lint
+on: [push]
+
+jobs:
+  sg-lint:
+    runs-on: ubuntu-latest
+    name: ast-grep-lint
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v4
+
+      - name: ast-grep lint step
+        uses: ast-grep/[email protected]
diff --git a/.github/workflows/test-rust.yml b/.github/workflows/test-rust.yml
@@ -32,9 +32,6 @@ jobs:
         with:
           submodules: recursive
 
-      - name: Service shouldn't depends on juniper::ID
-        run:  bash -c '! grep "juniper::ID" -r ./ee/tabby-webserver/src/service'
-
       - name: Install Rust
         uses: actions-rs/toolchain@v1
         with:

diff --git a/ee/tabby-webserver/src/schema/auth.rs b/ee/tabby-webserver/src/schema/auth.rs
@@ -9,7 +9,6 @@ use juniper_axum::relay;
 use lazy_static::lazy_static;
 use serde::{Deserialize, Serialize};
 use tabby_common::terminal::{HeaderFormat, InfoMessage};
-use tabby_db::InvitationDAO;
 use thiserror::Error;
 use tracing::{error, warn};
 use uuid::Uuid;
@@ -366,7 +365,7 @@ pub trait AuthenticationService: Send + Sync {
     async fn is_admin_initialized(&self) -> Result<bool>;
     async fn get_user_by_email(&self, email: &str) -> Result<User>;
 
-    async fn create_invitation(&self, email: String) -> Result<InvitationDAO>;
+    async fn create_invitation(&self, email: String) -> Result<Invitation>;
     async fn delete_invitation(&self, id: i32) -> Result<i32>;
 
     async fn reset_user_auth_token(&self, email: &str) -> Result<()>;

diff --git a/ee/tabby-webserver/src/schema/mod.rs b/ee/tabby-webserver/src/schema/mod.rs
@@ -1,5 +1,4 @@
 pub mod auth;
-mod dao;
 pub mod email;
 pub mod job;
 pub mod repository;
@@ -21,7 +20,6 @@ use juniper_axum::{
     FromAuth,
 };
 use tabby_common::api::{code::CodeSearch, event::RawEventLogger};
-use tabby_db::{DbConn, InvalidIDError};
 use tracing::{error, warn};
 use validator::ValidationErrors;
 use worker::{Worker, WorkerService};
@@ -30,9 +28,12 @@ use self::{
     email::{EmailService, EmailSetting},
     repository::{RepositoryError, RepositoryService},
 };
-use crate::schema::{
-    auth::{JWTPayload, OAuthCredential, OAuthProvider},
-    repository::Repository,
+use crate::{
+    schema::{
+        auth::{JWTPayload, OAuthCredential, OAuthProvider},
+        repository::Repository,
+    },
+    service::{IntoID, IntoRowid},
 };
 
 pub trait ServiceLocator: Send + Sync {
@@ -66,9 +67,6 @@ pub enum CoreError {
 
     #[error(transparent)]
     Other(#[from] anyhow::Error),
-
-    #[error("Malformed ID input")]
-    InvalidID(#[from] InvalidIDError),
 }
 
 impl<S: ScalarValue> IntoFieldError<S> for CoreError {
@@ -305,7 +303,7 @@ impl Mutation {
     async fn update_user_active(ctx: &Context, id: ID, active: bool) -> Result<bool> {
         ctx.locator
             .auth()
-            .update_user_active(DbConn::to_rowid(&id)?, active)
+            .update_user_active(id.into_rowid()?, active)
             .await?;
         Ok(true)
     }
@@ -359,7 +357,7 @@ impl Mutation {
                 "Failed to send invitation email, please check your SMTP settings are correct: {e}"
             );
         }
-        Ok(ID::new(DbConn::to_id(invitation.id)))
+        Ok(invitation.id)
     }
 
     async fn create_repository(
@@ -371,14 +369,14 @@ impl Mutation {
             .repository()
             .create_repository(name, git_url)
             .await
-            .map(|x| ID::new(DbConn::to_id(x)))
+            .map(|x| x.into_id())
     }
 
     async fn delete_repository(ctx: &Context, id: ID) -> Result<bool> {
         Ok(ctx
             .locator
             .repository()
-            .delete_repository(DbConn::to_rowid(&id)?)
+            .delete_repository(id.into_rowid()?)
             .await?)
     }
 
@@ -391,19 +389,19 @@ impl Mutation {
         Ok(ctx
             .locator
             .repository()
-            .update_repository(DbConn::to_rowid(&id)?, name, git_url)
+            .update_repository(id.into_rowid()?, name, git_url)
             .await?)
     }
 
     async fn delete_invitation(ctx: &Context, id: ID) -> Result<ID> {
         if let Some(claims) = &ctx.claims {
             if claims.is_admin {
-                return Ok(ID::new(DbConn::to_id(
-                    ctx.locator
-                        .auth()
-                        .delete_invitation(DbConn::to_rowid(&id)?)
-                        .await?,
-                )));
+                return Ok(ctx
+                    .locator
+                    .auth()
+                    .delete_invitation(id.into_rowid()?)
+                    .await?
+                    .into_id());
             }
         }
         Err(CoreError::Unauthorized(

diff --git a/ee/tabby-webserver/src/service/auth.rs b/ee/tabby-webserver/src/service/auth.rs
@@ -7,7 +7,7 @@ use argon2::{
     Argon2, PasswordHasher, PasswordVerifier,
 };
 use async_trait::async_trait;
-use tabby_db::{DbConn, InvitationDAO};
+use tabby_db::DbConn;
 use validator::{Validate, ValidationError};
 
 use super::graphql_pagination_to_filter;
@@ -296,9 +296,9 @@ impl AuthenticationService for DbConn {
         }
     }
 
-    async fn create_invitation(&self, email: String) -> Result<InvitationDAO> {
+    async fn create_invitation(&self, email: String) -> Result<Invitation> {
         let invitation = self.create_invitation(email).await?;
-        Ok(invitation)
+        Ok(invitation.into())
     }
 
     async fn delete_invitation(&self, id: i32) -> Result<i32> {

diff --git a/ee/tabby-webserver/src/schema/dao.rs → ee/tabby-webserver/src/service/dao.rs b/ee/tabby-webserver/src/schema/dao.rs → ee/tabby-webserver/src/service/dao.rs
@@ -4,11 +4,11 @@ use tabby_db::{
     JobRunDAO, RepositoryDAO, UserDAO,
 };
 
-use super::{email::EmailSetting, repository::Repository};
 use crate::schema::{
-    auth,
-    auth::{OAuthCredential, OAuthProvider},
+    auth::{self, OAuthCredential, OAuthProvider},
+    email::EmailSetting,
     job,
+    repository::Repository,
 };
 
 impl From<InvitationDAO> for auth::Invitation {
@@ -94,3 +94,23 @@ impl From<EmailSettingDAO> for EmailSetting {
         }
     }
 }
+
+pub trait IntoRowid {
+    fn into_rowid(self) -> anyhow::Result<i32>;
+}
+
+impl IntoRowid for juniper::ID {
+    fn into_rowid(self) -> anyhow::Result<i32> {
+        DbConn::to_rowid(&self).map_err(|_| anyhow::anyhow!("Malformed ID input"))
+    }
+}
+
+pub trait IntoID {
+    fn into_id(self) -> juniper::ID;
+}
+
+impl IntoID for i32 {
+    fn into_id(self) -> juniper::ID {
+        juniper::ID::new(DbConn::to_id(self))
+    }
+}
diff --git a/ee/tabby-webserver/src/service/mod.rs b/ee/tabby-webserver/src/service/mod.rs
@@ -1,4 +1,5 @@
 mod auth;
+mod dao;
 mod email;
 mod job;
 mod proxy;
@@ -14,6 +15,7 @@ use axum::{
     middleware::Next,
     response::IntoResponse,
 };
+pub use dao::{IntoID, IntoRowid};
 use hyper::{client::HttpConnector, Body, Client, StatusCode};
 use tabby_common::{
     api::{code::CodeSearch, event::RawEventLogger},

diff --git a/rules/only-service-can-depend-tabby-db.yml b/rules/only-service-can-depend-tabby-db.yml
@@ -0,0 +1,10 @@
+id: only-service-can-depend-tabby-db
+message: Only service can depend on tabby-db
+severity: error
+language: rust
+files:
+- ./ee/tabby-webserver/src/**
+ignores:
+- ./ee/tabby-webserver/src/service/**
+rule:
+  pattern: tabby_db
diff --git a/rules/service-shoudnt-depend-on-juniper-id.yml b/rules/service-shoudnt-depend-on-juniper-id.yml
@@ -0,0 +1,10 @@
+id: service-shoudnt-depend-on-juniper-id
+message: Don't use juniper::ID in services.
+severity: error
+language: rust
+files:
+- ./ee/tabby-webserver/src/service/**
+ignores:
+- ./ee/tabby-webserver/src/service/dao.rs
+rule:
+  pattern: juniper::ID
diff --git a/sgconfig.yml b/sgconfig.yml
@@ -0,0 +1,2 @@
+ruleDirs:
+- rules