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

Fix read model queries when selecting deeply-nested arrays #1552

Merged
merged 7 commits into from
Sep 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
MarcAstr0 marked this conversation as resolved.
Show resolved Hide resolved
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 ',
MarcAstr0 marked this conversation as resolved.
Show resolved Hide resolved
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
Loading