Skip to content

Decouple and remove direct mongo access from Schema/SchemaRouter. #729

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

Merged
merged 4 commits into from
Mar 1, 2016
Merged
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
53 changes: 31 additions & 22 deletions spec/Schema.spec.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
'use strict';

var Config = require('../src/Config');
var Schema = require('../src/Schema');
var dd = require('deep-diff');
Expand Down Expand Up @@ -483,7 +485,7 @@ describe('Schema', () => {
.then(schema => schema.deleteField('installationId', '_Installation'))
.catch(error => {
expect(error.code).toEqual(136);
expect(error.error).toEqual('field installationId cannot be changed');
expect(error.message).toEqual('field installationId cannot be changed');
done();
});
});
Expand All @@ -493,7 +495,7 @@ describe('Schema', () => {
.then(schema => schema.deleteField('field', 'NoClass'))
.catch(error => {
expect(error.code).toEqual(Parse.Error.INVALID_CLASS_NAME);
expect(error.error).toEqual('class NoClass does not exist');
expect(error.message).toEqual('Class NoClass does not exist.');
done();
});
});
Expand All @@ -504,32 +506,39 @@ describe('Schema', () => {
.then(schema => schema.deleteField('missingField', 'HasAllPOD'))
.fail(error => {
expect(error.code).toEqual(255);
expect(error.error).toEqual('field missingField does not exist, cannot delete');
expect(error.message).toEqual('Field missingField does not exist, cannot delete.');
done();
});
});

it('drops related collection when deleting relation field', done => {
var obj1 = hasAllPODobject();
obj1.save()
.then(savedObj1 => {
var obj2 = new Parse.Object('HasPointersAndRelations');
obj2.set('aPointer', savedObj1);
var relation = obj2.relation('aRelation');
relation.add(obj1);
return obj2.save();
})
.then(() => {
config.database.adapter.database.collection('test__Join:aRelation:HasPointersAndRelations', { strict: true }, (err, coll) => {
expect(err).toEqual(null);
config.database.loadSchema()
.then(schema => schema.deleteField('aRelation', 'HasPointersAndRelations', config.database.adapter.database, 'test_'))
.then(() => config.database.adapter.database.collection('test__Join:aRelation:HasPointersAndRelations', { strict: true }, (err, coll) => {
expect(err).not.toEqual(null);
done();
}))
.then(savedObj1 => {
var obj2 = new Parse.Object('HasPointersAndRelations');
obj2.set('aPointer', savedObj1);
var relation = obj2.relation('aRelation');
relation.add(obj1);
return obj2.save();
})
.then(() => config.database.collectionExists('_Join:aRelation:HasPointersAndRelations'))
.then(exists => {
if (!exists) {
fail('Relation collection should exist after save.');
}
})
.then(() => config.database.loadSchema())
.then(schema => schema.deleteField('aRelation', 'HasPointersAndRelations', config.database))
.then(() => config.database.collectionExists('_Join:aRelation:HasPointersAndRelations'))
.then(exists => {
if (exists) {
fail('Relation collection should not exist after deleting relation field.');
}
done();
}, error => {
fail(error);
done();
});
})
});

it('can delete string fields and resave as number field', done => {
Expand All @@ -538,7 +547,7 @@ describe('Schema', () => {
var obj2 = hasAllPODobject();
var p = Parse.Object.saveAll([obj1, obj2])
.then(() => config.database.loadSchema())
.then(schema => schema.deleteField('aString', 'HasAllPOD', config.database.adapter.database, 'test_'))
.then(schema => schema.deleteField('aString', 'HasAllPOD', config.database))
.then(() => new Parse.Query('HasAllPOD').get(obj1.id))
.then(obj1Reloaded => {
expect(obj1Reloaded.get('aString')).toEqual(undefined);
Expand Down Expand Up @@ -568,7 +577,7 @@ describe('Schema', () => {
expect(obj1.get('aPointer').id).toEqual(obj1.id);
})
.then(() => config.database.loadSchema())
.then(schema => schema.deleteField('aPointer', 'NewClass', config.database.adapter.database, 'test_'))
.then(schema => schema.deleteField('aPointer', 'NewClass', config.database))
.then(() => new Parse.Query('NewClass').get(obj1.id))
.then(obj1 => {
expect(obj1.get('aPointer')).toEqual(undefined);
Expand Down
56 changes: 33 additions & 23 deletions spec/schemas.spec.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
'use strict';

var Parse = require('parse/node').Parse;
var request = require('request');
var dd = require('deep-diff');
Expand Down Expand Up @@ -369,7 +371,7 @@ describe('schemas', () => {
}, (error, response, body) => {
expect(response.statusCode).toEqual(400);
expect(body.code).toEqual(Parse.Error.INVALID_CLASS_NAME);
expect(body.error).toEqual('class NoClass does not exist');
expect(body.error).toEqual('Class NoClass does not exist.');
done();
});
});
Expand All @@ -390,13 +392,13 @@ describe('schemas', () => {
}, (error, response, body) => {
expect(response.statusCode).toEqual(400);
expect(body.code).toEqual(255);
expect(body.error).toEqual('field aString exists, cannot update');
expect(body.error).toEqual('Field aString exists, cannot update.');
done();
});
})
});

it('refuses to delete non-existant fields', done => {
it('refuses to delete non-existent fields', done => {
var obj = hasAllPODobject();
obj.save()
.then(() => {
Expand All @@ -406,13 +408,13 @@ describe('schemas', () => {
json: true,
body: {
fields: {
nonExistantKey: {__op: "Delete"},
nonExistentKey: {__op: "Delete"},
}
}
}, (error, response, body) => {
expect(response.statusCode).toEqual(400);
expect(body.code).toEqual(255);
expect(body.error).toEqual('field nonExistantKey does not exist, cannot delete');
expect(body.error).toEqual('Field nonExistentKey does not exist, cannot delete.');
done();
});
});
Expand Down Expand Up @@ -660,7 +662,8 @@ describe('schemas', () => {
}, (error, response, body) => {
expect(response.statusCode).toEqual(400);
expect(body.code).toEqual(255);
expect(body.error).toEqual('class HasAllPOD not empty, contains 1 objects, cannot drop schema');
expect(body.error).toMatch(/HasAllPOD/);
expect(body.error).toMatch(/contains 1/);
done();
});
});
Expand Down Expand Up @@ -710,28 +713,35 @@ describe('schemas', () => {
}, (error, response, body) => {
expect(response.statusCode).toEqual(200);
expect(response.body).toEqual({});
config.database.adapter.database.collection('test__Join:aRelation:MyOtherClass', { strict: true }, (err, coll) => {
//Expect Join table to be gone
expect(err).not.toEqual(null);
config.database.adapter.database.collection('test_MyOtherClass', { strict: true }, (err, coll) => {
// Expect data table to be gone
expect(err).not.toEqual(null);
request.get({
url: 'http://localhost:8378/1/schemas/MyOtherClass',
headers: masterKeyHeaders,
json: true,
}, (error, response, body) => {
//Expect _SCHEMA entry to be gone.
expect(response.statusCode).toEqual(400);
expect(body.code).toEqual(Parse.Error.INVALID_CLASS_NAME);
expect(body.error).toEqual('class MyOtherClass does not exist');
done();
});
config.database.collectionExists('_Join:aRelation:MyOtherClass').then(exists => {
if (exists) {
fail('Relation collection should be deleted.');
done();
}
return config.database.collectionExists('MyOtherClass');
}).then(exists => {
if (exists) {
fail('Class collection should be deleted.');
done();
}
}).then(() => {
request.get({
url: 'http://localhost:8378/1/schemas/MyOtherClass',
headers: masterKeyHeaders,
json: true,
}, (error, response, body) => {
//Expect _SCHEMA entry to be gone.
expect(response.statusCode).toEqual(400);
expect(body.code).toEqual(Parse.Error.INVALID_CLASS_NAME);
expect(body.error).toEqual('class MyOtherClass does not exist');
done();
});
});
});
}).then(() => {
}, error => {
fail(error);
done();
});
});
});
11 changes: 11 additions & 0 deletions src/Adapters/Storage/Mongo/MongoStorageAdapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,17 @@ export class MongoStorageAdapter {
});
}

collectionExists(name: string) {
return this.connect().then(() => {
return this.database.listCollections({ name: name }).toArray();
}).then(collections => {
return collections.length > 0;
});
}

dropCollection(name: string) {
return this.collection(name).then(collection => collection.drop());
}
// Used for testing only right now.
collectionsContaining(match: string) {
return this.connect().then(() => {
Expand Down
8 changes: 8 additions & 0 deletions src/Controllers/DatabaseController.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,18 @@ DatabaseController.prototype.collection = function(className) {
return this.rawCollection(className);
};

DatabaseController.prototype.collectionExists = function(className) {
return this.adapter.collectionExists(this.collectionPrefix + className);
};

DatabaseController.prototype.rawCollection = function(className) {
return this.adapter.collection(this.collectionPrefix + className);
};

DatabaseController.prototype.dropCollection = function(className) {
return this.adapter.dropCollection(this.collectionPrefix + className);
};

function returnsTrue() {
return true;
}
Expand Down
Loading