Skip to content

[WIP] Reduce schema calls on object creation #7283

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

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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: 5 additions & 5 deletions spec/SchemaPerformance.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ describe_only_db('mongo')('Schema Performance', function () {
const object = new TestObject();
object.set('foo', 'bar');
await object.save();
expect(getAllSpy.calls.count()).toBe(2);
expect(getAllSpy.calls.count()).toBe(0);
});

it('test new object multiple fields', async () => {
Expand All @@ -30,7 +30,7 @@ describe_only_db('mongo')('Schema Performance', function () {
booleanField: true,
});
await container.save();
expect(getAllSpy.calls.count()).toBe(2);
expect(getAllSpy.calls.count()).toBe(0);
});

it('test update existing fields', async () => {
Expand Down Expand Up @@ -83,7 +83,7 @@ describe_only_db('mongo')('Schema Performance', function () {

object.set('new', 'barz');
await object.save();
expect(getAllSpy.calls.count()).toBe(1);
expect(getAllSpy.calls.count()).toBe(0);
});

it('test add multiple fields to existing object', async () => {
Expand All @@ -101,7 +101,7 @@ describe_only_db('mongo')('Schema Performance', function () {
booleanField: true,
});
await object.save();
expect(getAllSpy.calls.count()).toBe(1);
expect(getAllSpy.calls.count()).toBe(0);
});

it('test user', async () => {
Expand All @@ -110,7 +110,7 @@ describe_only_db('mongo')('Schema Performance', function () {
user.setPassword('testing');
await user.signUp();

expect(getAllSpy.calls.count()).toBe(1);
expect(getAllSpy.calls.count()).toBe(0);
});

it('test query include', async () => {
Expand Down
11 changes: 11 additions & 0 deletions src/Adapters/Cache/SchemaCache.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,17 @@ export default {
return this.all().find(cached => cached.className === className);
},

set(className, schema) {
const cache = this.all();
const index = cache.findIndex(cached => cached.className === className);
if (index >= 0) {
cache[index] = schema;
} else {
cache.push(schema);
}
this.put(cache);
},

put(allSchema) {
SchemaCache.allClasses = allSchema;
},
Expand Down
7 changes: 6 additions & 1 deletion src/Adapters/Storage/StorageAdapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,12 @@ export interface StorageAdapter {
classExists(className: string): Promise<boolean>;
setClassLevelPermissions(className: string, clps: any): Promise<void>;
createClass(className: string, schema: SchemaType): Promise<void>;
addFieldIfNotExists(className: string, fieldName: string, type: any): Promise<void>;
addFieldIfNotExists(
className: string,
fieldName: string,
type: any,
transactionalSession: ?any
): Promise<void>;
deleteClass(className: string): Promise<void>;
deleteAllClasses(fast: boolean): Promise<void>;
deleteFields(className: string, schema: SchemaType, fieldNames: Array<string>): Promise<void>;
Expand Down
31 changes: 19 additions & 12 deletions src/Controllers/SchemaController.js
Original file line number Diff line number Diff line change
Expand Up @@ -807,9 +807,8 @@ export default class SchemaController {
className,
})
);
// TODO: Remove by updating schema cache directly
await this.reloadData({ clearCache: true });
const parseSchema = convertAdapterSchemaToParseSchema(adapterSchema);
SchemaCache.set(className, parseSchema);
return parseSchema;
} catch (error) {
if (error && error.code === Parse.Error.DUPLICATE_VALUE) {
Expand Down Expand Up @@ -875,12 +874,14 @@ export default class SchemaController {
return (
deletePromise // Delete Everything
.then(() => this.reloadData({ clearCache: true })) // Reload our Schema, so we have all the new values
.then(() => {
const promises = insertedFields.map(fieldName => {
.then(async () => {
const results = [];
for (let i = 0; i < insertedFields.length; i += 1) {
const fieldName = insertedFields[i];
const type = submittedFields[fieldName];
return this.enforceFieldExists(className, fieldName, type);
});
return Promise.all(promises);
results.push(await this.enforceFieldExists(className, fieldName, type));
}
return results;
})
.then(results => {
enforceFields = results.filter(result => !!result);
Expand Down Expand Up @@ -933,6 +934,7 @@ export default class SchemaController {
return (
// The schema update succeeded. Reload the schema
this.addClassIfNotExists(className)
.then(() => this.reloadData())
.catch(() => {
// The schema update failed. This can be okay - it might
// have failed because there's a race condition and a different
Expand Down Expand Up @@ -1118,6 +1120,13 @@ export default class SchemaController {
return Promise.resolve();
})
.then(() => {
const cached = SchemaCache.get(className);
if (cached) {
if (cached && !cached.fields[fieldName]) {
cached.fields[fieldName] = type;
SchemaCache.set(className, cached);
}
}
return {
className,
fieldName,
Expand Down Expand Up @@ -1210,7 +1219,7 @@ export default class SchemaController {
async validateObject(className: string, object: any, query: any) {
let geocount = 0;
const schema = await this.enforceClassExists(className);
const promises = [];
const results = [];

for (const fieldName in object) {
if (object[fieldName] && getType(object[fieldName]) === 'GeoPoint') {
Expand All @@ -1237,14 +1246,12 @@ export default class SchemaController {
// Every object has ACL implicitly.
continue;
}
promises.push(schema.enforceFieldExists(className, fieldName, expected));
results.push(await schema.enforceFieldExists(className, fieldName, expected));
}
const results = await Promise.all(promises);
const enforceFields = results.filter(result => !!result);

if (enforceFields.length !== 0) {
// TODO: Remove by updating schema cache directly
await this.reloadData({ clearCache: true });
await this.reloadData();
}
this.ensureFields(enforceFields);

Expand Down