-
Notifications
You must be signed in to change notification settings - Fork 310
Fix many-to-many relationship queries with custom schema linking objects #3106
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
base: main
Are you sure you want to change the base?
Changes from all commits
0f5670d
ffcb725
e54990c
0d8203e
009e1da
0f2a111
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -437,6 +437,115 @@ string relationshipEntity | |
| configValidator.ValidateRelationships(runtimeConfig, _metadataProviderFactory.Object); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Test method to verify that many-to-many relationships work correctly when the linking object | ||
| /// is in a custom schema (not dbo). This test validates that schema names are correctly compared | ||
| /// using case-insensitive comparison, which is important for SQL Server where schema names are | ||
| /// case-insensitive. | ||
| /// </summary> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @copilot Add a Verify(..., Times.Never) to assert VerifyForeignKeyExistsInDB is never called with a DatabaseTable whose SchemaName is dbo, so the test explicitly guards against the original dbo-fallback regression.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added two |
||
| [DataRow("mySchema.TEST_SOURCE_LINK", "mySchema", "TEST_SOURCE_LINK", DisplayName = "Linking object with custom schema")] | ||
| [DataRow("MYSCHEMA.TEST_SOURCE_LINK", "MYSCHEMA", "TEST_SOURCE_LINK", DisplayName = "Linking object with uppercase custom schema")] | ||
| [DataRow("myschema.test_source_link", "myschema", "test_source_link", DisplayName = "Linking object with lowercase schema and table")] | ||
| [DataTestMethod] | ||
| public void TestRelationshipWithLinkingObjectInCustomSchema( | ||
| string linkingObject, | ||
| string expectedSchema, | ||
| string expectedTable | ||
| ) | ||
| { | ||
| // Creating an EntityMap with two sample entities | ||
| Dictionary<string, Entity> entityMap = GetSampleEntityMap( | ||
| sourceEntity: "SampleEntity1", | ||
| targetEntity: "SampleEntity2", | ||
| sourceFields: new string[] { "sourceField" }, | ||
| targetFields: new string[] { "targetField" }, | ||
| linkingObject: linkingObject, | ||
| linkingSourceFields: new string[] { "linkingSourceField" }, | ||
| linkingTargetFields: new string[] { "linkingTargetField" } | ||
| ); | ||
|
|
||
| RuntimeConfig runtimeConfig = new( | ||
| Schema: "UnitTestSchema", | ||
| DataSource: new DataSource(DatabaseType: DatabaseType.MSSQL, ConnectionString: "", Options: null), | ||
| Runtime: new( | ||
| Rest: new(), | ||
| GraphQL: new(), | ||
| Mcp: new(), | ||
| Host: new(null, null) | ||
| ), | ||
| Entities: new(entityMap) | ||
| ); | ||
|
|
||
| // Mocking EntityToDatabaseObject - entities are in the custom schema as well | ||
| MockFileSystem fileSystem = new(); | ||
| FileSystemRuntimeConfigLoader loader = new(fileSystem); | ||
| RuntimeConfigProvider provider = new(loader) { IsLateConfigured = true }; | ||
| RuntimeConfigValidator configValidator = new(provider, fileSystem, new Mock<ILogger<RuntimeConfigValidator>>().Object); | ||
| Mock<ISqlMetadataProvider> _sqlMetadataProvider = new(); | ||
|
|
||
| Dictionary<string, DatabaseObject> mockDictionaryForEntityDatabaseObject = new() | ||
| { | ||
| { | ||
| "SampleEntity1", | ||
| new DatabaseTable(expectedSchema, "TEST_SOURCE1") | ||
| }, | ||
| { | ||
| "SampleEntity2", | ||
| new DatabaseTable(expectedSchema, "TEST_SOURCE2") | ||
| } | ||
| }; | ||
|
|
||
| _sqlMetadataProvider.Setup(x => x.EntityToDatabaseObject).Returns(mockDictionaryForEntityDatabaseObject); | ||
|
|
||
| // To mock the schema name and dbObjectName for linkingObject | ||
| _sqlMetadataProvider.Setup(x => | ||
| x.ParseSchemaAndDbTableName(linkingObject)).Returns((expectedSchema, expectedTable)); | ||
|
|
||
| string discard; | ||
| _sqlMetadataProvider.Setup(x => x.TryGetExposedColumnName(It.IsAny<string>(), It.IsAny<string>(), out discard)).Returns(true); | ||
|
|
||
| Mock<IMetadataProviderFactory> _metadataProviderFactory = new(); | ||
| _metadataProviderFactory.Setup(x => x.GetMetadataProvider(It.IsAny<string>())).Returns(_sqlMetadataProvider.Object); | ||
|
|
||
| // Mock ForeignKeyPair to be defined in DB with the custom schema | ||
| // The schema comparison should be case-insensitive | ||
| // Use concrete DatabaseTable instances with differing casing so that | ||
| // Moq relies on DatabaseTable.Equals for argument matching. | ||
| // Linking table uses lowercase to ensure case-insensitive comparison is working. | ||
| DatabaseTable expectedLinkingTable = new(expectedSchema.ToLowerInvariant(), expectedTable.ToLowerInvariant()); | ||
| DatabaseTable expectedSource1Table = new(expectedSchema.ToUpperInvariant(), "TEST_SOURCE1"); | ||
| DatabaseTable expectedSource2Table = new(expectedSchema.ToUpperInvariant(), "TEST_SOURCE2"); | ||
|
|
||
| _sqlMetadataProvider.Setup(x => | ||
| x.VerifyForeignKeyExistsInDB( | ||
| expectedLinkingTable, | ||
| expectedSource1Table | ||
| )).Returns(true); | ||
|
|
||
| _sqlMetadataProvider.Setup(x => | ||
| x.VerifyForeignKeyExistsInDB( | ||
| expectedLinkingTable, | ||
| expectedSource2Table | ||
| )).Returns(true); | ||
|
|
||
| // Validation should pass with custom schema | ||
| configValidator.ValidateRelationships(runtimeConfig, _metadataProviderFactory.Object); | ||
|
|
||
| // Verify that VerifyForeignKeyExistsInDB is never called with 'dbo' schema, | ||
| // guarding against the original dbo-fallback regression. | ||
| _sqlMetadataProvider.Verify(x => | ||
| x.VerifyForeignKeyExistsInDB( | ||
| It.Is<DatabaseTable>(t => string.Equals(t.SchemaName, "dbo", StringComparison.OrdinalIgnoreCase)), | ||
| It.IsAny<DatabaseTable>() | ||
| ), Times.Never); | ||
|
|
||
| _sqlMetadataProvider.Verify(x => | ||
| x.VerifyForeignKeyExistsInDB( | ||
| It.IsAny<DatabaseTable>(), | ||
| It.Is<DatabaseTable>(t => string.Equals(t.SchemaName, "dbo", StringComparison.OrdinalIgnoreCase)) | ||
| ), Times.Never); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Test method to check that an exception is thrown when the relationship is one-many | ||
| /// or many-one (determined by the linking object being null), while both SourceFields | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot : Consider also casing-mismatching expectedLinkingTable (not just the source tables) so the test always fails if DatabaseTable.Equals becomes case-sensitive again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
expectedLinkingTableto use.ToLowerInvariant()for both schema and table name, ensuring the test fails ifDatabaseTable.Equalsbecomes case-sensitive again. Commit0f2a111.