fix: fallback to entity property if referencedColumn returns nothing

[akwodkiewicz]

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

	if (this.relationMetadata && this.referencedColumn) {
	const relatedEntity =
	this.relationMetadata.getEntityValue(entity)
	if (
	relatedEntity &&
	ObjectUtils.isObject(relatedEntity) &&
	!InstanceChecker.isFindOperator(relatedEntity) &&
	!(typeof relatedEntity === "function") &&
	!Buffer.isBuffer(relatedEntity)
	) {
	value = this.referencedColumn.getEntityValue(relatedEntity)
	} else if (
	entity[this.propertyName] &&
	ObjectUtils.isObject(entity[this.propertyName]) &&
	!InstanceChecker.isFindOperator(
	entity[this.propertyName],
	) &&
	!(typeof entity[this.propertyName] === "function") &&
	!Buffer.isBuffer(entity[this.propertyName]) &&
	!(entity[this.propertyName] instanceof Date)
	) {
	value = this.referencedColumn.getEntityValue(
	entity[this.propertyName],
	)
	} else {
	value = entity[this.propertyName]
	}

Eventually, execution falls into if in L806. Because our column is just the foreign key representation, but not the full entity, the relatedEntity in L807 is undefined. This means we'll skip the first nested if and start checking the else 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 call getEntityValue 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 in value being undefined, because the property to retrieve will not exist in our object literal. And because of that, the value returned eventually by the getEntityValue is going to be undefined, which causes the INSERT/save to fail.

The fix here is to assign raw entity[this.propertyName] to value whenever the this.referencedColumn.getEntityValue returns undefined. 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] to value 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 a TypeError 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 the entity[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

	/**
	* Apply some heuristics to be more certain that `value` is an object representing
	* another entity.
	*
	* This is not 100% correct -- `value` might be an object literal
	* representing a column value that is **not** an entity.
	*/
	private isEntityLike(value: unknown): boolean {
	return (
	ObjectUtils.isObject(value) &&
	value &&
	!InstanceChecker.isFindOperator(value) &&
	!Buffer.isBuffer(value) &&
	!(value instanceof Date)
	)
	}

If there is a way to make sure that the object literal passed to getEntityValue definitely is or is not an entity and replaced isEntityLike with such check, it should solve the issue entirely.

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

[akwodkiewicz]

@pleerock, apologies for reaching to you directly, but I'd like to make sure that the approach I chose here is a correct one. If you believe that the issue should be fixed in a different way, I can try doing it from scratch, just need your guidance here.