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): add list to member value enum for usage in sdk #2828

Merged
merged 2 commits into from
Dec 23, 2024
Merged

feat(torii-grpc): add list to member value enum for usage in sdk #2828

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dd193ff
feat(torii-grpc): add list to member value enum for usasge in sdk
Larkooo Dec 20, 2024
e4d9d86
fmt
Larkooo Dec 20, 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
4 changes: 4 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 @@ -151,6 +151,7 @@ pub enum PatternMatching {
pub enum MemberValue {
Primitive(Primitive),
String(String),
List(Vec<MemberValue>),
}

#[derive(Debug, Serialize, Deserialize, PartialEq, Hash, Eq, Clone)]
Expand Down Expand Up @@ -418,6 +419,9 @@ impl From<MemberValue> for member_value::ValueType {
member_value::ValueType::Primitive(primitive.into())
}
MemberValue::String(string) => member_value::ValueType::String(string),
MemberValue::List(list) => member_value::ValueType::List(proto::types::MemberValueList {
values: list.into_iter().map(|v| proto::types::MemberValue { value_type: Some(v.into()) }).collect(),
}),
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Ohayo! We need to implement the reverse conversion, sensei!

After reviewing the entire file, I can confirm that while we have the conversion from MemberValue to protobuf type, we're missing the reverse conversion (from protobuf to MemberValue). This could lead to issues when deserializing data from the protobuf format.

  • Missing implementation for From<proto::types::MemberValue> for MemberValue or TryFrom<proto::types::MemberValue> for MemberValue
  • The reverse conversion for the List variant needs to be handled recursively, similar to the forward conversion
🔗 Analysis chain

Implementation looks clean, but let's verify the reverse conversion, sensei!

The conversion to protobuf type is well-implemented using iterators and handles nested values correctly. However, we should ensure the reverse conversion (from proto to MemberValue) is also implemented.

Let's check for the reverse conversion implementation:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for the reverse conversion implementation
ast-grep --pattern 'impl $_ for MemberValue {
  $$$
  fn from($_: proto::types::MemberValue) -> Self {
    $$$
  }
}'

# Also check for TryFrom implementation
ast-grep --pattern 'impl TryFrom<proto::types::MemberValue> for MemberValue {
  $$$
  fn try_from($_: proto::types::MemberValue) -> Result<Self, Self::Error> {
    $$$
  }
}'

Length of output: 296

Script:

#!/bin/bash
# Let's try a broader search for MemberValue implementations
rg -A 10 "impl.*MemberValue" 

# Also search for From trait implementations in the file
rg -A 10 "impl.*From.*MemberValue" crates/torii/grpc/src/types/mod.rs

# And search specifically for the List variant conversion
rg -A 5 "MemberValue::List" crates/torii/grpc/src/types/mod.rs

Length of output: 1963

Script:

#!/bin/bash
# Let's search for From implementations in the reverse direction
rg -A 10 "impl From<proto::types::MemberValue.*> for MemberValue"

# Also search for any conversion from proto::types::MemberValueList
rg -A 10 "MemberValueList.*MemberValue"

# And let's check the entire file content to ensure we don't miss any conversion
cat crates/torii/grpc/src/types/mod.rs

Length of output: 16672

}
}
}
Expand Down
Loading