-
Notifications
You must be signed in to change notification settings - Fork 182
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.
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): model upgrades #2637
Changes from 35 commits
b1f3e35
fa71dbd
60a870d
62750f6
6313f44
f7fd6ae
e85bdd0
d641ba0
3336c4a
3742676
b3ee109
0d26d43
cbc39c7
14ffc8e
ddd9921
cf9c45c
3925cce
ae8bf42
210ac31
d86fcc4
ab855dd
5d57e6d
b04e848
e3d8a7b
b7f3264
87857c2
9b5d624
9ac0d21
a8d6737
d2c7696
255d4fb
6235080
612fc4b
0dbec7e
551c6bd
cea8102
155033d
4e921c3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -190,46 +190,29 @@ | |
} | ||
} | ||
|
||
pub fn to_sql_value(&self) -> Result<String, PrimitiveError> { | ||
let value = self.serialize()?; | ||
|
||
if value.is_empty() { | ||
return Err(PrimitiveError::MissingFieldElement); | ||
} | ||
|
||
pub fn to_sql_value(&self) -> String { | ||
match self { | ||
// Integers | ||
Primitive::I8(_) => Ok(format!("{}", try_from_felt::<i8>(value[0])?)), | ||
Primitive::I16(_) => Ok(format!("{}", try_from_felt::<i16>(value[0])?)), | ||
Primitive::I32(_) => Ok(format!("{}", try_from_felt::<i32>(value[0])?)), | ||
Primitive::I64(_) => Ok(format!("{}", try_from_felt::<i64>(value[0])?)), | ||
Primitive::I8(i8) => format!("{}", i8.unwrap_or_default()), | ||
Primitive::I16(i16) => format!("{}", i16.unwrap_or_default()), | ||
Primitive::I32(i32) => format!("{}", i32.unwrap_or_default()), | ||
Primitive::I64(i64) => format!("{}", i64.unwrap_or_default()), | ||
|
||
Primitive::U8(_) | ||
| Primitive::U16(_) | ||
| Primitive::U32(_) | ||
| Primitive::USize(_) | ||
| Primitive::Bool(_) => Ok(format!("{}", value[0])), | ||
Primitive::U8(u8) => format!("{}", u8.unwrap_or_default()), | ||
Primitive::U16(u16) => format!("{}", u16.unwrap_or_default()), | ||
Primitive::U32(u32) => format!("{}", u32.unwrap_or_default()), | ||
Primitive::USize(u32) => format!("{}", u32.unwrap_or_default()), | ||
Primitive::Bool(bool) => format!("{}", bool.unwrap_or_default() as i32), | ||
|
||
// Hex string | ||
Primitive::I128(_) => Ok(format!("{:#064x}", try_from_felt::<i128>(value[0])?)), | ||
Primitive::ContractAddress(_) | ||
| Primitive::ClassHash(_) | ||
| Primitive::Felt252(_) | ||
| Primitive::U128(_) | ||
| Primitive::U64(_) => Ok(format!("{:#064x}", value[0])), | ||
|
||
Primitive::U256(_) => { | ||
if value.len() < 2 { | ||
Err(PrimitiveError::NotEnoughFieldElements) | ||
} else { | ||
let mut buffer = [0u8; 32]; | ||
let value0_bytes = value[0].to_bytes_be(); | ||
let value1_bytes = value[1].to_bytes_be(); | ||
buffer[16..].copy_from_slice(&value0_bytes[16..]); | ||
buffer[..16].copy_from_slice(&value1_bytes[16..]); | ||
Ok(format!("0x{}", hex::encode(buffer))) | ||
} | ||
} | ||
Primitive::I128(i128) => format!("0x{:064x}", i128.unwrap_or_default()), | ||
Primitive::ContractAddress(felt) => format!("0x{:064x}", felt.unwrap_or_default()), | ||
Primitive::ClassHash(felt) => format!("0x{:064x}", felt.unwrap_or_default()), | ||
Primitive::Felt252(felt) => format!("0x{:064x}", felt.unwrap_or_default()), | ||
Primitive::U128(u128) => format!("0x{:064x}", u128.unwrap_or_default()), | ||
Primitive::U64(u64) => format!("0x{:064x}", u64.unwrap_or_default()), | ||
|
||
Primitive::U256(u256) => format!("0x{:064x}", u256.unwrap_or_default()), | ||
} | ||
} | ||
|
||
|
@@ -436,7 +419,7 @@ | |
let primitive = Primitive::U256(Some(U256::from_be_hex( | ||
"aaaaaaaaaaaaaaaabbbbbbbbbbbbbbbbccccccccccccccccdddddddddddddddd", | ||
))); | ||
let sql_value = primitive.to_sql_value().unwrap(); | ||
let sql_value = primitive.to_sql_value(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Ohayo, sensei! Add error case tests The test only covers the happy path with Add these test cases: #[test]
fn test_u256_none() {
let primitive = Primitive::U256(None);
assert!(matches!(
primitive.serialize(),
Err(PrimitiveError::MissingFieldElement)
));
} |
||
let serialized = primitive.serialize().unwrap(); | ||
|
||
let mut deserialized = primitive; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -173,6 +173,11 @@ | |
} | ||
|
||
pub fn deserialize(&mut self, felts: &mut Vec<Felt>) -> Result<(), PrimitiveError> { | ||
if felts.is_empty() { | ||
// return early if there are no felts to deserialize | ||
return Ok(()); | ||
} | ||
|
||
match self { | ||
Ty::Primitive(c) => { | ||
c.deserialize(felts)?; | ||
|
@@ -224,6 +229,85 @@ | |
} | ||
Ok(()) | ||
} | ||
|
||
/// Returns a new Ty containing only the differences between self and other | ||
pub fn diff(&self, other: &Ty) -> Option<Ty> { | ||
match (self, other) { | ||
(Ty::Struct(s1), Ty::Struct(s2)) => { | ||
// Find members that exist in s1 but not in s2, or are different | ||
let diff_children: Vec<Member> = s1 | ||
.children | ||
.iter() | ||
.filter(|m1| { | ||
s2.children | ||
.iter() | ||
.find(|m2| m2.name == m1.name) | ||
.map_or(true, |m2| *m1 != m2) | ||
}) | ||
.cloned() | ||
.collect(); | ||
|
||
if diff_children.is_empty() { | ||
None | ||
} else { | ||
Some(Ty::Struct(Struct { name: s1.name.clone(), children: diff_children })) | ||
} | ||
} | ||
(Ty::Enum(e1), Ty::Enum(e2)) => { | ||
// Find options that exist in e1 but not in e2, or are different | ||
let diff_options: Vec<EnumOption> = e1 | ||
.options | ||
.iter() | ||
.filter(|o1| { | ||
e2.options.iter().find(|o2| o2.name == o1.name).map_or(true, |o2| *o1 != o2) | ||
}) | ||
.cloned() | ||
.collect(); | ||
|
||
if diff_options.is_empty() { | ||
None | ||
} else { | ||
Some(Ty::Enum(Enum { | ||
name: e1.name.clone(), | ||
option: e1.option, | ||
options: diff_options, | ||
})) | ||
} | ||
} | ||
(Ty::Array(a1), Ty::Array(a2)) => { | ||
if a1 == a2 { | ||
None | ||
} else { | ||
Some(Ty::Array(a1.clone())) | ||
} | ||
} | ||
(Ty::Tuple(t1), Ty::Tuple(t2)) => { | ||
if t1 == t2 { | ||
None | ||
} else { | ||
Some(Ty::Tuple(t1.clone())) | ||
} | ||
} | ||
(Ty::ByteArray(b1), Ty::ByteArray(b2)) => { | ||
if b1 == b2 { | ||
None | ||
} else { | ||
Some(Ty::ByteArray(b1.clone())) | ||
} | ||
} | ||
(Ty::Primitive(p1), Ty::Primitive(p2)) => { | ||
if p1 == p2 { | ||
None | ||
} else { | ||
Some(Ty::Primitive(*p1)) | ||
} | ||
} | ||
// Different types entirely - we cannot diff them | ||
_ => { | ||
panic!("Type mismatch between self {:?} and other {:?}", self.name(), other.name()) | ||
} | ||
} | ||
} | ||
} | ||
|
||
#[derive(Debug)] | ||
|
@@ -351,8 +435,8 @@ | |
} | ||
} | ||
|
||
pub fn to_sql_value(&self) -> Result<String, EnumError> { | ||
self.option().map(|option| option.name.clone()) | ||
pub fn to_sql_value(&self) -> String { | ||
self.option().unwrap_or(&self.options[0]).name.clone() | ||
Comment on lines
+438
to
+439
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Handle potential panics in to_sql_value. Ohayo! The Consider this safer implementation: - pub fn to_sql_value(&self) -> String {
- self.option().unwrap_or(&self.options[0]).name.clone()
+ pub fn to_sql_value(&self) -> Result<String, EnumError> {
+ Ok(self.option().unwrap_or_else(|_| &self.options[0]).name.clone())
|
||
} | ||
} | ||
|
||
|
@@ -597,4 +681,77 @@ | |
assert_eq!(format_member(&member), expected); | ||
} | ||
} | ||
|
||
#[test] | ||
fn test_ty_diff() { | ||
// Test struct diff | ||
let struct1 = Ty::Struct(Struct { | ||
name: "TestStruct".to_string(), | ||
children: vec![ | ||
Member { | ||
name: "field1".to_string(), | ||
ty: Ty::Primitive(Primitive::U32(None)), | ||
key: false, | ||
}, | ||
Member { | ||
name: "field2".to_string(), | ||
ty: Ty::Primitive(Primitive::U32(None)), | ||
key: false, | ||
}, | ||
Member { | ||
name: "field3".to_string(), | ||
ty: Ty::Primitive(Primitive::U32(None)), | ||
key: false, | ||
}, | ||
], | ||
}); | ||
|
||
let struct2 = Ty::Struct(Struct { | ||
name: "TestStruct".to_string(), | ||
children: vec![Member { | ||
name: "field1".to_string(), | ||
ty: Ty::Primitive(Primitive::U32(None)), | ||
key: false, | ||
}], | ||
}); | ||
|
||
// Should show only field2 and field3 as differences | ||
let diff = struct1.diff(&struct2).unwrap(); | ||
if let Ty::Struct(s) = diff { | ||
assert_eq!(s.children.len(), 2); | ||
assert_eq!(s.children[0].name, "field2"); | ||
assert_eq!(s.children[1].name, "field3"); | ||
} else { | ||
panic!("Expected Struct diff"); | ||
} | ||
|
||
// Test enum diff | ||
let enum1 = Ty::Enum(Enum { | ||
name: "TestEnum".to_string(), | ||
option: None, | ||
options: vec![ | ||
EnumOption { name: "Option1".to_string(), ty: Ty::Tuple(vec![]) }, | ||
EnumOption { name: "Option2".to_string(), ty: Ty::Tuple(vec![]) }, | ||
], | ||
}); | ||
|
||
let enum2 = Ty::Enum(Enum { | ||
name: "TestEnum".to_string(), | ||
option: None, | ||
options: vec![EnumOption { name: "Option1".to_string(), ty: Ty::Tuple(vec![]) }], | ||
}); | ||
|
||
// Should show only Option2 as difference | ||
let diff = enum1.diff(&enum2).unwrap(); | ||
if let Ty::Enum(e) = diff { | ||
assert_eq!(e.options.len(), 1); | ||
assert_eq!(e.options[0].name, "Option2"); | ||
} else { | ||
panic!("Expected Enum diff"); | ||
} | ||
|
||
// Test no differences | ||
let same_struct = struct2.diff(&struct2); | ||
assert!(same_struct.is_none()); | ||
} | ||
Larkooo marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ohayo, sensei! Consider handling
None
values explicitly into_sql_value
The current implementation uses
unwrap_or_default()
which might lead to unintended default values being used, especially in a database context where data accuracy is crucial. Consider explicitly handlingNone
values to prevent data corruption.