Skip to content

Commit 5fa532c

Browse files
authored
Schema Cache Improvements (parse-community#5612)
* Cache Improvements * improve tests * more tests * clean-up * test with singlecache * ensure indexes exists * remove ALL_KEYS * Add Insert Test * enableSingleSchemaCache default true * Revert "enableSingleSchemaCache default true" This reverts commit 323e713. * further optimization * refactor enforceFieldExists * coverage improvements * improve tests * remove flaky test * cleanup * Learned something new
1 parent d80a225 commit 5fa532c

9 files changed

+403
-177
lines changed

spec/.eslintrc.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
"Item": true,
1414
"Container": true,
1515
"equal": true,
16+
"expectAsync": true,
1617
"notEqual": true,
1718
"it_only_db": true,
1819
"it_exclude_dbs": true,

spec/MongoStorageAdapter.spec.js

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,4 +286,27 @@ describe_only_db('mongo')('MongoStorageAdapter', () => {
286286
done();
287287
});
288288
});
289+
290+
it('getClass if exists', async () => {
291+
const adapter = new MongoStorageAdapter({ uri: databaseURI });
292+
293+
const schema = {
294+
fields: {
295+
array: { type: 'Array' },
296+
object: { type: 'Object' },
297+
date: { type: 'Date' },
298+
},
299+
};
300+
301+
await adapter.createClass('MyClass', schema);
302+
const myClassSchema = await adapter.getClass('MyClass');
303+
expect(myClassSchema).toBeDefined();
304+
});
305+
306+
it('getClass if not exists', async () => {
307+
const adapter = new MongoStorageAdapter({ uri: databaseURI });
308+
await expectAsync(adapter.getClass('UnknownClass')).toBeRejectedWith(
309+
undefined
310+
);
311+
});
289312
});

spec/PostgresStorageAdapter.spec.js

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,33 @@ describe_only_db('postgres')('PostgresStorageAdapter', () => {
114114
})
115115
.catch(done);
116116
});
117+
118+
it('getClass if exists', async () => {
119+
const schema = {
120+
fields: {
121+
array: { type: 'Array' },
122+
object: { type: 'Object' },
123+
date: { type: 'Date' },
124+
},
125+
};
126+
await adapter.createClass('MyClass', schema);
127+
const myClassSchema = await adapter.getClass('MyClass');
128+
expect(myClassSchema).toBeDefined();
129+
});
130+
131+
it('getClass if not exists', async () => {
132+
const schema = {
133+
fields: {
134+
array: { type: 'Array' },
135+
object: { type: 'Object' },
136+
date: { type: 'Date' },
137+
},
138+
};
139+
await adapter.createClass('MyClass', schema);
140+
await expectAsync(adapter.getClass('UnknownClass')).toBeRejectedWith(
141+
undefined
142+
);
143+
});
117144
});
118145

119146
describe_only_db('postgres')('PostgresStorageAdapter shutdown', () => {

spec/RedisCacheAdapter.spec.js

Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
const RedisCacheAdapter = require('../lib/Adapters/Cache/RedisCacheAdapter')
22
.default;
3+
const Config = require('../lib/Config');
4+
35
/*
46
To run this test part of the complete suite
57
set PARSE_SERVER_TEST_CACHE='redis'
@@ -163,3 +165,168 @@ describe_only(() => {
163165
.then(done);
164166
});
165167
});
168+
169+
describe_only(() => {
170+
return process.env.PARSE_SERVER_TEST_CACHE === 'redis';
171+
})('Redis Performance', function() {
172+
const cacheAdapter = new RedisCacheAdapter();
173+
let getSpy;
174+
let putSpy;
175+
176+
beforeEach(async () => {
177+
await cacheAdapter.clear();
178+
getSpy = spyOn(cacheAdapter, 'get').and.callThrough();
179+
putSpy = spyOn(cacheAdapter, 'put').and.callThrough();
180+
await reconfigureServer({
181+
cacheAdapter,
182+
enableSingleSchemaCache: true,
183+
});
184+
});
185+
186+
it('test new object', async () => {
187+
const object = new TestObject();
188+
object.set('foo', 'bar');
189+
await object.save();
190+
expect(getSpy.calls.count()).toBe(4);
191+
expect(putSpy.calls.count()).toBe(3);
192+
});
193+
194+
it('test new object multiple fields', async () => {
195+
const container = new Container({
196+
dateField: new Date(),
197+
arrayField: [],
198+
numberField: 1,
199+
stringField: 'hello',
200+
booleanField: true,
201+
});
202+
await container.save();
203+
expect(getSpy.calls.count()).toBe(4);
204+
expect(putSpy.calls.count()).toBe(3);
205+
});
206+
207+
it('test update existing fields', async () => {
208+
const object = new TestObject();
209+
object.set('foo', 'bar');
210+
await object.save();
211+
212+
getSpy.calls.reset();
213+
putSpy.calls.reset();
214+
215+
object.set('foo', 'barz');
216+
await object.save();
217+
expect(getSpy.calls.count()).toBe(3);
218+
expect(putSpy.calls.count()).toBe(0);
219+
});
220+
221+
it('test add new field to existing object', async () => {
222+
const object = new TestObject();
223+
object.set('foo', 'bar');
224+
await object.save();
225+
226+
getSpy.calls.reset();
227+
putSpy.calls.reset();
228+
229+
object.set('new', 'barz');
230+
await object.save();
231+
expect(getSpy.calls.count()).toBe(3);
232+
expect(putSpy.calls.count()).toBe(1);
233+
});
234+
235+
it('test add multiple fields to existing object', async () => {
236+
const object = new TestObject();
237+
object.set('foo', 'bar');
238+
await object.save();
239+
240+
getSpy.calls.reset();
241+
putSpy.calls.reset();
242+
243+
object.set({
244+
dateField: new Date(),
245+
arrayField: [],
246+
numberField: 1,
247+
stringField: 'hello',
248+
booleanField: true,
249+
});
250+
await object.save();
251+
expect(getSpy.calls.count()).toBe(3);
252+
expect(putSpy.calls.count()).toBe(1);
253+
});
254+
255+
it('test query', async () => {
256+
const object = new TestObject();
257+
object.set('foo', 'bar');
258+
await object.save();
259+
260+
getSpy.calls.reset();
261+
putSpy.calls.reset();
262+
263+
const query = new Parse.Query(TestObject);
264+
await query.get(object.id);
265+
expect(getSpy.calls.count()).toBe(2);
266+
expect(putSpy.calls.count()).toBe(0);
267+
});
268+
269+
it('test delete object', async () => {
270+
const object = new TestObject();
271+
object.set('foo', 'bar');
272+
await object.save();
273+
274+
getSpy.calls.reset();
275+
putSpy.calls.reset();
276+
277+
await object.destroy();
278+
expect(getSpy.calls.count()).toBe(3);
279+
expect(putSpy.calls.count()).toBe(0);
280+
});
281+
282+
it('test schema update class', async () => {
283+
const container = new Container();
284+
await container.save();
285+
286+
getSpy.calls.reset();
287+
putSpy.calls.reset();
288+
289+
const config = Config.get('test');
290+
const schema = await config.database.loadSchema();
291+
await schema.reloadData();
292+
293+
const levelPermissions = {
294+
find: { '*': true },
295+
get: { '*': true },
296+
create: { '*': true },
297+
update: { '*': true },
298+
delete: { '*': true },
299+
addField: { '*': true },
300+
protectedFields: { '*': [] },
301+
};
302+
303+
await schema.updateClass(
304+
'Container',
305+
{
306+
fooOne: { type: 'Number' },
307+
fooTwo: { type: 'Array' },
308+
fooThree: { type: 'Date' },
309+
fooFour: { type: 'Object' },
310+
fooFive: { type: 'Relation', targetClass: '_User' },
311+
fooSix: { type: 'String' },
312+
fooSeven: { type: 'Object' },
313+
fooEight: { type: 'String' },
314+
fooNine: { type: 'String' },
315+
fooTeen: { type: 'Number' },
316+
fooEleven: { type: 'String' },
317+
fooTwelve: { type: 'String' },
318+
fooThirteen: { type: 'String' },
319+
fooFourteen: { type: 'String' },
320+
fooFifteen: { type: 'String' },
321+
fooSixteen: { type: 'String' },
322+
fooEighteen: { type: 'String' },
323+
fooNineteen: { type: 'String' },
324+
},
325+
levelPermissions,
326+
{},
327+
config.database
328+
);
329+
expect(getSpy.calls.count()).toBe(3);
330+
expect(putSpy.calls.count()).toBe(2);
331+
});
332+
});

spec/Schema.spec.js

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1363,6 +1363,47 @@ describe('SchemaController', () => {
13631363
.then(done)
13641364
.catch(done.fail);
13651365
});
1366+
1367+
it('setAllClasses return classes if cache fails', async () => {
1368+
const schema = await config.database.loadSchema();
1369+
1370+
spyOn(schema._cache, 'setAllClasses').and.callFake(() =>
1371+
Promise.reject('Oops!')
1372+
);
1373+
const errorSpy = spyOn(console, 'error').and.callFake(() => {});
1374+
const allSchema = await schema.setAllClasses();
1375+
1376+
expect(allSchema).toBeDefined();
1377+
expect(errorSpy).toHaveBeenCalledWith(
1378+
'Error saving schema to cache:',
1379+
'Oops!'
1380+
);
1381+
});
1382+
1383+
it('should not throw on null field types', async () => {
1384+
const schema = await config.database.loadSchema();
1385+
const result = await schema.enforceFieldExists(
1386+
'NewClass',
1387+
'fieldName',
1388+
null
1389+
);
1390+
expect(result).toBeUndefined();
1391+
});
1392+
1393+
it('ensureFields should throw when schema is not set', async () => {
1394+
const schema = await config.database.loadSchema();
1395+
try {
1396+
schema.ensureFields([
1397+
{
1398+
className: 'NewClass',
1399+
fieldName: 'fieldName',
1400+
type: 'String',
1401+
},
1402+
]);
1403+
} catch (e) {
1404+
expect(e.message).toBe('Could not add field fieldName');
1405+
}
1406+
});
13661407
});
13671408

13681409
describe('Class Level Permissions for requiredAuth', () => {

spec/SchemaCache.spec.js

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -33,28 +33,12 @@ describe('SchemaCache', () => {
3333
});
3434
});
3535

36-
it('does not return all schemas after a single schema is stored', done => {
37-
const schemaCache = new SchemaCache(cacheController);
38-
const schema = {
39-
className: 'Class1',
40-
};
41-
schemaCache
42-
.setOneSchema(schema.className, schema)
43-
.then(() => {
44-
return schemaCache.getAllClasses();
45-
})
46-
.then(allSchemas => {
47-
expect(allSchemas).toBeNull();
48-
done();
49-
});
50-
});
51-
5236
it("doesn't persist cached data by default", done => {
5337
const schemaCache = new SchemaCache(cacheController);
5438
const schema = {
5539
className: 'Class1',
5640
};
57-
schemaCache.setOneSchema(schema.className, schema).then(() => {
41+
schemaCache.setAllClasses([schema]).then(() => {
5842
const anotherSchemaCache = new SchemaCache(cacheController);
5943
return anotherSchemaCache.getOneSchema(schema.className).then(schema => {
6044
expect(schema).toBeNull();
@@ -68,12 +52,26 @@ describe('SchemaCache', () => {
6852
const schema = {
6953
className: 'Class1',
7054
};
71-
schemaCache.setOneSchema(schema.className, schema).then(() => {
55+
schemaCache.setAllClasses([schema]).then(() => {
7256
const anotherSchemaCache = new SchemaCache(cacheController, 5000, true);
7357
return anotherSchemaCache.getOneSchema(schema.className).then(schema => {
7458
expect(schema).not.toBeNull();
7559
done();
7660
});
7761
});
7862
});
63+
64+
it('should not store if ttl is null', async () => {
65+
const ttl = null;
66+
const schemaCache = new SchemaCache(cacheController, ttl);
67+
expect(await schemaCache.getAllClasses()).toBeNull();
68+
expect(await schemaCache.setAllClasses()).toBeNull();
69+
expect(await schemaCache.getOneSchema()).toBeNull();
70+
});
71+
72+
it('should convert string ttl to number', async () => {
73+
const ttl = '5000';
74+
const schemaCache = new SchemaCache(cacheController, ttl);
75+
expect(schemaCache.ttl).toBe(5000);
76+
});
7977
});

src/Controllers/DatabaseController.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -844,7 +844,6 @@ class DatabaseController {
844844
: schemaController.validatePermission(className, aclGroup, 'create')
845845
)
846846
.then(() => schemaController.enforceClassExists(className))
847-
.then(() => schemaController.reloadData())
848847
.then(() => schemaController.getOneSchema(className, true))
849848
.then(schema => {
850849
transformAuthData(className, object, schema);

0 commit comments

Comments
 (0)