Skip to content

Commit 2ce4570

Browse files
ethowitzethowitz
authored
refactor: Add ToSpannerValue trait (#1046)
Closes mozilla-services#260
1 parent 57bd30a commit 2ce4570

File tree

4 files changed

+124
-70
lines changed

4 files changed

+124
-70
lines changed

‎src/db/spanner/batch.rs

+27-18
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,8 @@ use protobuf::{
1010
};
1111
use uuid::Uuid;
1212

13-
use super::support::{null_value, struct_type_field};
14-
use super::{
15-
models::{Result, SpannerDb, DEFAULT_BSO_TTL, PRETOUCH_TS},
16-
support::{as_list_value, as_value},
17-
};
13+
use super::models::{Result, SpannerDb, DEFAULT_BSO_TTL, PRETOUCH_TS};
14+
use super::support::{null_value, struct_type_field, ToSpannerValue};
1815
use crate::{
1916
db::{params, results, util::to_rfc3339, DbError, DbErrorKind, BATCH_LIFETIME},
2017
web::{extractors::HawkIdentifier, tags::Tags},
@@ -298,7 +295,10 @@ pub async fn do_append_async(
298295
"collection_id" => collection_id.to_string(),
299296
"batch_id" => batch.id.clone(),
300297
};
301-
params.insert("ids".to_owned(), as_list_value(bso_ids));
298+
params.insert(
299+
"ids".to_owned(),
300+
bso_ids.collect::<Vec<String>>().to_spanner_value(),
301+
);
302302
let mut existing_stream = db
303303
.sql(
304304
"SELECT batch_bso_id
@@ -355,23 +355,29 @@ pub async fn do_append_async(
355355
} else {
356356
let sortindex = bso
357357
.sortindex
358-
.map(|sortindex| as_value(sortindex.to_string()))
358+
.as_ref()
359+
.map(ToSpannerValue::to_spanner_value)
360+
.unwrap_or_else(null_value);
361+
let payload = bso
362+
.payload
363+
.as_ref()
364+
.map(ToSpannerValue::to_spanner_value)
359365
.unwrap_or_else(null_value);
360-
let payload = bso.payload.map(as_value).unwrap_or_else(null_value);
361366
let ttl = bso
362367
.ttl
363-
.map(|ttl| as_value(ttl.to_string()))
368+
.as_ref()
369+
.map(ToSpannerValue::to_spanner_value)
364370
.unwrap_or_else(null_value);
365371

366372
// convert to a protobuf structure for direct insertion to
367373
// avoid some mutation limits.
368374
let mut row = ListValue::new();
369375
row.set_values(RepeatedField::from_vec(vec![
370-
as_value(user_id.fxa_uid.clone()),
371-
as_value(user_id.fxa_kid.clone()),
372-
as_value(collection_id.to_string()),
373-
as_value(batch.id.clone()),
374-
as_value(bso.id),
376+
user_id.fxa_uid.clone().to_spanner_value(),
377+
user_id.fxa_kid.clone().to_spanner_value(),
378+
collection_id.to_spanner_value(),
379+
batch.id.clone().to_spanner_value(),
380+
bso.id.to_spanner_value(),
375381
sortindex,
376382
payload,
377383
ttl,
@@ -480,15 +486,15 @@ pub async fn do_append_async(
480486
};
481487
if let Some(sortindex) = val.sortindex {
482488
fields.push("sortindex");
483-
params.insert("sortindex".to_owned(), as_value(sortindex.to_string()));
489+
params.insert("sortindex".to_owned(), sortindex.to_spanner_value());
484490
}
485491
if let Some(payload) = val.payload {
486492
fields.push("payload");
487-
params.insert("payload".to_owned(), as_value(payload));
493+
params.insert("payload".to_owned(), payload.to_spanner_value());
488494
};
489495
if let Some(ttl) = val.ttl {
490496
fields.push("ttl");
491-
params.insert("ttl".to_owned(), as_value(ttl.to_string()));
497+
params.insert("ttl".to_owned(), ttl.to_spanner_value());
492498
}
493499
if fields.is_empty() {
494500
continue;
@@ -545,7 +551,10 @@ async fn pretouch_collection_async(
545551
.one_or_none()
546552
.await?;
547553
if result.is_none() {
548-
sqlparams.insert("modified".to_owned(), as_value(PRETOUCH_TS.to_owned()));
554+
sqlparams.insert(
555+
"modified".to_owned(),
556+
PRETOUCH_TS.to_owned().to_spanner_value(),
557+
);
549558
let sql = if db.quota.enabled {
550559
"INSERT INTO user_collections (fxa_uid, fxa_kid, collection_id, modified, count, total_bytes)
551560
VALUES (@fxa_uid, @fxa_kid, @collection_id, @modified, 0, 0)"

‎src/db/spanner/macros.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ macro_rules! params {
88
let _cap = params!(@count $($key),*);
99
let mut _map = ::std::collections::HashMap::with_capacity(_cap);
1010
$(
11-
_map.insert($key.to_owned(), as_value($value));
11+
_map.insert($key.to_owned(), ToSpannerValue::to_spanner_value(&$value));
1212
)*
1313
_map
1414
}

‎src/db/spanner/models.rs

+38-18
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,8 @@ use super::{
4343
batch,
4444
pool::{CollectionCache, Conn},
4545
support::{
46-
as_list_value, as_type, as_value, bso_from_row, bso_to_insert_row, bso_to_update_row,
47-
ExecuteSqlRequestBuilder, StreamedResultSetAsync,
46+
as_type, bso_from_row, bso_to_insert_row, bso_to_update_row, ExecuteSqlRequestBuilder,
47+
StreamedResultSetAsync, ToSpannerValue,
4848
},
4949
};
5050

@@ -639,7 +639,11 @@ impl SpannerDb {
639639
let mut params = HashMap::new();
640640
params.insert(
641641
"ids".to_owned(),
642-
as_list_value(uncached.into_iter().map(|id| id.to_string())),
642+
uncached
643+
.into_iter()
644+
.map(|id| id.to_string())
645+
.collect::<Vec<String>>()
646+
.to_spanner_value(),
643647
);
644648
let mut rs = self
645649
.sql(
@@ -892,9 +896,12 @@ impl SpannerDb {
892896
if self.quota.enabled {
893897
sqlparams.insert(
894898
"total_bytes".to_owned(),
895-
as_value(result[0].take_string_value()),
899+
result[0].take_string_value().to_spanner_value(),
900+
);
901+
sqlparams.insert(
902+
"count".to_owned(),
903+
result[1].take_string_value().to_spanner_value(),
896904
);
897-
sqlparams.insert("count".to_owned(), as_value(result[1].take_string_value()));
898905
sqltypes.insert(
899906
"total_bytes".to_owned(),
900907
crate::db::spanner::support::as_type(TypeCode::INT64),
@@ -1175,7 +1182,7 @@ impl SpannerDb {
11751182
"fxa_kid" => user_id.fxa_kid,
11761183
"collection_id" => collection_id.to_string(),
11771184
};
1178-
sqlparams.insert("ids".to_owned(), as_list_value(params.ids.into_iter()));
1185+
sqlparams.insert("ids".to_owned(), params.ids.to_spanner_value());
11791186
self.sql(
11801187
"DELETE FROM bsos
11811188
WHERE fxa_uid = @fxa_uid
@@ -1220,7 +1227,7 @@ impl SpannerDb {
12201227

12211228
if !ids.is_empty() {
12221229
query = format!("{} AND bso_id IN UNNEST(@ids)", query);
1223-
sqlparams.insert("ids".to_owned(), as_list_value(ids.into_iter()));
1230+
sqlparams.insert("ids".to_owned(), ids.to_spanner_value());
12241231
}
12251232

12261233
// issue559: Dead code (timestamp always None)
@@ -1243,12 +1250,12 @@ impl SpannerDb {
12431250
*/
12441251
if let Some(older) = older {
12451252
query = format!("{} AND modified < @older", query);
1246-
sqlparams.insert("older".to_string(), as_value(older.as_rfc3339()?));
1253+
sqlparams.insert("older".to_string(), older.as_rfc3339()?.to_spanner_value());
12471254
sqltypes.insert("older".to_string(), as_type(TypeCode::TIMESTAMP));
12481255
}
12491256
if let Some(newer) = newer {
12501257
query = format!("{} AND modified > @newer", query);
1251-
sqlparams.insert("newer".to_string(), as_value(newer.as_rfc3339()?));
1258+
sqlparams.insert("newer".to_string(), newer.as_rfc3339()?.to_spanner_value());
12521259
sqltypes.insert("newer".to_string(), as_type(TypeCode::TIMESTAMP));
12531260
}
12541261
query = match sort {
@@ -1527,7 +1534,12 @@ impl SpannerDb {
15271534
};
15281535
sqlparams.insert(
15291536
"ids".to_owned(),
1530-
as_list_value(params.bsos.iter().map(|pbso| pbso.id.clone())),
1537+
params
1538+
.bsos
1539+
.iter()
1540+
.map(|pbso| pbso.id.clone())
1541+
.collect::<Vec<String>>()
1542+
.to_spanner_value(),
15311543
);
15321544
let mut streaming = self
15331545
.sql(
@@ -1700,7 +1712,7 @@ impl SpannerDb {
17001712
"{}{}",
17011713
q,
17021714
if let Some(sortindex) = bso.sortindex {
1703-
sqlparams.insert("sortindex".to_string(), as_value(sortindex.to_string()));
1715+
sqlparams.insert("sortindex".to_string(), sortindex.to_spanner_value());
17041716
sqltypes.insert("sortindex".to_string(), as_type(TypeCode::INT64));
17051717

17061718
format!("{}{}", comma(&q), "sortindex = @sortindex")
@@ -1714,7 +1726,7 @@ impl SpannerDb {
17141726
q,
17151727
if let Some(ttl) = bso.ttl {
17161728
let expiry = timestamp.as_i64() + (i64::from(ttl) * 1000);
1717-
sqlparams.insert("expiry".to_string(), as_value(to_rfc3339(expiry)?));
1729+
sqlparams.insert("expiry".to_string(), to_rfc3339(expiry)?.to_spanner_value());
17181730
sqltypes.insert("expiry".to_string(), as_type(TypeCode::TIMESTAMP));
17191731
format!("{}{}", comma(&q), "expiry = @expiry")
17201732
} else {
@@ -1726,7 +1738,10 @@ impl SpannerDb {
17261738
"{}{}",
17271739
q,
17281740
if bso.payload.is_some() || bso.sortindex.is_some() {
1729-
sqlparams.insert("modified".to_string(), as_value(timestamp.as_rfc3339()?));
1741+
sqlparams.insert(
1742+
"modified".to_string(),
1743+
timestamp.as_rfc3339()?.to_spanner_value(),
1744+
);
17301745
sqltypes.insert("modified".to_string(), as_type(TypeCode::TIMESTAMP));
17311746
format!("{}{}", comma(&q), "modified = @modified")
17321747
} else {
@@ -1738,7 +1753,7 @@ impl SpannerDb {
17381753
"{}{}",
17391754
q,
17401755
if let Some(payload) = bso.payload {
1741-
sqlparams.insert("payload".to_string(), as_value(payload));
1756+
sqlparams.insert("payload".to_string(), payload.to_spanner_value());
17421757
format!("{}{}", comma(&q), "payload = @payload")
17431758
} else {
17441759
"".to_string()
@@ -1782,14 +1797,16 @@ impl SpannerDb {
17821797
use super::support::null_value;
17831798
let sortindex = bso
17841799
.sortindex
1785-
.map(|sortindex| as_value(sortindex.to_string()))
1800+
.map(|sortindex| sortindex.to_spanner_value())
17861801
.unwrap_or_else(null_value);
17871802
sqlparams.insert("sortindex".to_string(), sortindex);
17881803
sqltypes.insert("sortindex".to_string(), as_type(TypeCode::INT64));
17891804
}
17901805
sqlparams.insert(
17911806
"payload".to_string(),
1792-
as_value(bso.payload.unwrap_or_else(|| "".to_owned())),
1807+
bso.payload
1808+
.unwrap_or_else(|| "".to_owned())
1809+
.to_spanner_value(),
17931810
);
17941811
let now_millis = timestamp.as_i64();
17951812
let ttl = bso.ttl.map_or(i64::from(DEFAULT_BSO_TTL), |ttl| {
@@ -1801,10 +1818,13 @@ impl SpannerDb {
18011818
"!!!!! [test] INSERT expirystring:{:?}, timestamp:{:?}, ttl:{:?}",
18021819
&expirystring, timestamp, ttl
18031820
);
1804-
sqlparams.insert("expiry".to_string(), as_value(expirystring));
1821+
sqlparams.insert("expiry".to_string(), expirystring.to_spanner_value());
18051822
sqltypes.insert("expiry".to_string(), as_type(TypeCode::TIMESTAMP));
18061823

1807-
sqlparams.insert("modified".to_string(), as_value(timestamp.as_rfc3339()?));
1824+
sqlparams.insert(
1825+
"modified".to_string(),
1826+
timestamp.as_rfc3339()?.to_spanner_value(),
1827+
);
18081828
sqltypes.insert("modified".to_string(), as_type(TypeCode::TIMESTAMP));
18091829
sql.to_owned()
18101830
};

0 commit comments

Comments
 (0)