Issue with updating parent in Lazy relationships

[cuzzea]

Issue type:

[x] question
[x] bug report
[ ] feature request
[ ] documentation issue

Database system/driver:

[ ] cordova
[ ] mongodb
[ ] mssql
[x] mysql / mariadb
[ ] oracle
[ ] postgres
[ ] sqlite
[ ] sqljs
[ ] react-native
[ ] expo

TypeORM version:

[x] latest
[ ] @next
[ ] 0.x.x (or put your version here)

Steps to reproduce or a small repository showing the problem:

Hey, I am not sure if this is a bug, or I am doing something wrong, but I tried a lot of things to get this working and I couldn't. I hope you guys can help.

Basically I have a one to one relationship that I need to lazyLoad. The relation tree is kind of big in my project and I can't load it without promises.

The issue I face is that when I save a child, the parent update generated sql is missing the update fields: UPDATE `a` SET WHERE `id` = 1

This is working perfectly when I am not using lazyLoading (Promises).

I got a simple example set up using the generated code tool.

Entity A

@Entity()
export class A {

    @PrimaryGeneratedColumn()
    id: number;

    @Column()
    name: string;

    @OneToOne(
        (type: any) => B,
        async (o: B) => await o.a
    )
    @JoinColumn()
    public b: Promise<B>;
}

Entity B

@Entity()
export class B {

    @PrimaryGeneratedColumn()
    id: number;

    @Column()
    name: string;

    @OneToOne(
        (type: any) => A,
        async (o: A) => await o.b)
    a: Promise<A>;
  
}

main.ts

createConnection().then(async connection => {

    const aRepo = getRepository(A);
    const bRepo = getRepository(B);

    console.log("Inserting a new user into the database...");
    const a = new A();
    a.name = "something";
    const aCreated = aRepo.create(a);
    await aRepo.save(aCreated);
    
    const as = await aRepo.find();
    console.log("Loaded A: ", as);

    const b = new B();
    b.name = "something";
    const bCreated = bRepo.create(b);
    bCreated.a =  Promise.resolve(as[0]);
    await bRepo.save(bCreated);
    
    const as2 = await aRepo.find();
    console.log("Loaded A: ", as2);
    
}).catch(error => console.log(error));

Output

Inserting a new user into the database...
query: SELECT `b`.`id` AS `b_id`, `b`.`name` AS `b_name` FROM `b` `b` INNER JOIN `a` `A` ON `A`.`bId` = `b`.`id` WHERE `A`.`id` IN (?) -- PARAMETERS: [[null]]
query: START TRANSACTION
query: INSERT INTO `a`(`id`, `name`, `bId`) VALUES (DEFAULT, ?, DEFAULT) -- PARAMETERS: ["something"]
query: UPDATE `a` SET  WHERE `id` = ? -- PARAMETERS: [1]
query failed: UPDATE `a` SET  WHERE `id` = ? -- PARAMETERS: [1]

If I remove the promises from the entities, everything is working fine:

Entity A

...
    @OneToOne(
        (type: any) => B,
        (o: B) => o.a
    )
    @JoinColumn()
    public b: B;
}

Entity B

    @OneToOne(
        (type: any) => A,
       (o: A) => o.b)
    a: A;
  
}

main.ts

createConnection().then(async connection => {
...
    const bCreated = bRepo.create(b);
    bCreated.a =  as[0];
    await bRepo.save(bCreated);
...

Output

query: INSERT INTO `b`(`id`, `name`) VALUES (DEFAULT, ?) -- PARAMETERS: ["something"]
query: UPDATE `a` SET `bId` = ? WHERE `id` = ? -- PARAMETERS: [1,1]
query: COMMIT
query: SELECT `A`.`id` AS `A_id`, `A`.`name` AS `A_name`, `A`.`bId` AS `A_bId` FROM `a` `A`

I have also created a git project to illustrate this and for easy of testing.

1. using promises (not working) https://github.com/cuzzea/bug-typeorm/tree/promise-issue
2. no lazy loading (working) https://github.com/cuzzea/bug-typeorm/tree/no-promise-no-issue

[yazshel]

I've done a fair bit of investigation on this on @cuzzea's StackOverflow question about this issue, and tracked it down to Repository.create() not checking for or handling input Promise objects on lazy relation properties.

The OP experiences this issue because TypeORM's RelationLoader.enableLazyLoad() overloads lazy load properties with its own getter which always returns a Promise. So when an instance of A is passed to aRepo.create(), the a.b Promise is used to create a new instance of B.

This can be most easily be replicated using the following test case:

entity/a.ts:

@Entity()
export class A {

    @PrimaryGeneratedColumn()
    id: number;

    @Column()
    name: string;

    @OneToOne(() => B, b => b.a)
    @JoinColumn()
    public b: Promise<B>;
}

entity/b.ts

@Entity()
export class B {

    @PrimaryGeneratedColumn()
    id: number;

    @Column()
    name: string;

    @OneToOne(() => A, a => a.b)
    public a: Promise<A>;
}

index.ts

createConnection().then(async connection => {
...
    const a = aRepo.create({ name: "foo", b: Promise.resolve() });
    await aRepo.save(a);
});

The b: Promise.resolve() on b triggers the issue.

I'll add a pull request with the above tests shortly.

[afurculita]

I've created the following test for this issue: https://github.com/afurculita/typeorm/commit/977f87549b04d0754b1a100c51ad3e32ab6f9b94 and it works in the form used in the test. The following queries are produced during this test:

[2018-09-15T17:28:50.311Z][QUERY]: START TRANSACTION
[2018-09-15T17:28:50.319Z][QUERY]: INSERT INTO `parent`(`id`, `name`, `childId`) VALUES (DEFAULT, ?, DEFAULT) -- PARAMETERS: ["something"]
[2018-09-15T17:28:50.323Z][QUERY]: COMMIT
[2018-09-15T17:28:50.331Z][QUERY]: START TRANSACTION
[2018-09-15T17:28:50.332Z][QUERY]: INSERT INTO `child`(`id`, `name`) VALUES (DEFAULT, ?) -- PARAMETERS: ["something"]
[2018-09-15T17:28:50.337Z][QUERY]: UPDATE `parent` SET `childId` = ? WHERE `id` = ? -- PARAMETERS: [1,1]
[2018-09-15T17:28:50.339Z][QUERY]: COMMIT

The following changes have been made to the entities A and B:

1. const aCreated = aRepo.create(a); and const bCreated = bRepo.create(b); have been removed as there is no need for them when a and b are already entity instances. repo.create(object) is used to transform a plain object to an instance of an entity.
2. async (o: B) => await o.a has been changed to (o: B) => o.a and async (o: A) => await o.b) to (o: A) => o.b). This is VERY important for not altering the property metadata.

Now, one more important thing is how you run your application or the tests. If you use ts-node: version <= 5 instead of tsc && node and you don't explicitly mark the columns a and b as being lazy (e.g. @OneToOne(() => A, a => a.b, {lazy: true})), you need to use the --type-check flag for ts-node (if you run ts-node directly) or ts-node/register/type-check instead of ts-node/register. This is not necessary if you use a version of ts-node bigger than 5.

[afurculita]

@cuzzea can this issue be closed? Are you still having the problem from the issue?

[JemarJones]

For those finding there way here who are looking for a workaround in lieu of #2902 being merged, i think i've figured something out (assuming you're using the ActiveRecord pattern with typeorm). First to summarize, as most of this information is absent from documentation and needs to be pieced together from various issues:

As pointed out in this thread, when using create passing in a Promise for a lazy loaded relation field simply does not work despite the type signature of that function demanding otherwise (and despite the documentation suggesting that lazy loaded fields should be wrapped in Promise.resolve's for saving purposes). What does seem to work according to
@yazshel's comment in the aforementioned PR #2902 (comment) is:

nasty TypeScript type casts when assigning object literals to lazy-load properties

What this means is that with the create method, if you pass in a plain entity object (instead of a Promise containing said object) for one of these lazy loaded fields typeorm will actually set this value correctly and you will be able to save. You'll even magically get a promise when accessing this field later on. The above quote mentions that you can take advantage of this by force casting your entity objects as Promise's before passing them into create. But this needs to be done on a case by case basis and if you ever accidentally obey the type signature rather than force casting, you're going to have an unexpected result at runtime. Wouldn't it be great if we could correct this type signature to get the compiler to only yell at us when we use this function in a way that won't work? We can, heres how :).

import {
  BaseEntity,
  DeepPartial,
  ObjectType,
} from 'typeorm';

/**
 * Conditional type that takes a type and maps every property which is
 * a Promise to the unwrapped value of that Promise. Specifically to correct the type
 * of typeorm's create method. Using this otherwise would likely be incredibly unwise.
 *
 * For example this type:
 * {
 *   hey: number,
 *   thing: Promise<ThingEntity>,
 *   sup: string
 * }
 *
 * gets mapped to:
 * {
 *   hey: number,
 *   thing: ThingEntity,
 *   sup: string
 * }
 *
 */
type DePromisifyValue<T> = T extends Promise<infer U> ? U : T;
type DePromisifyObject<T> = T extends object
  ? { [K in keyof T]: DePromisifyValue<T[K]> }
  : T;

export abstract class CommonEntity extends BaseEntity {
  static create<T extends CommonEntity>(
    this: ObjectType<T>,
    entityLike?: DeepPartial<DePromisifyObject<T>>
  ): T {
    if (!entityLike) {
      return super.create<T>();
    }
    return super.create<T>(entityLike as DeepPartial<T>);
  }
}

What this does is define an overridden version of this create method which accepts the same object argument as the original create method, except where any Promise fields are there unwrapped versions (ie myLazyLoadedUser: Promise<UserEntity> becomes myLazyLoadedUser: UserEntity). It then passes that into the original create method and force casts it to the old version with all the Promise fields, the way that BaseEntity likes (or lies about liking that is). Force casting at some point can't be avoided without the issue being fixed within typeorm itself, but this solution only requires force casting in one central place where we can be confident we're doing the right thing. Just extend this CommonEntity (call it whatever you want) instead of BaseEntity and the create method will require the correct type of you. No need to wrap your values in Promise.resolve. And the entity returned from it will still have those lazy loaded fields with their original Promise types.

Note: I haven't handled the type signature for create where an array of objects is passed in. I've no need for this myself, but i'm sure the type for that with the same approach can be worked out with sufficient effort.

[yazshel]

Nice summary and excellent solution for the problem of assigning object literals @JemarJones!

Given it appears unlikely that #2902 will be merged, perhaps a PR integrating your DePromisify stuff into core TypeORM is warranted?