Skip to content

Commit 8b5bb4d

Browse files
committed
chg: Some optimizations by using &str and Rc instead of String and Vec
1 parent 6e0f4d6 commit 8b5bb4d

File tree

7 files changed

+58
-68
lines changed

7 files changed

+58
-68
lines changed

actix/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ passwords = "3.1.16"
3636
actix-session = { version = "0.11.0", features = [ "cookie-session" ] }
3737
nanoid = "0.4.0"
3838
serde_json = "1.0.115"
39-
serde = { version = "1.0.197", features = [ "derive" ] }
39+
serde = { version = "1.0.197", features = [ "derive", "rc" ] }
4040
argon2 = "0.5.3"
4141
chrono = "0.4.41"
4242
tokio = "1.44.2"

actix/src/auth.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,13 @@ use actix_web::HttpRequest;
66
use argon2::{password_hash::PasswordHash, Argon2, PasswordVerifier};
77
use log::{debug, warn};
88
use passwords::PasswordGenerator;
9+
use std::rc::Rc;
910
use std::time::SystemTime;
1011

1112
use crate::config::Config;
1213

1314
// Validate API key
14-
pub fn validate_key(key: String, config: &Config) -> bool {
15+
pub fn validate_key(key: &str, config: &Config) -> bool {
1516
if let Some(api_key) = &config.api_key {
1617
// Check if API Key is hashed using Argon2. More algorithms maybe added later.
1718
let authorized = if config.hash_algorithm.is_some() {
@@ -22,7 +23,7 @@ pub fn validate_key(key: String, config: &Config) -> bool {
2223
.is_ok()
2324
} else {
2425
// If hashing is not enabled, use the plaintext API key for matching
25-
api_key == &key
26+
api_key == key
2627
};
2728
if !authorized {
2829
warn!("Incorrect API key was provided when connecting to Chhoto URL.");
@@ -66,16 +67,16 @@ pub fn validate(session: Session, config: &Config) -> bool {
6667
}
6768

6869
if let Ok(token) = session.get::<String>("chhoto-url-auth") {
69-
check(token)
70+
check(token.as_deref())
7071
} else {
7172
false
7273
}
7374
}
7475

7576
// Check a token cryptographically
76-
fn check(token: Option<String>) -> bool {
77+
fn check(token: Option<&str>) -> bool {
7778
if let Some(token_body) = token {
78-
let token_parts: Vec<&str> = token_body.split(';').collect();
79+
let token_parts: Rc<[&str]> = token_body.split(';').collect();
7980
if token_parts.len() < 2 {
8081
false
8182
} else {

actix/src/database.rs

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,10 @@
22
// SPDX-License-Identifier: MIT
33

44
use log::info;
5+
use rusqlite::fallible_iterator::FallibleIterator;
56
use rusqlite::{Connection, Error};
67
use serde::Serialize;
8+
use std::rc::Rc;
79

810
// Struct for encoding a DB row
911
#[derive(Serialize)]
@@ -15,7 +17,7 @@ pub struct DBRow {
1517
}
1618

1719
// Find a single URL for /api/expand
18-
pub fn find_url(shortlink: String, db: &Connection) -> (Option<String>, Option<i64>, Option<i64>) {
20+
pub fn find_url(shortlink: &str, db: &Connection) -> (Option<String>, Option<i64>, Option<i64>) {
1921
// Long link, hits, expiry time
2022
let now = chrono::Utc::now().timestamp();
2123
let query = "SELECT long_url, hits, expiry_time FROM urls
@@ -37,10 +39,10 @@ pub fn find_url(shortlink: String, db: &Connection) -> (Option<String>, Option<i
3739
// Get all URLs in DB
3840
pub fn getall(
3941
db: &Connection,
40-
page_after: Option<String>,
42+
page_after: Option<&str>,
4143
page_no: Option<i64>,
4244
page_size: Option<i64>,
43-
) -> Vec<DBRow> {
45+
) -> Rc<[DBRow]> {
4446
let now = chrono::Utc::now().timestamp();
4547
let query = if page_after.is_some() {
4648
"SELECT short_url, long_url, hits, expiry_time FROM (
@@ -70,7 +72,7 @@ pub fn getall(
7072
.prepare_cached(query)
7173
.expect("Error preparing SQL statement for getall.");
7274

73-
let mut data = if let Some(pos) = page_after {
75+
let data = if let Some(pos) = page_after {
7476
let size = page_size.unwrap_or(10);
7577
statement
7678
.query((pos, now, size))
@@ -90,26 +92,24 @@ pub fn getall(
9092
.expect("Error executing query for getall: no pagination.")
9193
};
9294

93-
let mut links: Vec<DBRow> = Vec::new();
94-
while let Some(row) = data.next().expect("Error reading fetched rows.") {
95-
let row_struct = DBRow {
96-
shortlink: row
97-
.get("short_url")
98-
.expect("Error reading shortlink from row."),
99-
longlink: row
100-
.get("long_url")
101-
.expect("Error reading shortlink from row."),
102-
hits: row.get("hits").expect("Error reading shortlink from row."),
103-
expiry_time: row.get("expiry_time").unwrap_or_default(),
104-
};
105-
links.push(row_struct);
106-
}
95+
let links: Rc<[DBRow]> = data
96+
.map(|row| {
97+
let row_struct = DBRow {
98+
shortlink: row.get("short_url")?,
99+
longlink: row.get("long_url")?,
100+
hits: row.get("hits")?,
101+
expiry_time: row.get("expiry_time").unwrap_or_default(),
102+
};
103+
Ok(row_struct)
104+
})
105+
.collect()
106+
.expect("Error procecssing fetched row.");
107107

108108
links
109109
}
110110

111111
// Add a hit when site is visited during link resolution
112-
pub fn find_and_add_hit(shortlink: String, db: &Connection) -> Option<String> {
112+
pub fn find_and_add_hit(shortlink: &str, db: &Connection) -> Option<String> {
113113
let now = chrono::Utc::now().timestamp();
114114
let mut statement = db
115115
.prepare_cached(
@@ -208,7 +208,7 @@ pub fn cleanup(db: &Connection) {
208208
}
209209

210210
// Delete an existing link
211-
pub fn delete_link(shortlink: String, db: &Connection) -> bool {
211+
pub fn delete_link(shortlink: &str, db: &Connection) -> bool {
212212
let mut statement = db
213213
.prepare_cached("DELETE FROM urls WHERE short_url = ?1")
214214
.expect("Error preparing SQL statement for delete_link.");
@@ -219,7 +219,7 @@ pub fn delete_link(shortlink: String, db: &Connection) -> bool {
219219
}
220220
}
221221

222-
pub fn open_db(path: String, use_wal_mode: bool) -> Connection {
222+
pub fn open_db(path: &str, use_wal_mode: bool) -> Connection {
223223
// Set current user_version. Should be incremented on change of schema.
224224
let user_version = 1;
225225

actix/src/main.rs

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -79,26 +79,22 @@ async fn main() -> Result<()> {
7979
);
8080

8181
// Do periodic cleanup
82-
let db_location_clone = conf.db_location.clone();
82+
let db_location = conf.db_location.clone();
8383
// Create backups if WAL mode is being used
8484
if conf.use_wal_mode {
8585
info!("Creating database backups.");
86-
if fs::exists(format!("{db_location_clone}.bak1")).ok() == Some(true) {
87-
fs::rename(
88-
format!("{db_location_clone}.bak1"),
89-
format!("{db_location_clone}.bak2"),
90-
)
91-
.expect("Error creating backups.");
92-
}
93-
if fs::exists(&db_location_clone).ok() == Some(true) {
94-
fs::copy(&db_location_clone, format!("{db_location_clone}.bak1"))
86+
if fs::exists(format!("{db_location}.bak1")).ok() == Some(true) {
87+
fs::rename(format!("{db_location}.bak1"), format!("{db_location}.bak2"))
9588
.expect("Error creating backups.");
9689
}
90+
if fs::exists(&db_location).ok() == Some(true) {
91+
fs::copy(&db_location, format!("{db_location}.bak1")).expect("Error creating backups.");
92+
}
9793
}
9894

9995
info!("Starting cleanup service, will run once every hour.");
10096
spawn(async move {
101-
let db = database::open_db(db_location_clone, conf.use_wal_mode);
97+
let db = database::open_db(&db_location, conf.use_wal_mode);
10298
let mut interval = time::interval(time::Duration::from_secs(3600));
10399
loop {
104100
interval.tick().await;
@@ -123,7 +119,7 @@ async fn main() -> Result<()> {
123119
)
124120
// Maintain a single instance of database throughout
125121
.app_data(web::Data::new(AppState {
126-
db: database::open_db(conf_clone.db_location.clone(), conf.use_wal_mode),
122+
db: database::open_db(&conf.db_location, conf.use_wal_mode),
127123
config: conf_clone.clone(),
128124
}))
129125
.wrap(if let Some(header) = &conf.cache_control_header {

actix/src/services.rs

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ pub async fn add_link(
8585
let result = utils::is_api_ok(http, config);
8686
// If success, add new link
8787
if result.success {
88-
let (success, reply, expiry_time) = utils::add_link(req, &data.db, config, false);
88+
let (success, reply, expiry_time) = utils::add_link(&req, &data.db, config, false);
8989
if success {
9090
let site_url = config.site_url.clone();
9191
let shorturl = if let Some(url) = site_url {
@@ -119,9 +119,9 @@ pub async fn add_link(
119119
// If password authentication or public mode is used - keeps backwards compatibility
120120
} else {
121121
let (success, reply, _) = if auth::validate(session, config) {
122-
utils::add_link(req, &data.db, config, false)
122+
utils::add_link(&req, &data.db, config, false)
123123
} else if config.public_mode {
124-
utils::add_link(req, &data.db, config, true)
124+
utils::add_link(&req, &data.db, config, true)
125125
} else {
126126
return HttpResponse::Unauthorized().body("Not logged in!");
127127
};
@@ -162,7 +162,7 @@ pub async fn getall(
162162
pub async fn expand(req: String, data: web::Data<AppState>, http: HttpRequest) -> HttpResponse {
163163
let result = utils::is_api_ok(http, &data.config);
164164
if result.success {
165-
let (longurl, hits, expiry_time) = database::find_url(req, &data.db);
165+
let (longurl, hits, expiry_time) = database::find_url(&req, &data.db);
166166
if let Some(longlink) = longurl {
167167
let body = LinkInfo {
168168
success: true,
@@ -197,7 +197,7 @@ pub async fn edit_link(
197197
let config = &data.config;
198198
let result = utils::is_api_ok(http, config);
199199
if result.success || validate(session, config) {
200-
if let Some((server_error, error_msg)) = utils::edit_link(req, &data.db, config) {
200+
if let Some((server_error, error_msg)) = utils::edit_link(&req, &data.db, config) {
201201
let body = Response {
202202
success: false,
203203
error: true,
@@ -300,7 +300,7 @@ pub async fn link_handler(
300300
shortlink: web::Path<String>,
301301
data: web::Data<AppState>,
302302
) -> impl Responder {
303-
let shortlink_str = shortlink.to_string();
303+
let shortlink_str = shortlink.as_str();
304304
if let Some(longlink) = database::find_and_add_hit(shortlink_str, &data.db) {
305305
if data.config.use_temp_redirect {
306306
Either::Left(Redirect::to(longlink))
@@ -406,11 +406,7 @@ pub async fn delete_link(
406406
let result = utils::is_api_ok(http, config);
407407
// If success, delete shortlink
408408
if result.success {
409-
if utils::delete_link(
410-
shortlink.to_string(),
411-
&data.db,
412-
data.config.allow_capital_letters,
413-
) {
409+
if utils::delete_link(&shortlink, &data.db, data.config.allow_capital_letters) {
414410
let response = Response {
415411
success: true,
416412
error: false,
@@ -429,11 +425,7 @@ pub async fn delete_link(
429425
HttpResponse::Unauthorized().json(result)
430426
// If "pass" is true - keeps backwards compatibility
431427
} else if auth::validate(session, config) {
432-
if utils::delete_link(
433-
shortlink.to_string(),
434-
&data.db,
435-
data.config.allow_capital_letters,
436-
) {
428+
if utils::delete_link(&shortlink, &data.db, data.config.allow_capital_letters) {
437429
HttpResponse::Ok().body(format!("Deleted {shortlink}"))
438430
} else {
439431
HttpResponse::NotFound().body("Not found!")

actix/src/tests.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use actix_web::test;
44
use actix_web::{body::to_bytes, dev::ServiceResponse, web::Bytes, App, Error};
55
use regex::Regex;
66
use serde::Deserialize;
7+
use std::rc::Rc;
78
use std::{fmt::Display, fs, thread::sleep, time::Duration};
89

910
use super::*;
@@ -77,7 +78,7 @@ async fn create_app(
7778
App::new()
7879
.app_data(web::Data::new(AppState {
7980
db: database::open_db(
80-
format!("/tmp/chhoto-url-test-{test}.sqlite"),
81+
format!("/tmp/chhoto-url-test-{test}.sqlite").as_str(),
8182
conf.use_wal_mode,
8283
),
8384
config: conf.clone(),
@@ -295,7 +296,7 @@ async fn data_fetching_all() {
295296
let resp = test::call_service(&app, req).await;
296297
assert!(resp.status().is_success());
297298
let body = to_bytes(resp.into_body()).await.unwrap();
298-
let reply_chunks: Vec<URLData> = serde_json::from_str(body.as_str()).unwrap();
299+
let reply_chunks: Rc<[URLData]> = serde_json::from_str(body.as_str()).unwrap();
299300
assert_eq!(reply_chunks.len(), 2);
300301
assert_eq!(reply_chunks[0].shortlink, "test1");
301302
assert_eq!(reply_chunks[1].shortlink, "test3");
@@ -314,7 +315,7 @@ async fn data_fetching_all() {
314315
let resp = test::call_service(&app, req).await;
315316
assert!(resp.status().is_success());
316317
let body = to_bytes(resp.into_body()).await.unwrap();
317-
let reply_chunks: Vec<URLData> = serde_json::from_str(body.as_str()).unwrap();
318+
let reply_chunks: Rc<[URLData]> = serde_json::from_str(body.as_str()).unwrap();
318319
assert_eq!(reply_chunks.len(), 1);
319320
assert_eq!(reply_chunks[0].shortlink, "test1");
320321

@@ -326,7 +327,7 @@ async fn data_fetching_all() {
326327
let resp = test::call_service(&app, req).await;
327328
assert!(resp.status().is_success());
328329
let body = to_bytes(resp.into_body()).await.unwrap();
329-
let reply_chunks: Vec<URLData> = serde_json::from_str(body.as_str()).unwrap();
330+
let reply_chunks: Rc<[URLData]> = serde_json::from_str(body.as_str()).unwrap();
330331
assert_eq!(reply_chunks.len(), 1);
331332
assert_eq!(reply_chunks[0].shortlink, "test1");
332333

actix/src/utils.rs

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ pub fn is_api_ok(http: HttpRequest, config: &Config) -> Response {
4444
// If the header exists
4545
if let Some(header) = auth::api_header(&http) {
4646
// If the header is correct
47-
if auth::validate_key(header.to_string(), config) {
47+
if auth::validate_key(header, config) {
4848
Response {
4949
success: true,
5050
error: false,
@@ -105,20 +105,20 @@ pub fn getall(db: &Connection, params: GetReqParams) -> String {
105105
let page_after = params.page_after.filter(|s| !s.is_empty());
106106
let page_no = params.page_no.filter(|&n| n > 0);
107107
let page_size = params.page_size.filter(|&n| n > 0);
108-
let links = database::getall(db, page_after, page_no, page_size);
108+
let links = database::getall(db, page_after.as_deref(), page_no, page_size);
109109
serde_json::to_string(&links).expect("Failure during creation of json from db.")
110110
}
111111

112112
// Make checks and then request the DB to add a new URL entry
113113
pub fn add_link(
114-
req: String,
114+
req: &str,
115115
db: &Connection,
116116
config: &Config,
117117
using_public_mode: bool,
118118
) -> (bool, String, i64) {
119119
// Success status, response string, expiry time
120120
let mut chunks: NewURLRequest;
121-
if let Ok(json) = serde_json::from_str(&req) {
121+
if let Ok(json) = serde_json::from_str(req) {
122122
chunks = json;
123123
} else {
124124
return (false, String::from("Invalid request!"), 0);
@@ -173,13 +173,13 @@ pub fn add_link(
173173
}
174174

175175
// Make checks and then request the DB to edit an URL entry
176-
pub fn edit_link(req: String, db: &Connection, config: &Config) -> Option<(bool, String)> {
176+
pub fn edit_link(req: &str, db: &Connection, config: &Config) -> Option<(bool, String)> {
177177
// None means success
178178
// The boolean is true when it's a server error and false when it's a client error
179179
// The string is the error message
180180

181181
let chunks: EditURLRequest;
182-
if let Ok(json) = serde_json::from_str(&req) {
182+
if let Ok(json) = serde_json::from_str(req) {
183183
chunks = json;
184184
} else {
185185
return Some((false, String::from("Malformed request!")));
@@ -201,16 +201,16 @@ pub fn edit_link(req: String, db: &Connection, config: &Config) -> Option<(bool,
201201
}
202202
}
203203
// Check if link, and request DB to delete it if exists
204-
pub fn delete_link(shortlink: String, db: &Connection, allow_capital_letters: bool) -> bool {
205-
if validate_link(shortlink.as_str(), allow_capital_letters) {
204+
pub fn delete_link(shortlink: &str, db: &Connection, allow_capital_letters: bool) -> bool {
205+
if validate_link(shortlink, allow_capital_letters) {
206206
database::delete_link(shortlink, db)
207207
} else {
208208
false
209209
}
210210
}
211211

212212
// Generate a random link using either adjective-name pair (default) of a slug or a-z, 0-9
213-
fn gen_link(style: &String, len: usize, allow_capital_letters: bool) -> String {
213+
fn gen_link(style: &str, len: usize, allow_capital_letters: bool) -> String {
214214
#[rustfmt::skip]
215215
static ADJECTIVES: [&str; 108] = ["admiring", "adoring", "affectionate", "agitated", "amazing", "angry", "awesome", "beautiful",
216216
"blissful", "bold", "boring", "brave", "busy", "charming", "clever", "compassionate", "competent", "condescending", "confident", "cool",

0 commit comments

Comments
 (0)