From 48660a17520b0b74b16095ec35864c16af15a542 Mon Sep 17 00:00:00 2001 From: zerox80 Date: Wed, 1 Jul 2026 16:38:44 +0200 Subject: [PATCH] refactor: extract pure AD-fallback helpers and fold overview's double-pass get_ad_users mixed cache/TTL/fetch orchestration with two untested pure steps (fallback-user building, filter+truncate); extract those into a new ad_users module with unit tests, so the pure logic is testable independent of the tauri::State-based command. build_overview scanned devices twice to compute upgrade_needed (once in the main tally loop, once via a separate .filter().count()); fold the second pass into the existing loop. status.upgrade and upgrade_needed remain distinct counters as before (guarded by a new regression test). No behavior change; both refactors are pure code-quality follow-ups from a full repo review. Co-Authored-By: Claude Sonnet 5 --- app/src-tauri/src/ad_users.rs | 49 ++++++++++ app/src-tauri/src/ad_users_tests.rs | 111 ++++++++++++++++++++++ app/src-tauri/src/commands.rs | 34 +------ app/src-tauri/src/lib.rs | 3 + app/src-tauri/src/model.rs | 2 +- app/src-tauri/src/store/overview.rs | 6 +- app/src-tauri/src/store/overview_tests.rs | 28 ++++++ 7 files changed, 201 insertions(+), 32 deletions(-) create mode 100644 app/src-tauri/src/ad_users.rs create mode 100644 app/src-tauri/src/ad_users_tests.rs diff --git a/app/src-tauri/src/ad_users.rs b/app/src-tauri/src/ad_users.rs new file mode 100644 index 0000000..a706275 --- /dev/null +++ b/app/src-tauri/src/ad_users.rs @@ -0,0 +1,49 @@ +//! Reine Hilfsfunktionen fuer `get_ad_users`: CSV/Inventar-Fallback-Liste bauen und +//! Suchfilter + Truncate anwenden. Die zustandsbehaftete Cache/TTL/Fetch-Orchestrierung +//! (inkl. `ad_fetch`-vor-`inner`-Lock-Reihenfolge) bleibt bewusst in `commands.rs`. +use crate::identity::synth_sam; +use crate::model::{AdUser, DeviceFull}; +use std::collections::HashSet; + +/// Baut die Fallback-Benutzerliste aus Geraetedaten (CSV/Inventar), wenn AD deaktiviert +/// oder keine AD-Antwort verfuegbar ist. Dedupliziert nach (synthetisiertem oder echtem) +/// SAM und sortiert nach Anzeigename. +pub(crate) fn fallback_users_from_devices(devs: &[DeviceFull]) -> Vec { + let mut seen = HashSet::new(); + let mut users: Vec = Vec::new(); + for d in devs { + if d.user_display.is_empty() || d.user_display == "Unbekannt" { + continue; + } + let sam = if d.user_sam.is_empty() { + synth_sam(&d.user_display) + } else { + d.user_sam.clone() + }; + if seen.insert(sam.clone()) { + users.push(AdUser { + sam, + display: d.user_display.clone(), + dept: d.dept.clone(), + mail: String::new(), + }); + } + } + users.sort_by(|a, b| a.display.cmp(&b.display)); + users +} + +/// Filtert per Case-insensitive Substring-Suche ueber Anzeigename/SAM/Abteilung/Mail +/// (nur wenn `query_lower` nicht leer ist) und kappt das Ergebnis auf maximal 100 Treffer. +/// `query_lower` muss bereits kleingeschrieben sein. +pub(crate) fn filter_and_truncate(mut users: Vec, query_lower: &str) -> Vec { + if !query_lower.is_empty() { + users.retain(|u| { + format!("{} {} {} {}", u.display, u.sam, u.dept, u.mail) + .to_lowercase() + .contains(query_lower) + }); + } + users.truncate(100); + users +} diff --git a/app/src-tauri/src/ad_users_tests.rs b/app/src-tauri/src/ad_users_tests.rs new file mode 100644 index 0000000..ec84408 --- /dev/null +++ b/app/src-tauri/src/ad_users_tests.rs @@ -0,0 +1,111 @@ +use crate::ad_users::{fallback_users_from_devices, filter_and_truncate}; +use crate::model::{AdUser, DeviceFull}; + +fn device(host: &str, user_display: &str, user_sam: &str, dept: &str) -> DeviceFull { + DeviceFull { + host: host.into(), + user_display: user_display.into(), + user_sam: user_sam.into(), + dept: dept.into(), + ..Default::default() + } +} + +fn user(sam: &str, display: &str, dept: &str, mail: &str) -> AdUser { + AdUser { + sam: sam.into(), + display: display.into(), + dept: dept.into(), + mail: mail.into(), + } +} + +#[test] +fn fallback_users_from_devices_skips_empty_and_unbekannt() { + let devs = vec![ + device("WS-A", "", "", "IT"), + device("WS-B", "Unbekannt", "", "IT"), + device("WS-C", "Anna Berger", "a.berger", "IT"), + ]; + let users = fallback_users_from_devices(&devs); + assert_eq!(users.len(), 1); + assert_eq!(users[0].display, "Anna Berger"); +} + +#[test] +fn fallback_users_from_devices_dedupes_by_sam_and_synthesizes_when_missing() { + let devs = vec![ + // Kein user_sam -> wird aus dem Anzeigenamen synthetisiert. + device("WS-A", "Jürgen Müller", "", "IT"), + // Zweites Geraet mit gleichem synthetisiertem SAM -> dedupliziert (erstes gewinnt). + device("WS-B", "Jürgen Müller", "", "Marketing"), + // Eigener echter SAM -> eigener Eintrag. + device("WS-C", "Anna Berger", "a.berger", "IT"), + ]; + let users = fallback_users_from_devices(&devs); + assert_eq!(users.len(), 2); + let juergen = users.iter().find(|u| u.sam == "juergen.mueller").unwrap(); + assert_eq!(juergen.display, "Jürgen Müller"); + assert_eq!(juergen.dept, "IT", "erstes Geraet (IT) gewinnt beim Dedup"); + assert!(users.iter().any(|u| u.sam == "a.berger")); +} + +#[test] +fn fallback_users_from_devices_sorts_by_display_name() { + let devs = vec![ + device("WS-A", "Zoe Wagner", "z.wagner", "IT"), + device("WS-B", "Anna Berger", "a.berger", "IT"), + device("WS-C", "Markus Bauer", "m.bauer", "Vertrieb"), + ]; + let users = fallback_users_from_devices(&devs); + let names: Vec<&str> = users.iter().map(|u| u.display.as_str()).collect(); + assert_eq!(names, vec!["Anna Berger", "Markus Bauer", "Zoe Wagner"]); +} + +#[test] +fn filter_and_truncate_is_noop_for_empty_query() { + let users = vec![ + user("z.wagner", "Zoe Wagner", "IT", "z.wagner@example.com"), + user( + "a.berger", + "Anna Berger", + "Marketing", + "a.berger@example.com", + ), + ]; + let result = filter_and_truncate(users.clone(), ""); + assert_eq!(result.len(), users.len()); + assert_eq!(result[0].display, users[0].display); + assert_eq!(result[1].display, users[1].display); +} + +#[test] +fn filter_and_truncate_matches_case_insensitively_across_all_fields() { + let users = vec![ + user( + "a.berger", + "Anna Berger", + "Marketing", + "a.berger@example.com", + ), + user("m.bauer", "Markus Bauer", "Vertrieb", "m.bauer@example.com"), + ]; + // Treffer ueber Abteilung (Marketing), Gross-/Kleinschreibung ignoriert. + let by_dept = filter_and_truncate(users.clone(), "marketing"); + assert_eq!(by_dept.len(), 1); + assert_eq!(by_dept[0].sam, "a.berger"); + + // Treffer ueber SAM. + let by_sam = filter_and_truncate(users, "m.bauer"); + assert_eq!(by_sam.len(), 1); + assert_eq!(by_sam[0].sam, "m.bauer"); +} + +#[test] +fn filter_and_truncate_caps_result_at_100() { + let users: Vec = (0..150) + .map(|i| user(&format!("u{i}"), &format!("User {i}"), "IT", "")) + .collect(); + let result = filter_and_truncate(users, ""); + assert_eq!(result.len(), 100); +} diff --git a/app/src-tauri/src/commands.rs b/app/src-tauri/src/commands.rs index 2d9672f..55b2d10 100644 --- a/app/src-tauri/src/commands.rs +++ b/app/src-tauri/src/commands.rs @@ -1,6 +1,7 @@ //! Tauri-Befehle (Bruecke Frontend <-> Backend). Halten Geraeteliste & AD-Cache im State. use crate::ad; -use crate::identity::{current_user_domain, synth_sam}; +use crate::ad_users::{fallback_users_from_devices, filter_and_truncate}; +use crate::identity::current_user_domain; use crate::model::*; use crate::store; use std::collections::BTreeSet; @@ -137,37 +138,10 @@ pub fn get_ad_users(state: State, search: String) -> Result Overview { let mut status_upgrade = 0i64; let mut stale = 0i64; let mut missing = 0i64; + let mut needs_upgrade_total = 0i64; let mut dept_map: HashMap = HashMap::new(); for d in devs { match d.status.as_str() { @@ -26,13 +27,16 @@ pub fn build_overview(devs: &[DeviceFull], th: &Thresholds) -> Overview { "missing" => missing += 1, _ => {} } + if needs_upgrade(d) { + needs_upgrade_total += 1; + } let e = dept_map.entry(d.dept.clone()).or_insert((0, 0)); e.0 += 1; if needs_action(d) { e.1 += 1; } } - let upgrade = devs.iter().filter(|d| needs_upgrade(d)).count() as i64; + let upgrade = needs_upgrade_total; let aged: Vec = devs.iter().filter_map(|d| d.age_years).collect(); let avg = if aged.is_empty() { 0.0 diff --git a/app/src-tauri/src/store/overview_tests.rs b/app/src-tauri/src/store/overview_tests.rs index 12425cd..88fb386 100644 --- a/app/src-tauri/src/store/overview_tests.rs +++ b/app/src-tauri/src/store/overview_tests.rs @@ -1,9 +1,37 @@ use super::common::now_iso; use super::test_support::{temp_config, unique_temp_dir}; use super::{build_devices, build_overview}; +use crate::model::{DeviceFull, Thresholds}; use std::fs; use std::path::Path; +fn device(host: &str, status: &str, reasons: Vec<&str>) -> DeviceFull { + DeviceFull { + host: host.into(), + status: status.into(), + upgrade_reasons: reasons.into_iter().map(String::from).collect(), + has_inventory: true, + ..Default::default() + } +} + +#[test] +fn upgrade_needed_and_status_upgrade_diverge_for_stale_with_reasons() { + // Regression fuer den Single-Pass-Fold in build_overview: "stale"-Geraete mit + // nicht-leeren upgrade_reasons zaehlen in upgrade_needed, aber nicht in status.upgrade. + let devs = vec![ + device("WS-UPGRADE", "upgrade", vec!["RAM knapp (8 GB)"]), + device("WS-STALE-REASONS", "stale", vec!["HDD statt SSD"]), + device("WS-OK", "ok", vec![]), + ]; + let ov = build_overview(&devs, &Thresholds::default()); + assert_eq!(ov.status.upgrade, 1, "nur das 'upgrade'-Status-Geraet"); + assert_eq!( + ov.upgrade_needed, 2, + "zusaetzlich das 'stale'-Geraet mit Begruendungen" + ); +} + #[test] fn age_buckets_scale_with_custom_max_age_threshold() { let root = unique_temp_dir("age-buckets-custom-threshold");