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

when extending entity classes and redefining relations with child clases, load relations when using populate #3561

Open
rhyek opened this issue Oct 4, 2022 · 8 comments
Labels
enhancement New feature or request

Comments

@rhyek
Copy link
Contributor

rhyek commented Oct 4, 2022

Is your feature request related to a problem? Please describe.
I have entity classes that are being shared with multiple microservices in a monorepo. My goal is to define the entities once, import them in whatever microservice needs them, and allow for those entities to be extended with any custom additional properties that need not be shared elsewhere.

Describe the solution you'd like
Basic example of what I want to be able to do:
Somewhere:

@Entity({ tableName: 'company' })
class BaseCompanyEntity {
  @PrimaryKey()
  id!: string;

  @Property()
  name!: string;
}

@Entity({ tableName: 'user' })
class BaseUserEntity {
  @PrimaryKey()
  id!: string;

  @Property()
  firstName!: string;

  @Property()
  lastName!: string;

  @ManyToOne(() => BaseCompanyEntity, { nullable: true })
  company?: BaseCompanyEntity | null;
}

In some application:

@Entity({ tableName: 'company' })
class CompanyEntity extends BaseCompanyEntity {
  [OptionalProps]?: 'nameUpper';

  @Property({ formula: 'UPPER(name)' })
  nameUpper!: string;
}

@Entity({ tableName: 'user' })
class UserEntity extends BaseUserEntity {
  [OptionalProps]?: 'fullName';

  @Property({ formula: "CONCAT(first_name, ' ', last_name)" })
  fullName!: string;

  @ManyToOne(() => CompanyEntity, { nullable: true }) // notice i am changing the relation entity here
  company?: CompanyEntity | null;
}

Now if I do the following, it should work:

const found = await orm.em.findOne(
  UserEntity,
  { id },
  {
    populate: ['company'],
  }
);

However, company is not being initialized.

I have a reproduction of the issue here: https://github.com/rhyek/mikro-orm-extend-entities (look at last 2 test cases)

If you run the tests you will see how when using em.findOne, found.company.isInitialized() === false vs when using a query builder and joinAndSelect found.company.isInitialized() === true.

@rhyek rhyek added the enhancement New feature or request label Oct 4, 2022
@B4nan
Copy link
Member

B4nan commented Oct 4, 2022

FYI this is purely about metadata, not about runtime code (so not about populate). I guess it could work the way you written, can't say I like it, as you decorate base entities with @Entity() (unsupported/hack) just to be able to define relations (unsupported).

Maybe we should try to lift the restriction to define relations in base entities. It would probably unlock your use case (everything would stay the same, you'd just drop the @Entity() decorator from the base entities). We would need to carefully validate that the entity definition makes sense (regarding extension and relations targetting base entities, as if there woudnt be an extension, the relation would be invalid).

@rhyek
Copy link
Contributor Author

rhyek commented Oct 4, 2022

Well, the the base entities are meant to be used directly if no changes need to made to it. This would maybe be 70% of the time. An application should be able to just import an already defined entity and use it immediately in MikroOrm.init.

@PoppinMic
Copy link

I encountered the same use case when I building a new project in my company. Initially I was thinking just to have abstract base entities in libs and use @Entity() decorator in apps. However later I found defining Many-To-One relations are not supported in abstract entities. It would be nice if we can have those relations supported in abstract entities as well

@B4nan
Copy link
Member

B4nan commented Nov 16, 2022

You can define m:1 relations in a base entity, they just have to target an actual entity type, not a base type.

@PoppinMic
Copy link

Hi @B4nan , thanks for the reply. The problem for me is that this base entity would be sitting in a separate library package and in that context it won't know the actual entity type until I use the package in the app.

@B4nan
Copy link
Member

B4nan commented Nov 17, 2022

Well, you cant define a relation without knowing anything about it. I dont understand how you think it should work.

it won't know the actual entity type until I use the package in the app.

You extend that entity, but that is not adding any context to it. You still end up with a relation without any target.

@PoppinMic
Copy link

Hi @B4nan , sorry my description wasn't that clear, let me try to rephrase my use case.

We want to have 3 abstract entities in a library to have its own properties & relations, so that when we use this library in the app we can extends all 3 abstract entities to become a real entity and the real entities can automatically have the relations without us needing to write the relations again.

Essentially want to force the app using this library has those 3 entities/tables with the same relations. Also like @rhyek , 80% of time we just need the same properties and relations for those 3 entities.

Hopefully that's clear enough.

@B4nan
Copy link
Member

B4nan commented Nov 18, 2022

Your use case is clear, what you are missing is that there is no guarantee that your base entity won't be extended multiple times, totally breaking the invariants of what you want to achieve. That is the reason why you can't target a base entity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants