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

[Feature] Polymorphic embedded entities #1165

Closed
dvlalex opened this issue Dec 5, 2020 · 1 comment · Fixed by #2426
Closed

[Feature] Polymorphic embedded entities #1165

dvlalex opened this issue Dec 5, 2020 · 1 comment · Fixed by #2426
Labels
enhancement New feature or request
Milestone

Comments

@dvlalex
Copy link
Contributor

dvlalex commented Dec 5, 2020

Is your feature request related to a problem? Please describe.
I bumped into this earlier after upgrading to v4. I think it would be useful(I'm on mongo, so I use embedded properties a lot) to have embedded properties work the same as STI entities, where B & C inherits A, and D holds a property that can be either B or C. This is in a way related to #706, but it should be easier to achieve since there's no FKs to handle here.

Say we have:

enum AnimalType {
	CAT,
	DOG,
}

abstract class Animal {
	@Enum(() => AnimalType)
	type: AnimalType;
	
	@Property({ type: String })
	name: string;
}

@Embeddable()
class Cat extends Animal {
	@Property({ type: Boolean })
	canMeow = true;

	constructor(name: string) {
		this.type = AnimalType.CAT;
		this.name = name;
	}
}

@Embeddable()
class Dog extends Animal {
	@Property({ type: Boolean })
	canBark = true;
	
	constructor(name: string) {
		this.type = AnimalType.DOG;
		this.name = name;
	}
}

I would expect to be able to do:

@Entity()
class Profile {
	@Embedded()
	pet: Dog | Cat;
}

Describe the solution you'd like
I think we can make Embeddables take on some parameters that would reference the type and have the entity mapper look for it if provided. Same as STI works now for entity types. As for the Embedded decorator, I would have this receive an array of entities that can be mapped against instead of a single entity.

Describe alternatives you've considered
The way i achieved this is by having a custom type and doing my own mapping based on the type property of the "animal" in the example above. The issue with this though, is that custom types aren't diffed when changes are being made to the parent entity which is normal, but I have to look for the changes myself which beats the purpose.

@dvlalex dvlalex added the enhancement New feature or request label Dec 5, 2020
@B4nan B4nan mentioned this issue Mar 30, 2021
48 tasks
@B4nan B4nan added this to the 5.0 milestone Apr 19, 2021
@B4nan
Copy link
Member

B4nan commented Nov 16, 2021

Looking into this now. We will need more information in the metadata, similarly to STI, but I feel like I found a way to incorporate this into the codebase without too much rewriting - similarly to pivot tables, we can have virtual entities for such polymorphic entities (one for the whole tuple of possible entities, so here for both Cat and Dog). This way we can still internally refer to a single target metadata for such properties. This virtual poly entity would have references to all possible classes, as well as merged property map (will be needed mainly for SQL drivers and inline mode).

I can imagine something like this:

enum AnimalType {
  CAT,
  DOG,
}

@Embeddable({ abstract: true, discriminatorColumn: 'type' })
abstract class Animal {

  @Enum(() => AnimalType)
  type!: AnimalType;

  @Property({ type: String })
  name!: string;

}

@Embeddable({ discriminatorValue: AnimalType.CAT })
class Cat extends Animal {

  @Property({ type: Boolean, nullable: true })
  canMeow = true;

  constructor(name: string) {
    super();
    this.type = AnimalType.CAT;
    this.name = name;
  }

}

@Embeddable({ discriminatorValue: AnimalType.DOG })
class Dog extends Animal {

  @Property({ type: Boolean, nullable: true })
  canBark = true;

  constructor(name: string) {
    super();
    this.type = AnimalType.DOG;
    this.name = name;
  }

}

@Entity()
class Owner {

  @PrimaryKey()
  id!: number;

  @Property()
  name!: string;

  // we could probably infer this via ts-morph, but let's not overcomplicate things for now
  @Embedded(() => [Cat, Dog])
  pet!: Cat | Dog;

}

B4nan added a commit that referenced this issue Nov 16, 2021
Polymorphic embeddables allow us to define multiple classes for a single
embedded property and the right one will be used based on the discriminator
column, similar to how single table inheritance work.

```ts
enum AnimalType {
  CAT,
  DOG,
}

@embeddable({ abstract: true, discriminatorColumn: 'type' })
abstract class Animal {

  @enum(() => AnimalType)
  type!: AnimalType;

  @Property({ type: String })
  name!: string;

}

@embeddable({ discriminatorValue: AnimalType.CAT })
class Cat extends Animal {

  @Property({ type: Boolean, nullable: true })
  canMeow = true;

  constructor(name: string) {
    super();
    this.type = AnimalType.CAT;
    this.name = name;
  }

}

@embeddable({ discriminatorValue: AnimalType.DOG })
class Dog extends Animal {

  @Property({ type: Boolean, nullable: true })
  canBark = true;

  constructor(name: string) {
    super();
    this.type = AnimalType.DOG;
    this.name = name;
  }

}

@entity()
class Owner {

  @PrimaryKey()
  id!: number;

  @Property()
  name!: string;

  @Embedded(() => [Cat, Dog])
  pet!: Cat | Dog;

}
```

Closes #1165
B4nan added a commit that referenced this issue Nov 16, 2021
Polymorphic embeddables allow us to define multiple classes for a single
embedded property and the right one will be used based on the discriminator
column, similar to how single table inheritance work.

```ts
enum AnimalType {
  CAT,
  DOG,
}

@embeddable({ abstract: true, discriminatorColumn: 'type' })
abstract class Animal {

  @enum(() => AnimalType)
  type!: AnimalType;

  @Property()
  name!: string;

}

@embeddable({ discriminatorValue: AnimalType.CAT })
class Cat extends Animal {

  @Property({ nullable: true })
  canMeow?: boolean = true;

  constructor(name: string) {
    super();
    this.type = AnimalType.CAT;
    this.name = name;
  }

}

@embeddable({ discriminatorValue: AnimalType.DOG })
class Dog extends Animal {

  @Property({ nullable: true })
  canBark?: boolean = true;

  constructor(name: string) {
    super();
    this.type = AnimalType.DOG;
    this.name = name;
  }

}

@entity()
class Owner {

  @PrimaryKey()
  id!: number;

  @Property()
  name!: string;

  @Embedded(() => [Cat, Dog])
  pet!: Cat | Dog;

}
```

Closes #1165
B4nan added a commit that referenced this issue Nov 17, 2021
Polymorphic embeddables allow us to define multiple classes for a single embedded property and the right one will be used based on the discriminator column, similar to how single table inheritance work.

```ts
enum AnimalType {
  CAT,
  DOG,
}

@embeddable({ abstract: true, discriminatorColumn: 'type' })
abstract class Animal {

  @enum(() => AnimalType)
  type!: AnimalType;

  @Property()
  name!: string;

}

@embeddable({ discriminatorValue: AnimalType.CAT })
class Cat extends Animal {

  @Property({ nullable: true })
  canMeow?: boolean = true;

  constructor(name: string) {
    super();
    this.type = AnimalType.CAT;
    this.name = name;
  }

}

@embeddable({ discriminatorValue: AnimalType.DOG })
class Dog extends Animal {

  @Property({ nullable: true })
  canBark?: boolean = true;

  constructor(name: string) {
    super();
    this.type = AnimalType.DOG;
    this.name = name;
  }

}

@entity()
class Owner {

  @PrimaryKey()
  id!: number;

  @Property()
  name!: string;

  @Embedded(() => [Cat, Dog])
  pet!: Cat | Dog;

}
```

Closes #1165

BREAKING CHANGE:

Embeddable instances are now created via `EntityFactory` and they respect the
`forceEntityConstructor` configuration. Due to this we need to have EM instance
when assigning to embedded properties. 

Using `em.assign()` should be preferred to get around this.

Deep assigning of child entities now works by default based on the presence of PKs in the payload.
This behaviour can be disable via updateByPrimaryKey: false in the assign options.

`mergeObjects` option is now enabled by default.
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

Successfully merging a pull request may close this issue.

2 participants