From 501b14de18cfcb6258a01ea3ccd9d54930cf2490 Mon Sep 17 00:00:00 2001 From: ZoopOTheGoop <93421308+ZoopOTheGoop@users.noreply.github.com> Date: Thu, 25 Jan 2024 10:27:50 -0800 Subject: [PATCH] docs/Document everything (#52) * Document dependencies * add bits to manifest * clean up/unify imports and mods * Document app * Fix clippy warning in test helpers * Fix warnings on generated files * Add top-level comment for app mod * Add basic docs to protobuf generated modules * comment -> intra-doc link * Feature documentation * Documentation for utils * docs/clean feedback * refactor/unify naming and doc language * cleanup/O->o * cleanup/..->. * docs/clarify user * fix/doc typos * refactor/get_rating->rating --- Cargo.toml | 3 + README.md | 4 ++ src/app/context.rs | 20 ++++-- src/app/interfaces/authentication.rs | 3 + src/app/interfaces/errors.rs | 3 + src/app/interfaces/middleware.rs | 29 ++++++++- src/app/interfaces/mod.rs | 5 +- src/app/interfaces/routes.rs | 14 +++-- src/app/mod.rs | 5 +- src/app/run.rs | 18 ++++-- src/features/app/mod.rs | 5 -- src/features/app/service.rs | 9 --- src/features/app/use_cases.rs | 21 ------- src/features/chart/entities.rs | 24 ++++++- src/features/chart/errors.rs | 11 +++- src/features/chart/infrastructure.rs | 5 ++ src/features/chart/interface.rs | 3 +- src/features/chart/mod.rs | 7 ++- src/features/chart/service.rs | 6 ++ src/features/chart/use_cases.rs | 5 ++ src/features/common/entities.rs | 63 +++++++++++++------ src/features/common/mod.rs | 2 + src/features/mod.rs | 7 ++- src/features/pb.rs | 14 +++++ src/features/{app => rating}/errors.rs | 5 ++ .../{app => rating}/infrastructure.rs | 9 ++- src/features/{app => rating}/interface.rs | 5 +- src/features/rating/mod.rs | 10 +++ src/features/rating/service.rs | 12 ++++ src/features/rating/use_cases.rs | 28 +++++++++ src/features/user/entities.rs | 18 +++++- src/features/user/errors.rs | 7 +++ src/features/user/infrastructure.rs | 22 ++++++- src/features/user/interface.rs | 13 +++- src/features/user/mod.rs | 7 ++- src/features/user/service.rs | 5 ++ src/features/user/use_cases.rs | 18 +++++- src/lib.rs | 9 +++ src/utils/config.rs | 12 ++++ src/utils/infrastructure.rs | 20 +++--- src/utils/jwt.rs | 15 +++++ src/utils/migrator.rs | 20 ++++-- src/utils/mod.rs | 10 +-- tests/helpers/data_faker.rs | 16 +++-- 44 files changed, 432 insertions(+), 115 deletions(-) delete mode 100644 src/features/app/mod.rs delete mode 100644 src/features/app/service.rs delete mode 100644 src/features/app/use_cases.rs rename src/features/{app => rating}/errors.rs (56%) rename src/features/{app => rating}/infrastructure.rs (70%) rename src/features/{app => rating}/interface.rs (83%) create mode 100644 src/features/rating/mod.rs create mode 100644 src/features/rating/service.rs create mode 100644 src/features/rating/use_cases.rs diff --git a/Cargo.toml b/Cargo.toml index c18aa3e7..98323893 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,6 +3,9 @@ name = "ratings" version = "0.1.0" edition = "2021" rust-version = "1.69.0" +authors = ["Canonical"] +description = "Backend for ratings for the Ubuntu app center." +license = "GPL-3.0-only" [dependencies] dotenv = "0.15.0" diff --git a/README.md b/README.md index 00e00a79..031353dd 100644 --- a/README.md +++ b/README.md @@ -16,6 +16,10 @@ This is the code repository for **Ratings**, the backend service responsible for For general details, including installation, getting started and setting up a development environment, head over to our section on [Contributing to the code](CONTRIBUTING.md#contributing-to-the-code). +## Dependencies + +In order to run, this needs you to install at minimum `libssl-dev` (for OpenSSL) and `protobuf-compiler` (for `prost`) via `apt`. + ## Get involved This is an [open source](LICENSE) project and we warmly welcome community contributions, suggestions, and constructive feedback. If you're interested in contributing, please take a look at our [Contribution guidelines](CONTRIBUTING.md) first. diff --git a/src/app/context.rs b/src/app/context.rs index e147c9bd..1604708c 100644 --- a/src/app/context.rs +++ b/src/app/context.rs @@ -1,14 +1,18 @@ +//! Contains definitions for maintaining the current [`AppContext`]. + use std::sync::Arc; -use crate::utils::jwt::Claims; -use crate::utils::Config; -use crate::utils::Infrastructure; +use crate::utils::{jwt::Claims, Config, Infrastructure}; +/// An atomically reference counted app state. #[derive(Debug, Clone)] pub struct AppContext(Arc); #[allow(dead_code)] impl AppContext { + /// Builds a new [`AppContext`] from a user [`Config`] and [`Infrastructure`]. + /// + /// The [`Config`] will be cloned. pub fn new(config: &Config, infra: Infrastructure) -> Self { let inner = AppContextInner { infra, @@ -17,24 +21,32 @@ impl AppContext { Self(Arc::new(inner)) } + /// A reference to the held [`Infrastructure`]. pub fn infrastructure(&self) -> &Infrastructure { &self.0.infra } + /// A reference to the held [`Config`]. pub fn config(&self) -> &Config { &self.0.config } } -#[allow(dead_code)] +/// Contains the overall state and configuration of the app, only meant to be used +/// in terms of the [`AppContext`]. #[derive(Debug)] struct AppContextInner { + /// Contains JWT and postgres infrastructure for the app. infra: Infrastructure, + /// App configuration settings. config: Config, } +/// Contains the context for a given request #[derive(Debug, Clone)] pub struct RequestContext { + /// The URI this request is from. pub uri: String, + /// If applicable, the associated JWT claims. pub claims: Option, } diff --git a/src/app/interfaces/authentication.rs b/src/app/interfaces/authentication.rs index fab1436e..7f351a49 100644 --- a/src/app/interfaces/authentication.rs +++ b/src/app/interfaces/authentication.rs @@ -1,9 +1,12 @@ +//! Contains the utilities for authorizing user requests + use http::header; use tonic::{Request, Status}; use tracing::error; use crate::app::{context::AppContext, RequestContext}; +/// Authorizes the user in the request specified by `req`. pub fn authentication(req: Request<()>) -> Result, Status> { let app_ctx = req.extensions().get::().unwrap().clone(); diff --git a/src/app/interfaces/errors.rs b/src/app/interfaces/errors.rs index 01c4a2e7..b73d7067 100644 --- a/src/app/interfaces/errors.rs +++ b/src/app/interfaces/errors.rs @@ -1,7 +1,10 @@ +//! Error implementations for the app interface. use thiserror::Error; +/// A generic interface error, currently never constructed. #[derive(Error, Debug)] pub enum AppInterfaceError { + /// Something unpredictable went wrong #[error("app interface: unknown error")] #[allow(dead_code)] Unknown, diff --git a/src/app/interfaces/middleware.rs b/src/app/interfaces/middleware.rs index df28f25a..1d526073 100644 --- a/src/app/interfaces/middleware.rs +++ b/src/app/interfaces/middleware.rs @@ -1,3 +1,5 @@ +//! The middleware layers for the app context. + use std::pin::Pin; use hyper::{service::Service, Body}; @@ -6,19 +8,28 @@ use tower::Layer; use crate::app::context::{AppContext, RequestContext}; +/// Passthrough [`Layer`] containing the [`AppContext`], this is mainly used to construct +/// [`ContextMiddleware`]. #[derive(Clone)] pub struct ContextMiddlewareLayer { + /// The wrapped context app_ctx: AppContext, } impl ContextMiddlewareLayer { + /// Creates a new layer from the given [`AppContext`]. pub fn new(ctx: AppContext) -> ContextMiddlewareLayer { ContextMiddlewareLayer { app_ctx: ctx } } } -impl Layer for ContextMiddlewareLayer { +impl Layer for ContextMiddlewareLayer +where + S: Service, Response = hyper::Response> + Clone + Send + 'static, + S::Future: Send + 'static, +{ type Service = ContextMiddleware; + fn layer(&self, service: S) -> Self::Service { ContextMiddleware { app_ctx: self.app_ctx.clone(), @@ -27,12 +38,26 @@ impl Layer for ContextMiddlewareLayer { } } +/// A [`Service`] which delegates responses to a request by the inner `S`, +/// which is itself a [`Service`], by calling the inner [`Future`]. +/// +/// [`Future`]: std::future::Future #[derive(Clone)] -pub struct ContextMiddleware { +pub struct ContextMiddleware +where + S: Service, Response = hyper::Response> + Clone + Send + 'static, + S::Future: Send + 'static, +{ + /// The current context of the app, as passed in from the [`ContextMiddlewareLayer`] app_ctx: AppContext, + + /// The inner [`Service`] containing the [`Future`]. + /// + /// [`Future`]: std::future::Future inner: S, } +/// A type definition which is simply a future that's in a pinned location in the heap. type BoxFuture<'a, T> = Pin + Send + 'a>>; impl Service> for ContextMiddleware diff --git a/src/app/interfaces/mod.rs b/src/app/interfaces/mod.rs index ae69656c..89fec657 100644 --- a/src/app/interfaces/mod.rs +++ b/src/app/interfaces/mod.rs @@ -1,4 +1,7 @@ +//! Contains submodules with various middleware and utility layers for the app. + pub mod authentication; -mod errors; pub mod middleware; pub mod routes; + +mod errors; diff --git a/src/app/interfaces/routes.rs b/src/app/interfaces/routes.rs index 2122b14a..a815cb02 100644 --- a/src/app/interfaces/routes.rs +++ b/src/app/interfaces/routes.rs @@ -1,10 +1,12 @@ +//! Contains definitions for service routers and related components. + use tonic::transport::server::Router; -use tonic_reflection::server::ServerReflection; +use tonic_reflection::server::{ServerReflection, ServerReflectionServer}; -use crate::features::{app, chart, user}; +use crate::features::{chart, rating, user}; -pub fn build_reflection_service( -) -> tonic_reflection::server::ServerReflectionServer { +/// Creates a new default reflection server for this app +pub fn build_reflection_service() -> ServerReflectionServer { let file_descriptor_set = tonic::include_file_descriptor_set!("ratings_descriptor"); tonic_reflection::server::Builder::configure() @@ -13,9 +15,11 @@ pub fn build_reflection_service( .unwrap() } +/// Registers new services required to make the passed in [`Router`] work, +/// the [`Router`] won't be otherwise modified. pub fn build_servers(router: Router) -> Router { let user_service = user::service::build_service(); - let app_service = app::service::build_service(); + let app_service = rating::service::build_service(); let chart_service = chart::service::build_service(); router diff --git a/src/app/mod.rs b/src/app/mod.rs index 68e10d9b..5c03fde4 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -1,5 +1,6 @@ -pub use context::AppContext; -pub use context::RequestContext; +//! Contains all definitions of the main application interface part of the program. + +pub use context::{AppContext, RequestContext}; pub use run::run; mod context; diff --git a/src/app/run.rs b/src/app/run.rs index 1efd51f9..1399c4af 100644 --- a/src/app/run.rs +++ b/src/app/run.rs @@ -1,15 +1,21 @@ +//! Contains definitions for running the app context. use std::{net::SocketAddr, time::Duration}; -use crate::app::context::AppContext; use tonic::transport::Server; use tower::ServiceBuilder; use tracing::info; -use crate::utils::{Config, Infrastructure, Migrator}; - -use super::interfaces::routes::{build_reflection_service, build_servers}; -use super::interfaces::{authentication::authentication, middleware::ContextMiddlewareLayer}; - +use crate::{ + app::context::AppContext, + app::interfaces::{ + authentication::authentication, + middleware::ContextMiddlewareLayer, + routes::{build_reflection_service, build_servers}, + }, + utils::{Config, Infrastructure, Migrator}, +}; + +/// Runs the app given the associated [`Config`]. pub async fn run(config: Config) -> Result<(), Box> { let migrator = Migrator::new(&config.migration_postgres_uri).await?; migrator.run().await?; diff --git a/src/features/app/mod.rs b/src/features/app/mod.rs deleted file mode 100644 index d273b6e9..00000000 --- a/src/features/app/mod.rs +++ /dev/null @@ -1,5 +0,0 @@ -mod errors; -mod infrastructure; -pub mod interface; -pub mod service; -mod use_cases; diff --git a/src/features/app/service.rs b/src/features/app/service.rs deleted file mode 100644 index ef223907..00000000 --- a/src/features/app/service.rs +++ /dev/null @@ -1,9 +0,0 @@ -use crate::features::pb::app::app_server::AppServer; - -#[derive(Debug, Default)] -pub struct AppService; - -pub fn build_service() -> AppServer { - let service = AppService; - AppServer::new(service) -} diff --git a/src/features/app/use_cases.rs b/src/features/app/use_cases.rs deleted file mode 100644 index 49233beb..00000000 --- a/src/features/app/use_cases.rs +++ /dev/null @@ -1,21 +0,0 @@ -use crate::{ - app::AppContext, - features::{ - app::{errors::AppError, infrastructure::get_votes_by_snap_id}, - common::entities::Rating, - }, -}; -use tracing::error; - -pub async fn get_rating(app_ctx: &AppContext, snap_id: String) -> Result { - let votes = get_votes_by_snap_id(app_ctx, &snap_id) - .await - .map_err(|error| { - error!("{error:?}"); - AppError::Unknown - })?; - - let rating = Rating::new(votes); - - Ok(rating) -} diff --git a/src/features/chart/entities.rs b/src/features/chart/entities.rs index 12a4c5bd..2b065ca0 100644 --- a/src/features/chart/entities.rs +++ b/src/features/chart/entities.rs @@ -1,15 +1,27 @@ +//! Contains struct definitions for the charting feature for ratings. use crate::features::{ common::entities::{calculate_band, Rating, VoteSummary}, pb::chart as pb, }; use sqlx::FromRow; +/// A chart over a given [`Timeframe`] with the given [`ChartData`]. +/// +/// [`Timeframe`]: pb::Timeframe pub struct Chart { + /// The timeframe over which to display the data pub timeframe: pb::Timeframe, + /// The raw chart data to display pub chart_data: Vec, } impl Chart { + /// Creates a new [`Chart`] over the given [`Timeframe`]. The [`VoteSummary`] will be + /// interpreted as [`ChartData`]. + /// + /// If there are more than 20 elements, this will always truncate to the top 20. + /// + /// [`Timeframe`]: pb::Timeframe pub fn new(timeframe: pb::Timeframe, data: Vec) -> Self { let mut chart_data: Vec = data.into_iter().map(ChartData::from_vote_summary).collect(); @@ -30,13 +42,19 @@ impl Chart { } } +/// The actual information to be charted, contains a raw calculated value +/// as well as an overall [`Rating`] with more info struct. #[derive(Debug, Clone, FromRow)] pub struct ChartData { + /// The raw rating as it should be charted pub raw_rating: f32, + /// A complex rating with the band, overall votes, and the snap this rating is for pub rating: Rating, } impl ChartData { + /// Creates a single element of [`ChartData`] from a [`VoteSummary`] -- this calculates the overall + /// rating on demand. pub fn from_vote_summary(vote_summary: VoteSummary) -> Self { let (raw_rating, ratings_band) = calculate_band(&vote_summary); let rating = Rating { @@ -48,10 +66,12 @@ impl ChartData { Self { raw_rating, rating } } - pub fn into_dto(self) -> pb::ChartData { + /// Converts this [`ChartData`] into its wire version as defined in the + /// protobuf files for transmission. + pub fn into_protobuf_chart_data(self) -> pb::ChartData { pb::ChartData { raw_rating: self.raw_rating, - rating: Some(self.rating.into_dto()), + rating: Some(self.rating.into_protobuf_rating()), } } } diff --git a/src/features/chart/errors.rs b/src/features/chart/errors.rs index 897c9fb5..735b152a 100644 --- a/src/features/chart/errors.rs +++ b/src/features/chart/errors.rs @@ -1,11 +1,18 @@ +//! Contains errors for the [`Chart`] features +//! +//! [`Chart`]: crate::features::chart::entities::Chart use thiserror::Error; +/// An error that can occur while using the chart service #[derive(Error, Debug)] pub enum ChartError { + /// Could not retrieve the chart #[error("failed to get chart for timeframe")] FailedToGetChart, - #[error("unknown chart error")] - Unknown, + /// There was no data in the given timeframe #[error("could not find data for given timeframe")] NotFound, + /// Another unknown error (e.g. network failure) + #[error("unknown chart error")] + Unknown, } diff --git a/src/features/chart/infrastructure.rs b/src/features/chart/infrastructure.rs index 4ed0f07b..cc40ea69 100644 --- a/src/features/chart/infrastructure.rs +++ b/src/features/chart/infrastructure.rs @@ -1,9 +1,14 @@ +//! Definitions for getting vote summaries for the [`Chart`] implementations +//! +//! [`Chart`]: crate::features::chart::entities::Chart use crate::{ app::AppContext, features::{chart::errors::ChartError, common::entities::VoteSummary, pb::chart::Timeframe}, }; use tracing::error; +/// Retrieves the vote summary in the given [`AppContext`] over a given [`Timeframe`] +/// from the database. pub(crate) async fn get_votes_summary_by_timeframe( app_ctx: &AppContext, timeframe: Timeframe, diff --git a/src/features/chart/interface.rs b/src/features/chart/interface.rs index 61b5ddf1..1e74439d 100644 --- a/src/features/chart/interface.rs +++ b/src/features/chart/interface.rs @@ -1,3 +1,4 @@ +//! Contains trait implementations for the chart feature. use crate::{ app::AppContext, features::{ @@ -32,7 +33,7 @@ impl Chart for ChartService { let ordered_chart_data = result .chart_data .into_iter() - .map(|chart_data| chart_data.into_dto()) + .map(|chart_data| chart_data.into_protobuf_chart_data()) .collect(); let payload = GetChartResponse { diff --git a/src/features/chart/mod.rs b/src/features/chart/mod.rs index 6522bea9..50f60b6a 100644 --- a/src/features/chart/mod.rs +++ b/src/features/chart/mod.rs @@ -1,6 +1,9 @@ +//! Contains various feature implementations for charting snap ratings. + pub mod entities; -mod errors; -mod infrastructure; pub mod interface; pub mod service; + +mod errors; +mod infrastructure; mod use_cases; diff --git a/src/features/chart/service.rs b/src/features/chart/service.rs index a6321e2f..e5aaac99 100644 --- a/src/features/chart/service.rs +++ b/src/features/chart/service.rs @@ -1,8 +1,14 @@ +//! Definitions and utilities for building the [`ChartService`] for using the [`Chart`] feature. +//! +//! [`Chart`]: crate::features::chart::entities::Chart + use crate::features::pb::chart::chart_server::ChartServer; +/// An empty struct denoting that allows the building of a [`ChartServer`]. #[derive(Debug, Default)] pub struct ChartService; +/// Creates a [`ChartServer`] with default parameters from a [`ChartService`]. pub fn build_service() -> ChartServer { let service = ChartService; ChartServer::new(service) diff --git a/src/features/chart/use_cases.rs b/src/features/chart/use_cases.rs index e94e2caa..8405735e 100644 --- a/src/features/chart/use_cases.rs +++ b/src/features/chart/use_cases.rs @@ -1,3 +1,5 @@ +//! Utility functions for using the [`Chart`] feature in one call. + use crate::{ app::AppContext, features::{ @@ -9,6 +11,9 @@ use crate::{ }; use tracing::error; +/// Gets a chart over the given [`Timeframe`] within the given [`AppContext`]. Either ends up returning +/// a [`Chart`] or else one of the many [`ChartError`]s in case the timeframe is invalid or another database error +/// happens. pub async fn get_chart(app_ctx: &AppContext, timeframe: Timeframe) -> Result { let votes = get_votes_summary_by_timeframe(app_ctx, timeframe) .await diff --git a/src/features/common/entities.rs b/src/features/common/entities.rs index 87da1f70..ece361e7 100644 --- a/src/features/common/entities.rs +++ b/src/features/common/entities.rs @@ -1,9 +1,13 @@ +//! Common defintions used throughout the ratings service. use sqlx::FromRow; use crate::features::pb::common as pb; +/// An arbitrary fixed number of votes we've determined is below the threshold to be meaningful. const INSUFFICIENT_VOTES_QUANTITY: i64 = 25; +/// A descriptive mapping of a number of ratings to a general indicator of "how good" +/// an app can be said to be. #[derive(Debug, Clone, PartialEq, Eq)] pub enum RatingsBand { VeryGood = 0, @@ -15,11 +19,17 @@ pub enum RatingsBand { } impl RatingsBand { + /// The percentage of votes that denotes an upper bound between good and very good const GOOD_UPPER: f64 = 0.8; + /// The percentage of votes that denotes the line between neutral and good const NEUTRAL_UPPER: f64 = 0.55; + /// The percentage of votes that denotes the line between poor and neutral const POOR_UPPER: f64 = 0.45; + /// The percentage of votes that denotes a line between poor and very poor const VERY_POOR_UPPER: f64 = 0.2; + /// Converts a raw value into a [`RatingsBand`] value by comparing it with the associated + /// constants of the struct. pub fn from_value(value: f64) -> RatingsBand { if value > Self::GOOD_UPPER { RatingsBand::VeryGood @@ -35,14 +45,23 @@ impl RatingsBand { } } +/// A descriptive rating object, usually used converted and transferred over the wire. +/// This is an aggregated rating for a snap without holding every raw value, as determined +/// by [`RatingsBand`]. #[derive(Debug, Clone, FromRow)] pub struct Rating { + /// The ID of the snap this rating is for pub snap_id: String, + /// The total votes for this snap pub total_votes: u64, + /// The descriptive indicator of "how good" this snap is based + /// on aggregated ratings. pub ratings_band: RatingsBand, } impl Rating { + /// Creates a new [`Rating`] based on a given [`VoteSummary`], by calculating + /// the required [`RatingsBand`]. pub fn new(votes: VoteSummary) -> Self { let (_, ratings_band) = calculate_band(&votes); Self { @@ -52,7 +71,8 @@ impl Rating { } } - pub(crate) fn into_dto(self) -> pb::Rating { + /// Converts this into its protobuf version for wire transfer. + pub(crate) fn into_protobuf_rating(self) -> pb::Rating { pb::Rating { snap_id: self.snap_id, total_votes: self.total_votes, @@ -61,18 +81,26 @@ impl Rating { } } +/// A summary of votes for a given snap, this is then aggregated before transfer. #[derive(Debug, Clone, FromRow)] pub struct VoteSummary { + /// The ID of the snap being checked. pub snap_id: String, + /// The total votes this snap has received. pub total_votes: i64, + /// The number of the votes which are positive. pub positive_votes: i64, } +/// A single vote, either good or bad. #[derive(Debug, Clone)] pub struct Vote { + /// Whether the vote is an "upvote" or "downvote". pub vote_up: bool, } +/// Converts a given [`VoteSummary`] into a [`RatingsBand`], if applicable, along with a +/// confidence interval if applicable. pub fn calculate_band(votes: &VoteSummary) -> (Option, RatingsBand) { if votes.total_votes < INSUFFICIENT_VOTES_QUANTITY { return (None, RatingsBand::InsufficientVotes); @@ -85,28 +113,27 @@ pub fn calculate_band(votes: &VoteSummary) -> (Option, RatingsBand) { ) } +/// Calculates the Lower Bound of Wilson Score Confidence Interval for Ranking Snaps +/// +/// Purpose: +/// Provides a conservative adjusted rating for a Snap by offsetting the +/// actual ratio of positive votes. It penalizes Snaps with fewer ratings +/// more heavily to produce an adjusted ranking that approaches the mean as +/// ratings increase. +/// +/// Algorithm: +/// Starts with the observed proportion of positive ratings, adjusts it based on +/// total ratings, and incorporates a 95% confidence interval Z-score for +/// uncertainty. +/// +/// References: +/// - https://www.evanmiller.org/how-not-to-sort-by-average-rating.html +/// - https://en.wikipedia.org/wiki/Binomial_proportion_confidence_interval#Wilson_score_interval fn confidence_interval_lower_bound(positive_ratings: i64, total_ratings: i64) -> f64 { if total_ratings == 0 { return 0.0; } - // Lower Bound of Wilson Score Confidence Interval for Ranking Snaps - // - // Purpose: - // Provides a conservative adjusted rating for a Snap by offsetting the - // actual ratio of positive votes. It penalizes Snaps with fewer ratings - // more heavily to produce an adjusted ranking that approaches the mean as - // ratings increase. - // - // Algorithm: - // Starts with the observed proportion of positive ratings, adjusts it based on - // total ratings, and incorporates a 95% confidence interval Z-score for - // uncertainty. - // - // References: - // - https://www.evanmiller.org/how-not-to-sort-by-average-rating.html - // - https://en.wikipedia.org/wiki/Binomial_proportion_confidence_interval#Wilson_score_interval - let z_score: f64 = 1.96; // hardcoded for a ~95% confidence let total_ratings = total_ratings as f64; let positive_ratings_ratio = positive_ratings as f64 / total_ratings; diff --git a/src/features/common/mod.rs b/src/features/common/mod.rs index 0b8f0b5a..1b1df1f5 100644 --- a/src/features/common/mod.rs +++ b/src/features/common/mod.rs @@ -1 +1,3 @@ +//! Contains various common feature definitions. + pub mod entities; diff --git a/src/features/mod.rs b/src/features/mod.rs index 4824253a..1111ba07 100644 --- a/src/features/mod.rs +++ b/src/features/mod.rs @@ -1,5 +1,8 @@ -pub mod app; +//! Contains various feature implementations for the ratings backend. + pub mod chart; +pub mod rating; +pub mod user; + mod common; mod pb; -pub mod user; diff --git a/src/features/pb.rs b/src/features/pb.rs index 78e91188..d20971ed 100644 --- a/src/features/pb.rs +++ b/src/features/pb.rs @@ -1,15 +1,29 @@ +//! Contains autogenerated protobuf implementations from [`prost`]. + +#![allow(rustdoc::broken_intra_doc_links)] +#![allow(missing_docs)] +#![allow(clippy::missing_docs_in_private_items)] + pub mod app { + //! Contains protobufs relating to the app features + include!("../../proto/ratings.features.app.rs"); } pub mod common { + //! Contains common protobufs + include!("../../proto/ratings.features.common.rs"); } pub mod user { + //! Contains user protobufs + include!("../../proto/ratings.features.user.rs"); } pub mod chart { + //! Contains chart protobufs + include!("../../proto/ratings.features.chart.rs"); } diff --git a/src/features/app/errors.rs b/src/features/rating/errors.rs similarity index 56% rename from src/features/app/errors.rs rename to src/features/rating/errors.rs index 4b634b8b..80938138 100644 --- a/src/features/app/errors.rs +++ b/src/features/rating/errors.rs @@ -1,9 +1,14 @@ +//! Error definitions for the ratings feature. + use thiserror::Error; +/// Various app errors #[derive(Error, Debug)] pub enum AppError { + /// Could not get a rating for the snap #[error("failed to get rating for snap")] FailedToGetRating, + /// Unknown user error #[error("unknown user error")] Unknown, } diff --git a/src/features/app/infrastructure.rs b/src/features/rating/infrastructure.rs similarity index 70% rename from src/features/app/infrastructure.rs rename to src/features/rating/infrastructure.rs index 76c74dff..13870040 100644 --- a/src/features/app/infrastructure.rs +++ b/src/features/rating/infrastructure.rs @@ -1,9 +1,16 @@ +//! Infrastructure definitions for the ratings center backend + use crate::{ app::AppContext, - features::{app::errors::AppError, common::entities::VoteSummary}, + features::{common::entities::VoteSummary, rating::errors::AppError}, }; use tracing::error; +/// Retrieves votes for the snap indicated by `snap_id` for the given [`AppContext`]. +/// +/// See the documentation for the common caller, [`get_rating`], for more information. +/// +/// [`get_rating`]: crate::features::app::use_cases::get_rating pub(crate) async fn get_votes_by_snap_id( app_ctx: &AppContext, snap_id: &str, diff --git a/src/features/app/interface.rs b/src/features/rating/interface.rs similarity index 83% rename from src/features/app/interface.rs rename to src/features/rating/interface.rs index 3030621c..8c7bb1aa 100644 --- a/src/features/app/interface.rs +++ b/src/features/rating/interface.rs @@ -1,8 +1,9 @@ +//! Contains trait implementations for [`AppService`] and other app definitions. use crate::app::AppContext; use crate::features::{ - app::{service::AppService, use_cases}, pb::app::{app_server::App, GetRatingRequest, GetRatingResponse}, + rating::{service::AppService, use_cases}, }; use tonic::{Request, Response, Status}; @@ -26,7 +27,7 @@ impl App for AppService { match result { Ok(rating) => { let payload = GetRatingResponse { - rating: Some(rating.into_dto()), + rating: Some(rating.into_protobuf_rating()), }; Ok(Response::new(payload)) } diff --git a/src/features/rating/mod.rs b/src/features/rating/mod.rs new file mode 100644 index 00000000..6bef6bc6 --- /dev/null +++ b/src/features/rating/mod.rs @@ -0,0 +1,10 @@ +//! Contains various feature implementations for retrieving snap ratings. +//! +//! [`AppService`]: service::AppService + +pub mod interface; +pub mod service; + +mod errors; +mod infrastructure; +mod use_cases; diff --git a/src/features/rating/service.rs b/src/features/rating/service.rs new file mode 100644 index 00000000..28f84991 --- /dev/null +++ b/src/features/rating/service.rs @@ -0,0 +1,12 @@ +//! Contains generation and definitions for the [`AppService`] +use crate::features::pb::app::app_server::AppServer; + +/// The general service governing retrieving ratings for the store app. +#[derive(Debug, Default)] +pub struct AppService; + +/// Builds a new [`AppServer`] using the given [`AppService`] with default parameters. +pub fn build_service() -> AppServer { + let service = AppService; + AppServer::new(service) +} diff --git a/src/features/rating/use_cases.rs b/src/features/rating/use_cases.rs new file mode 100644 index 00000000..f01d8b44 --- /dev/null +++ b/src/features/rating/use_cases.rs @@ -0,0 +1,28 @@ +//! Various things you can do with an [`AppService`]. +//! +//! [`AppService`]: crate::features::app::service::AppService +use crate::{ + app::AppContext, + features::{ + common::entities::Rating, + rating::{errors::AppError, infrastructure::get_votes_by_snap_id}, + }, +}; +use tracing::error; + +/// Retrieves votes for the snap indicated by `snap_id` for the given [`AppContext`]. +/// +/// This will return either a [`VoteSummary`], if successful, or a relevant [`AppError`]. +/// The function will fail when failing to retrieve the rating, or if some other unknown network or similar error occurs. +pub async fn get_rating(app_ctx: &AppContext, snap_id: String) -> Result { + let votes = get_votes_by_snap_id(app_ctx, &snap_id) + .await + .map_err(|error| { + error!("{error:?}"); + AppError::Unknown + })?; + + let rating = Rating::new(votes); + + Ok(rating) +} diff --git a/src/features/user/entities.rs b/src/features/user/entities.rs index 3f9ee13f..888f3596 100644 --- a/src/features/user/entities.rs +++ b/src/features/user/entities.rs @@ -1,19 +1,28 @@ +//! Various struct definitions for handling user data. + use sqlx::FromRow; use time::OffsetDateTime; use crate::features::pb::user; +/// A hash of a given user client. pub type ClientHash = String; +/// Information about a user who may be rating snaps. #[derive(Debug, Clone, FromRow)] pub struct User { + /// The user's ID pub id: i32, + /// A hash of the user's client pub client_hash: ClientHash, + /// The time the user was created pub created: OffsetDateTime, + /// The time the user was last seen pub last_seen: OffsetDateTime, } impl User { + /// Creates a new user from the given [`ClientHash`] pub fn new(client_hash: &str) -> Self { let now = OffsetDateTime::now_utc(); Self { @@ -25,17 +34,24 @@ impl User { } } +/// A Vote, as submitted by a user #[derive(Debug, Clone)] pub struct Vote { + /// The hash of the user client pub client_hash: ClientHash, + /// The ID of the snap being voted on pub snap_id: String, + /// The revision of the snap being voted on pub snap_revision: u32, + /// Whether this is a positive or negative vote pub vote_up: bool, + /// The timestamp of the vote pub timestamp: OffsetDateTime, } impl Vote { - pub(crate) fn into_dto(self) -> user::Vote { + /// Converts this vote into its wire component for transfer over the network. + pub(crate) fn into_protobuf_vote(self) -> user::Vote { let timestamp = Some(prost_types::Timestamp { seconds: self.timestamp.unix_timestamp(), nanos: 0, diff --git a/src/features/user/errors.rs b/src/features/user/errors.rs index 5c56b604..d31b3b92 100644 --- a/src/features/user/errors.rs +++ b/src/features/user/errors.rs @@ -1,15 +1,22 @@ +//! Errors related to user voting use thiserror::Error; +/// Errors that can occur when a user votes. #[derive(Error, Debug)] pub enum UserError { + /// A record could not be created for the user #[error("failed to create user record")] FailedToCreateUserRecord, + /// We were unable to delete a user with the given instance ID #[error("failed to delete user by instance id")] FailedToDeleteUserRecord, + /// We could not get a vote by a given user #[error("failed to get user vote")] FailedToGetUserVote, + /// The user was unable to cast a vote #[error("failed to cast vote")] FailedToCastVote, + /// Anything else that can go wrong #[error("unknown user error")] Unknown, } diff --git a/src/features/user/infrastructure.rs b/src/features/user/infrastructure.rs index fc23323c..a50f4af6 100644 --- a/src/features/user/infrastructure.rs +++ b/src/features/user/infrastructure.rs @@ -1,9 +1,17 @@ -use crate::{app::AppContext, features::user::errors::UserError}; +//! Infrastructure for user handling use sqlx::Row; use tracing::error; -use super::entities::{User, Vote}; +use crate::{ + app::AppContext, + features::user::{ + entities::{User, Vote}, + errors::UserError, + }, +}; +/// Create a [`User`] entry, or note that the user has recently been seen, within the current +/// [`AppContext`]. pub(crate) async fn create_or_seen_user( app_ctx: &AppContext, user: User, @@ -45,6 +53,9 @@ pub(crate) async fn create_or_seen_user( Ok(user_with_id) } +/// Deletes a [`User`] with the given [`ClientHash`] +/// +/// [`ClientHash`]: crate::features::user::entities::ClientHash pub(crate) async fn delete_user_by_client_hash( app_ctx: &AppContext, client_hash: &str, @@ -75,6 +86,9 @@ pub(crate) async fn delete_user_by_client_hash( Ok(rows.rows_affected()) } +/// Gets votes for a snap with the given ID from a given [`ClientHash`] +/// +/// [`ClientHash`]: crate::features::user::entities::ClientHash pub(crate) async fn get_snap_votes_by_client_hash( app_ctx: &AppContext, snap_id: String, @@ -132,6 +146,7 @@ pub(crate) async fn get_snap_votes_by_client_hash( Ok(votes) } +/// Saves a [`Vote`] to the database, if possible. pub(crate) async fn save_vote_to_db(app_ctx: &AppContext, vote: Vote) -> Result { let mut pool = app_ctx .infrastructure() @@ -164,6 +179,9 @@ pub(crate) async fn save_vote_to_db(app_ctx: &AppContext, vote: Vote) -> Result< Ok(result.rows_affected()) } +/// Retrieve all votes for a given [`User`], within the current [`AppContext`]. +/// +/// May be filtered for a given snap ID. pub(crate) async fn find_user_votes( app_ctx: &AppContext, client_hash: String, diff --git a/src/features/user/interface.rs b/src/features/user/interface.rs index 7c52a2cb..2ee95f8f 100644 --- a/src/features/user/interface.rs +++ b/src/features/user/interface.rs @@ -1,3 +1,4 @@ +//! Trait impls for a given [`User`] use time::OffsetDateTime; use tonic::{Request, Response, Status}; @@ -97,7 +98,10 @@ impl User for UserService { match result { Ok(votes) => { - let votes = votes.into_iter().map(|vote| vote.into_dto()).collect(); + let votes = votes + .into_iter() + .map(|vote| vote.into_protobuf_vote()) + .collect(); let payload = ListMyVotesResponse { votes }; Ok(Response::new(payload)) } @@ -121,7 +125,10 @@ impl User for UserService { match result { Ok(votes) => { - let votes = votes.into_iter().map(|vote| vote.into_dto()).collect(); + let votes = votes + .into_iter() + .map(|vote| vote.into_protobuf_vote()) + .collect(); let payload = GetSnapVotesResponse { votes }; Ok(Response::new(payload)) } @@ -130,6 +137,7 @@ impl User for UserService { } } +/// Converts a request into a [`Claims`] value. fn claims(request: &Request) -> Claims { request .extensions() @@ -138,4 +146,5 @@ fn claims(request: &Request) -> Claims { .clone() } +/// The length we expect a client hash to be, in bytes pub const EXPECTED_CLIENT_HASH_LENGTH: usize = 64; diff --git a/src/features/user/mod.rs b/src/features/user/mod.rs index 6522bea9..953e52a3 100644 --- a/src/features/user/mod.rs +++ b/src/features/user/mod.rs @@ -1,6 +1,9 @@ +//! Contains various feature implementations for autenticating users and registering their snap votes. + pub mod entities; -mod errors; -mod infrastructure; pub mod interface; pub mod service; + +mod errors; +mod infrastructure; mod use_cases; diff --git a/src/features/user/service.rs b/src/features/user/service.rs index 0362e7b5..e04c4144 100644 --- a/src/features/user/service.rs +++ b/src/features/user/service.rs @@ -1,8 +1,13 @@ +//! Definitions and utilities for building the [`UserService`] for handling [`User`] data. +//! +//! [`User`]: crate::features::user::entities::User use crate::features::pb::user::user_server::UserServer; +/// An empty struct used to construct a [`UserServer`] #[derive(Debug, Default)] pub struct UserService; +/// Builds a new [`UserServer`] with default parameters. pub fn build_service() -> UserServer { let service = UserService; UserServer::new(service) diff --git a/src/features/user/use_cases.rs b/src/features/user/use_cases.rs index a3fc457e..d49cbe99 100644 --- a/src/features/user/use_cases.rs +++ b/src/features/user/use_cases.rs @@ -1,32 +1,43 @@ +//! Helper functions to call things related to [`User`] handling. + use crate::{ app::AppContext, + features::user::infrastructure::{find_user_votes, save_vote_to_db}, features::user::{ entities::{User, Vote}, errors::UserError, infrastructure::{ - create_or_seen_user, delete_user_by_client_hash, find_user_votes, - get_snap_votes_by_client_hash, save_vote_to_db, + create_or_seen_user, delete_user_by_client_hash, get_snap_votes_by_client_hash, }, }, }; +/// Create a [`User`] entry, or note that the user has recently been seen, within the current +/// [`AppContext`]. pub async fn authenticate(app_ctx: &AppContext, id: &str) -> Result { let user = User::new(id); create_or_seen_user(app_ctx, user).await } +/// Deletes a [`User`] with the given [`ClientHash`] +/// +/// [`ClientHash`]: crate::features::user::entities::ClientHash pub async fn delete_user(app_ctx: &AppContext, client_hash: &str) -> Result<(), UserError> { let result = delete_user_by_client_hash(app_ctx, client_hash).await; result?; Ok(()) } +/// Saves a [`Vote`] to the database, if possible. pub async fn vote(app_ctx: &AppContext, vote: Vote) -> Result<(), UserError> { let result = save_vote_to_db(app_ctx, vote).await; result?; Ok(()) } +/// Gets votes for a snap with the given ID from a given [`ClientHash`] +/// +/// [`ClientHash`]: crate::features::user::entities::ClientHash pub async fn get_snap_votes( app_ctx: &AppContext, snap_id: String, @@ -35,6 +46,9 @@ pub async fn get_snap_votes( get_snap_votes_by_client_hash(app_ctx, snap_id, client_hash).await } +/// Retrieve all votes for a given [`User`], within the current [`AppContext`]. +/// +/// May be filtered for a given snap ID. pub async fn list_my_votes( app_ctx: &AppContext, client_hash: String, diff --git a/src/lib.rs b/src/lib.rs index 173111e0..07003105 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,3 +1,12 @@ +//! The [`ratings`] library represents the code necessary for the Ubuntu app center's +//! ratings backend. +//! +//! [`ratings`]: crate + +#![deny(rustdoc::broken_intra_doc_links)] +#![warn(missing_docs)] +#![warn(clippy::missing_docs_in_private_items)] + pub mod app; pub mod features; pub mod utils; diff --git a/src/utils/config.rs b/src/utils/config.rs index ad91e704..cc25da1c 100644 --- a/src/utils/config.rs +++ b/src/utils/config.rs @@ -1,24 +1,36 @@ +//! Utility functions and definitions for configuring the service. use dotenv::dotenv; use serde::Deserialize; +/// Configuration for the general app center ratings backend service. #[derive(Deserialize, Debug, Clone)] pub struct Config { + /// Environment variables to use pub env: String, + /// The host configuration pub host: String, + /// The JWT secret value pub jwt_secret: String, + /// Log level to use pub log_level: String, + /// The service name pub name: String, + /// The port to run on pub port: u16, + /// The URI of the postgres database pub postgres_uri: String, + /// The URI of the migration resource for the DB pub migration_postgres_uri: String, } impl Config { + /// Loads the configuration from environment variables pub fn load() -> envy::Result { dotenv().ok(); envy::prefixed("APP_").from_env::() } + /// Return a [`String`] representing the socket to run the service on pub fn socket(&self) -> String { let Config { port, host, .. } = self; format!("{host}:{port}") diff --git a/src/utils/infrastructure.rs b/src/utils/infrastructure.rs index f06418b5..01923217 100644 --- a/src/utils/infrastructure.rs +++ b/src/utils/infrastructure.rs @@ -1,20 +1,25 @@ -use std::error::Error; -use std::fmt::{Debug, Formatter}; -use std::sync::Arc; +//! Utilities and structs for creating server infrastructure (database, etc). +use std::{ + error::Error, + fmt::{Debug, Formatter}, + sync::Arc, +}; -use sqlx::pool::PoolConnection; -use sqlx::{postgres::PgPoolOptions, PgPool, Postgres}; +use sqlx::{pool::PoolConnection, postgres::PgPoolOptions, PgPool, Postgres}; -use crate::utils::config::Config; -use crate::utils::jwt::Jwt; +use crate::utils::{config::Config, jwt::Jwt}; +/// Resources important to the server, but are not necessarily in-memory #[derive(Clone)] pub struct Infrastructure { + /// The postgres DB pub postgres: Arc, + /// The JWT instance pub jwt: Arc, } impl Infrastructure { + /// Tries to create a new [`Infrastructure`] from the given [`Config`] pub async fn new(config: &Config) -> Result> { let uri = config.postgres_uri.clone(); let uri = uri.as_str(); @@ -28,6 +33,7 @@ impl Infrastructure { Ok(Infrastructure { postgres, jwt }) } + /// Attempt to get a pooled connection to the database pub async fn repository(&self) -> Result, sqlx::Error> { self.postgres.acquire().await } diff --git a/src/utils/jwt.rs b/src/utils/jwt.rs index b247bdc4..bdcc8a9e 100644 --- a/src/utils/jwt.rs +++ b/src/utils/jwt.rs @@ -1,3 +1,4 @@ +//! JSON Web Tokens infrastructure and utlities. use std::ops::Add; use jsonwebtoken::{DecodingKey, EncodingKey, Header, Validation}; @@ -6,23 +7,31 @@ use thiserror::Error; use time::{Duration, OffsetDateTime}; use tracing::error; +/// How many days until JWT info expires static JWT_EXPIRY_IN_DAYS: i64 = 1; +/// An error for things that can go wrong with JWT handling #[derive(Error, Debug)] pub enum JwtError { + /// The shape of the data is invalid #[error("jwt: invalid shape")] InvalidShape, + /// Anything else that can go wrong #[error("jwt: unknown error")] Unknown, } +/// Information representating a claim on a specific subject at a specific time #[derive(Debug, Serialize, Deserialize, Clone)] pub struct Claims { + /// The subject pub sub: String, + /// The expiration time pub exp: usize, } impl Claims { + /// Creates a new claim with the current datetime for the subject given by `sub`. pub fn new(sub: String) -> Self { let exp = OffsetDateTime::now_utc().add(Duration::days(JWT_EXPIRY_IN_DAYS)); let exp = exp.unix_timestamp() as usize; @@ -31,12 +40,16 @@ impl Claims { } } +/// A JWT transaction representation pub struct Jwt { + /// An encoding key for transfer encoding_key: EncodingKey, + /// A decoding key for receipt decoding_key: DecodingKey, } impl Jwt { + /// Attempts to create a new JWT representation from a given secret pub fn new(secret: &str) -> Result { let encoding_key = EncodingKey::from_base64_secret(secret)?; let decoding_key = DecodingKey::from_base64_secret(secret)?; @@ -47,6 +60,7 @@ impl Jwt { }) } + /// Encodes a token for use pub fn encode(&self, sub: String) -> Result { let claims = Claims::new(sub); @@ -56,6 +70,7 @@ impl Jwt { }) } + /// Decodes a given token received pub fn decode(&self, token: &str) -> Result { jsonwebtoken::decode::(token, &self.decoding_key, &Validation::default()) .map(|t| t.claims) diff --git a/src/utils/migrator.rs b/src/utils/migrator.rs index 614a5633..f62c0e2a 100644 --- a/src/utils/migrator.rs +++ b/src/utils/migrator.rs @@ -1,31 +1,39 @@ -use std::env; -use std::error::Error; -use std::fmt::{Debug, Formatter}; -use std::sync::Arc; +//! Resources for database migration +use std::{ + env, + error::Error, + fmt::{Debug, Formatter}, + sync::Arc, +}; use sqlx::{postgres::PgPoolOptions, PgPool}; use tracing::info; +/// The path for the migration schematics const MIGRATIONS_PATH: &str = "./sql/migrations"; +/// A wrapper for a postgres pool for doing migrations #[derive(Clone)] pub struct Migrator { + /// A connection pool to a postgres resource. pub pool: Arc, } -#[allow(dead_code)] impl Migrator { + /// Attempts to create a new Migration object from the given resource URI. pub async fn new(uri: &str) -> Result> { let pool = PgPoolOptions::new().max_connections(1).connect(uri).await?; let pool = Arc::new(pool); Ok(Migrator { pool }) } + /// Get the paths for migration backups and templates fn migrations_path() -> String { let snap_path = std::env::var("SNAP").unwrap_or("./sql".to_string()); format!("{}/migrations", snap_path) } + /// Runs a database migration as specified by the URI given on input. pub async fn run(&self) -> Result<(), sqlx::Error> { match env::current_dir() { Ok(cur_dir) => info!("Current directory: {}", cur_dir.display()), @@ -38,6 +46,8 @@ impl Migrator { Ok(()) } + /// Attempts to revert a migration to a previous good state. + #[allow(dead_code)] pub async fn revert(&self) -> Result<(), sqlx::Error> { let m = sqlx::migrate::Migrator::new(std::path::Path::new(MIGRATIONS_PATH)).await?; diff --git a/src/utils/mod.rs b/src/utils/mod.rs index dea872e7..b6ca5d5b 100644 --- a/src/utils/mod.rs +++ b/src/utils/mod.rs @@ -1,8 +1,10 @@ -pub mod config; -pub mod infrastructure; -pub mod jwt; -pub mod migrator; +//! Helper utilties for the infrastructure layer pub use config::Config; pub use infrastructure::Infrastructure; pub use migrator::Migrator; + +pub mod config; +pub mod infrastructure; +pub mod jwt; +pub mod migrator; diff --git a/tests/helpers/data_faker.rs b/tests/helpers/data_faker.rs index ebc99ef6..cfea8a01 100644 --- a/tests/helpers/data_faker.rs +++ b/tests/helpers/data_faker.rs @@ -1,3 +1,5 @@ +use std::fmt::Write; + use rand::distributions::Alphanumeric; use rand::Rng; use sha2::{Digest, Sha256}; @@ -7,12 +9,16 @@ pub fn rnd_sha_256() -> String { let data = rnd_string(100); let mut hasher = Sha256::new(); hasher.update(data); - let result = hasher.finalize(); - let result: String = result + + hasher + .finalize() .iter() - .map(|b| format!("{:02x}", b)) - .collect::(); - result + .fold(String::new(), |mut output, b| { + // This ignores the error without the overhead of unwrap/expect, + // This is okay because writing to a string can't fail (barring OOM which won't happen) + let _ = write!(output, "{b:02x}"); + output + }) } pub fn rnd_id() -> String {