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

Wrong property types issue #970

Merged
merged 22 commits into from
Feb 4, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
7d34b70
Added tests to check deserialization and defaulting behaviour for wro…
Abhijeet-Bentley Jan 17, 2025
b7cc929
Schema is not read if unknown property tags present
Abhijeet-Bentley Jan 17, 2025
43a99c3
Better logic to compare ECXmlVersion and ECVersion Latest
Abhijeet-Bentley Jan 20, 2025
1a1e4cf
General code polishing for test cases
Abhijeet-Bentley Jan 20, 2025
21ab90e
Added tests for Schema Comparer
Abhijeet-Bentley Jan 21, 2025
705129b
Improved test checking logic
Abhijeet-Bentley Jan 21, 2025
d9ea7e3
Removed unnecessary local variable
Abhijeet-Bentley Jan 21, 2025
f172546
Merge branch 'main' into abhi/wrong-property-tags-issue
RohitPtnkr1996 Jan 28, 2025
3d15092
General polishing and minor changes
Abhijeet-Bentley Jan 28, 2025
c36de00
Merge branch 'abhi/wrong-property-tags-issue' of https://github.com/i…
Abhijeet-Bentley Jan 28, 2025
260df91
Adjusted tests for new deserialization behavior
Abhijeet-Bentley Jan 29, 2025
e8959b1
Merge branch 'main' into abhi/wrong-property-tags-issue
Abhijeet-Bentley Jan 30, 2025
9797cc3
Logic fix for checking versions and test changes
Abhijeet-Bentley Jan 30, 2025
dfe206b
Comment edit
Abhijeet-Bentley Jan 30, 2025
c695cb4
Merge branch 'main' into abhi/wrong-property-tags-issue
Abhijeet-Bentley Jan 31, 2025
5a755ad
Comment changes
Abhijeet-Bentley Jan 31, 2025
b455cee
Merge branch 'abhi/wrong-property-tags-issue' of https://github.com/i…
Abhijeet-Bentley Jan 31, 2025
4e7cff3
Merged and fixed two test cases
Abhijeet-Bentley Feb 3, 2025
026c35f
Minor schema change in a test
Abhijeet-Bentley Feb 3, 2025
c182cd9
Test case fix
Abhijeet-Bentley Feb 3, 2025
e083833
Merge branch 'main' into abhi/wrong-property-tags-issue
Abhijeet-Bentley Feb 3, 2025
80e2fa3
Merge branch 'main' into abhi/wrong-property-tags-issue
Abhijeet-Bentley Feb 4, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
117 changes: 112 additions & 5 deletions iModelCore/ECDb/Tests/NonPublished/ECSqlStatementTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1535,6 +1535,113 @@ TEST_F(ECSqlStatementTestFixture, IsNull)
}

//---------------------------------------------------------------------------------------
//@bsimethod
//---------------------------------------------------------------------------------------
TEST_F(ECSqlStatementTestFixture, UseOfWrongPropertyTags_ForAllVersions)
{
ASSERT_EQ(DbResult::BE_SQLITE_OK, SetupECDb("UseOfWrongPropertyTags.ecdb"));

unsigned int ecXmlMajorVersion;
unsigned int ecXmlMinorVersion;
EXPECT_EQ(ECObjectsStatus::Success, ECSchema::ParseECVersion(ecXmlMajorVersion, ecXmlMinorVersion, ECVersion::Latest));

//Below schema uses ECProperty tag for Struct property which is wrong. It should use ECStructProperty tag.
Utf8CP xmlSchema = R"xml(<ECSchema schemaName="TestSchema" alias="ts" version="1.0.0" xmlns="http://www.bentley.com/schemas/Bentley.ECXML.%d.%d">
<ECStructClass typeName="PrimStruct">
<ECProperty propertyName="p2d" typeName="Point2d" />
<ECProperty propertyName="p3d" typeName="Point3d" />
</ECStructClass>
<ECEntityClass typeName="UseOfWrongPropertyTags">
<ECProperty propertyName="Struct" typeName="PrimStruct" />
<ECStructArrayProperty propertyName="Struct_Array" typeName="PrimStruct" />
</ECEntityClass>
</ECSchema>)xml";

//Below schema uses ECProperty tag for Struct property which is wrong. It should use ECStructProperty tag.
//Declaring separate xmlSchema for version 2.0 since, previous versions had different xml schema format.
Utf8CP xmlSchemaFor_V2_0 = R"xml(<ECSchema schemaName="TestSchema" version="1.0" nameSpacePrefix="test" xmlns="http://www.bentley.com/schemas/Bentley.ECXML.2.0">
<ECClass typeName="PrimStruct" isStruct = "true">
<ECProperty propertyName="i" typeName="int" />
<ECArrayProperty propertyName="I" typeName="int" />
</ECClass>
<ECClass typeName="UseOfWrongPropertyTags">
<ECProperty propertyName="Struct" typeName="PrimStruct" />
</ECClass>
</ECSchema>)xml";

//Below schema uses ECProperty tag for Struct property which is wrong. It should use ECStructProperty tag.
//Declaring separate xmlSchema for version 3.0 since, previous versions had different xml schema format.
Utf8CP xmlSchemaFor_V3_0 = R"xml(<ECSchema schemaName="TestSchema" version="1.0" nameSpacePrefix="test" xmlns="http://www.bentley.com/schemas/Bentley.ECXML.3.0">
<ECStructClass typeName="PrimStruct">
<ECProperty propertyName="p2d" typeName="Point2d" />
<ECArrayProperty propertyName="p3d" typeName="Point3d" />
</ECStructClass>
<ECEntityClass typeName="UseOfWrongPropertyTags">
<ECProperty propertyName="Struct" typeName="PrimStruct" />
<ECStructArrayProperty propertyName="Struct_Array" typeName="PrimStruct" />
</ECEntityClass>
</ECSchema>)xml";

for (const auto& [testCaseNumber, majorVersion, minorVersion, xmlSchemaVar, deserializationStatus, importStatus, schemaReadStatus, schemaImportResult] : std::vector<std::tuple<unsigned int, unsigned int, unsigned int, Utf8CP, bool, bool, SchemaReadStatus, SchemaImportResult>>
{
{ 1, 2U, 0U, xmlSchemaFor_V2_0, true , true, SchemaReadStatus::Success, SchemaImportResult::OK }, // Older versions on encountering wrong property type default to a safe type in this case, "string"
{ 2, 3U, 0U, xmlSchemaFor_V3_0, true , true, SchemaReadStatus::Success, SchemaImportResult::OK }, // Older versions on encountering wrong property type default to a safe type in this case, "string"
{ 3, 3U, 1U, xmlSchema, true , true, SchemaReadStatus::Success, SchemaImportResult::OK }, // Older versions on encountering wrong property type default to a safe type in this case, "string"
{ 4, ecXmlMajorVersion, ecXmlMinorVersion, xmlSchema, false , false, SchemaReadStatus::InvalidECSchemaXml, SchemaImportResult::ERROR }, // Current version should always log an error on encountering wrong property type with no deserialization and import
{ 5, ecXmlMajorVersion, ecXmlMinorVersion + 1U, xmlSchema, true , false, SchemaReadStatus::Success, SchemaImportResult::ERROR }, // Future versions should deserialize successfully, default to a safe type but, import should fail
})
{
ECSchemaPtr schema;
const auto context = ECSchemaReadContext::CreateContext();

// Schema should always be deserialized successfully irrespective of the ECXml version
if (deserializationStatus)
{
ASSERT_EQ(schemaReadStatus, ECSchema::ReadFromXmlString(schema, Utf8PrintfString(xmlSchemaVar, majorVersion, minorVersion).c_str(), *context)) << "Test case number : " << testCaseNumber << " failed at deserializing.";
ASSERT_EQ(deserializationStatus, schema.IsValid()) << "Test case number : " << testCaseNumber << " failed due to invalid schemas.";
}
else
{
// But, schemas having unknown wrong property types with ecxml versions belonging to [V3_2, Latest] inclusive, should fail to deserialize
ASSERT_EQ(schemaReadStatus, ECSchema::ReadFromXmlString(schema, Utf8PrintfString(xmlSchemaVar, majorVersion, minorVersion).c_str(), *context)) << "Test case number : " << testCaseNumber << " failed since, a schema with wrong property type shouldn't deserialize for " << ecXmlMajorVersion << "." << ecXmlMinorVersion;
ASSERT_EQ(deserializationStatus, schema.IsValid()) << "Test case number : " << testCaseNumber << " failed since, a schema with wrong property type shouldn't deserialize for " << ecXmlMajorVersion << "." << ecXmlMinorVersion;
}

// Checking if the wrong property types are defaulted to string type
if (deserializationStatus)
{
// Fetching the class pointer
auto classP = schema->GetClassCP("UseOfWrongPropertyTags");
ASSERT_TRUE(classP) << "Test case : " << testCaseNumber << " failed to fetch class pointer.";
// Fetching the property pointer
auto propP = classP->GetPropertyByIndex(0);
ASSERT_TRUE(propP) << "Test case : " << testCaseNumber << " failed to fetch property pointer.";
// Fetching property as primitive property
auto primProp = propP->GetAsPrimitiveProperty();
ASSERT_TRUE(primProp) << "Test case failed : " << testCaseNumber << " failed to fetch the primitive property";
// Checking for equality of primivite property type to string
auto propType = primProp->GetType();
EXPECT_EQ(PRIMITIVETYPE_String, propType) << "Test case failed : " << testCaseNumber << " did not default to string type.";
}


// Schema import should fail when ECXml version of the schema is greater than the current version that the ECDb supports
if (importStatus)
RohitPtnkr1996 marked this conversation as resolved.
Show resolved Hide resolved
{
EXPECT_EQ(schemaImportResult, m_ecdb.Schemas().ImportSchemas(context->GetCache().GetSchemas())) << "Test case number : " << testCaseNumber << " failed to import schema.";
EXPECT_EQ(importStatus, m_ecdb.Schemas().GetSchema("TestSchema") != nullptr)<< "Test case number : " << testCaseNumber << " failed at fetching the schema.";
}
else
{
//But, import should also fail when the schema has wrong property types with ECXml versions belonging to [V3_2, Latest] inclusive
EXPECT_EQ(schemaImportResult, m_ecdb.Schemas().ImportSchemas(context->GetCache().GetSchemas())) << "Test case number : " << testCaseNumber << " failed since, a schema with wrong property type shouldn't import for " << ecXmlMajorVersion << "." << ecXmlMinorVersion;
EXPECT_EQ(importStatus, m_ecdb.Schemas().GetSchema("TestSchema") != nullptr)<< "Test case number : " << testCaseNumber << " failed since, a schema with wrong property type shouldn't import for " << ecXmlMajorVersion << "." << ecXmlMinorVersion;
}

m_ecdb.AbandonChanges();
}
}
//---------------------------------------------------------------------------------------
// @bsimethod
//+---------------+---------------+---------------+---------------+---------------+------
TEST_F(ECSqlStatementTestFixture, pragma_ecdb_version)
Expand Down Expand Up @@ -10524,8 +10631,8 @@ TEST_F(ECSqlStatementTestFixture, AliasedEnumProps)
<ECEntityClass typeName="Foo" >
<ECProperty propertyName="Status" typeName="Status" />
<ECArrayProperty propertyName="Statuses" typeName="Status" />
<ECProperty propertyName="Domain" typeName="Domain" />
<ECArrayProperty propertyName="Domains" typeName="Domain" />
<ECProperty propertyName="Domain" typeName="Domains" />
<ECArrayProperty propertyName="Domains" typeName="Domains" />
</ECEntityClass>
</ECSchema>)xml",
R"xml(<ECSchema schemaName="TestSchema" alias="ts" version="1.0.0" xmlns="http://www.bentley.com/schemas/Bentley.ECXML.3.2">
Expand All @@ -10540,8 +10647,8 @@ TEST_F(ECSqlStatementTestFixture, AliasedEnumProps)
<ECEntityClass typeName="Foo" >
<ECProperty propertyName="Status" typeName="Status" />
<ECArrayProperty propertyName="Statuses" typeName="Status" />
<ECProperty propertyName="Domain" typeName="Domain" />
<ECArrayProperty propertyName="Domains" typeName="Domain" />
<ECProperty propertyName="Domain" typeName="Domains" />
<ECArrayProperty propertyName="Domains" typeName="Domains" />
</ECEntityClass>
</ECSchema>)xml"})
{
Expand Down Expand Up @@ -10580,7 +10687,7 @@ TEST_F(ECSqlStatementTestFixture, AliasedEnumProps)

stmt.Finalize();
CloseECDb();
}
}
}

//---------------------------------------------------------------------------------------
Expand Down
9 changes: 9 additions & 0 deletions iModelCore/ecobjects/src/ECProperty.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,15 @@ SchemaReadStatus ReadTypeNameWithPruning(pugi::xml_node node, ECPropertyP prop,
// must come after prune check to make sure we don't default to string properties that should be pruned
if (setTypeStatus == ECObjectsStatus::ParseError && ignoreParseErrors)
{
// Unknown type(s) encountered in a schema for ECXml versions that are
// 'V3_2 <= ECXml version <= Latest'
// then we return InvalidECSchemaXml as SchemaReadStatus
if (prop->GetClass().GetSchema().OriginalECXmlVersionAtLeast(ECVersion::V3_2) && !prop->GetClass().GetSchema().OriginalECXmlVersionGreaterThan(ECVersion::Latest))
{
LOG.errorv("Invalid ECSchemaXML: The Property '%s.%s' has a type name '%s' that can not be resolved to a known type.",
prop->GetClass().GetFullName(), prop->GetName().c_str(), typeName.c_str());
return SchemaReadStatus::InvalidECSchemaXml;
}
LOG.warningv ("Defaulting the type of ECProperty '%s' to '%s' in reaction to non-fatal parse error.", prop->GetName().c_str(), prop->GetTypeName().c_str());
return SchemaReadStatus::Success;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1003,7 +1003,7 @@ TEST_F(SchemaCopyTest, SuccessfullyCopiesSchemaWithCustomAttributesInSchemaAndCl
<ECCustomAttributes>
<aCustomClass xmlns="testSchema.01.00.00"/>
</ECCustomAttributes>
<ECProperty propertyName="bProperty" typeName="aEntityClass">
<ECProperty propertyName="bProperty" typeName="int">
<ECCustomAttributes>
<cCustomClass xmlns="testSchema.01.00.00"/>
</ECCustomAttributes>
Expand Down
48 changes: 48 additions & 0 deletions iModelCore/ecobjects/test/NonPublished/SchemaComparerTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1121,6 +1121,54 @@ TEST_F(SchemaCompareTest, CompareECClassIdentical)
//----------------------------------------------------------------------------------------
// @bsimethod
//---------------+---------------+---------------+---------------+---------------+--------
TEST_F(SchemaComparerXmlTests, CompareSchemasWithWrongPropertyTags)
{
//Test created for 3.1 version of schema specifically
//to observe defaulting behavior for and when the property types are wrong
bvector<Utf8CP> firstSchemasXml {
R"schema(<?xml version='1.0' encoding='utf-8' ?>
<ECSchema schemaName="TestSchema" alias="ts" version="1.0.0" xmlns="http://www.bentley.com/schemas/Bentley.ECXML.3.1">
<ECStructClass typeName="PrimStruct">
<ECProperty propertyName="p2d" typeName="Point2d" />
<ECProperty propertyName="p3d" typeName="Point3d" />
</ECStructClass>
<ECEntityClass typeName="UseOfWrongPropertyTags">
<ECProperty propertyName="Struct" typeName="PrimStruct" />
<ECStructArrayProperty propertyName="Struct_Array" typeName="PrimStruct" />
</ECEntityClass>
</ECSchema>)schema"
};

LoadSchemasIntoContext(m_firstContext, firstSchemasXml);

bvector<Utf8CP> secondSchemasXml {
R"schema(<?xml version='1.0' encoding='utf-8' ?>
<ECSchema schemaName="TestSchema" alias="ts" version="1.0.0" xmlns="http://www.bentley.com/schemas/Bentley.ECXML.3.1">
<ECStructClass typeName="PrimStruct">
<ECProperty propertyName="p2d" typeName="Point2d" />
<ECProperty propertyName="p3d" typeName="Point3d" />
</ECStructClass>
<ECEntityClass typeName="UseOfWrongPropertyTags">
<ECStructProperty propertyName="Struct" typeName="PrimStruct" />
<ECStructArrayProperty propertyName="Struct_Array" typeName="PrimStruct" />
</ECEntityClass>
</ECSchema>)schema"
};

LoadSchemasIntoContext(m_secondContext, secondSchemasXml);

SchemaComparer comparer;
SchemaDiff changes;
comparer.Compare(changes, m_firstContext->GetCache().GetSchemas(), m_secondContext->GetCache().GetSchemas());

ASSERT_EQ(1, changes.Changes().Count());

// We default to string when the property type is wrong therefore, property change should be observed.
EXPECT_TRUE(changes.Changes()[0].Classes()[0].Properties()[0].IsPrimitive().IsChanged());
}
//----------------------------------------------------------------------------------------
// @bsimethod
//---------------+---------------+---------------+---------------+---------------+--------
TEST_F(SchemaCompareTest, CompareECSchemaClassPropertyDescriptionAgainstNull)
{
CreateFirstSchema();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,8 +175,8 @@ TEST_F(SchemaConverterTests, EmptyStructPropertyRemoval)
<ECStructArrayProperty propertyName="EmptyStructArrProperty" typeName="EmptyStruct"/>
</ECEntityClass>
<ECStructClass typeName="FullStruct">
<ECProperty propertyName="FullProp1" typeName="Type1"/>
<ECProperty propertyName="FullProp2" typeName="Type2"/>
<ECProperty propertyName="FullProp1" typeName="int"/>
<ECProperty propertyName="FullProp2" typeName="int"/>
</ECStructClass>
<ECEntityClass typeName="FullEntity">
<ECStructProperty propertyName="FullStructProperty" typeName="FullStruct"/>
Expand Down