Skip to content

Commit

Permalink
feat(core): Improve Collection tree data structure
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@michaelbromley
michaelbromley committed Mar 3, 2022
1 parent 9d9275c commit 5e7af0d
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 74 deletions.
43 changes: 43 additions & 0 deletions packages/core/src/connection/transactional-connection.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@ import {
FindOneOptions,
FindOptionsUtils,
getRepository,
getTreeRepository,
ObjectType,
Repository,
TreeRepository,
} from 'typeorm';

import { RequestContext } from '../api/common/request-context';
Expand Down Expand Up @@ -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
Expand Down
25 changes: 12 additions & 13 deletions packages/core/src/entity/collection/collection.entity.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
ManyToMany,
ManyToOne,
OneToMany,
Tree,
TreeChildren,
TreeParent,
} from 'typeorm';
Expand All @@ -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);
}
Expand All @@ -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()
Expand All @@ -80,7 +79,7 @@ export class Collection extends VendureEntity
@TreeParent()
parent: Collection;

@ManyToMany((type) => Channel)
@ManyToMany(type => Channel)
@JoinTable()
channels: Channel[];
}
40 changes: 9 additions & 31 deletions packages/core/src/service/services/collection.service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

/**
Expand Down Expand Up @@ -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>> {
Expand Down
33 changes: 3 additions & 30 deletions yarn.lock
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -13505,34 +13498,14 @@ [email protected], 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==
dependencies:
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"

[email protected], 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"
Expand Down Expand Up @@ -16020,7 +15993,7 @@ run-parallel@^1.1.9:
dependencies:
queue-microtask "^1.2.2"

[email protected], 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:
[email protected], 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==
Expand Down

0 comments on commit 5e7af0d

Please sign in to comment.