feat(core): Re-architect entity-asset relations to allow ordering

Relates to #156

BREAKING CHANGE: The internal representation of Asset relations has changed to enable explicit ordering of assets. This change means that the database schema had to be updated.

packages/core/e2e/__snapshots__/collection.e2e-spec.ts.snap

@@ -5,20 +5,20 @@ Object {
   "assets": Array [
     Object {
       "fileSize": 1680,
-      "id": "T_1",
+      "id": "T_2",
       "mimeType": "image/jpeg",
-      "name": "derick-david-409858-unsplash.jpg",
-      "preview": "test-url/test-assets/derick-david-409858-unsplash__preview.jpg",
-      "source": "test-url/test-assets/derick-david-409858-unsplash.jpg",
+      "name": "alexandru-acea-686569-unsplash.jpg",
+      "preview": "test-url/test-assets/alexandru-acea-686569-unsplash__preview.jpg",
+      "source": "test-url/test-assets/alexandru-acea-686569-unsplash.jpg",
       "type": "IMAGE",
     },
     Object {
       "fileSize": 1680,
-      "id": "T_2",
+      "id": "T_1",
       "mimeType": "image/jpeg",
-      "name": "alexandru-acea-686569-unsplash.jpg",
-      "preview": "test-url/test-assets/alexandru-acea-686569-unsplash__preview.jpg",
-      "source": "test-url/test-assets/alexandru-acea-686569-unsplash.jpg",
+      "name": "derick-david-409858-unsplash.jpg",
+      "preview": "test-url/test-assets/derick-david-409858-unsplash__preview.jpg",
+      "source": "test-url/test-assets/derick-david-409858-unsplash.jpg",
       "type": "IMAGE",
     },
   ],
@@ -69,7 +69,7 @@ Object {
 }
 `;
 
-exports[`Collection resolver updateCollection 1`] = `
+exports[`Collection resolver updateCollection updates with assets 1`] = `
 Object {
   "assets": Array [
     Object {
@@ -81,6 +81,15 @@ Object {
       "source": "test-url/test-assets/derick-david-409858-unsplash.jpg",
       "type": "IMAGE",
     },
+    Object {
+      "fileSize": 1680,
+      "id": "T_3",
+      "mimeType": "image/jpeg",
+      "name": "florian-olivo-1166419-unsplash.jpg",
+      "preview": "test-url/test-assets/florian-olivo-1166419-unsplash__preview.jpg",
+      "source": "test-url/test-assets/florian-olivo-1166419-unsplash.jpg",
+      "type": "IMAGE",
+    },
   ],
   "children": Array [],
   "description": "Apple stuff ",

packages/core/e2e/collection.e2e-spec.ts

@@ -194,6 +194,39 @@ describe('Collection resolver', () => {
         });
     });
 
+    describe('updateCollection', () => {
+        it('updates with assets', async () => {
+            const { updateCollection } = await client.query<
+                UpdateCollection.Mutation,
+                UpdateCollection.Variables
+            >(UPDATE_COLLECTION, {
+                input: {
+                    id: pearCollection.id,
+                    assetIds: [assets[1].id, assets[2].id],
+                    featuredAssetId: assets[1].id,
+                    translations: [{ languageCode: LanguageCode.en, description: 'Apple stuff ' }],
+                },
+            });
+
+            expect(updateCollection).toMatchSnapshot();
+        });
+
+        it('updating existing assets', async () => {
+            const { updateCollection } = await client.query<
+                UpdateCollection.Mutation,
+                UpdateCollection.Variables
+            >(UPDATE_COLLECTION, {
+                input: {
+                    id: pearCollection.id,
+                    assetIds: [assets[3].id, assets[0].id],
+                    featuredAssetId: assets[3].id,
+                },
+            });
+
+            expect(updateCollection.assets.map(a => a.id)).toEqual([assets[3].id, assets[0].id]);
+        });
+    });
+
     it('collection query', async () => {
         const result = await client.query<GetCollection.Query, GetCollection.Variables>(GET_COLLECTION, {
             id: computersCollection.id,
@@ -261,22 +294,6 @@ describe('Collection resolver', () => {
         expect(result.collection.breadcrumbs).toEqual([{ id: 'T_1', name: ROOT_COLLECTION_NAME }]);
     });
 
-    it('updateCollection', async () => {
-        const { updateCollection } = await client.query<
-            UpdateCollection.Mutation,
-            UpdateCollection.Variables
-        >(UPDATE_COLLECTION, {
-            input: {
-                id: pearCollection.id,
-                assetIds: [assets[1].id],
-                featuredAssetId: assets[1].id,
-                translations: [{ languageCode: LanguageCode.en, description: 'Apple stuff ' }],
-            },
-        });
-
-        expect(updateCollection).toMatchSnapshot();
-    });
-
     it('collections.assets', async () => {
         const { collections } = await client.query<GetCollectionsWithAssets.Query>(gql`
             query GetCollectionsWithAssets {

packages/core/e2e/product.e2e-spec.ts

@@ -536,6 +536,20 @@ describe('Product resolver', () => {
             expect(result.updateProduct.featuredAsset!.id).toBe(assets[0].id);
         });
 
+        it('updateProduct updates assets', async () => {
+            const result = await adminClient.query<UpdateProduct.Mutation, UpdateProduct.Variables>(
+                UPDATE_PRODUCT,
+                {
+                    input: {
+                        id: newProduct.id,
+                        featuredAssetId: 'T_1',
+                        assetIds: ['T_1', 'T_2'],
+                    },
+                },
+            );
+            expect(result.updateProduct.assets.map(a => a.id)).toEqual(['T_1', 'T_2']);
+        });
+
         it('updateProduct updates FacetValues', async () => {
             const result = await adminClient.query<UpdateProduct.Mutation, UpdateProduct.Variables>(
                 UPDATE_PRODUCT,
@@ -867,6 +881,29 @@ describe('Product resolver', () => {
                 expect(updatedVariant.featuredAsset!.id).toBe('T_2');
             });
 
+            it('updateProductVariants updates assets again', async () => {
+                const firstVariant = variants[0];
+                const result = await adminClient.query<
+                    UpdateProductVariants.Mutation,
+                    UpdateProductVariants.Variables
+                >(UPDATE_PRODUCT_VARIANTS, {
+                    input: [
+                        {
+                            id: firstVariant.id,
+                            assetIds: ['T_4', 'T_3'],
+                            featuredAssetId: 'T_4',
+                        },
+                    ],
+                });
+                const updatedVariant = result.updateProductVariants[0];
+                if (!updatedVariant) {
+                    fail('no updated variant returned.');
+                    return;
+                }
+                expect(updatedVariant.assets.map(a => a.id)).toEqual(['T_4', 'T_3']);
+                expect(updatedVariant.featuredAsset!.id).toBe('T_4');
+            });
+
             it('updateProductVariants updates taxCategory and priceBeforeTax', async () => {
                 const firstVariant = variants[0];
                 const result = await adminClient.query<

packages/core/src/api/resolvers/entity/collection-entity.resolver.ts

@@ -78,9 +78,6 @@ export class CollectionEntityResolver {
 
     @ResolveProperty()
     async assets(@Ctx() ctx: RequestContext, @Parent() collection: Collection): Promise<Asset[] | undefined> {
-        if (collection.assets) {
-            return collection.assets;
-        }
         return this.assetService.getEntityAssets(collection);
     }
 }

packages/core/src/api/resolvers/entity/product-entity.resolver.ts

@@ -1,10 +1,12 @@
 import { Parent, ResolveProperty, Resolver } from '@nestjs/graphql';
 
 import { Translated } from '../../../common/types/locale-types';
+import { Asset } from '../../../entity/asset/asset.entity';
 import { Collection } from '../../../entity/collection/collection.entity';
 import { ProductOptionGroup } from '../../../entity/product-option-group/product-option-group.entity';
 import { ProductVariant } from '../../../entity/product-variant/product-variant.entity';
 import { Product } from '../../../entity/product/product.entity';
+import { AssetService } from '../../../service/services/asset.service';
 import { CollectionService } from '../../../service/services/collection.service';
 import { ProductOptionGroupService } from '../../../service/services/product-option-group.service';
 import { ProductVariantService } from '../../../service/services/product-variant.service';
@@ -19,6 +21,7 @@ export class ProductEntityResolver {
         private productVariantService: ProductVariantService,
         private collectionService: CollectionService,
         private productOptionGroupService: ProductOptionGroupService,
+        private assetService: AssetService,
     ) {}
 
     @ResolveProperty()
@@ -28,7 +31,7 @@ export class ProductEntityResolver {
         @Api() apiType: ApiType,
     ): Promise<Array<Translated<ProductVariant>>> {
         const variants = await this.productVariantService.getVariantsByProductId(ctx, product.id);
-        return variants.filter(v => apiType === 'admin' ? true : v.enabled);
+        return variants.filter(v => (apiType === 'admin' ? true : v.enabled));
     }
 
     @ResolveProperty()
@@ -47,4 +50,17 @@ export class ProductEntityResolver {
     ): Promise<Array<Translated<ProductOptionGroup>>> {
         return this.productOptionGroupService.getOptionGroupsByProductId(ctx, product.id);
     }
+
+    @ResolveProperty()
+    async featuredAsset(@Ctx() ctx: RequestContext, @Parent() product: Product): Promise<Asset | undefined> {
+        if (product.featuredAsset) {
+            return product.featuredAsset;
+        }
+        return this.assetService.getFeaturedAsset(product);
+    }
+
+    @ResolveProperty()
+    async assets(@Ctx() ctx: RequestContext, @Parent() product: Product): Promise<Asset[] | undefined> {
+        return this.assetService.getEntityAssets(product);
+    }
 }

packages/core/src/api/resolvers/entity/product-variant-entity.resolver.ts

@@ -6,6 +6,7 @@ import { Translated } from '../../../common/types/locale-types';
 import { Asset, FacetValue, ProductOption } from '../../../entity';
 import { ProductVariant } from '../../../entity/product-variant/product-variant.entity';
 import { StockMovement } from '../../../entity/stock-movement/stock-movement.entity';
+import { AssetService } from '../../../service/services/asset.service';
 import { ProductVariantService } from '../../../service/services/product-variant.service';
 import { StockMovementService } from '../../../service/services/stock-movement.service';
 import { ApiType } from '../../common/get-api-type';
@@ -15,28 +16,25 @@ import { Ctx } from '../../decorators/request-context.decorator';
 
 @Resolver('ProductVariant')
 export class ProductVariantEntityResolver {
-    constructor(private productVariantService: ProductVariantService) {}
+    constructor(private productVariantService: ProductVariantService, private assetService: AssetService) {}
 
     @ResolveProperty()
     async assets(
         @Ctx() ctx: RequestContext,
         @Parent() productVariant: ProductVariant,
-    ): Promise<Asset[]> {
-        if (productVariant.assets) {
-            return productVariant.assets;
-        }
-        return this.productVariantService.getAssetsForVariant(ctx, productVariant.id);
+    ): Promise<Asset[] | undefined> {
+        return this.assetService.getEntityAssets(productVariant);
     }
 
     @ResolveProperty()
     async featuredAsset(
         @Ctx() ctx: RequestContext,
         @Parent() productVariant: ProductVariant,
-    ): Promise<Asset> {
+    ): Promise<Asset | undefined> {
         if (productVariant.featuredAsset) {
             return productVariant.featuredAsset;
         }
-        return this.productVariantService.getFeaturedAssetForVariant(ctx, productVariant.id);
+        return this.assetService.getFeaturedAsset(productVariant);
     }
 
     @ResolveProperty()
@@ -79,6 +77,10 @@ export class ProductVariantAdminEntityResolver {
         @Parent() productVariant: ProductVariant,
         @Args() args: { options: StockMovementListOptions },
     ): Promise<PaginatedList<StockMovement>> {
-        return this.stockMovementService.getStockMovementsByProductVariantId(ctx, productVariant.id, args.options);
+        return this.stockMovementService.getStockMovementsByProductVariantId(
+            ctx,
+            productVariant.id,
+            args.options,
+        );
     }
 }

packages/core/src/data-import/providers/importer/fast-importer.service.ts

@@ -14,9 +14,11 @@ import { ProductOptionGroupTranslation } from '../../../entity/product-option-gr
 import { ProductOptionGroup } from '../../../entity/product-option-group/product-option-group.entity';
 import { ProductOptionTranslation } from '../../../entity/product-option/product-option-translation.entity';
 import { ProductOption } from '../../../entity/product-option/product-option.entity';
+import { ProductVariantAsset } from '../../../entity/product-variant/product-variant-asset.entity';
 import { ProductVariantPrice } from '../../../entity/product-variant/product-variant-price.entity';
 import { ProductVariantTranslation } from '../../../entity/product-variant/product-variant-translation.entity';
 import { ProductVariant } from '../../../entity/product-variant/product-variant.entity';
+import { ProductAsset } from '../../../entity/product/product-asset.entity';
 import { ProductTranslation } from '../../../entity/product/product-translation.entity';
 import { Product } from '../../../entity/product/product.entity';
 import { TranslatableSaver } from '../../../service/helpers/translatable-saver/translatable-saver';
@@ -56,11 +58,19 @@ export class FastImporterService {
                 if (input.featuredAssetId) {
                     p.featuredAsset = { id: input.featuredAssetId } as any;
                 }
-                if (input.assetIds) {
-                    p.assets = input.assetIds.map(id => ({ id } as any));
-                }
             },
         });
+        if (input.assetIds) {
+            const productAssets = input.assetIds.map(
+                (id, i) =>
+                    new ProductAsset({
+                        assetId: id,
+                        productId: product.id,
+                        position: i,
+                    }),
+            );
+            await this.connection.getRepository(ProductAsset).save(productAssets);
+        }
         return product.id;
     }
 
@@ -116,11 +126,19 @@ export class FastImporterService {
                 if (input.featuredAssetId) {
                     variant.featuredAsset = { id: input.featuredAssetId } as any;
                 }
-                if (input.assetIds) {
-                    variant.assets = input.assetIds.map(id => ({ id } as any));
-                }
             },
         });
+        if (input.assetIds) {
+            const variantAssets = input.assetIds.map(
+                (id, i) =>
+                    new ProductVariantAsset({
+                        assetId: id,
+                        productVariantId: createdVariant.id,
+                        position: i,
+                    }),
+            );
+            await this.connection.getRepository(ProductVariantAsset).save(variantAssets);
+        }
         if (input.stockOnHand != null && input.stockOnHand !== 0) {
             await this.stockMovementService.adjustProductVariantStock(
                 createdVariant.id,

packages/core/src/entity/asset/orderable-asset.entity.ts

@@ -0,0 +1,28 @@
+import { DeepPartial, ID } from '@vendure/common/lib/shared-types';
+import { Column, ManyToOne } from 'typeorm';
+
+import { Orderable } from '../../common/types/common-types';
+import { Asset } from '../asset/asset.entity';
+import { VendureEntity } from '../base/base.entity';
+
+/**
+ * This base class is extended in order to enable specific ordering of the one-to-many
+ * Entity -> Assets relation. Using a many-to-many relation does not provide a way
+ * to guarantee order of the Assets, so this entity is used in place of the
+ * usual join table that would be created by TypeORM.
+ * See https://typeorm.io/#/many-to-many-relations/many-to-many-relations-with-custom-properties
+ */
+export abstract class OrderableAsset extends VendureEntity implements Orderable {
+    protected constructor(input?: DeepPartial<OrderableAsset>) {
+        super(input);
+    }
+
+    @Column()
+    assetId: ID;
+
+    @ManyToOne(type => Asset, { eager: true })
+    asset: Asset;
+
+    @Column()
+    position: number;
+}

packages/core/src/entity/collection/collection-asset.entity.ts

@@ -0,0 +1,18 @@
+import { DeepPartial, ID } from '@vendure/common/lib/shared-types';
+import { Column, Entity, ManyToOne } from 'typeorm';
+
+import { OrderableAsset } from '../asset/orderable-asset.entity';
+
+import { Collection } from './collection.entity';
+
+@Entity()
+export class CollectionAsset extends OrderableAsset {
+    constructor(input?: DeepPartial<CollectionAsset>) {
+        super(input);
+    }
+    @Column()
+    collectionId: ID;
+
+    @ManyToOne(type => Collection, collection => collection.assets)
+    collection: Collection;
+}

packages/core/src/entity/collection/collection.entity.ts

@@ -20,6 +20,7 @@ import { Channel } from '../channel/channel.entity';
 import { CustomCollectionFields } from '../custom-entity-fields';
 import { ProductVariant } from '../product-variant/product-variant.entity';
 
+import { CollectionAsset } from './collection-asset.entity';
 import { CollectionTranslation } from './collection-translation.entity';
 
 /**
@@ -59,9 +60,8 @@ export class Collection extends VendureEntity
     @ManyToOne(type => Asset)
     featuredAsset: Asset;
 
-    @ManyToMany(type => Asset)
-    @JoinTable()
-    assets: Asset[];
+    @OneToMany(type => CollectionAsset, collectionAsset => collectionAsset.collection)
+    assets: CollectionAsset[];
 
     @Column('simple-json') filters: ConfigurableOperation[];
 

packages/core/src/entity/entities.ts

@@ -2,6 +2,7 @@ import { Address } from './address/address.entity';
 import { Administrator } from './administrator/administrator.entity';
 import { Asset } from './asset/asset.entity';
 import { Channel } from './channel/channel.entity';
+import { CollectionAsset } from './collection/collection-asset.entity';
 import { CollectionTranslation } from './collection/collection-translation.entity';
 import { Collection } from './collection/collection.entity';
 import { CountryTranslation } from './country/country-translation.entity';
@@ -25,9 +26,11 @@ import { ProductOptionGroupTranslation } from './product-option-group/product-op
 import { ProductOptionGroup } from './product-option-group/product-option-group.entity';
 import { ProductOptionTranslation } from './product-option/product-option-translation.entity';
 import { ProductOption } from './product-option/product-option.entity';
+import { ProductVariantAsset } from './product-variant/product-variant-asset.entity';
 import { ProductVariantPrice } from './product-variant/product-variant-price.entity';
 import { ProductVariantTranslation } from './product-variant/product-variant-translation.entity';
 import { ProductVariant } from './product-variant/product-variant.entity';
+import { ProductAsset } from './product/product-asset.entity';
 import { ProductTranslation } from './product/product-translation.entity';
 import { Product } from './product/product.entity';
 import { Promotion } from './promotion/promotion.entity';
@@ -58,6 +61,7 @@ export const coreEntitiesMap = {
     Cancellation,
     Channel,
     Collection,
+    CollectionAsset,
     CollectionTranslation,
     Country,
     CountryTranslation,
@@ -77,12 +81,14 @@ export const coreEntitiesMap = {
     Payment,
     PaymentMethod,
     Product,
+    ProductAsset,
     ProductOption,
     ProductOptionGroup,
     ProductOptionGroupTranslation,
     ProductOptionTranslation,
     ProductTranslation,
     ProductVariant,
+    ProductVariantAsset,
     ProductVariantPrice,
     ProductVariantTranslation,
     Promotion,

packages/core/src/entity/index.ts

@@ -4,6 +4,7 @@ export * from './asset/asset.entity';
 export * from './base/base.entity';
 export * from './channel/channel.entity';
 export * from './collection/collection.entity';
+export * from './collection/collection-asset.entity';
 export * from './collection/collection-translation.entity';
 export * from './country/country.entity';
 export * from './country/country-translation.entity';
@@ -20,12 +21,14 @@ export * from './order-line/order-line.entity';
 export * from './payment/payment.entity';
 export * from './payment-method/payment-method.entity';
 export * from './product/product.entity';
+export * from './product/product-asset.entity';
 export * from './product/product-translation.entity';
 export * from './product-option/product-option.entity';
 export * from './product-option/product-option-translation.entity';
 export * from './product-option-group/product-option-group.entity';
 export * from './product-option-group/product-option-group-translation.entity';
 export * from './product-variant/product-variant.entity';
+export * from './product-variant/product-variant-asset.entity';
 export * from './product-variant/product-variant-translation.entity';
 export * from './product-variant/product-variant-price.entity';
 export * from './promotion/promotion.entity';

packages/core/src/entity/product-variant/product-variant-asset.entity.ts

@@ -0,0 +1,18 @@
+import { DeepPartial, ID } from '@vendure/common/lib/shared-types';
+import { Column, Entity, ManyToOne } from 'typeorm';
+
+import { OrderableAsset } from '../asset/orderable-asset.entity';
+
+import { ProductVariant } from './product-variant.entity';
+
+@Entity()
+export class ProductVariantAsset extends OrderableAsset {
+    constructor(input?: DeepPartial<ProductVariantAsset>) {
+        super(input);
+    }
+    @Column()
+    productVariantId: ID;
+
+    @ManyToOne(type => ProductVariant, variant => variant.assets)
+    productVariant: ProductVariant;
+}

packages/core/src/entity/product-variant/product-variant.entity.ts

@@ -16,6 +16,7 @@ import { StockMovement } from '../stock-movement/stock-movement.entity';
 import { TaxCategory } from '../tax-category/tax-category.entity';
 import { TaxRate } from '../tax-rate/tax-rate.entity';
 
+import { ProductVariantAsset } from './product-variant-asset.entity';
 import { ProductVariantPrice } from './product-variant-price.entity';
 import { ProductVariantTranslation } from './product-variant-translation.entity';
 
@@ -78,9 +79,8 @@ export class ProductVariant extends VendureEntity implements Translatable, HasCu
     @ManyToOne(type => Asset)
     featuredAsset: Asset;
 
-    @ManyToMany(type => Asset)
-    @JoinTable()
-    assets: Asset[];
+    @OneToMany(type => ProductVariantAsset, productVariantAsset => productVariantAsset.productVariant)
+    assets: ProductVariantAsset[];
 
     @ManyToOne(type => TaxCategory)
     taxCategory: TaxCategory;

packages/core/src/entity/product/product-asset.entity.ts

@@ -0,0 +1,18 @@
+import { DeepPartial, ID } from '@vendure/common/lib/shared-types';
+import { Column, Entity, ManyToOne } from 'typeorm';
+
+import { OrderableAsset } from '../asset/orderable-asset.entity';
+
+import { Product } from './product.entity';
+
+@Entity()
+export class ProductAsset extends OrderableAsset {
+    constructor(input?: DeepPartial<ProductAsset>) {
+        super(input);
+    }
+    @Column()
+    productId: ID;
+
+    @ManyToOne(type => Product, product => product.assets)
+    product: Product;
+}

packages/core/src/entity/product/product.entity.ts

@@ -12,6 +12,7 @@ import { FacetValue } from '../facet-value/facet-value.entity';
 import { ProductOptionGroup } from '../product-option-group/product-option-group.entity';
 import { ProductVariant } from '../product-variant/product-variant.entity';
 
+import { ProductAsset } from './product-asset.entity';
 import { ProductTranslation } from './product-translation.entity';
 
 /**
@@ -43,9 +44,8 @@ export class Product extends VendureEntity
     @ManyToOne(type => Asset)
     featuredAsset: Asset;
 
-    @ManyToMany(type => Asset)
-    @JoinTable()
-    assets: Asset[];
+    @OneToMany(type => ProductAsset, productAsset => productAsset.product)
+    assets: ProductAsset[];
 
     @OneToMany(type => ProductTranslation, translation => translation.base, { eager: true })
     translations: Array<Translation<Product>>;

packages/core/src/service/helpers/asset-updater/asset-updater.ts

@@ -1,48 +0,0 @@
-import { Injectable } from '@nestjs/common';
-import { InjectConnection } from '@nestjs/typeorm';
-import { Connection } from 'typeorm';
-
-import { idsAreEqual } from '../../../common/utils';
-import { Asset, VendureEntity } from '../../../entity';
-import { AssetService } from '../../services/asset.service';
-
-export interface EntityWithAssets extends VendureEntity {
-    featuredAsset: Asset;
-    assets: Asset[];
-}
-
-export interface AssetInput {
-    featuredAssetId?: string | null;
-    assetIds?: string[] | null;
-}
-
-@Injectable()
-export class AssetUpdater {
-    constructor(@InjectConnection() private connection: Connection, private assetService: AssetService) {}
-
-    /**
-     * Updates the assets / featuredAsset of an entity, ensuring that only valid assetIds are used.
-     */
-    async updateEntityAssets<T extends EntityWithAssets>(entity: T, input: AssetInput) {
-        if (input.assetIds || input.featuredAssetId) {
-            if (input.assetIds) {
-                const assets = await this.assetService.findByIds(input.assetIds);
-                entity.assets = assets;
-                const featuredAssetId = input.featuredAssetId;
-                if (featuredAssetId) {
-                    // If the featuredAsset is also being set, we save the additional
-                    // DB query since we already have that asset from the findByIds query.
-                    const featuredAsset = assets.find(a => idsAreEqual(a.id, featuredAssetId));
-                    if (featuredAsset) {
-                        entity.featuredAsset = featuredAsset;
-                    }
-                }
-            } else if (input.featuredAssetId) {
-                const featuredAsset = await this.assetService.findOne(input.featuredAssetId);
-                if (featuredAsset) {
-                    entity.featuredAsset = featuredAsset;
-                }
-            }
-        }
-    }
-}

packages/core/src/service/service.module.ts

@@ -7,7 +7,6 @@ import { EventBusModule } from '../event-bus/event-bus.module';
 import { WorkerServiceModule } from '../worker/worker-service.module';
 
 import { CollectionController } from './controllers/collection.controller';
-import { AssetUpdater } from './helpers/asset-updater/asset-updater';
 import { ListQueryBuilder } from './helpers/list-query-builder/list-query-builder';
 import { OrderCalculator } from './helpers/order-calculator/order-calculator';
 import { OrderMerger } from './helpers/order-merger/order-merger';
@@ -105,7 +104,6 @@ let workerTypeOrmModule: DynamicModule;
         PaymentStateMachine,
         ListQueryBuilder,
         ShippingCalculator,
-        AssetUpdater,
         VerificationTokenGenerator,
         RefundStateMachine,
         ShippingConfiguration,

packages/core/src/service/services/asset.service.ts

@@ -1,7 +1,8 @@
 import { Injectable } from '@nestjs/common';
 import { InjectConnection } from '@nestjs/typeorm';
 import { AssetType, CreateAssetInput } from '@vendure/common/lib/generated-types';
-import { ID, PaginatedList } from '@vendure/common/lib/shared-types';
+import { ID, PaginatedList, Type } from '@vendure/common/lib/shared-types';
+import { notNullOrUndefined } from '@vendure/common/lib/shared-utils';
 import { ReadStream } from 'fs-extra';
 import sizeOf from 'image-size';
 import mime from 'mime-types';
@@ -11,13 +12,19 @@ import { Connection, Like } from 'typeorm';
 
 import { InternalServerError } from '../../common/error/errors';
 import { ListQueryOptions } from '../../common/types/common-types';
-import { getAssetType } from '../../common/utils';
+import { getAssetType, idsAreEqual } from '../../common/utils';
 import { ConfigService } from '../../config/config.service';
 import { Logger } from '../../config/logger/vendure-logger';
 import { Asset } from '../../entity/asset/asset.entity';
-import { EntityWithAssets } from '../helpers/asset-updater/asset-updater';
+import { OrderableAsset } from '../../entity/asset/orderable-asset.entity';
+import { VendureEntity } from '../../entity/base/base.entity';
 import { ListQueryBuilder } from '../helpers/list-query-builder/list-query-builder';
 
+export interface EntityWithAssets extends VendureEntity {
+    featuredAsset: Asset;
+    assets: OrderableAsset[];
+}
+
 @Injectable()
 export class AssetService {
     constructor(
@@ -30,10 +37,6 @@ export class AssetService {
         return this.connection.getRepository(Asset).findOne(id);
     }
 
-    findByIds(ids: ID[]): Promise<Asset[]> {
-        return this.connection.getRepository(Asset).findByIds(ids);
-    }
-
     /**
      * Locates an Asset by the filename of the source file. If "exact" is set to false, filename will
      * be looked up without the extension and with a "%" wildcard at the end. This is useful for finding
@@ -69,13 +72,49 @@ export class AssetService {
     }
 
     async getEntityAssets<T extends EntityWithAssets>(entity: T): Promise<Asset[] | undefined> {
-        const entityType = Object.getPrototypeOf(entity).constructor;
-        const entityWithFeaturedAsset = await this.connection
-            .getRepository<EntityWithAssets>(entityType)
-            .findOne(entity.id, {
-                relations: ['assets'],
-            });
-        return entityWithFeaturedAsset && entityWithFeaturedAsset.assets;
+        let assets = entity.assets;
+        if (!assets) {
+            const entityType = Object.getPrototypeOf(entity).constructor;
+            const entityWithAssets = await this.connection
+                .getRepository<EntityWithAssets>(entityType)
+                .findOne(entity.id, {
+                    relations: ['assets'],
+                });
+            assets = (entityWithAssets && entityWithAssets.assets) || [];
+        }
+        return assets.sort((a, b) => a.position - b.position).map(a => a.asset);
+    }
+
+    async updateFeaturedAsset<T extends EntityWithAssets>(
+        entity: T,
+        featuredAssetId?: ID | null,
+    ): Promise<T> {
+        if (!featuredAssetId) {
+            return entity;
+        }
+        const featuredAsset = await this.findOne(featuredAssetId);
+        if (featuredAsset) {
+            entity.featuredAsset = featuredAsset;
+        }
+        return entity;
+    }
+
+    /**
+     * Updates the assets / featuredAsset of an entity, ensuring that only valid assetIds are used.
+     */
+    async updateEntityAssets<T extends EntityWithAssets>(entity: T, assetIds?: ID[] | null): Promise<T> {
+        if (!entity.id) {
+            throw new InternalServerError('error.entity-must-have-an-id');
+        }
+        if (assetIds && assetIds.length) {
+            const assets = await this.connection.getRepository(Asset).findByIds(assetIds);
+            const sortedAssets = assetIds
+                .map(id => assets.find(a => idsAreEqual(a.id, id)))
+                .filter(notNullOrUndefined);
+            this.removeExistingOrderableAssets(entity);
+            entity.assets = await this.createOrderableAssets(entity, sortedAssets);
+        }
+        return entity;
     }
 
     /**
@@ -165,4 +204,51 @@ export class AssetService {
             return { width: 0, height: 0 };
         }
     }
+
+    private createOrderableAssets(entity: EntityWithAssets, assets: Asset[]): Promise<OrderableAsset[]> {
+        const orderableAssets = assets.map((asset, i) => this.getOrderableAsset(entity, asset, i));
+        return this.connection.getRepository(orderableAssets[0].constructor).save(orderableAssets);
+    }
+
+    private getOrderableAsset(entity: EntityWithAssets, asset: Asset, index: number): OrderableAsset {
+        const entityIdProperty = this.getHostEntityIdProperty(entity);
+        const orderableAssetType = this.getOrderableAssetType(entity);
+        return new orderableAssetType({
+            assetId: asset.id,
+            position: index,
+            [entityIdProperty]: entity.id,
+        });
+    }
+
+    private async removeExistingOrderableAssets(entity: EntityWithAssets) {
+        const propertyName = this.getHostEntityIdProperty(entity);
+        const orderableAssetType = this.getOrderableAssetType(entity);
+        await this.connection.getRepository(orderableAssetType).delete({
+            [propertyName]: entity.id,
+        });
+    }
+
+    private getOrderableAssetType(entity: EntityWithAssets): Type<OrderableAsset> {
+        const assetRelation = this.connection
+            .getRepository(entity.constructor)
+            .metadata.relations.find(r => r.propertyName === 'assets');
+        if (!assetRelation || typeof assetRelation.type === 'string') {
+            throw new InternalServerError('error.could-not-find-matching-orderable-asset');
+        }
+        return assetRelation.type as Type<OrderableAsset>;
+    }
+
+    private getHostEntityIdProperty(entity: EntityWithAssets): string {
+        const entityName = entity.constructor.name;
+        switch (entityName) {
+            case 'Product':
+                return 'productId';
+            case 'ProductVariant':
+                return 'productVariantId';
+            case 'Collection':
+                return 'collectionId';
+            default:
+                throw new InternalServerError('error.could-not-find-matching-orderable-asset');
+        }
+    }
 }

packages/core/src/service/services/collection.service.ts

@@ -35,7 +35,6 @@ import { EventBus } from '../../event-bus/event-bus';
 import { CatalogModificationEvent } from '../../event-bus/events/catalog-modification-event';
 import { CollectionModificationEvent } from '../../event-bus/events/collection-modification-event';
 import { WorkerService } from '../../worker/worker.service';
-import { AssetUpdater } from '../helpers/asset-updater/asset-updater';
 import { ListQueryBuilder } from '../helpers/list-query-builder/list-query-builder';
 import { TranslatableSaver } from '../helpers/translatable-saver/translatable-saver';
 import { getEntityOrThrow } from '../helpers/utils/get-entity-or-throw';
@@ -43,6 +42,7 @@ import { moveToIndex } from '../helpers/utils/move-to-index';
 import { translateDeep } from '../helpers/utils/translate-entity';
 import { ApplyCollectionFiltersMessage } from '../types/collection-messages';
 
+import { AssetService } from './asset.service';
 import { ChannelService } from './channel.service';
 import { FacetValueService } from './facet-value.service';
 import { JobService } from './job.service';
@@ -57,7 +57,7 @@ export class CollectionService implements OnModuleInit {
     constructor(
         @InjectConnection() private connection: Connection,
         private channelService: ChannelService,
-        private assetUpdater: AssetUpdater,
+        private assetService: AssetService,
         private facetValueService: FacetValueService,
         private listQueryBuilder: ListQueryBuilder,
         private translatableSaver: TranslatableSaver,
@@ -67,13 +67,6 @@ export class CollectionService implements OnModuleInit {
     ) {}
 
     onModuleInit() {
-        /*this.eventBus.subscribe(CatalogModificationEvent, async event => {
-            const collections = await this.connection.getRepository(Collection).find({
-                relations: ['productVariants'],
-            });
-            this.applyCollectionFilters(event.ctx, collections);
-        });*/
-
         this.eventBus
             .ofType(CatalogModificationEvent)
             .pipe(debounceTime(50))
@@ -257,9 +250,10 @@ export class CollectionService implements OnModuleInit {
                 }
                 coll.position = await this.getNextPositionInParent(ctx, input.parentId || undefined);
                 coll.filters = this.getCollectionFiltersFromInput(input);
-                await this.assetUpdater.updateEntityAssets(coll, input);
+                await this.assetService.updateFeaturedAsset(coll, input.featuredAssetId);
             },
         });
+        await this.assetService.updateEntityAssets(collection, input.assetIds);
         this.applyCollectionFilters(ctx, [collection]);
         return assertFound(this.findOne(ctx, collection.id));
     }
@@ -273,7 +267,8 @@ export class CollectionService implements OnModuleInit {
                 if (input.filters) {
                     coll.filters = this.getCollectionFiltersFromInput(input);
                 }
-                await this.assetUpdater.updateEntityAssets(coll, input);
+                await this.assetService.updateFeaturedAsset(coll, input.featuredAssetId);
+                await this.assetService.updateEntityAssets(coll, input.assetIds);
             },
         });
         if (input.filters) {

packages/core/src/service/services/product-variant.service.ts

@@ -1,6 +1,11 @@
 import { Injectable } from '@nestjs/common';
 import { InjectConnection } from '@nestjs/typeorm';
-import { CreateProductVariantInput, DeletionResponse, DeletionResult, UpdateProductVariantInput } from '@vendure/common/lib/generated-types';
+import {
+    CreateProductVariantInput,
+    DeletionResponse,
+    DeletionResult,
+    UpdateProductVariantInput,
+} from '@vendure/common/lib/generated-types';
 import { ID, PaginatedList } from '@vendure/common/lib/shared-types';
 import { generateAllCombinations } from '@vendure/common/lib/shared-utils';
 import { Connection } from 'typeorm';
@@ -12,7 +17,7 @@ import { ListQueryOptions } from '../../common/types/common-types';
 import { Translated } from '../../common/types/locale-types';
 import { assertFound, idsAreEqual } from '../../common/utils';
 import { ConfigService } from '../../config/config.service';
-import { Asset, ProductOptionGroup, ProductVariantPrice, TaxCategory } from '../../entity';
+import { ProductOptionGroup, ProductVariantPrice, TaxCategory } from '../../entity';
 import { FacetValue } from '../../entity/facet-value/facet-value.entity';
 import { ProductOption } from '../../entity/product-option/product-option.entity';
 import { ProductVariantTranslation } from '../../entity/product-variant/product-variant-translation.entity';
@@ -20,7 +25,6 @@ import { ProductVariant } from '../../entity/product-variant/product-variant.ent
 import { Product } from '../../entity/product/product.entity';
 import { EventBus } from '../../event-bus/event-bus';
 import { CatalogModificationEvent } from '../../event-bus/events/catalog-modification-event';
-import { AssetUpdater } from '../helpers/asset-updater/asset-updater';
 import { ListQueryBuilder } from '../helpers/list-query-builder/list-query-builder';
 import { TaxCalculator } from '../helpers/tax-calculator/tax-calculator';
 import { TranslatableSaver } from '../helpers/translatable-saver/translatable-saver';
@@ -28,6 +32,7 @@ import { getEntityOrThrow } from '../helpers/utils/get-entity-or-throw';
 import { samplesEach } from '../helpers/utils/samples-each';
 import { translateDeep } from '../helpers/utils/translate-entity';
 
+import { AssetService } from './asset.service';
 import { FacetValueService } from './facet-value.service';
 import { GlobalSettingsService } from './global-settings.service';
 import { StockMovementService } from './stock-movement.service';
@@ -44,7 +49,7 @@ export class ProductVariantService {
         private facetValueService: FacetValueService,
         private taxRateService: TaxRateService,
         private taxCalculator: TaxCalculator,
-        private assetUpdater: AssetUpdater,
+        private assetService: AssetService,
         private zoneService: ZoneService,
         private translatableSaver: TranslatableSaver,
         private eventBus: EventBus,
@@ -132,20 +137,6 @@ export class ProductVariantService {
             .then(variant => (!variant ? [] : variant.options.map(o => translateDeep(o, ctx.languageCode))));
     }
 
-    async getAssetsForVariant(ctx: RequestContext, variantId: ID): Promise<Asset[]> {
-        const variant = await getEntityOrThrow(this.connection, ProductVariant, variantId, {
-            relations: ['assets'],
-        });
-        return variant.assets;
-    }
-
-    async getFeaturedAssetForVariant(ctx: RequestContext, variantId: ID): Promise<Asset> {
-        const variant = await getEntityOrThrow(this.connection, ProductVariant, variantId, {
-            relations: ['featuredAsset'],
-        });
-        return variant.featuredAsset;
-    }
-
     getFacetValuesForVariant(ctx: RequestContext, variantId: ID): Promise<Array<Translated<FacetValue>>> {
         return this.connection
             .getRepository(ProductVariant)
@@ -185,13 +176,14 @@ export class ProductVariantService {
                 }
                 variant.product = { id: input.productId } as any;
                 variant.taxCategory = { id: input.taxCategoryId } as any;
-                await this.assetUpdater.updateEntityAssets(variant, input);
+                await this.assetService.updateFeaturedAsset(variant, input.featuredAssetId);
             },
             typeOrmSubscriberData: {
                 channelId: ctx.channelId,
                 taxCategoryId: input.taxCategoryId,
             },
         });
+        await this.assetService.updateEntityAssets(createdVariant, input.assetIds);
         if (input.stockOnHand != null && input.stockOnHand !== 0) {
             await this.stockMovementService.adjustProductVariantStock(
                 createdVariant.id,
@@ -235,7 +227,8 @@ export class ProductVariantService {
                         input.stockOnHand,
                     );
                 }
-                await this.assetUpdater.updateEntityAssets(updatedVariant, input);
+                await this.assetService.updateFeaturedAsset(updatedVariant, input.featuredAssetId);
+                await this.assetService.updateEntityAssets(updatedVariant, input.assetIds);
             },
             typeOrmSubscriberData: {
                 channelId: ctx.channelId,
@@ -405,7 +398,7 @@ export class ProductVariantService {
 
     private sortJoin<T>(arr: T[], glue: string, prop?: keyof T): string {
         return arr
-            .map(x => prop ? x[prop] : x)
+            .map(x => (prop ? x[prop] : x))
             .sort()
             .join(glue);
     }

packages/core/src/service/services/product.service.ts

@@ -3,7 +3,7 @@ import { InjectConnection } from '@nestjs/typeorm';
 import {
     CreateProductInput,
     DeletionResponse,
-    DeletionResult, LanguageCode,
+    DeletionResult,
     UpdateProductInput,
 } from '@vendure/common/lib/generated-types';
 import { normalizeString } from '@vendure/common/lib/normalize-string';
@@ -20,12 +20,12 @@ import { ProductTranslation } from '../../entity/product/product-translation.ent
 import { Product } from '../../entity/product/product.entity';
 import { EventBus } from '../../event-bus/event-bus';
 import { CatalogModificationEvent } from '../../event-bus/events/catalog-modification-event';
-import { AssetUpdater } from '../helpers/asset-updater/asset-updater';
 import { ListQueryBuilder } from '../helpers/list-query-builder/list-query-builder';
 import { TranslatableSaver } from '../helpers/translatable-saver/translatable-saver';
 import { getEntityOrThrow } from '../helpers/utils/get-entity-or-throw';
 import { translateDeep } from '../helpers/utils/translate-entity';
 
+import { AssetService } from './asset.service';
 import { ChannelService } from './channel.service';
 import { CollectionService } from './collection.service';
 import { FacetValueService } from './facet-value.service';
@@ -34,18 +34,12 @@ import { TaxRateService } from './tax-rate.service';
 
 @Injectable()
 export class ProductService {
-    private readonly relations = [
-        'featuredAsset',
-        'assets',
-        'channels',
-        'facetValues',
-        'facetValues.facet',
-    ];
+    private readonly relations = ['featuredAsset', 'assets', 'channels', 'facetValues', 'facetValues.facet'];
 
     constructor(
         @InjectConnection() private connection: Connection,
         private channelService: ChannelService,
-        private assetUpdater: AssetUpdater,
+        private assetService: AssetService,
         private productVariantService: ProductVariantService,
         private facetValueService: FacetValueService,
         private taxRateService: TaxRateService,
@@ -68,10 +62,7 @@ export class ProductService {
             .getManyAndCount()
             .then(async ([products, totalItems]) => {
                 const items = products.map(product =>
-                    translateDeep(product, ctx.languageCode, [
-                        'facetValues',
-                        ['facetValues', 'facet'],
-                    ]),
+                    translateDeep(product, ctx.languageCode, ['facetValues', ['facetValues', 'facet']]),
                 );
                 return {
                     items,
@@ -90,10 +81,7 @@ export class ProductService {
         if (!product) {
             return;
         }
-        return translateDeep(product, ctx.languageCode, [
-            'facetValues',
-            ['facetValues', 'facet'],
-        ]);
+        return translateDeep(product, ctx.languageCode, ['facetValues', ['facetValues', 'facet']]);
     }
 
     async findOneBySlug(ctx: RequestContext, slug: string): Promise<Translated<Product> | undefined> {
@@ -120,9 +108,10 @@ export class ProductService {
                 if (input.facetValueIds) {
                     p.facetValues = await this.facetValueService.findByIds(input.facetValueIds);
                 }
-                await this.assetUpdater.updateEntityAssets(p, input);
+                await this.assetService.updateFeaturedAsset(p, input.featuredAssetId);
             },
         });
+        await this.assetService.updateEntityAssets(product, input.assetIds);
         this.eventBus.publish(new CatalogModificationEvent(ctx, product));
         return assertFound(this.findOne(ctx, product.id));
     }
@@ -138,7 +127,8 @@ export class ProductService {
                 if (input.facetValueIds) {
                     p.facetValues = await this.facetValueService.findByIds(input.facetValueIds);
                 }
-                await this.assetUpdater.updateEntityAssets(p, input);
+                await this.assetService.updateFeaturedAsset(p, input.featuredAssetId);
+                await this.assetService.updateEntityAssets(p, input.assetIds);
             },
         });
         this.eventBus.publish(new CatalogModificationEvent(ctx, product));
@@ -201,7 +191,10 @@ export class ProductService {
     private async getProductWithOptionGroups(productId: ID): Promise<Product> {
         const product = await this.connection
             .getRepository(Product)
-            .findOne(productId, { relations: ['optionGroups', 'variants', 'variants.options'], where: { deletedAt: null } });
+            .findOne(productId, {
+                relations: ['optionGroups', 'variants', 'variants.options'],
+                where: { deletedAt: null },
+            });
         if (!product) {
             throw new EntityNotFoundError('Product', productId);
         }
@@ -220,9 +213,13 @@ export class ProductService {
                     let suffix = 1;
                     const alreadySuffixed = /-\d+$/;
                     do {
-                        const qb = this.connection.getRepository(ProductTranslation).createQueryBuilder('translation')
+                        const qb = this.connection
+                            .getRepository(ProductTranslation)
+                            .createQueryBuilder('translation')
                             .where(`translation.slug = :slug`, { slug: t.slug })
-                            .andWhere(`translation.languageCode = :languageCode`, { languageCode: t.languageCode });
+                            .andWhere(`translation.languageCode = :languageCode`, {
+                                languageCode: t.languageCode,
+                            });
                         if ((input as UpdateProductInput).id) {
                             qb.andWhere(`translation.base != :id`, { id: (input as UpdateProductInput).id });
                         }