diff --git a/.github/workflows/build.yaml b/.github/workflows/build-and-test.yaml similarity index 61% rename from .github/workflows/build.yaml rename to .github/workflows/build-and-test.yaml index 6ca3be8d..4f0bf9bb 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build-and-test.yaml @@ -1,4 +1,4 @@ -name: Build +name: Build and test on: workflow_call: @@ -36,14 +36,21 @@ jobs: token: ${{ secrets.GITHUB_TOKEN }} args: --all-features - - name: Build binary - uses: actions-rs/cargo@v1 - with: - command: build - args: --release --all-features + # - name: Build binary + # uses: actions-rs/cargo@v1 + # with: + # command: build + # args: --release --all-features - - name: Upload binary - uses: actions/upload-artifact@v3 - with: - name: ratings - path: target/release/ratings + # - name: Upload binary + # uses: actions/upload-artifact@v3 + # with: + # name: ratings + # path: target/release/ratings + + - name: Setup and Run Tests + run: | + cp example.env .env + cargo install cargo-make + cargo make db-up + cargo make full-test diff --git a/.github/workflows/pull-request.yaml b/.github/workflows/pull-request.yaml index bfccc447..8efd62ab 100644 --- a/.github/workflows/pull-request.yaml +++ b/.github/workflows/pull-request.yaml @@ -11,6 +11,6 @@ concurrency: jobs: build: - uses: ./.github/workflows/build.yaml + uses: ./.github/workflows/build-and-test.yaml cla: uses: ./.github/workflows/cla.yaml diff --git a/.github/workflows/push.yaml b/.github/workflows/push.yaml index 068e26bd..ad04f226 100644 --- a/.github/workflows/push.yaml +++ b/.github/workflows/push.yaml @@ -11,4 +11,4 @@ concurrency: jobs: build: - uses: ./.github/workflows/build.yaml + uses: ./.github/workflows/build-and-test.yaml diff --git a/.gitignore b/.gitignore index b50ab416..ca23bcbe 100644 --- a/.gitignore +++ b/.gitignore @@ -8,6 +8,7 @@ *.rock *.nix +/proto/*.rs venv/ build/ *.charm @@ -15,3 +16,4 @@ build/ .coverage __pycache__/ *.py[cod] +server.pid diff --git a/build.rs b/build.rs index 093db1d2..2bb6c963 100644 --- a/build.rs +++ b/build.rs @@ -1,4 +1,10 @@ +use std::path::Path; + fn main() -> Result<(), Box> { + // Define the path to the output directory within the `src` folder + let out_dir = Path::new("proto"); + std::fs::create_dir_all(out_dir)?; + let descriptor_set_path = format!( "{}/ratings_descriptor.bin", std::env::var("OUT_DIR").unwrap() @@ -8,11 +14,13 @@ fn main() -> Result<(), Box> { "proto/ratings_features_app.proto", "proto/ratings_features_chart.proto", "proto/ratings_features_user.proto", + "proto/ratings_features_common.proto", ]; tonic_build::configure() .build_server(true) .file_descriptor_set_path(descriptor_set_path) + .out_dir(out_dir) .compile(files, &["proto"])?; Ok(()) diff --git a/example.env b/example.env index 0a385dbd..9eac6810 100644 --- a/example.env +++ b/example.env @@ -1,7 +1,7 @@ APP_ENV=dev APP_HOST=0.0.0.0 APP_JWT_SECRET=deadbeef -APP_LOG_LEVEL=info +APP_LOG_LEVEL=error APP_NAME=ratings APP_PORT=8080 # Update this with some real PostgreSQL details diff --git a/proto/ratings_features_app.proto b/proto/ratings_features_app.proto index e6494e4e..ac40cffb 100644 --- a/proto/ratings_features_app.proto +++ b/proto/ratings_features_app.proto @@ -2,6 +2,8 @@ syntax = "proto3"; package ratings.features.app; +import "ratings_features_common.proto"; + service App { rpc GetRating (GetRatingRequest) returns (GetRatingResponse) {} } @@ -11,20 +13,5 @@ message GetRatingRequest { } message GetRatingResponse { - Rating rating = 1; -} - -message Rating { - string snap_id = 1; - uint64 total_votes = 2; - RatingsBand ratings_band = 3; -} - -enum RatingsBand { - VERY_GOOD = 0; - GOOD = 1; - NEUTRAL = 2; - POOR = 3; - VERY_POOR = 4; - INSUFFICIENT_VOTES = 5; + ratings.features.common.Rating rating = 1; } diff --git a/proto/ratings_features_chart.proto b/proto/ratings_features_chart.proto index d5f1b8c4..19f39d37 100644 --- a/proto/ratings_features_chart.proto +++ b/proto/ratings_features_chart.proto @@ -2,30 +2,24 @@ syntax = "proto3"; package ratings.features.chart; +import "ratings_features_common.proto"; + service Chart { rpc GetChart (GetChartRequest) returns (GetChartResponse) {} } message GetChartRequest { Timeframe timeframe = 1; - ChartType type = 2; -} - -enum ChartType { - CHART_TYPE_TOP_UNSPECIFIED = 0; - CHART_TYPE_TOP = 1; } message GetChartResponse { Timeframe timeframe = 1; - ChartType type = 2; - repeated ChartData ordered_chart_data = 3; + repeated ChartData ordered_chart_data = 2; } message ChartData { - string app = 1; - uint64 total_up_votes = 2; - uint64 total_down_votes = 3; + float raw_rating = 1; + ratings.features.common.Rating rating = 2; } enum Timeframe { diff --git a/proto/ratings_features_common.proto b/proto/ratings_features_common.proto new file mode 100644 index 00000000..84a53d5e --- /dev/null +++ b/proto/ratings_features_common.proto @@ -0,0 +1,18 @@ +syntax = "proto3"; + +package ratings.features.common; + +message Rating { + string snap_id = 1; + uint64 total_votes = 2; + RatingsBand ratings_band = 3; +} + +enum RatingsBand { + VERY_GOOD = 0; + GOOD = 1; + NEUTRAL = 2; + POOR = 3; + VERY_POOR = 4; + INSUFFICIENT_VOTES = 5; +} diff --git a/snap/snapcraft.yaml b/snap/snapcraft.yaml index 24a52ce4..2d46bb36 100644 --- a/snap/snapcraft.yaml +++ b/snap/snapcraft.yaml @@ -1,6 +1,6 @@ name: ratings base: core22 -version: '1.6' +version: '1.8' license: GPL-3.0 summary: Ubuntu App Ratings Service description: | diff --git a/src/app/interfaces/authentication.rs b/src/app/interfaces/authentication.rs index 459ea548..fab1436e 100644 --- a/src/app/interfaces/authentication.rs +++ b/src/app/interfaces/authentication.rs @@ -2,8 +2,7 @@ use http::header; use tonic::{Request, Status}; use tracing::error; -use crate::app::context::AppContext; -use crate::app::RequestContext; +use crate::app::{context::AppContext, RequestContext}; pub fn authentication(req: Request<()>) -> Result, Status> { let app_ctx = req.extensions().get::().unwrap().clone(); diff --git a/src/app/interfaces/middleware.rs b/src/app/interfaces/middleware.rs index 9ead934f..df28f25a 100644 --- a/src/app/interfaces/middleware.rs +++ b/src/app/interfaces/middleware.rs @@ -1,7 +1,6 @@ use std::pin::Pin; -use hyper::service::Service; -use hyper::Body; +use hyper::{service::Service, Body}; use tonic::body::BoxBody; use tower::Layer; diff --git a/src/app/interfaces/routes.rs b/src/app/interfaces/routes.rs index ed9e4cb5..2122b14a 100644 --- a/src/app/interfaces/routes.rs +++ b/src/app/interfaces/routes.rs @@ -1,7 +1,7 @@ use tonic::transport::server::Router; use tonic_reflection::server::ServerReflection; -use crate::features::{app, user}; +use crate::features::{app, chart, user}; pub fn build_reflection_service( ) -> tonic_reflection::server::ServerReflectionServer { @@ -16,6 +16,10 @@ pub fn build_reflection_service( pub fn build_servers(router: Router) -> Router { let user_service = user::service::build_service(); let app_service = app::service::build_service(); + let chart_service = chart::service::build_service(); - router.add_service(user_service).add_service(app_service) + router + .add_service(user_service) + .add_service(app_service) + .add_service(chart_service) } diff --git a/src/features/app/infrastructure.rs b/src/features/app/infrastructure.rs index 6e8f30e1..76c74dff 100644 --- a/src/features/app/infrastructure.rs +++ b/src/features/app/infrastructure.rs @@ -1,13 +1,13 @@ -use crate::app::AppContext; -use sqlx::Row; +use crate::{ + app::AppContext, + features::{app::errors::AppError, common::entities::VoteSummary}, +}; use tracing::error; -use super::{entities::Vote, errors::AppError}; - pub(crate) async fn get_votes_by_snap_id( app_ctx: &AppContext, snap_id: &str, -) -> Result, AppError> { +) -> Result { let mut pool = app_ctx .infrastructure() .repository() @@ -16,32 +16,27 @@ pub(crate) async fn get_votes_by_snap_id( error!("{error:?}"); AppError::FailedToGetRating })?; - let result = sqlx::query( + + let result = sqlx::query_as::<_, VoteSummary>( r#" - SELECT - votes.id, - votes.snap_id, - votes.vote_up - FROM - votes - WHERE - votes.snap_id = $1 - "#, + SELECT + votes.snap_id, + COUNT(*) AS total_votes, + COUNT(*) FILTER (WHERE votes.vote_up) AS positive_votes + FROM + votes + WHERE + votes.snap_id = $1 + GROUP BY votes.snap_id + "#, ) .bind(snap_id) - .fetch_all(&mut *pool) + .fetch_one(&mut *pool) .await .map_err(|error| { error!("{error:?}"); AppError::Unknown })?; - let votes: Vec = result - .into_iter() - .map(|row| Vote { - vote_up: row.get("vote_up"), - }) - .collect(); - - Ok(votes) + Ok(result) } diff --git a/src/features/app/interface.rs b/src/features/app/interface.rs index 08b29354..2c568f1d 100644 --- a/src/features/app/interface.rs +++ b/src/features/app/interface.rs @@ -1,17 +1,11 @@ use crate::app::AppContext; -use self::protobuf::{App, GetRatingRequest, GetRatingResponse}; -pub use protobuf::app_server; +use crate::features::{ + app::{service::AppService, use_cases}, + pb::app::{app_server::App, GetRatingRequest, GetRatingResponse}, +}; use tonic::{Request, Response, Status}; -use super::{service::AppService, use_cases}; - -pub mod protobuf { - pub use self::app_server::App; - - tonic::include_proto!("ratings.features.app"); -} - #[tonic::async_trait] impl App for AppService { #[tracing::instrument] diff --git a/src/features/app/mod.rs b/src/features/app/mod.rs index 6522bea9..d273b6e9 100644 --- a/src/features/app/mod.rs +++ b/src/features/app/mod.rs @@ -1,4 +1,3 @@ -pub mod entities; mod errors; mod infrastructure; pub mod interface; diff --git a/src/features/app/service.rs b/src/features/app/service.rs index 78067566..ef223907 100644 --- a/src/features/app/service.rs +++ b/src/features/app/service.rs @@ -1,4 +1,4 @@ -use super::interface::app_server::AppServer; +use crate::features::pb::app::app_server::AppServer; #[derive(Debug, Default)] pub struct AppService; diff --git a/src/features/app/use_cases.rs b/src/features/app/use_cases.rs index e36bdf12..49233beb 100644 --- a/src/features/app/use_cases.rs +++ b/src/features/app/use_cases.rs @@ -1,8 +1,12 @@ -use crate::app::AppContext; +use crate::{ + app::AppContext, + features::{ + app::{errors::AppError, infrastructure::get_votes_by_snap_id}, + common::entities::Rating, + }, +}; use tracing::error; -use super::{entities::Rating, errors::AppError, infrastructure::get_votes_by_snap_id}; - pub async fn get_rating(app_ctx: &AppContext, snap_id: String) -> Result { let votes = get_votes_by_snap_id(app_ctx, &snap_id) .await @@ -11,7 +15,7 @@ pub async fn get_rating(app_ctx: &AppContext, snap_id: String) -> Result, +} + +impl Chart { + pub fn new(timeframe: pb::Timeframe, data: Vec) -> Self { + let mut chart_data: Vec = + data.into_iter().map(ChartData::from_vote_summary).collect(); + + chart_data.sort_by(|a, b| { + b.raw_rating + .partial_cmp(&a.raw_rating) + .unwrap_or(std::cmp::Ordering::Equal) + }); + + // Take only the first 20 elements from the sorted chart_data + let top_20: Vec = chart_data.into_iter().take(20).collect(); + + Chart { + timeframe, + chart_data: top_20, + } + } +} + +#[derive(Debug, Clone, FromRow)] +pub struct ChartData { + pub raw_rating: f32, + pub rating: Rating, +} + +impl ChartData { + pub fn from_vote_summary(vote_summary: VoteSummary) -> Self { + let (raw_rating, ratings_band) = calculate_band(&vote_summary); + let rating = Rating { + snap_id: vote_summary.snap_id, + total_votes: vote_summary.total_votes as u64, + ratings_band, + }; + let raw_rating = raw_rating.unwrap_or(0.0) as f32; + Self { raw_rating, rating } + } + + pub fn into_dto(self) -> pb::ChartData { + pb::ChartData { + raw_rating: self.raw_rating, + rating: Some(self.rating.into_dto()), + } + } +} diff --git a/src/features/chart/errors.rs b/src/features/chart/errors.rs new file mode 100644 index 00000000..897c9fb5 --- /dev/null +++ b/src/features/chart/errors.rs @@ -0,0 +1,11 @@ +use thiserror::Error; + +#[derive(Error, Debug)] +pub enum ChartError { + #[error("failed to get chart for timeframe")] + FailedToGetChart, + #[error("unknown chart error")] + Unknown, + #[error("could not find data for given timeframe")] + NotFound, +} diff --git a/src/features/chart/infrastructure.rs b/src/features/chart/infrastructure.rs new file mode 100644 index 00000000..4ed0f07b --- /dev/null +++ b/src/features/chart/infrastructure.rs @@ -0,0 +1,50 @@ +use crate::{ + app::AppContext, + features::{chart::errors::ChartError, common::entities::VoteSummary, pb::chart::Timeframe}, +}; +use tracing::error; + +pub(crate) async fn get_votes_summary_by_timeframe( + app_ctx: &AppContext, + timeframe: Timeframe, +) -> Result, ChartError> { + let mut pool = app_ctx + .infrastructure() + .repository() + .await + .map_err(|error| { + error!("{error:?}"); + ChartError::FailedToGetChart + })?; + + // Generate WHERE clause based on timeframe + let where_clause = match timeframe { + Timeframe::Week => "WHERE votes.created >= NOW() - INTERVAL '1 week'", + Timeframe::Month => "WHERE votes.created >= NOW() - INTERVAL '1 month'", + Timeframe::Unspecified => "", // Adjust as needed for Unspecified case + }; + + let query = format!( + r#" + SELECT + votes.snap_id, + COUNT(*) AS total_votes, + COUNT(*) FILTER (WHERE votes.vote_up) AS positive_votes + FROM + votes + {} + GROUP BY votes.snap_id + "#, + where_clause + ); + + let result = sqlx::query_as::<_, VoteSummary>(&query) + .fetch_all(&mut *pool) + .await + .map_err(|error| { + error!("{error:?}"); + ChartError::NotFound + })?; + + Ok(result) +} diff --git a/src/features/chart/interface.rs b/src/features/chart/interface.rs new file mode 100644 index 00000000..61b5ddf1 --- /dev/null +++ b/src/features/chart/interface.rs @@ -0,0 +1,52 @@ +use crate::{ + app::AppContext, + features::{ + chart::{errors::ChartError, service::ChartService, use_cases}, + pb::chart::{chart_server::Chart, GetChartRequest, GetChartResponse, Timeframe}, + }, +}; +use tonic::{Request, Response, Status}; + +#[tonic::async_trait] +impl Chart for ChartService { + #[tracing::instrument] + async fn get_chart( + &self, + request: Request, + ) -> Result, Status> { + let app_ctx = request.extensions().get::().unwrap().clone(); + + let GetChartRequest { timeframe } = request.into_inner(); + + let timeframe = match timeframe { + 0 => Timeframe::Unspecified, + 1 => Timeframe::Week, + 2 => Timeframe::Month, + _ => Timeframe::Unspecified, + }; + + let result = use_cases::get_chart(&app_ctx, timeframe).await; + + match result { + Ok(result) => { + let ordered_chart_data = result + .chart_data + .into_iter() + .map(|chart_data| chart_data.into_dto()) + .collect(); + + let payload = GetChartResponse { + timeframe: timeframe.into(), + ordered_chart_data, + }; + Ok(Response::new(payload)) + } + Err(error) => match error { + ChartError::NotFound => { + Err(Status::not_found("Cannot find data for given timeframe.")) + } + _ => Err(Status::unknown("Internal server error")), + }, + } + } +} diff --git a/src/features/chart/mod.rs b/src/features/chart/mod.rs new file mode 100644 index 00000000..6522bea9 --- /dev/null +++ b/src/features/chart/mod.rs @@ -0,0 +1,6 @@ +pub mod entities; +mod errors; +mod infrastructure; +pub mod interface; +pub mod service; +mod use_cases; diff --git a/src/features/chart/service.rs b/src/features/chart/service.rs new file mode 100644 index 00000000..a6321e2f --- /dev/null +++ b/src/features/chart/service.rs @@ -0,0 +1,9 @@ +use crate::features::pb::chart::chart_server::ChartServer; + +#[derive(Debug, Default)] +pub struct 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 new file mode 100644 index 00000000..e94e2caa --- /dev/null +++ b/src/features/chart/use_cases.rs @@ -0,0 +1,23 @@ +use crate::{ + app::AppContext, + features::{ + chart::{ + entities::Chart, errors::ChartError, infrastructure::get_votes_summary_by_timeframe, + }, + pb::chart::Timeframe, + }, +}; +use tracing::error; + +pub async fn get_chart(app_ctx: &AppContext, timeframe: Timeframe) -> Result { + let votes = get_votes_summary_by_timeframe(app_ctx, timeframe) + .await + .map_err(|error| { + error!("{error:?}"); + ChartError::Unknown + })?; + + let chart = Chart::new(timeframe, votes); + + Ok(chart) +} diff --git a/src/features/app/entities.rs b/src/features/common/entities.rs similarity index 72% rename from src/features/app/entities.rs rename to src/features/common/entities.rs index cda672de..87da1f70 100644 --- a/src/features/app/entities.rs +++ b/src/features/common/entities.rs @@ -1,8 +1,8 @@ use sqlx::FromRow; -use super::interface::protobuf; +use crate::features::pb::common as pb; -const INSUFFICIENT_VOTES_QUANTITY: usize = 25; +const INSUFFICIENT_VOTES_QUANTITY: i64 = 25; #[derive(Debug, Clone, PartialEq, Eq)] pub enum RatingsBand { @@ -43,18 +43,17 @@ pub struct Rating { } impl Rating { - pub fn new(snap_id: String, votes: Vec) -> Self { - let total_votes = votes.len(); - let ratings_band = calculate_band(votes); + pub fn new(votes: VoteSummary) -> Self { + let (_, ratings_band) = calculate_band(&votes); Self { - snap_id, - total_votes: total_votes as u64, + snap_id: votes.snap_id, + total_votes: votes.total_votes as u64, ratings_band, } } - pub(crate) fn into_dto(self) -> protobuf::Rating { - protobuf::Rating { + pub(crate) fn into_dto(self) -> pb::Rating { + pb::Rating { snap_id: self.snap_id, total_votes: self.total_votes, ratings_band: self.ratings_band as i32, @@ -62,27 +61,31 @@ impl Rating { } } +#[derive(Debug, Clone, FromRow)] +pub struct VoteSummary { + pub snap_id: String, + pub total_votes: i64, + pub positive_votes: i64, +} + #[derive(Debug, Clone)] pub struct Vote { pub vote_up: bool, } -fn calculate_band(votes: Vec) -> RatingsBand { - let total_ratings = votes.len(); - if total_ratings < INSUFFICIENT_VOTES_QUANTITY { - return RatingsBand::InsufficientVotes; +pub fn calculate_band(votes: &VoteSummary) -> (Option, RatingsBand) { + if votes.total_votes < INSUFFICIENT_VOTES_QUANTITY { + return (None, RatingsBand::InsufficientVotes); } - let positive_ratings = votes - .into_iter() - .filter(|vote| vote.vote_up) - .collect::>() - .len(); - let adjusted_ratio = confidence_interval_lower_bound(positive_ratings, total_ratings); - - RatingsBand::from_value(adjusted_ratio) + let adjusted_ratio = confidence_interval_lower_bound(votes.positive_votes, votes.total_votes); + + ( + Some(adjusted_ratio), + RatingsBand::from_value(adjusted_ratio), + ) } -fn confidence_interval_lower_bound(positive_ratings: usize, total_ratings: usize) -> f64 { +fn confidence_interval_lower_bound(positive_ratings: i64, total_ratings: i64) -> f64 { if total_ratings == 0 { return 0.0; } @@ -136,7 +139,7 @@ mod tests { let mut last_lower_bound = 0.0; for total_ratings in (100..1000).step_by(100) { - let positive_ratings = (total_ratings as f64 * ratio).round() as usize; + let positive_ratings = (total_ratings as f64 * ratio).round() as i64; let new_lower_bound = confidence_interval_lower_bound(positive_ratings, total_ratings); let raw_positive_ratio = positive_ratings as f64 / total_ratings as f64; @@ -152,23 +155,39 @@ mod tests { #[test] fn test_insufficient_votes() { - let votes = vec![Vote { vote_up: true }]; - let band = calculate_band(votes); + let votes = VoteSummary { + snap_id: 1.to_string(), + total_votes: 1, + positive_votes: 1, + }; + let (rating, band) = calculate_band(&votes); assert_eq!( band, RatingsBand::InsufficientVotes, "Should return InsufficientVotes when not enough votes exist for a given Snap." + ); + assert!( + rating.is_none(), + "Should return band = None for insufficient votes." ) } #[test] fn test_sufficient_votes() { - let votes = vec![Vote { vote_up: true }; INSUFFICIENT_VOTES_QUANTITY]; - let band = calculate_band(votes); + let votes = VoteSummary { + snap_id: 1.to_string(), + total_votes: 100, + positive_votes: 100, + }; + let (rating, band) = calculate_band(&votes); assert_eq!( band, RatingsBand::VeryGood, "Should return very good for a sufficient number of all positive votes." + ); + assert!( + rating > Some(0.7), + "Should return fairly positive raw rating for this ration and volume of positive votes." ) } } diff --git a/src/features/common/mod.rs b/src/features/common/mod.rs new file mode 100644 index 00000000..0b8f0b5a --- /dev/null +++ b/src/features/common/mod.rs @@ -0,0 +1 @@ +pub mod entities; diff --git a/src/features/mod.rs b/src/features/mod.rs index 836e47e5..4824253a 100644 --- a/src/features/mod.rs +++ b/src/features/mod.rs @@ -1,2 +1,5 @@ pub mod app; +pub mod chart; +mod common; +mod pb; pub mod user; diff --git a/src/features/pb.rs b/src/features/pb.rs new file mode 100644 index 00000000..78e91188 --- /dev/null +++ b/src/features/pb.rs @@ -0,0 +1,15 @@ +pub mod app { + include!("../../proto/ratings.features.app.rs"); +} + +pub mod common { + include!("../../proto/ratings.features.common.rs"); +} + +pub mod user { + include!("../../proto/ratings.features.user.rs"); +} + +pub mod chart { + include!("../../proto/ratings.features.chart.rs"); +} diff --git a/src/features/user/entities.rs b/src/features/user/entities.rs index 221b7427..3f9ee13f 100644 --- a/src/features/user/entities.rs +++ b/src/features/user/entities.rs @@ -1,7 +1,7 @@ use sqlx::FromRow; use time::OffsetDateTime; -use super::interface::protobuf; +use crate::features::pb::user; pub type ClientHash = String; @@ -35,13 +35,13 @@ pub struct Vote { } impl Vote { - pub(crate) fn into_dto(self) -> protobuf::Vote { + pub(crate) fn into_dto(self) -> user::Vote { let timestamp = Some(prost_types::Timestamp { seconds: self.timestamp.unix_timestamp(), nanos: 0, }); - protobuf::Vote { + user::Vote { snap_id: self.snap_id, snap_revision: self.snap_revision as i32, vote_up: self.vote_up, diff --git a/src/features/user/interface.rs b/src/features/user/interface.rs index ab117117..d9469569 100644 --- a/src/features/user/interface.rs +++ b/src/features/user/interface.rs @@ -1,27 +1,18 @@ use time::OffsetDateTime; - use tonic::{Request, Response, Status}; -pub use protobuf::user_server; - -use crate::app::AppContext; -use crate::utils::jwt::Claims; - -use super::entities::Vote; -use super::service::UserService; -use super::use_cases; - -use self::protobuf::{ - AuthenticateRequest, AuthenticateResponse, GetSnapVotesRequest, GetSnapVotesResponse, - ListMyVotesRequest, ListMyVotesResponse, User, VoteRequest, +use crate::{ + app::AppContext, + features::{ + pb::user::{ + user_server::User, AuthenticateRequest, AuthenticateResponse, GetSnapVotesRequest, + GetSnapVotesResponse, ListMyVotesRequest, ListMyVotesResponse, VoteRequest, + }, + user::{entities::Vote, service::UserService, use_cases}, + }, + utils::jwt::Claims, }; -pub mod protobuf { - pub use self::user_server::User; - - tonic::include_proto!("ratings.features.user"); -} - #[tonic::async_trait] impl User for UserService { #[tracing::instrument] diff --git a/src/features/user/service.rs b/src/features/user/service.rs index f594dd81..0362e7b5 100644 --- a/src/features/user/service.rs +++ b/src/features/user/service.rs @@ -1,4 +1,4 @@ -use super::interface::user_server::UserServer; +use crate::features::pb::user::user_server::UserServer; #[derive(Debug, Default)] pub struct UserService; diff --git a/src/features/user/use_cases.rs b/src/features/user/use_cases.rs index 4605fc6f..a3fc457e 100644 --- a/src/features/user/use_cases.rs +++ b/src/features/user/use_cases.rs @@ -1,10 +1,13 @@ -use crate::app::AppContext; -use crate::features::user::infrastructure::{find_user_votes, save_vote_to_db}; - -use super::entities::{User, Vote}; -use super::errors::UserError; -use super::infrastructure::{ - create_or_seen_user, delete_user_by_client_hash, get_snap_votes_by_client_hash, +use crate::{ + app::AppContext, + 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, + }, + }, }; pub async fn authenticate(app_ctx: &AppContext, id: &str) -> Result { diff --git a/tests/app_tests/lifecycle_test.rs b/tests/app_tests/lifecycle_test.rs index 80e7d991..4976c3a6 100644 --- a/tests/app_tests/lifecycle_test.rs +++ b/tests/app_tests/lifecycle_test.rs @@ -7,11 +7,10 @@ use ratings::{ use super::super::helpers::with_lifecycle::with_lifecycle; use crate::helpers::test_data::TestData; use crate::helpers::vote_generator::generate_votes; -use crate::helpers::{self, client_app::pb::RatingsBand, client_app::AppClient}; -use crate::helpers::{ - client_user::{pb::AuthenticateResponse, UserClient}, - data_faker, -}; +use crate::helpers::{self, client_app::AppClient}; +use crate::helpers::{client_user::UserClient, data_faker}; +use crate::pb::common::RatingsBand; +use crate::pb::user::AuthenticateResponse; #[tokio::test] async fn app_lifecycle_test() -> Result<(), Box> { @@ -26,6 +25,7 @@ async fn app_lifecycle_test() -> Result<(), Box> { token: None, app_client: Some(AppClient::new(&config.socket())), snap_id: Some(data_faker::rnd_id()), + chart_client: None, }; with_lifecycle(async { diff --git a/tests/chart_tests/lifecycle_test.rs b/tests/chart_tests/lifecycle_test.rs new file mode 100644 index 00000000..9dd9bc40 --- /dev/null +++ b/tests/chart_tests/lifecycle_test.rs @@ -0,0 +1,232 @@ +use futures::FutureExt; +use ratings::{ + app::AppContext, + utils::{Config, Infrastructure}, +}; + +use super::super::helpers::with_lifecycle::with_lifecycle; +use crate::helpers::vote_generator::generate_votes; +use crate::helpers::{self, client_app::AppClient}; +use crate::helpers::{client_user::UserClient, data_faker}; +use crate::pb::common::RatingsBand; +use crate::pb::user::AuthenticateResponse; +use crate::{ + helpers::{client_chart::ChartClient, test_data::TestData}, + pb::{chart::Timeframe, common::Rating}, +}; + +#[tokio::test] +async fn chart_lifecycle_test() -> Result<(), Box> { + let config = Config::load()?; + let infra = Infrastructure::new(&config).await?; + let app_ctx = AppContext::new(&config, infra); + + let data = TestData { + user_client: Some(UserClient::new(&config.socket())), + app_ctx, + id: None, + token: None, + app_client: Some(AppClient::new(&config.socket())), + snap_id: Some(data_faker::rnd_id()), + chart_client: Some(ChartClient::new(&config.socket())), + }; + + with_lifecycle(async { + vote_once(data.clone()) + .then(multiple_votes) + .then(timeframed_votes_dont_appear) + .await; + }) + .await; + Ok(()) +} + +// Does an app voted against once appear correctly in the chart? +async fn vote_once(mut data: TestData) -> TestData { + let vote_up = true; + + // Fill up chart with other votes so ours doesn't appear + for _ in 0..20 { + generate_votes(&data_faker::rnd_id(), 111, vote_up, 25, data.clone()) + .await + .expect("Votes should succeed"); + } + + let vote_up = true; + + generate_votes( + &data.snap_id.clone().unwrap(), + 111, + vote_up, + 1, + data.clone(), + ) + .await + .expect("Votes should succeed"); + + let id: String = helpers::data_faker::rnd_sha_256(); + data.id = Some(id.to_string()); + + let client = data.user_client.clone().unwrap(); + let response: AuthenticateResponse = client.authenticate(&id).await.unwrap().into_inner(); + let token: String = response.token; + data.token = Some(token.to_string()); + + let timeframe = Timeframe::Unspecified; + + let chart_data_result = data + .clone() + .chart_client + .unwrap() + .get_chart(timeframe, &data.token.clone().unwrap()) + .await + .expect("Get Chart should succeed") + .into_inner() + .ordered_chart_data; + + let result = chart_data_result.into_iter().find(|chart_data| { + if let Some(rating) = &chart_data.rating { + rating.snap_id == data.snap_id.clone().unwrap() + } else { + false + } + }); + + // Should not appear in chart + assert_eq!(result, None); + + data +} + +// Does an app voted against multiple times appear correctly in the chart? +async fn multiple_votes(mut data: TestData) -> TestData { + let vote_up = true; + let expected_raw_rating = 0.8; + let expected_rating = Rating { + snap_id: data.snap_id.clone().unwrap(), + total_votes: 101, + ratings_band: RatingsBand::VeryGood.into(), + }; + + // This should rank our snap_id at the top of the chart + generate_votes( + &data.snap_id.clone().unwrap(), + 111, + vote_up, + 100, + data.clone(), + ) + .await + .expect("Votes should succeed"); + + let id: String = helpers::data_faker::rnd_sha_256(); + data.id = Some(id.to_string()); + + let client = data.user_client.clone().unwrap(); + let response: AuthenticateResponse = client.authenticate(&id).await.unwrap().into_inner(); + let token: String = response.token; + data.token = Some(token.to_string()); + + let timeframe = Timeframe::Unspecified; + + let chart_data_result = data + .clone() + .chart_client + .unwrap() + .get_chart(timeframe, &data.token.clone().unwrap()) + .await + .expect("Get Chart should succeed") + .into_inner() + .ordered_chart_data; + + // Should be at the top of the chart + if let Some(chart_data) = chart_data_result.first() { + let actual_rating = chart_data.rating.clone().expect("Rating should exist"); + let actual_raw_rating = chart_data.raw_rating; + + assert_eq!(expected_rating, actual_rating); + assert!(expected_raw_rating < actual_raw_rating); + } else { + panic!("No chart data available"); + } + + data +} + +// Does the Timeframe correctly filter out app data? +async fn timeframed_votes_dont_appear(mut data: TestData) -> TestData { + let mut conn = data.repository().await.unwrap(); + + // Timewarp the votes back two months so they are out of the requested timeframe + sqlx::query("UPDATE votes SET created = created - INTERVAL '2 months' WHERE snap_id = $1") + .bind(&data.snap_id.clone().unwrap()) + .execute(&mut *conn) + .await + .unwrap(); + + let id: String = helpers::data_faker::rnd_sha_256(); + data.id = Some(id.to_string()); + + let client = data.user_client.clone().unwrap(); + let response: AuthenticateResponse = client.authenticate(&id).await.unwrap().into_inner(); + let token: String = response.token; + data.token = Some(token.to_string()); + + let timeframe = Timeframe::Month; + + let chart_data_result = data + .clone() + .chart_client + .unwrap() + .get_chart(timeframe, &data.token.clone().unwrap()) + .await + .expect("Get Chart should succeed") + .into_inner() + .ordered_chart_data; + + let result = chart_data_result.into_iter().find(|chart_data| { + if let Some(rating) = &chart_data.rating { + rating.snap_id == data.snap_id.clone().unwrap() + } else { + false + } + }); + + // Should no longer find the ratings as they are too old + assert_eq!(result, None); + + let expected_raw_rating = 0.8; + let expected_rating = Rating { + snap_id: data.snap_id.clone().unwrap(), + total_votes: 101, + ratings_band: RatingsBand::VeryGood.into(), + }; + + // Unspecified timeframe should now pick up the ratings again + let timeframe = Timeframe::Unspecified; + let chart_data_result = data + .clone() + .chart_client + .unwrap() + .get_chart(timeframe, &data.token.clone().unwrap()) + .await + .expect("Get Chart should succeed") + .into_inner() + .ordered_chart_data; + + let result = chart_data_result.into_iter().find(|chart_data| { + if let Some(rating) = &chart_data.rating { + rating.snap_id == data.snap_id.clone().unwrap() + } else { + false + } + }); + + let actual_rating = result.clone().unwrap().rating.unwrap(); + let actual_raw_rating = result.unwrap().raw_rating; + + assert_eq!(expected_rating, actual_rating); + assert!(expected_raw_rating < actual_raw_rating); + + data +} diff --git a/tests/helpers/client_app.rs b/tests/helpers/client_app.rs index 155f3373..e29aeffe 100644 --- a/tests/helpers/client_app.rs +++ b/tests/helpers/client_app.rs @@ -1,10 +1,8 @@ use tonic::{metadata::MetadataValue, transport::Endpoint, Request, Response, Status}; -pub mod pb { - pub use self::app_client::AppClient; +use crate::pb::app::{GetRatingRequest, GetRatingResponse}; - tonic::include_proto!("ratings.features.app"); -} +use crate::pb::app::app_client as pb; #[derive(Debug, Clone)] pub struct AppClient { @@ -22,7 +20,7 @@ impl AppClient { &self, id: &str, token: &str, - ) -> Result, Status> { + ) -> Result, Status> { let channel = Endpoint::from_shared(self.url.clone()) .unwrap() .connect() @@ -34,7 +32,7 @@ impl AppClient { Ok(req) }); client - .get_rating(pb::GetRatingRequest { + .get_rating(GetRatingRequest { snap_id: id.to_string(), }) .await diff --git a/tests/helpers/client_chart.rs b/tests/helpers/client_chart.rs new file mode 100644 index 00000000..4e952880 --- /dev/null +++ b/tests/helpers/client_chart.rs @@ -0,0 +1,41 @@ +use tonic::metadata::MetadataValue; +use tonic::transport::Endpoint; +use tonic::{Request, Response, Status}; + +use crate::pb::chart::{chart_client as pb, Timeframe}; +use crate::pb::chart::{GetChartRequest, GetChartResponse}; + +#[derive(Debug, Clone)] +pub struct ChartClient { + url: String, +} + +impl ChartClient { + pub fn new(socket: &str) -> Self { + Self { + url: format!("http://{socket}/"), + } + } + + pub async fn get_chart( + &self, + timeframe: Timeframe, + token: &str, + ) -> Result, Status> { + let channel = Endpoint::from_shared(self.url.clone()) + .unwrap() + .connect() + .await + .unwrap(); + let mut client = pb::ChartClient::with_interceptor(channel, move |mut req: Request<()>| { + let header: MetadataValue<_> = format!("Bearer {token}").parse().unwrap(); + req.metadata_mut().insert("authorization", header); + Ok(req) + }); + client + .get_chart(GetChartRequest { + timeframe: timeframe.into(), + }) + .await + } +} diff --git a/tests/helpers/client_user.rs b/tests/helpers/client_user.rs index 25a99995..c967f2a0 100644 --- a/tests/helpers/client_user.rs +++ b/tests/helpers/client_user.rs @@ -2,12 +2,11 @@ use tonic::metadata::MetadataValue; use tonic::transport::Endpoint; use tonic::{Request, Response, Status}; -use self::pb::GetSnapVotesResponse; -pub mod pb { - pub use self::user_client::UserClient; - - tonic::include_proto!("ratings.features.user"); -} +use crate::pb::user::user_client as pb; +use crate::pb::user::{ + AuthenticateRequest, AuthenticateResponse, GetSnapVotesRequest, GetSnapVotesResponse, + VoteRequest, +}; #[derive(Debug, Clone)] pub struct UserClient { @@ -22,18 +21,15 @@ impl UserClient { } #[allow(dead_code)] - pub async fn authenticate( - &self, - id: &str, - ) -> Result, Status> { + pub async fn authenticate(&self, id: &str) -> Result, Status> { let mut client = pb::UserClient::connect(self.url.clone()).await.unwrap(); client - .authenticate(pb::AuthenticateRequest { id: id.to_string() }) + .authenticate(AuthenticateRequest { id: id.to_string() }) .await } #[allow(dead_code)] - pub async fn vote(&self, token: &str, ballet: pb::VoteRequest) -> Result, Status> { + pub async fn vote(&self, token: &str, ballet: VoteRequest) -> Result, Status> { let channel = Endpoint::from_shared(self.url.clone()) .unwrap() .connect() @@ -51,7 +47,7 @@ impl UserClient { pub async fn get_snap_votes( &self, token: &str, - request: pb::GetSnapVotesRequest, + request: GetSnapVotesRequest, ) -> Result, Status> { let channel = Endpoint::from_shared(self.url.clone()) .unwrap() diff --git a/tests/helpers/mod.rs b/tests/helpers/mod.rs index 1dd7b283..ff410932 100644 --- a/tests/helpers/mod.rs +++ b/tests/helpers/mod.rs @@ -1,5 +1,6 @@ pub mod assert; pub mod client_app; +pub mod client_chart; pub mod client_user; pub mod data_faker; pub mod hooks; diff --git a/tests/helpers/test_data.rs b/tests/helpers/test_data.rs index 77bd80ca..fd1aaa7d 100644 --- a/tests/helpers/test_data.rs +++ b/tests/helpers/test_data.rs @@ -2,12 +2,14 @@ use ratings::app::AppContext; use sqlx::{pool::PoolConnection, Postgres}; use super::client_app::AppClient; +use super::client_chart::ChartClient; use super::client_user::UserClient; #[derive(Debug, Clone)] pub struct TestData { pub user_client: Option, pub app_client: Option, + pub chart_client: Option, pub id: Option, pub snap_id: Option, pub token: Option, diff --git a/tests/helpers/vote_generator.rs b/tests/helpers/vote_generator.rs index 509843dd..0ad012c5 100644 --- a/tests/helpers/vote_generator.rs +++ b/tests/helpers/vote_generator.rs @@ -1,6 +1,6 @@ -use super::super::helpers::client_user::pb::{AuthenticateResponse, VoteRequest}; use super::test_data::TestData; use crate::helpers; +use crate::pb::user::{AuthenticateResponse, VoteRequest}; pub async fn generate_votes( snap_id: &str, diff --git a/tests/mod.rs b/tests/mod.rs index e741ab7c..14dd3f57 100644 --- a/tests/mod.rs +++ b/tests/mod.rs @@ -9,4 +9,10 @@ mod app_tests { mod lifecycle_test; } +mod chart_tests { + mod lifecycle_test; +} + mod helpers; + +mod pb; diff --git a/tests/pb.rs b/tests/pb.rs new file mode 100644 index 00000000..e86f8dd4 --- /dev/null +++ b/tests/pb.rs @@ -0,0 +1,15 @@ +pub mod app { + include!("../proto/ratings.features.app.rs"); +} + +pub mod common { + include!("../proto/ratings.features.common.rs"); +} + +pub mod user { + include!("../proto/ratings.features.user.rs"); +} + +pub mod chart { + include!("../proto/ratings.features.chart.rs"); +} diff --git a/tests/user_tests/double_authenticate_test.rs b/tests/user_tests/double_authenticate_test.rs index d413215f..e640f3c0 100644 --- a/tests/user_tests/double_authenticate_test.rs +++ b/tests/user_tests/double_authenticate_test.rs @@ -1,9 +1,9 @@ use crate::helpers; use crate::helpers::test_data::TestData; -use super::super::helpers::client_user::pb::AuthenticateResponse; use super::super::helpers::client_user::UserClient; use super::super::helpers::with_lifecycle::with_lifecycle; +use crate::pb::user::AuthenticateResponse; use ratings::app::AppContext; use ratings::utils::{self, Infrastructure}; use sqlx::Row; @@ -22,6 +22,7 @@ async fn authenticate_twice_test() -> Result<(), Box> { token: None, app_client: None, snap_id: None, + chart_client: None, }; with_lifecycle(async { diff --git a/tests/user_tests/get_votes_lifecycle_test.rs b/tests/user_tests/get_votes_lifecycle_test.rs index 421beb3d..02b328f7 100644 --- a/tests/user_tests/get_votes_lifecycle_test.rs +++ b/tests/user_tests/get_votes_lifecycle_test.rs @@ -1,8 +1,8 @@ use crate::helpers; -use crate::helpers::client_user::pb::GetSnapVotesRequest; use crate::helpers::test_data::TestData; -use super::super::helpers::client_user::pb::{AuthenticateResponse, VoteRequest}; +use crate::pb::user::{AuthenticateResponse, GetSnapVotesRequest, VoteRequest}; + use super::super::helpers::client_user::UserClient; use super::super::helpers::with_lifecycle::with_lifecycle; use futures::FutureExt; @@ -25,6 +25,7 @@ async fn get_votes_lifecycle_test() -> Result<(), Box> { token: None, app_client: None, snap_id: None, + chart_client: None, }; with_lifecycle(async { diff --git a/tests/user_tests/simple_lifecycle_test.rs b/tests/user_tests/simple_lifecycle_test.rs index dde156ce..c613d6a5 100644 --- a/tests/user_tests/simple_lifecycle_test.rs +++ b/tests/user_tests/simple_lifecycle_test.rs @@ -1,10 +1,10 @@ use crate::helpers; use crate::helpers::test_data::TestData; +use futures::FutureExt; -use super::super::helpers::client_user::pb::{AuthenticateResponse, VoteRequest}; use super::super::helpers::client_user::UserClient; use super::super::helpers::with_lifecycle::with_lifecycle; -use futures::FutureExt; +use crate::pb::user::{AuthenticateResponse, VoteRequest}; use ratings::app::AppContext; use ratings::utils::{self, Infrastructure}; use sqlx::Row; @@ -24,6 +24,7 @@ async fn user_simple_lifecycle_test() -> Result<(), Box> token: None, app_client: None, snap_id: None, + chart_client: None, }; with_lifecycle(async {