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: Use concrete subclass for saving with single table inheritance #8819

Merged
merged 1 commit into from Apr 2, 2022

Conversation

felix-gohla
Copy link
Contributor

@felix-gohla felix-gohla commented Mar 28, 2022

Description of change

When using a STI scheme and an EntityManager or Repository with base class target, the wrong discriminator was written to the database despite giving a concrete entity.
This was because of the entity's target being ignored in favour of the target of the Repository or the EntityManager, which then is the (abstract) base class.

This PR introduces a fix by figuring out the concrete subclass of an entity when saving.

Further questions from my side:

  • Currently, it allows for subclass entities to be found by querying the class from childEntityMetadatas for every entity. What about performance problems?
  • Should this behaviour be deactivatable?
  • Is there any documentation that has to be updated?

This PR fixes #2927 (which was closed by the author without the problem being resolved).

Pull-Request Checklist

  • Code is up-to-date with the master branch
  • npm run format to apply prettier formatting
  • npm run test passes with this change
  • This pull request links relevant issues as Fixes #0000
  • There are new or updated unit tests validating the change
  • Documentation has been updated to reflect this change
  • The new commits follow conventions explained in CONTRIBUTING.md

@@ -81,10 +81,28 @@ export class EntityPersistExecutor {
if (entityTarget === Object)
throw new CannotDetermineEntityError(this.mode)

var metadata = this.connection.getMetadata(entityTarget)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you please use let instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. 😊

When using a STI scheme and an EntityManager or Repository
with base class target, the wrong discriminator was written to the
database despite giving an concrete entity.
This was because of the entity's target being ignored in favor of the
target of the Repository or the EntityManager.

fixes typeorm#2927
@felix-gohla felix-gohla force-pushed the feature/2927-inheritance-types branch from 946c54d to f4f67c8 Compare March 30, 2022 15:34
@pleerock pleerock merged commit 9d1e246 into typeorm:master Apr 2, 2022
@pleerock
Copy link
Member

pleerock commented Apr 2, 2022

Thank you!

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.

When using base class' custom repository, the discriminator is ignored
2 participants