Browse Source

feat(core): Implement size limits for paginated list results

Relates to #751
Michael Bromley 4 years ago
parent
commit
92be4e0622

+ 1 - 1
packages/admin-ui/src/lib/core/src/shared/dynamic-form-inputs/customer-group-form-input/customer-group-form-input.component.ts

@@ -26,7 +26,7 @@ export class CustomerGroupFormInputComponent implements FormInputComponent, OnIn
     ngOnInit() {
         this.customerGroups$ = this.dataService.customer
             .getCustomerGroupList({
-                take: 9999,
+                take: 1000,
             })
             .mapSingle(res => res.customerGroups.items)
             .pipe(startWith([]));

+ 10 - 3
packages/core/e2e/fixtures/test-plugins/list-query-plugin.ts

@@ -43,7 +43,10 @@ export class TestEntity extends VendureEntity implements Translatable {
     @Column()
     date: Date;
 
-    @Calculated({ expression: 'LENGTH(description)' })
+    @Calculated({
+        query: qb => qb.addSelect('description', 'description'),
+        expression: 'LENGTH(description)',
+    })
     get descriptionLength() {
         return this.description.length || 0;
     }
@@ -118,7 +121,7 @@ export class ListQueryResolver {
     }
 }
 
-const adminApiExtensions = gql`
+const apiExtensions = gql`
     type TestEntity implements Node {
         id: ID!
         createdAt: DateTime!
@@ -149,7 +152,11 @@ const adminApiExtensions = gql`
     imports: [PluginCommonModule],
     entities: [TestEntity, TestEntityPrice, TestEntityTranslation],
     adminApiExtensions: {
-        schema: adminApiExtensions,
+        schema: apiExtensions,
+        resolvers: [ListQueryResolver],
+    },
+    shopApiExtensions: {
+        schema: apiExtensions,
         resolvers: [ListQueryResolver],
     },
 })

+ 28 - 1
packages/core/e2e/list-query-builder.e2e-spec.ts

@@ -8,13 +8,18 @@ import { testConfig, TEST_SETUP_TIMEOUT_MS } from '../../../e2e-common/test-conf
 
 import { ListQueryPlugin } from './fixtures/test-plugins/list-query-plugin';
 import { LanguageCode, SortOrder } from './graphql/generated-e2e-admin-types';
+import { assertThrowsWithMessage } from './utils/assert-throws-with-message';
 import { fixPostgresTimezone } from './utils/fix-pg-timezone';
 
 fixPostgresTimezone();
 
 describe('ListQueryBuilder', () => {
-    const { server, adminClient } = createTestEnvironment(
+    const { server, adminClient, shopClient } = createTestEnvironment(
         mergeConfig(testConfig, {
+            apiOptions: {
+                shopListQueryLimit: 10,
+                adminListQueryLimit: 30,
+            },
             plugins: [ListQueryPlugin],
         }),
     );
@@ -131,6 +136,28 @@ describe('ListQueryBuilder', () => {
             expect(testEntities.totalItems).toBe(5);
             expect(testEntities.items.length).toBe(5);
         });
+
+        it(
+            'take beyond adminListQueryLimit',
+            assertThrowsWithMessage(async () => {
+                await adminClient.query(GET_LIST, {
+                    options: {
+                        take: 50,
+                    },
+                });
+            }, 'Cannot take more than 30 results from a list query'),
+        );
+
+        it(
+            'take beyond shopListQueryLimit',
+            assertThrowsWithMessage(async () => {
+                await shopClient.query(GET_LIST, {
+                    options: {
+                        take: 50,
+                    },
+                });
+            }, 'Cannot take more than 10 results from a list query'),
+        );
     });
 
     describe('string filtering', () => {

+ 1 - 11
packages/core/src/api/resolvers/shop/shop-order.resolver.ts

@@ -63,17 +63,7 @@ export class ShopOrderResolver {
         @Ctx() ctx: RequestContext,
         @Args() args: QueryCountriesArgs,
     ): Promise<Array<Translated<Country>>> {
-        return this.countryService
-            .findAll(ctx, {
-                filter: {
-                    enabled: {
-                        eq: true,
-                    },
-                },
-                skip: 0,
-                take: 99999,
-            })
-            .then(data => data.items);
+        return this.countryService.findAllAvailable(ctx);
     }
 
     @Query()

+ 2 - 0
packages/core/src/config/default-config.ts

@@ -48,10 +48,12 @@ export const defaultConfig: RuntimeVendureConfig = {
         adminApiPath: 'admin-api',
         adminApiPlayground: false,
         adminApiDebug: false,
+        adminListQueryLimit: 1000,
         adminApiValidationRules: [],
         shopApiPath: 'shop-api',
         shopApiPlayground: false,
         shopApiDebug: false,
+        shopListQueryLimit: 100,
         shopApiValidationRules: [],
         channelTokenKey: 'vendure-token',
         cors: {

+ 16 - 0
packages/core/src/config/vendure-config.ts

@@ -107,6 +107,22 @@ export interface ApiOptions {
      * @default false
      */
     shopApiDebug?: boolean;
+    /**
+     * @description
+     * The maximum number of items that may be returned by a query which returns a `PaginatedList` response. In other words,
+     * this is the upper limit of the `take` input option.
+     *
+     * @default 100
+     */
+    shopListQueryLimit?: number;
+    /**
+     * @description
+     * The maximum number of items that may be returned by a query which returns a `PaginatedList` response. In other words,
+     * this is the upper limit of the `take` input option.
+     *
+     * @default 1000
+     */
+    adminListQueryLimit?: number;
     /**
      * @description
      * Custom functions to use as additional validation rules when validating the schema for the admin GraphQL API

+ 1 - 0
packages/core/src/i18n/messages/en.json

@@ -27,6 +27,7 @@
     "field-invalid-string-pattern": "The custom field '{ name }' value ['{ value }'] does not match the pattern [{ pattern }]",
     "forbidden": "You are not currently authorized to perform this action",
     "invalid-sort-field": "The sort field '{ fieldName }' is invalid. Valid fields are: { validFields }",
+    "list-query-limit-exceeded": "Cannot take more than { limit } results from a list query",
     "no-active-tax-zone": "The active tax zone could not be determined. Ensure a default tax zone is set for the current channel.",
     "no-configurable-operation-def-with-code-found": "No { type } with the code '{ code }' could be found",
     "no-price-found-for-channel": "No price information was found for ProductVariant ID '{ variantId}' in the Channel '{ channel }'.",

+ 10 - 2
packages/core/src/service/helpers/list-query-builder/list-query-builder.ts

@@ -7,6 +7,7 @@ import { SqljsDriver } from 'typeorm/driver/sqljs/SqljsDriver';
 import { FindOptionsUtils } from 'typeorm/find-options/FindOptionsUtils';
 
 import { RequestContext } from '../../../api/common/request-context';
+import { UserInputError } from '../../../common/error/errors';
 import { ListQueryOptions } from '../../../common/types/common-types';
 import { ConfigService } from '../../../config/config.service';
 import { VendureEntity } from '../../../entity/base/base.entity';
@@ -46,11 +47,18 @@ export class ListQueryBuilder implements OnApplicationBootstrap {
         options: ListQueryOptions<T> = {},
         extendedOptions: ExtendedListQueryOptions<T> = {},
     ): SelectQueryBuilder<T> {
+        const apiType = extendedOptions.ctx?.apiType ?? 'shop';
+        const { shopListQueryLimit, adminListQueryLimit } = this.configService.apiOptions;
+        const takeLimit = apiType === 'admin' ? adminListQueryLimit : shopListQueryLimit;
+        if (options.take && options.take > takeLimit) {
+            throw new UserInputError('error.list-query-limit-exceeded', { limit: takeLimit });
+        }
         const rawConnection = this.connection.rawConnection;
         const skip = Math.max(options.skip ?? 0, 0);
-        let take = Math.max(options.take ?? 0, 0);
+        // `take` must not be negative, and must not be greater than takeLimit
+        let take = Math.min(Math.max(options.take ?? 0, 0), takeLimit) || takeLimit;
         if (options.skip !== undefined && options.take === undefined) {
-            take = Number.MAX_SAFE_INTEGER;
+            take = takeLimit;
         }
         const sort = parseSortParams(
             rawConnection,

+ 10 - 0
packages/core/src/service/services/country.service.ts

@@ -54,6 +54,16 @@ export class CountryService {
             .then(country => country && translateDeep(country, ctx.languageCode));
     }
 
+    /**
+     * Returns an array of enabled Countries, intended for use in a public-facing (ie. Shop) API.
+     */
+    findAllAvailable(ctx: RequestContext): Promise<Array<Translated<Country>>> {
+        return this.connection
+            .getRepository(ctx, Country)
+            .find({ where: { enabled: true } })
+            .then(items => items.map(country => translateDeep(country, ctx.languageCode)));
+    }
+
     async findOneByCode(ctx: RequestContext, countryCode: string): Promise<Translated<Country>> {
         const country = await this.connection.getRepository(ctx, Country).findOne({
             where: {