Skip to content

Commit 6d5a3d4

Browse files
patrickfreedpatrickfreed
authored
RUST-882 Stop relying on From<u64> for Bson (#402)
1 parent 25ea495 commit 6d5a3d4

File tree

14 files changed

+85
-64
lines changed

14 files changed

+85
-64
lines changed

‎src/client/executor.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ impl Client {
217217
&self,
218218
op: &mut T,
219219
session: &mut Option<&mut ClientSession>,
220-
txn_number: Option<u64>,
220+
txn_number: Option<i64>,
221221
first_error: Error,
222222
) -> Result<T::O> {
223223
let server = match self.select_server(op.selection_criteria()).await {
@@ -270,7 +270,7 @@ impl Client {
270270
op: &mut T,
271271
connection: &mut Connection,
272272
session: &mut Option<&mut ClientSession>,
273-
txn_number: Option<u64>,
273+
txn_number: Option<i64>,
274274
retryability: &Retryability,
275275
) -> Result<T::O> {
276276
if let Some(wc) = op.write_concern() {
@@ -281,7 +281,7 @@ impl Client {
281281
self.inner
282282
.topology
283283
.update_command_with_read_pref(connection.address(), &mut cmd, op.selection_criteria())
284-
.await;
284+
.await?;
285285

286286
match session {
287287
Some(ref mut session) if op.supports_sessions() && op.is_acknowledged() => {

‎src/client/session/mod.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,7 @@ impl ClientSession {
230230
}
231231

232232
/// Gets the current txn_number.
233-
pub(crate) fn txn_number(&self) -> u64 {
233+
pub(crate) fn txn_number(&self) -> i64 {
234234
self.server_session.txn_number
235235
}
236236

@@ -240,8 +240,8 @@ impl ClientSession {
240240
}
241241

242242
/// Increments the txn_number and returns the new value.
243-
pub(crate) fn get_and_increment_txn_number(&mut self) -> u64 {
244-
self.server_session.txn_number += 1;
243+
pub(crate) fn get_and_increment_txn_number(&mut self) -> i64 {
244+
self.increment_txn_number();
245245
self.server_session.txn_number
246246
}
247247

@@ -556,7 +556,7 @@ pub(crate) struct ServerSession {
556556
dirty: bool,
557557

558558
/// A monotonically increasing transaction number for this session.
559-
txn_number: u64,
559+
txn_number: i64,
560560
}
561561

562562
impl ServerSession {

‎src/cmap/conn/command.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ impl Command {
4141
}
4242
}
4343

44-
pub(crate) fn set_txn_number(&mut self, txn_number: u64) {
44+
pub(crate) fn set_txn_number(&mut self, txn_number: i64) {
4545
self.body.insert("txnNumber", txn_number);
4646
}
4747

@@ -58,9 +58,10 @@ impl Command {
5858
}
5959
}
6060

61-
pub(crate) fn set_read_preference(&mut self, read_preference: ReadPreference) {
61+
pub(crate) fn set_read_preference(&mut self, read_preference: ReadPreference) -> Result<()> {
6262
self.body
63-
.insert("$readPreference", read_preference.into_document());
63+
.insert("$readPreference", read_preference.into_document()?);
64+
Ok(())
6465
}
6566

6667
pub(crate) fn set_start_transaction(&mut self) {

‎src/cmap/establish/test.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,11 @@ async fn speculative_auth_test(
6363
authorized_db_name.into(),
6464
doc! { "find": "foo", "limit": 1 },
6565
);
66-
command.set_read_preference(ReadPreference::PrimaryPreferred {
67-
options: Default::default(),
68-
});
66+
command
67+
.set_read_preference(ReadPreference::PrimaryPreferred {
68+
options: Default::default(),
69+
})
70+
.unwrap();
6971

7072
let response = conn.send_command(command, None).await.unwrap();
7173
let doc_response = response.into_document_response().unwrap();

‎src/cmap/test/integration.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ async fn acquire_connection_and_send_command() {
4949
options: Default::default(),
5050
};
5151
let mut cmd = Command::new("listDatabases".to_string(), "admin".to_string(), body);
52-
cmd.set_read_preference(read_pref);
52+
cmd.set_read_preference(read_pref).unwrap();
5353
if let Some(server_api) = client_options.server_api.as_ref() {
5454
cmd.set_server_api(server_api);
5555
}

‎src/coll/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,7 @@ impl<T> Collection<T> {
269269
resolve_read_concern_with_session!(self, options, session.as_ref())?;
270270
resolve_selection_criteria_with_session!(self, options, session.as_ref())?;
271271

272-
let op = CountDocuments::new(self.namespace(), filter.into(), options);
272+
let op = CountDocuments::new(self.namespace(), filter.into(), options)?;
273273
self.client().execute_operation(op, session).await
274274
}
275275

‎src/operation/count/test.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ async fn handle_success() {
8181
let count_op = Count::empty();
8282

8383
let n = 26;
84-
let response = doc! { "ok": 1.0, "n": n };
84+
let response = doc! { "ok": 1.0, "n": n as i32 };
8585

8686
let actual_values = handle_response_test(&count_op, response).unwrap();
8787
assert_eq!(actual_values, n);
@@ -98,7 +98,7 @@ async fn handle_success_agg() {
9898
"cursor": {
9999
"id": 0,
100100
"ns": "a.b",
101-
"firstBatch": [ { "n": n } ]
101+
"firstBatch": [ { "n": n as i32 } ]
102102
}
103103
};
104104

‎src/operation/count_documents/mod.rs

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
#[cfg(test)]
22
mod test;
33

4+
use std::convert::TryInto;
5+
46
use bson::{doc, Document};
57

68
use super::{CursorBody, CursorResponse, Operation, Retryability};
79
use crate::{
810
bson_util,
911
cmap::{Command, StreamDescription},
10-
error::{ErrorKind, Result},
12+
error::{Error, ErrorKind, Result},
1113
operation::aggregate::Aggregate,
1214
options::{AggregateOptions, CountOptions},
1315
selection_criteria::SelectionCriteria,
@@ -23,20 +25,30 @@ impl CountDocuments {
2325
namespace: Namespace,
2426
filter: Option<Document>,
2527
options: Option<CountOptions>,
26-
) -> Self {
28+
) -> Result<Self> {
2729
let mut pipeline = vec![doc! {
2830
"$match": filter.unwrap_or_default(),
2931
}];
3032

3133
if let Some(skip) = options.as_ref().and_then(|opts| opts.skip) {
34+
let s: i64 = skip.try_into().map_err(|_| {
35+
Error::from(ErrorKind::InvalidArgument {
36+
message: format!("skip exceeds range of i64: {}", skip),
37+
})
38+
})?;
3239
pipeline.push(doc! {
33-
"$skip": skip
40+
"$skip": s
3441
});
3542
}
3643

3744
if let Some(limit) = options.as_ref().and_then(|opts| opts.limit) {
45+
let l: i64 = limit.try_into().map_err(|_| {
46+
Error::from(ErrorKind::InvalidArgument {
47+
message: format!("limit exceeds range of i64: {}", limit),
48+
})
49+
})?;
3850
pipeline.push(doc! {
39-
"$limit": limit
51+
"$limit": l
4052
});
4153
}
4254

@@ -57,9 +69,9 @@ impl CountDocuments {
5769
.build()
5870
});
5971

60-
Self {
72+
Ok(Self {
6173
aggregate: Aggregate::new(namespace, pipeline, aggregate_options),
62-
}
74+
})
6375
}
6476
}
6577

‎src/operation/count_documents/test.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ async fn build() {
2020
db: "test_db".to_string(),
2121
coll: "test_coll".to_string(),
2222
};
23-
let mut count_op = CountDocuments::new(ns, Some(doc! { "x": 1 }), None);
23+
let mut count_op = CountDocuments::new(ns, Some(doc! { "x": 1 }), None).unwrap();
2424
let mut count_command = count_op
2525
.build(&StreamDescription::new_testing())
2626
.expect("error on build");
@@ -56,7 +56,7 @@ async fn build_with_options() {
5656
db: "test_db".to_string(),
5757
coll: "test_coll".to_string(),
5858
};
59-
let mut count_op = CountDocuments::new(ns, None, Some(options));
59+
let mut count_op = CountDocuments::new(ns, None, Some(options)).unwrap();
6060
let mut count_command = count_op
6161
.build(&StreamDescription::new_testing())
6262
.expect("error on build");
@@ -65,8 +65,8 @@ async fn build_with_options() {
6565
"aggregate": "test_coll",
6666
"pipeline": [
6767
{ "$match": {} },
68-
{ "$skip": skip },
69-
{ "$limit": limit },
68+
{ "$skip": skip as i64 },
69+
{ "$limit": limit as i64 },
7070
{ "$group": { "_id": 1, "n": { "$sum": 1 } } },
7171
],
7272
"hint": "_id_1",
@@ -89,7 +89,7 @@ async fn op_selection_criteria() {
8989
selection_criteria,
9090
..Default::default()
9191
};
92-
CountDocuments::new(Namespace::empty(), None, Some(options))
92+
CountDocuments::new(Namespace::empty(), None, Some(options)).unwrap()
9393
});
9494
}
9595

@@ -100,15 +100,15 @@ async fn handle_success() {
100100
db: "test_db".to_string(),
101101
coll: "test_coll".to_string(),
102102
};
103-
let count_op = CountDocuments::new(ns, None, None);
103+
let count_op = CountDocuments::new(ns, None, None).unwrap();
104104

105105
let n = 26;
106106
let response = doc! {
107107
"ok": 1.0,
108108
"cursor": {
109109
"id": 0,
110110
"ns": "test_db.test_coll",
111-
"firstBatch": [ { "_id": 1, "n": n } ],
111+
"firstBatch": [ { "_id": 1, "n": n as i32 } ],
112112
}
113113
};
114114

‎src/sdam/description/topology/mod.rs

Lines changed: 16 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -158,13 +158,13 @@ impl TopologyDescription {
158158
server_type: ServerType,
159159
command: &mut Command,
160160
criteria: Option<&SelectionCriteria>,
161-
) {
161+
) -> crate::error::Result<()> {
162162
match (self.topology_type, server_type) {
163163
(TopologyType::Sharded, ServerType::Mongos)
164164
| (TopologyType::Single, ServerType::Mongos) => {
165-
self.update_command_read_pref_for_mongos(command, criteria);
165+
self.update_command_read_pref_for_mongos(command, criteria)
166166
}
167-
(TopologyType::Single, ServerType::Standalone) => {}
167+
(TopologyType::Single, ServerType::Standalone) => Ok(()),
168168
(TopologyType::Single, _) => {
169169
let specified_read_pref = criteria
170170
.and_then(SelectionCriteria::as_read_pref)
@@ -177,7 +177,7 @@ impl TopologyDescription {
177177
Some(other) => other,
178178
};
179179

180-
command.set_read_preference(resolved_read_pref);
180+
command.set_read_preference(resolved_read_pref)
181181
}
182182
_ => {
183183
let read_pref = match criteria {
@@ -187,7 +187,7 @@ impl TopologyDescription {
187187
},
188188
None => ReadPreference::Primary,
189189
};
190-
command.set_read_preference(read_pref);
190+
command.set_read_preference(read_pref)
191191
}
192192
}
193193
}
@@ -196,33 +196,30 @@ impl TopologyDescription {
196196
&self,
197197
command: &mut Command,
198198
criteria: Option<&SelectionCriteria>,
199-
) {
199+
) -> crate::error::Result<()> {
200200
match criteria {
201201
Some(SelectionCriteria::ReadPreference(ReadPreference::Secondary { ref options })) => {
202202
command.set_read_preference(ReadPreference::Secondary {
203203
options: options.clone(),
204-
});
204+
})
205205
}
206206
Some(SelectionCriteria::ReadPreference(ReadPreference::PrimaryPreferred {
207207
ref options,
208-
})) => {
209-
command.set_read_preference(ReadPreference::PrimaryPreferred {
210-
options: options.clone(),
211-
});
212-
}
208+
})) => command.set_read_preference(ReadPreference::PrimaryPreferred {
209+
options: options.clone(),
210+
}),
213211
Some(SelectionCriteria::ReadPreference(ReadPreference::SecondaryPreferred {
214212
ref options,
215-
})) if options.max_staleness.is_some() || options.tag_sets.is_some() => {
216-
command.set_read_preference(ReadPreference::SecondaryPreferred {
213+
})) if options.max_staleness.is_some() || options.tag_sets.is_some() => command
214+
.set_read_preference(ReadPreference::SecondaryPreferred {
217215
options: options.clone(),
218-
});
219-
}
216+
}),
220217
Some(SelectionCriteria::ReadPreference(ReadPreference::Nearest { ref options })) => {
221218
command.set_read_preference(ReadPreference::Nearest {
222219
options: options.clone(),
223-
});
220+
})
224221
}
225-
_ => {}
222+
_ => Ok(()),
226223
}
227224
}
228225

@@ -777,7 +774,7 @@ fn verify_max_staleness(max_staleness: Option<Duration>) -> crate::error::Result
777774
.map_err(|s| crate::error::ErrorKind::InvalidArgument { message: s }.into())
778775
}
779776

780-
fn verify_max_staleness_inner(max_staleness: Option<Duration>) -> Result<(), String> {
777+
fn verify_max_staleness_inner(max_staleness: Option<Duration>) -> std::result::Result<(), String> {
781778
if max_staleness
782779
.map(|staleness| staleness > Duration::from_secs(0) && staleness < Duration::from_secs(90))
783780
.unwrap_or(false)

0 commit comments

Comments
 (0)