review feedback

shotover · Nov 8, 2022 · 4e7909f · 4e7909f
1 parent fcbfc98
commit 4e7909f
Show file tree

Hide file tree

Showing 3 changed files with 69 additions and 56 deletions.
diff --git a/shotover-proxy/src/frame/cassandra.rs b/shotover-proxy/src/frame/cassandra.rs
@@ -205,45 +205,38 @@ impl CassandraFrame {
  if let ResponseBody::Result(result) = frame.response_body()? {
  match result {
  ResResultBody::Rows(rows) => {
- let converted_rows = if rows
- .metadata
- .flags
- .contains(RowsMetadataFlags::NO_METADATA)
- {
- rows.rows_content
- .into_iter()
- .map(|row| {
- row.into_iter()
- .map(|row_content| {
- if row_content.is_empty() {
- MessageValue::Null
- } else {
+ let converted_rows =
+ if rows.metadata.flags.contains(RowsMetadataFlags::NO_METADATA) {
+ rows.rows_content
+ .into_iter()
+ .map(|row| {
+ row.into_iter()
+ .map(|row_content| {
  MessageValue::Bytes(
  row_content.into_bytes().unwrap().into(),
  )
- }
- })
- .collect()
- })
- .collect()
- } else {
- rows.rows_content
- .into_iter()
- .map(|row| {
- row.into_iter()
- .enumerate()
- .map(|(i, row_content)| {
- let col_spec = &rows.metadata.col_specs[i];
- MessageValue::build_value_from_cstar_col_type(
- frame.version,
- col_spec,
- &row_content,
- )
- })
- .collect()
- })
- .collect()
- };
+ })
+ .collect()
+ })
+ .collect()
+ } else {
+ rows.rows_content
+ .into_iter()
+ .map(|row| {
+ row.into_iter()
+ .enumerate()
+ .map(|(i, row_content)| {
+ let col_spec = &rows.metadata.col_specs[i];
+ MessageValue::build_value_from_cstar_col_type(
+ frame.version,
+ col_spec,
+ &row_content,
+ )
+ })
+ .collect()
+ })
+ .collect()
+ };
  CassandraOperation::Result(CassandraResult::Rows {
  rows: converted_rows,
  metadata: Box::new(rows.metadata),

diff --git a/shotover-proxy/tests/cassandra_int_tests/prepared_statements.rs b/shotover-proxy/tests/cassandra_int_tests/prepared_statements.rs
@@ -1,8 +1,8 @@
-use futures::Future;
-
+use crate::helpers::cassandra::CassandraDriver;
 use crate::helpers::cassandra::{
  assert_query_result, assert_rows, run_query, CassandraConnection, ResultValue,
 };
+use futures::Future;
 
 async fn delete(session: &CassandraConnection) {
  let prepared = session
@@ -102,12 +102,6 @@ async fn use_statement(session: &CassandraConnection) {
  .await;
 }
 
-async fn hardcoded_prepared(session: &CassandraConnection) {
- let cql = "SELECT * FROM system.local WHERE key = 'local'";
- let prepared = session.prepare(cql).await;
- session.execute_prepared(&prepared, None).await;
-}
-
 pub async fn test<Fut>(session: &CassandraConnection, connection_creator: impl Fn() -> Fut)
 where
  Fut: Future<Output = CassandraConnection>,
@@ -125,5 +119,10 @@ where
  select_cross_connection(session, connection_creator).await;
  delete(session).await;
  use_statement(session).await;
- hardcoded_prepared(session).await;
+
+ if session.is(&[CassandraDriver::Scylla, CassandraDriver::CdrsTokio]) {
+ let cql = "SELECT * FROM system.local WHERE key = 'local'";
+ let prepared = session.prepare(cql).await;
+ session.execute_prepared(&prepared, None).await;
+ }
 }
diff --git a/shotover-proxy/tests/helpers/cassandra.rs b/shotover-proxy/tests/helpers/cassandra.rs
@@ -83,7 +83,7 @@ fn cpp_error_to_cdrs(code: CassErrorCode, message: String) -> ErrorBody {
 }
 
 #[allow(dead_code)]
-#[derive(Copy, Clone)]
+#[derive(Copy, Clone, Eq, PartialEq)]
 pub enum CassandraDriver {
  #[cfg(feature = "cassandra-cpp-driver-tests")]
  Datastax,
@@ -197,6 +197,16 @@ impl CassandraConnection {
  }
  }
 
+ #[allow(dead_code)]
+ pub fn is(&self, drivers: &[CassandraDriver]) -> bool {
+ match self {
+ Self::CdrsTokio { .. } => drivers.contains(&CassandraDriver::CdrsTokio),
+ #[cfg(feature = "cassandra-cpp-driver-tests")]
+ Self::Datastax { .. } => drivers.contains(&CassandraDriver::Datastax),
+ Self::Scylla { .. } => drivers.contains(&CassandraDriver::Scylla),
+ }
+ }
+
  #[cfg(feature = "cassandra-cpp-driver-tests")]
  #[allow(dead_code)]
  pub fn as_datastax(&self) -> &DatastaxSession {
@@ -300,12 +310,11 @@ impl CassandraConnection {
  Self::process_cdrs_response(response)
  }
  Self::Scylla { session, .. } => {
- let rows = session.query(query, ()).await.unwrap().rows;
- match rows {
- Some(rows) => rows
- .into_iter()
- .map(|x| {
- x.columns
+ let response = session.query(query, ()).await.unwrap();
+ if let Ok(rows) = response.rows() {
+ rows.into_iter()
+ .map(|row| {
+ row.columns
  .into_iter()
  .map(|col| {
  if let Some(col) = col {
@@ -316,8 +325,9 @@ impl CassandraConnection {
  })
  .collect()
  })
- .collect(),
- None => vec![],
+ .collect()
+ } else {
+ vec![]
  }
  }
  };
@@ -515,14 +525,25 @@ impl CassandraConnection {
  }
  Self::Scylla { session, .. } => {
  let statement = prepared_query.as_scylla();
- let response = session.execute(statement, (value,)).await.unwrap();
+
+ let response = if let Some(value) = value {
+ session.execute(statement, (value,)).await.unwrap()
+ } else {
+ session.execute(statement, ()).await.unwrap()
+ };
 
  if let Ok(rows) = response.rows() {
  rows.into_iter()
  .map(|row| {
  row.columns
  .into_iter()
- .map(|col| ResultValue::new_from_scylla(col.unwrap()))
+ .map(|col| {
+ if let Some(col) = col {
+ ResultValue::new_from_scylla(col)
+ } else {
+ ResultValue::Null
+ }
+ })
  .collect()
  })
  .collect()