Skip to content

Commit 38cb38f

Browse files
pjenveypjenvey
committed
refactor: switch syncstorage Db methods to &mut self
Closes STOR-327
1 parent 614e390 commit 38cb38f

File tree

10 files changed

+166
-153
lines changed

10 files changed

+166
-153
lines changed

‎syncserver-db-common/src/lib.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ macro_rules! sync_db_method {
4343
sync_db_method!($name, $sync_name, $type, results::$type);
4444
};
4545
($name:ident, $sync_name:ident, $type:ident, $result:ty) => {
46-
fn $name(&self, params: params::$type) -> DbFuture<'_, $result, DbError> {
46+
fn $name(&mut self, params: params::$type) -> DbFuture<'_, $result, DbError> {
4747
let db = self.clone();
4848
Box::pin(
4949
self.blocking_threadpool

‎syncserver/src/web/handlers.rs‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ pub async fn get_collections(
4141
state: Data<ServerState>,
4242
) -> Result<HttpResponse, ApiError> {
4343
db_pool
44-
.transaction_http(request.clone(), |db| async move {
44+
.transaction_http(request.clone(), |mut db| async move {
4545
meta.emit_api_metric("request.get_collections");
4646
if state.glean_enabled {
4747
// Values below are be passed to the Glean logic to emit metrics.
@@ -82,7 +82,7 @@ pub async fn get_collection_counts(
8282
request: HttpRequest,
8383
) -> Result<HttpResponse, ApiError> {
8484
db_pool
85-
.transaction_http(request, |db| async move {
85+
.transaction_http(request, |mut db| async move {
8686
meta.emit_api_metric("request.get_collection_counts");
8787
let result = db.get_collection_counts(meta.user_id).await?;
8888

@@ -99,7 +99,7 @@ pub async fn get_collection_usage(
9999
request: HttpRequest,
100100
) -> Result<HttpResponse, ApiError> {
101101
db_pool
102-
.transaction_http(request, |db| async move {
102+
.transaction_http(request, |mut db| async move {
103103
meta.emit_api_metric("request.get_collection_usage");
104104
let usage: HashMap<_, _> = db
105105
.get_collection_usage(meta.user_id)
@@ -121,7 +121,7 @@ pub async fn get_quota(
121121
request: HttpRequest,
122122
) -> Result<HttpResponse, ApiError> {
123123
db_pool
124-
.transaction_http(request, |db| async move {
124+
.transaction_http(request, |mut db| async move {
125125
meta.emit_api_metric("request.get_quota");
126126
let usage = db.get_storage_usage(meta.user_id).await?;
127127
Ok(HttpResponse::Ok().json(vec![Some(usage as f64 / ONE_KB), None]))
@@ -135,7 +135,7 @@ pub async fn delete_all(
135135
request: HttpRequest,
136136
) -> Result<HttpResponse, ApiError> {
137137
db_pool
138-
.transaction_http(request, |db| async move {
138+
.transaction_http(request, |mut db| async move {
139139
meta.emit_api_metric("request.delete_all");
140140
Ok(HttpResponse::Ok().json(db.delete_storage(meta.user_id).await?))
141141
})
@@ -148,7 +148,7 @@ pub async fn delete_collection(
148148
request: HttpRequest,
149149
) -> Result<HttpResponse, ApiError> {
150150
db_pool
151-
.transaction_http(request, |db| async move {
151+
.transaction_http(request, |mut db| async move {
152152
let delete_bsos = !coll.query.ids.is_empty();
153153
let timestamp = if delete_bsos {
154154
coll.emit_api_metric("request.delete_bsos");
@@ -193,7 +193,7 @@ pub async fn get_collection(
193193
request: HttpRequest,
194194
) -> Result<HttpResponse, ApiError> {
195195
db_pool
196-
.transaction_http(request, |db| async move {
196+
.transaction_http(request, |mut db| async move {
197197
coll.emit_api_metric("request.get_collection");
198198
let params = params::GetBsos {
199199
user_id: coll.user_id.clone(),
@@ -221,7 +221,7 @@ pub async fn get_collection(
221221

222222
async fn finish_get_collection<T>(
223223
coll: &CollectionRequest,
224-
db: Box<dyn Db<Error = DbError>>,
224+
mut db: Box<dyn Db<Error = DbError>>,
225225
result: Result<Paginated<T>, DbError>,
226226
) -> Result<HttpResponse, DbError>
227227
where
@@ -275,7 +275,7 @@ pub async fn post_collection(
275275
request: HttpRequest,
276276
) -> Result<HttpResponse, ApiError> {
277277
db_pool
278-
.transaction_http(request, |db| async move {
278+
.transaction_http(request, |mut db| async move {
279279
coll.emit_api_metric("request.post_collection");
280280
trace!("Collection: Post");
281281

@@ -312,7 +312,7 @@ pub async fn post_collection(
312312
// the entire, accumulated if the `commit` flag is set.
313313
pub async fn post_collection_batch(
314314
coll: CollectionPostRequest,
315-
db: Box<dyn Db<Error = DbError>>,
315+
mut db: Box<dyn Db<Error = DbError>>,
316316
) -> Result<HttpResponse, ApiError> {
317317
coll.emit_api_metric("request.post_collection_batch");
318318
trace!("Batch: Post collection batch");
@@ -488,7 +488,7 @@ pub async fn delete_bso(
488488
request: HttpRequest,
489489
) -> Result<HttpResponse, ApiError> {
490490
db_pool
491-
.transaction_http(request, |db| async move {
491+
.transaction_http(request, |mut db| async move {
492492
bso_req.emit_api_metric("request.delete_bso");
493493
let result = db
494494
.delete_bso(params::DeleteBso {
@@ -508,7 +508,7 @@ pub async fn get_bso(
508508
request: HttpRequest,
509509
) -> Result<HttpResponse, ApiError> {
510510
db_pool
511-
.transaction_http(request, |db| async move {
511+
.transaction_http(request, |mut db| async move {
512512
bso_req.emit_api_metric("request.get_bso");
513513
let result = db
514514
.get_bso(params::GetBso {
@@ -532,7 +532,7 @@ pub async fn put_bso(
532532
request: HttpRequest,
533533
) -> Result<HttpResponse, ApiError> {
534534
db_pool
535-
.transaction_http(request, |db| async move {
535+
.transaction_http(request, |mut db| async move {
536536
bso_req.emit_api_metric("request.put_bso");
537537
let result = db
538538
.put_bso(params::PutBso {
@@ -574,7 +574,7 @@ pub async fn heartbeat(hb: HeartbeatRequest) -> Result<HttpResponse, ApiError> {
574574
"version".to_owned(),
575575
Value::String(env!("CARGO_PKG_VERSION").to_owned()),
576576
);
577-
let db = hb.db_pool.get().await?;
577+
let mut db = hb.db_pool.get().await?;
578578

579579
checklist.insert("quota".to_owned(), serde_json::to_value(hb.quota)?);
580580

‎syncserver/src/web/transaction.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,8 @@ impl DbTransactionPool {
5353
F: Future<Output = Result<R, ApiError>>,
5454
{
5555
// Get connection from pool
56-
let db = self.pool.get().await?;
57-
let db2 = db.clone();
56+
let mut db = self.pool.get().await?;
57+
let mut db2 = db.clone();
5858

5959
// Lock for transaction
6060
let result = match (self.get_lock_collection(), self.is_read) {
@@ -98,7 +98,7 @@ impl DbTransactionPool {
9898
A: FnOnce(Box<dyn Db<Error = DbError>>) -> F,
9999
F: Future<Output = Result<R, ApiError>> + 'a,
100100
{
101-
let (resp, db) = self.transaction_internal(request, action).await?;
101+
let (resp, mut db) = self.transaction_internal(request, action).await?;
102102

103103
// No further processing before commit is possible
104104
db.commit().await?;
@@ -117,7 +117,7 @@ impl DbTransactionPool {
117117
F: Future<Output = Result<HttpResponse, ApiError>> + 'a,
118118
{
119119
let mreq = request.clone();
120-
let check_precondition = move |db: Box<dyn Db<Error = DbError>>| {
120+
let check_precondition = move |mut db: Box<dyn Db<Error = DbError>>| {
121121
async move {
122122
// set the extra information for all requests so we capture default err handlers.
123123
set_extra(&mreq, db.get_connection_info());
@@ -168,7 +168,7 @@ impl DbTransactionPool {
168168
}
169169
};
170170

171-
let (resp, db) = self
171+
let (resp, mut db) = self
172172
.transaction_internal(request.clone(), check_precondition)
173173
.await?;
174174
// match on error and return a composed HttpResponse (so we can use the tags?)

‎syncstorage-db-common/src/lib.rs‎

Lines changed: 44 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -67,124 +67,131 @@ impl<E> Clone for Box<dyn DbPool<Error = E>> {
6767
pub trait Db: Debug {
6868
type Error: DbErrorIntrospect + 'static;
6969

70-
fn lock_for_read(&self, params: params::LockCollection) -> DbFuture<'_, (), Self::Error>;
70+
fn lock_for_read(&mut self, params: params::LockCollection) -> DbFuture<'_, (), Self::Error>;
7171

72-
fn lock_for_write(&self, params: params::LockCollection) -> DbFuture<'_, (), Self::Error>;
72+
fn lock_for_write(&mut self, params: params::LockCollection) -> DbFuture<'_, (), Self::Error>;
7373

74-
fn begin(&self, for_write: bool) -> DbFuture<'_, (), Self::Error>;
74+
fn begin(&mut self, for_write: bool) -> DbFuture<'_, (), Self::Error>;
7575

76-
fn commit(&self) -> DbFuture<'_, (), Self::Error>;
76+
fn commit(&mut self) -> DbFuture<'_, (), Self::Error>;
7777

78-
fn rollback(&self) -> DbFuture<'_, (), Self::Error>;
78+
fn rollback(&mut self) -> DbFuture<'_, (), Self::Error>;
7979

8080
fn get_collection_timestamps(
81-
&self,
81+
&mut self,
8282
params: params::GetCollectionTimestamps,
8383
) -> DbFuture<'_, results::GetCollectionTimestamps, Self::Error>;
8484

8585
fn get_collection_timestamp(
86-
&self,
86+
&mut self,
8787
params: params::GetCollectionTimestamp,
8888
) -> DbFuture<'_, results::GetCollectionTimestamp, Self::Error>;
8989

9090
fn get_collection_counts(
91-
&self,
91+
&mut self,
9292
params: params::GetCollectionCounts,
9393
) -> DbFuture<'_, results::GetCollectionCounts, Self::Error>;
9494

9595
fn get_collection_usage(
96-
&self,
96+
&mut self,
9797
params: params::GetCollectionUsage,
9898
) -> DbFuture<'_, results::GetCollectionUsage, Self::Error>;
9999

100100
fn get_storage_timestamp(
101-
&self,
101+
&mut self,
102102
params: params::GetStorageTimestamp,
103103
) -> DbFuture<'_, results::GetStorageTimestamp, Self::Error>;
104104

105105
fn get_storage_usage(
106-
&self,
106+
&mut self,
107107
params: params::GetStorageUsage,
108108
) -> DbFuture<'_, results::GetStorageUsage, Self::Error>;
109109

110110
fn get_quota_usage(
111-
&self,
111+
&mut self,
112112
params: params::GetQuotaUsage,
113113
) -> DbFuture<'_, results::GetQuotaUsage, Self::Error>;
114114

115115
fn delete_storage(
116-
&self,
116+
&mut self,
117117
params: params::DeleteStorage,
118118
) -> DbFuture<'_, results::DeleteStorage, Self::Error>;
119119

120120
fn delete_collection(
121-
&self,
121+
&mut self,
122122
params: params::DeleteCollection,
123123
) -> DbFuture<'_, results::DeleteCollection, Self::Error>;
124124

125125
fn delete_bsos(
126-
&self,
126+
&mut self,
127127
params: params::DeleteBsos,
128128
) -> DbFuture<'_, results::DeleteBsos, Self::Error>;
129129

130-
fn get_bsos(&self, params: params::GetBsos) -> DbFuture<'_, results::GetBsos, Self::Error>;
130+
fn get_bsos(&mut self, params: params::GetBsos) -> DbFuture<'_, results::GetBsos, Self::Error>;
131131

132-
fn get_bso_ids(&self, params: params::GetBsos)
133-
-> DbFuture<'_, results::GetBsoIds, Self::Error>;
132+
fn get_bso_ids(
133+
&mut self,
134+
params: params::GetBsos,
135+
) -> DbFuture<'_, results::GetBsoIds, Self::Error>;
134136

135-
fn post_bsos(&self, params: params::PostBsos) -> DbFuture<'_, results::PostBsos, Self::Error>;
137+
fn post_bsos(
138+
&mut self,
139+
params: params::PostBsos,
140+
) -> DbFuture<'_, results::PostBsos, Self::Error>;
136141

137142
fn delete_bso(
138-
&self,
143+
&mut self,
139144
params: params::DeleteBso,
140145
) -> DbFuture<'_, results::DeleteBso, Self::Error>;
141146

142-
fn get_bso(&self, params: params::GetBso)
143-
-> DbFuture<'_, Option<results::GetBso>, Self::Error>;
147+
fn get_bso(
148+
&mut self,
149+
params: params::GetBso,
150+
) -> DbFuture<'_, Option<results::GetBso>, Self::Error>;
144151

145152
fn get_bso_timestamp(
146-
&self,
153+
&mut self,
147154
params: params::GetBsoTimestamp,
148155
) -> DbFuture<'_, results::GetBsoTimestamp, Self::Error>;
149156

150-
fn put_bso(&self, params: params::PutBso) -> DbFuture<'_, results::PutBso, Self::Error>;
157+
fn put_bso(&mut self, params: params::PutBso) -> DbFuture<'_, results::PutBso, Self::Error>;
151158

152159
fn create_batch(
153-
&self,
160+
&mut self,
154161
params: params::CreateBatch,
155162
) -> DbFuture<'_, results::CreateBatch, Self::Error>;
156163

157164
fn validate_batch(
158-
&self,
165+
&mut self,
159166
params: params::ValidateBatch,
160167
) -> DbFuture<'_, results::ValidateBatch, Self::Error>;
161168

162169
fn append_to_batch(
163-
&self,
170+
&mut self,
164171
params: params::AppendToBatch,
165172
) -> DbFuture<'_, results::AppendToBatch, Self::Error>;
166173

167174
fn get_batch(
168-
&self,
175+
&mut self,
169176
params: params::GetBatch,
170177
) -> DbFuture<'_, Option<results::GetBatch>, Self::Error>;
171178

172179
fn commit_batch(
173-
&self,
180+
&mut self,
174181
params: params::CommitBatch,
175182
) -> DbFuture<'_, results::CommitBatch, Self::Error>;
176183

177184
fn box_clone(&self) -> Box<dyn Db<Error = Self::Error>>;
178185

179-
fn check(&self) -> DbFuture<'_, results::Check, Self::Error>;
186+
fn check(&mut self) -> DbFuture<'_, results::Check, Self::Error>;
180187

181188
fn get_connection_info(&self) -> results::ConnectionInfo;
182189

183190
/// Retrieve the timestamp for an item/collection
184191
///
185192
/// Modeled on the Python `get_resource_timestamp` function.
186193
fn extract_resource(
187-
&self,
194+
&mut self,
188195
user_id: UserIdentifier,
189196
collection: Option<String>,
190197
bso: Option<String>,
@@ -230,22 +237,22 @@ pub trait Db: Debug {
230237

231238
// Internal methods used by the db tests
232239

233-
fn get_collection_id(&self, name: String) -> DbFuture<'_, i32, Self::Error>;
240+
fn get_collection_id(&mut self, name: String) -> DbFuture<'_, i32, Self::Error>;
234241

235-
fn create_collection(&self, name: String) -> DbFuture<'_, i32, Self::Error>;
242+
fn create_collection(&mut self, name: String) -> DbFuture<'_, i32, Self::Error>;
236243

237244
fn update_collection(
238-
&self,
245+
&mut self,
239246
params: params::UpdateCollection,
240247
) -> DbFuture<'_, SyncTimestamp, Self::Error>;
241248

242249
fn timestamp(&self) -> SyncTimestamp;
243250

244-
fn set_timestamp(&self, timestamp: SyncTimestamp);
251+
fn set_timestamp(&mut self, timestamp: SyncTimestamp);
245252

246-
fn delete_batch(&self, params: params::DeleteBatch) -> DbFuture<'_, (), Self::Error>;
253+
fn delete_batch(&mut self, params: params::DeleteBatch) -> DbFuture<'_, (), Self::Error>;
247254

248-
fn clear_coll_cache(&self) -> DbFuture<'_, (), Self::Error>;
255+
fn clear_coll_cache(&mut self) -> DbFuture<'_, (), Self::Error>;
249256

250257
fn set_quota(&mut self, enabled: bool, limit: usize, enforce: bool);
251258
}

0 commit comments

Comments
 (0)