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

qs.parse returns invalid key with brackets when using nested array query #1751

Merged
merged 13 commits into from
Nov 6, 2024
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -414,4 +414,70 @@ module(`Integration | prerendered-card-search`, function (hooks) {
.dom('.card-container:nth-child(1)')
.containsText('Cardy Stackington Jr. III');
});

test<TestContextWithSSE>(`can parse objects in nested array`, async function (assert) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant to come up with a better name here

let query: Query = {
filter: {
on: {
module: `${testRealmURL}book`,
name: 'Book',
},
every: [
{
eq: {
'author.firstName': 'Cardy',
},
},
{
any: [
{
eq: {
'author.lastName': 'Jones',
},
},
{
eq: {
'author.lastName': 'Stackington Jr. III',
},
},
],
},
],
},
sort: [
{
by: 'author.lastName',
on: { module: `${testRealmURL}book`, name: 'Book' },
},
],
};
let realms = [testRealmURL];
await render(<template>
<PrerenderedCardSearch
@query={{query}}
@format='fitted'
@realms={{realms}}
>
<:loading>
Loading...
</:loading>
<:response as |cards|>
{{#each cards as |card|}}
<div class='card-container'>
<card.component />
</div>
{{/each}}
</:response>
</PrerenderedCardSearch>

{{! to support incremental indexing }}
<CardPrerender />
</template>);
await waitFor('.card-container');
assert.dom('.card-container').exists({ count: 2 });
assert
.dom('.card-container:nth-child(2)')
.containsText('Cardy Stackington Jr. III');
assert.dom('.card-container:nth-child(1)').containsText('Cardy Jones');
});
});
59 changes: 50 additions & 9 deletions packages/runtime-common/query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -307,9 +307,9 @@ function assertEveryFilter(
`${pointer.join('/') || '/'}: every must be an array of Filters`,
);
} else {
filter.every.every((value: any, index: number) =>
assertFilter(value, pointer.concat(`[${index}]`)),
);
Comment on lines -310 to -312
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.every is problematic because it expects to return a boolean. So, not all assertions are being run thru

filter.every.forEach((value: any, index: number) => {
assertFilter(value, pointer.concat(`[${index}]`));
});
}
}

Expand Down Expand Up @@ -348,9 +348,10 @@ function assertEqFilter(
if (typeof filter.eq !== 'object' || filter.eq == null) {
throw new Error(`${pointer.join('/') || '/'}: eq must be an object`);
}
Object.entries(filter.eq).every(([key, value]) =>
assertJSONValue(value, pointer.concat(key)),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pointer.concat doesn't even mutate the pointer. I would prefer to handle in another PR

);
Object.entries(filter.eq).forEach(([key, value]) => {
assertKey(key, pointer);
assertJSONValue(value, pointer.concat(key));
});
}

function assertContainsFilter(
Expand All @@ -371,9 +372,10 @@ function assertContainsFilter(
if (typeof filter.contains !== 'object' || filter.contains == null) {
throw new Error(`${pointer.join('/') || '/'}: contains must be an object`);
}
Object.entries(filter.contains).every(([key, value]) =>
assertJSONValue(value, pointer.concat(key)),
);
Object.entries(filter.contains).forEach(([key, value]) => {
assertKey(key, pointer);
assertJSONValue(value, pointer.concat(key));
});
}

function assertRangeFilter(
Expand Down Expand Up @@ -419,3 +421,42 @@ function assertRangeFilter(
});
});
}

export function assertKey(key: string, pointer: string[]) {
if (key.startsWith('[') && key.endsWith(']')) {
throw new Error(
`${pointer.join('/')}: field names cannot be wrapped in brackets: ${key}`,
);
}
}

const removeBrackets = (obj: any): any => {
if (!obj || typeof obj !== 'object') return obj;

// Handle arrays
if (Array.isArray(obj)) {
return obj.map((item) => removeBrackets(item));
}

return Object.entries(obj).reduce((acc, [key, value]) => {
// Remove surrounding brackets if they exist
const newKey = key.replace(/^\[(.*)\]$/, '$1');

// Handle arrays in values
if (Array.isArray(value)) {
acc[newKey] = value.map((item) => removeBrackets(item));
} else if (typeof value === 'object' && value !== null) {
// Recursively removeBrackets nested objects
acc[newKey] = removeBrackets(value);
} else {
// Handle primitive values
acc[newKey] = value;
}

return acc;
}, {} as any);
};

export const parseQuery = (queryString: string) => {
return removeBrackets(qs.parse(queryString));
};
8 changes: 4 additions & 4 deletions packages/runtime-common/realm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ import {
SupportedMimeType,
lookupRouteTable,
} from './router';
import { assertQuery } from './query';
import { assertQuery, parseQuery } from './query';
import type { Readable } from 'stream';
import { type CardDef } from 'https://cardstack.com/base/card-api';
import type * as CardAPI from 'https://cardstack.com/base/card-api';
Expand All @@ -75,7 +75,6 @@ import { fetcher } from './fetcher';
import { RealmIndexQueryEngine } from './realm-index-query-engine';
import { RealmIndexUpdater } from './realm-index-updater';

import qs from 'qs';
import {
MatrixBackendAuthentication,
Utils,
Expand Down Expand Up @@ -1531,7 +1530,7 @@ export class Realm {
request.headers.get('X-Boxel-Building-Index'),
);

let cardsQuery = qs.parse(new URL(request.url).search.slice(1));
let cardsQuery = parseQuery(new URL(request.url).search.slice(1));
assertQuery(cardsQuery);

let doc = await this.#realmIndexQueryEngine.search(cardsQuery, {
Expand All @@ -1555,7 +1554,8 @@ export class Realm {
request.headers.get('X-Boxel-Building-Index'),
);

let parsedQueryString = qs.parse(new URL(request.url).search.slice(1));
let href = new URL(request.url).search.slice(1);
let parsedQueryString = parseQuery(href);
let htmlFormat = parsedQueryString.prerenderedHtmlFormat as string;
let cardUrls = parsedQueryString.cardUrls as string[];

Expand Down