Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: getChart #41

Merged
merged 10 commits into from
Jan 8, 2024
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
name: Build
name: Build and test

on:
workflow_call:
Expand Down Expand Up @@ -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
2 changes: 1 addition & 1 deletion .github/workflows/pull-request.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion .github/workflows/push.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,4 @@ concurrency:

jobs:
build:
uses: ./.github/workflows/build.yaml
uses: ./.github/workflows/build-and-test.yaml
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,12 @@
*.rock
*.nix

/proto/*.rs
venv/
build/
*.charm
.tox/
.coverage
__pycache__/
*.py[cod]
server.pid
8 changes: 8 additions & 0 deletions build.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
use std::path::Path;

fn main() -> Result<(), Box<dyn std::error::Error>> {
// 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()
Expand All @@ -8,11 +14,13 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
"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(())
Expand Down
2 changes: 1 addition & 1 deletion example.env
Original file line number Diff line number Diff line change
@@ -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
Expand Down
19 changes: 3 additions & 16 deletions proto/ratings_features_app.proto
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ syntax = "proto3";

package ratings.features.app;

import "ratings_features_common.proto";

service App {
rpc GetRating (GetRatingRequest) returns (GetRatingResponse) {}
}
Expand All @@ -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;
}
16 changes: 5 additions & 11 deletions proto/ratings_features_chart.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
18 changes: 18 additions & 0 deletions proto/ratings_features_common.proto
Original file line number Diff line number Diff line change
@@ -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;
}
8 changes: 6 additions & 2 deletions src/app/interfaces/routes.rs
Original file line number Diff line number Diff line change
@@ -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<impl ServerReflection> {
Expand All @@ -16,6 +16,10 @@ pub fn build_reflection_service(
pub fn build_servers<R>(router: Router<R>) -> Router<R> {
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)
}
40 changes: 17 additions & 23 deletions src/features/app/infrastructure.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
use crate::app::AppContext;
use sqlx::Row;
use crate::{app::AppContext, features::common::entities::VoteSummary};
use tracing::error;

use super::{entities::Vote, errors::AppError};
use super::errors::AppError;

pub(crate) async fn get_votes_by_snap_id(
app_ctx: &AppContext,
snap_id: &str,
) -> Result<Vec<Vote>, AppError> {
) -> Result<VoteSummary, AppError> {
let mut pool = app_ctx
.infrastructure()
.repository()
Expand All @@ -16,32 +15,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<Vote> = result
.into_iter()
.map(|row| Vote {
vote_up: row.get("vote_up"),
})
.collect();

Ok(votes)
Ok(result)
}
11 changes: 3 additions & 8 deletions src/features/app/interface.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,11 @@
use crate::app::AppContext;

use self::protobuf::{App, GetRatingRequest, GetRatingResponse};
pub use protobuf::app_server;
use crate::features::pb::app::{GetRatingRequest, GetRatingResponse};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also applies to Line 1 and later lines, but probably best to merge all the use crate::{...} into a single line so it's easier to find all imports.

use tonic::{Request, Response, Status};

use super::{service::AppService, use_cases};

pub mod protobuf {
pub use self::app_server::App;
use crate::features::pb::app::app_server::App;

tonic::include_proto!("ratings.features.app");
}
use super::{service::AppService, use_cases};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason to not use crate:: here? I rarely see Rust crates use super except in unit tests, but I won't be too picky.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No reason, totally agree with use crate:: 🙂


#[tonic::async_trait]
impl App for AppService {
Expand Down
1 change: 0 additions & 1 deletion src/features/app/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
pub mod entities;
mod errors;
mod infrastructure;
pub mod interface;
Expand Down
2 changes: 1 addition & 1 deletion src/features/app/service.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use super::interface::app_server::AppServer;
use crate::features::pb::app::app_server::AppServer;

#[derive(Debug, Default)]
pub struct AppService;
Expand Down
6 changes: 3 additions & 3 deletions src/features/app/use_cases.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::app::AppContext;
use crate::{app::AppContext, features::common::entities::Rating};
use tracing::error;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe merge with the super block below (as per previous comment)


use super::{entities::Rating, errors::AppError, infrastructure::get_votes_by_snap_id};
use super::{errors::AppError, infrastructure::get_votes_by_snap_id};

pub async fn get_rating(app_ctx: &AppContext, snap_id: String) -> Result<Rating, AppError> {
let votes = get_votes_by_snap_id(app_ctx, &snap_id)
Expand All @@ -11,7 +11,7 @@ pub async fn get_rating(app_ctx: &AppContext, snap_id: String) -> Result<Rating,
AppError::Unknown
})?;

let rating = Rating::new(snap_id, votes);
let rating = Rating::new(votes);

Ok(rating)
}
56 changes: 56 additions & 0 deletions src/features/chart/entities.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
use sqlx::FromRow;

use crate::features::common::entities::{calculate_band, Rating, VoteSummary};
use crate::features::pb::chart as pb;

pub struct Chart {
pub timeframe: pb::Timeframe,
pub chart_data: Vec<ChartData>,
}

impl Chart {
pub fn new(timeframe: pb::Timeframe, data: Vec<VoteSummary>) -> Self {
let mut chart_data: Vec<ChartData> =
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<ChartData> = 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()),
}
}
}
11 changes: 11 additions & 0 deletions src/features/chart/errors.rs
Original file line number Diff line number Diff line change
@@ -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,
}
Loading
Loading