From e5d8cec2d6c603dedec8cb410c6640955f1806ae Mon Sep 17 00:00:00 2001 From: Xevion Date: Fri, 12 Sep 2025 20:50:47 -0500 Subject: [PATCH] refactor: reorganize banner api files, fix clippy lints, reformat --- src/banner/api.rs | 460 +++++++++++++--------------------- src/banner/errors.rs | 11 + src/banner/json.rs | 39 +++ src/banner/middleware.rs | 49 ++++ src/banner/mod.rs | 4 + src/banner/models/meetings.rs | 2 +- src/banner/models/terms.rs | 2 +- src/banner/session.rs | 6 +- src/services/manager.rs | 6 + 9 files changed, 283 insertions(+), 296 deletions(-) create mode 100644 src/banner/errors.rs create mode 100644 src/banner/json.rs create mode 100644 src/banner/middleware.rs diff --git a/src/banner/api.rs b/src/banner/api.rs index 8bb3f58..2184dbc 100644 --- a/src/banner/api.rs +++ b/src/banner/api.rs @@ -7,26 +7,19 @@ use std::{ }; use crate::banner::{ - BannerSession, SessionPool, models::*, nonce, query::SearchQuery, util::user_agent, + BannerSession, SessionPool, errors::BannerApiError, json::parse_json_with_context, + middleware::TransparentMiddleware, models::*, nonce, query::SearchQuery, util::user_agent, }; use anyhow::{Context, Result, anyhow}; use cookie::Cookie; use dashmap::DashMap; -use http::{Extensions, HeaderValue}; +use http::HeaderValue; use reqwest::{Client, Request, Response}; -use reqwest_middleware::{ClientBuilder, ClientWithMiddleware, Middleware, Next}; +use reqwest_middleware::{ClientBuilder, ClientWithMiddleware}; use serde_json; use tl; use tracing::{Level, Metadata, Span, debug, error, field::ValueSet, info, span, trace, warn}; -#[derive(Debug, thiserror::Error)] -pub enum BannerApiError { - #[error("Banner session is invalid or expired: {0}")] - InvalidSession(String), - #[error(transparent)] - RequestFailed(#[from] anyhow::Error), -} - /// Main Banner API client. pub struct BannerApi { pub sessions: SessionPool, @@ -34,49 +27,6 @@ pub struct BannerApi { base_url: String, } -pub struct TransparentMiddleware; - -#[async_trait::async_trait] -impl Middleware for TransparentMiddleware { - async fn handle( - &self, - req: Request, - extensions: &mut Extensions, - next: Next<'_>, - ) -> std::result::Result { - trace!( - domain = req.url().domain(), - headers = ?req.headers(), - "{method} {path}", - method = req.method().to_string(), - path = req.url().path(), - ); - let response_result = next.run(req, extensions).await; - - match response_result { - Ok(response) => { - if response.status().is_success() { - trace!( - "{code} {reason} {path}", - code = response.status().as_u16(), - reason = response.status().canonical_reason().unwrap_or("??"), - path = response.url().path(), - ); - Ok(response) - } else { - let e = response.error_for_status_ref().unwrap_err(); - warn!(error = ?e, "Request failed (server)"); - Ok(response) - } - } - Err(error) => { - warn!(?error, "Request failed (middleware)"); - Err(error) - } - } - } -} - impl BannerApi { /// Creates a new Banner API client. pub fn new(base_url: String) -> Result { @@ -101,6 +51,163 @@ impl BannerApi { }) } + /// Validates offset parameter for search methods. + fn validate_offset(offset: i32) -> Result<()> { + if offset <= 0 { + Err(anyhow::anyhow!("Offset must be greater than 0")) + } else { + Ok(()) + } + } + + /// Builds common search parameters for list endpoints. + fn build_list_params( + &self, + search: &str, + term: &str, + offset: i32, + max_results: i32, + session_id: &str, + ) -> Vec<(&str, String)> { + vec![ + ("searchTerm", search.to_string()), + ("term", term.to_string()), + ("offset", offset.to_string()), + ("max", max_results.to_string()), + ("uniqueSessionId", session_id.to_string()), + ("_", nonce()), + ] + } + + /// Makes a GET request to a list endpoint and parses JSON response. + async fn get_list_endpoint( + &self, + endpoint: &str, + search: &str, + term: &str, + offset: i32, + max_results: i32, + ) -> Result> + where + T: for<'de> serde::Deserialize<'de>, + { + Self::validate_offset(offset)?; + + let session = self.sessions.acquire(term.parse()?).await?; + let url = format!("{}/classSearch/{}", self.base_url, endpoint); + let params = self.build_list_params(search, term, offset, max_results, &session.id()); + + let response = self + .http + .get(&url) + .query(¶ms) + .send() + .await + .with_context(|| format!("Failed to get {}", endpoint))?; + + let data: Vec = response + .json() + .await + .with_context(|| format!("Failed to parse {} response", endpoint))?; + + Ok(data) + } + + /// Builds search parameters for course search methods. + fn build_search_params( + &self, + query: &SearchQuery, + term: &str, + session_id: &str, + sort: &str, + sort_descending: bool, + ) -> HashMap { + let mut params = query.to_params(); + params.insert("txt_term".to_string(), term.to_string()); + params.insert("uniqueSessionId".to_string(), session_id.to_string()); + params.insert("sortColumn".to_string(), sort.to_string()); + params.insert( + "sortDirection".to_string(), + if sort_descending { "desc" } else { "asc" }.to_string(), + ); + params.insert("startDatepicker".to_string(), String::new()); + params.insert("endDatepicker".to_string(), String::new()); + params + } + + /// Performs a course search and handles common response processing. + async fn perform_search( + &self, + term: &str, + query: &SearchQuery, + sort: &str, + sort_descending: bool, + ) -> Result { + let mut session = self.sessions.acquire(term.parse()?).await?; + + if session.been_used() { + self.http + .post(format!("{}/classSearch/resetDataForm", self.base_url)) + .header("Cookie", session.cookie()) + .send() + .await + .map_err(|e| BannerApiError::RequestFailed(e.into()))?; + } + + session.touch(); + + let params = self.build_search_params(query, term, &session.id(), sort, sort_descending); + + debug!( + term = term, + query = ?query, + sort = sort, + sort_descending = sort_descending, + "Searching for courses with params: {:?}", params + ); + + let response = self + .http + .get(format!("{}/searchResults/searchResults", self.base_url)) + .header("Cookie", session.cookie()) + .query(¶ms) + .send() + .await + .context("Failed to search courses")?; + + let status = response.status(); + let url = response.url().clone(); + let body = response + .text() + .await + .with_context(|| format!("Failed to read body (status={status})"))?; + + let search_result: SearchResult = parse_json_with_context(&body).map_err(|e| { + BannerApiError::RequestFailed(anyhow!( + "Failed to parse search response (status={status}, url={url}): {e}\nBody: {body}" + )) + })?; + + // Check for signs of an invalid session + if search_result.path_mode.is_none() { + return Err(BannerApiError::InvalidSession( + "Search result path mode is none".to_string(), + )); + } else if search_result.data.is_none() { + return Err(BannerApiError::InvalidSession( + "Search result data is none".to_string(), + )); + } + + if !search_result.success { + return Err(BannerApiError::RequestFailed(anyhow!( + "Search marked as unsuccessful by Banner API" + ))); + } + + Ok(search_result) + } + /// Retrieves a list of subjects from the Banner API. pub async fn get_subjects( &self, @@ -109,35 +216,8 @@ impl BannerApi { offset: i32, max_results: i32, ) -> Result> { - if offset <= 0 { - return Err(anyhow::anyhow!("Offset must be greater than 0")); - } - - let session = self.sessions.acquire(term.parse()?).await?; - let url = format!("{}/classSearch/get_subject", self.base_url); - let params = [ - ("searchTerm", search), - ("term", term), - ("offset", &offset.to_string()), - ("max", &max_results.to_string()), - ("uniqueSessionId", &session.id()), - ("_", &nonce()), - ]; - - let response = self - .http - .get(&url) - .query(¶ms) - .send() + self.get_list_endpoint("get_subject", search, term, offset, max_results) .await - .context("Failed to get subjects")?; - - let subjects: Vec = response - .json() - .await - .context("Failed to parse subjects response")?; - - Ok(subjects) } /// Retrieves a list of instructors from the Banner API. @@ -148,35 +228,8 @@ impl BannerApi { offset: i32, max_results: i32, ) -> Result> { - if offset <= 0 { - return Err(anyhow::anyhow!("Offset must be greater than 0")); - } - - let session = self.sessions.acquire(term.parse()?).await?; - let url = format!("{}/classSearch/get_instructor", self.base_url); - let params = [ - ("searchTerm", search), - ("term", term), - ("offset", &offset.to_string()), - ("max", &max_results.to_string()), - ("uniqueSessionId", &session.id()), - ("_", &nonce()), - ]; - - let response = self - .http - .get(&url) - .query(¶ms) - .send() + self.get_list_endpoint("get_instructor", search, term, offset, max_results) .await - .context("Failed to get instructors")?; - - let instructors: Vec = response - .json() - .await - .context("Failed to parse instructors response")?; - - Ok(instructors) } /// Retrieves a list of campuses from the Banner API. @@ -187,35 +240,8 @@ impl BannerApi { offset: i32, max_results: i32, ) -> Result> { - if offset <= 0 { - return Err(anyhow::anyhow!("Offset must be greater than 0")); - } - - let session = self.sessions.acquire(term.parse()?).await?; - let url = format!("{}/classSearch/get_campus", self.base_url); - let params = [ - ("searchTerm", search), - ("term", term), - ("offset", &offset.to_string()), - ("max", &max_results.to_string()), - ("uniqueSessionId", &session.id()), - ("_", &nonce()), - ]; - - let response = self - .http - .get(&url) - .query(¶ms) - .send() + self.get_list_endpoint("get_campus", search, term, offset, max_results) .await - .context("Failed to get campuses")?; - - let campuses: Vec = response - .json() - .await - .context("Failed to parse campuses response")?; - - Ok(campuses) } /// Retrieves meeting time information for a course. @@ -277,81 +303,8 @@ impl BannerApi { sort: &str, sort_descending: bool, ) -> Result { - // self.sessions.reset_data_form().await?; - - let mut session = self.sessions.acquire(term.parse()?).await?; - - if session.been_used() { - self.http - .post(&format!("{}/classSearch/resetDataForm", self.base_url)) - .header("Cookie", session.cookie()) - .send() - .await - .map_err(|e| BannerApiError::RequestFailed(e.into()))?; - } - - session.touch(); - - let mut params = query.to_params(); - - // Add additional parameters - params.insert("txt_term".to_string(), term.to_string()); - params.insert("uniqueSessionId".to_string(), session.id()); - params.insert("sortColumn".to_string(), sort.to_string()); - params.insert( - "sortDirection".to_string(), - if sort_descending { "desc" } else { "asc" }.to_string(), - ); - params.insert("startDatepicker".to_string(), String::new()); - params.insert("endDatepicker".to_string(), String::new()); - - debug!( - term = term, - query = ?query, - sort = sort, - sort_descending = sort_descending, - "Searching for courses with params: {:?}", params); - - let response = self - .http - .get(format!("{}/searchResults/searchResults", self.base_url)) - .header("Cookie", session.cookie()) - .query(¶ms) - .send() + self.perform_search(term, query, sort, sort_descending) .await - .context("Failed to search courses")?; - - let status = response.status(); - let url = response.url().clone(); - let body = response - .text() - .await - .with_context(|| format!("Failed to read body (status={status})"))?; - - let search_result: SearchResult = parse_json_with_context(&body).map_err(|e| { - BannerApiError::RequestFailed(anyhow!( - "Failed to parse search response (status={status}, url={url}): {e}\nBody: {body}" - )) - })?; - - // Check for signs of an invalid session, based on docs/Sessions.md - if search_result.path_mode.is_none() { - return Err(BannerApiError::InvalidSession( - "Search result path mode is none".to_string(), - )); - } else if search_result.data.is_none() { - return Err(BannerApiError::InvalidSession( - "Search result data is none".to_string(), - )); - } - - if !search_result.success { - return Err(BannerApiError::RequestFailed(anyhow!( - "Search marked as unsuccessful by Banner API" - ))); - } - - Ok(search_result) } /// Retrieves a single course by CRN by issuing a minimal search @@ -360,48 +313,15 @@ impl BannerApi { term: &str, crn: &str, ) -> Result, BannerApiError> { - // self.sessions.reset_data_form().await?; - // Ensure session is configured for this term - // self.select_term(term).await?; - - let session = self.sessions.acquire(term.parse()?).await?; - let query = SearchQuery::new() .course_reference_number(crn) .max_results(1); - let mut params = query.to_params(); - params.insert("txt_term".to_string(), term.to_string()); - params.insert("uniqueSessionId".to_string(), session.id()); - params.insert("sortColumn".to_string(), "subjectDescription".to_string()); - params.insert("sortDirection".to_string(), "asc".to_string()); - params.insert("startDatepicker".to_string(), String::new()); - params.insert("endDatepicker".to_string(), String::new()); + let search_result = self + .perform_search(term, &query, "subjectDescription", false) + .await?; - let url = format!("{}/searchResults/searchResults", self.base_url); - let response = self - .http - .get(&url) - .header("Cookie", session.cookie()) - .query(¶ms) - .send() - .await - .context("Failed to search course by CRN")?; - - let status = response.status(); - let url = response.url().clone(); - let body = response - .text() - .await - .with_context(|| format!("Failed to read body (status={status})"))?; - - let search_result: SearchResult = parse_json_with_context(&body).map_err(|e| { - BannerApiError::RequestFailed(anyhow!( - "Failed to parse search response for CRN (status={status}, url={url}): {e}" - )) - })?; - - // Check for signs of an invalid session, based on docs/Sessions.md + // Additional validation for CRN search if search_result.path_mode == Some("registration".to_string()) && search_result.data.is_none() { @@ -410,12 +330,6 @@ impl BannerApi { )); } - if !search_result.success { - return Err(BannerApiError::RequestFailed(anyhow!( - "Search marked as unsuccessful by Banner API" - ))); - } - Ok(search_result .data .and_then(|courses| courses.into_iter().next())) @@ -446,39 +360,3 @@ impl BannerApi { Ok(details) } } - -/// Attempt to parse JSON and, on failure, include a contextual snippet of the -/// line where the error occurred. This prevents dumping huge JSON bodies to logs. -fn parse_json_with_context(body: &str) -> Result { - match serde_json::from_str::(body) { - Ok(value) => Ok(value), - Err(err) => { - let (line, column) = (err.line(), err.column()); - let snippet = build_error_snippet(body, line, column, 80); - Err(anyhow::anyhow!( - "{err} at line {line}, column {column}\nSnippet:\n{snippet}", - )) - } - } -} - -fn build_error_snippet(body: &str, line: usize, column: usize, context_len: usize) -> String { - let target_line = body.lines().nth(line.saturating_sub(1)).unwrap_or(""); - if target_line.is_empty() { - return "(empty line)".to_string(); - } - - // column is 1-based, convert to 0-based for slicing - let error_idx = column.saturating_sub(1); - - let half_len = context_len / 2; - let start = error_idx.saturating_sub(half_len); - let end = (error_idx + half_len).min(target_line.len()); - - let slice = &target_line[start..end]; - let indicator_pos = error_idx - start; - - let indicator = " ".repeat(indicator_pos) + "^"; - - format!("...{slice}...\n {indicator}") -} diff --git a/src/banner/errors.rs b/src/banner/errors.rs new file mode 100644 index 0000000..e043d5d --- /dev/null +++ b/src/banner/errors.rs @@ -0,0 +1,11 @@ +//! Error types for the Banner API client. + +use thiserror::Error; + +#[derive(Debug, thiserror::Error)] +pub enum BannerApiError { + #[error("Banner session is invalid or expired: {0}")] + InvalidSession(String), + #[error(transparent)] + RequestFailed(#[from] anyhow::Error), +} diff --git a/src/banner/json.rs b/src/banner/json.rs new file mode 100644 index 0000000..746704b --- /dev/null +++ b/src/banner/json.rs @@ -0,0 +1,39 @@ +//! JSON parsing utilities for the Banner API client. + +use anyhow::Result; + +/// Attempt to parse JSON and, on failure, include a contextual snippet of the +/// line where the error occurred. This prevents dumping huge JSON bodies to logs. +pub fn parse_json_with_context(body: &str) -> Result { + match serde_json::from_str::(body) { + Ok(value) => Ok(value), + Err(err) => { + let (line, column) = (err.line(), err.column()); + let snippet = build_error_snippet(body, line, column, 80); + Err(anyhow::anyhow!( + "{err} at line {line}, column {column}\nSnippet:\n{snippet}", + )) + } + } +} + +fn build_error_snippet(body: &str, line: usize, column: usize, context_len: usize) -> String { + let target_line = body.lines().nth(line.saturating_sub(1)).unwrap_or(""); + if target_line.is_empty() { + return "(empty line)".to_string(); + } + + // column is 1-based, convert to 0-based for slicing + let error_idx = column.saturating_sub(1); + + let half_len = context_len / 2; + let start = error_idx.saturating_sub(half_len); + let end = (error_idx + half_len).min(target_line.len()); + + let slice = &target_line[start..end]; + let indicator_pos = error_idx - start; + + let indicator = " ".repeat(indicator_pos) + "^"; + + format!("...{slice}...\n {indicator}") +} diff --git a/src/banner/middleware.rs b/src/banner/middleware.rs new file mode 100644 index 0000000..8560e65 --- /dev/null +++ b/src/banner/middleware.rs @@ -0,0 +1,49 @@ +//! HTTP middleware for the Banner API client. + +use http::Extensions; +use reqwest::{Request, Response}; +use reqwest_middleware::{Middleware, Next}; +use tracing::{trace, warn}; + +pub struct TransparentMiddleware; + +#[async_trait::async_trait] +impl Middleware for TransparentMiddleware { + async fn handle( + &self, + req: Request, + extensions: &mut Extensions, + next: Next<'_>, + ) -> std::result::Result { + trace!( + domain = req.url().domain(), + headers = ?req.headers(), + "{method} {path}", + method = req.method().to_string(), + path = req.url().path(), + ); + let response_result = next.run(req, extensions).await; + + match response_result { + Ok(response) => { + if response.status().is_success() { + trace!( + "{code} {reason} {path}", + code = response.status().as_u16(), + reason = response.status().canonical_reason().unwrap_or("??"), + path = response.url().path(), + ); + Ok(response) + } else { + let e = response.error_for_status_ref().unwrap_err(); + warn!(error = ?e, "Request failed (server)"); + Ok(response) + } + } + Err(error) => { + warn!(?error, "Request failed (middleware)"); + Err(error) + } + } + } +} diff --git a/src/banner/mod.rs b/src/banner/mod.rs index 43f5d06..1ef50be 100644 --- a/src/banner/mod.rs +++ b/src/banner/mod.rs @@ -9,12 +9,16 @@ //! - Generate ICS files and calendar links pub mod api; +pub mod errors; +pub mod json; +pub mod middleware; pub mod models; pub mod query; pub mod session; pub mod util; pub use api::*; +pub use errors::*; pub use models::*; pub use query::*; pub use session::*; diff --git a/src/banner/models/meetings.rs b/src/banner/models/meetings.rs index 517a079..1f70a1d 100644 --- a/src/banner/models/meetings.rs +++ b/src/banner/models/meetings.rs @@ -419,7 +419,7 @@ impl MeetingScheduleInfo { pub fn from_meeting_time(meeting_time: &MeetingTime) -> Self { let days = MeetingDays::from_meeting_time(meeting_time); let time_range = match (&meeting_time.begin_time, &meeting_time.end_time) { - (Some(begin), Some(end)) => TimeRange::from_hhmm(&begin, &end), + (Some(begin), Some(end)) => TimeRange::from_hhmm(begin, end), _ => None, }; diff --git a/src/banner/models/terms.rs b/src/banner/models/terms.rs index 2b44cd3..c131d8f 100644 --- a/src/banner/models/terms.rs +++ b/src/banner/models/terms.rs @@ -193,7 +193,7 @@ impl std::fmt::Display for Term { impl Season { /// Returns the season code as a string - fn to_str(&self) -> &'static str { + fn to_str(self) -> &'static str { match self { Season::Fall => "10", Season::Spring => "20", diff --git a/src/banner/session.rs b/src/banner/session.rs index cd18c9c..d408cce 100644 --- a/src/banner/session.rs +++ b/src/banner/session.rs @@ -203,7 +203,7 @@ impl SessionPool { pub async fn acquire(&self, term: Term) -> Result { let term_pool = self .sessions - .entry(term.clone()) + .entry(term) .or_insert_with(|| Arc::new(TermPool::new())) .clone(); @@ -309,7 +309,7 @@ impl SessionPool { }) .collect::>(); - if cookies.get("JSESSIONID").is_none() || cookies.get("SSB_COOKIE").is_none() { + if !cookies.contains_key("JSESSIONID") || !cookies.contains_key("SSB_COOKIE") { return Err(anyhow::anyhow!("Failed to get cookies")); } @@ -386,7 +386,7 @@ impl SessionPool { .query(¶ms) .send() .await - .with_context(|| format!("Failed to get terms"))?; + .with_context(|| "Failed to get terms".to_string())?; let terms: Vec = response .json() diff --git a/src/services/manager.rs b/src/services/manager.rs index ce350b3..68e50f0 100644 --- a/src/services/manager.rs +++ b/src/services/manager.rs @@ -13,6 +13,12 @@ pub struct ServiceManager { shutdown_tx: broadcast::Sender<()>, } +impl Default for ServiceManager { + fn default() -> Self { + Self::new() + } +} + impl ServiceManager { pub fn new() -> Self { let (shutdown_tx, _) = broadcast::channel(1);