Skip to content

Commit 781a3b6

Browse files
authored
refactor: Improve conditional parallel tasks (#485)
Refactor the code to better support conditional parallel tasks (role assignments and listing users). This allows making the code more flexible with fewer if-else conditions and code duplication.
1 parent ba499de commit 781a3b6

File tree

9 files changed

+426
-231
lines changed

9 files changed

+426
-231
lines changed

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ derive_builder = { version = "0.20" }
4646
dyn-clone = { version = "1.0" }
4747
eyre = { version = "0.6" }
4848
fernet = { version = "0.2", default-features = false, features = ["rustcrypto"] }
49+
# futures = { version = "0.3" }
4950
futures-util = { version = "0.3" }
5051
itertools = { version = "0.14" }
5152
mockall_double = { version = "0.3" }

src/assignment/backend/sql/assignment/list.rs

Lines changed: 219 additions & 175 deletions
Large diffs are not rendered by default.

src/identity/backends/sql/authenticate.rs

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ mod tests {
182182
let db = MockDatabase::new(DatabaseBackend::Postgres).into_connection();
183183
let config = Config::default();
184184
assert!(
185-
!should_lock(&config, &db, &get_local_user_mock())
185+
!should_lock(&config, &db, &get_local_user_mock("user_id"))
186186
.await
187187
.unwrap(),
188188
"Default config does not request any validation and user is not considered locked"
@@ -233,7 +233,7 @@ mod tests {
233233
#[tokio::test]
234234
async fn test_should_lock_no_failed_auth_at() {
235235
let db = MockDatabase::new(DatabaseBackend::Postgres)
236-
.append_query_results([vec![get_local_user_mock()]])
236+
.append_query_results([vec![get_local_user_mock("user_id")]])
237237
.into_connection();
238238
let mut config = Config::default();
239239
config.security_compliance.lockout_failure_attempts = Some(5);
@@ -273,7 +273,7 @@ mod tests {
273273
#[tokio::test]
274274
async fn test_should_lock_expired() {
275275
let db = MockDatabase::new(DatabaseBackend::Postgres)
276-
.append_query_results([vec![get_local_user_mock()]])
276+
.append_query_results([vec![get_local_user_mock("user_id")]])
277277
.into_connection();
278278
let mut config = Config::default();
279279
config.security_compliance.lockout_failure_attempts = Some(5);
@@ -378,7 +378,7 @@ mod tests {
378378
password_hash: String,
379379
) -> (db_local_user::Model, db_password::Model) {
380380
(
381-
get_local_user_mock(),
381+
get_local_user_mock("user_id"),
382382
db_password::ModelBuilder::default()
383383
.password_hash(password_hash)
384384
.build()
@@ -397,6 +397,7 @@ mod tests {
397397
.unwrap(),
398398
)]])
399399
.append_query_results([user_option::tests::get_user_options_mock(
400+
"user_id",
400401
&UserOptions::default(),
401402
)])
402403
.append_query_results([vec![user::tests::get_user_mock("user_id")]])
@@ -465,6 +466,7 @@ mod tests {
465466
.unwrap(),
466467
)]])
467468
.append_query_results([user_option::tests::get_user_options_mock(
469+
"user_id",
468470
&UserOptions::default(),
469471
)])
470472
.into_connection();
@@ -515,10 +517,13 @@ mod tests {
515517
.build()
516518
.unwrap(),
517519
)]])
518-
.append_query_results([user_option::tests::get_user_options_mock(&UserOptions {
519-
ignore_lockout_failure_attempts: Some(true),
520-
..Default::default()
521-
})])
520+
.append_query_results([user_option::tests::get_user_options_mock(
521+
"user_id",
522+
&UserOptions {
523+
ignore_lockout_failure_attempts: Some(true),
524+
..Default::default()
525+
},
526+
)])
522527
.append_query_results([vec![user::tests::get_user_mock("user_id")]])
523528
.append_query_results([vec![user::tests::get_user_mock("user_id")]])
524529
.into_connection();
@@ -544,13 +549,14 @@ mod tests {
544549
let config = Config::default();
545550
let db = MockDatabase::new(DatabaseBackend::Postgres)
546551
.append_query_results([vec![(
547-
get_local_user_mock(),
552+
get_local_user_mock("user_id"),
548553
db_password::ModelBuilder::default()
549554
.password_hash("wrong_password")
550555
.build()
551556
.unwrap(),
552557
)]])
553558
.append_query_results([user_option::tests::get_user_options_mock(
559+
"user_id",
554560
&UserOptions::default(),
555561
)])
556562
.into_connection();
@@ -582,7 +588,7 @@ mod tests {
582588
let password = String::from("foo_pass");
583589
let db = MockDatabase::new(DatabaseBackend::Postgres)
584590
.append_query_results([vec![(
585-
get_local_user_mock(),
591+
get_local_user_mock("user_id"),
586592
db_password::ModelBuilder::default()
587593
.password_hash(
588594
password_hashing::hash_password(&config, &password)
@@ -594,6 +600,7 @@ mod tests {
594600
.unwrap(),
595601
)]])
596602
.append_query_results([user_option::tests::get_user_options_mock(
603+
"user_id",
597604
&UserOptions::default(),
598605
)])
599606
.into_connection();
@@ -626,7 +633,7 @@ mod tests {
626633
let password = String::from("foo_pass");
627634
let db = MockDatabase::new(DatabaseBackend::Postgres)
628635
.append_query_results([vec![(
629-
get_local_user_mock(),
636+
get_local_user_mock("user_id"),
630637
db_password::ModelBuilder::expired()
631638
.password_hash(
632639
password_hashing::hash_password(&config, &password)
@@ -636,10 +643,13 @@ mod tests {
636643
.build()
637644
.unwrap(),
638645
)]])
639-
.append_query_results([user_option::tests::get_user_options_mock(&UserOptions {
640-
ignore_password_expiry: Some(true),
641-
..Default::default()
642-
})])
646+
.append_query_results([user_option::tests::get_user_options_mock(
647+
"user_id",
648+
&UserOptions {
649+
ignore_password_expiry: Some(true),
650+
..Default::default()
651+
},
652+
)])
643653
.append_query_results([vec![user::tests::get_user_mock("user_id")]])
644654
.append_query_results([vec![user::tests::get_user_mock("user_id")]])
645655
.into_connection();

src/identity/backends/sql/federated_user.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,3 +48,19 @@ impl UserResponseBuilder {
4848
self
4949
}
5050
}
51+
52+
#[cfg(test)]
53+
pub(crate) mod tests {
54+
use crate::db::entity::federated_user as db_federated_user;
55+
56+
pub fn get_federated_user_mock<UID: Into<String>>(user_id: UID) -> db_federated_user::Model {
57+
db_federated_user::Model {
58+
id: 1,
59+
user_id: user_id.into(),
60+
idp_id: "idp_id".into(),
61+
protocol_id: "protocol_id".into(),
62+
unique_id: "uid".into(),
63+
display_name: Some("foo".into()),
64+
}
65+
}
66+
}

src/identity/backends/sql/local_user.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,15 +32,15 @@ impl UserResponseBuilder {
3232
}
3333

3434
#[cfg(test)]
35-
pub(super) mod tests {
35+
pub(crate) mod tests {
3636
use chrono::Utc;
3737

3838
use crate::db::entity::{local_user as db_local_user, password as db_password};
3939

40-
pub fn get_local_user_mock() -> db_local_user::Model {
40+
pub fn get_local_user_mock<UID: Into<String>>(user_id: UID) -> db_local_user::Model {
4141
db_local_user::Model {
4242
id: 1,
43-
user_id: "user_id".into(),
43+
user_id: user_id.into(),
4444
domain_id: "foo_domain".into(),
4545
name: "foo_domain".into(),
4646
failed_auth_count: Some(0),

src/identity/backends/sql/local_user/load.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ pub async fn load_local_user_with_passwords<S1: AsRef<str>, S2: AsRef<str>, S3:
6363
///
6464
/// Returns vector of optional vectors with passwords in the same order as
6565
/// requested keeping None in place where local_user was empty.
66-
pub async fn load_local_users_passwords<L: IntoIterator<Item = Option<i32>>>(
66+
pub async fn load_local_users_passwords<L: IntoIterator<Item = Option<i32>> + std::fmt::Debug>(
6767
db: &DatabaseConnection,
6868
user_ids: L,
6969
) -> Result<Vec<Option<Vec<password::Model>>>, IdentityDatabaseError> {

src/identity/backends/sql/nonlocal_user.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,3 +21,16 @@ impl UserResponseBuilder {
2121
self
2222
}
2323
}
24+
25+
#[cfg(test)]
26+
pub(crate) mod tests {
27+
use crate::db::entity::nonlocal_user as db_nonlocal_user;
28+
29+
pub fn get_nonlocal_user_mock<UID: Into<String>>(user_id: UID) -> db_nonlocal_user::Model {
30+
db_nonlocal_user::Model {
31+
user_id: user_id.into(),
32+
domain_id: "foo_domain".into(),
33+
name: "foo".into(),
34+
}
35+
}
36+
}

0 commit comments

Comments
 (0)