From 9f40388f45cc18b2686b41f2e23d0ecaf3bd697c Mon Sep 17 00:00:00 2001
From: ByteBaker <42913098+ByteBaker@users.noreply.github.com>
Date: Tue, 8 Jul 2025 12:00:00 +0530
Subject: [PATCH] chore: add unit tests (#7487)

### **PR Type**
Tests, Enhancement


___

### **Description**
- Add extensive unit tests across common meta modules

- Refactor LokiError mapping and tag filtering logic

- Simplify record_batch usage and string interpolation

- Fix histogram interval functions and cache invalidation
---
 src/common/meta/http.rs                       |  30 +++++
 src/common/meta/ingestion.rs                  |  43 ++++++
 src/common/meta/loki.rs                       |  89 ++++++++-----
 src/common/meta/middleware_data.rs            |  48 +++----
 src/common/meta/organization.rs               | 124 ++++++++++++++++++
 src/common/meta/proxy.rs                      |  15 +++
 src/common/meta/search.rs                     |  34 +----
 src/common/meta/service.rs                    |  11 +-
 src/common/meta/stream.rs                     |   9 +-
 src/common/meta/syslog.rs                     |  10 ++
 src/common/meta/telemetry.rs                  |   8 ++
 src/config/src/utils/arrow.rs                 |  23 +---
 src/config/src/utils/sysinfo/cgroup.rs        |   6 +-
 src/service/search/cache/cacher.rs            |  21 ++-
 .../datafusion/optimizer/rewrite_histogram.rs |   1 +
 src/service/search/sql.rs                     |  78 ++++++-----
 src/service/tantivy/mod.rs                    |   2 +-
 src/service/tantivy/puffin/writer.rs          |   6 +-
 .../puffin_directory/caching_directory.rs     |   4 +-
 .../tantivy/puffin_directory/footer_cache.rs  |   4 +-
 src/service/tantivy/puffin_directory/mod.rs   |  12 +-
 .../tantivy/puffin_directory/reader_cache.rs  |  16 +--
 .../tantivy/puffin_directory/writer.rs        |  14 +-
 src/service/users.rs                          |  23 ++--
 24 files changed, 436 insertions(+), 195 deletions(-)

diff --git a/src/common/meta/http.rs b/src/common/meta/http.rs
index 874ed7a345..188b7760a1 100644
--- a/src/common/meta/http.rs
+++ b/src/common/meta/http.rs
@@ -312,4 +312,34 @@ mod tests {
         let response = HttpResponse::json(payload);
         assert_eq!(response.status(), http::StatusCode::OK);
     }
+
+    #[test]
+    fn test_optional_fields_not_serialized_when_none() {
+        let response = HttpResponse {
+            code: 200,
+            message: "ok".to_string(),
+            id: None,
+            name: None,
+            error_detail: None,
+            trace_id: None,
+        };
+        let serialized = serde_json::to_string(&response).unwrap();
+        assert!(!serialized.contains("id"));
+        assert!(!serialized.contains("name"));
+        assert!(!serialized.contains("error_detail"));
+        assert!(!serialized.contains("trace_id"));
+    }
+
+    #[test]
+    fn test_http_response_builder_chaining() {
+        let mut response = HttpResponse::message(http::StatusCode::OK, "test");
+        response
+            .with_id("id-1".into())
+            .with_name("name-1".into())
+            .with_trace_id("trace-1".into());
+
+        assert_eq!(response.id, Some("id-1".into()));
+        assert_eq!(response.name, Some("name-1".into()));
+        assert_eq!(response.trace_id, Some("trace-1".into()));
+    }
 }
diff --git a/src/common/meta/ingestion.rs b/src/common/meta/ingestion.rs
index cca9eb36ba..222887f0a9 100644
--- a/src/common/meta/ingestion.rs
+++ b/src/common/meta/ingestion.rs
@@ -650,4 +650,47 @@ mod tests {
         assert_eq!(request.message.message_id, "test-id");
         assert_eq!(request.subscription, "test-subscription");
     }
+
+    #[test]
+    fn test_ingestion_response_skip_empty_status_and_error() {
+        let response = IngestionResponse {
+            code: 200,
+            status: vec![],
+            error: None,
+        };
+        let serialized = serde_json::to_string(&response).unwrap();
+        assert!(!serialized.contains("status"));
+        assert!(!serialized.contains("error"));
+    }
+
+    #[test]
+    fn test_record_status_skip_empty_error() {
+        let status = RecordStatus {
+            successful: 1,
+            failed: 0,
+            error: "".to_string(),
+        };
+        let serialized = serde_json::to_string(&status).unwrap();
+        assert!(!serialized.contains("error"));
+    }
+
+    #[test]
+    fn test_hec_status_conversion() {
+        // Test all variants
+        let success: HecResponse = HecStatus::Success.into();
+        assert_eq!(success.text, "Success");
+        assert_eq!(success.code, 200);
+
+        let invalid_format: HecResponse = HecStatus::InvalidFormat.into();
+        assert_eq!(invalid_format.text, "Invalid data format");
+        assert_eq!(invalid_format.code, 400);
+
+        let invalid_index: HecResponse = HecStatus::InvalidIndex.into();
+        assert_eq!(invalid_index.text, "Incorrect index");
+        assert_eq!(invalid_index.code, 400);
+
+        let custom: HecResponse = HecStatus::Custom("Test error".to_string(), 418).into();
+        assert_eq!(custom.text, "Test error");
+        assert_eq!(custom.code, 418);
+    }
 }
diff --git a/src/common/meta/loki.rs b/src/common/meta/loki.rs
index e84f8a9e9e..b3a62e78dd 100644
--- a/src/common/meta/loki.rs
+++ b/src/common/meta/loki.rs
@@ -114,41 +114,37 @@ impl From<LokiError> for actix_web::HttpResponse {
     fn from(error: LokiError) -> Self {
         use actix_web::HttpResponse;
 
-        match error {
-            // Client errors (400)
-            LokiError::InvalidTimestamp { message } => HttpResponse::BadRequest()
+        if let LokiError::Ingestion { .. } = &error {
+            return HttpResponse::InternalServerError()
                 .content_type("text/plain")
-                .body(format!("invalid timestamp: {message}")),
-            LokiError::InvalidLabels { message } => HttpResponse::BadRequest()
-                .content_type("text/plain")
-                .body(format!("invalid labels: {message}")),
-            LokiError::EmptyStream => HttpResponse::BadRequest()
-                .content_type("text/plain")
-                .body("empty stream data"),
-            LokiError::UnsupportedContentType { content_type } => HttpResponse::BadRequest()
-                .content_type("text/plain")
-                .body(format!("unsupported content type: {content_type}")),
-            LokiError::UnsupportedContentEncoding { encoding } => HttpResponse::BadRequest()
-                .content_type("text/plain")
-                .body(format!("unsupported content encoding: {encoding}")),
-            LokiError::ProtobufDecode { source } => HttpResponse::BadRequest()
-                .content_type("text/plain")
-                .body(format!("failed to decode protobuf: {source}")),
-            LokiError::SnappyDecompression { source } => HttpResponse::BadRequest()
-                .content_type("text/plain")
-                .body(format!("failed to decompress snappy: {source}")),
-            LokiError::JsonParse { source } => HttpResponse::BadRequest()
-                .content_type("text/plain")
-                .body(format!("failed to parse JSON: {source}")),
-            LokiError::GzipDecompression { source } => HttpResponse::BadRequest()
-                .content_type("text/plain")
-                .body(format!("failed to decompress gzip: {source}")),
-
-            // Server errors (500)
-            LokiError::Ingestion { .. } => HttpResponse::InternalServerError()
-                .content_type("text/plain")
-                .body("internal server error during log ingestion"),
+                .body("internal server error during log ingestion");
         }
+        let body = match error {
+            LokiError::InvalidTimestamp { message } => format!("invalid timestamp: {message}"),
+            LokiError::InvalidLabels { message } => format!("invalid labels: {message}"),
+            LokiError::EmptyStream => "empty stream data".to_string(),
+            LokiError::UnsupportedContentType { content_type } => {
+                format!("unsupported content type: {content_type}")
+            }
+            LokiError::UnsupportedContentEncoding { encoding } => {
+                format!("unsupported content encoding: {encoding}")
+            }
+            LokiError::ProtobufDecode { source } => {
+                format!("failed to decode protobuf: {source}")
+            }
+            LokiError::SnappyDecompression { source } => {
+                format!("failed to decompress snappy: {source}")
+            }
+            LokiError::JsonParse { source } => format!("failed to parse JSON: {source}"),
+            LokiError::GzipDecompression { source } => {
+                format!("failed to decompress gzip: {source}")
+            }
+            LokiError::Ingestion { .. } => unreachable!("Already tested above"),
+        };
+
+        HttpResponse::BadRequest()
+            .content_type("text/plain")
+            .body(body)
     }
 }
 
@@ -257,4 +253,31 @@ mod tests {
         assert_eq!(request.streams[0].values[0].line, "log message");
         assert!(request.streams[0].values[0].structured_metadata.is_some());
     }
+    #[test]
+    fn test_loki_entry_default() {
+        let entry = LokiEntry::default();
+        assert_eq!(entry.timestamp, "");
+        assert_eq!(entry.line, "");
+        assert!(entry.structured_metadata.is_none());
+    }
+
+    #[test]
+    fn test_loki_stream_structure() {
+        let mut labels = HashMap::new();
+        labels.insert("service".to_string(), "api".to_string());
+
+        let entry = LokiEntry {
+            timestamp: "123".to_string(),
+            line: "log".to_string(),
+            structured_metadata: None,
+        };
+
+        let stream = LokiStream {
+            stream: labels.clone(),
+            values: vec![entry],
+        };
+
+        assert_eq!(stream.stream, labels);
+        assert_eq!(stream.values.len(), 1);
+    }
 }
diff --git a/src/common/meta/middleware_data.rs b/src/common/meta/middleware_data.rs
index 99acfc713d..b4a4b84de8 100644
--- a/src/common/meta/middleware_data.rs
+++ b/src/common/meta/middleware_data.rs
@@ -67,17 +67,16 @@ impl RumExtraData {
     }
 
     fn filter_tags(data: &HashMap<String, String>) -> HashMap<String, serde_json::Value> {
-        match data.get("ootags").or_else(|| data.get("o2tags")) {
-            Some(tags) => tags
-                .split(',')
-                .map(|tag| {
-                    let key_val: Vec<_> = tag.split(':').collect();
-                    (key_val[0].to_string(), key_val[1].into())
-                })
-                .collect(),
-
-            None => HashMap::default(),
-        }
+        data.get("ootags")
+            .or_else(|| data.get("o2tags"))
+            .map_or_else(HashMap::default, |tags| {
+                tags.split(',')
+                    .map(|tag| {
+                        let key_val: Vec<_> = tag.split(':').collect();
+                        (key_val[0].to_string(), key_val[1].into())
+                    })
+                    .collect()
+            })
     }
 
     pub async fn extractor(
@@ -101,21 +100,27 @@ impl RumExtraData {
         user_agent_hashmap.extend(tags);
         {
             let headers = req.headers();
-            let conn_info = req.connection_info().clone();
-            let ip_address = match headers.contains_key("X-Forwarded-For")
-                || headers.contains_key("Forwarded")
-            {
-                true => conn_info.realip_remote_addr().unwrap(),
-                false => conn_info.peer_addr().unwrap(),
+            // Borrow checker can't reliably tell that RefCell has been dropped
+            // before an await, even if `drop` is directly used
+            // This little contraption helps avoid clippy issues
+            let ip_address = {
+                let conn_info = req.connection_info();
+                match headers.contains_key("X-Forwarded-For") || headers.contains_key("Forwarded") {
+                    true => conn_info.realip_remote_addr(),
+                    false => conn_info.peer_addr(),
+                }
+                .unwrap()
+                .to_string()
             };
 
-            user_agent_hashmap.insert("ip".into(), ip_address.into());
-            let ip = match parse_ip_addr(ip_address) {
+            let ip = match parse_ip_addr(&ip_address) {
                 Ok((ip, _)) => ip,
                 // Default to ipv4 loopback address
                 Err(_) => IpAddr::V4(std::net::Ipv4Addr::new(127, 0, 0, 1)),
             };
 
+            user_agent_hashmap.insert("ip".into(), ip_address.into());
+
             let maxminddb_client = MAXMIND_DB_CLIENT.read().await;
             let geo_info = if let Some(client) = maxminddb_client.as_ref() {
                 if let Ok(city_info) = client.city_reader.lookup::<maxminddb::geoip2::City>(ip) {
@@ -151,11 +156,10 @@ impl RumExtraData {
             let user_agent = req
                 .headers()
                 .get("User-Agent")
-                .map(|v| v.to_str().unwrap_or(""))
+                .and_then(|v| v.to_str().ok())
                 .unwrap_or_default();
 
-            let ua_parser = UA_PARSER.clone();
-            let parsed_user_agent = ua_parser.parse(user_agent);
+            let parsed_user_agent = (*UA_PARSER).parse(user_agent);
 
             user_agent_hashmap.insert(
                 "user_agent".into(),
diff --git a/src/common/meta/organization.rs b/src/common/meta/organization.rs
index aefebb75f8..2d681c8545 100644
--- a/src/common/meta/organization.rs
+++ b/src/common/meta/organization.rs
@@ -361,3 +361,127 @@ impl ClusterInfoResponse {
         region_entry.clusters.insert(cluster_name, cluster_info);
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use config::meta::cluster::Node;
+
+    use super::*;
+
+    #[test]
+    fn test_organization_setting_defaults() {
+        let setting = OrganizationSetting::default();
+        assert_eq!(setting.trace_id_field_name, "trace_id");
+        assert_eq!(setting.span_id_field_name, "span_id");
+        assert!(!setting.toggle_ingestion_logs);
+        assert!(!setting.enable_streaming_search);
+    }
+
+    #[test]
+    fn test_node_list_response_add_node() {
+        let node = Node {
+            name: "node-1".into(),
+            ..Default::default()
+        };
+
+        let mut response = NodeListResponse::new();
+        response.add_node(node.clone(), "us-east".into(), "cluster-a".into());
+
+        let nodes = &response
+            .regions
+            .get("us-east")
+            .unwrap()
+            .clusters
+            .get("cluster-a")
+            .unwrap();
+        assert_eq!(nodes.len(), 1);
+        assert_eq!(nodes[0].name, "node-1");
+    }
+
+    #[test]
+    fn test_node_list_response_add_nodes() {
+        let node1 = Node {
+            name: "node-1".into(),
+            ..Default::default()
+        };
+        let node2 = Node {
+            name: "node-2".into(),
+            ..Default::default()
+        };
+
+        let mut response = NodeListResponse::new();
+        response.add_nodes(vec![
+            (node1.clone(), "us-west".into(), "cluster-x".into()),
+            (node2.clone(), "us-west".into(), "cluster-x".into()),
+        ]);
+
+        let nodes = &response
+            .regions
+            .get("us-west")
+            .unwrap()
+            .clusters
+            .get("cluster-x")
+            .unwrap();
+        assert_eq!(nodes.len(), 2);
+    }
+
+    #[test]
+    fn test_cluster_info_response_add() {
+        let mut response = ClusterInfoResponse::default();
+        let info = ClusterInfo { pending_jobs: 5 };
+
+        response.add_cluster_info(info.clone(), "cluster-1".into(), "eu".into());
+
+        let cluster = response
+            .regions
+            .get("eu")
+            .unwrap()
+            .clusters
+            .get("cluster-1")
+            .unwrap();
+        assert_eq!(cluster.pending_jobs, 5);
+    }
+
+    #[cfg(feature = "cloud")]
+    #[test]
+    fn test_organization_invite_response_data_serialization() {
+        let data = OrganizationInviteResponseData {
+            valid_members: Some(vec!["user1@example.com".into(), "user2@example.com".into()]),
+            existing_members: Some(vec!["existing@example.com".into()]),
+            invalid_email: Some("bad-email".into()),
+        };
+
+        let json = serde_json::to_string(&data).unwrap();
+        assert!(json.contains("user1@example.com"));
+        assert!(json.contains("bad-email"));
+    }
+
+    #[cfg(feature = "cloud")]
+    #[test]
+    fn test_organization_invites_creation() {
+        let invites = OrganizationInvites {
+            invites: vec!["a@example.com".into(), "b@example.com".into()],
+            role: UserRole::Member,
+        };
+
+        assert_eq!(invites.invites.len(), 2);
+        assert_eq!(invites.role, UserRole::Member);
+    }
+
+    #[cfg(feature = "cloud")]
+    #[test]
+    fn test_organization_invite_user_record() {
+        let record = OrganizationInviteUserRecord {
+            email: "user@example.com".into(),
+            first_name: "First".into(),
+            last_name: "Last".into(),
+            role: "admin".into(),
+            status: InviteStatus::Pending,
+            expires_at: 999999,
+            is_external: true,
+        };
+
+        assert_eq!(record.status, InviteStatus::Pending);
+        assert!(record.is_external);
+    }
+}
diff --git a/src/common/meta/proxy.rs b/src/common/meta/proxy.rs
index 87d57b4e22..340d9097ba 100644
--- a/src/common/meta/proxy.rs
+++ b/src/common/meta/proxy.rs
@@ -1,3 +1,18 @@
+// Copyright 2025 OpenObserve Inc.
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
 use serde::{Deserialize, Serialize};
 use utoipa::ToSchema;
 
diff --git a/src/common/meta/search.rs b/src/common/meta/search.rs
index 08edddd4aa..1eb59a5fba 100644
--- a/src/common/meta/search.rs
+++ b/src/common/meta/search.rs
@@ -259,6 +259,7 @@ mod tests {
 #[cfg(test)]
 #[cfg(feature = "enterprise")]
 mod enterprise_tests {
+    use arrow::array::record_batch;
     use arrow_schema::{DataType, Field};
     #[cfg(feature = "enterprise")]
     use o2_enterprise::enterprise::common::streaming_agg_cache::{
@@ -267,37 +268,14 @@ mod enterprise_tests {
 
     #[test]
     fn test_calculate_record_batches_deltas() {
-        use std::sync::Arc;
-
-        use arrow::{
-            array::{Int64Array, StringArray},
-            datatypes::Schema,
-        };
-
-        // Create a simple schema for testing
-        let schema = Arc::new(Schema::new(vec![
-            Field::new("status", DataType::Utf8, false),
-            Field::new("count", DataType::Int64, false),
-        ]));
-
-        // Create test record batches
-        let batch1 = arrow::array::RecordBatch::try_new(
-            schema.clone(),
-            vec![
-                Arc::new(StringArray::from(vec!["200", "404"])),
-                Arc::new(Int64Array::from(vec![100, 50])),
-            ],
+        let batch1 = record_batch!(
+            ("status", Utf8, ["200", "404"]),
+            ("count", Int64, [100, 50])
         )
         .unwrap();
 
-        let batch2 = arrow::array::RecordBatch::try_new(
-            schema.clone(),
-            vec![
-                Arc::new(StringArray::from(vec!["200", "500"])),
-                Arc::new(Int64Array::from(vec![80, 20])),
-            ],
-        )
-        .unwrap();
+        let batch2 =
+            record_batch!(("status", Utf8, ["200", "500"]), ("count", Int64, [80, 20])).unwrap();
 
         // Test case: Query: 10:00 - 16:00, Cache: 11:00 - 12:00, 14:00 - 15:00
         // Expected Deltas: 10:00 - 11:00, 12:00 - 14:00, 15:00 - 16:00
diff --git a/src/common/meta/service.rs b/src/common/meta/service.rs
index d8da05d65b..6e62bdef87 100644
--- a/src/common/meta/service.rs
+++ b/src/common/meta/service.rs
@@ -19,6 +19,8 @@ pub const TRACES: &str = "traces";
 
 #[cfg(test)]
 mod tests {
+    use std::collections::HashSet;
+
     use super::*;
 
     #[test]
@@ -31,13 +33,6 @@ mod tests {
     #[test]
     fn test_service_constants_are_unique() {
         let services = [LOGS, METRICS, TRACES];
-        for i in 0..services.len() {
-            for j in (i + 1)..services.len() {
-                assert_ne!(
-                    services[i], services[j],
-                    "Service constants should be unique"
-                );
-            }
-        }
+        assert!(HashSet::from(services).len() == services.len());
     }
 }
diff --git a/src/common/meta/stream.rs b/src/common/meta/stream.rs
index 07ad474762..33263f30c4 100644
--- a/src/common/meta/stream.rs
+++ b/src/common/meta/stream.rs
@@ -234,7 +234,12 @@ mod tests {
             total_fields: 1,
         };
 
-        assert!(stream.uds_schema.is_some());
-        assert_eq!(stream.uds_schema.unwrap().len(), 1);
+        assert!(stream.uds_schema.is_some_and(|schema| schema.len() == 1));
+    }
+
+    #[test]
+    fn test_empty_stream_delete_fields_default() {
+        let delete_fields = StreamDeleteFields::default();
+        assert!(delete_fields.fields.is_empty());
     }
 }
diff --git a/src/common/meta/syslog.rs b/src/common/meta/syslog.rs
index fd22ce0dee..78cbcd1810 100644
--- a/src/common/meta/syslog.rs
+++ b/src/common/meta/syslog.rs
@@ -157,4 +157,14 @@ mod tests {
         assert!(serialized.contains("orgId"));
         assert!(serialized.contains("streamName"));
     }
+
+    #[test]
+    fn test_empty_syslog_routes() {
+        let routes = SyslogRoutes { routes: vec![] };
+        assert!(routes.routes.is_empty());
+
+        let serialized = serde_json::to_string(&routes).unwrap();
+        let deserialized: SyslogRoutes = serde_json::from_str(&serialized).unwrap();
+        assert!(deserialized.routes.is_empty());
+    }
 }
diff --git a/src/common/meta/telemetry.rs b/src/common/meta/telemetry.rs
index 833cf109fb..78a62dddc4 100644
--- a/src/common/meta/telemetry.rs
+++ b/src/common/meta/telemetry.rs
@@ -305,4 +305,12 @@ mod test_telemetry {
         add_zo_info(&mut props).await;
         assert!(!tel.base_info.is_empty())
     }
+
+    #[tokio::test]
+    async fn test_telemetry_send_track_event_without_base_info_or_zo_data() {
+        let mut telemetry = Telemetry::new();
+        telemetry
+            .send_track_event("test_event", None, false, false)
+            .await;
+    }
 }
diff --git a/src/config/src/utils/arrow.rs b/src/config/src/utils/arrow.rs
index d6ae712c85..9e2e88002c 100644
--- a/src/config/src/utils/arrow.rs
+++ b/src/config/src/utils/arrow.rs
@@ -59,7 +59,7 @@ pub fn record_batches_to_json_rows(
 #[cfg(test)]
 mod tests {
     use arrow::{
-        array::{Int32Array, StringArray, UInt64Array},
+        array::{Int32Array, StringArray, record_batch},
         datatypes::{DataType, Field, Schema},
         record_batch::RecordBatch,
     };
@@ -67,23 +67,10 @@ mod tests {
     use super::*;
 
     fn create_test_batch() -> RecordBatch {
-        let schema = Schema::new(vec![
-            Field::new("id", DataType::Int32, false),
-            Field::new("name", DataType::Utf8, false),
-            Field::new("value", DataType::UInt64, false),
-        ]);
-
-        let id_array = Int32Array::from(vec![1, 2, 3]);
-        let name_array = StringArray::from(vec!["Alice", "Bob", "Charlie"]);
-        let value_array = UInt64Array::from(vec![100, 200, 300]);
-
-        RecordBatch::try_new(
-            schema.into(),
-            vec![
-                std::sync::Arc::new(id_array),
-                std::sync::Arc::new(name_array),
-                std::sync::Arc::new(value_array),
-            ],
+        record_batch!(
+            ("id", Int32, [1, 2, 3]),
+            ("name", Utf8, ["Alice", "Bob", "Charlie"]),
+            ("value", UInt64, [100, 200, 300])
         )
         .unwrap()
     }
diff --git a/src/config/src/utils/sysinfo/cgroup.rs b/src/config/src/utils/sysinfo/cgroup.rs
index ee5522f121..c16766b7cb 100644
--- a/src/config/src/utils/sysinfo/cgroup.rs
+++ b/src/config/src/utils/sysinfo/cgroup.rs
@@ -53,8 +53,10 @@ fn read_cpu_cgroup_v1() -> usize {
         && !val.is_empty()
         && !val.to_lowercase().starts_with("max")
     {
-        let columns = val.split(' ').collect::<Vec<&str>>();
-        return columns[0].parse::<usize>().unwrap_or_default();
+        return val
+            .split_once(' ')
+            .and_then(|(v, _)| v.parse::<usize>().ok())
+            .unwrap_or_default();
     };
     0
 }
diff --git a/src/service/search/cache/cacher.rs b/src/service/search/cache/cacher.rs
index 14d75de06f..61389fd405 100644
--- a/src/service/search/cache/cacher.rs
+++ b/src/service/search/cache/cacher.rs
@@ -62,16 +62,13 @@ pub async fn invalidate_cached_response_by_stream_min_ts(
 
     let filtered_responses = responses
         .iter()
+        .filter(|meta| meta.response_end_time >= stream_min_ts)
         .cloned()
-        .filter_map(|mut meta| {
-            if meta.response_end_time >= stream_min_ts {
-                if meta.response_start_time < stream_min_ts {
-                    meta.response_start_time = stream_min_ts;
-                }
-                Some(meta) // Keep the entry after updating
-            } else {
-                None // Remove the entry
+        .map(|mut meta| {
+            if meta.response_start_time < stream_min_ts {
+                meta.response_start_time = stream_min_ts;
             }
+            meta
         })
         .collect();
 
@@ -726,10 +723,10 @@ fn handle_histogram(
             .map(|v| v.trim().trim_matches(|v| (v == '\'' || v == '"')))
             .collect::<Vec<&str>>();
 
-        attrs.get(1).map_or_else(
-            || generate_histogram_interval(q_time_range),
-            |v| v.to_string(),
-        )
+        attrs
+            .get(1)
+            .map_or_else(|| generate_histogram_interval(q_time_range), |v| *v)
+            .to_string()
     };
 
     *origin_sql = origin_sql.replace(
diff --git a/src/service/search/datafusion/optimizer/rewrite_histogram.rs b/src/service/search/datafusion/optimizer/rewrite_histogram.rs
index fa06e9bf24..b06cc1a55d 100644
--- a/src/service/search/datafusion/optimizer/rewrite_histogram.rs
+++ b/src/service/search/datafusion/optimizer/rewrite_histogram.rs
@@ -129,6 +129,7 @@ impl TreeNodeRewriter for HistogramToDatebin {
                             format!("{} second", self.histogram_interval)
                         } else {
                             generate_histogram_interval(Some((self.start_time, self.end_time)))
+                                .to_string()
                         };
                         cast(
                             Expr::Literal(ScalarValue::from(interval)),
diff --git a/src/service/search/sql.rs b/src/service/search/sql.rs
index 026e09cac8..553cd30dd8 100644
--- a/src/service/search/sql.rs
+++ b/src/service/search/sql.rs
@@ -1492,7 +1492,7 @@ impl VisitorMut for HistogramIntervalVisitor {
 
     fn pre_visit_expr(&mut self, expr: &mut Expr) -> ControlFlow<Self::Break> {
         if let Expr::Function(func) = expr
-            && func.name.to_string().to_lowercase() == "histogram"
+            && func.name.to_string().eq_ignore_ascii_case("histogram")
         {
             if let FunctionArguments::List(list) = &func.args {
                 let mut args = list.args.iter();
@@ -1505,7 +1505,7 @@ impl VisitorMut for HistogramIntervalVisitor {
                         .trim_matches(|v| v == '\'' || v == '"')
                         .to_string()
                 } else {
-                    generate_histogram_interval(self.time_range)
+                    generate_histogram_interval(self.time_range).to_string()
                 };
                 self.interval = match convert_histogram_interval_to_seconds(&interval) {
                     Ok(v) => Some(v),
@@ -1643,12 +1643,12 @@ impl VisitorMut for TrackTotalHitsVisitor {
 
 fn generate_table_reference(idents: &[Ident]) -> (TableReference, String) {
     if idents.len() == 2 {
-        let table_name = idents[0].value.clone();
+        let table_name = idents[0].value.as_str();
         let field_name = idents[1].value.clone();
         (TableReference::from(table_name), field_name)
     } else {
-        let stream_type = idents[0].value.clone();
-        let table_name = idents[1].value.clone();
+        let stream_type = idents[0].value.as_str();
+        let table_name = idents[1].value.as_str();
         let field_name = idents[2].value.clone();
         (TableReference::partial(stream_type, table_name), field_name)
     }
@@ -1717,11 +1717,14 @@ macro_rules! intervals {
     (@unit m) => { chrono::Duration::minutes };
 }
 
-pub fn generate_histogram_interval(time_range: Option<(i64, i64)>) -> String {
-    if time_range.is_none() || time_range.unwrap().eq(&(0, 0)) {
-        return "1 hour".to_string();
+pub fn generate_histogram_interval(time_range: Option<(i64, i64)>) -> &'static str {
+    let Some((start, end)) = time_range else {
+        return "1 hour";
+    };
+    if (start, end).eq(&(0, 0)) {
+        return "1 hour";
     }
-    let duration = time_range.map(|r| r.1 - r.0).unwrap();
+    let duration = end - start;
 
     const INTERVALS: [(i64, &str); 10] = intervals![
         (h, 24 * 60, "1 day"),
@@ -1738,35 +1741,39 @@ pub fn generate_histogram_interval(time_range: Option<(i64, i64)>) -> String {
 
     for (time, interval) in INTERVALS.iter() {
         if duration >= *time {
-            return interval.to_string();
+            return interval;
         }
     }
-    "10 second".to_string()
+    "10 second"
 }
 
 pub fn convert_histogram_interval_to_seconds(interval: &str) -> Result<i64, Error> {
     let interval = interval.trim();
-    let (num, unit) = interval
+    let pos = interval
         .find(|c: char| !c.is_numeric())
-        .map(|pos| interval.split_at(pos))
         .ok_or_else(|| Error::Message(format!("Invalid interval format: '{interval}'")))?;
 
-    let seconds = match unit.trim().to_lowercase().as_str() {
-        "second" | "seconds" | "s" | "secs" | "sec" => num.parse::<i64>(),
-        "minute" | "minutes" | "m" | "mins" | "min" => num.parse::<i64>().map(|n| n * 60),
-        "hour" | "hours" | "h" | "hrs" | "hr" => num.parse::<i64>().map(|n| n * 3600),
-        "day" | "days" | "d" => num.parse::<i64>().map(|n| n * 86400),
+    let (num_str, unit_str) = interval.split_at(pos);
+    let num = num_str.parse::<i64>().map_err(|_| {
+        Error::ErrorCode(ErrorCodes::SearchSQLNotValid(
+            "Invalid number format".to_string(),
+        ))
+    })?;
+
+    let unit = unit_str.trim().to_lowercase();
+    let seconds = match unit.as_str() {
+        "second" | "seconds" | "s" | "secs" | "sec" => num,
+        "minute" | "minutes" | "m" | "mins" | "min" => num * 60,
+        "hour" | "hours" | "h" | "hrs" | "hr" => num * 3600,
+        "day" | "days" | "d" => num * 86400,
         _ => {
             return Err(Error::ErrorCode(ErrorCodes::SearchSQLNotValid(
                 "Unsupported histogram interval unit".to_string(),
             )));
         }
     };
-    seconds.map_err(|_| {
-        Error::ErrorCode(ErrorCodes::SearchSQLNotValid(
-            "Invalid number format".to_string(),
-        ))
-    })
+
+    Ok(seconds)
 }
 
 pub fn pickup_where(sql: &str, meta: Option<MetaSql>) -> Result<Option<String>, Error> {
@@ -1786,17 +1793,28 @@ pub fn pickup_where(sql: &str, meta: Option<MetaSql>) -> Result<Option<String>,
         return Ok(None);
     };
     let mut where_str = caps.get(1).unwrap().as_str().to_string();
-    if !meta.group_by.is_empty() {
-        where_str = where_str[0..where_str.to_lowercase().rfind(" group ").unwrap()].to_string();
+    let where_str_lower = where_str.to_lowercase();
+
+    let clause_to_strip = if !meta.group_by.is_empty() {
+        Some(" group ")
     } else if meta.having {
-        where_str = where_str[0..where_str.to_lowercase().rfind(" having ").unwrap()].to_string();
+        Some(" having ")
     } else if !meta.order_by.is_empty() {
-        where_str = where_str[0..where_str.to_lowercase().rfind(" order ").unwrap()].to_string();
-    } else if meta.limit > 0 || where_str.to_lowercase().ends_with(" limit 0") {
-        where_str = where_str[0..where_str.to_lowercase().rfind(" limit ").unwrap()].to_string();
+        Some(" order ")
+    } else if meta.limit > 0 || where_str_lower.ends_with(" limit 0") {
+        Some(" limit ")
     } else if meta.offset > 0 {
-        where_str = where_str[0..where_str.to_lowercase().rfind(" offset ").unwrap()].to_string();
+        Some(" offset ")
+    } else {
+        None
+    };
+
+    if let Some(clause) = clause_to_strip
+        && let Some(pos) = where_str_lower.rfind(clause)
+    {
+        where_str.truncate(pos);
     }
+
     Ok(Some(where_str))
 }
 
diff --git a/src/service/tantivy/mod.rs b/src/service/tantivy/mod.rs
index 0c2836a9e3..da7eadb2db 100644
--- a/src/service/tantivy/mod.rs
+++ b/src/service/tantivy/mod.rs
@@ -320,7 +320,7 @@ mod tests {
         if include_fts_field {
             fields.push(Field::new("content", DataType::Utf8, false));
             let content: Vec<String> = (0..num_rows)
-                .map(|i| format!("Test content number {}", i))
+                .map(|i| format!("Test content number {i}"))
                 .collect();
             columns.push(Arc::new(StringArray::from(content)));
         }
diff --git a/src/service/tantivy/puffin/writer.rs b/src/service/tantivy/puffin/writer.rs
index b5e9b00232..77830828bb 100644
--- a/src/service/tantivy/puffin/writer.rs
+++ b/src/service/tantivy/puffin/writer.rs
@@ -442,8 +442,8 @@ mod tests {
         ];
 
         for (i, blob_type) in blob_types.iter().enumerate() {
-            let blob_data = format!("blob data {}", i).into_bytes();
-            let blob_tag = format!("blob_{}", i);
+            let blob_data = format!("blob data {i}").into_bytes();
+            let blob_tag = format!("blob_{i}");
 
             writer
                 .add_blob(&blob_data, blob_type.clone(), blob_tag.clone())
@@ -456,7 +456,7 @@ mod tests {
             assert_eq!(writer.blobs_metadata[i].blob_type, *expected_type);
             assert_eq!(
                 writer.blobs_metadata[i].properties.get("blob_tag"),
-                Some(&format!("blob_{}", i))
+                Some(&format!("blob_{i}"))
             );
         }
 
diff --git a/src/service/tantivy/puffin_directory/caching_directory.rs b/src/service/tantivy/puffin_directory/caching_directory.rs
index dca8e71639..2216d8350f 100644
--- a/src/service/tantivy/puffin_directory/caching_directory.rs
+++ b/src/service/tantivy/puffin_directory/caching_directory.rs
@@ -495,7 +495,7 @@ mod tests {
         let caching_directory = CachingDirectory::new(ram_directory);
 
         // Should be able to debug print
-        let debug_str = format!("{:?}", caching_directory);
+        let debug_str = format!("{caching_directory:?}");
         assert!(debug_str.contains("CachingDirectory"));
     }
 
@@ -509,7 +509,7 @@ mod tests {
         let handle = caching_directory.get_file_handle(test_path)?;
 
         // Should be able to debug print
-        let debug_str = format!("{:?}", handle);
+        let debug_str = format!("{handle:?}");
         assert!(debug_str.contains("CachingFileHandle"));
         assert!(debug_str.contains("debug_test"));
 
diff --git a/src/service/tantivy/puffin_directory/footer_cache.rs b/src/service/tantivy/puffin_directory/footer_cache.rs
index 7183180a03..8e440d48a4 100644
--- a/src/service/tantivy/puffin_directory/footer_cache.rs
+++ b/src/service/tantivy/puffin_directory/footer_cache.rs
@@ -603,9 +603,9 @@ mod tests {
         for i in 0..10 {
             let cache_clone = cache.clone();
             let handle = thread::spawn(move || {
-                let path = PathBuf::from(format!("file_{}.term", i));
+                let path = PathBuf::from(format!("file_{i}.term"));
                 let range = 0..10;
-                let data = OwnedBytes::new(format!("data_{:04}", i).into_bytes());
+                let data = OwnedBytes::new(format!("data_{i:04}").into_bytes());
 
                 cache_clone.put_slice(path.clone(), range.clone(), data.clone());
                 let retrieved = cache_clone.get_slice(&path, range);
diff --git a/src/service/tantivy/puffin_directory/mod.rs b/src/service/tantivy/puffin_directory/mod.rs
index 9dc720d179..a233214919 100644
--- a/src/service/tantivy/puffin_directory/mod.rs
+++ b/src/service/tantivy/puffin_directory/mod.rs
@@ -179,7 +179,7 @@ mod tests {
         assert!(result.is_ok());
 
         let data = result.unwrap();
-        assert!(data.len() > 0); // Should have some data
+        assert!(!data.is_empty()); // Should have some data
     }
 
     #[test]
@@ -188,7 +188,7 @@ mod tests {
         assert!(result.is_ok());
 
         let data = result.unwrap();
-        assert!(data.len() > 0); // Should have some data
+        assert!(!data.is_empty()); // Should have some data
     }
 
     #[test]
@@ -197,7 +197,7 @@ mod tests {
         assert!(result.is_ok());
 
         let data = result.unwrap();
-        assert!(data.len() > 0); // Should have some data
+        assert!(!data.is_empty()); // Should have some data
     }
 
     #[test]
@@ -206,7 +206,7 @@ mod tests {
         assert!(result.is_ok());
 
         let data = result.unwrap();
-        assert!(data.len() > 0); // Should have some data
+        assert!(!data.is_empty()); // Should have some data
     }
 
     #[test]
@@ -302,7 +302,7 @@ mod tests {
 
         for ext in test_extensions {
             let result = get_file_from_empty_puffin_dir_with_ext(ext);
-            assert!(result.is_ok(), "Failed to get file with extension: {}", ext);
+            assert!(result.is_ok(), "Failed to get file with extension: {ext}");
 
             let data = result.unwrap();
             // Data should be valid OwnedBytes
@@ -440,7 +440,7 @@ mod tests {
         // Should be able to get files using the utility function
         for ext in ALLOWED_FILE_EXT {
             let data = get_file_from_empty_puffin_dir_with_ext(ext).unwrap();
-            assert!(data.len() > 0);
+            assert!(!data.is_empty());
         }
 
         for ext in EMPTY_FILE_EXT {
diff --git a/src/service/tantivy/puffin_directory/reader_cache.rs b/src/service/tantivy/puffin_directory/reader_cache.rs
index c47d52560e..0a6b22cda6 100644
--- a/src/service/tantivy/puffin_directory/reader_cache.rs
+++ b/src/service/tantivy/puffin_directory/reader_cache.rs
@@ -153,7 +153,7 @@ mod tests {
 
         for i in 0..5 {
             let (index, reader) = create_test_index();
-            let key = format!("key_{}", i);
+            let key = format!("key_{i}");
             cache.put(key.clone(), (index, reader));
         }
 
@@ -161,7 +161,7 @@ mod tests {
 
         // Verify all entries can be retrieved
         for i in 0..5 {
-            let key = format!("key_{}", i);
+            let key = format!("key_{i}");
             assert!(cache.get(&key).is_some());
         }
     }
@@ -173,7 +173,7 @@ mod tests {
         // Fill cache beyond capacity
         for i in 0..15 {
             let (index, reader) = create_test_index();
-            let key = format!("key_{}", i);
+            let key = format!("key_{i}");
             cache.put(key, (index, reader));
         }
 
@@ -190,7 +190,7 @@ mod tests {
         // Fill cache to capacity
         for i in 0..20 {
             let (index, reader) = create_test_index();
-            let key = format!("key_{}", i);
+            let key = format!("key_{i}");
             cache.put(key, (index, reader));
         }
 
@@ -245,7 +245,7 @@ mod tests {
             let cache_clone = cache.clone();
             let handle = thread::spawn(move || {
                 let (index, reader) = create_test_index();
-                let key = format!("concurrent_key_{}", i);
+                let key = format!("concurrent_key_{i}");
 
                 // Put entry
                 cache_clone.put(key.clone(), (index.clone(), reader.clone()));
@@ -278,7 +278,7 @@ mod tests {
             let cache_clone = cache.clone();
             let handle = thread::spawn(move || {
                 let (index, reader) = create_test_index();
-                let key = format!("eviction_key_{}", i);
+                let key = format!("eviction_key_{i}");
                 cache_clone.put(key, (index, reader));
 
                 // Small delay to increase chance of concurrent operations
@@ -318,7 +318,7 @@ mod tests {
         // Add many entries
         for i in 0..200 {
             let (index, reader) = create_test_index();
-            let key = format!("stress_key_{}", i);
+            let key = format!("stress_key_{i}");
             cache.put(key, (index, reader));
         }
 
@@ -393,7 +393,7 @@ mod tests {
         // Add many entries without triggering eviction
         for i in 0..500 {
             let (index, reader) = create_test_index();
-            let key = format!("large_key_{}", i);
+            let key = format!("large_key_{i}");
             cache.put(key, (index, reader));
         }
 
diff --git a/src/service/tantivy/puffin_directory/writer.rs b/src/service/tantivy/puffin_directory/writer.rs
index e0cc180254..82a728fff0 100644
--- a/src/service/tantivy/puffin_directory/writer.rs
+++ b/src/service/tantivy/puffin_directory/writer.rs
@@ -488,8 +488,8 @@ mod tests {
         for i in 0..10 {
             let writer_clone = writer.clone();
             let handle = thread::spawn(move || {
-                let path = format!("file_{}.txt", i);
-                let data = format!("data for file {}", i);
+                let path = format!("file_{i}.txt");
+                let data = format!("data for file {i}");
                 writer_clone
                     .atomic_write(Path::new(&path), data.as_bytes())
                     .unwrap();
@@ -506,9 +506,9 @@ mod tests {
 
         // All files should be readable
         for i in 0..10 {
-            let path = format!("file_{}.txt", i);
+            let path = format!("file_{i}.txt");
             let data = writer.atomic_read(Path::new(&path)).unwrap();
-            let expected = format!("data for file {}", i);
+            let expected = format!("data for file {i}");
             assert_eq!(data, expected.as_bytes());
         }
     }
@@ -556,7 +556,7 @@ mod tests {
         ];
 
         for filename in &special_files {
-            let data = format!("data for {}", filename);
+            let data = format!("data for {filename}");
             writer
                 .atomic_write(Path::new(filename), data.as_bytes())
                 .unwrap();
@@ -568,7 +568,7 @@ mod tests {
         for filename in &special_files {
             assert!(files.contains(&PathBuf::from(filename)));
             let read_data = writer.atomic_read(Path::new(filename)).unwrap();
-            let expected = format!("data for {}", filename);
+            let expected = format!("data for {filename}");
             assert_eq!(read_data, expected.as_bytes());
         }
     }
@@ -698,7 +698,7 @@ mod tests {
         // Test all allowed extensions
         let allowed_extensions = ["term", "idx", "pos", "fast"];
         for ext in &allowed_extensions {
-            let filename = format!("test.{}", ext);
+            let filename = format!("test.{ext}");
             writer.atomic_write(Path::new(&filename), b"data").unwrap();
         }
 
diff --git a/src/service/users.rs b/src/service/users.rs
index 8b45275b86..c45a6efddb 100644
--- a/src/service/users.rs
+++ b/src/service/users.rs
@@ -810,12 +810,14 @@ pub async fn list_users(
         if is_list_all {
             let (org, id) = org_user.key().split_once('/').unwrap();
             if let Some(org_record) = organization::get_org(org).await {
-                let user_org = user_orgs.entry(id.to_string()).or_insert(vec![]);
-                user_org.push(OrgRoleMapping {
-                    org_id: org.to_string(),
-                    role: org_user.value().role.clone(),
-                    org_name: org_record.name,
-                });
+                user_orgs
+                    .entry(id.to_string())
+                    .or_default()
+                    .push(OrgRoleMapping {
+                        org_id: org.to_string(),
+                        role: org_user.value().role.clone(),
+                        org_name: org_record.name,
+                    });
             }
         } else if org_user.key().starts_with(&format!("{org_id}/"))
             && let Some(user) = get_user(Some(org_id), org_user.value().email.as_str()).await
@@ -935,7 +937,7 @@ pub async fn remove_user_from_org(
                 }
 
                 if !user.organizations.is_empty() {
-                    let mut orgs = user.clone().organizations;
+                    let orgs = &mut user.organizations;
                     if orgs.len() == 1 {
                         if orgs[0].role.eq(&UserRole::ServiceAccount) && user.is_external {
                             return Ok(HttpResponse::Forbidden().json(MetaHttpResponse::error(
@@ -978,7 +980,7 @@ pub async fn remove_user_from_org(
                                     ),
                                 ));
                             }
-                            if org.name.eq(&org_id.to_string()) {
+                            if org.name.eq(org_id) {
                                 let user_role = &org.role;
                                 is_service_account = user_role.eq(&UserRole::ServiceAccount);
                                 _user_fga_role =
@@ -989,8 +991,7 @@ pub async fn remove_user_from_org(
                                     };
                             }
                         }
-                        orgs.retain(|x| !x.name.eq(&org_id.to_string()));
-                        user.organizations = orgs;
+                        orgs.retain(|x| !x.name.eq(org_id));
                         let resp = db::org_users::remove(org_id, &email_id).await;
                         // special case as we cache flattened user struct
                         if resp.is_ok() {
@@ -1069,7 +1070,7 @@ pub fn is_user_from_org(orgs: Vec<UserOrg>, org_id: &str) -> (bool, UserOrg) {
         (false, get_default_user_org())
     } else {
         let mut local_orgs = orgs;
-        local_orgs.retain(|org| !org.name.eq(&org_id.to_string()));
+        local_orgs.retain(|org| !org.name.eq(org_id));
         if local_orgs.is_empty() {
             (false, get_default_user_org())
         } else {