Browse Source

fix(core): Correctly prefix asset urls for resolved properties

Closes #146
Michael Bromley 6 years ago
parent
commit
0517b6c5fa

+ 14 - 0
packages/core/e2e/__snapshots__/import.e2e-spec.ts.snap

@@ -7,10 +7,14 @@ Array [
       Object {
         "id": "T_1",
         "name": "pps1.jpg",
+        "preview": "test-url/test-assets/pps1__preview.jpg",
+        "source": "test-url/test-assets/pps1.jpg",
       },
       Object {
         "id": "T_2",
         "name": "pps2.jpg",
+        "preview": "test-url/test-assets/pps2__preview.jpg",
+        "source": "test-url/test-assets/pps2.jpg",
       },
     ],
     "description": "A great device for stretching paper.",
@@ -18,6 +22,8 @@ Array [
     "featuredAsset": Object {
       "id": "T_1",
       "name": "pps1.jpg",
+      "preview": "test-url/test-assets/pps1__preview.jpg",
+      "source": "test-url/test-assets/pps1.jpg",
     },
     "id": "T_1",
     "name": "Perfect Paper Stretcher",
@@ -244,6 +250,8 @@ Array [
           Object {
             "id": "T_3",
             "name": "box-of-8.jpg",
+            "preview": "test-url/test-assets/box-of-8__preview.jpg",
+            "source": "test-url/test-assets/box-of-8.jpg",
           },
         ],
         "facetValues": Array [
@@ -260,6 +268,8 @@ Array [
         "featuredAsset": Object {
           "id": "T_3",
           "name": "box-of-8.jpg",
+          "preview": "test-url/test-assets/box-of-8__preview.jpg",
+          "source": "test-url/test-assets/box-of-8.jpg",
         },
         "id": "T_5",
         "name": "Giotto Mega Pencils Box of 8",
@@ -286,6 +296,8 @@ Array [
           Object {
             "id": "T_4",
             "name": "box-of-12.jpg",
+            "preview": "test-url/test-assets/box-of-12__preview.jpg",
+            "source": "test-url/test-assets/box-of-12.jpg",
           },
         ],
         "facetValues": Array [
@@ -302,6 +314,8 @@ Array [
         "featuredAsset": Object {
           "id": "T_4",
           "name": "box-of-12.jpg",
+          "preview": "test-url/test-assets/box-of-12__preview.jpg",
+          "source": "test-url/test-assets/box-of-12.jpg",
         },
         "id": "T_6",
         "name": "Giotto Mega Pencils Box of 12",

+ 9 - 1
packages/core/e2e/import.e2e-spec.ts

@@ -57,10 +57,14 @@ describe('Import resolver', () => {
                             featuredAsset {
                                 id
                                 name
+                                preview
+                                source
                             }
                             assets {
                                 id
                                 name
+                                preview
+                                source
                             }
                             optionGroups {
                                 id
@@ -91,10 +95,14 @@ describe('Import resolver', () => {
                                 assets {
                                     id
                                     name
+                                    preview
+                                    source
                                 }
                                 featuredAsset {
                                     id
                                     name
+                                    preview
+                                    source
                                 }
                                 facetValues {
                                     id
@@ -109,7 +117,7 @@ describe('Import resolver', () => {
                                 trackInventory
                                 stockMovements {
                                     items {
-                                        ...on StockMovement {
+                                        ... on StockMovement {
                                             id
                                             type
                                             quantity

+ 0 - 5
packages/core/src/api/api.module.ts

@@ -8,7 +8,6 @@ import { ServiceModule } from '../service/service.module';
 import { AdminApiModule, ApiSharedModule, ShopApiModule } from './api-internal-modules';
 import { RequestContextService } from './common/request-context.service';
 import { configureGraphQLModule } from './config/configure-graphql-module';
-import { AssetInterceptor } from './middleware/asset-interceptor';
 import { AuthGuard } from './middleware/auth-guard';
 import { IdInterceptor } from './middleware/id-interceptor';
 import { ValidateCustomFieldsInterceptor } from './middleware/validate-custom-fields-interceptor';
@@ -48,10 +47,6 @@ import { ValidateCustomFieldsInterceptor } from './middleware/validate-custom-fi
             provide: APP_GUARD,
             useClass: AuthGuard,
         },
-        {
-            provide: APP_INTERCEPTOR,
-            useClass: AssetInterceptor,
-        },
         {
             provide: APP_INTERCEPTOR,
             useClass: IdInterceptor,

+ 6 - 1
packages/core/src/api/config/configure-graphql-module.ts

@@ -16,6 +16,7 @@ import { getDynamicGraphQlModulesForPlugins } from '../../plugin/dynamic-plugin-
 import { getPluginAPIExtensions } from '../../plugin/plugin-metadata';
 import { ApiSharedModule } from '../api-internal-modules';
 import { IdCodecService } from '../common/id-codec.service';
+import { AssetInterceptorPlugin } from '../middleware/asset-interceptor-plugin';
 import { IdCodecPlugin } from '../middleware/id-codec-plugin';
 import { TranslateErrorsPlugin } from '../middleware/translate-errors-plugin';
 
@@ -141,7 +142,11 @@ async function createGraphQLOptions(
         context: (req: any) => req,
         // This is handled by the Express cors plugin
         cors: false,
-        plugins: [new IdCodecPlugin(idCodecService), new TranslateErrorsPlugin(i18nService)],
+        plugins: [
+            new IdCodecPlugin(idCodecService),
+            new TranslateErrorsPlugin(i18nService),
+            new AssetInterceptorPlugin(configService),
+        ],
     } as GqlModuleOptions;
 
     /**

+ 77 - 0
packages/core/src/api/middleware/asset-interceptor-plugin.ts

@@ -0,0 +1,77 @@
+import { Asset } from '@vendure/common/lib/generated-types';
+import { ApolloServerPlugin, GraphQLRequestListener, GraphQLServiceContext } from 'apollo-server-plugin-base';
+import { DocumentNode } from 'graphql';
+
+import { AssetStorageStrategy } from '../../config/asset-storage-strategy/asset-storage-strategy';
+import { ConfigService } from '../../config/config.service';
+import { GraphqlValueTransformer } from '../common/graphql-value-transformer';
+
+/**
+ * Transforms outputs so that any Asset instances are run through the {@link AssetStorageStrategy.toAbsoluteUrl}
+ * method before being returned in the response.
+ */
+export class AssetInterceptorPlugin implements ApolloServerPlugin {
+    private graphqlValueTransformer: GraphqlValueTransformer;
+    private readonly toAbsoluteUrl: AssetStorageStrategy['toAbsoluteUrl'] | undefined;
+
+    constructor(private configService: ConfigService) {
+        const { assetOptions } = this.configService;
+        if (assetOptions.assetStorageStrategy.toAbsoluteUrl) {
+            this.toAbsoluteUrl = assetOptions.assetStorageStrategy.toAbsoluteUrl.bind(
+                assetOptions.assetStorageStrategy,
+            );
+        }
+    }
+
+    serverWillStart(service: GraphQLServiceContext): Promise<void> | void {
+        this.graphqlValueTransformer = new GraphqlValueTransformer(service.schema);
+    }
+
+    requestDidStart(): GraphQLRequestListener {
+        return {
+            willSendResponse: requestContext => {
+                const { document } = requestContext;
+                if (document) {
+                    const data = requestContext.response.data;
+                    const req = requestContext.context.req;
+                    if (data) {
+                        this.prefixAssetUrls(req, document, data);
+                    }
+                }
+            },
+        };
+    }
+
+    private prefixAssetUrls(request: any, document: DocumentNode, data: Record<string, any>) {
+        const typeTree = this.graphqlValueTransformer.getOutputTypeTree(document);
+        const toAbsoluteUrl = this.toAbsoluteUrl;
+        if (!toAbsoluteUrl) {
+            return;
+        }
+        this.graphqlValueTransformer.transformValues(typeTree, data, (value, type) => {
+            const isAssetType = type && type.name === 'Asset';
+            if (isAssetType) {
+                if (value && !Array.isArray(value)) {
+                    if (value.preview) {
+                        value.preview = toAbsoluteUrl(request, value.preview);
+                    }
+                    if (value.source) {
+                        value.source = toAbsoluteUrl(request, value.source);
+                    }
+                }
+            }
+            const isSearchResultType = type && type.name === 'SearchResult';
+            if (isSearchResultType) {
+                if (value && !Array.isArray(value)) {
+                    if (value.productPreview) {
+                        value.productPreview = toAbsoluteUrl(request, value.productPreview);
+                    }
+                    if (value.productVariantPreview) {
+                        value.productVariantPreview = toAbsoluteUrl(request, value.productVariantPreview);
+                    }
+                }
+            }
+            return value;
+        });
+    }
+}

+ 0 - 202
packages/core/src/api/middleware/asset-interceptor.spec.ts

@@ -1,202 +0,0 @@
-import { CallHandler } from '@nestjs/common';
-import { ExecutionContextHost } from '@nestjs/core/helpers/execution-context-host';
-import { of } from 'rxjs';
-
-import { MockConfigService } from '../../config/config.service.mock';
-import { Asset } from '../../entity/asset/asset.entity';
-
-import { AssetInterceptor } from './asset-interceptor';
-
-describe('AssetInterceptor', () => {
-    function testInterceptor<T>(
-        response: T,
-        assertFn: (response: T, result: T, toAbsoluteUrlFn: jest.Mock) => void,
-    ) {
-        return (done: jest.DoneCallback) => {
-            const toAbsoluteUrl = jest.fn().mockReturnValue('visited');
-            const configService = new MockConfigService();
-            configService.assetOptions.assetStorageStrategy = { toAbsoluteUrl };
-            const interceptor = new AssetInterceptor(configService as any);
-            const executionContext = new ExecutionContextHost([0, 0, { req: {} }]);
-            const next: CallHandler = { handle: () => of(response) };
-
-            interceptor.intercept(executionContext, next).subscribe(result => {
-                assertFn(response, result, toAbsoluteUrl);
-                done();
-            });
-        };
-    }
-
-    function mockAsset() {
-        return new Asset({ preview: 'original', source: 'original' });
-    }
-
-    it(
-        'passes through responses without Assets',
-        testInterceptor(
-            {
-                foo: 1,
-                bar: {
-                    baz: false,
-                },
-            },
-            (response, result, toAbsoluteUrl) => {
-                expect(result).toBe(response);
-                expect(toAbsoluteUrl).toHaveBeenCalledTimes(0);
-            },
-        ),
-    );
-
-    it(
-        'handles an Asset entity directly',
-        testInterceptor(
-            mockAsset(),
-            (response, result, toAbsoluteUrl) => {
-                expect(result.source).toBe('visited');
-                expect(result.preview).toBe('visited');
-                expect(toAbsoluteUrl).toHaveBeenCalledTimes(2);
-            },
-        ),
-    );
-
-    it(
-        'visits a top-level Asset',
-        testInterceptor(
-            {
-                foo: 1,
-                bar: mockAsset(),
-            },
-            (response, result, toAbsoluteUrl) => {
-                expect(result.bar.source).toBe('visited');
-                expect(result.bar.preview).toBe('visited');
-                expect(toAbsoluteUrl).toHaveBeenCalledTimes(2);
-            },
-        ),
-    );
-
-    it(
-        'visits a top-level array of Assets',
-        testInterceptor(
-            {
-                foo: 1,
-                bar: [mockAsset(), mockAsset()],
-            },
-            (response, result, toAbsoluteUrl) => {
-                expect(result.bar[0].source).toBe('visited');
-                expect(result.bar[0].preview).toBe('visited');
-                expect(result.bar[1].source).toBe('visited');
-                expect(result.bar[1].preview).toBe('visited');
-                expect(toAbsoluteUrl).toHaveBeenCalledTimes(4);
-            },
-        ),
-    );
-
-    it(
-        'visits a nested Asset',
-        testInterceptor(
-            {
-                foo: 1,
-                bar: [
-                    {
-                        baz: {
-                            quux: mockAsset(),
-                        },
-                    },
-                ],
-            },
-            (response, result, toAbsoluteUrl) => {
-                expect(result.bar[0].baz.quux.source).toBe('visited');
-                expect(result.bar[0].baz.quux.preview).toBe('visited');
-                expect(toAbsoluteUrl).toHaveBeenCalledTimes(2);
-            },
-        ),
-    );
-
-    it(
-        'handles null values',
-        testInterceptor(
-            {
-                foo: 1,
-                bar: null,
-            },
-            (response, result, toAbsoluteUrl) => {
-                expect(result).toEqual({ foo: 1, bar: null });
-                expect(toAbsoluteUrl).toHaveBeenCalledTimes(0);
-            },
-        ),
-    );
-
-    it(
-        'handles undefined values',
-        testInterceptor(
-            {
-                foo: 1,
-                bar: undefined,
-            },
-            (response, result, toAbsoluteUrl) => {
-                expect(result).toEqual({ foo: 1, bar: undefined });
-                expect(toAbsoluteUrl).toHaveBeenCalledTimes(0);
-            },
-        ),
-    );
-
-    it(
-        'handles productPreview property',
-        testInterceptor(
-            {
-                items: [
-                    {
-                        productPreview: 'image.jpg',
-                    },
-                ],
-            },
-            (response, result, toAbsoluteUrl) => {
-                expect(result).toEqual({ items: [{ productPreview: 'visited' }] });
-                expect(toAbsoluteUrl).toHaveBeenCalledTimes(1);
-            },
-        ),
-    );
-
-    it(
-        'handles productVariantPreview property',
-        testInterceptor(
-            {
-                items: [
-                    {
-                        productVariantPreview: 'image.jpg',
-                    },
-                ],
-            },
-            (response, result, toAbsoluteUrl) => {
-                expect(result).toEqual({ items: [{ productVariantPreview: 'visited' }] });
-                expect(toAbsoluteUrl).toHaveBeenCalledTimes(1);
-            },
-        ),
-    );
-
-    describe('cyclic objects', () => {
-        const fido: any = {
-            name: 'fido',
-            avatar: mockAsset(),
-        };
-        const person = {
-            name: 'joe',
-            pet: fido,
-        };
-        fido.owner = person;
-
-        it(
-               'handles objects with cycles',
-               testInterceptor(
-                   {
-                       result: person,
-                   },
-                   (response, result, toAbsoluteUrl) => {
-                       expect(result.result.pet.avatar).toEqual({ source: 'visited', preview: 'visited' });
-                       expect(toAbsoluteUrl).toHaveBeenCalledTimes(2);
-                   },
-               ),
-           );
-    });
-
-});

+ 0 - 87
packages/core/src/api/middleware/asset-interceptor.ts

@@ -1,87 +0,0 @@
-import { CallHandler, ExecutionContext, Injectable, NestInterceptor } from '@nestjs/common';
-import { Type } from '@vendure/common/lib/shared-types';
-import { Observable } from 'rxjs';
-import { map } from 'rxjs/operators';
-
-import { AssetStorageStrategy } from '../../config/asset-storage-strategy/asset-storage-strategy';
-import { ConfigService } from '../../config/config.service';
-import { Asset } from '../../entity/asset/asset.entity';
-import { parseContext } from '../common/parse-context';
-
-/**
- * Transforms outputs so that any Asset instances are run through the {@link AssetStorageStrategy.toAbsoluteUrl}
- * method before being returned in the response.
- */
-@Injectable()
-export class AssetInterceptor implements NestInterceptor {
-    private readonly toAbsoluteUrl: AssetStorageStrategy['toAbsoluteUrl'] | undefined;
-
-    constructor(private configService: ConfigService) {
-        const { assetOptions } = this.configService;
-        if (assetOptions.assetStorageStrategy.toAbsoluteUrl) {
-            this.toAbsoluteUrl = assetOptions.assetStorageStrategy.toAbsoluteUrl.bind(
-                assetOptions.assetStorageStrategy,
-            );
-        }
-    }
-
-    intercept<T = any>(context: ExecutionContext, next: CallHandler<T>): Observable<T> {
-        const toAbsoluteUrl = this.toAbsoluteUrl;
-        if (toAbsoluteUrl === undefined) {
-            return next.handle();
-        }
-        const { req } = parseContext(context);
-        return next.handle().pipe(
-            map(data => {
-                if (data instanceof Asset) {
-                    data.preview = toAbsoluteUrl(req, data.preview);
-                    data.source = toAbsoluteUrl(req, data.source);
-                } else {
-                    visitType(data, [Asset, 'productPreview', 'productVariantPreview'], asset => {
-                        if (asset instanceof Asset) {
-                            asset.preview = toAbsoluteUrl(req, asset.preview);
-                            asset.source = toAbsoluteUrl(req, asset.source);
-                        } else {
-                            asset = toAbsoluteUrl(req, asset);
-                        }
-                        return asset;
-                    });
-                }
-                return data;
-            }),
-        );
-    }
-}
-
-/**
- * Traverses the object and when encountering a property with a value which
- * is an instance of class T, invokes the visitor function on that value.
- */
-function visitType<T>(
-    obj: any,
-    types: Array<Type<T> | string>,
-    visit: (instance: T | string) => T | string,
-    seen: Set<any> = new Set(),
-) {
-    const keys = Object.keys(obj || {});
-    for (const key of keys) {
-        const value = obj[key];
-
-        for (const type of types) {
-            if (typeof type === 'string') {
-                if (type === key) {
-                    obj[key] = visit(value);
-                }
-            } else if (value instanceof type) {
-                visit(value);
-            }
-        }
-        if (typeof value === 'object' && !seen.has(value)) {
-            // add this object to the set of "seen" objects,
-            // which prevents us getting stuck in the case of a
-            // cyclic graph.
-            seen.add(value);
-            visitType(value, types, visit, seen);
-        }
-    }
-}