Skip to content

Commit 9a5c305

Browse files
lutterlutter
committed
all: Do not panic in Entity::make when given uninterned keys
1 parent 22c82ba commit 9a5c305

File tree

12 files changed

+66
-38
lines changed

12 files changed

+66
-38
lines changed

‎chain/substreams/src/trigger.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ where
217217
logger,
218218
);
219219

220-
let entity = state.entity_cache.make_entity(data);
220+
let entity = state.entity_cache.make_entity(data)?;
221221
state.entity_cache.set(key, entity)?;
222222
}
223223
Operation::Delete => {

‎core/src/subgraph/runner.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1046,7 +1046,7 @@ async fn update_proof_of_indexing(
10461046
entity_cache.schema =>
10471047
id: entity_key.entity_id.to_string(),
10481048
digest: updated_proof_of_indexing,
1049-
};
1049+
}?;
10501050

10511051
entity_cache.set(entity_key, new_poi_entity)?;
10521052
}

‎graph/src/components/store/entity_cache.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ impl EntityCache {
7373
}
7474

7575
/// Make a new entity. The entity is not part of the cache
76-
pub fn make_entity<I: IntoEntityIterator>(&self, iter: I) -> Entity {
76+
pub fn make_entity<I: IntoEntityIterator>(&self, iter: I) -> Result<Entity, anyhow::Error> {
7777
self.schema.make_entity(iter)
7878
}
7979

‎graph/src/data/store/mod.rs

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -612,6 +612,10 @@ impl<E, T: IntoIterator<Item = Result<(Word, Value), E>>> TryIntoEntityIterator<
612612
/// schema-less form can only be used in tests, since it creates an
613613
/// `AtomPool` just for this entity behind the scenes.
614614
///
615+
/// The form with schema returns a `Result<Entity, Error>` since it can be
616+
/// used in production code. The schemaless form returns an `Entity` because
617+
/// it unwraps the `Result` for you.
618+
///
615619
/// Production code should always use the form with the schema
616620
/// ```
617621
/// use graph::entity;
@@ -621,7 +625,7 @@ impl<E, T: IntoIterator<Item = Result<(Word, Value), E>>> TryIntoEntityIterator<
621625
/// let id = DeploymentHash::new("Qm123").unwrap();
622626
/// let schema = InputSchema::parse("type User @entity { id: String!, name: String! }", id).unwrap();
623627
///
624-
/// let entity = entity! { schema => id: "1", name: "John Doe" };
628+
/// let entity = entity! { schema => id: "1", name: "John Doe" }.unwrap();
625629
/// ```
626630
///
627631
/// Test code which often doesn't have access to an `InputSchema` can use
@@ -656,7 +660,7 @@ macro_rules! entity {
656660
pool.intern(stringify!($name));
657661
pairs.push(($crate::data::value::Word::from(stringify!($name)), $crate::data::store::Value::from($value)));
658662
)*
659-
$crate::data::store::Entity::make(std::sync::Arc::new(pool), pairs)
663+
$crate::data::store::Entity::make(std::sync::Arc::new(pool), pairs).unwrap()
660664
}
661665
};
662666
($($name:ident: $value:expr),*) => {
@@ -673,7 +677,7 @@ macro_rules! entity {
673677
$(
674678
pool.intern(stringify!($extra));
675679
)*
676-
$crate::data::store::Entity::make(std::sync::Arc::new(pool), pairs)
680+
$crate::data::store::Entity::make(std::sync::Arc::new(pool), pairs).unwrap()
677681
}
678682
};
679683
($($name:ident: $value:expr),*; $($extra:ident),*) => {
@@ -711,12 +715,17 @@ macro_rules! entity {
711715
}
712716

713717
impl Entity {
714-
pub fn make<I: IntoEntityIterator>(pool: Arc<AtomPool>, iter: I) -> Entity {
718+
pub fn make<I: IntoEntityIterator>(pool: Arc<AtomPool>, iter: I) -> Result<Entity, Error> {
715719
let mut obj = Object::new(pool);
716720
for (key, value) in iter {
717-
obj.insert(key, value).expect("key is in AtomPool");
721+
obj.insert(key, value).map_err(|e| {
722+
anyhow!(
723+
"Unknown key `{}`. It probably is not part of the schema",
724+
e.not_interned()
725+
)
726+
})?;
718727
}
719-
Entity(obj)
728+
Ok(Entity(obj))
720729
}
721730

722731
pub fn try_make<E, I: TryIntoEntityIterator<E>>(

‎graph/src/schema/input_schema.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,7 @@ impl Inner {
283283
self.schema.validate()
284284
}
285285

286-
pub fn make_entity<I: IntoEntityIterator>(&self, iter: I) -> Entity {
286+
pub fn make_entity<I: IntoEntityIterator>(&self, iter: I) -> Result<Entity, Error> {
287287
Entity::make(self.pool.clone(), iter)
288288
}
289289

‎graph/src/util/intern.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,14 @@ pub enum Error {
5151
NotInterned(String),
5252
}
5353

54+
impl Error {
55+
pub fn not_interned(&self) -> &str {
56+
match self {
57+
Error::NotInterned(s) => s,
58+
}
59+
}
60+
}
61+
5462
#[derive(Debug, PartialEq)]
5563
/// A pool of interned strings. Pools can be organized hierarchically with
5664
/// lookups in child pools also considering the parent pool. The chain of

‎runtime/test/src/test.rs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -939,11 +939,13 @@ async fn test_entity_store(api_version: Version) {
939939
None
940940
} else {
941941
Some(
942-
schema.make_entity(
943-
module
944-
.asc_get::<HashMap<Word, Value>, _>(entity_ptr)
945-
.unwrap(),
946-
),
942+
schema
943+
.make_entity(
944+
module
945+
.asc_get::<HashMap<Word, Value>, _>(entity_ptr)
946+
.unwrap(),
947+
)
948+
.unwrap(),
947949
)
948950
}
949951
};

‎runtime/wasm/src/host_exports.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ impl<C: Blockchain> HostExports<C> {
184184

185185
let entity = state
186186
.entity_cache
187-
.make_entity(data.into_iter().map(|(key, value)| (key, value)));
187+
.make_entity(data.into_iter().map(|(key, value)| (key, value)))?;
188188

189189
state.entity_cache.set(key, entity)?;
190190

‎store/postgres/src/fork.rs

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ query Query ($id: String) {{
220220
map
221221
};
222222

223-
Ok(Some(schema.make_entity(map)))
223+
Ok(Some(schema.make_entity(map)?))
224224
}
225225
}
226226

@@ -368,18 +368,20 @@ mod tests {
368368

369369
assert_eq!(
370370
entity.unwrap(),
371-
schema.make_entity(vec![
372-
("id".into(), Value::String("0x00".to_string())),
373-
(
374-
"owner".into(),
375-
Value::Bytes(scalar::Bytes::from_str("0x01").unwrap())
376-
),
377-
("displayName".into(), Value::String("test".to_string())),
378-
(
379-
"imageUrl".into(),
380-
Value::String("http://example.com/image.png".to_string())
381-
),
382-
])
371+
schema
372+
.make_entity(vec![
373+
("id".into(), Value::String("0x00".to_string())),
374+
(
375+
"owner".into(),
376+
Value::Bytes(scalar::Bytes::from_str("0x01").unwrap())
377+
),
378+
("displayName".into(), Value::String("test".to_string())),
379+
(
380+
"imageUrl".into(),
381+
Value::String("http://example.com/image.png".to_string())
382+
),
383+
])
384+
.unwrap()
383385
);
384386
}
385387
}

‎store/test-store/tests/graphql/query.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -344,12 +344,12 @@ async fn insert_test_entities(
344344
entity! { is => __typename: "Album", id: "rl1", title: "Pop and Folk", songs: vec![s[3], s[4]] },
345345
entity! { is => __typename: "Single", id: "rl2", title: "Rock", songs: vec![s[2]] },
346346
entity! { is => __typename: "Single", id: "rl3", title: "Cheesy", songs: vec![s[1]] },
347-
];
347+
].into_iter().collect::<Result<_,_>>().unwrap();
348348

349349
let entities1 = vec![
350350
entity! { is => __typename: "Musician", id: "m3", name: "Tom", mainBand: "b2", bands: vec!["b1", "b2"] },
351351
entity! { is => __typename: "Musician", id: "m4", name: "Valerie", bands: Vec::<String>::new() },
352-
];
352+
].into_iter().collect::<Result<_,_>>().unwrap();
353353

354354
async fn insert_at(entities: Vec<Entity>, deployment: &DeploymentLocator, block_ptr: BlockPtr) {
355355
let insert_ops = entities.into_iter().map(|data| EntityOperation::Set {

0 commit comments

Comments
 (0)