diff --git a/packages/js-dpp/test/unit/dataContract/stateTransition/DataContractUpdateTransition/validation/basic/validateIndicesAreBackwardCompatible.spec.js b/packages/js-dpp/test/unit/dataContract/stateTransition/DataContractUpdateTransition/validation/basic/validateIndicesAreBackwardCompatible.spec.js index d8b4c693150..70030e292d7 100644 --- a/packages/js-dpp/test/unit/dataContract/stateTransition/DataContractUpdateTransition/validation/basic/validateIndicesAreBackwardCompatible.spec.js +++ b/packages/js-dpp/test/unit/dataContract/stateTransition/DataContractUpdateTransition/validation/basic/validateIndicesAreBackwardCompatible.spec.js @@ -18,7 +18,7 @@ describe('validateIndicesAreBackwardCompatible', () => { }); it('should return invalid result if some of unique indices have changed', async () => { - newDocumentsSchema.indexedDocument.indices[0].properties[0].lastName = 'asc'; + newDocumentsSchema.indexedDocument.indices[0].properties[1].firstName = 'desc'; const result = validateIndicesAreBackwardCompatible(oldDocumentsSchema, newDocumentsSchema); @@ -31,7 +31,7 @@ describe('validateIndicesAreBackwardCompatible', () => { }); it('should return invalid result if already defined properties are changed in existing index', async () => { - newDocumentsSchema.indexedDocument.indices[2].properties[0].otherName = 'asc'; + newDocumentsSchema.indexedDocument.indices[2].properties[0].lastName = 'desc'; const result = validateIndicesAreBackwardCompatible(oldDocumentsSchema, newDocumentsSchema); diff --git a/packages/rs-dpp/src/data_contract/state_transition/data_contract_update_transition/validation/basic/validate_indices_are_backward_compatible.rs b/packages/rs-dpp/src/data_contract/state_transition/data_contract_update_transition/validation/basic/validate_indices_are_backward_compatible.rs index a8e17b3a569..6e049fcec61 100644 --- a/packages/rs-dpp/src/data_contract/state_transition/data_contract_update_transition/validation/basic/validate_indices_are_backward_compatible.rs +++ b/packages/rs-dpp/src/data_contract/state_transition/data_contract_update_transition/validation/basic/validate_indices_are_backward_compatible.rs @@ -75,7 +75,7 @@ pub fn validate_indices_are_backward_compatible<'a>( &existing_schema_indices, &name_new_index_map, existing_schema, - ); + )?; if let Some(index) = maybe_wrongly_updated_index { result.add_error(BasicError::DataContractInvalidIndexDefinitionUpdateError { document_type: document_type.to_owned(), @@ -211,7 +211,7 @@ fn get_wrongly_updated_non_unique_index<'a>( existing_schema_indices: &'a [Index], new_indices: &'a BTreeMap, existing_schema: &'a JsonSchema, -) -> Option<&'a Index> { +) -> Result, ProtocolError> { // Checking every existing non-unique index, and it's respective new index // if they are changed per spec for index_definition in existing_schema_indices.iter().filter(|i| !i.unique) { @@ -223,20 +223,39 @@ fn get_wrongly_updated_non_unique_index<'a>( if new_index_definition.properties[0..index_properties_len] != index_definition.properties { - return Some(index_definition); + return Ok(Some(index_definition)); } // Check if the rest of new indexes are defined in the existing schema for property in new_index_definition.properties[index_definition.properties.len()..].iter() { - if existing_schema.get_value(&property.name).is_ok() { - return Some(index_definition); + if let Ok(indices) = existing_schema.get_value("indices") { + let indices_array = indices.as_array().ok_or_else(|| { + ProtocolError::ParsingError( + "Error parsing schema: indices is not an array".to_string(), + ) + })?; + + for index in indices_array { + let properties_value = index.get_value("properties")?; + let properties_array = properties_value.as_array().ok_or_else(|| { + ProtocolError::ParsingError( + "Error parsing schema: properties is not an array".to_string(), + ) + })?; + + for property_to_check in properties_array { + if property_to_check.get_value(&property.name).is_ok() { + return Ok(Some(index_definition)); + } + } + } } } } } - None + Ok(None) } #[cfg(test)] diff --git a/packages/rs-dpp/src/tests/data_contract/state_transition/data_contract_update_transition/validation/basic/validate_indices_are_backward_compatible_spec.rs b/packages/rs-dpp/src/tests/data_contract/state_transition/data_contract_update_transition/validation/basic/validate_indices_are_backward_compatible_spec.rs index 1b7c69efe8b..af4fa45a2ca 100644 --- a/packages/rs-dpp/src/tests/data_contract/state_transition/data_contract_update_transition/validation/basic/validate_indices_are_backward_compatible_spec.rs +++ b/packages/rs-dpp/src/tests/data_contract/state_transition/data_contract_update_transition/validation/basic/validate_indices_are_backward_compatible_spec.rs @@ -130,14 +130,15 @@ fn should_return_invalid_result_if_non_unique_index_update_failed_due_to_changed } #[test] -fn should_return_invalid_result_if_non_unique_index_update_failed_due_old_properties_used() { +fn should_return_invalid_result_if_already_indexed_properties_are_added_to_existing_index() { let TestData { old_documents_schema, mut new_documents_schema, .. } = setup_test(); - new_documents_schema.get_mut("indexedDocument").unwrap()["indices"][2]["properties"][0] = - json!({ "firstName": "asc" }); + new_documents_schema.get_mut("indexedDocument").unwrap()["indices"][2]["properties"] + .push(json!({ "firstName": "asc" })) + .expect("the new index property should be added"); let result = validate_indices_are_backward_compatible( old_documents_schema.iter(), diff --git a/packages/wasm-dpp/test/unit/dataContract/stateTransition/DataContractUpdateTransition/validation/basic/validateIndicesAreBackwardCompatible.spec.js b/packages/wasm-dpp/test/unit/dataContract/stateTransition/DataContractUpdateTransition/validation/basic/validateIndicesAreBackwardCompatible.spec.js index d9254e4f340..367084c72d8 100644 --- a/packages/wasm-dpp/test/unit/dataContract/stateTransition/DataContractUpdateTransition/validation/basic/validateIndicesAreBackwardCompatible.spec.js +++ b/packages/wasm-dpp/test/unit/dataContract/stateTransition/DataContractUpdateTransition/validation/basic/validateIndicesAreBackwardCompatible.spec.js @@ -23,31 +23,13 @@ describe('validateIndicesAreBackwardCompatible', () => { const oldDataContract = getDataContractFixture(); const newDataContract = getDataContractFixture(); - newDataContract.getDocumentSchema('indexedDocument').properties.otherName = { - type: 'string', - }; - - newDataContract.getDocumentSchema('indexedDocument').indices.push({ - name: 'index42', - unique: false, - properties: [ - { otherName: 'asc' }, - ], - }); - - newDataContract.getDocumentSchema('indexedDocument').indices.push({ - name: 'index42', - properties: [ - { otherName: 'asc' }, - ], - }); - oldDocumentsSchema = oldDataContract.getDocuments(); newDocumentsSchema = newDataContract.getDocuments(); }); it('should return invalid result if some of unique indices have changed', async () => { newDocumentsSchema.indexedDocument.indices[0].properties[1].firstName = 'desc'; + const result = validateIndicesAreBackwardCompatible(oldDocumentsSchema, newDocumentsSchema); expect(result.isValid()).to.be.false(); @@ -58,7 +40,7 @@ describe('validateIndicesAreBackwardCompatible', () => { expect(error.getIndexName()).to.equal(newDocumentsSchema.indexedDocument.indices[0].name); }); - it('should return invalid result if non-unique index update failed due to changed old properties', async () => { + it('should return invalid result if already defined properties are changed in existing index', async () => { newDocumentsSchema.indexedDocument.indices[2].properties[0].lastName = 'desc'; const result = validateIndicesAreBackwardCompatible(oldDocumentsSchema, newDocumentsSchema); @@ -71,24 +53,17 @@ describe('validateIndicesAreBackwardCompatible', () => { expect(error.getIndexName()).to.equal(newDocumentsSchema.indexedDocument.indices[2].name); }); - it('should return invalid result if non-unique index update failed due old properties used', async () => { - newDocumentsSchema.indexedDocument.indices.push({ - name: 'oldFieldIndex', - properties: [ - { - otherProperty: 'asc', - }, - ], - }); + it('should return invalid result if already indexed properties are added to existing index', async () => { + newDocumentsSchema.indexedDocument.indices[2].properties.push({ firstName: 'asc' }); const result = validateIndicesAreBackwardCompatible(oldDocumentsSchema, newDocumentsSchema); expect(result.isValid()).to.be.false(); - const error = result.getErrors()[0]; + const [error] = result.getErrors(); expect(error).to.be.an.instanceOf(DataContractInvalidIndexDefinitionUpdateError); - expect(error.getIndexName()).to.equal('oldFieldIndex'); + expect(error.getIndexName()).to.equal(newDocumentsSchema.indexedDocument.indices[2].name); }); it('should return invalid result if one of new indices contains old properties in the wrong order', async () => { @@ -130,6 +105,25 @@ describe('validateIndicesAreBackwardCompatible', () => { expect(error.getIndexName()).to.equal('index_other'); }); + it('should return invalid result if existing property was used in a new index', async () => { + newDocumentsSchema.indexedDocument.indices.push({ + name: 'oldFieldIndex', + properties: [ + { + otherProperty: 'asc', + }, + ], + }); + + const result = validateIndicesAreBackwardCompatible(oldDocumentsSchema, newDocumentsSchema); + + expect(result.isValid()).to.be.false(); + + const [error] = result.getErrors(); + expect(error).to.be.an.instanceOf(DataContractInvalidIndexDefinitionUpdateError); + expect(error.getIndexName()).to.equal('oldFieldIndex'); + }); + it('should return valid result if indices are not changed', async () => { const result = validateIndicesAreBackwardCompatible(oldDocumentsSchema, newDocumentsSchema);