Merge branch 'master' into minor

packages/admin-ui/src/lib/catalog/src/components/product-assets/product-assets.component.html

@@ -44,22 +44,17 @@
 </ng-template>
 
 <ng-template #assetList>
-    <div class="all-assets" [class.compact]="compact" cdkDropListGroup #dlg>
+    <div class="all-assets" [class.compact]="compact" cdkDropListGroup>
         <div
+            *ngFor="let asset of assets; let index = index"
+            class="drop-list"
             cdkDropList
-            #dl
+            cdkDropListOrientation="horizontal"
+            [cdkDropListData]="index"
             [cdkDropListDisabled]="!(updatePermissions | hasPermission)"
-            [cdkDropListEnterPredicate]="dropListEnterPredicate"
-            (cdkDropListDropped)="dropListDropped()"
-        ></div>
-        <div
-            *ngFor="let asset of assets"
-            cdkDropList
-            [cdkDropListDisabled]="!(updatePermissions | hasPermission)"
-            [cdkDropListEnterPredicate]="dropListEnterPredicate"
-            (cdkDropListDropped)="dropListDropped()"
+            (cdkDropListDropped)="dropListDropped($event)"
         >
-            <vdr-dropdown cdkDrag (cdkDragMoved)="dragMoved($event)">
+            <vdr-dropdown cdkDrag>
                 <div
                     class="asset-thumb"
                     vdrDropdownTrigger

packages/admin-ui/src/lib/catalog/src/components/product-assets/product-assets.component.scss

@@ -38,6 +38,10 @@
     display: flex;
     flex-wrap: wrap;
 
+    .drop-list {
+        min-width: 60px;
+    }
+
     .asset-thumb {
         margin: 3px;
         padding: 0;
@@ -54,6 +58,9 @@
     }
 
     &.compact {
+        .drop-list {
+            min-width: 54px;
+        }
         .asset-thumb {
             margin: 1px;
             border-width: 1px;
@@ -61,31 +68,6 @@
     }
 }
 
-.cdk-drag-preview {
-    display: flex;
-    align-items: center;
-    justify-content: center;
-    width: 50px;
-    background-color: var(--color-component-bg-100);
-    opacity: 0.9;
-    box-shadow: 0 5px 5px -3px rgba(0, 0, 0, 0.2),
-    0 8px 10px 1px rgba(0, 0, 0, 0.14),
-    0 3px 14px 2px rgba(0, 0, 0, 0.12);
-}
-
-.cdk-drag-placeholder {
-    opacity: 0.8;
-    width: 60px;
-    height: 50px;
-    .asset-thumb {
-        background-color: var(--color-component-bg-300);
-        height: 100%;
-        width: 54px;
-    }
-    img {
-        display: none;
-    }
-}
 .all-assets.compact .cdk-drag-placeholder {
     width: 50px;
     .asset-thumb {
@@ -104,3 +86,7 @@
 .all-assets.cdk-drop-list-dragging vdr-dropdown:not(.cdk-drag-placeholder) {
     transition: transform 250ms cubic-bezier(0, 0, 0.2, 1);
 }
+
+.cdk-drop-list-dragging > *:not(.cdk-drag-placeholder) {
+  display: none;
+}

packages/admin-ui/src/lib/catalog/src/components/product-assets/product-assets.component.ts

@@ -1,7 +1,6 @@
-import { CdkDrag, CdkDragMove, CdkDropList, CdkDropListGroup, moveItemInArray } from '@angular/cdk/drag-drop';
+import { CdkDragDrop, moveItemInArray } from '@angular/cdk/drag-drop';
 import { ViewportRuler } from '@angular/cdk/overlay';
 import {
-    AfterViewInit,
     ChangeDetectionStrategy,
     ChangeDetectorRef,
     Component,
@@ -10,7 +9,6 @@ import {
     Input,
     Optional,
     Output,
-    ViewChild,
 } from '@angular/core';
 import {
     Asset,
@@ -41,25 +39,18 @@ export interface AssetChange {
     styleUrls: ['./product-assets.component.scss'],
     changeDetection: ChangeDetectionStrategy.OnPush,
 })
-export class ProductAssetsComponent implements AfterViewInit {
+export class ProductAssetsComponent {
     @Input('assets') set assetsSetter(val: Asset[]) {
         // create a new non-readonly array of assets
         this.assets = val.slice();
     }
+
     @Input() featuredAsset: Asset | undefined;
     @HostBinding('class.compact')
     @Input()
     compact = false;
     @Output() change = new EventEmitter<AssetChange>();
-    @ViewChild('dlg', { static: false, read: CdkDropListGroup }) listGroup: CdkDropListGroup<CdkDropList>;
-    @ViewChild('dl', { static: false, read: CdkDropList }) placeholder: CdkDropList;
 
-    public target: CdkDropList | null;
-    public targetIndex: number;
-    public source: CdkDropList | null;
-    public sourceIndex: number;
-    public dragIndex: number;
-    public activeContainer;
     public assets: Asset[] = [];
 
     private readonly updateCollectionPermissions = [Permission.UpdateCatalog, Permission.UpdateCollection];
@@ -80,15 +71,6 @@ export class ProductAssetsComponent implements AfterViewInit {
         @Optional() private collectionDetailComponent?: CollectionDetailComponent,
     ) {}
 
-    ngAfterViewInit() {
-        const phElement = this.placeholder.element.nativeElement;
-
-        phElement.style.display = 'none';
-        if (phElement.parentElement) {
-            phElement.parentElement.removeChild(phElement);
-        }
-    }
-
     selectAssets() {
         this.modalService
             .fromComponent(AssetPickerDialogComponent, {
@@ -140,114 +122,8 @@ export class ProductAssetsComponent implements AfterViewInit {
         });
     }
 
-    dragMoved(e: CdkDragMove) {
-        const point = this.getPointerPositionOnPage(e.event);
-
-        this.listGroup._items.forEach(dropList => {
-            if (__isInsideDropListClientRect(dropList, point.x, point.y)) {
-                this.activeContainer = dropList;
-                return;
-            }
-        });
-    }
-
-    dropListDropped() {
-        if (!this.target || !this.source) {
-            return;
-        }
-
-        const phElement = this.placeholder.element.nativeElement;
-        // tslint:disable-next-line:no-non-null-assertion
-        const parent = phElement.parentElement!;
-
-        phElement.style.display = 'none';
-
-        parent.removeChild(phElement);
-        parent.appendChild(phElement);
-        parent.insertBefore(this.source.element.nativeElement, parent.children[this.sourceIndex]);
-
-        this.target = null;
-        this.source = null;
-
-        if (this.sourceIndex !== this.targetIndex) {
-            moveItemInArray(this.assets, this.sourceIndex, this.targetIndex);
-            this.emitChangeEvent(this.assets, this.featuredAsset);
-        }
-    }
-
-    dropListEnterPredicate = (drag: CdkDrag, drop: CdkDropList) => {
-        if (drop === this.placeholder) {
-            return true;
-        }
-        if (drop !== this.activeContainer) {
-            return false;
-        }
-
-        const phElement = this.placeholder.element.nativeElement;
-        const sourceElement = drag.dropContainer.element.nativeElement;
-        const dropElement = drop.element.nativeElement;
-        const children = dropElement.parentElement && dropElement.parentElement.children;
-
-        const dragIndex = __indexOf(children, this.source ? phElement : sourceElement);
-        const dropIndex = __indexOf(children, dropElement);
-
-        if (!this.source) {
-            this.sourceIndex = dragIndex;
-            this.source = drag.dropContainer;
-
-            phElement.style.width = sourceElement.clientWidth + 'px';
-            phElement.style.height = sourceElement.clientHeight + 'px';
-
-            if (sourceElement.parentElement) {
-                sourceElement.parentElement.removeChild(sourceElement);
-            }
-        }
-
-        this.targetIndex = dropIndex;
-        this.target = drop;
-
-        phElement.style.display = '';
-        if (dropElement.parentElement) {
-            dropElement.parentElement.insertBefore(
-                phElement,
-                dropIndex > dragIndex ? dropElement.nextSibling : dropElement,
-            );
-        }
-
-        this.placeholder._dropListRef.enter(
-            drag._dragRef,
-            drag.element.nativeElement.offsetLeft,
-            drag.element.nativeElement.offsetTop,
-        );
-        return false;
-    };
-
-    /** Determines the point of the page that was touched by the user. */
-    getPointerPositionOnPage(event: MouseEvent | TouchEvent) {
-        // `touches` will be empty for start/end events so we have to fall back to `changedTouches`.
-        const point = __isTouchEvent(event) ? event.touches[0] || event.changedTouches[0] : event;
-        const scrollPosition = this.viewportRuler.getViewportScrollPosition();
-
-        return {
-            x: point.pageX - scrollPosition.left,
-            y: point.pageY - scrollPosition.top,
-        };
-    }
-}
-
-function __indexOf(collection: HTMLCollection | null, node: HTMLElement) {
-    if (!collection) {
-        return -1;
+    dropListDropped(event: CdkDragDrop<number>) {
+        moveItemInArray(this.assets, event.previousContainer.data, event.container.data);
+        this.emitChangeEvent(this.assets, this.featuredAsset);
     }
-    return Array.prototype.indexOf.call(collection, node);
-}
-
-/** Determines whether an event is a touch event. */
-function __isTouchEvent(event: MouseEvent | TouchEvent): event is TouchEvent {
-    return event.type.startsWith('touch');
-}
-
-function __isInsideDropListClientRect(dropList: CdkDropList, x: number, y: number) {
-    const { top, bottom, left, right } = dropList.element.nativeElement.getBoundingClientRect();
-    return y >= top && y <= bottom && x >= left && x <= right;
 }

packages/admin-ui/src/lib/core/src/data/utils/transform-relation-custom-field-inputs.spec.ts

@@ -88,4 +88,66 @@ describe('transformRelationCustomFieldInput()', () => {
             },
         } as any);
     });
+
+    it('transforms input object', () => {
+        const config: CustomFieldConfig[] = [
+            { name: 'weight', type: 'int', list: false },
+            { name: 'avatar', type: 'relation', list: false, entity: 'Asset' },
+        ];
+        const entity = {
+            id: 1,
+            name: 'test',
+            customFields: {
+                weight: 500,
+                avatar: {
+                    id: 123,
+                    preview: '...',
+                },
+            },
+        };
+
+        const result = transformRelationCustomFieldInputs({ input: entity }, config);
+        expect(result).toEqual({
+            input: {
+                id: 1,
+                name: 'test',
+                customFields: {
+                    weight: 500,
+                    avatarId: 123,
+                },
+            },
+        } as any);
+    });
+
+    it('transforms input array (as in UpdateProductVariantsInput)', () => {
+        const config: CustomFieldConfig[] = [
+            { name: 'weight', type: 'int', list: false },
+            { name: 'avatar', type: 'relation', list: false, entity: 'Asset' },
+        ];
+        const entity = {
+            id: 1,
+            name: 'test',
+            customFields: {
+                weight: 500,
+                avatar: {
+                    id: 123,
+                    preview: '...',
+                },
+            },
+        };
+
+        const result = transformRelationCustomFieldInputs({ input: [entity] }, config);
+        expect(result).toEqual({
+            input: [
+                {
+                    id: 1,
+                    name: 'test',
+                    customFields: {
+                        weight: 500,
+                        avatarId: 123,
+                    },
+                },
+            ],
+        } as any);
+    });
 });

packages/admin-ui/src/lib/core/src/data/utils/transform-relation-custom-field-inputs.ts

@@ -7,12 +7,17 @@ import { CustomFieldConfig } from '../../common/generated-types';
  * Transforms any custom field "relation" type inputs into the corresponding `<name>Id` format,
  * as expected by the server.
  */
-export function transformRelationCustomFieldInputs<T extends { input?: any } & Record<string, any> = any>(
-    variables: T,
-    customFieldConfig: CustomFieldConfig[],
-): T {
+export function transformRelationCustomFieldInputs<
+    T extends { input?: Record<string, any> | Array<Record<string, any>> } & Record<string, any> = any
+>(variables: T, customFieldConfig: CustomFieldConfig[]): T {
     if (variables.input) {
-        transformRelations(variables.input, customFieldConfig);
+        if (Array.isArray(variables.input)) {
+            for (const item of variables.input) {
+                transformRelations(item, customFieldConfig);
+            }
+        } else {
+            transformRelations(variables.input, customFieldConfig);
+        }
     }
     return transformRelations(variables, customFieldConfig);
 }
@@ -22,7 +27,7 @@ export function transformRelationCustomFieldInputs<T extends { input?: any } & R
  * When persisting custom fields, we need to send just the IDs of the relations,
  * rather than the objects themselves.
  */
-function transformRelations(input: any, customFieldConfig: CustomFieldConfig[]) {
+function transformRelations<T>(input: T, customFieldConfig: CustomFieldConfig[]) {
     for (const field of customFieldConfig) {
         if (field.type === 'relation') {
             if (hasCustomFields(input)) {

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

@@ -3,6 +3,7 @@ import { omit } from '@vendure/common/lib/omit';
 import { pick } from '@vendure/common/lib/pick';
 import { mergeConfig } from '@vendure/core';
 import { createTestEnvironment } from '@vendure/testing';
+import fs from 'fs-extra';
 import path from 'path';
 
 import { initialData } from '../../../e2e-common/e2e-initial-data';
@@ -326,6 +327,40 @@ describe('Asset resolver', () => {
                 },
             ]);
         });
+
+        // https://github.com/vendure-ecommerce/vendure/issues/990
+        it('errors if the filesize is too large', async () => {
+            /**
+             * Based on https://stackoverflow.com/a/49433633/772859
+             */
+            function createEmptyFileOfSize(fileName: string, sizeInBytes: number) {
+                return new Promise((resolve, reject) => {
+                    const fh = fs.openSync(fileName, 'w');
+                    fs.writeSync(fh, 'ok', Math.max(0, sizeInBytes - 2));
+                    fs.closeSync(fh);
+                    resolve(true);
+                });
+            }
+
+            const twentyOneMib = 22020096;
+            const filename = path.join(__dirname, 'fixtures/assets/temp_large_file.pdf');
+            await createEmptyFileOfSize(filename, twentyOneMib);
+
+            try {
+                const { createAssets }: CreateAssets.Mutation = await adminClient.fileUploadMutation({
+                    mutation: CREATE_ASSETS,
+                    filePaths: [filename],
+                    mapVariables: filePaths => ({
+                        input: filePaths.map(p => ({ file: null })),
+                    }),
+                });
+                fail('Should have thrown');
+            } catch (e) {
+                expect(e.message).toContain('File truncated as it exceeds the 20971520 byte size limit');
+            } finally {
+                fs.rmSync(filename);
+            }
+        });
     });
 
     describe('filter by tags', () => {

packages/core/src/plugin/default-job-queue-plugin/sql-job-queue-strategy.ts

@@ -86,18 +86,24 @@ export class SqlJobQueueStrategy extends PollingJobQueueStrategy implements Insp
 
         return new Promise(async (resolve, reject) => {
             if (isSQLite) {
-                // SQLite driver does not support concurrent transactions. See https://github.com/typeorm/typeorm/issues/1884
-                const result = await this.getNextAndSetAsRunning(connection.manager, queueName, false);
-                resolve(result);
+                try {
+                    // SQLite driver does not support concurrent transactions. See https://github.com/typeorm/typeorm/issues/1884
+                    const result = await this.getNextAndSetAsRunning(connection.manager, queueName, false);
+                    resolve(result);
+                } catch (e) {
+                    reject(e);
+                }
             } else {
                 // Selecting the next job is wrapped in a transaction so that we can
                 // set a lock on that row and immediately update the status to "RUNNING".
                 // This prevents multiple worker processes from taking the same job when
                 // running concurrent workers.
-                connection.transaction(async transactionManager => {
-                    const result = await this.getNextAndSetAsRunning(transactionManager, queueName, true);
-                    resolve(result);
-                });
+                connection
+                    .transaction(async transactionManager => {
+                        const result = await this.getNextAndSetAsRunning(transactionManager, queueName, true);
+                        resolve(result);
+                    })
+                    .catch(err => reject(err));
             }
         });
     }

packages/core/src/service/initializer.service.ts

@@ -1,10 +1,15 @@
 import { Injectable } from '@nestjs/common';
 
+import { Logger } from '../config/logger/vendure-logger';
+import { Administrator } from '../entity/administrator/administrator.entity';
+
 import { AdministratorService } from './services/administrator.service';
 import { ChannelService } from './services/channel.service';
 import { GlobalSettingsService } from './services/global-settings.service';
 import { RoleService } from './services/role.service';
 import { ShippingMethodService } from './services/shipping-method.service';
+import { ZoneService } from './services/zone.service';
+import { TransactionalConnection } from './transaction/transactional-connection';
 
 /**
  * Only used internally to run the various service init methods in the correct
@@ -13,6 +18,8 @@ import { ShippingMethodService } from './services/shipping-method.service';
 @Injectable()
 export class InitializerService {
     constructor(
+        private connection: TransactionalConnection,
+        private zoneService: ZoneService,
         private channelService: ChannelService,
         private roleService: RoleService,
         private administratorService: AdministratorService,
@@ -21,16 +28,45 @@ export class InitializerService {
     ) {}
 
     async onModuleInit() {
+        await this.awaitDbSchemaGeneration();
         // IMPORTANT - why manually invoke these init methods rather than just relying on
         // Nest's "onModuleInit" lifecycle hook within each individual service class?
-        // The reason is that the order of invokation matters. By explicitly invoking the
+        // The reason is that the order of invocation matters. By explicitly invoking the
         // methods below, we can e.g. guarantee that the default channel exists
         // (channelService.initChannels()) before we try to create any roles (which assume that
         // there is a default Channel to work with.
+        await this.zoneService.initZones();
         await this.globalSettingsService.initGlobalSettings();
         await this.channelService.initChannels();
         await this.roleService.initRoles();
         await this.administratorService.initAdministrators();
         await this.shippingMethodService.initShippingMethods();
     }
+
+    /**
+     * On the first run of the server & worker, when dbConnectionOptions.synchronize = true, there can be
+     * a race condition where the worker starts up before the server process has had a chance to generate
+     * the DB schema. This results in a fatal error as the worker is not able to run its initialization
+     * tasks which interact with the DB.
+     *
+     * This method applies retry logic to give the server time to populate the schema before the worker
+     * continues with its bootstrap process.
+     */
+    private async awaitDbSchemaGeneration() {
+        const retries = 20;
+        const delayMs = 100;
+        for (let attempt = 0; attempt < retries; attempt++) {
+            try {
+                const result = await this.connection.getRepository(Administrator).find();
+                return;
+            } catch (e) {
+                if (attempt < retries - 1) {
+                    Logger.warn(`Awaiting DB schema creation... (attempt ${attempt})`);
+                    await new Promise(resolve => setTimeout(resolve, delayMs));
+                } else {
+                    Logger.error(`Timed out when awaiting the DB schema to be ready!`, undefined, e.stack);
+                }
+            }
+        }
+    }
 }

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

@@ -235,20 +235,32 @@ export class AssetService {
      * Create an Asset based on a file uploaded via the GraphQL API.
      */
     async create(ctx: RequestContext, input: CreateAssetInput): Promise<CreateAssetResult> {
-        const { createReadStream, filename, mimetype } = await input.file;
-        const stream = createReadStream();
-        const result = await this.createAssetInternal(ctx, stream, filename, mimetype, input.customFields);
-        if (isGraphQlErrorResult(result)) {
-            return result;
-        }
-        await this.customFieldRelationService.updateRelations(ctx, Asset, input, result);
-        if (input.tags) {
-            const tags = await this.tagService.valuesToTags(ctx, input.tags);
-            result.tags = tags;
-            await this.connection.getRepository(ctx, Asset).save(result);
-        }
-        this.eventBus.publish(new AssetEvent(ctx, result, 'created'));
-        return result;
+        return new Promise(async (resolve, reject) => {
+            const { createReadStream, filename, mimetype } = await input.file;
+            const stream = createReadStream();
+            stream.on('error', (err: any) => {
+                reject(err);
+            });
+            const result = await this.createAssetInternal(
+                ctx,
+                stream,
+                filename,
+                mimetype,
+                input.customFields,
+            );
+            if (isGraphQlErrorResult(result)) {
+                resolve(result);
+                return;
+            }
+            await this.customFieldRelationService.updateRelations(ctx, Asset, input, result);
+            if (input.tags) {
+                const tags = await this.tagService.valuesToTags(ctx, input.tags);
+                result.tags = tags;
+                await this.connection.getRepository(ctx, Asset).save(result);
+            }
+            this.eventBus.publish(new AssetEvent(ctx, result, 'created'));
+            resolve(result);
+        });
     }
 
     async update(ctx: RequestContext, input: UpdateAssetInput): Promise<Asset> {

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

@@ -1,4 +1,4 @@
-import { Injectable, OnModuleInit } from '@nestjs/common';
+import { Injectable } from '@nestjs/common';
 import {
     CreateZoneInput,
     DeletionResponse,
@@ -20,14 +20,14 @@ import { translateDeep } from '../helpers/utils/translate-entity';
 import { TransactionalConnection } from '../transaction/transactional-connection';
 
 @Injectable()
-export class ZoneService implements OnModuleInit {
+export class ZoneService {
     /**
      * We cache all Zones to avoid hitting the DB many times per request.
      */
     private zones: Zone[] = [];
     constructor(private connection: TransactionalConnection) {}
 
-    onModuleInit() {
+    initZones() {
         return this.updateZonesCache();
     }