Skip to content

Commit 3bf4b26

Browse files
committed
introduce CollectionType enum for type safety
The driver was using the C enum `CassCollectionType` directly in its `CassCollection` struct, which led to necessity of handling the "else" case (values other than the explicitly listed variants) in many places. To bring type safety to the driver internals, this commit introduces a new Rust `CollectionType` enum, which is used internally instead of the C enum.
1 parent 901b26e commit 3bf4b26

File tree

1 file changed

+48
-21
lines changed

1 file changed

+48
-21
lines changed

scylla-rust-wrapper/src/collection.rs

Lines changed: 48 additions & 21 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() })
@@ -106,11 +138,11 @@ impl TryFrom<&CassCollection> for CassCqlValue {
106138
// FIXME: validate that collection items are correct
107139
let data_type = collection.data_type.clone();
108140
match collection.collection_type {
109-
CassCollectionType::CASS_COLLECTION_TYPE_LIST => Ok(CassCqlValue::List {
141+
CollectionType::List => Ok(CassCqlValue::List {
110142
data_type,
111143
values: collection.items.clone(),
112144
}),
113-
CassCollectionType::CASS_COLLECTION_TYPE_MAP => {
145+
CollectionType::Map => {
114146
let mut grouped_items = Vec::new();
115147
// FIXME: validate even number of items
116148
for i in (0..collection.items.len()).step_by(2) {
@@ -125,11 +157,10 @@ impl TryFrom<&CassCollection> for CassCqlValue {
125157
values: grouped_items,
126158
})
127159
}
128-
CassCollectionType::CASS_COLLECTION_TYPE_SET => Ok(CassCqlValue::Set {
160+
CollectionType::Set => Ok(CassCqlValue::Set {
129161
data_type,
130162
values: collection.items.clone(),
131163
}),
132-
_ => Err(()),
133164
}
134165
}
135166
}
@@ -146,6 +177,11 @@ pub unsafe extern "C" fn cass_collection_new(
146177
_ => item_count,
147178
} as usize;
148179

180+
let Ok(collection_type) = CollectionType::try_from(collection_type) else {
181+
tracing::error!("Provided invalid CassCollectionType to cass_collection_new!");
182+
return BoxFFI::null_mut();
183+
};
184+
149185
BoxFFI::into_ptr(Box::new(CassCollection {
150186
collection_type,
151187
data_type: None,
@@ -164,15 +200,11 @@ pub unsafe extern "C" fn cass_collection_new_from_data_type(
164200
};
165201

166202
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),
203+
CassDataTypeInner::List { .. } => (item_count, CollectionType::List),
204+
CassDataTypeInner::Set { .. } => (item_count, CollectionType::Set),
171205
// Maps consist of a key and a value, so twice
172206
// the number of CassCqlValue will be stored.
173-
CassDataTypeInner::Map { .. } => {
174-
(item_count * 2, CassCollectionType::CASS_COLLECTION_TYPE_MAP)
175-
}
207+
CassDataTypeInner::Map { .. } => (item_count * 2, CollectionType::Map),
176208
_ => return BoxFFI::null_mut(),
177209
};
178210
let capacity = capacity as usize;
@@ -196,14 +228,9 @@ pub unsafe extern "C" fn cass_collection_data_type(
196228
match &collection_ref.data_type {
197229
Some(dt) => ArcFFI::as_ptr(dt),
198230
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-
),
231+
CollectionType::List => ArcFFI::as_ptr(&UNTYPED_LIST_TYPE),
232+
CollectionType::Set => ArcFFI::as_ptr(&UNTYPED_SET_TYPE),
233+
CollectionType::Map => ArcFFI::as_ptr(&UNTYPED_MAP_TYPE),
207234
},
208235
}
209236
}

0 commit comments

Comments
 (0)