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.

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

Add new API Version to validate when setting fields not defined in the schema #4894

Merged
merged 7 commits into from
Oct 8, 2023
24 changes: 6 additions & 18 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion graph/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ bytes = "1.0.1"
cid = "0.10.1"
diesel = { version = "1.4.8", features = ["postgres", "serde_json", "numeric", "r2d2", "chrono"] }
diesel_derives = "1.4"
chrono = "0.4.25"
chrono = "0.4.31"
envconfig = "0.10.0"
Inflector = "0.11.3"
isatty = "0.1.9"
Expand Down
3 changes: 3 additions & 0 deletions graph/src/data/subgraph/api_version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ pub const API_VERSION_0_0_6: Version = Version::new(0, 0, 6);
/// Enables event handlers to require transaction receipts in the runtime.
pub const API_VERSION_0_0_7: Version = Version::new(0, 0, 7);

/// Enables validation for fields that doesnt exist in the schema for an entity.
Copy link
Collaborator

Choose a reason for hiding this comment

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

small typo: s/doesnt/don't/

pub const API_VERSION_0_0_8: Version = Version::new(0, 0, 8);

/// Before this check was introduced, there were already subgraphs in the wild with spec version
/// 0.0.3, due to confusion with the api version. To avoid breaking those, we accept 0.0.3 though it
/// doesn't exist.
Expand Down
14 changes: 14 additions & 0 deletions graph/src/schema/input_schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,20 @@ impl InputSchema {
.map(|fields| fields.contains(&field))
.unwrap_or(false)
}

pub fn has_field_with_name(&self, entity_type: &EntityType, field: &str) -> bool {
let field = self.inner.pool.lookup(field);

match field {
Some(field) => self
.inner
Copy link
Collaborator

Choose a reason for hiding this comment

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

You can just call self.has_field at this point; avoids duplicating logic

.field_names
.get(entity_type)
.map(|fields| fields.contains(&field))
.unwrap_or(false),
None => false,
}
}
}

/// Create a new pool that contains the names of all the types defined
Expand Down
11 changes: 10 additions & 1 deletion runtime/wasm/src/host_exports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,9 +199,18 @@ impl<C: Blockchain> HostExports<C> {
}
}

// Filter out any key-value pairs from 'data' where
// the key (field name) is not defined in the schema for the given entity type.
let filtered_entity_data = data.into_iter().filter(|(k, _)| {
state
.entity_cache
.schema
.has_field_with_name(&key.entity_type, k.as_str())
});

let entity = state
.entity_cache
.make_entity(data.into_iter().map(|(key, value)| (key, value)))
.make_entity(filtered_entity_data.map(|(key, value)| (key, value)))
Copy link
Collaborator

Choose a reason for hiding this comment

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

That map is a noop (was one before, too) and can just be removed

.map_err(|e| HostExportError::Deterministic(anyhow!(e)))?;

let poi_section = stopwatch.start_section("host_export_store_set__proof_of_indexing");
Expand Down