Skip to content

Commit

Permalink
fix: get Postman tests passing
Browse files Browse the repository at this point in the history
fixed bugs:

* Login takes `email`, not `username`, my mistake.
* `updatedAt` must always be set for Articles and Comments
    * We prefer to only set `updatedAt` when the row is actually updated;
      it saves space and tells us if a row has ever been updated or not.
* Article listing and feed routes must return `articleCount`
    * Spec doesn't say whether or not this should be the total number of rows returned;
      it's likely that it is for showing in a pagination scheme but the Postman collection passes as-is.
* An article's `tagList` must be sorted lexicographically.
    * Not stated by the spec but implied by the example objects and required by the Postman collection.

feat: add API logging
  • Loading branch information
abonander committed Dec 14, 2021
1 parent 45d0362 commit 7bea37f
Show file tree
Hide file tree
Showing 8 changed files with 88 additions and 32 deletions.
28 changes: 27 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ argon2 = "0.3.1"

# Axum builds on the types in Tower
tower = "0.4.11"
tower-http = { version = "0.2.0", features = ["trace"] }

jwt = "0.15.0"
hmac = "0.11.0"
Expand Down
9 changes: 7 additions & 2 deletions migrations/4_article.sql
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,11 @@ create table article

-- These fields are actually in the Realworld spec so we will be making use of them.
created_at timestamptz not null default now(),
updated_at timestamptz

-- The Realworld spec requires this to always be set,
-- but we prefer to leave it null unless the row has actually been updated.
-- It saves space as well as informs us whether a row has ever been updated or not.
updated_at timestamptz not null default now()
);

select trigger_updated_at('article');
Expand Down Expand Up @@ -88,7 +92,8 @@ create table article_comment

created_at timestamptz not null default now(),

updated_at timestamptz
-- Same thing here.
updated_at timestamptz not null default now()
);

select trigger_updated_at('article_comment');
Expand Down
6 changes: 3 additions & 3 deletions src/http/articles/comments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ struct AddComment {
struct Comment {
id: i64,
created_at: Timestamptz,
updated_at: Option<Timestamptz>,
updated_at: Timestamptz,
body: String,
author: Profile,
}
Expand All @@ -51,7 +51,7 @@ struct Comment {
struct CommentFromQuery {
comment_id: i64,
created_at: OffsetDateTime,
updated_at: Option<OffsetDateTime>,
updated_at: OffsetDateTime,
body: String,
author_username: String,
author_bio: String,
Expand All @@ -65,7 +65,7 @@ impl CommentFromQuery {
id: self.comment_id,
// doing this conversion in-code does save having to use the type overrides in query
created_at: Timestamptz(self.created_at),
updated_at: self.updated_at.map(Timestamptz),
updated_at: Timestamptz(self.updated_at),
body: self.body,
author: Profile {
username: self.author_username,
Expand Down
22 changes: 18 additions & 4 deletions src/http/articles/listing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,10 @@ pub struct FeedArticlesQuery {
}

#[derive(serde::Serialize)]
#[serde(rename_all = "camelCase")]
pub struct MultipleArticlesBody {
articles: Vec<Article>,
articles_count: usize,
}

// https://gothinkster.github.io/realworld/docs/specs/backend-specs/endpoints#list-articles
Expand All @@ -56,7 +58,7 @@ pub(in crate::http) async fn list_articles(
ctx: Extension<ApiContext>,
query: Query<ListArticlesQuery>,
) -> http::Result<Json<MultipleArticlesBody>> {
let articles = sqlx::query_as!(
let articles: Vec<_> = sqlx::query_as!(
ArticleFromQuery,
// language=PostgreSQL
r#"
Expand Down Expand Up @@ -119,7 +121,13 @@ pub(in crate::http) async fn list_articles(
.try_collect()
.await?;

Ok(Json(MultipleArticlesBody { articles }))
Ok(Json(MultipleArticlesBody {
// FIXME: this is probably supposed to be the *total* number of rows returned by the query.
//
// However, the Postman collection passes it as-is and the specification doesn't say.
articles_count: articles.len(),
articles,
}))
}

// https://gothinkster.github.io/realworld/docs/specs/backend-specs/endpoints#feed-articles
Expand All @@ -128,7 +136,7 @@ pub(in crate::http) async fn feed_articles(
ctx: Extension<ApiContext>,
query: Query<FeedArticlesQuery>,
) -> http::Result<Json<MultipleArticlesBody>> {
let articles = sqlx::query_as!(
let articles: Vec<_> = sqlx::query_as!(
ArticleFromQuery,
// As a rule of thumb, you always want the most specific dataset to be your outermost
// `SELECT` so the query planner does as little extraneous work as possible, and then
Expand Down Expand Up @@ -177,5 +185,11 @@ pub(in crate::http) async fn feed_articles(
.try_collect()
.await?;

Ok(Json(MultipleArticlesBody { articles }))
Ok(Json(MultipleArticlesBody {
// FIXME: this is probably supposed to be the *total* number of rows returned by the query.
//
// However, the Postman collection passes it as-is and the specification doesn't say.
articles_count: articles.len(),
articles,
}))
}
14 changes: 11 additions & 3 deletions src/http/articles/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,9 @@ struct Article {
body: String,
tag_list: Vec<String>,
created_at: Timestamptz,
updated_at: Option<Timestamptz>,
// Note: the Postman collection included with the spec assumes that this is never null.
// We prefer to leave it unset unless the row has actually be updated.
updated_at: Timestamptz,
favorited: bool,
favorites_count: i64,
author: Profile,
Expand All @@ -97,7 +99,7 @@ struct ArticleFromQuery {
body: String,
tag_list: Vec<String>,
created_at: Timestamptz,
updated_at: Option<Timestamptz>,
updated_at: Timestamptz,
favorited: bool,
favorites_count: i64,
author_username: String,
Expand Down Expand Up @@ -135,10 +137,16 @@ impl ArticleFromQuery {
async fn create_article(
auth_user: AuthUser,
ctx: Extension<ApiContext>,
Json(req): Json<ArticleBody<CreateArticle>>,
Json(mut req): Json<ArticleBody<CreateArticle>>,
) -> Result<Json<ArticleBody>> {
let slug = slugify(&req.article.title);

// Never specified unless you count just showing them sorted in the examples:
// https://gothinkster.github.io/realworld/docs/specs/backend-specs/api-response-format#single-article
//
// However, it is required by the Postman collection.
req.article.tag_list.sort();

// For fun, this is how we combine several operations into a single query for brevity.
let article = sqlx::query_as!(
ArticleFromQuery,
Expand Down
29 changes: 17 additions & 12 deletions src/http/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ pub use error::{Error, ResultExt};

pub type Result<T, E = Error> = std::result::Result<T, E>;

use tower_http::trace::TraceLayer;

#[derive(Clone)]
struct ApiContext {
config: Arc<Config>,
Expand All @@ -27,18 +29,21 @@ struct ApiContext {

pub async fn serve(config: Config, db: PgPool) -> anyhow::Result<()> {
let app = api_router().layer(
ServiceBuilder::new().layer(AddExtensionLayer::new(ApiContext {
// In other projects I've passed this stuff as separate objects, e.g.
// using a separate actix-web `Data` extractor for each of `Config`, `PgPool`, etc.
// It just ends up being kind of annoying that way, but does have the whole
// "pass only what you need where you need it" angle.
//
// It may not be a bad idea if you need your API to be more modular (turn routes
// on and off, and disable any unused extension objects) but it's really up to a
// judgement call.
config: Arc::new(config),
db,
})),
ServiceBuilder::new()
.layer(AddExtensionLayer::new(ApiContext {
// In other projects I've passed this stuff as separate objects, e.g.
// using a separate actix-web `Data` extractor for each of `Config`, `PgPool`, etc.
// It just ends up being kind of annoying that way, but does have the whole
// "pass only what you need where you need it" angle.
//
// It may not be a bad idea if you need your API to be more modular (turn routes
// on and off, and disable any unused extension objects) but it's really up to a
// judgement call.
config: Arc::new(config),
db,
}))
// Enables logging. Use `RUST_LOG=tower_http=debug`
.layer(TraceLayer::new_for_http()),
);

// We use 8080 as our default HTTP server port, it's pretty easy to remember.
Expand Down
11 changes: 4 additions & 7 deletions src/http/users.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ struct NewUser {

#[derive(serde::Deserialize)]
struct LoginUser {
username: String,
email: String,
password: String,
}

Expand Down Expand Up @@ -103,16 +103,13 @@ async fn login_user(
let user = sqlx::query!(
r#"
select user_id, email, username, bio, image, password_hash
from "user" where username = $1
from "user" where email = $1
"#,
req.user.username,
req.user.email,
)
.fetch_optional(&ctx.db)
.await?
.ok_or(Error::unprocessable_entity([(
"username",
"does not exist",
)]))?;
.ok_or(Error::unprocessable_entity([("email", "does not exist")]))?;

verify_password(req.user.password, user.password_hash).await?;

Expand Down

0 comments on commit 7bea37f

Please sign in to comment.