Skip to content

Cleanup delete schema #1604

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 3 commits into from
Apr 25, 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
36 changes: 23 additions & 13 deletions src/Adapters/Storage/Mongo/MongoStorageAdapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,21 @@ let MongoClient = mongodb.MongoClient;
const MongoSchemaCollectionName = '_SCHEMA';
const DefaultMongoURI = 'mongodb://localhost:27017/parse';

const storageAdapterAllCollections = mongoAdapter => {
return mongoAdapter.connect()
.then(() => mongoAdapter.database.collections())
.then(collections => {
return collections.filter(collection => {
if (collection.namespace.match(/\.system\./)) {
return false;
}
// TODO: If you have one app with a collection prefix that happens to be a prefix of another
// apps prefix, this will go very very badly. We should fix that somehow.
return (collection.collectionName.indexOf(mongoAdapter._collectionPrefix) == 0);
});
});
}

export class MongoStorageAdapter {
// Private
_uri: string;
Expand Down Expand Up @@ -70,7 +85,10 @@ export class MongoStorageAdapter {
});
}

dropCollection(className: string) {
// Deletes a schema. Resolve if successful. If the schema doesn't
// exist, resolve with undefined. If schema exists, but can't be deleted for some other reason,
// reject with INTERNAL_SERVER_ERROR.
deleteOneSchema(className: string) {
return this.collection(this._collectionPrefix + className).then(collection => collection.drop())
.catch(error => {
// 'ns not found' means collection was already gone. Ignore deletion attempt.
Expand All @@ -81,18 +99,10 @@ export class MongoStorageAdapter {
});
}

// Used for testing only right now.
allCollections() {
return this.connect().then(() => {
return this.database.collections();
}).then(collections => {
return collections.filter(collection => {
if (collection.namespace.match(/\.system\./)) {
return false;
}
return (collection.collectionName.indexOf(this._collectionPrefix) == 0);
});
});
// Delete all data known to this adatper. Used for testing.
deleteAllSchemas() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the naming is somewhat misleading, not deleting all schemas from the schemas collection but actually deleting all objects from the database.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think from the DB adapter point of view this is the correct name. SQL dbs, for example, won't have a schemas collection, and deleting the schema is actually the same thing as deleting all the data.

Copy link
Contributor

@flovilmart flovilmart Apr 25, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would just have called it dropAll, but that's fine

return storageAdapterAllCollections(this)
.then(collections => Promise.all(collections.map(collection => collection.drop())));
}

// Remove the column and all the data. For Relations, the _Join collection is handled
Expand Down
30 changes: 12 additions & 18 deletions src/Controllers/DatabaseController.js
Original file line number Diff line number Diff line change
Expand Up @@ -381,11 +381,7 @@ DatabaseController.prototype.mongoFind = function(className, query, options = {}
// Returns a promise.
DatabaseController.prototype.deleteEverything = function() {
this.schemaPromise = null;

return this.adapter.allCollections().then(collections => {
let promises = collections.map(collection => collection.drop());
return Promise.all(promises);
});
return this.adapter.deleteAllSchemas();
};

// Finds the keys in a query. Returns a Set. REST format only
Expand Down Expand Up @@ -652,21 +648,19 @@ DatabaseController.prototype.find = function(className, query, {

DatabaseController.prototype.deleteSchema = function(className) {
return this.collectionExists(className)
.then(exist => {
if (!exist) {
return Promise.resolve();
.then(exist => {
if (!exist) {
return Promise.resolve();
}
return this.adapter.adaptiveCollection(className)
.then(collection => collection.count())
.then(count => {
if (count > 0) {
throw new Parse.Error(255, `Class ${className} is not empty, contains ${count} objects, cannot drop schema.`);
}
return this.adapter.adaptiveCollection(className)
.then(collection => {
return collection.count()
.then(count => {
if (count > 0) {
throw new Parse.Error(255, `Class ${className} is not empty, contains ${count} objects, cannot drop schema.`);
}
return collection.drop();
})
})
return this.adapter.deleteOneSchema(className);
})
});
}

DatabaseController.prototype.addPointerPermissions = function(schema, className, operation, query, aclGroup = []) {
Expand Down
8 changes: 4 additions & 4 deletions src/Controllers/SchemaController.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ function validateCLP(perms, fields) {
}
return;
}

Object.keys(perms[operation]).forEach((key) => {
verifyPermissionKey(key);
let perm = perms[operation][key];
Expand Down Expand Up @@ -543,7 +543,7 @@ class SchemaController {
if (this.data[className][fieldName].type == 'Relation') {
//For relations, drop the _Join table
return database.adapter.deleteFields(className, [fieldName], [])
.then(() => database.adapter.dropCollection(`_Join:${fieldName}:${className}`));
.then(() => database.adapter.deleteOneSchema(`_Join:${fieldName}:${className}`));
}

const fieldNames = [fieldName];
Expand Down Expand Up @@ -632,15 +632,15 @@ class SchemaController {
found = true;
}
}

if (found) {
return Promise.resolve();
}

// No matching CLP, let's check the Pointer permissions
// And handle those later
let permissionField = ['get', 'find'].indexOf(operation) > -1 ? 'readUserFields' : 'writeUserFields';

// Reject create when write lockdown
if (permissionField == 'writeUserFields' && operation == 'create') {
throw new Parse.Error(Parse.Error.OPERATION_FORBIDDEN,
Expand Down
29 changes: 13 additions & 16 deletions src/Routers/SchemasRouter.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ var removeJoinTables = (database, mongoSchema) => {
.filter(field => mongoSchema[field].startsWith('relation<'))
.map(field => {
let collectionName = `_Join:${field}:${mongoSchema._id}`;
return database.adapter.dropCollection(collectionName);
return database.adapter.deleteOneSchema(collectionName);
})
);
};
Expand All @@ -79,22 +79,19 @@ function deleteSchema(req) {
if (!SchemaController.classNameIsValid(req.params.className)) {
throw new Parse.Error(Parse.Error.INVALID_CLASS_NAME, SchemaController.invalidClassNameMessage(req.params.className));
}

return req.config.database.deleteSchema(req.params.className)
.then(() => {
// We've dropped the collection now, so delete the item from _SCHEMA
// and clear the _Join collections
return req.config.database.schemaCollection()
.then(coll => coll.findAndDeleteSchema(req.params.className))
.then(document => {
if (document === null) {
//tried to delete non-existent class
return Promise.resolve();
}
return removeJoinTables(req.config.database, document);
});
})
.then(() => ({ response: {} }));
.then(() => req.config.database.schemaCollection())
// We've dropped the collection now, so delete the item from _SCHEMA
// and clear the _Join collections
.then(coll => coll.findAndDeleteSchema(req.params.className))
.then(document => {
if (document === null) {
//tried to delete non-existent class
return Promise.resolve();
}
return removeJoinTables(req.config.database, document);
})
.then(() => ({ response: {} }));
}

export class SchemasRouter extends PromiseRouter {
Expand Down