Skip to content

Commit 9c74e2f

Browse files
wprzytulawprzytula
authored
Merge pull request #345 from wprzytula/minor-refactors
A bunch of minor refactors - type safety, readability, robustness
2 parents b5670c1 + e5df40b commit 9c74e2f

File tree

14 files changed

+243
-234
lines changed

14 files changed

+243
-234
lines changed

‎scylla-rust-wrapper/src/api.rs

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -717,6 +717,7 @@ pub mod error {
717717
CassError,
718718
CassErrorResult,
719719
CassErrorSource,
720+
cass_error_desc,
720721
cass_error_num_arg_types,
721722
cass_error_result_arg_type,
722723
cass_error_result_code,
@@ -731,12 +732,6 @@ pub mod error {
731732
cass_error_result_table,
732733
cass_error_result_write_type,
733734
};
734-
735-
// Disabling rustfmt to have one item per line for better readability.
736-
#[rustfmt::skip]
737-
pub use crate::external::{
738-
cass_error_desc,
739-
};
740735
}
741736

742737
pub mod iterator {

‎scylla-rust-wrapper/src/batch.rs

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -33,19 +33,20 @@ pub(crate) struct CassBatchState {
3333

3434
#[unsafe(no_mangle)]
3535
pub unsafe extern "C" fn cass_batch_new(
36-
type_: CassBatchType,
36+
typ: CassBatchType,
3737
) -> CassOwnedExclusivePtr<CassBatch, CMut> {
38-
if let Some(batch_type) = make_batch_type(type_) {
39-
BoxFFI::into_ptr(Box::new(CassBatch {
40-
state: Arc::new(CassBatchState {
41-
batch: Batch::new(batch_type),
42-
bound_values: Vec::new(),
43-
}),
44-
exec_profile: None,
45-
}))
46-
} else {
47-
BoxFFI::null_mut()
48-
}
38+
let Ok(batch_type) = make_batch_type(typ) else {
39+
tracing::error!("Provided invalid batch type to cass_batch_new: {typ:?}");
40+
return BoxFFI::null_mut();
41+
};
42+
43+
BoxFFI::into_ptr(Box::new(CassBatch {
44+
state: Arc::new(CassBatchState {
45+
batch: Batch::new(batch_type),
46+
bound_values: Vec::new(),
47+
}),
48+
exec_profile: None,
49+
}))
4950
}
5051

5152
#[unsafe(no_mangle)]

‎scylla-rust-wrapper/src/binding.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -319,10 +319,7 @@ macro_rules! invoke_binder_maker_macro_with_type {
319319
$consume_v,
320320
$fn,
321321
|p: CassBorrowedSharedPtr<crate::collection::CassCollection, CConst>| {
322-
match std::convert::TryInto::try_into(BoxFFI::as_ref(p).unwrap()) {
323-
Ok(v) => Ok(Some(v)),
324-
Err(_) => Err(CassError::CASS_ERROR_LIB_INVALID_VALUE_TYPE),
325-
}
322+
Ok(Some(std::convert::Into::into(BoxFFI::as_ref(p).unwrap())))
326323
},
327324
[p @ CassBorrowedSharedPtr<crate::collection::CassCollection, CConst>]
328325
);

‎scylla-rust-wrapper/src/cass_types.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -898,11 +898,11 @@ pub unsafe extern "C" fn cass_data_type_add_sub_value_type_by_name_n(
898898
}
899899
}
900900

901-
pub(crate) fn make_batch_type(type_: CassBatchType) -> Option<BatchType> {
901+
pub(crate) fn make_batch_type(type_: CassBatchType) -> Result<BatchType, ()> {
902902
match type_ {
903-
CassBatchType::CASS_BATCH_TYPE_LOGGED => Some(BatchType::Logged),
904-
CassBatchType::CASS_BATCH_TYPE_UNLOGGED => Some(BatchType::Unlogged),
905-
CassBatchType::CASS_BATCH_TYPE_COUNTER => Some(BatchType::Counter),
906-
_ => None,
903+
CassBatchType::CASS_BATCH_TYPE_LOGGED => Ok(BatchType::Logged),
904+
CassBatchType::CASS_BATCH_TYPE_UNLOGGED => Ok(BatchType::Unlogged),
905+
CassBatchType::CASS_BATCH_TYPE_COUNTER => Ok(BatchType::Counter),
906+
_ => Err(()),
907907
}
908908
}

‎scylla-rust-wrapper/src/collection.rs

Lines changed: 54 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,41 @@ static UNTYPED_MAP_TYPE: LazyLock<Arc<CassDataType>> = LazyLock::new(|| {
2828
})
2929
});
3030

31+
/// Introduced to bring type safety to the driver internals,
32+
/// which is not possible with the C enum `CassCollectionType`.
33+
#[derive(Clone, Copy)]
34+
pub(crate) enum CollectionType {
35+
List,
36+
Set,
37+
Map,
38+
}
39+
40+
impl TryFrom<CassCollectionType> for CollectionType {
41+
type Error = ();
42+
43+
fn try_from(value: CassCollectionType) -> Result<Self, Self::Error> {
44+
match value {
45+
CassCollectionType::CASS_COLLECTION_TYPE_LIST => Ok(CollectionType::List),
46+
CassCollectionType::CASS_COLLECTION_TYPE_SET => Ok(CollectionType::Set),
47+
CassCollectionType::CASS_COLLECTION_TYPE_MAP => Ok(CollectionType::Map),
48+
_ => Err(()),
49+
}
50+
}
51+
}
52+
53+
impl From<CollectionType> for CassCollectionType {
54+
fn from(value: CollectionType) -> Self {
55+
match value {
56+
CollectionType::List => CassCollectionType::CASS_COLLECTION_TYPE_LIST,
57+
CollectionType::Set => CassCollectionType::CASS_COLLECTION_TYPE_SET,
58+
CollectionType::Map => CassCollectionType::CASS_COLLECTION_TYPE_MAP,
59+
}
60+
}
61+
}
62+
3163
#[derive(Clone)]
3264
pub struct CassCollection {
33-
pub(crate) collection_type: CassCollectionType,
65+
pub(crate) collection_type: CollectionType,
3466
pub(crate) data_type: Option<Arc<CassDataType>>,
3567
pub(crate) items: Vec<CassCqlValue>,
3668
}
@@ -45,7 +77,7 @@ impl CassCollection {
4577
let index = self.items.len();
4678

4779
// Do validation only if it's a typed collection.
48-
if let Some(data_type) = &self
80+
if let Some(data_type) = self
4981
.data_type
5082
.as_ref()
5183
.map(|dt| unsafe { dt.get_unchecked() })
@@ -100,17 +132,16 @@ impl CassCollection {
100132
}
101133
}
102134

103-
impl TryFrom<&CassCollection> for CassCqlValue {
104-
type Error = ();
105-
fn try_from(collection: &CassCollection) -> Result<Self, Self::Error> {
135+
impl From<&CassCollection> for CassCqlValue {
136+
fn from(collection: &CassCollection) -> Self {
106137
// FIXME: validate that collection items are correct
107138
let data_type = collection.data_type.clone();
108139
match collection.collection_type {
109-
CassCollectionType::CASS_COLLECTION_TYPE_LIST => Ok(CassCqlValue::List {
140+
CollectionType::List => CassCqlValue::List {
110141
data_type,
111142
values: collection.items.clone(),
112-
}),
113-
CassCollectionType::CASS_COLLECTION_TYPE_MAP => {
143+
},
144+
CollectionType::Map => {
114145
let mut grouped_items = Vec::new();
115146
// FIXME: validate even number of items
116147
for i in (0..collection.items.len()).step_by(2) {
@@ -120,16 +151,15 @@ impl TryFrom<&CassCollection> for CassCqlValue {
120151
grouped_items.push((key, value));
121152
}
122153

123-
Ok(CassCqlValue::Map {
154+
CassCqlValue::Map {
124155
data_type,
125156
values: grouped_items,
126-
})
157+
}
127158
}
128-
CassCollectionType::CASS_COLLECTION_TYPE_SET => Ok(CassCqlValue::Set {
159+
CollectionType::Set => CassCqlValue::Set {
129160
data_type,
130161
values: collection.items.clone(),
131-
}),
132-
_ => Err(()),
162+
},
133163
}
134164
}
135165
}
@@ -146,6 +176,11 @@ pub unsafe extern "C" fn cass_collection_new(
146176
_ => item_count,
147177
} as usize;
148178

179+
let Ok(collection_type) = CollectionType::try_from(collection_type) else {
180+
tracing::error!("Provided invalid CassCollectionType to cass_collection_new!");
181+
return BoxFFI::null_mut();
182+
};
183+
149184
BoxFFI::into_ptr(Box::new(CassCollection {
150185
collection_type,
151186
data_type: None,
@@ -164,15 +199,11 @@ pub unsafe extern "C" fn cass_collection_new_from_data_type(
164199
};
165200

166201
let (capacity, collection_type) = match unsafe { data_type.get_unchecked() } {
167-
CassDataTypeInner::List { .. } => {
168-
(item_count, CassCollectionType::CASS_COLLECTION_TYPE_LIST)
169-
}
170-
CassDataTypeInner::Set { .. } => (item_count, CassCollectionType::CASS_COLLECTION_TYPE_SET),
202+
CassDataTypeInner::List { .. } => (item_count, CollectionType::List),
203+
CassDataTypeInner::Set { .. } => (item_count, CollectionType::Set),
171204
// Maps consist of a key and a value, so twice
172205
// the number of CassCqlValue will be stored.
173-
CassDataTypeInner::Map { .. } => {
174-
(item_count * 2, CassCollectionType::CASS_COLLECTION_TYPE_MAP)
175-
}
206+
CassDataTypeInner::Map { .. } => (item_count * 2, CollectionType::Map),
176207
_ => return BoxFFI::null_mut(),
177208
};
178209
let capacity = capacity as usize;
@@ -196,14 +227,9 @@ pub unsafe extern "C" fn cass_collection_data_type(
196227
match &collection_ref.data_type {
197228
Some(dt) => ArcFFI::as_ptr(dt),
198229
None => match collection_ref.collection_type {
199-
CassCollectionType::CASS_COLLECTION_TYPE_LIST => ArcFFI::as_ptr(&UNTYPED_LIST_TYPE),
200-
CassCollectionType::CASS_COLLECTION_TYPE_SET => ArcFFI::as_ptr(&UNTYPED_SET_TYPE),
201-
CassCollectionType::CASS_COLLECTION_TYPE_MAP => ArcFFI::as_ptr(&UNTYPED_MAP_TYPE),
202-
// CassCollectionType is a C enum. Panic, if it's out of range.
203-
_ => panic!(
204-
"CassCollectionType enum value out of range: {}",
205-
collection_ref.collection_type.0
206-
),
230+
CollectionType::List => ArcFFI::as_ptr(&UNTYPED_LIST_TYPE),
231+
CollectionType::Set => ArcFFI::as_ptr(&UNTYPED_SET_TYPE),
232+
CollectionType::Map => ArcFFI::as_ptr(&UNTYPED_MAP_TYPE),
207233
},
208234
}
209235
}

‎scylla-rust-wrapper/src/execution_error.rs

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ pub use crate::cass_error::{CassError, CassErrorSource};
44
use crate::cass_error_types::CassWriteType;
55
use crate::cass_types::CassConsistency;
66
use crate::types::*;
7+
use libc::c_char;
78
use scylla::deserialize::DeserializationError;
89
use scylla::errors::{DbError, ExecutionError, RequestAttemptError, WriteType};
910
use scylla::frame::frame_errors::ResultMetadataAndRowsCountParseError;
@@ -350,3 +351,78 @@ pub unsafe extern "C" fn cass_error_result_arg_type(
350351
_ => CassError::CASS_ERROR_LIB_INVALID_ERROR_RESULT_TYPE,
351352
}
352353
}
354+
355+
#[unsafe(no_mangle)]
356+
pub unsafe extern "C" fn cass_error_desc(error: CassError) -> *const c_char {
357+
let desc = match error {
358+
CassError::CASS_ERROR_LIB_BAD_PARAMS => c"Bad parameters",
359+
CassError::CASS_ERROR_LIB_NO_STREAMS => c"No streams available",
360+
CassError::CASS_ERROR_LIB_UNABLE_TO_INIT => c"Unable to initialize",
361+
CassError::CASS_ERROR_LIB_MESSAGE_ENCODE => c"Unable to encode message",
362+
CassError::CASS_ERROR_LIB_HOST_RESOLUTION => c"Unable to resolve host",
363+
CassError::CASS_ERROR_LIB_UNEXPECTED_RESPONSE => c"Unexpected response from server",
364+
CassError::CASS_ERROR_LIB_REQUEST_QUEUE_FULL => c"The request queue is full",
365+
CassError::CASS_ERROR_LIB_NO_AVAILABLE_IO_THREAD => c"No available IO threads",
366+
CassError::CASS_ERROR_LIB_WRITE_ERROR => c"Write error",
367+
CassError::CASS_ERROR_LIB_NO_HOSTS_AVAILABLE => c"No hosts available",
368+
CassError::CASS_ERROR_LIB_INDEX_OUT_OF_BOUNDS => c"Index out of bounds",
369+
CassError::CASS_ERROR_LIB_INVALID_ITEM_COUNT => c"Invalid item count",
370+
CassError::CASS_ERROR_LIB_INVALID_VALUE_TYPE => c"Invalid value type",
371+
CassError::CASS_ERROR_LIB_REQUEST_TIMED_OUT => c"Request timed out",
372+
CassError::CASS_ERROR_LIB_UNABLE_TO_SET_KEYSPACE => c"Unable to set keyspace",
373+
CassError::CASS_ERROR_LIB_CALLBACK_ALREADY_SET => c"Callback already set",
374+
CassError::CASS_ERROR_LIB_INVALID_STATEMENT_TYPE => c"Invalid statement type",
375+
CassError::CASS_ERROR_LIB_NAME_DOES_NOT_EXIST => c"No value or column for name",
376+
CassError::CASS_ERROR_LIB_UNABLE_TO_DETERMINE_PROTOCOL => {
377+
c"Unable to find supported protocol version"
378+
}
379+
CassError::CASS_ERROR_LIB_NULL_VALUE => c"NULL value specified",
380+
CassError::CASS_ERROR_LIB_NOT_IMPLEMENTED => c"Not implemented",
381+
CassError::CASS_ERROR_LIB_UNABLE_TO_CONNECT => c"Unable to connect",
382+
CassError::CASS_ERROR_LIB_UNABLE_TO_CLOSE => c"Unable to close",
383+
CassError::CASS_ERROR_LIB_NO_PAGING_STATE => c"No paging state",
384+
CassError::CASS_ERROR_LIB_PARAMETER_UNSET => c"Parameter unset",
385+
CassError::CASS_ERROR_LIB_INVALID_ERROR_RESULT_TYPE => c"Invalid error result type",
386+
CassError::CASS_ERROR_LIB_INVALID_FUTURE_TYPE => c"Invalid future type",
387+
CassError::CASS_ERROR_LIB_INTERNAL_ERROR => c"Internal error",
388+
CassError::CASS_ERROR_LIB_INVALID_CUSTOM_TYPE => c"Invalid custom type",
389+
CassError::CASS_ERROR_LIB_INVALID_DATA => c"Invalid data",
390+
CassError::CASS_ERROR_LIB_NOT_ENOUGH_DATA => c"Not enough data",
391+
CassError::CASS_ERROR_LIB_INVALID_STATE => c"Invalid state",
392+
CassError::CASS_ERROR_LIB_NO_CUSTOM_PAYLOAD => c"No custom payload",
393+
CassError::CASS_ERROR_LIB_EXECUTION_PROFILE_INVALID => {
394+
c"Invalid execution profile specified"
395+
}
396+
CassError::CASS_ERROR_LIB_NO_TRACING_ID => c"No tracing ID",
397+
CassError::CASS_ERROR_SERVER_SERVER_ERROR => c"Server error",
398+
CassError::CASS_ERROR_SERVER_PROTOCOL_ERROR => c"Protocol error",
399+
CassError::CASS_ERROR_SERVER_BAD_CREDENTIALS => c"Bad credentials",
400+
CassError::CASS_ERROR_SERVER_UNAVAILABLE => c"Unavailable",
401+
CassError::CASS_ERROR_SERVER_OVERLOADED => c"Overloaded",
402+
CassError::CASS_ERROR_SERVER_IS_BOOTSTRAPPING => c"Is bootstrapping",
403+
CassError::CASS_ERROR_SERVER_TRUNCATE_ERROR => c"Truncate error",
404+
CassError::CASS_ERROR_SERVER_WRITE_TIMEOUT => c"Write timeout",
405+
CassError::CASS_ERROR_SERVER_READ_TIMEOUT => c"Read timeout",
406+
CassError::CASS_ERROR_SERVER_READ_FAILURE => c"Read failure",
407+
CassError::CASS_ERROR_SERVER_FUNCTION_FAILURE => c"Function failure",
408+
CassError::CASS_ERROR_SERVER_WRITE_FAILURE => c"Write failure",
409+
CassError::CASS_ERROR_SERVER_SYNTAX_ERROR => c"Syntax error",
410+
CassError::CASS_ERROR_SERVER_UNAUTHORIZED => c"Unauthorized",
411+
CassError::CASS_ERROR_SERVER_INVALID_QUERY => c"Invalid query",
412+
CassError::CASS_ERROR_SERVER_CONFIG_ERROR => c"Configuration error",
413+
CassError::CASS_ERROR_SERVER_ALREADY_EXISTS => c"Already exists",
414+
CassError::CASS_ERROR_SERVER_UNPREPARED => c"Unprepared",
415+
CassError::CASS_ERROR_SSL_INVALID_CERT => c"Unable to load certificate",
416+
CassError::CASS_ERROR_SSL_INVALID_PRIVATE_KEY => c"Unable to load private key",
417+
CassError::CASS_ERROR_SSL_NO_PEER_CERT => c"No peer certificate",
418+
CassError::CASS_ERROR_SSL_INVALID_PEER_CERT => c"Invalid peer certificate",
419+
CassError::CASS_ERROR_SSL_IDENTITY_MISMATCH => {
420+
c"Certificate does not match host or IP address"
421+
}
422+
CassError::CASS_ERROR_SSL_PROTOCOL_ERROR => c"Protocol error",
423+
CassError::CASS_ERROR_SSL_CLOSED => c"Connection closed",
424+
_ => c"",
425+
};
426+
427+
desc.as_ptr()
428+
}

‎scylla-rust-wrapper/src/external.rs

Lines changed: 0 additions & 77 deletions
This file was deleted.

0 commit comments

Comments
 (0)