Schema ADO #580
Schema ADO #580
Conversation
WalkthroughThe pull request introduces a new configuration file Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Outside diff range and nitpick comments (10)
contracts/modules/andromeda-schema/examples/schema.rs (2)
4-9: LGTM: Correct usage of write_api! macro.The
write_api!macro is correctly used to define the contract's API, specifying the necessary message types (InstantiateMsg, QueryMsg, and ExecuteMsg) as per CosmWasm standards. This implementation aligns well with the PR objectives for creating a Schema ADO.Consider adding a brief comment above the
write_api!macro to explain its purpose, e.g.:// Generate JSON schema for contract's messages write_api! { // ... (existing code) }This would enhance code readability and maintainability.
1-10: Overall assessment: Well-structured and aligned with PR objectives.This file successfully defines the API for the Schema ADO as a CosmWasm smart contract. It correctly imports necessary types, uses the
write_api!macro appropriately, and follows CosmWasm best practices. The implementation aligns well with the PR objectives of creating a Schema ADO with instantiate, query, and execute functionalities.As the project grows, consider creating a separate module or file for common message types if they are shared across multiple contracts. This would enhance modularity and reduce code duplication.
packages/andromeda-modules/src/schema.rs (4)
4-8: LGTM: InstantiateMsg structure is well-defined.The
InstantiateMsgstruct is correctly defined with the necessary field for schema initialization. The use of#[andr_instantiate]and#[cw_serde]macros is appropriate for Andromeda instantiation and CosmWasm serialization.Consider adding input validation to ensure that
schema_json_stringis a valid JSON schema string during instantiation. This could prevent issues with invalid schemas being stored.
10-14: LGTM: ExecuteMsg enum is correctly structured.The
ExecuteMsgenum is well-defined with theUpdateSchemavariant, allowing for schema updates as per the PR objectives. The use of#[andr_exec]and#[cw_serde]macros is appropriate.Consider the following improvements:
- Add input validation for
new_schema_json_stringto ensure it's a valid JSON schema before updating.- Consider adding more variants to
ExecuteMsgfor future extensibility, such as operations for schema versioning or deletion if applicable to your use case.
16-27: LGTM: QueryMsg and ValidateDataResponse structures are well-defined.The
QueryMsgenum andValidateDataResponsestruct are correctly implemented to handle data validation queries. The use of appropriate macros ensures proper integration with Andromeda and CosmWasm systems.Consider the following enhancements:
- Extend
ValidateDataResponseto include more detailed information, especially for validation failures. For example:pub struct ValidateDataResponse { pub is_valid: bool, pub errors: Option<Vec<String>>, }- Add a query variant to retrieve the current schema, which could be useful for clients:
#[returns(SchemaResponse)] GetSchema {},- Consider adding pagination support if you expect to handle large amounts of validation errors in the future.
1-27: Overall, the implementation aligns well with the PR objectives, but there's room for improvement.The schema.rs file successfully introduces the basic structure for schema handling and validation in the Andromeda modules. It covers instantiation, schema updates, and data validation as outlined in the PR objectives. However, consider the following improvements to enhance robustness and functionality:
- Implement input validation for JSON schema strings in both
InstantiateMsgandExecuteMsg::UpdateSchema.- Extend error handling in
ValidateDataResponseto provide more detailed feedback on validation failures.- Add a query to retrieve the current schema.
- Consider future extensibility by preparing for additional execute and query message variants.
These enhancements will contribute to a more robust and user-friendly schema validation system.
contracts/modules/andromeda-schema/src/testing/mock.rs (2)
16-28: LGTM: Well-structured initialization function with a minor suggestion.The
proper_initializationfunction correctly sets up the mock environment and initializes the contract. It uses appropriate mock functions and structures.Consider adding error handling for the
instantiatecall:- let res = instantiate(deps.as_mut(), env, info.clone(), msg).unwrap(); + let res = instantiate(deps.as_mut(), env, info.clone(), msg).expect("Instantiation failed");This change would provide a more informative error message if instantiation fails.
42-51: LGTM: Query validate data function is well-implemented, with room for improvement.The
query_validate_datafunction correctly constructs theQueryMsg::ValidateDataand calls thequeryfunction with appropriate parameters. The handling of the query result is good, converting successful responses from JSON.Consider improving error handling to provide more context:
pub fn query_validate_data( deps: Deps, data: String, ) -> Result<ValidateDataResponse, ContractError> { let res = query(deps, mock_env(), QueryMsg::ValidateData { data }); match res { - Ok(res) => Ok(from_json(res).unwrap()), - Err(err) => Err(err), + Ok(res) => from_json(res).map_err(|e| ContractError::Std(e.into())), + Err(err) => Err(ContractError::Std(anyhow::anyhow!("Query failed: {}", err))), } }This change would provide more informative error messages and handle potential JSON parsing errors.
contracts/modules/andromeda-schema/src/query.rs (1)
10-31: Improve error messages for better clarityThe current error message
"Validation Error"returned when the schema is invalid is generic and might not provide sufficient information for debugging.Consider providing more specific error messages to aid in debugging:
Similarly, you might want to enhance the error handling for data validation errors to include details about why the data is invalid.
contracts/modules/andromeda-schema/src/execute.rs (1)
38-38: Avoid unnecessary cloning of the sender addressThe
clone()method onctx.info.sendermay not be necessary if you can use a reference instead. Reducing unnecessary cloning can improve performance and reduce memory usage.Consider modifying the code as follows:
-let sender = ctx.info.sender.clone(); +let sender = &ctx.info.sender;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (14)
- contracts/modules/andromeda-schema/.cargo/config (1 hunks)
- contracts/modules/andromeda-schema/Cargo.toml (1 hunks)
- contracts/modules/andromeda-schema/examples/schema.rs (1 hunks)
- contracts/modules/andromeda-schema/src/contract.rs (1 hunks)
- contracts/modules/andromeda-schema/src/execute.rs (1 hunks)
- contracts/modules/andromeda-schema/src/lib.rs (1 hunks)
- contracts/modules/andromeda-schema/src/mock.rs (1 hunks)
- contracts/modules/andromeda-schema/src/query.rs (1 hunks)
- contracts/modules/andromeda-schema/src/state.rs (1 hunks)
- contracts/modules/andromeda-schema/src/testing/mock.rs (1 hunks)
- contracts/modules/andromeda-schema/src/testing/mod.rs (1 hunks)
- contracts/modules/andromeda-schema/src/testing/tests.rs (1 hunks)
- packages/andromeda-modules/src/lib.rs (1 hunks)
- packages/andromeda-modules/src/schema.rs (1 hunks)
✅ Files skipped from review due to trivial changes (3)
- contracts/modules/andromeda-schema/.cargo/config
- contracts/modules/andromeda-schema/src/state.rs
- contracts/modules/andromeda-schema/src/testing/mod.rs
🧰 Additional context used
🔇 Additional comments (21)
packages/andromeda-modules/src/lib.rs (1)
5-5: LGTM! New schema module added successfully.The addition of the
schemamodule aligns with the PR objectives to introduce a Schema ADO. The placement maintains the alphabetical order of the module declarations, adhering to the existing structure.Let's verify the implementation of the new
schemamodule:contracts/modules/andromeda-schema/src/lib.rs (4)
1-4: LGTM: Core modules are well-structured.The core modules (
contract,execute,query, andstate) are clearly defined and follow a logical structure for a smart contract. This separation of concerns will enhance maintainability and readability of the code.
5-6: Good practice: Separate testing module.The
testingmodule is correctly configured to be compiled only for tests. This is a good practice as it keeps test-specific code separate from the main implementation.
8-9: Well-configured mock module for testing.The
mockmodule is properly configured to be compiled only for non-wasm32 architectures with the "testing" feature enabled. This setup allows for effective mocking in test environments without affecting the production build.
1-9: Overall structure aligns with best practices.The module structure in this file follows Rust best practices for organizing smart contract code. It separates core functionality, testing, and mocking into distinct modules, which will facilitate easier maintenance and testing of the contract.
However, to ensure completeness:
Let's verify if all necessary modules for a typical smart contract are present:
This script will help us confirm if any essential modules are missing from the contract structure.
contracts/modules/andromeda-schema/examples/schema.rs (2)
1-2: LGTM: Imports are correct and aligned with PR objectives.The import statements are appropriate for defining a CosmWasm smart contract API using the Schema ADO. They correctly import the necessary message types and the
write_apimacro.
3-3: LGTM: Main function declaration is correct.The
mainfunction is appropriately declared as the entry point for generating the API schema.packages/andromeda-modules/src/schema.rs (1)
1-2: LGTM: Appropriate imports for schema functionality.The imports from
andromeda_stdandcosmwasm_schemaare correctly used to bring in necessary macros and types for schema handling and serialization.contracts/modules/andromeda-schema/Cargo.toml (3)
1-5: LGTM: Package metadata is well-defined.The package metadata is correctly specified and aligns with the PR objectives. The use of Rust edition 2021 and a recent Rust version (1.75.0) is commendable, as it allows for leveraging newer language features and improvements.
7-8: LGTM: Appropriate crate types specified.The crate types are correctly set to ["cdylib", "rlib"], which is the recommended configuration for CosmWasm contracts. This allows the crate to be compiled as both a WebAssembly dynamic library and a Rust library.
10-15: LGTM: Well-defined features for various use cases.The features section is well-structured:
- The "backtraces" feature is useful for debugging.
- The "library" feature provides flexibility in crate usage.
- The "testing" feature includes necessary dependencies for comprehensive testing.
These features align with best practices and provide good flexibility for development and testing.
contracts/modules/andromeda-schema/src/testing/mock.rs (2)
1-14: LGTM: Imports and type alias are well-structured.The imports cover all necessary components from andromeda_modules, andromeda_std, and cosmwasm_std. The
MockDepstype alias is a good practice for simplifying the usage ofOwnedDepswith mock components.
30-40: LGTM: Update schema function is well-implemented.The
update_schemafunction correctly constructs theExecuteMsg::UpdateSchemaand calls theexecutefunction with appropriate parameters. The use ofmock_infofor creating message info is correct, and the return typeResult<Response, ContractError>is appropriate for an execute function.contracts/modules/andromeda-schema/src/execute.rs (2)
13-20: Review the placement ofcall_actionfor correct execution flowCurrently,
call_actionis invoked before handling theUpdateSchemaexecution message. Depending on the intended behavior, you might want to ensure thatcall_actionis called after the message handling to correctly process any actions that are a result of the execution.Please confirm whether
call_actionshould be invoked before or after processing the execution message. If it should be after, consider rearranging the code as follows:pub fn handle_execute(mut ctx: ExecuteContext, msg: ExecuteMsg) -> Result<Response, ContractError> { - let action_response = call_action( - &mut ctx.deps, - &ctx.info, - &ctx.env, - &ctx.amp_ctx, - msg.as_ref(), - )?; let res = match msg { ExecuteMsg::UpdateSchema { new_schema_json_string, } => execute_update_schema(ctx, new_schema_json_string), _ => ADOContract::default().execute(ctx, msg), }?; + let action_response = call_action( + &mut ctx.deps, + &ctx.info, + &ctx.env, + &ctx.amp_ctx, + msg.as_ref(), + )?; Ok(res .add_submessages(action_response.messages) .add_attributes(action_response.attributes) .add_events(action_response.events)) }
25-26: Ensure compatibility withADOContract::executemethod signatureWhen delegating to
ADOContract::execute, verify that themsgbeing passed is compatible with the expected message type ofADOContract. SinceADOContract::executemight expect a different message type, ensure thatExecuteMsgcan be properly handled or converted.Run the following script to check for method signatures and compatibility:
contracts/modules/andromeda-schema/src/contract.rs (3)
35-36: EnsureCONTRACT_NAMEandCONTRACT_VERSIONare correctly setThe
ado_typeandado_versionfields are set usingCONTRACT_NAME.to_string()andCONTRACT_VERSION.to_string(). Please verify thatCONTRACT_NAMEandCONTRACT_VERSIONaccurately reflect the intended contract name and version.
57-61: Confirm allExecuteMsgvariants are properly handledIn the
executefunction, ensure that all variants ofExecuteMsgare adequately covered. This guarantees that no execution messages are missed and that the contract behaves as expected for each possible message.
73-75: Verify migration logic updates the contract versionIn the
migratefunction, make sure that the contract's version is updated correctly and any necessary state migrations are performed to maintain compatibility with the new version.contracts/modules/andromeda-schema/src/mock.rs (1)
12-13: Struct and macro definition are well-implementedThe
MockSchemastruct and the usage of themock_ado!macro are correctly defined.contracts/modules/andromeda-schema/src/testing/tests.rs (2)
306-342: Confirm theupdate_schemafunction updates the schema correctlyThe
test_update_schemafunction updates the schema and then tests validation with both invalid and valid data against the new schema. This ensures that the schema update mechanism works as expected and that data validation aligns with the updated schema.
49-82:⚠️ Potential issueEnsure mutually exclusive options in
oneOfsubschemasIn the definition of
"component_type", theoneOfkeyword is used with two subschemas: one for"new"and one for"symlink". To enforce that an instance must match exactly one of these subschemas, ensure that the properties are required and that"additionalProperties": falseis specified within each subschema.You can verify the exclusivity by testing an instance that includes both
"new"and"symlink"and confirming it fails validation. Additionally, consider adding"additionalProperties": falseto prevent unspecified properties."oneOf": [ { + "additionalProperties": false, "properties": { "new": { "type": "string" } }, "required": [ "new" ], "title": "New", "type": "object" }, { + "additionalProperties": false, "properties": { "symlink": { "type": "string" } }, "required": [ "symlink" ], "title": "Symlink", "type": "object" } ]
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
CHANGELOG.md (2)
31-31: LGTM! The changelog entry is correctly formatted and placed.The new entry for the Schema ADO is consistent with the PR objectives and follows the established format for the changelog. The pull request link is correct and matches the PR number (580) mentioned in the PR objectives.
However, for completeness, consider adding a brief description of the Schema ADO's functionality, similar to other entries in the changelog. For example:
- Schema ADO [(#580)](https://github.com/andromedaprotocol/andromeda-core/pull/580) + Schema ADO: Added ability to store and validate data against JSON schemas [(#580)](https://github.com/andromedaprotocol/andromeda-core/pull/580)
Line range hint
1-31: Consider enhancing the changelog for better clarity and completeness.The changelog is well-structured and follows the Keep a Changelog format. However, consider the following suggestions to improve its overall quality:
If this is intended to be part of a new release, consider adding a version number and release date to the "Unreleased" section.
Ensure that all relevant changes related to the Schema ADO are captured. This might include:
- Any modifications to existing ADOs to work with the new Schema ADO.
- Updates to documentation or examples that showcase the use of the Schema ADO.
For consistency with other entries, consider expanding the Schema ADO entry to provide more context about its functionality and purpose.
These enhancements will provide users with a more comprehensive understanding of the changes and their impact on the project.
| new_schema_json_string, | ||
| }; | ||
| if let Some(funds) = funds { | ||
| app.execute_contract(sender, self.addr().clone(), &msg, &[funds]) |
There was a problem hiding this comment.
nit: conditionally build the funds vector and call this method once
There was a problem hiding this comment.
Sure, I resolved it.
| })?; | ||
|
|
||
| if config.validate(&data_instance).is_ok() { | ||
| Ok(ValidateDataResponse { is_valid: true }) |
There was a problem hiding this comment.
| Ok(ValidateDataResponse { is_valid: true }) | |
| Ok(ValidateDataResponse { is_valid: config.validate(&data_instance).is_ok() }) |
Remove the if/else statement
There was a problem hiding this comment.
I think we're actually losing a potential error here. We can tell the user if the data is valid or not but we can't tell them what's wrong with it when it isn't valid.
There was a problem hiding this comment.
For this, we can use Result<(), ContractError> return value instead of true/false...
In your opinion?
There was a problem hiding this comment.
I think we can return a response but yes we should bubble up any errors from config.validate.
There was a problem hiding this comment.
Resolved this.
There was a problem hiding this comment.
And also I implemented as we can show what's wrong with it when it isn't valid.
| }"#; | ||
|
|
||
| #[test] | ||
| fn test_valid_data_against_schema_instantiation_msg_schema() { |
There was a problem hiding this comment.
This should use a test cases structure
There was a problem hiding this comment.
I resolved it.
| } | ||
|
|
||
| #[test] | ||
| fn test_invalid_data_against_schema_instantiation_msg_schema() { |
There was a problem hiding this comment.
I resolved it.
| } | ||
|
|
||
| #[test] | ||
| fn test_valid_data_against_app_contract_schema() { |
There was a problem hiding this comment.
I resolved it.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (3)
contracts/modules/andromeda-schema/src/testing/tests.rs (3)
3-27: Consider adding an$idfield to the schemaThe schema is well-structured, but it's missing an
$idfield. Adding an$idfield can improve the schema's reusability and make it easier to reference in other schemas.Consider adding an
$idfield like this:{ "$schema": "http://json-schema.org/draft-07/schema#", + "$id": "https://andromeda.com/schemas/schema-ado-instantiation-msg.json", "title": "InstantiateMsg", ... }
201-224: LGTM with suggestion: Valid test case for app contract schemaThis test function correctly validates a proper app contract instantiation message against the defined schema. It covers the essential fields and structure.
Consider adding more test cases to cover different valid scenarios, such as:
- Using a symlink instead of a new binary
- Including optional fields like
chain_infoandowner- Testing multiple app components
This will ensure a more comprehensive coverage of valid inputs for the app contract schema.
305-342: LGTM with suggestion: Test case for updating schemaThis test function effectively demonstrates the ability to update the schema and verifies that the new schema is correctly applied for validation. It covers both invalid and valid data scenarios after the update.
Consider adding the following improvements to make the test more robust:
- Test a valid case against the original schema before updating.
- Test an invalid case against the new schema that would have been valid in the old schema.
- Add assertions to verify that the update operation itself was successful.
Example:
// Test valid data against original schema let original_valid_data = r#" { "app_components": [...], "kernel_address": "test_kernel", "name": "test_contract" }"#; let original_valid_query_res = query_validate_data(deps.as_ref(), original_valid_data.to_string()).unwrap(); assert!(original_valid_query_res.is_valid); // Update schema let update_result = update_schema( deps.as_mut(), info.sender.as_ref(), SCHEMA_ADO_INSTANTIATION_MSG_SCHEMA.to_string(), ); assert!(update_result.is_ok()); // Rest of the existing test cases...These additions will provide a more comprehensive test of the schema update functionality.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- contracts/modules/andromeda-schema/src/contract.rs (1 hunks)
- contracts/modules/andromeda-schema/src/execute.rs (1 hunks)
- contracts/modules/andromeda-schema/src/query.rs (1 hunks)
- contracts/modules/andromeda-schema/src/testing/tests.rs (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- contracts/modules/andromeda-schema/src/query.rs
🧰 Additional context used
🔇 Additional comments (10)
contracts/modules/andromeda-schema/src/execute.rs (4)
1-11: LGTM: Import statements are appropriate and concise.The import statements cover all necessary modules and types required for the implementation of the Andromeda schema execution logic.
13-33: LGTM: Well-structured message handling implementation.The
handle_executefunction effectively processes different message types:
- It first calls
call_actionto handle any associated actions.- Uses pattern matching to route
UpdateSchemamessages to the specific handler.- Defaults to
ADOContract::default().executefor other message types, providing extensibility.- Properly combines the action response with the main execution response.
This structure allows for easy addition of new message types in the future.
35-59: LGTM: Robust implementation with proper authorization and error handling.The
execute_update_schemafunction effectively addresses previous review comments:
- Implements proper access control using
is_owner_or_operator.- Handles JSON parsing errors, preventing storage of invalid schemas.
Additional positive aspects:
- Use of
ensure!macro for clear authorization checks.- Appropriate error handling for JSON parsing.
- Correct storage of the schema as a JSON object for flexibility.
This implementation provides a secure and robust way to update the schema.
1-59: Overall: Well-implemented execution logic for the Andromeda schema module.This file demonstrates a robust implementation of the execution logic for the Andromeda schema module. Key strengths include:
- Clear and extensible message handling in
handle_execute.- Secure schema update process with proper authorization and error handling in
execute_update_schema.- Addressed previous review comments regarding access control and JSON parsing.
The code is well-structured, follows best practices, and provides a solid foundation for the schema module's functionality.
contracts/modules/andromeda-schema/src/testing/tests.rs (3)
151-166: LGTM: Valid test case for schema instantiation messageThis test function correctly validates a proper schema instantiation message against the defined schema. It covers the essential fields and includes a null value for the optional
ownerfield, which is a good edge case to test.
168-199: LGTM: Comprehensive invalid test cases for schema instantiation messageThis test function effectively covers multiple invalid scenarios for the schema instantiation message:
- Missing required fields
- Invalid type for a field
- Extra properties not allowed
These cases ensure that the schema validation is working correctly for various types of invalid input.
226-303: LGTM: Comprehensive invalid test cases for app contract schemaThis test function effectively covers multiple invalid scenarios for the app contract schema:
- Missing required field (
app_components)- Invalid type for a field (
name)- Invalid structure for
component_type- Additional properties in
component_type- Conflicting component types (both
newandsymlinkpresent)These cases ensure that the schema validation is working correctly for various types of invalid input, covering both structural and logical constraints of the app contract schema.
contracts/modules/andromeda-schema/src/contract.rs (3)
23-54: Instantiation logic is correctly implementedThe
instantiatefunction properly initializes the contract, sets up the ADOContract, and stores the schema after validating it. The error handling for invalid JSON is appropriate.
63-70: Execution logic is properly handledThe
executefunction correctly processes theExecuteMsg. It neatly delegatesAMPReceivemessages to theADOContract'sexecute_amp_receivemethod and routes other messages tohandle_execute, ensuring modular and maintainable code.
72-78: Query function implementation is acceptableThe
queryfunction correctly matches onQueryMsg::ValidateDataand handles it appropriately. The use ofencode_binaryensures that the response is correctly formatted.
…or and call app.execute_contract once
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (1)
Cargo.toml (1)
60-60: Consider updatingjsonschema-validto the latest version.The addition of
jsonschema-validaligns well with the PR objectives for implementing schema validation. However, version 0.5.2 is slightly outdated. Consider updating to the latest version (0.5.3 as of October 2024) to ensure you have the most recent bug fixes and improvements.You can update the dependency by changing the line to:
jsonschema-valid = { version = "0.5.3" }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
- Cargo.toml (1 hunks)
- contracts/modules/andromeda-schema/Cargo.toml (1 hunks)
- contracts/modules/andromeda-schema/src/mock.rs (1 hunks)
- contracts/modules/andromeda-schema/src/query.rs (1 hunks)
- contracts/modules/andromeda-schema/src/testing/tests.rs (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- contracts/modules/andromeda-schema/Cargo.toml
- contracts/modules/andromeda-schema/src/mock.rs
🧰 Additional context used
🔇 Additional comments (6)
Cargo.toml (2)
Line range hint
1-61: Overall changes to Cargo.toml look good.The additions of
jsonschema-validandserde_jsondependencies align with the PR objectives for implementing the Schema ADO. The rest of the file, including workspace configuration and other dependencies, remains unchanged, which is appropriate.Once the version updates and clarifications suggested in the previous comments are addressed, these changes to Cargo.toml will be ready for merging.
61-61: Clarify the need forserde_jsonand update to the correct latest version.The addition of
serde_jsonis not directly mentioned in the PR objectives. Could you please clarify its intended use in the context of this PR? Is it being used in conjunction with the mentioned cw-json package or for other JSON-related operations in the Schema ADO?Also, the specified version (1.0.128) seems to be higher than the latest available version. This might be a typo. The latest version as of October 2024 is 1.0.108.
Please update the dependency to the correct latest version:
serde_json = { version = "1.0.108" }To verify the latest version and ensure we're using the correct one, you can run the following command:
This will display the latest version of serde_json available on crates.io.
✅ Verification successful
serde_jsondependency version is up to date. No changes required.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the latest version of serde_json cargo search serde_json --limit 1Length of output: 108
Script:
#!/bin/bash # Description: Fetch the latest version of serde_json from crates.io using curl and jq curl -s https://crates.io/api/v1/crates/serde_json | jq -r '.crate.max_version'Length of output: 87
contracts/modules/andromeda-schema/src/testing/tests.rs (3)
1-28: LGTM: Imports and SCHEMA_ADO_INSTANTIATION_MSG_SCHEMA definitionThe imports and the SCHEMA_ADO_INSTANTIATION_MSG_SCHEMA definition look good. The schema is well-structured and follows JSON Schema conventions.
152-189: LGTM: Test cases for SCHEMA_ADO_INSTANTIATION_MSG_SCHEMAThe test cases for SCHEMA_ADO_INSTANTIATION_MSG_SCHEMA are well-structured and comprehensive. They cover both valid and invalid scenarios, ensuring robust validation of the schema.
191-283: LGTM: Test cases for APP_CONTRACT_INSTANTIATION_MSG_SCHEMA and update_schemaThe test cases for APP_CONTRACT_INSTANTIATION_MSG_SCHEMA validation and schema update functionality are well-designed and comprehensive. They cover various scenarios, including valid and invalid cases, ensuring robust testing of the schema validation and update processes.
contracts/modules/andromeda-schema/src/query.rs (1)
9-28: LGTM!The
validate_datafunction effectively loads the schema, parses the input data, and validates it against the schema. The logic is clear, and error handling is appropriately managed.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
contracts/modules/andromeda-schema/src/testing/tests.rs (1)
212-304: LGTM with suggestion: Comprehensive test cases for APP_CONTRACT_INSTANTIATION_MSG_SCHEMA and update_schemaThe test cases for
APP_CONTRACT_INSTANTIATION_MSG_SCHEMAandupdate_schemaare well-structured and cover important scenarios. The use of thetest_casemacro is effective.Suggestion for improvement:
Consider adding a test case forAPP_CONTRACT_INSTANTIATION_MSG_SCHEMAthat includes thechain_infofield, as it's defined in the schema but not covered in the current test cases.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- contracts/modules/andromeda-schema/src/query.rs (1 hunks)
- contracts/modules/andromeda-schema/src/testing/tests.rs (1 hunks)
- packages/andromeda-modules/src/schema.rs (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- contracts/modules/andromeda-schema/src/query.rs
- packages/andromeda-modules/src/schema.rs
🧰 Additional context used
🔇 Additional comments (2)
contracts/modules/andromeda-schema/src/testing/tests.rs (2)
1-29: LGTM: Imports and SCHEMA_ADO_INSTANTIATION_MSG_SCHEMA definitionThe imports are appropriate for the file's purpose, and the
SCHEMA_ADO_INSTANTIATION_MSG_SCHEMAis well-structured, following JSON Schema standards.
153-210: LGTM: Comprehensive test cases for SCHEMA_ADO_INSTANTIATION_MSG_SCHEMAThe test cases for
SCHEMA_ADO_INSTANTIATION_MSG_SCHEMAare well-structured and cover important validation scenarios, including:
- Valid schema
- Missing required fields
- Invalid types
- Extra properties
The
test_instantiation_schema_validation_with_errorsfunction is implemented correctly to handle these test cases.
Motivation
Schema ADO stores a schema and has a query that can validate a data against the schema
https://json-schema.org/overview/what-is-jsonschema
Implementation
Instantiation
Schema JSON string
Execute
Update Schema - Update Schema JSON String
Query
Validate - Validate Data Against Schema
https://github.com/SlayerAnsh/cw-json
This package allows one to access json values with syntax commonly used in lodash get() method. You can use this to handle json traversal
Testing
Unit test case were added.
Version Changes
Current version is set as 1.0.0
Notes
This ADO is associated with a form ADO that can receive submissions for a form. The form will be a schema ADO that can validate the submission against a valid JSON schema. The Form ADO will be generated.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation