fix: fallback to entity property if referencedColumn returns nothing #9572
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of change
This PR adds a fallback mechanism to one of
ColumnMetadata.getEntityValue
conditional branches. It fixes #9565 -- saving an entity with an object literal foreign key.The root cause of the linked issue is that there is no differentiation between object literal that represents an actual entity and an object literal representing a column value. This PR does not solve this, but it applies a band-aid around one of edge cases that are affected by this behaviour.
Let's assume we're performing an INSERT/save of an entity, where one of its columns is a foreign key column and this column is an object literal (for example, an opaque type boxing a string value). During the query execution, at some point there's a call to
ColumnMetadata.getEntityValue
. Now see the following snippet (point in history before this PR):typeorm/src/metadata/ColumnMetadata.ts
Lines 806 to 832 in d305e5f
Eventually, execution falls into
if
in L806. Because our column is just the foreign key representation, but not the full entity, therelatedEntity
in L807 is undefined. This means we'll skip the first nestedif
and start checking theelse if
in L809-L815. The problem is that because the column value is an object literal it passes all the predicates and we enter the branch. And in L816 we're going to callgetEntityValue
on the related column, passing our object literal as the entity. This is not correct, because the object literal only represents the "id", not the full entity. This call will most likely (but not always, more on that in a while) result invalue
being undefined, because the property to retrieve will not exist in our object literal. And because of that, thevalue
returned eventually by thegetEntityValue
is going to beundefined
, which causes the INSERT/save to fail.The fix here is to assign raw
entity[this.propertyName]
tovalue
whenever thethis.referencedColumn.getEntityValue
returnsundefined
. This addresses directly the scenario that I described here.Very similar solution, but structured in a slightly different way, might be to assign
entity[this.propertyName]
tovalue
almost at the end of the method (before transformation) if it's not assigned yet. I looked at the branching in this method and it looks it should work, but this might have side-effects to cases I did not look into, that's why I decided to put the fallback assignment specifically in the affected case.There's still risk that our object literal will contain the property of name
this.referencedColumn.propertyName
, and then my fix will not work -- typeorm will try to assign extracted value from object literal instead of the whole object literal (so most likely the operation will fail with aTypeError
during the transformation step or not fail at all, saving wrong data to the database).To solve this problem completely, the execution should not fall into this 2nd nested branch at all -- it should go directly to the final
else
and use theentity[this.propertyName]
value verbatim. I refactored the method a bit and named the compound predicate to better visualize what decisions are made in the branches:typeorm/src/metadata/ColumnMetadata.ts
Lines 969 to 984 in cc8d76b
If there is a way to make sure that the object literal passed to
getEntityValue
definitely is or is not an entity and replacedisEntityLike
with such check, it should solve the issue entirely.Pull-Request Checklist
master
branchnpm run format
to apply prettier formattingnpm run test
passes with this changeFixes #0000