Skip to content

Commit

Permalink
feat(core): allow adding items to not initialized collections (#489)
Browse files Browse the repository at this point in the history
  • Loading branch information
@B4nan
B4nan committed Aug 9, 2020
1 parent bb23262 commit ca5eb64
Show file tree
Hide file tree
Showing 9 changed files with 122 additions and 21 deletions.
10 changes: 5 additions & 5 deletions packages/core/src/entity/ArrayCollection.ts
View file
Expand Up @@ -48,7 +48,7 @@ export class ArrayCollection<T extends AnyEntity<T>, O extends AnyEntity<O>> {

add(...items: T[]): void {
for (const item of items) {
if (!this.contains(item)) {
if (!this.contains(item, false)) {
this.items.push(item);
this.propagate(item, 'add');
}
Expand Down Expand Up @@ -86,7 +86,7 @@ export class ArrayCollection<T extends AnyEntity<T>, O extends AnyEntity<O>> {
this.remove(...this.items);
}

contains(item: T): boolean {
contains(item: T, check?: boolean): boolean {
return !!this.items.find(i => {
const objectIdentity = i === item;
const primaryKeyIdentity = !!wrap(i).__primaryKey && !!wrap(item).__primaryKey && wrap(i).__serializedPrimaryKey === wrap(item).__serializedPrimaryKey;
Expand Down Expand Up @@ -149,16 +149,16 @@ export class ArrayCollection<T extends AnyEntity<T>, O extends AnyEntity<O>> {
}

protected shouldPropagateToCollection(collection: Collection<O, T>, method: 'add' | 'remove'): boolean {
if (!collection || !collection.isInitialized()) {
if (!collection) {
return false;
}

if (method === 'add') {
return !collection.contains(this.owner);
return !collection.contains(this.owner, false);
}

// remove
return collection.contains(this.owner);
return collection.contains(this.owner, false);
}

}
21 changes: 15 additions & 6 deletions packages/core/src/entity/Collection.ts
View file
Expand Up @@ -33,8 +33,11 @@ export class Collection<T extends AnyEntity<T>, O extends AnyEntity<O> = AnyEnti
/**
* Returns the items (the collection must be initialized)
*/
getItems(): T[] {
this.checkInitialized();
getItems(check = true): T[] {
if (check) {
this.checkInitialized();
}

return super.getItems();
}

Expand Down Expand Up @@ -79,8 +82,11 @@ export class Collection<T extends AnyEntity<T>, O extends AnyEntity<O> = AnyEnti
}
}

contains(item: T): boolean {
this.checkInitialized();
contains(item: T, check = true): boolean {
if (check) {
this.checkInitialized();
}

return super.contains(item);
}

Expand Down Expand Up @@ -206,7 +212,10 @@ export class Collection<T extends AnyEntity<T>, O extends AnyEntity<O> = AnyEnti
}

private modify(method: 'add' | 'remove', items: T[]): void {
this.checkInitialized();
if (method === 'remove') {
this.checkInitialized();
}

this.validateModification(items);
super[method](...items);
this.setDirty();
Expand Down Expand Up @@ -249,7 +258,7 @@ export class Collection<T extends AnyEntity<T>, O extends AnyEntity<O> = AnyEnti
// throw if we are modifying inverse side of M:N collection when owning side is initialized (would be ignored when persisting)
const manyToManyInverse = this.property.reference === ReferenceType.MANY_TO_MANY && this.property.mappedBy;

if (manyToManyInverse && items.find(item => !item[this.property.mappedBy] || !item[this.property.mappedBy].isInitialized())) {
if (manyToManyInverse && items.find(item => !wrap(item).isInitialized() || !item[this.property.mappedBy])) {
throw ValidationError.cannotModifyInverseCollection(this.owner, this.property);
}
}
Expand Down
11 changes: 7 additions & 4 deletions packages/core/src/unit-of-work/UnitOfWork.ts
View file
Expand Up @@ -241,7 +241,7 @@ export class UnitOfWork {
return;
}

reference.getItems()
reference.getItems(false)
.filter(item => !this.originalEntityData[wrap(item).__uuid])
.forEach(item => this.findNewEntities(item, visited));
}
Expand Down Expand Up @@ -350,13 +350,16 @@ export class UnitOfWork {
const collection = reference as Collection<AnyEntity>;
const requireFullyInitialized = type === Cascade.PERSIST; // only cascade persist needs fully initialized items

if ([ReferenceType.ONE_TO_MANY, ReferenceType.MANY_TO_MANY].includes(prop.reference) && collection && collection.isInitialized(requireFullyInitialized)) {
collection.getItems().forEach(item => this.cascade(item, type, visited, options));
if ([ReferenceType.ONE_TO_MANY, ReferenceType.MANY_TO_MANY].includes(prop.reference) && collection) {
collection
.getItems(false)
.filter(item => !requireFullyInitialized || wrap(item).isInitialized())
.forEach(item => this.cascade(item, type, visited, options));
}
}

private isCollectionSelfReferenced(collection: Collection<AnyEntity>, visited: AnyEntity[]): boolean {
const filtered = collection.getItems().filter(item => !this.originalEntityData[wrap(item).__uuid]);
const filtered = collection.getItems(false).filter(item => !this.originalEntityData[wrap(item).__uuid]);
return filtered.some(items => visited.includes(items));
}

Expand Down
2 changes: 1 addition & 1 deletion packages/knex/src/AbstractSqlDriver.ts
View file
Expand Up @@ -138,7 +138,7 @@ export abstract class AbstractSqlDriver<C extends AbstractSqlConnection = Abstra
const meta = wrap(coll.owner).__meta;
const pks = wrap(coll.owner).__primaryKeys;
const snapshot = coll.getSnapshot().map(item => wrap(item).__primaryKeys);
const current = coll.getItems().map(item => wrap(item).__primaryKeys);
const current = coll.getItems(false).map(item => wrap(item).__primaryKeys);
const deleteDiff = snapshot.filter(item => !current.includes(item));
const insertDiff = current.filter(item => !snapshot.includes(item));
const target = snapshot.filter(item => current.includes(item)).concat(...insertDiff);
Expand Down
45 changes: 44 additions & 1 deletion tests/EntityManager.mongo.test.ts
View file
Expand Up @@ -8,6 +8,7 @@ import { AuthorRepository } from './repositories/AuthorRepository';
import { initORMMongo, wipeDatabase } from './bootstrap';
import FooBar from './entities/FooBar';
import { FooBaz } from './entities/FooBaz';
import { Author2, Book2, BookTag2 } from './entities-sql';

describe('EntityManagerMongo', () => {

Expand Down Expand Up @@ -626,7 +627,6 @@ describe('EntityManagerMongo', () => {
expect(tags[0].books.isInitialized()).toBe(false);
expect(tags[0].books.isDirty()).toBe(false);
expect(() => tags[0].books.getItems()).toThrowError(/Collection<Book> of entity BookTag\[\w{24}] not initialized/);
expect(() => tags[0].books.add(book1)).toThrowError(/Collection<Book> of entity BookTag\[\w{24}] not initialized/);
expect(() => tags[0].books.remove(book1, book2)).toThrowError(/Collection<Book> of entity BookTag\[\w{24}] not initialized/);
expect(() => tags[0].books.removeAll()).toThrowError(/Collection<Book> of entity BookTag\[\w{24}] not initialized/);
expect(() => tags[0].books.contains(book1)).toThrowError(/Collection<Book> of entity BookTag\[\w{24}] not initialized/);
Expand Down Expand Up @@ -1829,6 +1829,49 @@ describe('EntityManagerMongo', () => {
expect(res1).toBeNull();
});

test('adding items to not initialized collection', async () => {
const god = new Author('God', 'hello@heaven.god');
const b1 = new Book('Bible 1', god);
await orm.em.persistAndFlush(b1);
orm.em.clear();

const a = await orm.em.findOneOrFail(Author, god.id);
expect(a.books.isInitialized()).toBe(false);
const b2 = new Book('Bible 2');
const b3 = new Book('Bible 3');
a.books.add(b2, b3);
await orm.em.flush();
orm.em.clear();

const a2 = await orm.em.findOneOrFail(Author, god.id, ['books']);
expect(a2.books.count()).toBe(3);
expect(a2.books.getIdentifiers('id')).toEqual([b1.id, b2.id, b3.id]);

const tag1 = new BookTag('silly');
const tag2 = new BookTag('funny');
const tag3 = new BookTag('sick');
const tag4 = new BookTag('strange');
let tag5 = new BookTag('sexy');
a2.books[0].tags.add(tag1);
a2.books[1].tags.add(tag1);
a2.books[2].tags.add(tag5);
await orm.em.flush();
orm.em.clear();

const a3 = await orm.em.findOneOrFail(Author, god.id, ['books']);
tag5 = orm.em.getReference(BookTag, tag5.id);
a3.books[0].tags.add(tag3);
a3.books[1].tags.add(tag2, tag5);
a3.books[2].tags.add(tag4);
await orm.em.flush();
orm.em.clear();

const a4 = await orm.em.findOneOrFail(Author, god.id, ['books.tags']);
expect(a4.books[0].tags.getIdentifiers()).toEqual([tag1.id, tag3.id]);
expect(a4.books[1].tags.getIdentifiers()).toEqual([tag1.id, tag2.id, tag5.id]);
expect(a4.books[2].tags.getIdentifiers()).toEqual([tag5.id, tag4.id]);
});

// this should run in ~600ms (when running single test locally)
test('perf: one to many', async () => {
const author = new Author('Jon Snow', 'snow@wall.st');
Expand Down
51 changes: 50 additions & 1 deletion tests/EntityManager.mysql.test.ts
View file
Expand Up @@ -913,7 +913,6 @@ describe('EntityManagerMySql', () => {
expect(tags[0].books.isInitialized()).toBe(false);
expect(tags[0].books.isDirty()).toBe(false);
expect(() => tags[0].books.getItems()).toThrowError(/Collection<Book2> of entity BookTag2\[\d+] not initialized/);
expect(() => tags[0].books.add(book1)).toThrowError(/Collection<Book2> of entity BookTag2\[\d+] not initialized/);
expect(() => tags[0].books.remove(book1, book2)).toThrowError(/Collection<Book2> of entity BookTag2\[\d+] not initialized/);
expect(() => tags[0].books.removeAll()).toThrowError(/Collection<Book2> of entity BookTag2\[\d+] not initialized/);
expect(() => tags[0].books.contains(book1)).toThrowError(/Collection<Book2> of entity BookTag2\[\d+] not initialized/);
Expand Down Expand Up @@ -1854,6 +1853,56 @@ describe('EntityManagerMySql', () => {
expect(() => orm.em.remove(Book2, null)).toThrowError(`You cannot call 'EntityManager.remove()' with empty 'where' parameter. If you want to remove all entities, use 'em.remove(Book2, {})'.`);
});

test('adding items to not initialized collection', async () => {
const god = new Author2('God', 'hello@heaven.god');
const b1 = new Book2('Bible 1', god);
await orm.em.persistAndFlush(b1);
orm.em.clear();

const a = await orm.em.findOneOrFail(Author2, god.id);
expect(a.books.isInitialized()).toBe(false);
const b2 = new Book2('Bible 2', a);
const b3 = new Book2('Bible 3', a);
a.books.add(b2, b3);
await orm.em.flush();
orm.em.clear();

const a2 = await orm.em.findOneOrFail(Author2, god.id, ['books']);
expect(a2.books.count()).toBe(3);
expect(a2.books.getIdentifiers()).toEqual([b1.uuid, b2.uuid, b3.uuid]);

const tag1 = new BookTag2('silly');
const tag2 = new BookTag2('funny');
const tag3 = new BookTag2('sick');
const tag4 = new BookTag2('strange');
let tag5 = new BookTag2('sexy');
a2.books[0].tags.add(tag1);
a2.books[1].tags.add(tag1);
a2.books[2].tags.add(tag5);
await orm.em.flush();
orm.em.clear();

const a3 = await orm.em.findOneOrFail(Author2, god.id, ['books']);
tag5 = orm.em.getReference(BookTag2, tag5.id);
a3.books[0].tags.add(tag3);
a3.books[1].tags.add(tag2, tag5);
a3.books[2].tags.add(tag4);
await orm.em.flush();
orm.em.clear();

const a4 = await orm.em.findOneOrFail(Author2, god.id, ['books.tags']);
expect(a4.books[0].tags.getIdentifiers()).toEqual([tag1.id, tag3.id]);
expect(a4.books[1].tags.getIdentifiers()).toEqual([tag1.id, tag2.id, tag5.id]);
expect(a4.books[2].tags.getIdentifiers()).toEqual([tag5.id, tag4.id]);
orm.em.clear();

const tag = await orm.em.findOneOrFail(BookTag2, tag1.id);
const book = await orm.em.findOneOrFail(Book2, b3.uuid);
tag.books.add(book);
await orm.em.flush();
orm.em.clear();
});

afterAll(async () => orm.close(true));

});
1 change: 0 additions & 1 deletion tests/EntityManager.postgre.test.ts
View file
Expand Up @@ -706,7 +706,6 @@ describe('EntityManagerPostgre', () => {
expect(tags[0].books.isInitialized()).toBe(false);
expect(tags[0].books.isDirty()).toBe(false);
expect(() => tags[0].books.getItems()).toThrowError(/Collection<Book2> of entity BookTag2\[\d+] not initialized/);
expect(() => tags[0].books.add(book1)).toThrowError(/Collection<Book2> of entity BookTag2\[\d+] not initialized/);
expect(() => tags[0].books.remove(book1, book2)).toThrowError(/Collection<Book2> of entity BookTag2\[\d+] not initialized/);
expect(() => tags[0].books.removeAll()).toThrowError(/Collection<Book2> of entity BookTag2\[\d+] not initialized/);
expect(() => tags[0].books.contains(book1)).toThrowError(/Collection<Book2> of entity BookTag2\[\d+] not initialized/);
Expand Down
1 change: 0 additions & 1 deletion tests/EntityManager.sqlite.test.ts
View file
Expand Up @@ -582,7 +582,6 @@ describe('EntityManagerSqlite', () => {
expect(tags[0].books.isInitialized()).toBe(false);
expect(tags[0].books.isDirty()).toBe(false);
expect(() => tags[0].books.getItems()).toThrowError(/Collection<Book3> of entity BookTag3\[\d+] not initialized/);
expect(() => tags[0].books.add(book1)).toThrowError(/Collection<Book3> of entity BookTag3\[\d+] not initialized/);
expect(() => tags[0].books.remove(book1, book2)).toThrowError(/Collection<Book3> of entity BookTag3\[\d+] not initialized/);
expect(() => tags[0].books.removeAll()).toThrowError(/Collection<Book3> of entity BookTag3\[\d+] not initialized/);
expect(() => tags[0].books.contains(book1)).toThrowError(/Collection<Book3> of entity BookTag3\[\d+] not initialized/);
Expand Down
1 change: 0 additions & 1 deletion tests/EntityManager.sqlite2.test.ts
View file
Expand Up @@ -541,7 +541,6 @@ describe('EntityManagerSqlite2', () => {
expect(tags[0].books.isInitialized()).toBe(false);
expect(tags[0].books.isDirty()).toBe(false);
expect(() => tags[0].books.getItems()).toThrowError(/Collection<Book4> of entity BookTag4\[\d+] not initialized/);
expect(() => tags[0].books.add(book1)).toThrowError(/Collection<Book4> of entity BookTag4\[\d+] not initialized/);
expect(() => tags[0].books.remove(book1, book2)).toThrowError(/Collection<Book4> of entity BookTag4\[\d+] not initialized/);
expect(() => tags[0].books.removeAll()).toThrowError(/Collection<Book4> of entity BookTag4\[\d+] not initialized/);
expect(() => tags[0].books.contains(book1)).toThrowError(/Collection<Book4> of entity BookTag4\[\d+] not initialized/);
Expand Down

0 comments on commit ca5eb64

Please sign in to comment.