Skip to content

Commit

Permalink
Fix cascade delete on third level dependents
Browse files Browse the repository at this point in the history
  • Loading branch information
Ricardo Gama committed Jul 20, 2016
1 parent 92e0169 commit a7488d4
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 33 deletions.
30 changes: 14 additions & 16 deletions src/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*/

import { mapSeries } from 'bluebird';
import { compact, flatten, reduce } from 'lodash';
import { flattenDeep, reduce } from 'lodash';

/**
* Export `bookshelf-cascade-delete` plugin.
Expand All @@ -16,7 +16,9 @@ export default Bookshelf => {

Bookshelf.Model = Bookshelf.Model.extend({
cascadeDelete(transaction, options) {
return mapSeries(this.constructor.recursiveDeletes(this.get(this.idAttribute) || this._knex.column(this.idAttribute), options), query => query(transaction))
const queries = this.constructor.recursiveDeletes(this.get(this.idAttribute) || this._knex.column(this.idAttribute), options);

return mapSeries(flattenDeep(queries).reverse(), query => query(transaction))
.then(() => Model.destroy.call(this, {
...options,
transacting: transaction
Expand Down Expand Up @@ -45,24 +47,22 @@ export default Bookshelf => {
const { relatedData } = this.prototype[dependent]();
const skipDependents = relatedData.type === 'belongsToMany';

return {
...result,
[dependent]: {
dependents: relatedData.target.dependencyMap(skipDependents),
key: relatedData.key('foreignKey'),
model: relatedData.target,
skipDependents,
tableName: skipDependents ? relatedData.joinTable() : relatedData.target.prototype.tableName
}
};
}, {});
return [
...result, {
dependents: relatedData.target.dependencyMap(skipDependents),
key: relatedData.key('foreignKey'),
model: relatedData.target,
skipDependents,
tableName: skipDependents ? relatedData.joinTable() : relatedData.target.prototype.tableName
}];
}, []);
},
recursiveDeletes(parent) {
// Stringify in case of parent being an instance of query.
const parentValue = typeof parent === 'number' || typeof parent === 'string' ? `'${parent}'` : parent.toString();

// Build delete queries for each dependent.
const queries = reduce(this.dependencyMap(), (result, { tableName, key, model, skipDependents }) => {
return reduce(this.dependencyMap(), (result, { tableName, key, model, skipDependents }) => {
const whereClause = `${client === 'postgres' ? `"${key}"` : key} IN (${parentValue})`;

return [
Expand All @@ -71,8 +71,6 @@ export default Bookshelf => {
skipDependents ? [] : model.recursiveDeletes(Bookshelf.knex(tableName).column(model.prototype.idAttribute).whereRaw(whereClause))
];
}, []);

return flatten(compact(queries)).reverse();
}
});
};
31 changes: 23 additions & 8 deletions test/mysql/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ describe('with MySQL client', () => {

repository.plugin(cascadeDelete);

const { Account, Author, Comment, Post, Tag, TagPost } = fixtures(repository);
const { Account, Author, Comment, Commentator, Post, Tag, TagPost } = fixtures(repository);

before(async () => {
await recreateTables(repository);
Expand Down Expand Up @@ -66,9 +66,10 @@ describe('with MySQL client', () => {
it('should not delete model and its dependents if an error is thrown on destroy', async () => {
const author = await Author.forge().save();
const post = await Post.forge().save({ authorId: author.get('author_id') });
const comment = await Comment.forge().save({ postId: post.get('post_id') });

await Account.forge().save({ authorId: author.get('author_id') });
await Comment.forge().save({ postId: post.get('post_id') });
await Commentator.forge().save({ commentId: comment.get('comment_id') });

sinon.stub(Model, 'destroy').throws(new Error('foobar'));

Expand All @@ -82,11 +83,13 @@ describe('with MySQL client', () => {

const accounts = await Account.fetchAll();
const authors = await Author.fetchAll();
const commentators = await Commentator.fetchAll();
const comments = await Comment.fetchAll();
const posts = await Post.fetchAll();

accounts.length.should.equal(1);
authors.length.should.equal(1);
commentators.length.should.equal(1);
comments.length.should.equal(1);
posts.length.should.equal(1);

Expand Down Expand Up @@ -118,25 +121,29 @@ describe('with MySQL client', () => {
const author = await Author.forge().save();
const post1 = await Post.forge().save({ authorId: author.get('author_id') });
const post2 = await Post.forge().save({ authorId: author.get('author_id') });
const comment1 = await Comment.forge().save({ postId: post1.get('post_id') });
const comment2 = await Comment.forge().save({ postId: post2.get('post_id') });
const tag1 = await Tag.forge().save();
const tag2 = await Tag.forge().save();

await Account.forge().save({ authorId: author.get('author_id') });
await Comment.forge().save({ postId: post1.get('post_id') });
await Comment.forge().save({ postId: post2.get('post_id') });
await Commentator.forge().save({ commentId: comment1.get('comment_id') });
await Commentator.forge().save({ commentId: comment2.get('comment_id') });
await TagPost.forge().save({ postId: post1.get('post_id'), tagId: tag1.get('tag_id') });
await TagPost.forge().save({ postId: post2.get('post_id'), tagId: tag2.get('tag_id') });

await author.destroy();

const accounts = await Account.fetchAll();
const authors = await Author.fetchAll();
const commentators = await Commentator.fetchAll();
const comments = await Comment.fetchAll();
const posts = await Post.fetchAll();
const tagPosts = await TagPost.fetchAll();

accounts.length.should.equal(0);
authors.length.should.equal(0);
commentators.length.should.equal(0);
comments.length.should.equal(0);
posts.length.should.equal(0);
tagPosts.length.should.equal(0);
Expand All @@ -146,25 +153,29 @@ describe('with MySQL client', () => {
const author = await Author.forge().save({ name: 'foobar' });
const post1 = await Post.forge().save({ authorId: author.get('author_id') });
const post2 = await Post.forge().save({ authorId: author.get('author_id') });
const comment1 = await Comment.forge().save({ postId: post1.get('post_id') });
const comment2 = await Comment.forge().save({ postId: post2.get('post_id') });
const tag1 = await Tag.forge().save();
const tag2 = await Tag.forge().save();

await Account.forge().save({ authorId: author.get('author_id') });
await Comment.forge().save({ postId: post1.get('post_id') });
await Comment.forge().save({ postId: post2.get('post_id') });
await Commentator.forge().save({ commentId: comment1.get('comment_id') });
await Commentator.forge().save({ commentId: comment2.get('comment_id') });
await TagPost.forge().save({ postId: post1.get('post_id'), tagId: tag1.get('tag_id') });
await TagPost.forge().save({ postId: post2.get('post_id'), tagId: tag2.get('tag_id') });

await Author.forge().where({ name: 'foobar' }).destroy();

const accounts = await Account.fetchAll();
const authors = await Author.fetchAll();
const commentators = await Commentator.fetchAll();
const comments = await Comment.fetchAll();
const posts = await Post.fetchAll();
const tagPosts = await TagPost.fetchAll();

accounts.length.should.equal(0);
authors.length.should.equal(0);
commentators.length.should.equal(0);
comments.length.should.equal(0);
posts.length.should.equal(0);
tagPosts.length.should.equal(0);
Expand All @@ -175,26 +186,30 @@ describe('with MySQL client', () => {
const author2 = await Author.forge().save();
const post1 = await Post.forge().save({ authorId: author1.get('author_id') });
const post2 = await Post.forge().save({ authorId: author2.get('author_id') });
const comment1 = await Comment.forge().save({ postId: post1.get('post_id') });
const comment2 = await Comment.forge().save({ postId: post2.get('post_id') });
const tag1 = await Tag.forge().save();
const tag2 = await Tag.forge().save();

await Account.forge().save({ authorId: author1.get('author_id') });
await Account.forge().save({ authorId: author2.get('author_id') });
await Comment.forge().save({ postId: post1.get('post_id') });
await Comment.forge().save({ postId: post2.get('post_id') });
await Commentator.forge().save({ commentId: comment1.get('comment_id') });
await Commentator.forge().save({ commentId: comment2.get('comment_id') });
await TagPost.forge().save({ postId: post1.get('post_id'), tagId: tag1.get('tag_id') });
await TagPost.forge().save({ postId: post2.get('post_id'), tagId: tag2.get('tag_id') });

await author1.destroy();

const accounts = await Account.fetchAll();
const authors = await Author.fetchAll();
const commentators = await Commentator.fetchAll();
const comments = await Comment.fetchAll();
const posts = await Post.fetchAll();
const tagPosts = await TagPost.fetchAll();

accounts.length.should.equal(1);
authors.length.should.equal(1);
commentators.length.should.equal(1);
comments.length.should.equal(1);
posts.length.should.equal(1);
tagPosts.length.should.equal(1);
Expand Down
31 changes: 23 additions & 8 deletions test/postgres/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ describe('with PostgreSQL client', () => {

repository.plugin(cascadeDelete);

const { Account, Author, Comment, Post, Tag, TagPost } = fixtures(repository);
const { Account, Author, Comment, Commentator, Post, Tag, TagPost } = fixtures(repository);

before(async () => {
await recreateTables(repository);
Expand Down Expand Up @@ -66,9 +66,10 @@ describe('with PostgreSQL client', () => {
it('should not delete model and its dependents if an error is thrown on destroy', async () => {
const author = await Author.forge().save();
const post = await Post.forge().save({ authorId: author.get('author_id') });
const comment = await Comment.forge().save({ postId: post.get('post_id') });

await Account.forge().save({ authorId: author.get('author_id') });
await Comment.forge().save({ postId: post.get('post_id') });
await Commentator.forge().save({ commentId: comment.get('comment_id') });

sinon.stub(Model, 'destroy').throws(new Error('foobar'));

Expand All @@ -82,11 +83,13 @@ describe('with PostgreSQL client', () => {

const accounts = await Account.fetchAll();
const authors = await Author.fetchAll();
const commentators = await Commentator.fetchAll();
const comments = await Comment.fetchAll();
const posts = await Post.fetchAll();

accounts.length.should.equal(1);
authors.length.should.equal(1);
commentators.length.should.equal(1);
comments.length.should.equal(1);
posts.length.should.equal(1);

Expand Down Expand Up @@ -118,25 +121,29 @@ describe('with PostgreSQL client', () => {
const author = await Author.forge().save();
const post1 = await Post.forge().save({ authorId: author.get('author_id') });
const post2 = await Post.forge().save({ authorId: author.get('author_id') });
const comment1 = await Comment.forge().save({ postId: post1.get('post_id') });
const comment2 = await Comment.forge().save({ postId: post2.get('post_id') });
const tag1 = await Tag.forge().save();
const tag2 = await Tag.forge().save();

await Account.forge().save({ authorId: author.get('author_id') });
await Comment.forge().save({ postId: post1.get('post_id') });
await Comment.forge().save({ postId: post2.get('post_id') });
await Commentator.forge().save({ commentId: comment1.get('comment_id') });
await Commentator.forge().save({ commentId: comment2.get('comment_id') });
await TagPost.forge().save({ postId: post1.get('post_id'), tagId: tag1.get('tag_id') });
await TagPost.forge().save({ postId: post2.get('post_id'), tagId: tag2.get('tag_id') });

await author.destroy();

const accounts = await Account.fetchAll();
const authors = await Author.fetchAll();
const commentators = await Commentator.fetchAll();
const comments = await Comment.fetchAll();
const posts = await Post.fetchAll();
const tagPosts = await TagPost.fetchAll();

accounts.length.should.equal(0);
authors.length.should.equal(0);
commentators.length.should.equal(0);
comments.length.should.equal(0);
posts.length.should.equal(0);
tagPosts.length.should.equal(0);
Expand All @@ -146,25 +153,29 @@ describe('with PostgreSQL client', () => {
const author = await Author.forge().save({ name: 'foobar' });
const post1 = await Post.forge().save({ authorId: author.get('author_id') });
const post2 = await Post.forge().save({ authorId: author.get('author_id') });
const comment1 = await Comment.forge().save({ postId: post1.get('post_id') });
const comment2 = await Comment.forge().save({ postId: post2.get('post_id') });
const tag1 = await Tag.forge().save();
const tag2 = await Tag.forge().save();

await Account.forge().save({ authorId: author.get('author_id') });
await Comment.forge().save({ postId: post1.get('post_id') });
await Comment.forge().save({ postId: post2.get('post_id') });
await Commentator.forge().save({ commentId: comment1.get('comment_id') });
await Commentator.forge().save({ commentId: comment2.get('comment_id') });
await TagPost.forge().save({ postId: post1.get('post_id'), tagId: tag1.get('tag_id') });
await TagPost.forge().save({ postId: post2.get('post_id'), tagId: tag2.get('tag_id') });

await Author.forge().where({ name: 'foobar' }).destroy();

const accounts = await Account.fetchAll();
const authors = await Author.fetchAll();
const commentators = await Commentator.fetchAll();
const comments = await Comment.fetchAll();
const posts = await Post.fetchAll();
const tagPosts = await TagPost.fetchAll();

accounts.length.should.equal(0);
authors.length.should.equal(0);
commentators.length.should.equal(0);
comments.length.should.equal(0);
posts.length.should.equal(0);
tagPosts.length.should.equal(0);
Expand All @@ -175,26 +186,30 @@ describe('with PostgreSQL client', () => {
const author2 = await Author.forge().save();
const post1 = await Post.forge().save({ authorId: author1.get('author_id') });
const post2 = await Post.forge().save({ authorId: author2.get('author_id') });
const comment1 = await Comment.forge().save({ postId: post1.get('post_id') });
const comment2 = await Comment.forge().save({ postId: post2.get('post_id') });
const tag1 = await Tag.forge().save();
const tag2 = await Tag.forge().save();

await Account.forge().save({ authorId: author1.get('author_id') });
await Account.forge().save({ authorId: author2.get('author_id') });
await Comment.forge().save({ postId: post1.get('post_id') });
await Comment.forge().save({ postId: post2.get('post_id') });
await Commentator.forge().save({ commentId: comment1.get('comment_id') });
await Commentator.forge().save({ commentId: comment2.get('comment_id') });
await TagPost.forge().save({ postId: post1.get('post_id'), tagId: tag1.get('tag_id') });
await TagPost.forge().save({ postId: post2.get('post_id'), tagId: tag2.get('tag_id') });

await author1.destroy();

const accounts = await Account.fetchAll();
const authors = await Author.fetchAll();
const commentators = await Commentator.fetchAll();
const comments = await Comment.fetchAll();
const posts = await Post.fetchAll();
const tagPosts = await TagPost.fetchAll();

accounts.length.should.equal(1);
authors.length.should.equal(1);
commentators.length.should.equal(1);
comments.length.should.equal(1);
posts.length.should.equal(1);
tagPosts.length.should.equal(1);
Expand Down
18 changes: 17 additions & 1 deletion test/utils/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ export function recreateTables(repository) {
.createTable('Comment', table => {
table.increments('comment_id').primary();
table.integer('postId').unsigned().references('Post.post_id');
})
.createTable('Commentator', table => {
table.increments('commentator_id').primary();
table.integer('commentId').unsigned().references('Comment.comment_id');
});
}

Expand All @@ -42,6 +46,7 @@ export function recreateTables(repository) {

export async function clearTables(repository) {
await repository.knex('Account').del();
await repository.knex('Commentator').del();
await repository.knex('Comment').del();
await repository.knex('TagPost').del();
await repository.knex('Post').del();
Expand All @@ -57,6 +62,7 @@ export function dropTables(repository) {
return repository.knex.schema
.dropTable('TagPost')
.dropTable('Account')
.dropTable('Commentator')
.dropTable('Comment')
.dropTable('Post')
.dropTable('Tag')
Expand All @@ -73,9 +79,19 @@ export function fixtures(repository) {
tableName: 'Account'
});

const Commentator = repository.Model.extend({
idAttribute: 'commentator_id',
tableName: 'Commentator'
});

const Comment = repository.Model.extend({
commentator() {
return this.hasOne(Commentator, 'commentId');
},
idAttribute: 'comment_id',
tableName: 'Comment'
}, {
dependents: ['commentator']
});

const Tag = repository.Model.extend({
Expand Down Expand Up @@ -114,5 +130,5 @@ export function fixtures(repository) {
dependents: ['account', 'posts']
});

return { Account, Author, Comment, Post, Tag, TagPost };
return { Account, Author, Comment, Commentator, Post, Tag, TagPost };
}

0 comments on commit a7488d4

Please sign in to comment.