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.

Sign up for GitHub

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

Jump to bottom

bind params on queries #189

Merged
merged 2 commits into from
Mar 30, 2023
Merged

bind params on queries #189

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
20809bd
configure logger'
Mar 30, 2023
13c14cc
use bind params on queries
Mar 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions trunk/registry/Cargo.lock
View file
Open in desktop

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

2 changes: 1 addition & 1 deletion trunk/registry/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ publish = false
[dependencies]
actix-web = "4.3.1"
anyhow = "1.0.69"
sqlx = { version = "0.6.2", features = [ "runtime-tokio-native-tls" , "postgres" ] }
sqlx = { version = "0.6.2", features = [ "json", "runtime-tokio-native-tls" , "postgres", "time" ] }
url = "2.3.1"
thiserror = "1.0.39"
log = "0.4.17"
Expand Down
1 change: 1 addition & 0 deletions trunk/registry/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use trunk_registry::{config, download, publish, routes};

#[actix_web::main]
async fn main() -> std::io::Result<()> {
env_logger::init();
// load configurations from environment
let cfg = config::Config::default();
HttpServer::new(move || {
Expand Down
108 changes: 57 additions & 51 deletions trunk/registry/src/publish.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,99 +46,105 @@ pub async fn publish(
check_input(&new_extension.name)?;

// Check if extension exists
let query = format!(
"SELECT * FROM extensions WHERE name = '{}' IS TRUE",
let exists = sqlx::query!(
"SELECT * FROM extensions WHERE name = $1",
new_extension.name
);

let exists = sqlx::query(&query).fetch_optional(&mut tx).await?;
)
.fetch_optional(&mut tx)
.await?;

match exists {
// TODO(ianstanton) Refactor into separate functions
Some(exists) => {
// Extension exists
let mut tx = conn.begin().await?;
let time = chrono::offset::Utc::now().naive_utc();
let extension_id: i64 = exists.get(0);
let extension_id = exists.id;

// Check if version exists
let query = format!(
"SELECT * FROM versions WHERE extension_id = {} and num = '{}' IS TRUE",
extension_id, new_extension.vers
);

let version_exists = sqlx::query(&query).fetch_optional(&mut tx).await?;
let version_exists = sqlx::query!(
"SELECT *
FROM versions
WHERE
extension_id = $1
and num = $2",
extension_id as i32,
new_extension.vers.to_string()
)
.fetch_optional(&mut tx)
.await?;

match version_exists {
Some(_version_exists) => {
// Update updated_at timestamp
let query = format!(
sqlx::query!(
"UPDATE versions
SET updated_at = '{}'
WHERE extension_id = {}
AND num = '{}'",
time, extension_id, new_extension.vers
);
sqlx::query(&query).execute(&mut tx).await?;
SET updated_at = (now() at time zone 'utc')
Copy link
Member

Choose a reason for hiding this comment

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

If we use (now() at time zone 'utc') here (updating version record) and below when we update the extension's updated_at timestamp, will we have slightly different values?

Copy link
Member

Choose a reason for hiding this comment

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

For example, if we assign time above we ensure the timestamps are the same when an extension version is updated. Similar for created_at timestamps:
image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

now() will stay the same within the transaction.

https://database.guide/how-now-works-in-postgresql/#:~:text=In%20PostgreSQL%2C%20the%20now(),when%20called%20without%20an%20argument).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

postgres=# begin;
BEGIN
postgres=*# select now();
              now              
-------------------------------
 2023-03-30 19:32:06.570287+00
(1 row)

postgres=*# select now();
              now              
-------------------------------
 2023-03-30 19:32:06.570287+00
(1 row)

postgres=*# commit;
COMMIT
postgres=# select now();
              now              
-------------------------------
 2023-03-30 19:32:23.667648+00

Copy link
Member

Choose a reason for hiding this comment

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

Makes sense! That's exactly what we want.

WHERE extension_id = $1
AND num = $2",
extension_id as i32,
new_extension.vers.to_string()
)
.execute(&mut tx)
.await?;
}
None => {
// Create new record in versions table
let query = format!(
sqlx::query!(
"
INSERT INTO versions(extension_id, num, created_at, yanked, license)
VALUES ('{}', '{}', '{}', '{}', '{}')
VALUES ($1, $2, (now() at time zone 'utc'), $3, $4)
",
extension_id,
new_extension.vers,
time,
"f",
extension_id as i32,
new_extension.vers.to_string(),
false,
new_extension.license.unwrap()
);
sqlx::query(&query).execute(&mut tx).await?;
)
.execute(&mut tx)
.await?;
}
}

// Set updated_at time on extension
let query = format!(
sqlx::query!(
"UPDATE extensions
SET updated_at = '{}'
WHERE name = '{}'",
time, new_extension.name,
);
sqlx::query(&query).execute(&mut tx).await?;
SET updated_at = (now() at time zone 'utc')
WHERE name = $1",
new_extension.name,
)
.execute(&mut tx)
.await?;
tx.commit().await?;
}
None => {
// Else, create new record in extensions table
let mut tx = conn.begin().await?;
let time = chrono::offset::Utc::now().naive_utc();
let query = format!(
let id_row = sqlx::query!(
"
INSERT INTO extensions(name, created_at, description, homepage)
VALUES ('{}', '{}', '{}', '{}')
VALUES ($1, (now() at time zone 'utc'), $2, $3)
RETURNING id
",
new_extension.name,
time,
new_extension.description.unwrap(),
new_extension.homepage.unwrap()
);
let id_row = sqlx::query(&query).fetch_one(&mut tx).await?;
let extension_id: i64 = id_row.get(0);
)
.fetch_one(&mut tx)
.await?;
let extension_id = id_row.id;

// Create new record in versions table
let query = format!(
sqlx::query!(
"
INSERT INTO versions(extension_id, num, created_at, yanked, license)
VALUES ('{}', '{}', '{}', '{}', '{}')
",
extension_id,
new_extension.vers,
time,
"f",
INSERT INTO versions(extension_id, num, created_at, yanked, license)
VALUES ($1, $2, (now() at time zone 'utc'), $3, $4)
",
extension_id as i32,
new_extension.vers.to_string(),
false,
new_extension.license.unwrap()
);
sqlx::query(&query).execute(&mut tx).await?;
)
.execute(&mut tx)
.await?;
tx.commit().await?;
}
}
Expand Down
8 changes: 3 additions & 5 deletions trunk/registry/src/routes.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ use crate::config::Config;
use crate::connect;
use crate::errors::ExtensionRegistryError;
use actix_web::{get, web, HttpResponse, Responder};
use sqlx::Row;

use log::info;
#[get("/")]
pub async fn running() -> impl Responder {
HttpResponse::Ok().body("API is up and running!")
Expand All @@ -19,10 +18,9 @@ pub async fn get_all_extensions(
// Create a transaction on the database, if there are no errors,
// commit the transactions to record a new or updated extension.
let mut tx = conn.begin().await?;
let query = format!("SELECT * FROM extensions",);
let rows = sqlx::query(&query).fetch_all(&mut tx).await?;
let rows = sqlx::query!("SELECT * FROM extensions").fetch_all(&mut tx).await?;
for row in rows.iter() {
extensions.push(row.get(1));
extensions.push(row.name.as_ref().unwrap().to_owned());
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think this is going to change very soon right? This should probably return json formatted instead of just the name as a string.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, in the cor-526 branch

}
// Return results in response
Ok(HttpResponse::Ok().body(format!("{:?}", extensions)))
Expand Down