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

Setting a relational customField on Asset with relation Asset results in Errors #2891

Open
Sascha6790 opened this issue Jun 12, 2024 · 3 comments
Assignees
Labels
ORM Issues relating to potential bugs in the ORM type: bug 🐛 Something isn't working

Comments

@Sascha6790
Copy link

Describe the bug
Can not save product when using the entity 'Asset' as an relational customField on Asset in the admin ui.

To Reproduce
Steps to reproduce the behavior:

  1. Create a customField on Asset
export function addPoster(config: RuntimeVendureConfig) {
  config.customFields.Asset.push({
    type: 'relation',
    entity: Asset,
    name: 'poster',
    nullable: true,
  });
}
  1. Create a Product with a variant
  2. Assign an asset to the variant
  3. go back to the product and update the product
  4. GetProductVariantListForProduct throws an Error
{
  "errors": [
    {
      "message": "table name \"Asset\" specified more than once",
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ],
      "path": [
        "productVariants"
      ]
    }
  ],
  "data": null
}
[TypeORM] Query error: SELECT "Asset"."createdAt" AS "Asset_createdAt", "Asset"."updatedAt" AS "Asset_updatedAt", "Asset"."name" AS "Asset_name", "Asset"."type" AS "Asset_type", "Asset"."mimeType" AS "Asset_mimeType", "Asset"."width" AS "Asset_width", "Asset"."height" AS "Asset_height", "Asset"."fileSize" AS "Asset_fileSize", "Asset"."source" AS "Asset_source", "Asset"."preview" AS "Asset_preview", "Asset"."focalPoint" AS "Asset_focalPoint", "Asset"."id" AS "Asset_id", "Asset"."customFieldsPosterid" AS "Asset_customFieldsPosterid", "Asset"."customFieldsAlten" AS "Asset_customFieldsAlten", "Asset"."customFieldsAltde" AS "Asset_customFieldsAltde", "Asset"."customFieldsDescription" AS "Asset_customFieldsDescription", "Asset"."createdAt" AS "Asset_createdAt", "Asset"."updatedAt" AS "Asset_updatedAt", "Asset"."name" AS "Asset_name", "Asset"."type" AS "Asset_type", "Asset"."mimeType" AS "Asset_mimeType", "Asset"."width" AS "Asset_width", "Asset"."height" AS "Asset_height", "Asset"."fileSize" AS "Asset_fileSize", "Asset"."source" AS "Asset_source", "Asset"."preview" AS "Asset_preview", "Asset"."focalPoint" AS "Asset_focalPoint", "Asset"."id" AS "Asset_id", "Asset"."customFieldsPosterid" AS "Asset_customFieldsPosterid", "Asset"."customFieldsAlten" AS "Asset_customFieldsAlten", "Asset"."customFieldsAltde" AS "Asset_customFieldsAltde", "Asset"."customFieldsDescription" AS "Asset_customFieldsDescription" 
FROM "public"."asset" "Asset" 
INNER JOIN "public"."asset" "Asset" ON "Asset"."customFieldsPosterid" = "Asset"."id" 
WHERE "Asset"."id" IN ($1) -- PARAMETERS: ["98aa9ae4-f32e-4aa4-b87f-6e37c9f39838"] 

Expected behavior
The product gets saved.

Environment (please complete the following information):
vendure 2.2.4,
npm 10.5.0
node 20.12.0
postgres:15.4-alpine

Additional context
i can set internal: true and it works.

export function addPoster(config: RuntimeVendureConfig) {
  config.customFields.Asset.push({
    type: 'relation',
    entity: Asset,
    name: 'poster',
    internal: true,
    nullable: true,
  });

}

@Sascha6790 Sascha6790 added the type: bug 🐛 Something isn't working label Jun 12, 2024
@michaelbromley
Copy link
Member

Thanks for the report. I'm looking into this, and it appears it may well be a bug inside TypeORM. I have defined the "poster" custom field and when I attempt to load a product detail in the Admin UI, I get the reported error. Debugging through all the generated SQL queries generated by this request, I get to the specific one that fails:

SELECT `Asset`.`createdAt`                                    AS `Asset_createdAt`,
       `Asset`.`updatedAt`                                    AS `Asset_updatedAt`,
       `Asset`.`name`                                         AS `Asset_name`,
       `Asset`.`type`                                         AS `Asset_type`,
       `Asset`.`mimeType`                                     AS `Asset_mimeType`,
       `Asset`.`width`                                        AS `Asset_width`,
       `Asset`.`height`                                       AS `Asset_height`,
       `Asset`.`fileSize`                                     AS `Asset_fileSize`,
       `Asset`.`source`                                       AS `Asset_source`,
       `Asset`.`preview`                                      AS `Asset_preview`,
       `Asset`.`focalPoint`                                   AS `Asset_focalPoint`,
       `Asset`.`id`                                           AS `Asset_id`,
       `Asset`.`customFieldsPosterid`                         AS `Asset_customFieldsPosterid`,
       `Asset`.`customFields__fix_relational_custom_fields__` AS `Asset_customFields__fix_relational_custom_fields__`,
       `Asset`.`createdAt`                                    AS `Asset_createdAt`,
       `Asset`.`updatedAt`                                    AS `Asset_updatedAt`,
       `Asset`.`name`                                         AS `Asset_name`,
       `Asset`.`type`                                         AS `Asset_type`,
       `Asset`.`mimeType`                                     AS `Asset_mimeType`,
       `Asset`.`width`                                        AS `Asset_width`,
       `Asset`.`height`                                       AS `Asset_height`,
       `Asset`.`fileSize`                                     AS `Asset_fileSize`,
       `Asset`.`source`                                       AS `Asset_source`,
       `Asset`.`preview`                                      AS `Asset_preview`,
       `Asset`.`focalPoint`                                   AS `Asset_focalPoint`,
       `Asset`.`id`                                           AS `Asset_id`,
       `Asset`.`customFieldsPosterid`                         AS `Asset_customFieldsPosterid`,
       `Asset`.`customFields__fix_relational_custom_fields__` AS `Asset_customFields__fix_relational_custom_fields__`
FROM `asset` `Asset`
         INNER JOIN `asset` `Asset` ON `Asset`.`customFieldsPosterid` = `Asset`.`id`
WHERE `Asset`.`id` IN (?)

with the following stack trace:

image

I'm investigating whether there is any kind of work-around we can perform on our side.

@michaelbromley
Copy link
Member

Given the following custom field config:

 customFields: {
        Asset: [
            {
                type: 'relation',
                entity: Asset,
                name: 'self',
                nullable: true,
            },
        ],
    },

here's a minimal query to the Admin or Shop API which reproduces the error:

{
  product(id: 1) {
    featuredAsset {
      customFields {
        self {
          id
        }
      }
    }
  }
}

Interestingly, running the same query against collection will not produce the error. This may be due to the fact that there is currently a work-around which causes the collection query to opt-out of the relationLoadStrategy: 'query' strategy of TypeORM.

This is supported by the fact that if I comment out this line, the error will not appear.

So we can narrow this down to the internal implementation in TypeORM of the "query" relationLoadStrategy - something that has proven a source of bugs in the past. Which is very unfortunate because it brings huge performance improvements overall.

On further investigation I believe we are running into exactly this bug:

@michaelbromley michaelbromley added the ORM Issues relating to potential bugs in the ORM label Jun 17, 2024
@Sascha6790
Copy link
Author

Sascha6790 commented Jun 18, 2024

I see, that's unfortunate but you still helped me a lot, thank you.

Patching typeorm with npx patch-package works for now as a temporarily solution
Maybe it's possible to make relationLoadStrategy configurable for now but actually should be solved on typeorms end

Leaving the patch here:

diff --git a/node_modules/typeorm/browser/query-builder/SelectQueryBuilder.js b/node_modules/typeorm/browser/query-builder/SelectQueryBuilder.js
index c78a50f..c4e90e6 100644
--- a/node_modules/typeorm/browser/query-builder/SelectQueryBuilder.js
+++ b/node_modules/typeorm/browser/query-builder/SelectQueryBuilder.js
@@ -2052,7 +2052,12 @@ export class SelectQueryBuilder extends QueryBuilder {
             const queryStrategyRelationIdLoader = new QueryStrategyRelationIdLoader(this.connection, queryRunner);
             await Promise.all(this.relationMetadatas.map(async (relation) => {
                 const relationTarget = relation.inverseEntityMetadata.target;
-                const relationAlias = relation.inverseEntityMetadata.targetName;
+                const relationAlias = this.connection.namingStrategy.joinTableName(
+                  relation.propertyName,
+                  relation.inverseEntityMetadata.name,
+                  relation.propertyName,
+                  relation.inverseSidePropertyPath,
+                );
                 const select = Array.isArray(this.findOptions.select)
                     ? OrmUtils.propertyPathsToTruthyObject(this.findOptions.select)
                     : this.findOptions.select;
diff --git a/node_modules/typeorm/query-builder/SelectQueryBuilder.js b/node_modules/typeorm/query-builder/SelectQueryBuilder.js
index 8eda00e..fdf7e7c 100644
--- a/node_modules/typeorm/query-builder/SelectQueryBuilder.js
+++ b/node_modules/typeorm/query-builder/SelectQueryBuilder.js
@@ -2055,7 +2055,11 @@ class SelectQueryBuilder extends QueryBuilder_1.QueryBuilder {
             const queryStrategyRelationIdLoader = new RelationIdLoader_2.RelationIdLoader(this.connection, queryRunner);
             await Promise.all(this.relationMetadatas.map(async (relation) => {
                 const relationTarget = relation.inverseEntityMetadata.target;
-                const relationAlias = relation.inverseEntityMetadata.targetName;
+                const relationAlias =  this.connection.namingStrategy.joinTableName(
+                  relation.propertyName,
+                  relation.propertyName,
+                  relation.inverseSidePropertyPath,
+                )
                 const select = Array.isArray(this.findOptions.select)
                     ? OrmUtils_1.OrmUtils.propertyPathsToTruthyObject(this.findOptions.select)
                     : this.findOptions.select;

File: typeorm+0.3.20.patch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ORM Issues relating to potential bugs in the ORM type: bug 🐛 Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants