Skip to content
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

validation error when persisting entity from parent constructor #4781

Closed
JulianCissen opened this issue Oct 4, 2023 · 5 comments
Closed

Comments

@JulianCissen
Copy link

Describe the bug
When creating an entity and assigning a related entity as relation in it's constructor, an issue occurs persisting the related entity.

Stack trace

C:\sources\mikro-orm-sandbox\node_modules\@mikro-orm\core\errors.js:68
        return new ValidationError(`Value for ${entityName}.${property.name} is required, '${entity[property.name]}' found\nentity: ${(0, util_1.inspect)(entity)}`, entity);               ^
ValidationError: Value for Book.author is required, 'undefined' found
entity: Book {
  id: 'acc9d787-abbc-469e-b8cf-1560a2ede99a',
  releaseDate: 2023-10-04T10:12:08.773Z,
  name: 'My first book'
}
    at Function.propertyRequired (C:\sources\mikro-orm-sandbox\node_modules\@mikro-orm\core\errors.js:68:16)
    at EntityValidator.validateRequired (C:\sources\mikro-orm-sandbox\node_modules\@mikro-orm\core\entity\EntityValidator.js:49:48)
    at ChangeSetPersister.processProperties (C:\sources\mikro-orm-sandbox\node_modules\@mikro-orm\core\unit-of-work\ChangeSetPersister.js:77:28)
    at C:\sources\mikro-orm-sandbox\node_modules\@mikro-orm\core\unit-of-work\ChangeSetPersister.js:24:46
    at Array.forEach (<anonymous>)
    at ChangeSetPersister.executeInserts (C:\sources\mikro-orm-sandbox\node_modules\@mikro-orm\core\unit-of-work\ChangeSetPersister.js:24:20)
    at ChangeSetPersister.runForEachSchema (C:\sources\mikro-orm-sandbox\node_modules\@mikro-orm\core\unit-of-work\ChangeSetPersister.js:68:31)
    at ChangeSetPersister.executeInserts (C:\sources\mikro-orm-sandbox\node_modules\@mikro-orm\core\unit-of-work\ChangeSetPersister.js:21:25)
    at UnitOfWork.commitCreateChangeSets (C:\sources\mikro-orm-sandbox\node_modules\@mikro-orm\core\unit-of-work\UnitOfWork.js:741:39)
    at processTicksAndRejections (node:internal/process/task_queues:95:5) {
  entity: Book {
    id: 'acc9d787-abbc-469e-b8cf-1560a2ede99a',
    releaseDate: [Getter/Setter],
    name: [Getter/Setter]
  }
}

To Reproduce
Steps to reproduce the behavior:

  1. Run the following code:
import { Entity, Property, PrimaryKey, OneToOne, OneToMany, ManyToOne, Collection, Ref, DateType, ref } from "@mikro-orm/core";
import { MikroORM } from "@mikro-orm/postgresql";
import { v4 } from "uuid";

@Entity()
export class Author {
  @PrimaryKey()
  id = v4();

  @Property()
  name!: string

  @OneToMany({
    entity: () => Book,
    mappedBy: 'author',
  })
  books = new Collection<Book>(this);

  @OneToOne({
    entity: () => Book,
    ref: true,
    formula: (alias) =>
      `(select "b"."id"
      from (
        select "b"."author_id", min("b"."release_date") "release_date"
        from "book" "b"
        where "b"."author_id" = ${alias}."id"
        group by "b"."author_id"
      ) "s1"
      join "book" "b"
      on "b"."author_id" = "s1"."author_id"
      and "b"."release_date" = "s1"."release_date")`,
  })
  firstBook!: Ref<Book>;

  constructor(name: string) {
    this.name = name;
    const myFirstBook = new Book(new Date(), 'My first book', this);
    this.books.add(myFirstBook)
    this.firstBook = ref(myFirstBook)
  }
}

@Entity()
export class Book {
  @PrimaryKey()
  id = v4();

  @Property({ type: DateType })
  releaseDate!: Date;

  @Property()
  name!: string;

  @ManyToOne({
    entity: () => Author,
    ref: true,
    inversedBy: 'books',
  })
  author!: Ref<Author>;

  constructor(releaseDate: Date = new Date(), name: string, author: Author) {
    this.releaseDate = releaseDate;
    this.name = name;
    this.author = ref((author));
  }
}

const start = async () => {
  const db = await MikroORM.init({
      entities: [Book, Author],
      type: 'postgresql',
      dbName: 'debug',
      host: 'localhost',
      port: 5432,
      user: 'postgres',
      password: 'postgres',
      debug: true,
  });

  let em = db.em.fork();

  const author = new Author('John');

  em.persistAndFlush([author]);
}

start();

Expected behavior
The newly constructed entities are persisted properly.

Additional context
Relates to #4759. Based on the resolution of that issue I assume that using virtual relations this way is fine (I understand that it hasn't been tested much, but that's something I can live with).

This issue only seems to occur when initializing the parent entity (Author) at the same time as the related entity (Book). If the parent entity already exists and has been retrieved from EM, and then the book is added as shown in the constructor (initialize the entity, relate it to the normal and the virtual relations), it seems like this issue doesn't occur.

Similarly, the issue does not occur when only the actual relation is set, not the virtual relation. My use case however requires me to make the virtual relation (firstBook) required (mostly due to backwards compatibility), and the virtual relation itself is needed to be able to query on a specific (most recent) entry of the collection (books).

Versions

Dependency Version
node 18.18.0
typescript 5.1.6
mikro-orm 5.8.5
your-driver 5.8.5
@B4nan
Copy link
Member

B4nan commented Oct 4, 2023

FYI you can disable this validation for now via validateRequired: false in the ORM config. But if it's triggered, it really means the value is not there, and a following insert query would fail on the database level, so its probably not gonna help.

@JulianCissen
Copy link
Author

You're correct. Disabling validation results in a constraint check fail because it's inserting the child entity without a reference to the parent entity. Something under water is causing the relation to disappear from the child entity.

@JulianCissen
Copy link
Author

JulianCissen commented Oct 4, 2023

Looking at the code again makes me think it might have to do with the fact that the relation isn't inversed on book, after trying a couple things I think the following seems to fix it.

It's just that this is a bit ugly, since it means I have to introduce two relations from Book to Author where 1 would suffice. I can work with this for now, but maybe this could be supported in a bit nicer way. I can't make firstBookAuthor private or add a second decorator to author.

Is there a way to make the property on Book protected without a TS error on the Author side? And is it expected that having no inverse side of the relation seems to mess with other relations?

import { Entity, Property, PrimaryKey, OneToOne, OneToMany, ManyToOne, Collection, Ref, DateType, ref } from "@mikro-orm/core";
import { MikroORM } from "@mikro-orm/postgresql";
import { v4 } from "uuid";

@Entity()
export class Author {
  @PrimaryKey()
  id = v4();

  @Property()
  name!: string

  @OneToMany({
    entity: () => Book,
    mappedBy: 'author',
  })
  books = new Collection<Book>(this);

  @OneToOne({
    entity: () => Book,
    ref: true,
    formula: (alias) =>
      `(select "b"."id"
      from (
        select "b"."author_id", min("b"."release_date") "release_date"
        from "book" "b"
        where "b"."author_id" = ${alias}."id"
        group by "b"."author_id"
      ) "s1"
      join "book" "b"
      on "b"."author_id" = "s1"."author_id"
      and "b"."release_date" = "s1"."release_date")`,
    mappedBy: 'firstBookAuthor'
  })
  firstBook!: Ref<Book>;

  constructor(name: string) {
    this.name = name;
    const myFirstBook = new Book(new Date(), 'My first book', this);
    this.books.add(myFirstBook)
    this.firstBook = ref(myFirstBook)
  }
}

@Entity()
export class Book {
  @PrimaryKey()
  id = v4();

  @Property({ type: DateType })
  releaseDate!: Date;

  @Property()
  name!: string;

  @ManyToOne({
    entity: () => Author,
    ref: true,
    inversedBy: 'books',
  })
  author!: Ref<Author>;

  @OneToOne({
    entity: () => Author,
    ref: true,
    inversedBy: 'firstBook',
    persist: false,
  })
  firstBookAuthor!: Ref<Author>;

  constructor(releaseDate: Date = new Date(), name: string, author: Author) {
    this.releaseDate = releaseDate;
    this.name = name;
    this.author = ref((author));
  }
}

const start = async () => {
  const db = await MikroORM.init({
      entities: [Book, Author],
      type: 'postgresql',
      dbName: 'debug',
      host: 'localhost',
      port: 5432,
      user: 'postgres',
      password: 'postgres',
      debug: true,
  });

  let em = db.em.fork();

  const author = new Author('John');

  em.persistAndFlush([author]);
}

start();

@B4nan
Copy link
Member

B4nan commented Oct 4, 2023

The problem is hidden in the commit order calculator, as because of the new owning side relation, it thinks that an extra update is necessary. Your workaround works because you now defined the owning side on the other end instead.

@B4nan
Copy link
Member

B4nan commented Oct 4, 2023

Making the property nullable: true is actually all you need to work this around, as that way the commit order calculator will know its fine to not provide a value for it.

@B4nan B4nan closed this as completed in 606d633 Oct 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants