From 9f09b07de9eef7b71b8306ff4bdeaa4264d58810 Mon Sep 17 00:00:00 2001 From: Andras Schmelczer Date: Sun, 16 Mar 2025 15:38:13 +0000 Subject: [PATCH] Lint --- README.md | 71 +++++++++--------- backend/Cargo.toml | 16 ++++ backend/reconcile/src/diffs/myers.rs | 1 - backend/reconcile/src/diffs/raw_operation.rs | 6 +- .../operation_transformation/edited_text.rs | 1 + .../src/operation_transformation/operation.rs | 16 ++-- backend/reconcile/src/utils/merge_iters.rs | 3 +- backend/sync_lib/src/lib.rs | 44 +++++++++++ backend/sync_server/src/config.rs | 9 ++- backend/sync_server/src/database.rs | 2 +- backend/sync_server/src/errors.rs | 17 +++-- backend/sync_server/src/server.rs | 73 +++++++++++-------- .../src/server/fetch_document_version.rs | 14 ++-- .../server/fetch_document_version_content.rs | 14 ++-- .../server/fetch_latest_document_version.rs | 14 ++-- .../sync_server/src/server/update_document.rs | 4 +- 16 files changed, 191 insertions(+), 114 deletions(-) diff --git a/README.md b/README.md index 280e65c0..6ad857e0 100644 --- a/README.md +++ b/README.md @@ -1,57 +1,56 @@ -## VaultLink self-hosted Obsidian sync plugin +# VaultLink self-hosted Obsidian plugin for file syncing [![Check](https://github.com/schmelczer/vault-link/actions/workflows/check.yml/badge.svg)](https://github.com/schmelczer/vault-link/actions/workflows/check.yml) +[![E2E tests](https://github.com/schmelczer/vault-link/actions/workflows/e2e.yml/badge.svg)](https://github.com/schmelczer/vault-link/actions/workflows/e2e.yml) [![Publish server Docker image](https://github.com/schmelczer/vault-link/actions/workflows/publish-docker.yml/badge.svg)](https://github.com/schmelczer/vault-link/actions/workflows/publish-docker.yml) [![Publish Obsidian plugin](https://github.com/schmelczer/vault-link/actions/workflows/publish-plugin.yml/badge.svg)](https://github.com/schmelczer/vault-link/actions/workflows/publish-plugin.yml) -## Install [nvm](https://github.com/nvm-sh/nvm) +## Develop + +### Install [nvm](https://github.com/nvm-sh/nvm) - `curl -o- https://raw.githubusercontent.com/nvm-sh/nvm/v0.40.1/install.sh | bash` -- `nvm install 20` -- `nvm use 20` -- Optionally set the system-wide default: `nvm alias default 20` +- `nvm install 22` +- `nvm use 22` +- Optionally set the system-wide default: `nvm alias default 22` -## Set up Rust +### Set up Rust - Install [`rustup`](https://rustup.rs): `curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh` - Install [`wasm-pack`](https://rustwasm.github.io/wasm-pack/installer): `curl https://rustwasm.github.io/wasm-pack/installer/init.sh -sSf | sh` - `cargo install cargo-insta sqlx-cli cargo-edit` - -## Publish new version +### Install Obsidian on Linux ```sh -./bump-version.sh patch -``` - - -## Update HTTP API TS bindings - -```sh -./update-api-types.sh -``` - -``` - -todo: enable -[workspace.lints.clippy] -single_call_fn = { level = "allow", priority = 1 } -absolute_paths = { level = "allow", priority = 1 } -arithmetic_side_effects = { level = "allow", priority = 1 } -similar_names = { level = "allow", priority = 1 } -self_named_module_files = { level = "allow", priority = 1 } -single_char_lifetime_names = { level = "allow", priority = 1 } -missing_docs_in_private_items = { level = "allow", priority = 1 } -question_mark_used = { level = "allow", priority = 1 } -implicit_return = { level = "allow", priority = 1 } -pedantic = { level = "warn", priority = 0 } -cargo = { level = "warn", priority = 0 } - -``` - apt install flatpak flatpak remote-add --if-not-exists flathub https://dl.flathub.org/repo/flathub.flatpakrepo flatpak install flathub md.obsidian.Obsidian flatpak run md.obsidian.Obsidian +``` + +### Scripts + +#### Update HTTP API TS bindings + +```sh +scripts/update-api-types.sh +``` + +#### Publish new version + +```sh +scripts/bump-version.sh patch +``` + + +#### Run E2E tests + +```sh +scripts/e2e.sh +``` + +And to clean up the logs & database files, run `scripts/clean-up.sh` +``` diff --git a/backend/Cargo.toml b/backend/Cargo.toml index c405cf52..5c3768a5 100644 --- a/backend/Cargo.toml +++ b/backend/Cargo.toml @@ -56,3 +56,19 @@ uninlined_format_args = "warn" unnested_or_patterns = "warn" unused_self = "warn" verbose_file_reads = "warn" + +cast_possible_truncation = { level = "allow", priority = 1 } +doc_link_with_quotes = { level = "allow", priority = 1 } +cast_sign_loss = { level = "allow", priority = 1 } +cast_possible_wrap = { level = "allow", priority = 1 } +struct_field_names = { level = "allow", priority = 1 } +single_call_fn = { level = "allow", priority = 1 } +absolute_paths = { level = "allow", priority = 1 } +arithmetic_side_effects = { level = "allow", priority = 1 } +similar_names = { level = "allow", priority = 1 } +self_named_module_files = { level = "allow", priority = 1 } +single_char_lifetime_names = { level = "allow", priority = 1 } +missing_docs_in_private_items = { level = "allow", priority = 1 } +question_mark_used = { level = "allow", priority = 1 } +implicit_return = { level = "allow", priority = 1 } +pedantic = { level = "warn", priority = 0 } diff --git a/backend/reconcile/src/diffs/myers.rs b/backend/reconcile/src/diffs/myers.rs index cce51d54..9692c221 100644 --- a/backend/reconcile/src/diffs/myers.rs +++ b/backend/reconcile/src/diffs/myers.rs @@ -99,7 +99,6 @@ impl IndexMut for V { } } -#[inline(always)] fn split_at(range: Range, at: usize) -> (Range, Range) { (range.start..at, at..range.end) } diff --git a/backend/reconcile/src/diffs/raw_operation.rs b/backend/reconcile/src/diffs/raw_operation.rs index bf970062..0df48f5d 100644 --- a/backend/reconcile/src/diffs/raw_operation.rs +++ b/backend/reconcile/src/diffs/raw_operation.rs @@ -16,9 +16,9 @@ where { pub fn tokens(&self) -> &Vec> { match self { - RawOperation::Insert(tokens) => tokens, - RawOperation::Delete(tokens) => tokens, - RawOperation::Equal(tokens) => tokens, + RawOperation::Insert(tokens) + | RawOperation::Delete(tokens) + | RawOperation::Equal(tokens) => tokens, } } diff --git a/backend/reconcile/src/operation_transformation/edited_text.rs b/backend/reconcile/src/operation_transformation/edited_text.rs index 87a5df40..d3ae3832 100644 --- a/backend/reconcile/src/operation_transformation/edited_text.rs +++ b/backend/reconcile/src/operation_transformation/edited_text.rs @@ -234,6 +234,7 @@ where } /// Apply the operations to the text and return the resulting text. + #[must_use] pub fn apply(&self) -> String { let mut builder: StringBuilder<'_> = StringBuilder::new(self.text); diff --git a/backend/reconcile/src/operation_transformation/operation.rs b/backend/reconcile/src/operation_transformation/operation.rs index ffc4f7d6..73fa6140 100644 --- a/backend/reconcile/src/operation_transformation/operation.rs +++ b/backend/reconcile/src/operation_transformation/operation.rs @@ -1,8 +1,8 @@ -use core::{ - fmt::{Debug, Display}, +use core::fmt::{Debug, Display}; +use std::{ + hash::{DefaultHasher, Hash, Hasher}, ops::Range, }; -use std::hash::{DefaultHasher, Hash, Hasher}; #[cfg(feature = "serde")] use serde::{Deserialize, Serialize}; @@ -57,7 +57,7 @@ where index.hash(state); deleted_character_count.hash(state); } - }; + } } } @@ -133,7 +133,7 @@ where builder.delete(self.range()); } - }; + } builder } @@ -141,8 +141,7 @@ where /// Returns the index of the first character that the operation affects. pub fn start_index(&self) -> usize { match self { - Operation::Insert { index, .. } => *index, - Operation::Delete { index, .. } => *index, + Operation::Insert { index, .. } | Operation::Delete { index, .. } => *index, } } @@ -156,6 +155,7 @@ where } /// Returns the range of indices of characters that the operation affects. + #[allow(clippy::range_plus_one)] pub fn range(&self) -> Range { self.start_index()..self.end_index() + 1 } /// Returns the number of affected characters. It is always greater than 0 @@ -382,7 +382,7 @@ mod tests { use super::*; #[test] - #[should_panic] + #[should_panic(expected = "Shifted index must be non-negative")] fn test_shifting_error() { insta::assert_debug_snapshot!( Operation::create_insert(1, vec!["hi".into()]) diff --git a/backend/reconcile/src/utils/merge_iters.rs b/backend/reconcile/src/utils/merge_iters.rs index c7b73345..2730c336 100644 --- a/backend/reconcile/src/utils/merge_iters.rs +++ b/backend/reconcile/src/utils/merge_iters.rs @@ -46,8 +46,7 @@ where }; match order { - Some(Ordering::Less) | None => self.left.next(), - Some(Ordering::Equal) => self.left.next(), + Some(Ordering::Less | Ordering::Equal) | None => self.left.next(), Some(Ordering::Greater) => self.right.next(), } } diff --git a/backend/sync_lib/src/lib.rs b/backend/sync_lib/src/lib.rs index e0fa5eb7..6f27e055 100644 --- a/backend/sync_lib/src/lib.rs +++ b/backend/sync_lib/src/lib.rs @@ -18,7 +18,20 @@ pub mod errors; /// Encode binary data for easy transport over HTTP. Inverse of /// `base64_to_bytes`. +/// +/// # Arguments +/// +/// - `input`: The binary data to encode. +/// +/// # Returns +/// +/// The base64-encoded string. +/// +/// # Panics +/// +/// If the input is not valid UTF-8. #[wasm_bindgen(js_name = bytesToBase64)] +#[must_use] pub fn bytes_to_base64(input: &[u8]) -> String { set_panic_hook(); @@ -26,6 +39,19 @@ pub fn bytes_to_base64(input: &[u8]) -> String { } /// Inverse of `bytes_to_base64`. +/// Decode base64-encoded data into binary data. +/// +/// # Arguments +/// +/// - `input`: The base64-encoded string. +/// +/// # Returns +/// +/// The decoded binary data. +/// +/// # Errors +/// +/// If the input is not valid base64. #[wasm_bindgen(js_name = base64ToBytes)] pub fn base64_to_bytes(input: &str) -> Result, SyncLibError> { set_panic_hook(); @@ -36,7 +62,22 @@ pub fn base64_to_bytes(input: &str) -> Result, SyncLibError> { /// Merge two documents with a common parent. Relies on `reconcile::reconcile` /// for texts and returns the right document as-is if either of the updated /// documents is binary. +/// +/// # Arguments +/// +/// - `parent`: The common parent document. +/// - `left`: The left document updated by one user. +/// - `right`: The right document updated by another user. +/// +/// # Returns +/// +/// The merged document. +/// +/// # Panics +/// +/// If any of the input documents are not valid UTF-8 strings. #[wasm_bindgen] +#[must_use] pub fn merge(parent: &[u8], left: &[u8], right: &[u8]) -> Vec { set_panic_hook(); @@ -54,6 +95,7 @@ pub fn merge(parent: &[u8], left: &[u8], right: &[u8]) -> Vec { /// WASM wrapper around `reconcile::reconcile` for text merging. #[wasm_bindgen(js_name = mergeText)] +#[must_use] pub fn merge_text(parent: &str, left: &str, right: &str) -> String { set_panic_hook(); @@ -63,6 +105,7 @@ pub fn merge_text(parent: &str, left: &str, right: &str) -> String { /// Heuristically determine if the given data is a binary or a text file's /// content. #[wasm_bindgen(js_name = isBinary)] +#[must_use] pub fn is_binary(data: &[u8]) -> bool { set_panic_hook(); @@ -77,6 +120,7 @@ pub fn is_binary(data: &[u8]) -> bool { /// We don't want to support merging structured data like JSON, YAML, etc. #[wasm_bindgen(js_name = isFileTypeMergable)] +#[must_use] pub fn is_file_type_mergable(path_or_file_name: &str) -> bool { set_panic_hook(); diff --git a/backend/sync_server/src/config.rs b/backend/sync_server/src/config.rs index 829375da..0de65f1a 100644 --- a/backend/sync_server/src/config.rs +++ b/backend/sync_server/src/config.rs @@ -42,9 +42,12 @@ impl Config { } pub async fn load_from_file(path: &Path) -> Result { - let contents = fs::read_to_string(path) - .await - .with_context(|| format!("Cannot load configuration from disk from ({path:?})"))?; + let contents = fs::read_to_string(path).await.with_context(|| { + format!( + "Cannot load configuration from disk from ({})", + path.display() + ) + })?; let config = serde_yaml::from_str(&contents).context("Failed to parse configuration")?; diff --git a/backend/sync_server/src/database.rs b/backend/sync_server/src/database.rs index a2a7499e..5492d911 100644 --- a/backend/sync_server/src/database.rs +++ b/backend/sync_server/src/database.rs @@ -79,7 +79,7 @@ impl Database { .test_before_acquire(true) .connect_with(connection_options) .await - .with_context(|| format!("Cannot open database at '{file_name:?}'"))?; + .with_context(|| format!("Cannot open database at '{}'", file_name.display()))?; Self::run_migrations(&pool).await?; diff --git a/backend/sync_server/src/errors.rs b/backend/sync_server/src/errors.rs index aa109c82..5aec9c32 100644 --- a/backend/sync_server/src/errors.rs +++ b/backend/sync_server/src/errors.rs @@ -33,12 +33,12 @@ pub enum SyncServerError { impl SyncServerError { pub fn serialize(&self) -> SerializedError { match self { - Self::InitError(error) => error.into(), - Self::ClientError(error) => error.into(), - Self::ServerError(error) => error.into(), - Self::NotFound(error) => error.into(), - Self::Unauthorized(error) => error.into(), - Self::PermissionDeniedError(error) => error.into(), + Self::InitError(error) + | Self::ClientError(error) + | Self::ServerError(error) + | Self::NotFound(error) + | Self::Unauthorized(error) + | Self::PermissionDeniedError(error) => error.into(), } } } @@ -48,9 +48,10 @@ impl IntoResponse for SyncServerError { let body = Json(self.serialize()); match self { - Self::InitError(_) => (StatusCode::INTERNAL_SERVER_ERROR, body).into_response(), + Self::InitError(_) | Self::ServerError(_) => { + (StatusCode::INTERNAL_SERVER_ERROR, body).into_response() + } Self::ClientError(_) => (StatusCode::BAD_REQUEST, body).into_response(), - Self::ServerError(_) => (StatusCode::INTERNAL_SERVER_ERROR, body).into_response(), Self::NotFound(_) => (StatusCode::NOT_FOUND, body).into_response(), Self::Unauthorized(_) => (StatusCode::UNAUTHORIZED, body).into_response(), Self::PermissionDeniedError(_) => (StatusCode::FORBIDDEN, body).into_response(), diff --git a/backend/sync_server/src/server.rs b/backend/sync_server/src/server.rs index bf62fec5..511187e0 100644 --- a/backend/sync_server/src/server.rs +++ b/backend/sync_server/src/server.rs @@ -16,6 +16,7 @@ use axum::{ extract::{DefaultBodyLimit, Request}, http::{self, HeaderValue, Method}, response::IntoResponse, + routing::IntoMakeService, }; use log::{error, info}; use tokio::signal; @@ -30,7 +31,10 @@ use tower_http::{ }; use tracing::{Level, info_span}; -use crate::errors::{SerializedError, not_found_error}; +use crate::{ + config::server_config::ServerConfig, + errors::{SerializedError, not_found_error}, +}; mod app_state; mod auth; mod create_document; @@ -52,24 +56,9 @@ pub async fn create_server() -> Result<()> { .await .context("Failed to initialise app state")?; - let address = format!( - "{}:{}", - &app_state.config.server.host, &app_state.config.server.port - ); - - let mut api = OpenApi { - info: Info { - title: "VaultLink sync server".to_owned(), - summary: Some( - "Simple API for syncing documents between concurrent clients.".to_owned(), - ), - description: Some(include_str!("../README.md").to_owned()), - version: env!("CARGO_PKG_VERSION").to_owned(), - ..Info::default() - }, - ..OpenApi::default() - }; + let server_config = app_state.config.server.clone(); + let mut api = create_open_api(); let app = ApiRouter::new() .api_route("/ping", get(ping::ping)) .api_route( @@ -140,11 +129,42 @@ pub async fn create_server() -> Result<()> { .allow_methods([Method::GET, Method::POST, Method::PUT, Method::DELETE]), ) .with_state(app_state) - .finish_api_with(&mut api, api_docs) + .finish_api_with(&mut api, add_api_docs_error_example) .layer(Extension(Arc::new(api))) // https://github.com/tamasfe/aide/blob/507f4a8822bc0c13cbda0f589da1e0f4cbcdb812/examples/example-axum/src/main.rs#L39 .fallback(handler_404) .into_make_service(); + start_server(app, &server_config).await +} + +async fn serve_api(Extension(api): Extension>) -> impl IntoResponse { Json(api) } + +fn create_open_api() -> OpenApi { + OpenApi { + info: Info { + title: "VaultLink sync server".to_owned(), + summary: Some( + "Simple API for syncing documents between concurrent clients.".to_owned(), + ), + description: Some(include_str!("../README.md").to_owned()), + version: env!("CARGO_PKG_VERSION").to_owned(), + ..Info::default() + }, + ..OpenApi::default() + } +} + +fn add_api_docs_error_example(api: TransformOpenApi<'_>) -> TransformOpenApi<'_> { + api.default_response_with::, _>(|res| { + res.example(SerializedError { + message: "An error has occurred".to_owned(), + causes: vec![], + }) + }) +} + +async fn start_server(app: IntoMakeService, config: &ServerConfig) -> Result<()> { + let address = format!("{}:{}", config.host, config.port); let listener = tokio::net::TcpListener::bind(address.clone()) .await .with_context(|| format!("Failed to bind to address: {address}"))?; @@ -163,17 +183,6 @@ pub async fn create_server() -> Result<()> { .context("Failed to start server") } -async fn serve_api(Extension(api): Extension>) -> impl IntoResponse { Json(api) } - -fn api_docs(api: TransformOpenApi<'_>) -> TransformOpenApi<'_> { - api.default_response_with::, _>(|res| { - res.example(SerializedError { - message: "An error has occurred".to_owned(), - causes: vec![], - }) - }) -} - async fn shutdown_signal() { let ctrl_c = async { signal::ctrl_c() @@ -193,8 +202,8 @@ async fn shutdown_signal() { let terminate = std::future::pending::<()>(); tokio::select! { - _ = ctrl_c => {}, - _ = terminate => {}, + () = ctrl_c => {}, + () = terminate => {}, } } diff --git a/backend/sync_server/src/server/fetch_document_version.rs b/backend/sync_server/src/server/fetch_document_version.rs index 13978b8f..a2b157e3 100644 --- a/backend/sync_server/src/server/fetch_document_version.rs +++ b/backend/sync_server/src/server/fetch_document_version.rs @@ -39,12 +39,14 @@ pub async fn fetch_document_version( .get_document_version(&vault_id, vault_update_id, None) .await .map_err(server_error)? - .map(Ok) - .unwrap_or_else(|| { - Err(not_found_error(anyhow!( - "Document with vault update id `{vault_update_id}` not found", - ))) - })?; + .map_or_else( + || { + Err(not_found_error(anyhow!( + "Document with vault update id `{vault_update_id}` not found", + ))) + }, + Ok, + )?; if result.document_id != document_id { return Err(not_found_error(anyhow!( diff --git a/backend/sync_server/src/server/fetch_document_version_content.rs b/backend/sync_server/src/server/fetch_document_version_content.rs index 2889d435..203f0afb 100644 --- a/backend/sync_server/src/server/fetch_document_version_content.rs +++ b/backend/sync_server/src/server/fetch_document_version_content.rs @@ -41,12 +41,14 @@ pub async fn fetch_document_version_content( .get_document_version(&vault_id, vault_update_id, None) .await .map_err(server_error)? - .map(Ok) - .unwrap_or_else(|| { - Err(not_found_error(anyhow!( - "Document with vault update id `{vault_update_id}` not found", - ))) - })?; + .map_or_else( + || { + Err(not_found_error(anyhow!( + "Document with vault update id `{vault_update_id}` not found", + ))) + }, + Ok, + )?; if result.document_id != document_id { return Err(not_found_error(anyhow!( diff --git a/backend/sync_server/src/server/fetch_latest_document_version.rs b/backend/sync_server/src/server/fetch_latest_document_version.rs index 89e35882..331730e0 100644 --- a/backend/sync_server/src/server/fetch_latest_document_version.rs +++ b/backend/sync_server/src/server/fetch_latest_document_version.rs @@ -37,12 +37,14 @@ pub async fn fetch_latest_document_version( .get_latest_document(&vault_id, &document_id, None) .await .map_err(server_error)? - .map(Ok) - .unwrap_or_else(|| { - Err(not_found_error(anyhow!( - "Document with id `{document_id}` not found", - ))) - })?; + .map_or_else( + || { + Err(not_found_error(anyhow!( + "Document with id `{document_id}` not found", + ))) + }, + Ok, + )?; Ok(Json(latest_version.into())) } diff --git a/backend/sync_server/src/server/update_document.rs b/backend/sync_server/src/server/update_document.rs index 93ed5417..a9b9c13e 100644 --- a/backend/sync_server/src/server/update_document.rs +++ b/backend/sync_server/src/server/update_document.rs @@ -80,7 +80,7 @@ pub async fn update_document_json( .await } -#[allow(clippy::too_many_arguments)] +#[allow(clippy::too_many_arguments, clippy::too_many_lines)] async fn internal_update_document( auth_header: Authorization, mut state: AppState, @@ -176,7 +176,7 @@ async fn internal_update_document( let new_relative_path = if parent_document.relative_path == latest_version.relative_path && latest_version.relative_path != sanitized_relative_path { - let mut new_relative_path = Default::default(); + let mut new_relative_path = String::default(); for candidate in deduped_file_paths(&sanitized_relative_path) { if state .database