-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
feat: Allow short circuit of beforeFind #9770
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
base: alpha
Are you sure you want to change the base?
Changes from 36 commits
6f65ec5
41bf1b7
21c9289
ca74929
207c3c0
bc68816
d66fa48
34f7741
5825abf
6daa039
ec37789
09fcde2
8de0edb
dd75062
8d6aae6
c5dd665
55c3b3b
d8525e9
04ae88f
f083527
f679ba0
dfd4082
43a87bb
c417118
c6463c3
0c2ad57
92ef147
546e5b0
00b104e
2a5dce9
98be232
0d42676
f197119
06e2bcc
52d5594
c08b3f6
37f74c1
468a6ec
b157aa2
83a918b
840a3e4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -201,6 +201,267 @@ describe('Cloud Code', () => { | |
done(); | ||
} | ||
}); | ||
it('beforeFind can return object without DB operation', async () => { | ||
const config = Config.get('test'); | ||
mtrezza marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const databaseAdapter = config.database.adapter; | ||
const findSpy = spyOn(databaseAdapter, 'find').and.callThrough(); | ||
|
||
Parse.Cloud.beforeFind('TestObject', () => { | ||
return new Parse.Object('TestObject', { foo: 'bar' }); | ||
}); | ||
|
||
Parse.Cloud.afterFind('TestObject', req => { | ||
expect(req.objects).toBeDefined(); | ||
expect(req.objects[0].get('foo')).toBe('bar'); | ||
}); | ||
|
||
const newObj = await new Parse.Query('TestObject').first(); | ||
expect(newObj.className).toBe('TestObject'); | ||
expect(newObj.toJSON()).toEqual({ foo: 'bar' }); | ||
|
||
expect(findSpy).not.toHaveBeenCalled(); | ||
|
||
await newObj.save(); | ||
}); | ||
|
||
it('beforeFind can return array of objects without DB operation', async () => { | ||
const config = Config.get('test'); | ||
const databaseAdapter = config.database.adapter; | ||
const findSpy = spyOn(databaseAdapter, 'find').and.callThrough(); | ||
|
||
Parse.Cloud.beforeFind('TestObject', () => { | ||
return [new Parse.Object('TestObject', { foo: 'bar' })]; | ||
}); | ||
|
||
Parse.Cloud.afterFind('TestObject', req => { | ||
expect(req.objects).toBeDefined(); | ||
expect(req.objects[0].get('foo')).toBe('bar'); | ||
}); | ||
|
||
const newObj = await new Parse.Query('TestObject').first(); | ||
expect(newObj.className).toBe('TestObject'); | ||
expect(newObj.toJSON()).toEqual({ foo: 'bar' }); | ||
|
||
expect(findSpy).not.toHaveBeenCalled(); | ||
|
||
await newObj.save(); | ||
}); | ||
|
||
it('beforeFind can return object for get query without DB operation', async () => { | ||
const config = Config.get('test'); | ||
const databaseAdapter = config.database.adapter; | ||
const findSpy = spyOn(databaseAdapter, 'find').and.callThrough(); | ||
|
||
Parse.Cloud.beforeFind('TestObject', () => { | ||
return [new Parse.Object('TestObject', { foo: 'bar' })]; | ||
}); | ||
|
||
Parse.Cloud.afterFind('TestObject', req => { | ||
expect(req.objects).toBeDefined(); | ||
expect(req.objects[0].get('foo')).toBe('bar'); | ||
}); | ||
|
||
const testObj = new Parse.Object('TestObject'); | ||
await testObj.save(); | ||
|
||
findSpy.calls.reset(); | ||
|
||
const newObj = await new Parse.Query('TestObject').get(testObj.id); | ||
expect(newObj.className).toBe('TestObject'); | ||
expect(newObj.toJSON()).toEqual({ foo: 'bar' }); | ||
|
||
expect(findSpy).not.toHaveBeenCalled(); | ||
|
||
await newObj.save(); | ||
}); | ||
|
||
it('beforeFind can return empty array without DB operation', async () => { | ||
const config = Config.get('test'); | ||
const databaseAdapter = config.database.adapter; | ||
const findSpy = spyOn(databaseAdapter, 'find').and.callThrough(); | ||
|
||
Parse.Cloud.beforeFind('TestObject', () => { | ||
return []; | ||
}); | ||
|
||
Parse.Cloud.afterFind('TestObject', req => { | ||
expect(req.objects.length).toBe(0); | ||
}); | ||
|
||
const obj = new Parse.Object('TestObject'); | ||
await obj.save(); | ||
|
||
findSpy.calls.reset(); | ||
|
||
const newObj = await new Parse.Query('TestObject').first(); | ||
expect(newObj).toBeUndefined(); | ||
|
||
expect(findSpy).not.toHaveBeenCalled(); | ||
}); | ||
|
||
const { maybeRunAfterFindTrigger } = require('../lib/triggers'); | ||
|
||
describe('maybeRunAfterFindTrigger - direct function tests', () => { | ||
const testConfig = { | ||
applicationId: 'test', | ||
logLevels: { triggerBeforeSuccess: 'info', triggerAfter: 'info' }, | ||
}; | ||
|
||
it('should convert Parse.Object instances to JSON when no trigger defined', async () => { | ||
const className = 'TestParseObjectDirect_' + Date.now(); | ||
|
||
const parseObj1 = new Parse.Object(className); | ||
parseObj1.set('name', 'test1'); | ||
parseObj1.id = 'obj1'; | ||
|
||
const parseObj2 = new Parse.Object(className); | ||
parseObj2.set('name', 'test2'); | ||
parseObj2.id = 'obj2'; | ||
|
||
const result = await maybeRunAfterFindTrigger( | ||
'afterFind', | ||
null, | ||
className, | ||
[parseObj1, parseObj2], | ||
testConfig, | ||
null, | ||
{} | ||
); | ||
|
||
expect(result).toBeDefined(); | ||
expect(Array.isArray(result)).toBe(true); | ||
expect(result.length).toBe(2); | ||
expect(result[0].name).toBe('test1'); | ||
expect(result[1].name).toBe('test2'); | ||
}); | ||
|
||
it('should handle null/undefined objectsInput when no trigger', async () => { | ||
const className = 'TestNullDirect_' + Date.now(); | ||
|
||
const resultNull = await maybeRunAfterFindTrigger( | ||
'afterFind', | ||
null, | ||
className, | ||
null, | ||
testConfig, | ||
null, | ||
{} | ||
); | ||
expect(resultNull).toEqual([]); | ||
|
||
const resultUndefined = await maybeRunAfterFindTrigger( | ||
'afterFind', | ||
null, | ||
className, | ||
undefined, | ||
testConfig, | ||
null, | ||
{} | ||
); | ||
expect(resultUndefined).toEqual([]); | ||
|
||
const resultEmpty = await maybeRunAfterFindTrigger( | ||
'afterFind', | ||
null, | ||
className, | ||
[], | ||
testConfig, | ||
null, | ||
{} | ||
); | ||
expect(resultEmpty).toEqual([]); | ||
}); | ||
|
||
it('should handle plain object query with where clause', async () => { | ||
const className = 'TestQueryWhereDirect_' + Date.now(); | ||
let receivedQuery = null; | ||
|
||
Parse.Cloud.afterFind(className, req => { | ||
receivedQuery = req.query; | ||
return req.objects; | ||
}); | ||
|
||
const mockObject = { id: 'test123', className: className, name: 'test' }; | ||
|
||
const result = await maybeRunAfterFindTrigger( | ||
'afterFind', | ||
null, | ||
className, | ||
[mockObject], | ||
testConfig, | ||
{ where: { name: 'test' }, limit: 10 }, | ||
{} | ||
); | ||
|
||
expect(receivedQuery).toBeInstanceOf(Parse.Query); | ||
expect(result).toBeDefined(); | ||
}); | ||
|
||
it('should handle plain object query without where clause', async () => { | ||
const className = 'TestQueryNoWhereDirect_' + Date.now(); | ||
let receivedQuery = null; | ||
|
||
Parse.Cloud.afterFind(className, req => { | ||
receivedQuery = req.query; | ||
return req.objects; | ||
}); | ||
|
||
const mockObject = { id: 'test456', className: className, name: 'test' }; | ||
|
||
const result = await maybeRunAfterFindTrigger( | ||
'afterFind', | ||
null, | ||
className, | ||
[mockObject], | ||
testConfig, | ||
{ limit: 5, skip: 0 }, | ||
{} | ||
); | ||
|
||
expect(receivedQuery).toBeInstanceOf(Parse.Query); | ||
expect(result).toBeDefined(); | ||
}); | ||
Comment on lines
+446
to
+469
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainPlain object query without where: likely bug in conversion; assert desired behavior and fix implementation. Current implementation in Test update: expect(receivedQuery).toBeInstanceOf(Parse.Query);
-expect(result).toBeDefined();
+const qJSON = receivedQuery.toJSON();
+expect(qJSON.limit).toBe(5);
+expect(qJSON.skip).toBe(0);
+expect(qJSON.where).toEqual({}); // no explicit filters provided
+expect(result).toBeDefined(); Implementation fix (src/triggers.js), replace the else-branch that wraps the entire object under - } else if (typeof query === 'object' && query !== null) {
- const parseQueryInstance = new Parse.Query(classNameQuery);
- if (query.where) {
- parseQueryInstance.withJSON(query);
- } else {
- parseQueryInstance.withJSON({ where: query });
- }
- request.query = parseQueryInstance;
- } else {
+ } else if (typeof query === 'object' && query !== null) {
+ const parseQueryInstance = new Parse.Query(classNameQuery);
+ const TOP_LEVEL = [
+ 'limit',
+ 'skip',
+ 'order',
+ 'keys',
+ 'include',
+ 'excludeKeys',
+ 'readPreference',
+ 'includeReadPreference',
+ 'subqueryReadPreference',
+ 'hint',
+ 'explain',
+ ];
+ const json = {};
+ if (query.where && typeof query.where === 'object') {
+ json.where = query.where;
+ for (const k of TOP_LEVEL) if (k in query) json[k] = query[k];
+ } else {
+ json.where = query;
+ }
+ parseQueryInstance.withJSON(json);
+ request.query = parseQueryInstance;
+ } else { This preserves standard query options at the top level while treating a plain object as the 🏁 Script executed: #!/bin/bash
# Locate the implementation of maybeRunAfterFindTrigger in the codebase
rg -n 'function maybeRunAfterFindTrigger' -C3 --glob '*.js' Length of output: 292 🏁 Script executed: #!/bin/bash
# Display the implementation of maybeRunAfterFindTrigger around the object-handling branch
sed -n '430,550p' src/triggers.js Length of output: 3489 🏁 Script executed: #!/bin/bash
# Search for the definition and usage of withJSON in the codebase
rg -n 'withJSON' -C3 --glob '*.js' Length of output: 6252 Refactor The current logic nests all keys into the
Locations to update:
Suggested diff for spec/CloudCode.spec.js: it('should handle plain object query without where clause', async () => {
// …setup…
expect(receivedQuery).toBeInstanceOf(Parse.Query);
- expect(result).toBeDefined();
+ // The JSON form should honor limit/skip at top level and no filters in where
+ const qJSON = receivedQuery.toJSON();
+ expect(qJSON.limit).toBe(5);
+ expect(qJSON.skip).toBe(0);
+ expect(qJSON.where).toEqual({});
+ expect(result).toBeDefined();
}); Suggested refactor for src/triggers.js: @@ export function maybeRunAfterFindTrigger(
- } else if (typeof query === 'object' && query !== null) {
- const parseQueryInstance = new Parse.Query(classNameQuery);
- if (query.where) {
- parseQueryInstance.withJSON(query);
- } else {
- parseQueryInstance.withJSON({ where: query });
- }
- request.query = parseQueryInstance;
- } else {
+ } else if (typeof query === 'object' && query !== null) {
+ const parseQueryInstance = new Parse.Query(classNameQuery);
+ // Define top-level keys to extract
+ const TOP_LEVEL = [
+ 'limit','skip','order','keys','include','excludeKeys',
+ 'readPreference','includeReadPreference','subqueryReadPreference',
+ 'hint','explain'
+ ];
+ // Build JSON: default empty where, then copy filters and top-level options
+ const json = { where: {} };
+ // If a filter object was passed explicitly, use it
+ if (typeof query.where === 'object') {
+ json.where = query.where;
+ }
+ // Extract any top-level options
+ for (const key of TOP_LEVEL) {
+ if (key in query) {
+ json[key] = query[key];
+ }
+ }
+ parseQueryInstance.withJSON(json);
+ request.query = parseQueryInstance;
+ } else { This ensures that for a call like maybeRunAfterFindTrigger(..., { limit: 5, skip: 0 }, ...) you end up with a query whose JSON is { where: {}, limit: 5, skip: 0 } and thus 🤖 Prompt for AI Agents
|
||
|
||
it('should create default query for invalid query parameter', async () => { | ||
const className = 'TestInvalidQueryDirect_' + Date.now(); | ||
let receivedQuery = null; | ||
|
||
Parse.Cloud.afterFind(className, req => { | ||
receivedQuery = req.query; | ||
return req.objects; | ||
}); | ||
|
||
const mockObject = { id: 'test789', className: className, name: 'test' }; | ||
|
||
await maybeRunAfterFindTrigger( | ||
'afterFind', | ||
null, | ||
className, | ||
[mockObject], | ||
testConfig, | ||
'invalid_query_string', | ||
{} | ||
); | ||
|
||
expect(receivedQuery).toBeInstanceOf(Parse.Query); | ||
expect(receivedQuery.className).toBe(className); | ||
|
||
receivedQuery = null; | ||
|
||
await maybeRunAfterFindTrigger( | ||
'afterFind', | ||
null, | ||
className, | ||
[mockObject], | ||
testConfig, | ||
null, | ||
{} | ||
); | ||
|
||
expect(receivedQuery).toBeInstanceOf(Parse.Query); | ||
expect(receivedQuery.className).toBe(className); | ||
}); | ||
}); | ||
|
||
it('beforeSave rejection with custom error code', function (done) { | ||
Parse.Cloud.beforeSave('BeforeSaveFailWithErrorCode', function () { | ||
|
Uh oh!
There was an error while loading. Please reload this page.