Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(torii-grpc): IN operator for comparison #2812

Merged
merged 4 commits into from
Dec 17, 2024
Merged

feat(torii-grpc): IN operator for comparison #2812

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2c18b72
feat: in operator grpc finish
Larkooo Dec 17, 2024
0abc2e8
feat: clean up grpc
Larkooo Dec 17, 2024
06e557f
fmt
Larkooo Dec 17, 2024
5dc0c8c
fmt
Larkooo Dec 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions crates/torii/grpc/proto/types.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,14 @@ message MemberValue {
oneof value_type {
Primitive primitive = 1;
string string = 2;
MemberValueList list = 3;
}
}

message MemberValueList {
repeated MemberValue values = 1;
}

message MemberClause {
string model = 2;
string member = 3;
Expand Down Expand Up @@ -152,6 +157,8 @@ enum ComparisonOperator {
GTE = 3;
LT = 4;
LTE = 5;
IN = 6;
NOT_IN = 7;
}

message Token {
Expand Down
41 changes: 25 additions & 16 deletions crates/torii/grpc/src/server/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -773,15 +773,19 @@
let comparison_operator = ComparisonOperator::from_repr(member_clause.operator as usize)
.expect("invalid comparison operator");

let comparison_value =
match member_clause.value.ok_or(QueryError::MissingParam("value".into()))?.value_type {
Some(ValueType::String(value)) => value,
fn comparison_value(value: &proto::types::MemberValue) -> Result<String, Error> {
match &value.value_type {
Some(ValueType::String(value)) => Ok(value.to_string()),

Check warning on line 778 in crates/torii/grpc/src/server/mod.rs

View check run for this annotation

Codecov / codecov/patch

crates/torii/grpc/src/server/mod.rs#L776-L778 

Added lines #L776 - L778 were not covered by tests
Some(ValueType::Primitive(value)) => {
let primitive: Primitive = value.try_into()?;
primitive.to_sql_value()
let primitive: Primitive = (value.clone()).try_into()?;
Ok(primitive.to_sql_value())

Check warning on line 781 in crates/torii/grpc/src/server/mod.rs

View check run for this annotation

Codecov / codecov/patch

crates/torii/grpc/src/server/mod.rs#L780-L781 

Added lines #L780 - L781 were not covered by tests
}
Some(ValueType::List(values)) => {
Ok(format!("({})", values.values.iter().map(|v| comparison_value(v)).collect::<Result<Vec<String>, Error>>()?.join(", ")))

Check warning on line 784 in crates/torii/grpc/src/server/mod.rs

View check run for this annotation

Codecov / codecov/patch

crates/torii/grpc/src/server/mod.rs#L783-L784 

Added lines #L783 - L784 were not covered by tests
}
None => return Err(QueryError::MissingParam("value_type".into()).into()),
};
}
}

Check warning on line 788 in crates/torii/grpc/src/server/mod.rs

View check run for this annotation

Codecov / codecov/patch

crates/torii/grpc/src/server/mod.rs#L788 

Added line #L788 was not covered by tests

let (namespace, model) = member_clause
.model
Expand Down Expand Up @@ -838,14 +842,15 @@
offset,
)?;

let value = comparison_value(&member_clause.value.clone().ok_or(QueryError::MissingParam("value".into()))?)?;

Check warning on line 845 in crates/torii/grpc/src/server/mod.rs

View check run for this annotation

Codecov / codecov/patch

crates/torii/grpc/src/server/mod.rs#L845 

Added line #L845 was not covered by tests
let total_count = sqlx::query_scalar(&count_query)
.bind(comparison_value.clone())
.bind(value.clone())

Check warning on line 847 in crates/torii/grpc/src/server/mod.rs

View check run for this annotation

Codecov / codecov/patch

crates/torii/grpc/src/server/mod.rs#L847 

Added line #L847 was not covered by tests
.bind(entity_updated_after.clone())
.fetch_optional(&self.pool)
.await?
.unwrap_or(0);

let mut query = sqlx::query(&entity_query).bind(comparison_value);
let mut query = sqlx::query(&entity_query).bind(value);

Check warning on line 853 in crates/torii/grpc/src/server/mod.rs

View check run for this annotation

Codecov / codecov/patch

crates/torii/grpc/src/server/mod.rs#L853 

Added line #L853 was not covered by tests
if let Some(entity_updated_after) = entity_updated_after.clone() {
query = query.bind(entity_updated_after);
}
Expand Down Expand Up @@ -1356,17 +1361,21 @@
ClauseType::Member(member) => {
let comparison_operator = ComparisonOperator::from_repr(member.operator as usize)
.expect("invalid comparison operator");
let value = member.value.clone();
let comparison_value =
match value.ok_or(QueryError::MissingParam("value".into()))?.value_type {
Some(ValueType::String(value)) => value,
let value = member.value.clone().ok_or(QueryError::MissingParam("value".into()))?;
fn comparison_value(value: &proto::types::MemberValue) -> Result<String, Error> {
match &value.value_type {
Some(ValueType::String(value)) => Ok(value.to_string()),

Check warning on line 1367 in crates/torii/grpc/src/server/mod.rs

View check run for this annotation

Codecov / codecov/patch

crates/torii/grpc/src/server/mod.rs#L1364-L1367 

Added lines #L1364 - L1367 were not covered by tests
Some(ValueType::Primitive(value)) => {
let primitive: Primitive = value.try_into()?;
primitive.to_sql_value()
let primitive: Primitive = (value.clone()).try_into()?;
Ok(primitive.to_sql_value())

Check warning on line 1370 in crates/torii/grpc/src/server/mod.rs

View check run for this annotation

Codecov / codecov/patch

crates/torii/grpc/src/server/mod.rs#L1369-L1370 

Added lines #L1369 - L1370 were not covered by tests
}
Some(ValueType::List(values)) => {
Ok(format!("({})", values.values.iter().map(|v| comparison_value(v)).collect::<Result<Vec<String>, Error>>()?.join(", ")))

Check warning on line 1373 in crates/torii/grpc/src/server/mod.rs

View check run for this annotation

Codecov / codecov/patch

crates/torii/grpc/src/server/mod.rs#L1372-L1373 

Added lines #L1372 - L1373 were not covered by tests
}
None => return Err(QueryError::MissingParam("value_type".into()).into()),
};
bind_values.push(comparison_value);
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Code duplication detected, sensei!

The comparison_value function is duplicated. Consider extracting it to a single location, perhaps as a private function at module level, to maintain DRY principles.

Apply this refactor to remove the duplication:

// Add at module level
-fn comparison_value(value: &proto::types::MemberValue) -> Result<String, Error> {
-    match &value.value_type {
-        Some(ValueType::String(value)) => Ok(value.to_string()),
-        Some(ValueType::Primitive(value)) => {
-            let primitive: Primitive = (value.clone()).try_into()?;
-            Ok(primitive.to_sql_value())
-        }
-        Some(ValueType::List(values)) => {
-            Ok(format!("({})", values.values.iter().map(|v| comparison_value(v)).collect::<Result<Vec<String>, Error>>()?.join(", ")))
-        }
-        None => return Err(QueryError::MissingParam("value_type".into()).into()),
-    }
-}

// Remove the first implementation and use the module level function
// Remove the second implementation and use the module level function

Committable suggestion skipped: line range outside the PR's diff.

bind_values.push(comparison_value(&value)?);

Check warning on line 1378 in crates/torii/grpc/src/server/mod.rs

View check run for this annotation

Codecov / codecov/patch

crates/torii/grpc/src/server/mod.rs#L1377-L1378 

Added lines #L1377 - L1378 were not covered by tests

let model = member.model.clone();
// Get or create unique alias for this model
Expand Down
6 changes: 6 additions & 0 deletions crates/torii/grpc/src/types/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,8 @@
Gte,
Lt,
Lte,
In,
NotIn,
}

impl fmt::Display for ComparisonOperator {
Expand All @@ -198,6 +200,8 @@
ComparisonOperator::Lte => write!(f, "<="),
ComparisonOperator::Neq => write!(f, "!="),
ComparisonOperator::Eq => write!(f, "="),
ComparisonOperator::In => write!(f, "IN"),
ComparisonOperator::NotIn => write!(f, "NOT IN"),

Check warning on line 204 in crates/torii/grpc/src/types/mod.rs

View check run for this annotation

Codecov / codecov/patch

crates/torii/grpc/src/types/mod.rs#L203-L204 

Added lines #L203 - L204 were not covered by tests
}
}
}
Expand All @@ -211,6 +215,8 @@
proto::types::ComparisonOperator::Lt => ComparisonOperator::Lt,
proto::types::ComparisonOperator::Lte => ComparisonOperator::Lte,
proto::types::ComparisonOperator::Neq => ComparisonOperator::Neq,
proto::types::ComparisonOperator::In => ComparisonOperator::In,
proto::types::ComparisonOperator::NotIn => ComparisonOperator::NotIn,

Check warning on line 219 in crates/torii/grpc/src/types/mod.rs

View check run for this annotation

Codecov / codecov/patch

crates/torii/grpc/src/types/mod.rs#L218-L219 

Added lines #L218 - L219 were not covered by tests
}
}
}
Expand Down
Loading