From 53b6f03f66ce702173247765d242ea16427199cf Mon Sep 17 00:00:00 2001 From: JR Conlin Date: Wed, 29 Nov 2023 12:27:53 -0800 Subject: [PATCH] bug: remove unneeded clone operation (#521) * bug: remove unneeded clone operation Closes SYNC-3836 --- autoendpoint/src/settings.rs | 8 ++--- .../src/db/bigtable/bigtable_client/merge.rs | 2 +- .../src/db/bigtable/bigtable_client/mod.rs | 34 +++++++++---------- .../src/db/bigtable/bigtable_client/row.rs | 11 +++--- 4 files changed, 27 insertions(+), 28 deletions(-) diff --git a/autoendpoint/src/settings.rs b/autoendpoint/src/settings.rs index 412a7676e..8cd59d91a 100644 --- a/autoendpoint/src/settings.rs +++ b/autoendpoint/src/settings.rs @@ -56,10 +56,10 @@ impl Default for Settings { db_settings: "".to_owned(), router_table_name: "router".to_string(), message_table_name: "message".to_string(), - /// max data is a bit hard to figure out, due to encryption. Using something - /// like pywebpush, if you encode a block of 4096 bytes, you'll get a - /// 4216 byte data block. Since we're going to be receiving this, we have to - /// presume base64 encoding, so we can bump things up to 5630 bytes max. + // max data is a bit hard to figure out, due to encryption. Using something + // like pywebpush, if you encode a block of 4096 bytes, you'll get a + // 4216 byte data block. Since we're going to be receiving this, we have to + // presume base64 encoding, so we can bump things up to 5630 bytes max. max_data_bytes: 5630, crypto_keys: format!("[{}]", Fernet::generate_key()), auth_keys: r#"["AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAB="]"#.to_string(), diff --git a/autopush-common/src/db/bigtable/bigtable_client/merge.rs b/autopush-common/src/db/bigtable/bigtable_client/merge.rs index 72762415a..2e70d891c 100644 --- a/autopush-common/src/db/bigtable/bigtable_client/merge.rs +++ b/autopush-common/src/db/bigtable/bigtable_client/merge.rs @@ -437,7 +437,7 @@ impl RowMerger { // Check to see if we can add this row, or if it's blocked by the timestamp filter. let finished_row = merger.row_complete(&mut chunk)?; if let Some(timestamp) = timestamp_filter { - if let Some(sk_ts) = finished_row.clone().get_cell("sortkey_timestamp") { + if let Some(sk_ts) = finished_row.clone().take_cell("sortkey_timestamp") { let ts_val = crate::db::bigtable::bigtable_client::to_u64( sk_ts.value, "sortkey_timestamp", diff --git a/autopush-common/src/db/bigtable/bigtable_client/mod.rs b/autopush-common/src/db/bigtable/bigtable_client/mod.rs index 743d1fff6..204234802 100644 --- a/autopush-common/src/db/bigtable/bigtable_client/mod.rs +++ b/autopush-common/src/db/bigtable/bigtable_client/mod.rs @@ -321,7 +321,7 @@ impl BigTableClientImpl { } } // get the dominant family type for this row. - if let Some(cell) = row.get_cell("channel_id") { + if let Some(cell) = row.take_cell("channel_id") { let mut notif = Notification { channel_id: Uuid::from_str(&to_string(cell.value, "channel_id")?).map_err( |e| { @@ -333,31 +333,31 @@ impl BigTableClientImpl { )?, ..Default::default() }; - if let Some(cell) = row.get_cell("version") { + if let Some(cell) = row.take_cell("version") { notif.version = to_string(cell.value, "version")?; } - if let Some(cell) = row.get_cell("topic") { + if let Some(cell) = row.take_cell("topic") { notif.topic = Some(to_string(cell.value, "topic")?); } - if let Some(cell) = row.get_cell("ttl") { + if let Some(cell) = row.take_cell("ttl") { notif.ttl = to_u64(cell.value, "ttl")?; } - if let Some(cell) = row.get_cell("data") { + if let Some(cell) = row.take_cell("data") { notif.data = Some(to_string(cell.value, "data")?); } - if let Some(cell) = row.get_cell("sortkey_timestamp") { + if let Some(cell) = row.take_cell("sortkey_timestamp") { let sk_ts = to_u64(cell.value, "sortkey_timestamp")?; notif.sortkey_timestamp = Some(sk_ts); if sk_ts > max_timestamp { max_timestamp = sk_ts; } } - if let Some(cell) = row.get_cell("timestamp") { + if let Some(cell) = row.take_cell("timestamp") { notif.timestamp = to_u64(cell.value, "timestamp")?; } - if let Some(cell) = row.get_cell("headers") { + if let Some(cell) = row.take_cell("headers") { notif.headers = Some( serde_json::from_str::>(&to_string( cell.value, "headers", @@ -505,9 +505,9 @@ impl DbClient for BigTableClientImpl { ..Default::default() }; - if let Some(record) = self.read_row(&key, None).await? { + if let Some(mut record) = self.read_row(&key, None).await? { trace!("🉑 Found a record for that user"); - if let Some(mut cells) = record.get_cells("connected_at") { + if let Some(mut cells) = record.take_cells("connected_at") { if let Some(cell) = cells.pop() { let v: [u8; 8] = cell.value.try_into().map_err(|e| { DbError::Serialization(format!( @@ -519,7 +519,7 @@ impl DbClient for BigTableClientImpl { } } - if let Some(mut cells) = record.get_cells("router_type") { + if let Some(mut cells) = record.take_cells("router_type") { if let Some(cell) = cells.pop() { result.router_type = String::from_utf8(cell.value).map_err(|e| { DbError::Serialization(format!( @@ -530,7 +530,7 @@ impl DbClient for BigTableClientImpl { } } - if let Some(mut cells) = record.get_cells("router_data") { + if let Some(mut cells) = record.take_cells("router_data") { if let Some(cell) = cells.pop() { result.router_data = from_str(&String::from_utf8(cell.value).map_err(|e| { DbError::Serialization(format!( @@ -547,7 +547,7 @@ impl DbClient for BigTableClientImpl { } } - if let Some(mut cells) = record.get_cells("last_connect") { + if let Some(mut cells) = record.take_cells("last_connect") { if let Some(cell) = cells.pop() { let v: [u8; 8] = cell.value.try_into().map_err(|e| { DbError::Serialization(format!( @@ -559,7 +559,7 @@ impl DbClient for BigTableClientImpl { } } - if let Some(mut cells) = record.get_cells("node_id") { + if let Some(mut cells) = record.take_cells("node_id") { if let Some(cell) = cells.pop() { result.node_id = Some(String::from_utf8(cell.value).map_err(|e| { DbError::Serialization(format!( @@ -570,7 +570,7 @@ impl DbClient for BigTableClientImpl { } } - if let Some(mut cells) = record.get_cells("record_version") { + if let Some(mut cells) = record.take_cells("record_version") { if let Some(mut cell) = cells.pop() { // there's only one byte, so pop it off and use it. if let Some(b) = cell.value.pop() { @@ -579,7 +579,7 @@ impl DbClient for BigTableClientImpl { } } - if let Some(mut cells) = record.get_cells("current_month") { + if let Some(mut cells) = record.take_cells("current_month") { if let Some(cell) = cells.pop() { result.current_month = Some(String::from_utf8(cell.value).map_err(|e| { DbError::Serialization(format!( @@ -591,7 +591,7 @@ impl DbClient for BigTableClientImpl { } //TODO: rename this to `last_notification_timestamp` - if let Some(mut cells) = record.get_cells("current_timestamp") { + if let Some(mut cells) = record.take_cells("current_timestamp") { if let Some(cell) = cells.pop() { let v: [u8; 8] = cell.value.try_into().map_err(|e| { DbError::Serialization(format!( diff --git a/autopush-common/src/db/bigtable/bigtable_client/row.rs b/autopush-common/src/db/bigtable/bigtable_client/row.rs index 01c876996..3dd6c63b5 100644 --- a/autopush-common/src/db/bigtable/bigtable_client/row.rs +++ b/autopush-common/src/db/bigtable/bigtable_client/row.rs @@ -15,15 +15,14 @@ pub struct Row { impl Row { /// Return all cells for a given column - pub fn get_cells(&self, column: &str) -> Option> { - self.cells.get(column).cloned() + pub fn take_cells(&mut self, column: &str) -> Option> { + self.cells.remove(column) } /// get only the "top" cell value. Ignore other values. - /// TODO: https://mozilla-hub.atlassian.net/browse/SYNC-3836 - pub fn get_cell(&mut self, column: &str) -> Option { - if let Some(cells) = self.cells.get(column) { - return cells.last().cloned(); + pub fn take_cell(&mut self, column: &str) -> Option { + if let Some(mut cells) = self.cells.remove(column) { + return cells.pop(); } None }