Skip to content

Commit

Permalink
fix(core): fix changing 1:1 relations value
Browse files Browse the repository at this point in the history 
Closes #3614
  • Loading branch information
@B4nan
B4nan committed Oct 22, 2022
1 parent 1e559ae commit 7b6e6f7
Show file tree
Hide file tree
Showing 6 changed files with 197 additions and 53 deletions.
1 change: 1 addition & 0 deletions packages/cli/src/commands/SchemaCommandFactory.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ export class SchemaCommandFactory {
const m = `get${method.substr(0, 1).toUpperCase()}${method.substr(1)}SchemaSQL` as 'getCreateSchemaSQL' | 'getUpdateSchemaSQL' | 'getDropSchemaSQL';
const dump = await generator[m](params);

/* istanbul ignore next */
if (dump) {
CLIHelper.dump(dump, orm.config);
successMessage = '';
Expand Down
36 changes: 23 additions & 13 deletions packages/core/src/entity/EntityHelper.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,17 +150,19 @@ export class EntityHelper {
return helper(ref).__data[prop.name];
},
set(val: AnyEntity | Reference<AnyEntity>) {
const entity = Reference.unwrapReference(val ?? helper(ref).__data[prop.name]);
helper(ref).__data[prop.name] = Reference.wrapReference(val as T, prop);
helper(ref).__touched = true;
EntityHelper.propagate(meta, entity, this, prop, Reference.unwrapReference(val));
const wrapped = helper(ref);
const entity = Reference.unwrapReference(val ?? wrapped.__data[prop.name]);
const old = Reference.unwrapReference(wrapped.__data[prop.name]);
wrapped.__data[prop.name] = Reference.wrapReference(val as T, prop);
wrapped.__touched = true;
EntityHelper.propagate(meta, entity, this, prop, Reference.unwrapReference(val), old);
},
enumerable: true,
configurable: true,
});
}

private static propagate<T extends object, O extends object>(meta: EntityMetadata<O>, entity: T, owner: O, prop: EntityProperty<O>, value?: O[keyof O]): void {
private static propagate<T extends object, O extends object>(meta: EntityMetadata<O>, entity: T, owner: O, prop: EntityProperty<O>, value?: T[keyof T & string], old?: object): void {
const inverseProps = prop.targetMeta!.relations.filter(prop2 => (prop2.inversedBy || prop2.mappedBy) === prop.name && prop2.targetMeta!.root.className === meta.root.className);

for (const prop2 of inverseProps) {
Expand All @@ -171,25 +173,33 @@ export class EntityHelper {
}

if (prop.reference === ReferenceType.ONE_TO_ONE && entity && helper(entity).__initialized && Reference.unwrapReference(inverse) !== owner && value != null) {
EntityHelper.propagateOneToOne(entity, owner, prop, prop2);
EntityHelper.propagateOneToOne(entity, owner, prop, prop2, value, old);
}

if (prop.reference === ReferenceType.ONE_TO_ONE && entity && helper(entity).__initialized && entity[prop2.name] != null && value == null) {
helper(entity).__pk = helper(entity).getPrimaryKey()!;
entity[prop2.name] = value;
if (prop.orphanRemoval) {
helper(entity).__em?.getUnitOfWork().scheduleOrphanRemoval(entity);
}
EntityHelper.propagateOneToOne(entity, owner, prop, prop2, value, old);
}
}
}

private static propagateOneToOne<T extends object, O extends object>(entity: T, owner: O, prop: EntityProperty<O>, prop2: EntityProperty<T>): void {
if (prop2.mapToPk) {
private static propagateOneToOne<T extends object, O extends object>(entity: T, owner: O, prop: EntityProperty<O>, prop2: EntityProperty<T>, value?: T[keyof T & string], old?: T): void {
helper(entity).__pk = helper(entity).getPrimaryKey()!;

if (value == null) {
entity[prop2.name] = value as T[keyof T & string];
} else if (prop2.mapToPk) {
entity[prop2.name] = helper(owner).getPrimaryKey() as T[keyof T & string];
} else {
entity[prop2.name] = Reference.wrapReference(owner, prop) as T[keyof T & string];
}

if (old && prop.orphanRemoval) {
helper(old).__em?.getUnitOfWork().scheduleOrphanRemoval(old);
}

if (old?.[prop2.name] != null) {
delete helper(old).__data[prop2.name];
}
}

}
2 changes: 2 additions & 0 deletions packages/core/src/metadata/MetadataDiscovery.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -897,6 +897,7 @@ export class MetadataDiscovery {
return prop.defaultRaw;
}

/* istanbul ignore next */
if (prop.default != null) {
return '' + this.platform.quoteVersionValue(prop.default as number, prop);
}
Expand Down Expand Up @@ -1067,6 +1068,7 @@ export class MetadataDiscovery {
path = Utils.normalizePath(this.config.get('baseDir'), path);
const exports: Dictionary = {};

/* istanbul ignore next */
try {
Object.assign(exports, await Utils.dynamicImport(path));
} catch (e) {
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/unit-of-work/ChangeSet.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,5 +54,6 @@ export enum ChangeSetType {
CREATE = 'create',
UPDATE = 'update',
DELETE = 'delete',
UPDATE_EARLY = 'update_early',
DELETE_EARLY = 'delete_early',
}
87 changes: 47 additions & 40 deletions packages/core/src/unit-of-work/UnitOfWork.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,6 @@ export class UnitOfWork {
}

this.initIdentifier(entity);
this.checkOrphanRemoval(cs);
this.changeSets.set(entity, cs);
this.persistStack.delete(entity);
this.queuedActions.delete(cs.name);
Expand All @@ -261,7 +260,6 @@ export class UnitOfWork {

/* istanbul ignore else */
if (cs && !this.checkUniqueProps(changeSet)) {
this.checkOrphanRemoval(cs);
Object.assign(changeSet.payload, cs.payload);
helper(entity).__originalEntityData = this.comparator.prepareEntity(entity);
helper(entity).__touched = false;
Expand Down Expand Up @@ -401,21 +399,20 @@ export class UnitOfWork {
this.identityMap.delete(entity);
const wrapped = helper(entity);

wrapped.__meta.bidirectionalRelations
.map(prop => [prop.name, prop.reference, prop.mappedBy || prop.inversedBy] as const)
.forEach(([name, kind, inverse]) => {
const rel = Reference.unwrapReference(entity[name]);
if ([ReferenceType.MANY_TO_MANY, ReferenceType.ONE_TO_MANY].includes(kind)) {
if ((rel as Collection<any>)?.isInitialized()) {
(rel as Collection<any>).removeAll();
}
} else {
// there is a value, and it is still a self-reference (e.g. not replaced by user manually)
if (rel?.[inverse] && entity === rel?.[inverse]) {
delete rel[inverse];
}
}
});
for (const prop of wrapped.__meta.bidirectionalRelations) {
const inverse = prop.mappedBy || prop.inversedBy;
const rel = Reference.unwrapReference(entity[prop.name]);

if (Utils.isCollection(rel) && rel.isInitialized()) {
rel.removeAll();
continue;
}

// there is a value, and it is still a self-reference (e.g. not replaced by user manually)
if (rel?.[inverse] && entity === rel[inverse]) {
delete helper(rel).__data[inverse];
}
}

delete wrapped.__identifier;
delete wrapped.__originalEntityData;
Expand Down Expand Up @@ -464,7 +461,14 @@ export class UnitOfWork {
}
}

for (const cs of this.changeSets.values()) {
if (cs.type === ChangeSetType.UPDATE) {
this.findEarlyUpdates(cs, inserts);
}
}

for (const entity of this.removeStack) {
/* istanbul ignore next */
if (helper(entity).__processing) {
continue;
}
Expand Down Expand Up @@ -534,11 +538,13 @@ export class UnitOfWork {
const changeSet = this.changeSetComputer.computeChangeSet(entity);

if (changeSet && !this.checkUniqueProps(changeSet)) {
this.checkOrphanRemoval(changeSet);
this.changeSets.set(entity, changeSet);
}
}

/**
* Returns `true` when the change set should be skipped as it will be empty after the extra update.
*/
private checkUniqueProps<T extends object>(changeSet: ChangeSet<T>): boolean {
if (this.platform.allowsUniqueBatchUpdates() || changeSet.type !== ChangeSetType.UPDATE) {
return false;
Expand All @@ -561,22 +567,6 @@ export class UnitOfWork {
}).filter(i => i) as string[];
}

private checkOrphanRemoval<T extends object>(changeSet: ChangeSet<T>): void {
const meta = this.metadata.find(changeSet.name)!;
const props = meta.relations.filter(prop => prop.reference === ReferenceType.ONE_TO_ONE);

for (const prop of props) {
// check diff, if we had a value on 1:1 before, and now it changed (nulled or replaced), we need to trigger orphan removal
const wrapped = helper(changeSet.entity);
const data = wrapped.__originalEntityData;

if (prop.orphanRemoval && data && data[prop.name] && prop.name in changeSet.payload) {
const orphan = this.getById(prop.type, data[prop.name], wrapped.__schema);
this.scheduleOrphanRemoval(orphan as AnyEntity);
}
}
}

private initIdentifier<T extends object>(entity: T): void {
const wrapped = helper(entity);

Expand Down Expand Up @@ -810,17 +800,22 @@ export class UnitOfWork {
await this.commitDeleteChangeSets(groups[ChangeSetType.DELETE_EARLY].get(name) ?? [], ctx);
}

// 2. create
// 2. early update - when we recreate entity in the same UoW, we need to issue those delete queries before inserts
for (const name of commitOrder) {
await this.commitUpdateChangeSets(groups[ChangeSetType.UPDATE_EARLY].get(name) ?? [], ctx);
}

// 3. create
for (const name of commitOrder) {
await this.commitCreateChangeSets(groups[ChangeSetType.CREATE].get(name) ?? [], ctx);
}

// 3. update
// 4. update
for (const name of commitOrder) {
await this.commitUpdateChangeSets(groups[ChangeSetType.UPDATE].get(name) ?? [], ctx);
}

// 4. extra updates
// 5. extra updates
const extraUpdates: ChangeSet<any>[] = [];

for (const extraUpdate of this.extraUpdates) {
Expand All @@ -839,19 +834,20 @@ export class UnitOfWork {

await this.commitUpdateChangeSets(extraUpdates, ctx, false);

// 5. collection updates
// 6. collection updates
for (const coll of this.collectionUpdates) {
await this.em.getDriver().syncCollection(coll, { ctx });
coll.takeSnapshot();
}

// 6. delete - entity deletions need to be in reverse commit order
// 7. delete - entity deletions need to be in reverse commit order
for (const name of commitOrderReversed) {
await this.commitDeleteChangeSets(groups[ChangeSetType.DELETE].get(name) ?? [], ctx);
}

// 7. take snapshots of all persisted collections
// 8. take snapshots of all persisted collections
const visited = new Set<object>();

for (const changeSet of this.changeSets.values()) {
this.takeCollectionSnapshots(changeSet.entity, visited);
}
Expand Down Expand Up @@ -898,6 +894,16 @@ export class UnitOfWork {
}
}

private findEarlyUpdates<T extends object>(changeSet: ChangeSet<T>, inserts: ChangeSet<T>[]): void {
const props = changeSet.meta.uniqueProps;

for (const prop of props) {
if (prop.name in changeSet.payload && inserts.find(c => Utils.equals(c.payload[prop.name], changeSet.originalEntity![prop.name as string]))) {
changeSet.type = ChangeSetType.UPDATE_EARLY;
}
}
}

private async commitUpdateChangeSets<T extends object>(changeSets: ChangeSet<T>[], ctx?: Transaction, batched = true): Promise<void> {
if (changeSets.length === 0) {
return;
Expand Down Expand Up @@ -942,6 +948,7 @@ export class UnitOfWork {
[ChangeSetType.CREATE]: new Map<string, ChangeSet<any>[]>(),
[ChangeSetType.UPDATE]: new Map<string, ChangeSet<any>[]>(),
[ChangeSetType.DELETE]: new Map<string, ChangeSet<any>[]>(),
[ChangeSetType.UPDATE_EARLY]: new Map<string, ChangeSet<any>[]>(),
[ChangeSetType.DELETE_EARLY]: new Map<string, ChangeSet<any>[]>(),
};

Expand Down

0 comments on commit 7b6e6f7

Please sign in to comment.