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.

Sign up for GitHub

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 assetlibrary handle undefined qs params #186

Merged
merged 4 commits into from
Feb 8, 2024
Merged

Fix assetlibrary handle undefined qs params #186

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b2aef67
fix(assetlibrary-client): only send offset and count if they are defined
aaronatbissell Jan 24, 2024
00c0034
fix(assetlibrary): properly handle string undefined when parsing qs p…
aaronatbissell Jan 24, 2024
269b178
test(assetlibrary): added tests for parsing of undefined string for c…
aaronatbissell Jan 24, 2024
68078ea
chore(misc): changelogs
aaronatbissell Jan 24, 2024
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
10 changes: 10 additions & 0 deletions ...cdf-assetlibrary-client/fix_assetlibrary_handle_undefined_qs_params_2024-01-24-12-52.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@awssolutions/cdf-assetlibrary-client",
"comment": "don't pass count and offset if they are undefined",
"type": "none"
}
],
"packageName": "@awssolutions/cdf-assetlibrary-client"
}
10 changes: 10 additions & 0 deletions ...utions/cdf-assetlibrary/fix_assetlibrary_handle_undefined_qs_params_2024-01-24-12-52.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@awssolutions/cdf-assetlibrary",
"comment": "properly handle undefined string for count and offset",
"type": "none"
}
],
"packageName": "@awssolutions/cdf-assetlibrary"
}
12 changes: 6 additions & 6 deletions source/packages/libraries/clients/assetlibrary-client/src/client/groups.lambda.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,8 @@ export class GroupsLambdaService extends GroupsServiceBase implements GroupsServ
.setQueryStringParameters({
template,
state,
offset: `${offset}`,
count: `${count}`,
offset: offset ? `${offset}` : undefined,
count: count ? `${count}` : undefined,
})
.setHeaders(super.buildHeaders(additionalHeaders));

Expand All @@ -149,8 +149,8 @@ export class GroupsLambdaService extends GroupsServiceBase implements GroupsServ
.setPath(super.groupGroupMembersRelativeUrl(groupPath))
.setQueryStringParameters({
template,
offset: `${offset}`,
count: `${count}`,
offset: offset ? `${offset}` : undefined,
count: count ? `${count}` : undefined,
})
.setHeaders(super.buildHeaders(additionalHeaders));

Expand All @@ -170,8 +170,8 @@ export class GroupsLambdaService extends GroupsServiceBase implements GroupsServ
.setMethod('GET')
.setPath(super.groupMembershipsRelativeUrl(groupPath))
.setQueryStringParameters({
offset: `${offset}`,
count: `${count}`,
offset: offset ? `${offset}` : undefined,
count: count ? `${count}` : undefined,
})
.setHeaders(super.buildHeaders(additionalHeaders));

Expand Down
32 changes: 13 additions & 19 deletions source/packages/services/assetlibrary/src/groups/groups.full.service.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,16 @@ describe('GroupsService', () => {
await instance.getMembers('agrouppath', TypeCategory.Group, 'testtemplate', 'active', 1);
expect(mockedDao.listRelated).toBeCalledTimes(4);

// validate undefined string
await instance.getMembers(
'agrouppath',
TypeCategory.Group,
'testtemplate',
'active',
'undefined' as any
);
expect(mockedDao.listRelated).toBeCalledTimes(5);

// validate errors with non-number string
try {
const result = await instance.getMembers(
Expand All @@ -485,23 +495,7 @@ describe('GroupsService', () => {
expect(err.name).toEqual('ArgumentError');
expect(err.message).toMatch('Invalid offset = something');
}
expect(mockedDao.listRelated).toBeCalledTimes(4);

// validate errors with undefined string
try {
const result = await instance.getMembers(
'agrouppath',
TypeCategory.Group,
'testtemplate',
'active',
'undefined' as any
);
fail(`Call should trigger an error. Got ${result}`);
} catch (err) {
expect(err.name).toEqual('ArgumentError');
expect(err.message).toMatch('Invalid offset = undefined');
}
expect(mockedDao.listRelated).toBeCalledTimes(4);
expect(mockedDao.listRelated).toBeCalledTimes(5);

// validate errors with NaN
try {
Expand All @@ -517,7 +511,7 @@ describe('GroupsService', () => {
expect(err.name).toEqual('ArgumentError');
expect(err.message).toMatch('Invalid offset = NaN');
}
expect(mockedDao.listRelated).toBeCalledTimes(4);
expect(mockedDao.listRelated).toBeCalledTimes(5);

// validate errors with -1
try {
Expand All @@ -535,6 +529,6 @@ describe('GroupsService', () => {
'Expected number `offset` to be greater than or equal to 0, got -1'
);
}
expect(mockedDao.listRelated).toBeCalledTimes(4);
expect(mockedDao.listRelated).toBeCalledTimes(5);
});
});
2 changes: 2 additions & 0 deletions source/packages/services/assetlibrary/src/utils/typeUtils.service.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ describe('typeUtils', () => {
{ label: 'number as number', input: 1, expected: 1 },
{ label: 'number as string', input: '1', expected: 1 },
{ label: 'undefined', input: undefined, expected: 0 },
{ label: 'undefined as string', input: 'undefined', expected: 0 },
];

offsetsAndExpectedSuccesses.forEach((metadata) => {
Expand Down Expand Up @@ -62,6 +63,7 @@ describe('typeUtils', () => {
{ label: 'number as number', input: 1, expected: 1 },
{ label: 'number as string', input: '1', expected: 1 },
{ label: 'undefined', input: undefined, expected: 500 }, // default is five hundred
{ label: 'undefined as string', input: 'undefined', expected: 500 }, // default is five hundred
];

countsAndExpectedSuccess.forEach((metadata) => {
Expand Down
12 changes: 6 additions & 6 deletions source/packages/services/assetlibrary/src/utils/typeUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,15 @@ export class TypeUtils {
* range using ow base methods instead of owCheckOptionalNumber.
*/
public parseAndValidateOffsetAndCount(
offset?: number,
count?: number
offset?: number | string,
count?: number | string
): { offsetAsInt: number; countAsInt: number } {
// use default count and offset if omitted
if (offset === undefined) {
offset = this.DEFAULT_PAGINATION_OFFSET;
if (offset === undefined || offset === 'undefined') {
offset = this.DEFAULT_PAGINATION_OFFSET.toString();
}
if (count === undefined) {
count = this.DEFAULT_PAGINATION_COUNT;
if (count === undefined || count === 'undefined') {
count = this.DEFAULT_PAGINATION_COUNT.toString();
}

// do the previous parse int (which should be a )
Expand Down
Loading