From b07c732475c9dd9cfd31357e1d816e879c1514b8 Mon Sep 17 00:00:00 2001 From: Valentin Brandl Date: Tue, 13 Jun 2023 09:59:33 +0200 Subject: [PATCH 1/6] Remove usage of outdated actions Closes #506 --- .github/workflows/audit.yml | 2 +- .github/workflows/rust.yml | 37 +++++++++---------------------------- 2 files changed, 10 insertions(+), 29 deletions(-) diff --git a/.github/workflows/audit.yml b/.github/workflows/audit.yml index e9020c1..51b44c5 100644 --- a/.github/workflows/audit.yml +++ b/.github/workflows/audit.yml @@ -16,6 +16,6 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v3 - - uses: actions-rs/audit-check@v1 + - uses: rustsec/audit-check@v1 with: token: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index b25068b..144422c 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -16,18 +16,11 @@ jobs: uses: actions/checkout@v3 - name: Install stable toolchain - uses: actions-rs/toolchain@v1 - with: - toolchain: stable - profile: minimal - override: true - components: rustfmt + uses: dtolnay/rust-toolchain@stable - name: Check Formatting - uses: actions-rs/cargo@v1 - with: - command: fmt - args: --all -- --check + run: cargo fmt --all -- --check + clippy: name: Clippy @@ -37,12 +30,7 @@ jobs: uses: actions/checkout@v3 - name: Install stable toolchain - uses: actions-rs/toolchain@v1 - with: - toolchain: stable - profile: minimal - override: true - components: rustfmt + uses: dtolnay/rust-toolchain@stable - name: Cache cargo registry, index and build directory uses: actions/cache@v3 @@ -54,10 +42,9 @@ jobs: key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }} - name: Clippy Linting - uses: actions-rs/cargo@v1 - with: - command: clippy - args: -- -D warnings + run: cargo clippy --all-features -- -Dclippy::all -Dclippy::pedantic + - name: Clippy Test Linting + run: cargo clippy --tests -- -Dclippy::all -Dclippy::pedantic test: @@ -74,11 +61,7 @@ jobs: uses: actions/checkout@v3 - name: Install stable toolchain - uses: actions-rs/toolchain@v1 - with: - toolchain: stable - profile: minimal - override: true + uses: dtolnay/rust-toolchain@stable - name: Cache cargo registry, index and build directory uses: actions/cache@v3 @@ -90,6 +73,4 @@ jobs: key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }} - name: Run Tests - uses: actions-rs/cargo@v1 - with: - command: test + run: cargo test From 361eacbe1d48ec1b696b3f8ec8237afe13f0b7cc Mon Sep 17 00:00:00 2001 From: Valentin Brandl Date: Tue, 13 Jun 2023 10:22:37 +0200 Subject: [PATCH 2/6] Fix some pedantic lints --- src/cache.rs | 8 +++++--- src/config.rs | 6 ++++++ src/lib.rs | 33 +++++++++++++++++++++++---------- src/service.rs | 20 ++++++++++---------- src/statics.rs | 1 + src/template.rs | 5 +++-- templates/base.rs.html | 2 +- templates/overview.rs.html | 2 +- 8 files changed, 50 insertions(+), 27 deletions(-) diff --git a/src/cache.rs b/src/cache.rs index b96a7eb..716f8ec 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -39,7 +39,10 @@ impl<'a> CacheState<'a> { Ok(cache .entries .get(branch) - .map(|c| { + .map_or_else( + // TODO: get rid of clone +|| CacheState::NoneForBranch(cache.clone()), + |c| { if c.head == head { trace!("Cache is up to date"); CacheState::Current { @@ -57,8 +60,7 @@ impl<'a> CacheState<'a> { } } }) - // TODO: get rid of clone - .unwrap_or_else(|| CacheState::NoneForBranch(cache.clone()))) + ) } else { Ok(CacheState::No) } diff --git a/src/config.rs b/src/config.rs index 9b6f706..59e425f 100644 --- a/src/config.rs +++ b/src/config.rs @@ -18,6 +18,12 @@ pub struct Settings { } impl Settings { + /// Load the configuration from file and environment. + /// + /// # Errors + /// + /// * File cannot be read or parsed + /// * Environment variables cannot be parsed pub fn load() -> Result { Config::builder() .add_source(File::with_name("hoc.toml").required(false)) diff --git a/src/lib.rs b/src/lib.rs index d24abf8..efb230f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -22,7 +22,7 @@ use crate::{ cache::CacheState, config::Settings, error::{Error, Result}, - service::{Bitbucket, FormService, GitHub, Gitlab, Service, Sourcehut}, + service::{Bitbucket, FormValue, GitHub, Gitlab, Service, Sourcehut}, statics::{CLIENT, VERSION_INFO}, template::{RepoGeneratorInfo, RepoInfo}, }; @@ -53,7 +53,7 @@ include!(concat!(env!("OUT_DIR"), "/templates.rs")); #[derive(Deserialize, Serialize)] struct GeneratorForm<'a> { - service: FormService, + service: FormValue, user: Cow<'a, str>, repo: Cow<'a, str>, branch: Option>, @@ -309,7 +309,7 @@ pub(crate) async fn json_hoc( let branch = branch.branch.as_deref().unwrap_or("master"); let rc_clone = repo_count.clone(); let mapper = move |r| match r { - HocResult::NotFound => p404(rc_clone), + HocResult::NotFound => p404(&rc_clone), HocResult::Hoc { hoc, head, commits, .. } => Ok(HttpResponse::Ok().json(JsonResponse { @@ -345,7 +345,7 @@ pub(crate) async fn calculate_hoc( let rc_clone = repo_count.clone(); let label = query.label.clone(); let mapper = move |r| match r { - HocResult::NotFound => p404(rc_clone), + HocResult::NotFound => p404(&rc_clone), HocResult::Hoc { hoc_pretty, .. } => { let badge_opt = BadgeOptions { subject: label, @@ -386,7 +386,7 @@ async fn overview( let base_url = state.settings.base_url.clone(); let rc_clone = repo_count.clone(); let mapper = move |r| match r { - HocResult::NotFound => p404(rc_clone), + HocResult::NotFound => p404(&rc_clone), HocResult::Hoc { hoc, commits, @@ -413,7 +413,7 @@ async fn overview( VERSION_INFO, rc_clone.load(Ordering::Relaxed), repo_info, - label, + &label, )?; Ok(HttpResponse::Ok().content_type("text/html").body(buf)) @@ -423,11 +423,13 @@ async fn overview( } #[get("/health_check")] +#[allow(clippy::unused_async)] async fn health_check() -> HttpResponse { HttpResponse::Ok().finish() } #[get("/")] +#[allow(clippy::unused_async)] async fn index( state: web::Data, repo_count: web::Data, @@ -443,6 +445,7 @@ async fn index( } #[post("/generate")] +#[allow(clippy::unused_async)] async fn generate( params: web::Form>, state: web::Data, @@ -470,20 +473,22 @@ async fn generate( Ok(HttpResponse::Ok().content_type("text/html").body(buf)) } -fn p404(repo_count: web::Data) -> Result { +fn p404(repo_count: &web::Data) -> Result { let mut buf = Vec::new(); templates::p404_html(&mut buf, VERSION_INFO, repo_count.load(Ordering::Relaxed))?; Ok(HttpResponse::NotFound().content_type("text/html").body(buf)) } +#[allow(clippy::unused_async)] async fn async_p404(repo_count: web::Data) -> Result { - p404(repo_count) + p404(&repo_count) } /// A duration to add to current time for a far expires header. static FAR: Duration = Duration::from_secs(180 * 24 * 60 * 60); #[get("/static/{filename}")] +#[allow(clippy::unused_async)] async fn static_file( path: web::Path, repo_count: web::Data, @@ -496,11 +501,13 @@ async fn static_file( .content_type(data.mime.clone()) .body(data.content) }) - .map(Result::Ok) - .unwrap_or_else(|| p404(repo_count)) + .map_or_else( + || p404(&repo_count), + Result::Ok) } #[get("/favicon.ico")] +#[allow(clippy::unused_async)] async fn favicon32() -> HttpResponse { let data = &template_statics::favicon32_png; HttpResponse::Ok() @@ -508,6 +515,7 @@ async fn favicon32() -> HttpResponse { .body(data.content) } +#[allow(clippy::unused_async)] async fn start_server(listener: TcpListener, settings: Settings) -> std::io::Result { let workers = settings.workers; let repo_count = @@ -536,6 +544,11 @@ async fn start_server(listener: TcpListener, settings: Settings) -> std::io::Res .run()) } +/// Start the server. +/// +/// # Errors +/// +/// * server cannot bind to `listener` pub async fn run(listener: TcpListener, settings: Settings) -> std::io::Result { let span = info_span!("hoc", version = env!("CARGO_PKG_VERSION")); let _ = span.enter(); diff --git a/src/service.rs b/src/service.rs index 531bd9b..509e05e 100644 --- a/src/service.rs +++ b/src/service.rs @@ -29,7 +29,7 @@ pub(crate) trait Service: Sized + 'static { } #[derive(Deserialize, Serialize, Clone, Copy)] -pub enum FormService { +pub enum FormValue { #[serde(rename = "github")] GitHub, #[serde(rename = "gitlab")] @@ -40,22 +40,22 @@ pub enum FormService { Sourcehut, } -impl FormService { +impl FormValue { pub(crate) fn url(&self) -> &str { match self { - FormService::GitHub => "github.com", - FormService::Gitlab => "gitlab.com", - FormService::Bitbucket => "bitbucket.org", - FormService::Sourcehut => "git.sr.ht", + FormValue::GitHub => "github.com", + FormValue::Gitlab => "gitlab.com", + FormValue::Bitbucket => "bitbucket.org", + FormValue::Sourcehut => "git.sr.ht", } } pub(crate) fn service(&self) -> &str { match self { - FormService::GitHub => "github", - FormService::Gitlab => "gitlab", - FormService::Bitbucket => "bitbucket", - FormService::Sourcehut => "sourcehut", + FormValue::GitHub => "github", + FormValue::Gitlab => "gitlab", + FormValue::Bitbucket => "bitbucket", + FormValue::Sourcehut => "sourcehut", } } } diff --git a/src/statics.rs b/src/statics.rs index 3e799f2..82ae98f 100644 --- a/src/statics.rs +++ b/src/statics.rs @@ -1,3 +1,4 @@ +#[derive(Clone, Copy)] pub struct VersionInfo<'a> { pub commit: &'a str, pub version: &'a str, diff --git a/src/template.rs b/src/template.rs index d90b05d..05918c4 100644 --- a/src/template.rs +++ b/src/template.rs @@ -1,5 +1,6 @@ -use crate::service::FormService; +use crate::service::FormValue; +#[derive(Clone, Copy)] pub struct RepoInfo<'a> { pub commit_url: &'a str, pub commits: u64, @@ -13,7 +14,7 @@ pub struct RepoInfo<'a> { } pub struct RepoGeneratorInfo<'a> { - pub service: FormService, + pub service: FormValue, pub user: &'a str, pub repo: &'a str, pub branch: &'a str, diff --git a/templates/base.rs.html b/templates/base.rs.html index da9a3cf..e42295c 100644 --- a/templates/base.rs.html +++ b/templates/base.rs.html @@ -1,4 +1,4 @@ -@use super::statics::*; +@use super::statics::tacit_css_min_css; @use crate::statics::VersionInfo; @(title: &str, header: &str, content: Content, version_info: VersionInfo, repo_count: usize) diff --git a/templates/overview.rs.html b/templates/overview.rs.html index 578da5d..0f261db 100644 --- a/templates/overview.rs.html +++ b/templates/overview.rs.html @@ -2,7 +2,7 @@ @use crate::statics::VersionInfo; @use crate::template::RepoInfo; -@(version_info: VersionInfo, repo_count: usize, repo_info: RepoInfo, label: String) +@(version_info: VersionInfo, repo_count: usize, repo_info: RepoInfo, label: &str) @:base_html("Hits-of-Code Badges", "Overview", { From 4ce8c9e52390043dbf268d15a096dddd8feddbc1 Mon Sep 17 00:00:00 2001 From: Valentin Brandl Date: Tue, 13 Jun 2023 10:23:09 +0200 Subject: [PATCH 3/6] Disable pedantic lints for now --- .github/workflows/rust.yml | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 144422c..adec173 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -42,9 +42,13 @@ jobs: key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }} - name: Clippy Linting - run: cargo clippy --all-features -- -Dclippy::all -Dclippy::pedantic + run: cargo clippy --all-features -- -Dclippy::all + # TODO + # run: cargo clippy --all-features -- -Dclippy::all -Dclippy::pedantic - name: Clippy Test Linting - run: cargo clippy --tests -- -Dclippy::all -Dclippy::pedantic + run: cargo clippy --tests -- -Dclippy::all + # TODO + # run: cargo clippy --tests -- -Dclippy::all -Dclippy::pedantic test: From 9f3c2813abf8e43c8fc549348b139cb7b8a3d92e Mon Sep 17 00:00:00 2001 From: Valentin Brandl Date: Tue, 13 Jun 2023 10:26:25 +0200 Subject: [PATCH 4/6] Apply cargo fmt --- src/cache.rs | 13 +++++-------- src/lib.rs | 4 +--- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/src/cache.rs b/src/cache.rs index 716f8ec..b8fc803 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -36,13 +36,10 @@ impl<'a> CacheState<'a> { trace!("Reading cache"); if path.as_ref().exists() { let cache: Cache = serde_json::from_reader(BufReader::new(File::open(path)?))?; - Ok(cache - .entries - .get(branch) - .map_or_else( + Ok(cache.entries.get(branch).map_or_else( // TODO: get rid of clone -|| CacheState::NoneForBranch(cache.clone()), - |c| { + || CacheState::NoneForBranch(cache.clone()), + |c| { if c.head == head { trace!("Cache is up to date"); CacheState::Current { @@ -59,8 +56,8 @@ impl<'a> CacheState<'a> { cache: cache.clone(), } } - }) - ) + }, + )) } else { Ok(CacheState::No) } diff --git a/src/lib.rs b/src/lib.rs index efb230f..0884d6e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -501,9 +501,7 @@ async fn static_file( .content_type(data.mime.clone()) .body(data.content) }) - .map_or_else( - || p404(&repo_count), - Result::Ok) + .map_or_else(|| p404(&repo_count), Result::Ok) } #[get("/favicon.ico")] From a64f55f362c87172ab5c39b635fb394f4a58fa8c Mon Sep 17 00:00:00 2001 From: Valentin Brandl Date: Tue, 13 Jun 2023 10:42:53 +0200 Subject: [PATCH 5/6] Properly await the server future --- tests/util/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/util/mod.rs b/tests/util/mod.rs index 296f983..54f2e87 100644 --- a/tests/util/mod.rs +++ b/tests/util/mod.rs @@ -38,7 +38,7 @@ pub async fn spawn_app() -> TestApp { .await .expect("Failed to bind address"); - let _ = tokio::spawn(server); + let _ = tokio::spawn(server).await; TestApp { address, From 87bae1bbd16e739d5ebcd770551829857d4a8ab9 Mon Sep 17 00:00:00 2001 From: Valentin Brandl Date: Tue, 13 Jun 2023 11:09:54 +0200 Subject: [PATCH 6/6] Fix blocking test --- tests/util/mod.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/util/mod.rs b/tests/util/mod.rs index 54f2e87..371268b 100644 --- a/tests/util/mod.rs +++ b/tests/util/mod.rs @@ -38,7 +38,9 @@ pub async fn spawn_app() -> TestApp { .await .expect("Failed to bind address"); - let _ = tokio::spawn(server).await; + #[allow(clippy::let_underscore_future)] + // don't await so the test server runs in the background + let _ = tokio::spawn(server); TestApp { address,