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

feat: add option to change the default value of the Parse.Query.limit() constraint #8152

Merged
merged 25 commits into from
Sep 29, 2022
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
e6a4c50
Adding 'defaultLimit' option to allow changing the default limit for …
vzukanov Sep 5, 2022
049b361
Merge branch 'alpha' into alpha
mtrezza Sep 8, 2022
fefa91f
Merge branch 'alpha' into alpha
mtrezza Sep 8, 2022
6c05435
Refactoring unit tests to use async/await and better names
vzukanov Sep 10, 2022
ffc23a4
Additional unit test for defaultLimit value
vzukanov Sep 10, 2022
1d5290e
Improving the description of defaultLimit config option
vzukanov Sep 10, 2022
562c713
Merge branch 'alpha' into alpha
vzukanov Sep 10, 2022
9dcc1af
Merge branch 'alpha' into alpha
vzukanov Sep 14, 2022
23e8e20
optimize test syntax
mtrezza Sep 14, 2022
752d1ac
Update spec/index.spec.js
mtrezza Sep 19, 2022
f0fd15e
Update spec/index.spec.js
mtrezza Sep 19, 2022
2fb72be
Merge branch 'alpha' into alpha
mtrezza Sep 19, 2022
2ddfb48
Update spec/index.spec.js
mtrezza Sep 23, 2022
98f8761
Merge branch 'alpha' into alpha
mtrezza Sep 23, 2022
b606730
Moving the definition of the default value for defaultLimit option fr…
vzukanov Sep 26, 2022
d0e4ab6
Merge branch 'alpha' into alpha
vzukanov Sep 26, 2022
d3eb60e
Update src/Options/index.js
mtrezza Sep 26, 2022
3698600
Update src/Options/Definitions.js
mtrezza Sep 26, 2022
b794742
Update src/Options/docs.js
mtrezza Sep 26, 2022
d43ff5f
refactor
dblythy Sep 28, 2022
9d405c3
Merge branch 'alpha' into alpha
dblythy Sep 28, 2022
d6fa103
Merge branch 'alpha' into alpha
mtrezza Sep 28, 2022
0d5f7d7
Update src/Config.js
dblythy Sep 28, 2022
a5741c4
Update Config.js
dblythy Sep 28, 2022
283b7f0
Merge branch 'alpha' into alpha
dblythy Sep 29, 2022
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
60 changes: 39 additions & 21 deletions spec/ParseQuery.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -314,39 +314,57 @@ describe('Parse.Query testing', () => {
equal(results.length, 0);
});

it('query with limit', function (done) {
const baz = new TestObject({ foo: 'baz' });
const qux = new TestObject({ foo: 'qux' });
Parse.Object.saveAll([baz, qux]).then(function () {
const query = new Parse.Query(TestObject);
query.limit(1);
query.find().then(function (results) {
equal(results.length, 1);
done();
});
});
it('query without limit respects default limit', async () => {
const obj1 = new TestObject({ foo: 'baz' });
const obj2 = new TestObject({ foo: 'qux' });
await reconfigureServer({ defaultLimit: 1 });
await Parse.Object.saveAll([obj1, obj2]);
const query = new Parse.Query(TestObject);
const result = await query.find();
expect(result.length).toBe(1);
});

it('query with limit', async () => {
const obj1 = new TestObject({ foo: 'baz' });
const obj2 = new TestObject({ foo: 'qux' });
await Parse.Object.saveAll([obj1, obj2]);
const query = new Parse.Query(TestObject);
query.limit(1);
const result = await query.find();
expect(result.length).toBe(1);
});

it('query with limit overrides default limit', async () => {
const obj1 = new TestObject({ foo: 'baz' });
const obj2 = new TestObject({ foo: 'qux' });
await reconfigureServer({ defaultLimit: 2 });
await Parse.Object.saveAll([obj1, obj2]);
const query = new Parse.Query(TestObject);
query.limit(1);
const result = await query.find();
expect(result.length).toBe(1);
});

it('query with limit equal to maxlimit', async () => {
const baz = new TestObject({ foo: 'baz' });
const qux = new TestObject({ foo: 'qux' });
const obj1 = new TestObject({ foo: 'baz' });
const obj2 = new TestObject({ foo: 'qux' });
await reconfigureServer({ maxLimit: 1 });
await Parse.Object.saveAll([baz, qux]);
await Parse.Object.saveAll([obj1, obj2]);
const query = new Parse.Query(TestObject);
query.limit(1);
const results = await query.find();
equal(results.length, 1);
const result = await query.find();
expect(result.length).toBe(1);
});

it('query with limit exceeding maxlimit', async () => {
const baz = new TestObject({ foo: 'baz' });
const qux = new TestObject({ foo: 'qux' });
const obj1 = new TestObject({ foo: 'baz' });
const obj2 = new TestObject({ foo: 'qux' });
await reconfigureServer({ maxLimit: 1 });
await Parse.Object.saveAll([baz, qux]);
await Parse.Object.saveAll([obj1, obj2]);
const query = new Parse.Query(TestObject);
query.limit(2);
const results = await query.find();
equal(results.length, 1);
const result = await query.find();
expect(result.length).toBe(1);
});

it('containedIn object array queries', function (done) {
Expand Down
18 changes: 18 additions & 0 deletions spec/index.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -462,13 +462,31 @@ describe('server', () => {
.then(done);
});

it('fails if default limit is negative', async () => {
await expectAsync(reconfigureServer({ defaultLimit: -1 })).toBeRejectedWith('Default limit must be a value greater than 0.');
});

it('fails if default limit is zero', done => {
reconfigureServer({ defaultLimit: 0 }).catch(error => {
expect(error).toEqual('Default limit must be a value greater than 0.');
done();
});
});
mtrezza marked this conversation as resolved.
Show resolved Hide resolved

it('fails if maxLimit is negative', done => {
reconfigureServer({ maxLimit: -100 }).catch(error => {
expect(error).toEqual('Max limit must be a value greater than 0.');
done();
});
});

it('fails if maxLimit is smaller than the default limit', done => {
mtrezza marked this conversation as resolved.
Show resolved Hide resolved
reconfigureServer({ defaultLimit: 101, maxLimit: 100 }).catch(error => {
expect(error).toEqual('Max limit must be greater than the default limit.');
done();
});
});
mtrezza marked this conversation as resolved.
Show resolved Hide resolved

it('fails if you try to set revokeSessionOnPasswordReset to non-boolean', done => {
reconfigureServer({ revokeSessionOnPasswordReset: 'non-bool' }).catch(done);
});
Expand Down
15 changes: 13 additions & 2 deletions src/Config.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ export class Config {
revokeSessionOnPasswordReset,
expireInactiveSessions,
sessionLength,
defaultLimit,
maxLimit,
emailVerifyTokenValidityDuration,
accountLockout,
Expand Down Expand Up @@ -110,7 +111,8 @@ export class Config {
}
this.validateSessionConfiguration(sessionLength, expireInactiveSessions);
this.validateMasterKeyIps(masterKeyIps);
this.validateMaxLimit(maxLimit);
this.validateDefaultLimit(defaultLimit);
this.validateMaxLimit(maxLimit, defaultLimit);
this.validateAllowHeaders(allowHeaders);
this.validateIdempotencyOptions(idempotencyOptions);
this.validatePagesOptions(pages);
Expand Down Expand Up @@ -453,10 +455,19 @@ export class Config {
}
}

static validateMaxLimit(maxLimit) {
static validateDefaultLimit(defaultLimit) {
if (defaultLimit <= 0) {
mtrezza marked this conversation as resolved.
Show resolved Hide resolved
mtrezza marked this conversation as resolved.
Show resolved Hide resolved
throw 'Default limit must be a value greater than 0.';
}
}

static validateMaxLimit(maxLimit, defaultLimit) {
if (maxLimit <= 0) {
throw 'Max limit must be a value greater than 0.';
}
if (maxLimit < defaultLimit) {
throw 'Max limit must be greater than the default limit.';
}
}

static validateAllowHeaders(allowHeaders) {
Expand Down
2 changes: 1 addition & 1 deletion src/GraphQL/helpers/objectsQueries.js
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ const calculateSkipAndLimit = (skipInput, first, after, last, before, maxLimit)
}

if ((skip || 0) >= before) {
// If the before index is less then the skip, no objects will be returned
// If the before index is less than the skip, no objects will be returned
limit = 0;
} else if ((!limit && limit !== 0) || (skip || 0) + limit > before) {
// If there is no limit set, the limit is calculated. Or, if the limit (plus skip) is bigger than the before index, the new limit is set.
Expand Down
5 changes: 5 additions & 0 deletions src/Options/Definitions.js
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,11 @@ module.exports.ParseServerOptions = {
required: true,
default: 'mongodb://localhost:27017/parse',
},
defaultLimit: {
env: 'PARSE_SERVER_DEFAULT_LIMIT',
help: 'Default limit for the size of results set on queries, defaults to `100`.',
mtrezza marked this conversation as resolved.
Show resolved Hide resolved
action: parsers.numberParser('defaultLimit'),
},
directAccess: {
env: 'PARSE_SERVER_DIRECT_ACCESS',
help:
Expand Down
1 change: 1 addition & 0 deletions src/Options/docs.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
* @property {Adapter<StorageAdapter>} databaseAdapter Adapter module for the database
* @property {DatabaseOptions} databaseOptions Options to pass to the database client
* @property {String} databaseURI The full URI to your database. Supported databases are mongodb or postgres.
* @property {Number} defaultLimit Default limit for the size of results set on queries, defaults to `100`.
mtrezza marked this conversation as resolved.
Show resolved Hide resolved
* @property {Boolean} directAccess Set to `true` if Parse requests within the same Node.js environment as Parse Server should be routed to Parse Server directly instead of via the HTTP interface. Default is `false`.<br><br>If set to `false` then Parse requests within the same Node.js environment as Parse Server are executed as HTTP requests sent to Parse Server via the `serverURL`. For example, a `Parse.Query` in Cloud Code is calling Parse Server via a HTTP request. The server is essentially making a HTTP request to itself, unnecessarily using network resources such as network ports.<br><br>⚠️ In environments where multiple Parse Server instances run behind a load balancer and Parse requests within the current Node.js environment should be routed via the load balancer and distributed as HTTP requests among all instances via the `serverURL`, this should be set to `false`.
* @property {String} dotNetKey Key for Unity and .Net SDK
* @property {Adapter<MailAdapter>} emailAdapter Adapter module for email sending
Expand Down
2 changes: 2 additions & 0 deletions src/Options/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,8 @@ export interface ParseServerOptions {
/* Session duration, in seconds, defaults to 1 year
:DEFAULT: 31536000 */
sessionLength: ?number;
/* Default limit for the size of results set on queries, defaults to `100`. */
mtrezza marked this conversation as resolved.
Show resolved Hide resolved
defaultLimit: ?number;
/* Max value for limit option on queries, defaults to unlimited */
maxLimit: ?number;
/* Sets whether we should expire the inactive sessions, defaults to true. If false, all new sessions are created with no expiration date.
Expand Down
6 changes: 3 additions & 3 deletions src/Routers/ClassesRouter.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export class ClassesRouter extends PromiseRouter {

handleFind(req) {
const body = Object.assign(req.body, ClassesRouter.JSONFromQuery(req.query));
const options = ClassesRouter.optionsFromBody(body);
const options = ClassesRouter.optionsFromBody(body, req.config.defaultLimit);
if (req.config.maxLimit && body.limit > req.config.maxLimit) {
// Silently replace the limit on the query with the max configured
options.limit = Number(req.config.maxLimit);
Expand Down Expand Up @@ -149,7 +149,7 @@ export class ClassesRouter extends PromiseRouter {
return json;
}

static optionsFromBody(body) {
static optionsFromBody(body, defaultLimit) {
const allowConstraints = [
'skip',
'limit',
Expand Down Expand Up @@ -180,7 +180,7 @@ export class ClassesRouter extends PromiseRouter {
if (body.limit || body.limit === 0) {
options.limit = Number(body.limit);
} else {
options.limit = Number(100);
options.limit = Number(defaultLimit || 100);
mtrezza marked this conversation as resolved.
Show resolved Hide resolved
}
if (body.order) {
options.order = String(body.order);
Expand Down