diff --git a/.github/workflows/test_python.yml b/.github/workflows/test_python.yml index ddc395d..8496ad4 100644 --- a/.github/workflows/test_python.yml +++ b/.github/workflows/test_python.yml @@ -33,7 +33,7 @@ jobs: maturin develop working-directory: ./wrappers/python - name: Start docker containers - run: docker-compose up -d + run: docker compose up -d working-directory: ./docker/${{ matrix.docker-version }} - name: Run pyright working-directory: ./wrappers/python diff --git a/.github/workflows/test_rust.yml b/.github/workflows/test_rust.yml index e6e08dc..7a83505 100644 --- a/.github/workflows/test_rust.yml +++ b/.github/workflows/test_rust.yml @@ -1,6 +1,31 @@ name: Unit tests, linting, and formatting -on: [push] +on: [ push ] jobs: + msrv: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - name: Update Rust + run: | + rustup override set stable + rustup update stable + - name: Install cargo-msrv + run: cargo install cargo-msrv --locked + - name: Check MSRV + run: cargo msrv verify --all-features --path framework/ + format: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - name: Update Rust + run: | + rustup override set stable + rustup update stable + - name: Lint + run: cargo clippy + - name: Check formatting + run: cargo fmt --check + checks: runs-on: ubuntu-latest strategy: @@ -13,11 +38,7 @@ jobs: rustup override set stable rustup update stable - name: Start docker containers - run: docker-compose up -d + run: docker compose up -d working-directory: ./docker/${{ matrix.docker-version }} - name: Run tests - run: cargo test --all-features - - name: Lint - run: cargo clippy - - name: Check formatting - run: cargo fmt --check \ No newline at end of file + run: cargo test --all-features \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index f13439d..e6fecd4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +# v0.6.0 + +* Breaking changes to error handling. More consistent and clearer error messages. + # v0.5.0 * Add logging of solr requests diff --git a/Cargo.toml b/Cargo.toml index 785dde3..e16d6a8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -7,7 +7,7 @@ members = [ [workspace.package] edition = "2021" -version = "0.5.0" +version = "0.6.0" [workspace.dependencies] solrstice = { path = "framework" } diff --git a/docker/8_0/docker-compose.yml b/docker/8_0/docker-compose.yml index d14d81b..f9e2cbe 100644 --- a/docker/8_0/docker-compose.yml +++ b/docker/8_0/docker-compose.yml @@ -10,14 +10,14 @@ services: ZOO_PORT: 2181 ZOO_SERVERS: 'server.1=0.0.0.0:2888:3888' ports: - - "2181:2181" + - "127.0.0.1:2181:2181" restart: unless-stopped solr1: build: context: . hostname: solr1 ports: - - "8983:8983" + - "127.0.0.1:8983:8983" volumes: - 'solr1_varsolr:/var/solr' environment: @@ -27,9 +27,15 @@ services: speedbump: image: kffl/speedbump:latest ports: - - "8984:8984" + - "127.0.0.1:8984:8984" command: --latency 2s --port 8984 solr1:8983 restart: unless-stopped + error_nginx: + image: nginx:alpine + volumes: + - '../error-nginx.conf:/etc/nginx/nginx.conf' + ports: + - '127.0.0.1:8985:80' volumes: zoo1_data: solr1_varsolr: diff --git a/docker/8_11/docker-compose.yml b/docker/8_11/docker-compose.yml index d14d81b..f9e2cbe 100644 --- a/docker/8_11/docker-compose.yml +++ b/docker/8_11/docker-compose.yml @@ -10,14 +10,14 @@ services: ZOO_PORT: 2181 ZOO_SERVERS: 'server.1=0.0.0.0:2888:3888' ports: - - "2181:2181" + - "127.0.0.1:2181:2181" restart: unless-stopped solr1: build: context: . hostname: solr1 ports: - - "8983:8983" + - "127.0.0.1:8983:8983" volumes: - 'solr1_varsolr:/var/solr' environment: @@ -27,9 +27,15 @@ services: speedbump: image: kffl/speedbump:latest ports: - - "8984:8984" + - "127.0.0.1:8984:8984" command: --latency 2s --port 8984 solr1:8983 restart: unless-stopped + error_nginx: + image: nginx:alpine + volumes: + - '../error-nginx.conf:/etc/nginx/nginx.conf' + ports: + - '127.0.0.1:8985:80' volumes: zoo1_data: solr1_varsolr: diff --git a/docker/9_0/docker-compose.yml b/docker/9_0/docker-compose.yml index d14d81b..f9e2cbe 100644 --- a/docker/9_0/docker-compose.yml +++ b/docker/9_0/docker-compose.yml @@ -10,14 +10,14 @@ services: ZOO_PORT: 2181 ZOO_SERVERS: 'server.1=0.0.0.0:2888:3888' ports: - - "2181:2181" + - "127.0.0.1:2181:2181" restart: unless-stopped solr1: build: context: . hostname: solr1 ports: - - "8983:8983" + - "127.0.0.1:8983:8983" volumes: - 'solr1_varsolr:/var/solr' environment: @@ -27,9 +27,15 @@ services: speedbump: image: kffl/speedbump:latest ports: - - "8984:8984" + - "127.0.0.1:8984:8984" command: --latency 2s --port 8984 solr1:8983 restart: unless-stopped + error_nginx: + image: nginx:alpine + volumes: + - '../error-nginx.conf:/etc/nginx/nginx.conf' + ports: + - '127.0.0.1:8985:80' volumes: zoo1_data: solr1_varsolr: diff --git a/docker/9_3/docker-compose.yml b/docker/9_3/docker-compose.yml index d14d81b..f9e2cbe 100644 --- a/docker/9_3/docker-compose.yml +++ b/docker/9_3/docker-compose.yml @@ -10,14 +10,14 @@ services: ZOO_PORT: 2181 ZOO_SERVERS: 'server.1=0.0.0.0:2888:3888' ports: - - "2181:2181" + - "127.0.0.1:2181:2181" restart: unless-stopped solr1: build: context: . hostname: solr1 ports: - - "8983:8983" + - "127.0.0.1:8983:8983" volumes: - 'solr1_varsolr:/var/solr' environment: @@ -27,9 +27,15 @@ services: speedbump: image: kffl/speedbump:latest ports: - - "8984:8984" + - "127.0.0.1:8984:8984" command: --latency 2s --port 8984 solr1:8983 restart: unless-stopped + error_nginx: + image: nginx:alpine + volumes: + - '../error-nginx.conf:/etc/nginx/nginx.conf' + ports: + - '127.0.0.1:8985:80' volumes: zoo1_data: solr1_varsolr: diff --git a/docker/error-nginx.conf b/docker/error-nginx.conf new file mode 100644 index 0000000..554ccc4 --- /dev/null +++ b/docker/error-nginx.conf @@ -0,0 +1,22 @@ +events {} + +http { + server { + listen 80; + location /solr/notfound_collection/ { + return 404 "Collection not found"; + } + + location /solr/error_collection/ { + return 500 "Internal Server Error"; + } + + location /solr/always_200/ { + return 200 "Always 200"; + } + + location /status { + return 200 "We are up"; + } + } +} diff --git a/docs/src/index.md b/docs/src/index.md index 377dfcb..6eff2cf 100644 --- a/docs/src/index.md +++ b/docs/src/index.md @@ -8,7 +8,7 @@ The library is written in Rust, and has a wrapper to Python. Both async and bloc ### Rust You can install the library by putting this in your `Cargo.toml` ```toml -solrstice = { version = "0.5.0", features = ["blocking"] } +solrstice = { version = "0.6.0", features = ["blocking"] } ``` If the `blocking` feature is not provided, only async will work. * [Rust mdBook docs]() diff --git a/framework/Cargo.toml b/framework/Cargo.toml index 5b294ca..7b9926e 100644 --- a/framework/Cargo.toml +++ b/framework/Cargo.toml @@ -8,6 +8,7 @@ keywords = ["solr", "search"] categories = ["api-bindings"] readme = "README.md" repository = "https://github.com/Sh1nku/solrstice" +rust-version = "1.73.0" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html diff --git a/framework/src/error.rs b/framework/src/error.rs index 27ec2f6..7b747fd 100644 --- a/framework/src/error.rs +++ b/framework/src/error.rs @@ -13,18 +13,20 @@ pub enum Error { #[error(transparent)] SerdeJsonError(#[from] serde_json::Error), - #[error("Error from Solr {code:?}: {msg:?}")] - SolrResponseError { code: usize, msg: String }, - #[error("Authentication error: {0}")] - SolrAuthError(String), #[error(transparent)] ZkError(#[from] zookeeper_async::ZkError), #[error(transparent)] StripPrefixError(#[from] std::path::StripPrefixError), - #[error("Solr Connection error: {0}")] - SolrConnectionError(String), + #[error("Solr setup error: {0}")] + SolrSetupError(String), + #[error("Solr connection error: {code:?} - {url:?}\n{msg:?}")] + SolrConnectionError { code: u16, url: String, msg: String }, + #[error("Solr response error: {code:?} - {url:?}\n{msg:?}")] + SolrResponseError { code: u16, url: String, msg: String }, + #[error("Solr auth error: {code:?} - {url:?}\n{msg:?}")] + SolrAuthError { code: u16, url: String, msg: String }, #[error("Unknown error: {0}")] Unknown(String), @@ -37,7 +39,7 @@ impl From<&str> for Error { } /// Helper function to check if a SolrResponse contains an error -pub fn try_solr_error(response: &SolrResponse) -> Result<(), Error> { +pub fn try_solr_error(url: String, response: &SolrResponse) -> Result<(), Error> { match &response.error { None => Ok(()), Some(err) => { @@ -49,6 +51,7 @@ pub fn try_solr_error(response: &SolrResponse) -> Result<(), Error> { } Err(Error::SolrResponseError { code: err.code, + url, msg, }) } diff --git a/framework/src/hosts/solr_server_host.rs b/framework/src/hosts/solr_server_host.rs index 916c160..048f0c7 100644 --- a/framework/src/hosts/solr_server_host.rs +++ b/framework/src/hosts/solr_server_host.rs @@ -59,9 +59,7 @@ impl SolrHost for SolrMultipleServerHost { async fn get_solr_node(&self) -> Result, Error> { let mut server_indices: Vec = (0..self.hosts.len()).collect(); if server_indices.is_empty() { - return Err(Error::SolrConnectionError( - "No Solr Host Specified".to_string(), - )); + return Err(Error::SolrSetupError("No Solr Host Specified".to_string())); } fastrand::shuffle(&mut server_indices); for i in server_indices { @@ -82,9 +80,7 @@ impl SolrHost for SolrMultipleServerHost { } } } - Err(Error::SolrConnectionError( - "No Solr Host answered".to_string(), - )) + Err(Error::SolrSetupError("No Solr Host answered".to_string())) } } diff --git a/framework/src/hosts/zookeeper_host.rs b/framework/src/hosts/zookeeper_host.rs index 58705aa..c99bfe3 100644 --- a/framework/src/hosts/zookeeper_host.rs +++ b/framework/src/hosts/zookeeper_host.rs @@ -119,7 +119,7 @@ impl SolrHost for ZookeeperEnsembleHost { async fn get_solr_node(&self) -> Result, Error> { let hosts = get_hosts_from_zookeeper(&self.client).await?; match hosts.get(fastrand::usize(0..hosts.len())) { - None => Err(Error::SolrConnectionError( + None => Err(Error::SolrSetupError( "No ready Solr nodes from Zookeeper".to_string(), )), //TODO Investigate this further. Is it always http://, and do people use auth? diff --git a/framework/src/models/response.rs b/framework/src/models/response.rs index f072a93..70b8727 100644 --- a/framework/src/models/response.rs +++ b/framework/src/models/response.rs @@ -28,7 +28,7 @@ pub struct SolrResponseError { /// The trace of the error. pub trace: Option, /// The code of the error. - pub code: usize, + pub code: u16, } fn default_true() -> bool { diff --git a/framework/src/queries/request_builder.rs b/framework/src/queries/request_builder.rs index c873a9b..036142e 100644 --- a/framework/src/queries/request_builder.rs +++ b/framework/src/queries/request_builder.rs @@ -1,6 +1,7 @@ use crate::error::{try_solr_error, Error}; use crate::models::context::SolrServerContext; use crate::models::response::SolrResponse; +use crate::Error::SolrConnectionError; use log::debug; use reqwest::header::HeaderMap; use reqwest::{Body, Request, RequestBuilder, Response, Url}; @@ -73,10 +74,7 @@ impl<'a> SolrRequestBuilder<'a> { log_request_info(&request, self.context.logging_policy); let response = client.execute(request).await?; - try_request_auth_error(&response).await?; - let solr_response = response.json::().await?; - try_solr_error(&solr_response)?; - Ok(solr_response) + handle_solr_response(response).await } pub async fn send_post_with_json( @@ -98,10 +96,7 @@ impl<'a> SolrRequestBuilder<'a> { log_request_info(&request, self.context.logging_policy); let response = client.execute(request).await?; - try_request_auth_error(&response).await?; - let solr_response = response.json::().await?; - try_solr_error(&solr_response)?; - Ok(solr_response) + handle_solr_response(response).await } pub async fn send_post_with_body>(self, data: T) -> Result { @@ -120,10 +115,7 @@ impl<'a> SolrRequestBuilder<'a> { log_request_info(&request, self.context.logging_policy); let response = client.execute(request).await?; - try_request_auth_error(&response).await?; - let solr_response = response.json::().await?; - try_solr_error(&solr_response)?; - Ok(solr_response) + handle_solr_response(response).await } } @@ -154,22 +146,27 @@ async fn create_standard_request<'a>( Ok(request) } -async fn try_request_auth_error(response: &Response) -> Result<(), Error> { - match response.error_for_status_ref() { - Ok(_) => Ok(()), - Err(e) => { - if e.status().ok_or(Error::Unknown( - "Error while getting response code from request".to_string(), - ))? == 401 - { - Err(Error::SolrAuthError( - "Authentication failed with 401. Check credentials.".to_string(), - )) - } else { - Ok(()) - } - } +async fn handle_solr_response(response: Response) -> Result { + let url = response.url().clone(); + let status_code = response.status(); + let body = response.text().await.unwrap_or_default(); + let solr_response = serde_json::from_str::(&body); + if let Ok(r) = solr_response { + try_solr_error(url.to_string(), &r)?; + return Ok(r); + } + if status_code == 401 { + return Err(Error::SolrAuthError { + code: status_code.as_u16(), + url: url.to_string(), + msg: body, + }); } + Err(SolrConnectionError { + url: url.to_string(), + code: status_code.as_u16(), + msg: body, + }) } static NO_BODY: &[u8] = "No body".as_bytes(); diff --git a/framework/tests/functionality/auth_test.rs b/framework/tests/functionality/auth_test.rs index c1b4bfd..40a017b 100644 --- a/framework/tests/functionality/auth_test.rs +++ b/framework/tests/functionality/auth_test.rs @@ -18,7 +18,7 @@ async fn auth_gives_sensible_error_when_not_provided() -> Result<(), Error> { match response { Ok(_) => Err(Error::Unknown("Should not have succeeded".to_string())), Err(e) => match e { - Error::SolrAuthError(_) => Ok(()), + Error::SolrAuthError { .. } => Ok(()), _ => Err(Error::Unknown("Should have been auth error".to_string())), }, } @@ -39,7 +39,7 @@ async fn auth_gives_sensible_error_when_wrong() -> Result<(), Error> { match response { Ok(_) => Err(Error::Unknown("Should not have succeeded".to_string())), Err(e) => match e { - Error::SolrAuthError(_) => Ok(()), + Error::SolrAuthError { .. } => Ok(()), _ => Err(Error::Unknown("Should have been auth error".to_string())), }, } diff --git a/framework/tests/functionality/error_test.rs b/framework/tests/functionality/error_test.rs new file mode 100644 index 0000000..f8a01a2 --- /dev/null +++ b/framework/tests/functionality/error_test.rs @@ -0,0 +1,39 @@ +use crate::structures::ErrrorTestsSetup; +use serial_test::serial; +use solrstice::{AsyncSolrCloudClient, Error, SelectQuery}; + +#[tokio::test] +#[serial] +async fn sensible_error_message_if_not_solr_server() -> Result<(), Error> { + let config = ErrrorTestsSetup::new().await; + let client = AsyncSolrCloudClient::new(config.context); + + let result = client.select(SelectQuery::new(), "error_collection").await; + assert!(result.is_err() && result.unwrap_err().to_string().contains("500")); + Ok(()) +} + +#[tokio::test] +#[serial] +async fn sensible_error_message_if_non_existent_collection() -> Result<(), Error> { + let config = ErrrorTestsSetup::new().await; + let client = AsyncSolrCloudClient::new(config.context); + + let result = client + .select(SelectQuery::new(), "notfound_collection") + .await; + let msg = result.unwrap_err().to_string(); + assert!(msg.contains("404")); + Ok(()) +} + +#[tokio::test] +#[serial] +async fn sensible_error_message_if_200_but_not_solr() -> Result<(), Error> { + let config = ErrrorTestsSetup::new().await; + let client = AsyncSolrCloudClient::new(config.context); + + let result = client.select(SelectQuery::new(), "always_200").await; + assert!(result.is_err() && result.unwrap_err().to_string().contains("200")); + Ok(()) +} diff --git a/framework/tests/functionality/logging_test.rs b/framework/tests/functionality/logging_test.rs index eae750b..e2da049 100644 --- a/framework/tests/functionality/logging_test.rs +++ b/framework/tests/functionality/logging_test.rs @@ -1,10 +1,10 @@ -use crate::structures::BaseTestsBuildup; +use crate::structures::{BaseTestsBuildup, ErrrorTestsSetup, FunctionalityTestsBuildup}; use log::{Metadata, Record}; use serial_test::serial; -use solrstice::AsyncSolrCloudClient; use solrstice::Error; use solrstice::LoggingPolicy; use solrstice::SolrServerContextBuilder; +use solrstice::{AsyncSolrCloudClient, SelectQuery}; use std::sync::{Arc, Mutex, OnceLock}; struct TestLogger { @@ -96,3 +96,50 @@ async fn logging_does_not_log_message_if_disabled() -> Result<(), Error> { } Ok(()) } + +#[tokio::test] +#[serial] +async fn logging_works_if_request_fails() -> Result<(), Error> { + let config = FunctionalityTestsBuildup::build_up("SyntaxErrorLogging") + .await + .unwrap(); + let client = AsyncSolrCloudClient::new(config.context); + + LOGGER_MESSAGES + .get_or_init(init_logger) + .lock() + .unwrap() + .clear(); + + let query = SelectQuery::new().fq(["this_is_a_syntax_error::0"]); + let _ = client.select(query, &config.collection_name).await; + let messages = LOGGER_MESSAGES.get().unwrap().lock().unwrap(); + for message in messages.iter() { + if message.contains("Sending Solr request to") { + return Ok(()); + } + } + Err(Error::Unknown("No log message found".to_string())) +} + +#[tokio::test] +#[serial] +async fn logging_works_if_request_never_reaches_solr() -> Result<(), Error> { + let config = ErrrorTestsSetup::new().await; + let client = AsyncSolrCloudClient::new(config.context); + + LOGGER_MESSAGES + .get_or_init(init_logger) + .lock() + .unwrap() + .clear(); + + let _ = client.select(SelectQuery::new(), "error_collection").await; + let messages = LOGGER_MESSAGES.get().unwrap().lock().unwrap(); + for message in messages.iter() { + if message.contains("Sending Solr request to") { + return Ok(()); + } + } + Err(Error::Unknown("No log message found".to_string())) +} diff --git a/framework/tests/functionality/mod.rs b/framework/tests/functionality/mod.rs index 1d7941b..41d9015 100644 --- a/framework/tests/functionality/mod.rs +++ b/framework/tests/functionality/mod.rs @@ -13,6 +13,8 @@ pub mod zk_test; pub mod auth_test; +pub mod error_test; +pub mod logging_test; + #[cfg(feature = "blocking")] pub mod blocking_tests; -mod logging_test; diff --git a/framework/tests/structures.rs b/framework/tests/structures.rs index bfdb96b..9f54ae8 100644 --- a/framework/tests/structures.rs +++ b/framework/tests/structures.rs @@ -47,6 +47,46 @@ impl BaseTestsBuildup { } } +pub struct ErrrorTestsSetup { + pub context: SolrServerContext, + pub config_path: String, + pub host: SolrSingleServerHost, + pub auth: Option, +} + +impl ErrrorTestsSetup { + pub async fn new() -> Self { + dotenv::from_filename("../test_setup/.env").ok(); + let username = std::env::var("SOLR_USERNAME").unwrap(); + let password = std::env::var("SOLR_PASSWORD").unwrap(); + let auth = match username.is_empty() { + true => None, + false => Some(SolrBasicAuth::new( + username.as_str(), + Some(password.as_str()), + )), + }; + let error_nginx_hostname = std::env::var("ERROR_NGINX_HOST") + .unwrap() + .as_str() + .to_string(); + let host = SolrSingleServerHost::new(&error_nginx_hostname); + let builder = SolrServerContextBuilder::new(host.clone()); + let context = if let Some(auth) = auth.clone() { + builder.with_auth(auth).build() + } else { + builder.build() + }; + wait_for_error_nginx(&error_nginx_hostname, Duration::from_secs(30)).await; + ErrrorTestsSetup { + context, + config_path: "../test_setup/test_collection".to_string(), + host, + auth, + } + } +} + pub struct FunctionalityTestsBuildup { pub context: SolrServerContext, pub async_client: AsyncSolrCloudClient, @@ -147,3 +187,14 @@ pub async fn wait_for_solr(context: &SolrServerContext, max_time: Duration) { } panic!("Solr did not respond within {:?} seconds", max_time); } + +pub async fn wait_for_error_nginx(host: &str, max_time: Duration) { + let end = std::time::Instant::now() + max_time; + while std::time::Instant::now() < end { + let response = reqwest::get(format!("{}/status", host)).await.unwrap(); + if response.status().is_success() { + return; + } + tokio::time::sleep(Duration::from_secs(1)).await; + } +} diff --git a/test_setup/.env b/test_setup/.env index f743824..5229ff4 100644 --- a/test_setup/.env +++ b/test_setup/.env @@ -1,5 +1,6 @@ -ZK_HOST="localhost:2181" -SOLR_HOST="http://localhost:8983" +ZK_HOST="127.0.0.1:2181" +SOLR_HOST="http://127.0.0.1:8983" SOLR_USERNAME=solr SOLR_PASSWORD=SolrRocks -SPEEDBUMP_HOST="http://localhost:8984" \ No newline at end of file +SPEEDBUMP_HOST="http://127.0.0.1:8984" +ERROR_NGINX_HOST="http://127.0.0.1:8985" \ No newline at end of file diff --git a/wrappers/python/.gitignore b/wrappers/python/.gitignore index 5985622..56fe1c5 100644 --- a/wrappers/python/.gitignore +++ b/wrappers/python/.gitignore @@ -72,4 +72,5 @@ docs/_build/ .python-version docs/ -Cargo.lock \ No newline at end of file +Cargo.lock +.dmypy.json \ No newline at end of file diff --git a/wrappers/python/solrstice/__init__.pyi b/wrappers/python/solrstice/__init__.pyi index a1c4483..1cd51d3 100644 --- a/wrappers/python/solrstice/__init__.pyi +++ b/wrappers/python/solrstice/__init__.pyi @@ -9,7 +9,10 @@ if TYPE_CHECKING: # region auth class SolrAuth(ABC): - """Base class for Solr authentication""" + """ + Base class for Solr authentication + Valid implementations are :class:`SolrBasicAuth` + """ class SolrBasicAuth(SolrAuth): """Basic authentication for Solr @@ -25,10 +28,19 @@ class SolrBasicAuth(SolrAuth): # region def_type class QueryOperator(Enum): + """ + The default query operator + """ + AND = "AND" OR = "OR" class DefType(abc.ABC): + """ + Specify query type. + Valid implementations are :class:`LuceneQuery`, :class:`DismaxQuery`, and :class:`EdismaxQuery` + """ + pass class LuceneQuery(DefType): @@ -239,6 +251,11 @@ class JsonFacetComponent: pass class JsonFacetType(abc.ABC): + """ + Base class for a json facet type + Valid implementations are :class:`JsonTermsFacet`, :class:`JsonQueryFacet`, and :class:`JsonStatFacet` + """ + pass class JsonTermsFacet(JsonFacetType): @@ -298,7 +315,10 @@ class JsonStatFacet(JsonFacetType): # region hosts class SolrHost(ABC): - """Base class for Solr hosts""" + """ + Base class for Solr hosts + Valid implementations are :class:`SolrSingleServerHost`, :class:`SolrMultipleServerHost`, and :class:`ZookeeperEnsembleHost` + """ class SolrSingleServerHost(SolrHost): """Solr host for a single Solr instance @@ -341,7 +361,10 @@ class ZookeeperEnsembleHostConnector: pass class LoggingPolicy(abc.ABC): - """Policy describing how to log solr queries. Valid values are :class:`OffLoggingPolicy`, :class:`FastLoggingPolicy`, and :class:`PrettyLoggingPolicy`""" + """ + Policy describing how to log solr queries. + Valid values are :class:`OffLoggingPolicy`, :class:`FastLoggingPolicy`, and :class:`PrettyLoggingPolicy` + """ pass diff --git a/wrappers/python/tests/helpers.py b/wrappers/python/tests/helpers.py index a10eef3..c2aecf1 100644 --- a/wrappers/python/tests/helpers.py +++ b/wrappers/python/tests/helpers.py @@ -10,6 +10,7 @@ from solrstice import ( AsyncSolrCloudClient, + OffLoggingPolicy, SolrBasicAuth, SolrServerContext, SolrSingleServerHost, @@ -23,6 +24,7 @@ class Config: solr_host: str speedbump_host: Optional[str] + error_nginx_host: Optional[str] solr_username: Optional[str] solr_password: Optional[str] solr_auth: Optional[SolrBasicAuth] @@ -32,13 +34,20 @@ class Config: def get_path_prefix() -> str: - path_prefix = "../../" - if not os.path.exists(os.path.join(path_prefix, "test_setup/.env")): - path_prefix = "../../../" + iterations = 0 + path_prefix = "" + while True: + if not os.path.exists(os.path.join(path_prefix, "test_setup/.env")): + path_prefix += "../" + else: + break + iterations += 1 + if iterations > 100: + raise FileNotFoundError("Could not find test_setup/.env") return path_prefix -def create_config() -> Config: +def create_config(logging: bool = False) -> Config: path = os.path.join(get_path_prefix(), "test_setup/.env") load_dotenv(path) solr_auth = None @@ -49,12 +58,16 @@ def create_config() -> Config: host = os.getenv("SOLR_HOST") assert host is not None speedbump_host = os.getenv("SPEEDBUMP_HOST") + error_nginx_host = os.getenv("ERROR_NGINX_HOST") solr_host = SolrSingleServerHost(host) - context = SolrServerContext(solr_host, solr_auth) + context = SolrServerContext( + solr_host, solr_auth, OffLoggingPolicy() if not logging else None + ) wait_for_solr(host, 30) return Config( host, speedbump_host, + error_nginx_host, solr_username, solr_password, solr_auth, @@ -82,6 +95,36 @@ def wait_for_solr(host: str, max_time: int) -> None: raise RuntimeError(f"Solr did not respond within {max_time} seconds") +@dataclass +class ErrorTestsSetup: + error_nginx_host: str + context: SolrServerContext + async_client: AsyncSolrCloudClient + + +def create_nginx_error_config() -> ErrorTestsSetup: + path = os.path.join(get_path_prefix(), "test_setup/.env") + load_dotenv(path) + error_nginx_host = os.getenv("ERROR_NGINX_HOST") + assert error_nginx_host is not None + context = SolrServerContext(SolrSingleServerHost(error_nginx_host)) + wait_for_error_nginx(error_nginx_host, 30) + return ErrorTestsSetup(error_nginx_host, context, AsyncSolrCloudClient(context)) + + +def wait_for_error_nginx(host: str, max_time: int) -> None: + end = time.time() + max_time + while time.time() < end: + try: + with urlopen(f'{host}{"/status"}') as response: + if response.status == 200: + return + except Exception: + pass + time.sleep(1) + raise RuntimeError(f"Error nginx did not respond within {max_time} seconds") + + @dataclass_json @dataclass class Population(DataClassJsonMixin): diff --git a/wrappers/python/tests/test_external_error.py b/wrappers/python/tests/test_external_error.py new file mode 100644 index 0000000..eea47b5 --- /dev/null +++ b/wrappers/python/tests/test_external_error.py @@ -0,0 +1,42 @@ +from typing import Generator + +import pytest + +from solrstice import SelectQuery + +from .helpers import ErrorTestsSetup, create_nginx_error_config + + +@pytest.fixture() +def config() -> Generator[ErrorTestsSetup, None, None]: + yield create_nginx_error_config() + + +@pytest.mark.asyncio +async def test_sensible_error_message_if_not_solr_server( + config: ErrorTestsSetup, +) -> None: + try: + await config.async_client.select(SelectQuery(), "error_collection") + except Exception as e: + assert "500" in str(e) + + +@pytest.mark.asyncio +async def test_sensible_error_message_if_non_existent_collection( + config: ErrorTestsSetup, +) -> None: + try: + await config.async_client.select(SelectQuery(), "notfound_collection") + except Exception as e: + assert "404" in str(e) + + +@pytest.mark.asyncio +async def test_sensible_error_message_if_200_but_not_solr( + config: ErrorTestsSetup, +) -> None: + try: + await config.async_client.select(SelectQuery(), "always_200") + except Exception as e: + assert "200" in str(e) diff --git a/wrappers/python/tests/test_hosts.py b/wrappers/python/tests/test_hosts.py index 2a4d43a..b9e562b 100644 --- a/wrappers/python/tests/test_hosts.py +++ b/wrappers/python/tests/test_hosts.py @@ -33,7 +33,7 @@ def config() -> Generator[Config, None, None]: solr_username = os.getenv("SOLR_USERNAME") solr_password = os.getenv("SOLR_PASSWORD") - if solr_username is not None and solr_password is not "": + if solr_username is not None and solr_password != "": solr_auth = SolrBasicAuth( solr_username, solr_password, diff --git a/wrappers/python/tests/test_internal_error.py b/wrappers/python/tests/test_internal_error.py new file mode 100644 index 0000000..293ef1f --- /dev/null +++ b/wrappers/python/tests/test_internal_error.py @@ -0,0 +1,34 @@ +from typing import Generator + +import pytest + +from solrstice import SelectQuery + +from .helpers import ( + Config, + create_config, + setup_collection, + teardown_collection, + wait_for_solr, +) + + +@pytest.fixture() +def config() -> Generator[Config, None, None]: + yield create_config() + + +@pytest.mark.asyncio +async def test_syntax_error_returns_sensible_error(config: Config) -> None: + name = "SyntaxError" + wait_for_solr(config.solr_host, 30) + + try: + await setup_collection(config.context, name, config.config_path) + with pytest.raises(Exception) as e: + await config.async_client.select( + SelectQuery(fq=["some_field::syntax_error"]), name + ) + assert "400" in str(e) + finally: + await teardown_collection(config.context, name) diff --git a/wrappers/python/tests/test_logging.py b/wrappers/python/tests/test_logging.py index 53568ad..e48788f 100644 --- a/wrappers/python/tests/test_logging.py +++ b/wrappers/python/tests/test_logging.py @@ -1,3 +1,4 @@ +import asyncio import logging from typing import Generator @@ -11,22 +12,29 @@ @pytest.fixture() def config() -> Generator[Config, None, None]: - yield create_config() + yield create_config(logging=True) + + +log_lock = asyncio.Lock() # TODO This test fails if run in parallel with the rest of the test suite, but not if run alone # @pytest.mark.asyncio -# async def test_logging_logs_message(config: Config, caplog: LogCaptureFixture): +# async def test_logging_logs_message(config: Config, caplog: LogCaptureFixture) -> None: # wait_for_solr(config.solr_host, 30) # -# with caplog.at_level(logging.DEBUG): -# caplog.clear() -# assert not any( -# "Sending Solr request to" in msg for msg in [x.getMessage() for x in caplog.records]), "Logs are not empty" -# await config.async_client.get_configs() -# assert any( -# "Sending Solr request to" in msg for msg in -# [x.getMessage() for x in caplog.records]), "Expected log message not found" +# async with log_lock: +# with caplog.at_level(logging.DEBUG): +# caplog.clear() +# assert not any( +# "Sending Solr request to" in msg +# for msg in [x.getMessage() for x in caplog.records] +# ), "Logs are not empty" +# await config.async_client.get_configs() +# assert any( +# "Sending Solr request to" in msg +# for msg in [x.getMessage() for x in caplog.records] +# ), "Expected log message not found" @pytest.mark.asyncio @@ -38,14 +46,46 @@ async def test_logging_does_not_log_message_if_disabled( context = SolrServerContext(config.solr_host, config.solr_auth, OffLoggingPolicy()) client = AsyncSolrCloudClient(context) - with caplog.at_level(logging.DEBUG): - caplog.clear() - assert not any( - "Sending Solr request to" in msg - for msg in [x.getMessage() for x in caplog.records] - ), "Logs are not empty" - await client.get_configs() - assert not any( - "Sending Solr request to" in msg - for msg in [x.getMessage() for x in caplog.records] - ), "Logs are not empty" + async with log_lock: + with caplog.at_level(logging.DEBUG): + caplog.clear() + assert not any( + "Sending Solr request to" in msg + for msg in [x.getMessage() for x in caplog.records] + ), "Logs are not empty" + await client.get_configs() + assert not any( + "Sending Solr request to" in msg + for msg in [x.getMessage() for x in caplog.records] + ), "Logs are not empty" + + +# @pytest.mark.asyncio +# async def test_logging_works_if_request_fails( +# config: Config, caplog: LogCaptureFixture +# ) -> None: +# name = "LoggingWorksIfRequestFails" +# wait_for_solr(config.solr_host, 30) +# async with log_lock: +# with caplog.at_level(logging.DEBUG): +# try: +# await setup_collection(config.context, name, config.config_path) +# await index_test_data(config.context, name) +# +# caplog.clear() +# assert not any( +# "Sending Solr request to" in msg +# for msg in [x.getMessage() for x in caplog.records] +# ), "Logs are not empty" +# try: +# await config.async_client.select( +# SelectQuery(fq=["this_is_a_syntax_error::0"]), name +# ) +# except Exception: +# pass +# assert any( +# "Sending Solr request to" in msg +# for msg in [x.getMessage() for x in caplog.records] +# ), "Expected log message not found" +# finally: +# await teardown_collection(config.context, name)