feat(rs-dpp): validate indices are backwards compatible backport

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);
 

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<IndexName, Index>,
     existing_schema: &'a JsonSchema,
-) -> Option<&'a Index> {
+) -> Result<Option<&'a Index>, 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)]

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(),

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);