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

fix(mikro): infinite loop when using OneToOne eager loading #546

Merged
merged 2 commits into from
Jan 22, 2024

Conversation

zgid123
Copy link
Contributor

@zgid123 zgid123 commented May 22, 2023

I am using Mikro version 6.0.0-dev.75, I am not sure if version 5 face this issue or not. But I think it faces the same issue.

My usage is I have two models with OneToOne association, when eager loading, mikro-orm will load the model as reference, and then automapper will call serializeEntity infinitely.

Example:

@Entity()
export class Post extends Base {
  @AutoMap(() => SeriesPost)
  @OneToOne({
    ref: true,
    mappedBy: 'post',
    orphanRemoval: true,
    entity: () => SeriesPost,
  })
  seriesPost: Ref<SeriesPost>;
@Entity()
export class SeriesPost extends Base {
  @OneToOne({
    ref: true,
    entity: () => Post,
    deleteRule: 'cascade',
    inversedBy: 'seriesPost',
  })
  post: Ref<Post>;
}

In controller, I use

    return this.postRepository.findOneOrFail(
      {
        id,
      },
      {
        populate: [
          'seriesPost',
        ],
      },
    );

It will cause infinite loop because post.seriesPost.post is a reference of post.

The main idea of the fix is memorized the POJO/raw entity of the mikro-orm, then when we have a cycle, we will get from memorized instead of calling the serializeEntity.

Currently, when I do

    return this.postRepository.findOneOrFail(
      {
        id,
      },
      {
        populate: [
          'seriesPost',
          'seriesPost.post',
        ],
      },
    );

I cannot get the data of seriesPost.post, my project does not need that, but I still not know why it does not get that property. Maybe my config is wrong. I will test it later and will have a fix for this case (if it is a bug)

@zgid123
Copy link
Contributor Author

zgid123 commented May 22, 2023

one small issue I found out, if I have an entity

@Entity()
export class Series extends Base {
  @OneToMany(() => SeriesPost, (seriesPost) => seriesPost.series)
  seriesPosts = new Collection<SeriesPost>(this);

  @AutoMap()
  @Property({ persist: false })
  get chapterNumbers() {
    if (!this.seriesPosts.isInitialized()) {
      return 0;
    }

    return this.seriesPosts.length;
  }
}

when automapper runs, it will ignore the chapterNumbers

    if (!Utils.isEntity(item)) return item;
    if (toPojo) return wrap(item).toPOJO();

If we always returns wrap(item).toPOJO() here, we can get the chapterNumbers, but I am not sure the the design why we need to return the entity instead of its pojo.

When I use beforeMap, the source.chapterNumbers is undefined. I am not sure how we deal with this one. So if can, can you gimme the idea why we need return the entity instead of pojo?

@zgid123
Copy link
Contributor Author

zgid123 commented May 22, 2023

for version 6, we can do

const keys = Object.keys((wrap(item) as IWrappedEntityInternal<AnyEntity>).__meta.properties);

instead of

Reflect.ownKeys(item)

When using beforeMap, there is value for source.chapterNumbers, but the result does not contain it. I still not know why. I will try mikro-orm version 5 to test if it is working or not

@toonvanstrijp
Copy link

Version 5 of MikroORM is also facing this issue. We're facing the exact same issue as @zgid123, did you happen to find any work around to use until this gets merged?

@zgid123
Copy link
Contributor Author

zgid123 commented May 31, 2023

@toonvanstrijp currently I copy my fix to my repo and then import from that file instead of using the @automapper/mikro. I already contacted the author, but he is really busy. It will take time for this one to be merged

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@zgid123
Copy link
Contributor Author

zgid123 commented Jun 17, 2023

@toonvanstrijp FYI: I created a temporary package while waiting for the author here. I tested with my project and worked

@nartc nartc merged commit ec00a0d into nartc:main Jan 22, 2024
@nartc
Copy link
Owner

nartc commented Jan 22, 2024

Thanks @zgid123 for the PR

nartc pushed a commit to jersmart/automapper-typescript that referenced this pull request Jan 22, 2024
* fix(mikro): infinite loop when using OneToOne eager loading

* fix(mikro): mapping custom property of mikro entity to automapper
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

Successfully merging this pull request may close these issues.

3 participants