Sfoglia il codice sorgente

feat(core): Improve Collection tree data structure

This commit uses the "closure table" method of representing the tree of Collections.
Prior versions of TypeORM only had partial support, but the current version fixes that
so we can move away from the less efficient "adjacency list" approach and take
advantage of the built-in TypeORM tree structure support.

BREAKING CHANGE: The data structure used to represent the tree of Collections has changed,
which will require a DB migration.
Michael Bromley 3 anni fa
parent
commit
5e7af0d898

+ 43 - 0
packages/core/src/connection/transactional-connection.ts

@@ -8,8 +8,10 @@ import {
     FindOneOptions,
     FindOptionsUtils,
     getRepository,
+    getTreeRepository,
     ObjectType,
     Repository,
+    TreeRepository,
 } from 'typeorm';
 
 import { RequestContext } from '../api/common/request-context';
@@ -85,6 +87,47 @@ export class TransactionalConnection {
         }
     }
 
+    /**
+     * @description
+     * Returns a TypeORM tree repository. Note that when no RequestContext is supplied, the repository will not
+     * be aware of any existing transaction. Therefore calling this method without supplying a RequestContext
+     * is discouraged without a deliberate reason.
+     *
+     * @since 2.0.0
+     */
+    getTreeRepository<Entity>(
+        target: ObjectType<Entity> | EntitySchema<Entity> | string,
+    ): TreeRepository<Entity>;
+    /**
+     * @description
+     * Returns a TypeORM tree repository which is bound to any existing transactions. It is recommended to _always_ pass
+     * the RequestContext argument when possible, otherwise the queries will be executed outside of any
+     * ongoing transactions which have been started by the {@link Transaction} decorator.
+     *
+     * @since 2.0.0
+     */
+    getTreeRepository<Entity>(
+        ctx: RequestContext | undefined,
+        target: ObjectType<Entity> | EntitySchema<Entity> | string,
+    ): TreeRepository<Entity>;
+    getTreeRepository<Entity>(
+        ctxOrTarget: RequestContext | ObjectType<Entity> | EntitySchema<Entity> | string | undefined,
+        maybeTarget?: ObjectType<Entity> | EntitySchema<Entity> | string,
+    ): TreeRepository<Entity> {
+        if (ctxOrTarget instanceof RequestContext) {
+            const transactionManager = this.getTransactionManager(ctxOrTarget);
+            if (transactionManager && maybeTarget && !transactionManager.queryRunner?.isReleased) {
+                return transactionManager.getTreeRepository(maybeTarget);
+            } else {
+                // tslint:disable-next-line:no-non-null-assertion
+                return getTreeRepository(maybeTarget!);
+            }
+        } else {
+            // tslint:disable-next-line:no-non-null-assertion
+            return getTreeRepository(ctxOrTarget ?? maybeTarget!);
+        }
+    }
+
     /**
      * @description
      * Allows database operations to be wrapped in a transaction, ensuring that in the event of an error being

+ 12 - 13
packages/core/src/entity/collection/collection.entity.ts

@@ -7,6 +7,7 @@ import {
     ManyToMany,
     ManyToOne,
     OneToMany,
+    Tree,
     TreeChildren,
     TreeParent,
 } from 'typeorm';
@@ -30,13 +31,11 @@ import { CollectionTranslation } from './collection-translation.entity';
  * @docsCategory entities
  */
 @Entity()
-// TODO: It would be ideal to use the TypeORM built-in tree support, but unfortunately it is incomplete
-// at this time - does not support moving or deleting. See https://github.com/typeorm/typeorm/issues/2032
-// Therefore we will just use an adjacency list which will have a perf impact when needing to lookup
-// decendants or ancestors more than 1 level removed.
-// @Tree('closure-table')
-export class Collection extends VendureEntity
-    implements Translatable, HasCustomFields, ChannelAware, Orderable {
+@Tree('closure-table')
+export class Collection
+    extends VendureEntity
+    implements Translatable, HasCustomFields, ChannelAware, Orderable
+{
     constructor(input?: DeepPartial<Collection>) {
         super(input);
     }
@@ -56,22 +55,22 @@ export class Collection extends VendureEntity
 
     slug: LocaleString;
 
-    @OneToMany((type) => CollectionTranslation, (translation) => translation.base, { eager: true })
+    @OneToMany(type => CollectionTranslation, translation => translation.base, { eager: true })
     translations: Array<Translation<Collection>>;
 
-    @ManyToOne((type) => Asset, { onDelete: 'SET NULL' })
+    @ManyToOne(type => Asset, { onDelete: 'SET NULL' })
     featuredAsset: Asset;
 
-    @OneToMany((type) => CollectionAsset, (collectionAsset) => collectionAsset.collection)
+    @OneToMany(type => CollectionAsset, collectionAsset => collectionAsset.collection)
     assets: CollectionAsset[];
 
     @Column('simple-json') filters: ConfigurableOperation[];
 
-    @ManyToMany((type) => ProductVariant, (productVariant) => productVariant.collections)
+    @ManyToMany(type => ProductVariant, productVariant => productVariant.collections)
     @JoinTable()
     productVariants: ProductVariant[];
 
-    @Column((type) => CustomCollectionFields)
+    @Column(type => CustomCollectionFields)
     customFields: CustomCollectionFields;
 
     @TreeChildren()
@@ -80,7 +79,7 @@ export class Collection extends VendureEntity
     @TreeParent()
     parent: Collection;
 
-    @ManyToMany((type) => Channel)
+    @ManyToMany(type => Channel)
     @JoinTable()
     channels: Channel[];
 }

+ 9 - 31
packages/core/src/service/services/collection.service.ts

@@ -245,14 +245,14 @@ export class CollectionService implements OnModuleInit {
     async getBreadcrumbs(
         ctx: RequestContext,
         collection: Collection,
-    ): Promise<Array<{ name: string; id: ID }>> {
+    ): Promise<Array<{ name: string; id: ID; slug: string }>> {
         const rootCollection = await this.getRootCollection(ctx);
         if (idsAreEqual(collection.id, rootCollection.id)) {
             return [pick(rootCollection, ['id', 'name', 'slug'])];
         }
         const pickProps = pick(['id', 'name', 'slug']);
         const ancestors = await this.getAncestors(collection.id, ctx);
-        return [pickProps(rootCollection), ...ancestors.map(pickProps).reverse(), pickProps(collection)];
+        return ancestors.map(pickProps).reverse();
     }
 
     /**
@@ -319,36 +319,14 @@ export class CollectionService implements OnModuleInit {
         collectionId: ID,
         ctx?: RequestContext,
     ): Promise<Array<Translated<Collection> | Collection>> {
-        const getParent = async (id: ID, _ancestors: Collection[] = []): Promise<Collection[]> => {
-            const parent = await this.connection
-                .getRepository(ctx, Collection)
-                .createQueryBuilder()
-                .relation(Collection, 'parent')
-                .of(id)
-                .loadOne();
-            if (parent) {
-                if (!parent.isRoot) {
-                    _ancestors.push(parent);
-                    return getParent(parent.id, _ancestors);
-                }
-            }
-            return _ancestors;
-        };
-        const ancestors = await getParent(collectionId);
-
         return this.connection
-            .getRepository(Collection)
-            .findByIds(ancestors.map(c => c.id))
-            .then(categories => {
-                const resultCategories: Array<Collection | Translated<Collection>> = [];
-                ancestors.forEach(a => {
-                    const category = categories.find(c => c.id === a.id);
-                    if (category) {
-                        resultCategories.push(ctx ? translateDeep(category, ctx.languageCode) : category);
-                    }
-                });
-                return resultCategories;
-            });
+            .getTreeRepository(ctx, Collection)
+            .findAncestors(new Collection({ id: collectionId }), ctx ? { relations: ['translations'] } : {})
+            .then(collections =>
+                collections.map(collection =>
+                    ctx ? translateDeep(collection, ctx.languageCode) : collection,
+                ),
+            );
     }
 
     async create(ctx: RequestContext, input: CreateCollectionInput): Promise<Translated<Collection>> {

+ 3 - 30
yarn.lock

@@ -9870,20 +9870,13 @@ ieee754@^1.1.13, ieee754@^1.1.4, ieee754@^1.2.1:
   resolved "https://registry.npmjs.org/ieee754/-/ieee754-1.2.1.tgz#8eb7a10a63fff25d15a57b001586d177d1b0d352"
   integrity sha512-dcyqhDvX1C46lXZcVqCpK+FtMRQVdIMN6/Df5js2zouUsqG7I6sFxitIC+7KYK29KdXOLHdu9zL4sFnoVQnqaA==
 
-ignore-walk@^3.0.1, ignore-walk@^3.0.3:
+ignore-walk@^3.0.1:
   version "3.0.4"
   resolved "https://registry.npmjs.org/ignore-walk/-/ignore-walk-3.0.4.tgz#c9a09f69b7c7b479a5d74ac1a3c0d4236d2a6335"
   integrity sha512-PY6Ii8o1jMRA1z4F2hRkH/xN59ox43DavKvD3oDpfurRlOJyAHpifIwpbdv1n4jt4ov0jSpw3kQ4GhJnpBL6WQ==
   dependencies:
     minimatch "^3.0.4"
 
-ignore-walk@^4.0.1:
-  version "4.0.1"
-  resolved "https://registry.npmjs.org/ignore-walk/-/ignore-walk-4.0.1.tgz#fc840e8346cf88a3a9380c5b17933cd8f4d39fa3"
-  integrity sha512-rzDQLaW4jQbh2YrOFlJdCtX8qgJTehFRYiUB2r1osqTeDzV/3+Jh8fz1oAPzUThf3iku8Ds4IDqawI5d8mUiQw==
-  dependencies:
-    minimatch "^3.0.4"
-
 ignore@^5.1.9, ignore@^5.2.0:
   version "5.2.0"
   resolved "https://registry.npmjs.org/ignore/-/ignore-5.2.0.tgz#6d3bac8fa7fe0d45d9f9be7bac2fc279577e345a"
@@ -13505,7 +13498,7 @@ npm-package-arg@8.1.5, npm-package-arg@^8.0.0, npm-package-arg@^8.0.1, npm-packa
     semver "^7.3.4"
     validate-npm-package-name "^3.0.0"
 
-npm-packlist@^1.1.6:
+npm-packlist@1.1.12, npm-packlist@^1.1.6, npm-packlist@^2.1.4, npm-packlist@^3.0.0:
   version "1.1.12"
   resolved "https://registry.npmjs.org/npm-packlist/-/npm-packlist-1.1.12.tgz#22bde2ebc12e72ca482abd67afc51eb49377243a"
   integrity sha512-WJKFOVMeAlsU/pjXuqVdzU0WfgtIBCupkEVwn+1Y0ERAbUfWw8R4GjgVbaKnUjRoD2FoQbHOCbOyT5Mbs9Lw4g==
@@ -13513,26 +13506,6 @@ npm-packlist@^1.1.6:
     ignore-walk "^3.0.1"
     npm-bundled "^1.0.1"
 
-npm-packlist@^2.1.4:
-  version "2.2.2"
-  resolved "https://registry.npmjs.org/npm-packlist/-/npm-packlist-2.2.2.tgz#076b97293fa620f632833186a7a8f65aaa6148c8"
-  integrity sha512-Jt01acDvJRhJGthnUJVF/w6gumWOZxO7IkpY/lsX9//zqQgnF7OJaxgQXcerd4uQOLu7W5bkb4mChL9mdfm+Zg==
-  dependencies:
-    glob "^7.1.6"
-    ignore-walk "^3.0.3"
-    npm-bundled "^1.1.1"
-    npm-normalize-package-bin "^1.0.1"
-
-npm-packlist@^3.0.0:
-  version "3.0.0"
-  resolved "https://registry.npmjs.org/npm-packlist/-/npm-packlist-3.0.0.tgz#0370df5cfc2fcc8f79b8f42b37798dd9ee32c2a9"
-  integrity sha512-L/cbzmutAwII5glUcf2DBRNY/d0TFd4e/FnaZigJV6JD85RHZXJFGwCndjMWiiViiWSsWt3tiOLpI3ByTnIdFQ==
-  dependencies:
-    glob "^7.1.6"
-    ignore-walk "^4.0.1"
-    npm-bundled "^1.1.1"
-    npm-normalize-package-bin "^1.0.1"
-
 npm-pick-manifest@6.1.1, npm-pick-manifest@^6.0.0, npm-pick-manifest@^6.1.1:
   version "6.1.1"
   resolved "https://registry.npmjs.org/npm-pick-manifest/-/npm-pick-manifest-6.1.1.tgz#7b5484ca2c908565f43b7f27644f36bb816f5148"
@@ -16020,7 +15993,7 @@ run-parallel@^1.1.9:
   dependencies:
     queue-microtask "^1.2.2"
 
-rxjs@6.6.7, rxjs@^6.3.3, rxjs@^6.5.1, rxjs@^6.5.2, rxjs@^6.5.3, rxjs@^6.6.0, rxjs@^6.6.3, rxjs@^6.6.6:
+rxjs@6.6.7, rxjs@^6.3.3, rxjs@^6.5.1, rxjs@^6.5.2, rxjs@^6.5.3, rxjs@^6.6.0, rxjs@^6.6.3:
   version "6.6.7"
   resolved "https://registry.npmjs.org/rxjs/-/rxjs-6.6.7.tgz#90ac018acabf491bf65044235d5863c4dab804c9"
   integrity sha512-hTdwr+7yYNIT5n4AMYp85KA6yw2Va0FLa3Rguvbpa4W3I5xynaBZo41cM3XM+4Q6fRMj3sBYIR1VAmZMXYJvRQ==