Skip to content

Commit

Permalink
Fix read model queries when selecting deeply-nested arrays (#1552)
Browse files Browse the repository at this point in the history
* Fix handling of deeply-nested arrays of objects in read model queries

* Handle case of top-level array property followed by sub array prop.

* Add rush change file

* Update docs

* Refactor preprocessProjections

* Update @cdktf/provider-azurerm version

* Refactor some code due to changes in azurerm

---------

Co-authored-by: Castro, Mario <mariocs@optum.com>
  • Loading branch information
MarcAstr0 and Castro, Mario authored Sep 25, 2024
1 parent 7e0efb2 commit d5ed537
Show file tree
Hide file tree
Showing 10 changed files with 231 additions and 89 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@boostercloud/framework-core",
"comment": "Correct handling of deeply-nested arrays and sub-array properties in Read Model queries",
"type": "patch"
}
],
"packageName": "@boostercloud/framework-core"
}
195 changes: 119 additions & 76 deletions common/config/rush/pnpm-lock.yaml

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
"@boostercloud/framework-core": "workspace:^2.18.1",
"@boostercloud/framework-provider-azure": "workspace:^2.18.1",
"@boostercloud/framework-types": "workspace:^2.18.1",
"@cdktf/provider-azurerm": "11.2.0",
"@cdktf/provider-azurerm": "13.3.0",
"@cdktf/provider-time": "9.0.2",
"@types/archiver": "5.1.0",
"@types/needle": "^2.0.4",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export class ApplicationSynth {
public constructor(terraformStack: TerraformStack) {
this.config = readProjectConfig(process.cwd())
const azurermProvider = new AzurermProvider(terraformStack, 'azureFeature', {
features: {},
features: [{}],
})
const appPrefix = buildAppPrefix(this.config)
const resourceGroupName = createResourceGroupName(this.config.appName, this.config.environmentName)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ export class TerraformContainers {
resourceGroupName: cosmosdbDatabaseResource.resourceGroupName,
accountName: cosmosdbDatabaseResource.name,
databaseName: cosmosdbSqlDatabaseResource.name,
partitionKeyPath: `/${eventsStoreAttributes.partitionKey}`,
partitionKeyPaths: [`/${eventsStoreAttributes.partitionKey}`],
partitionKeyVersion: 2,
autoscaleSettings: {
maxThroughput: MAX_CONTAINER_THROUGHPUT,
Expand All @@ -108,7 +108,7 @@ export class TerraformContainers {
resourceGroupName: cosmosdbDatabaseResource.resourceGroupName,
accountName: cosmosdbDatabaseResource.name,
databaseName: cosmosdbSqlDatabaseResource.name,
partitionKeyPath: '/eventId',
partitionKeyPaths: ['/eventId'],
partitionKeyVersion: 2,
uniqueKey: [{ paths: ['/eventId'] }],
autoscaleSettings: {
Expand All @@ -134,7 +134,7 @@ export class TerraformContainers {
resourceGroupName: cosmosdbDatabaseResource.resourceGroupName,
accountName: cosmosdbDatabaseResource.name,
databaseName: cosmosdbSqlDatabaseResource.name,
partitionKeyPath: '/id',
partitionKeyPaths: ['/id'],
partitionKeyVersion: 2,
autoscaleSettings: {
maxThroughput: MAX_CONTAINER_THROUGHPUT,
Expand All @@ -157,7 +157,7 @@ export class TerraformContainers {
resourceGroupName: cosmosdbDatabaseResource.resourceGroupName,
accountName: cosmosdbDatabaseResource.name,
databaseName: cosmosdbSqlDatabaseResource.name,
partitionKeyPath: `/${subscriptionsStoreAttributes.partitionKey}`,
partitionKeyPaths: [`/${subscriptionsStoreAttributes.partitionKey}`],
partitionKeyVersion: 2,
defaultTtl: -1,
autoscaleSettings: {
Expand All @@ -181,7 +181,7 @@ export class TerraformContainers {
resourceGroupName: cosmosdbDatabaseResource.resourceGroupName,
accountName: cosmosdbDatabaseResource.name,
databaseName: cosmosdbSqlDatabaseResource.name,
partitionKeyPath: `/${connectionsStoreAttributes.partitionKey}`,
partitionKeyPaths: [`/${connectionsStoreAttributes.partitionKey}`],
partitionKeyVersion: 2,
defaultTtl: -1,
autoscaleSettings: {
Expand All @@ -207,7 +207,7 @@ export class TerraformContainers {
resourceGroupName: cosmosdbDatabase.resourceGroupName,
accountName: cosmosdbDatabase.name,
databaseName: cosmosdbSqlDatabase.name,
partitionKeyPath: `/${dedupAttributes.partitionKey}`,
partitionKeyPaths: [`/${dedupAttributes.partitionKey}`],
uniqueKey: [{ paths: [`/${dedupAttributes.partitionKey}`] }],
partitionKeyVersion: 2,
defaultTtl: -1,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ export class TerraformCosmosdbDatabase {
resourceGroupName: resourceGroupName,
offerType: 'Standard',
kind: 'GlobalDocumentDB',
enableMultipleWriteLocations: false,
multipleWriteLocationsEnabled: false,
isVirtualNetworkFilterEnabled: false,
enableAutomaticFailover: true,
automaticFailoverEnabled: true,
geoLocation: [
{
location: resourceGroup.location,
Expand Down
3 changes: 2 additions & 1 deletion packages/framework-provider-azure/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@
"@boostercloud/framework-types": "workspace:^2.18.1",
"tslib": "^2.4.0",
"@effect-ts/core": "^0.60.4",
"@azure/web-pubsub": "~1.1.0"
"@azure/web-pubsub": "~1.1.0",
"@cdktf/provider-azurerm": "13.3.0"
},
"devDependencies": {
"@boostercloud/eslint-config": "workspace:^2.18.1",
Expand Down
50 changes: 49 additions & 1 deletion packages/framework-provider-azure/src/helpers/query-helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,9 @@ function buildProjections(projections: ProjectionFor<unknown> | string = '*'): s
return projections
}

// Preprocess the projections
const preprocessedProjections = preprocessProjections(projections)

// Helper function to convert dot notation to square-bracket notation
const toSquareBracketsNotation = (path: string): string => {
return path
Expand All @@ -253,7 +256,7 @@ function buildProjections(projections: ProjectionFor<unknown> | string = '*'): s

// Group fields by the root property
const groupedFields: { [key: string]: string[] } = {}
Object.values(projections).forEach((field: string) => {
Object.values(preprocessedProjections).forEach((field: string) => {
const root: string = field.split('.')[0]
if (!groupedFields[root]) {
groupedFields[root] = []
Expand Down Expand Up @@ -314,6 +317,51 @@ function buildProjections(projections: ProjectionFor<unknown> | string = '*'): s
.join(', ')
}

/**
* Preprocesses the projections to handle nested arrays and objects.
*
* @param {ProjectionFor<unknown>} projections - The projections to preprocess.
* @returns {ProjectionFor<unknown>} - The preprocessed projections.
*/
function preprocessProjections(projections: ProjectionFor<unknown>): ProjectionFor<unknown> {
const processed = new Set<string>()

Object.values(projections).forEach((field: string) => {
const parts = field.split('.')
const arrayIndices = parts.reduce((acc, part, index) => {
if (part.endsWith('[]')) acc.push(index)
return acc
}, [] as number[])

if (
arrayIndices.length === 0 ||
(arrayIndices[0] === 0 && arrayIndices.length === 1) ||
(arrayIndices[0] === 1 && arrayIndices.length === 1)
) {
// This block is accessed when one of the following occurs:
// - No arrays in the projection
// - Top-level array not followed by another array
// - Array nested within a top-level property, no arrays follow
processed.add(field)
} else {
// Cases with nested arrays or arrays deeper in the structure
const processToIndex = arrayIndices[0] === 0 || arrayIndices[0] === 1 ? arrayIndices[1] : arrayIndices[0]
const processedField = parts.slice(0, processToIndex + 1).join('.')
processed.add(processedField.slice(0, -2)) // Remove the '[]' from the last part
}
})

// Convert the Set back to the original type of projections
if (Array.isArray(projections)) {
return Array.from(processed) as ProjectionFor<unknown>
} else {
return Array.from(processed).reduce((acc, field) => {
;(acc as any)[field] = field
return acc
}, {} as ProjectionFor<unknown>)
}
}

/**
* Transforms the flat properties returned by Cosmos DB into a nested structure. For example, the following object:
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,46 @@ describe('Query helper', () => {
)
})

it('Executes a SQL query with a projectionFor projection that has a deeply nested array of objects', async () => {
await search(
mockCosmosDbClient as any,
mockConfig,
mockReadModelName,
{},
undefined,
undefined,
false,
undefined,
[
'id',
'x.arr[].z',
'foo.bar.items[].id',
'foo.bar.baz.items[].id',
'arr[].subArr[].id',
'arr[].id',
] as ProjectionFor<unknown>
)

expect(mockCosmosDbClient.database).to.have.been.calledWithExactly(mockConfig.resourceNames.applicationStack)
expect(
mockCosmosDbClient.database(mockConfig.resourceNames.applicationStack).container
).to.have.been.calledWithExactly(`${mockReadModelName}`)
expect(
mockCosmosDbClient.database(mockConfig.resourceNames.applicationStack).container(`${mockReadModelName}`).items
.query
).to.have.been.calledWith(
match({
query:
'SELECT c["id"], ' +
'ARRAY(SELECT item["z"] FROM item IN c["x"]["arr"]) AS "x.arr", ' +
'c["foo"]["bar"]["items"] AS "foo.bar.items", c["foo"]["bar"]["baz"]["items"] AS "foo.bar.baz.items", ' +
'ARRAY(SELECT item["subArr"], item["id"] FROM item IN c["arr"]) AS arr ' +
'FROM c ',
parameters: [],
})
)
})

it('Executes a SQL query with a star projection in the read model table', async () => {
await search(
mockCosmosDbClient as any,
Expand Down
2 changes: 1 addition & 1 deletion website/docs/06_graphql.md
Original file line number Diff line number Diff line change
Expand Up @@ -891,7 +891,7 @@ Using `select` will skip any Read Models migrations that need to be applied to t
:::
:::warning
Support for selecting fields from objects inside arrays is limited to arrays that are at most nested inside another property, e.g., `['category.relatedCategories[].name']`. Selecting fields from arrays that are nested deeper than that (e.g., `['foo.bar.items[].id']`) is not expected to yield the expected results.
Support for selecting fields from objects inside arrays is limited to arrays that are at most nested inside another property, e.g., `['category.relatedCategories[].name']`. Selecting fields from arrays that are nested deeper than that (e.g., `['foo.bar.items[].id']`) will return the entire object.
:::
:::warning
Expand Down

0 comments on commit d5ed537

Please sign in to comment.