github-mirror/n8n
1
0
Fork 0
mirror of https://github.com/n8n-io/n8n.git synced 2026-10-22 20:41:50 +02:00
Code Issues Actions 14 Packages Projects Releases Wiki Activity Graph

refactor(core): Compose repositories instead of extending BaseRepository (no-changelog) (#41842)

Browse Source 
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Andreas Fitzek 2026-10-22 16:28:51 +00:00 committed by GitHub
parent a58472ea4d
commit c9774aae07
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
17 changed files with 242 additions and 140 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
AGENTS.md
View File

@ -253,18 +253,36 @@ baseline. A new business-logic import fails CI.
repository methods you call. `run` **requires** a context — pass an empty `{}`
at the operation entry point, and reuse the one you were handed everywhere
below it (a context that already carries a transaction is joined, not nested).
Repositories extend `BaseRepository` and resolve the right `EntityManager` with
`this.managerFor(ctx)`; the `Transaction` handle is opaque and never exposes a
driver type to business logic. See `oauth-token.service.ts` +
`oauth-*-token.repository.ts` for a worked example.
The `Transaction` handle is opaque and never exposes a driver type to
business logic. See `oauth-token.service.ts` for a worked service example
(its repositories predate the composition rule below; do not copy their shape).
- **Repositories compose, they don't extend.** A repository is the boundary
that keeps TypeORM out of business logic. If it extended `BaseRepository`,
every service that injects it would also get `find`, `save`, `delete`,
`createQueryBuilder` and `.manager` for free, and TypeORM would leak through
inferred types that no lint rule can see. So a repository is a plain
`@Service()` class that holds a **file-private `BaseRepository` subclass (the
table)** as a private field and resolves the manager with
`this.table.managerFor(ctx)`. The table is never exported or registered with
DI. Every data access is a named use-case method on the repository that
calls the table internally; there is no generic `find`/`save`/`delete`
unless you write it for one operation. See
`packages/cli/src/modules/inbound-auth-core/database/repositories/trusted-source.repository.ts`
for a worked example. The existing `extends BaseRepository` / `extends
Repository` repositories are legacy. Do not copy them, and do not convert them
in unrelated PRs. This rule is not yet lint-enforced.
- **Anti-patterns reviewers reject** — they hide the dependency instead of
removing it:
- String-matching TypeORM errors, e.g. `error.name === 'QueryFailedError'`.
- Relabeling the import from `@n8n/typeorm` to `@n8n/db` to silence the rule
(`@n8n/db` re-exports several operators/types, but this relabels the
dependency rather than removing it).
- Pushing `.manager` / `createQueryBuilder` into business logic to avoid an
operator import — trades a visible leak for an invisible one.
- Pushing `.manager` / `createQueryBuilder` / `repo.managerFor(ctx)` into
business logic to avoid an operator import — trades a visible leak for an
invisible one.
- A repository that `extends BaseRepository` / `extends Repository`, or that
exposes its table (public property, getter, or a `find(options)`
passthrough) — the inherited TypeORM surface must stay private to the table.
### ESLint configuration layers

7
packages/@n8n/db/AGENTS.md
View File

@ -16,6 +16,13 @@ type its parameters and return value in **domain terms** — don't surface
add generic `find(options)` passthroughs. This keeps operators and query
construction inside the persistence layer where they belong.
Repositories in this package follow the same rule as everywhere else: compose a
file-private `BaseRepository` subclass (the table), do not extend it. Otherwise
every injecting service inherits the full TypeORM surface and the boundary is
gone. `src/repositories/deployment-key.repository.ts` is the in-package example;
the root AGENTS.md "Repositories compose, they don't extend" bullet has the
rules. Do not add new `extends BaseRepository` repositories.
## Transactions
Both transaction primitives live here: the deprecated `withTransaction` helper

21
packages/@n8n/db/src/repositories/base-repository.ts
View File

@ -6,12 +6,17 @@ import type { OperationContext, TransactionRunner } from '../services/transactio
import { TypeOrmTransaction } from '../services/typeorm-transaction';
/**
* Base class for repositories that participate in {@link TransactionRunner} transactions.
* Base class for the table behind a repository. It participates in {@link TransactionRunner}
* transactions and owns the single seam that turns an opaque {@link Transaction} back into a
* native `EntityManager`: {@link BaseRepository.managerFor}.
*
* It owns the single seam that turns an opaque {@link Transaction} back into a native
* `EntityManager`: {@link BaseRepository.managerFor}. That method is `protected`, so the
* unwrap is reachable only from repository subclasses — business logic can hold a
* `Transaction` but never resolve it to a driver handle.
* A repository does not extend this class. It declares a file-private subclass (the table),
* which is never exported or registered with DI, and holds it as a private field. Every data
* access is a named use-case method on the repository that calls the table internally. If the
* repository extended this class instead, every service that injects it would also get `find`,
* `save`, `delete`, `createQueryBuilder` and `.manager`, and TypeORM would leak through inferred
* types that no lint rule can see. See `trusted-source.repository.ts` in `packages/cli` for the
* pattern.
*/
export abstract class BaseRepository<Entity extends ObjectLiteral> extends Repository<Entity> {
constructor(
@ -26,8 +31,12 @@ export abstract class BaseRepository<Entity extends ObjectLiteral> extends Repos
* The manager a query should run against: the context's active transaction if present,
* otherwise this repository's default manager. `ctx` is always required — thread the one
* received from `TransactionRunner.run`; a non-transactional caller passes the root `{}`.
*
* Public, because `.manager` and {@link runInTransaction} already expose the manager. The
* boundary is not this modifier: it is that the table is a private field of its repository
* and business logic never holds a `BaseRepository`.
*/
protected managerFor(ctx: OperationContext): EntityManager {
managerFor(ctx: OperationContext): EntityManager {
const { trx } = ctx;
if (!trx) return this.manager;

11
packages/@n8n/db/src/repositories/deployment-key.repository.ts
View File

@ -10,7 +10,6 @@ import {
OAUTH_SIGNING_KEY_TYPE,
} from '../entities/deployment-key';
import { DbLock, DbLockService } from '../services/db-lock.service';
import type { OperationContext } from '../services/transaction';
import { TransactionRunner } from '../services/transaction';
/**
@ -36,10 +35,6 @@ class DeploymentKeyStore extends BaseRepository<DeploymentKey> {
constructor(dataSource: DataSource, transactionRunner: TransactionRunner) {
super(DeploymentKey, dataSource.manager, transactionRunner);
}
managerForContext(ctx: OperationContext) {
return this.managerFor(ctx);
}
}
@Service()
@ -124,7 +119,7 @@ export class DeploymentKeyRepository {
*/
async seedLegacyCbcKey(encryptedValue: string): Promise<void> {
await this.dbLockService.withLockContext(DbLock.DATA_ENCRYPTION_KEY_SEED, async (ctx) => {
const repo = this.store.managerForContext(ctx).getRepository(DeploymentKey);
const repo = this.store.managerFor(ctx).getRepository(DeploymentKey);
const existing = await repo.findOne({
where: { type: 'data_encryption', algorithm: 'aes-256-cbc' },
});
@ -255,7 +250,7 @@ export class DeploymentKeyRepository {
status: 'active',
});
return await this.transactionRunner.run({}, async (ctx) => {
const tx = this.store.managerForContext(ctx);
const tx = this.store.managerFor(ctx);
await tx.update(
DeploymentKey,
{ type: entity.type, status: 'active' },
@ -267,7 +262,7 @@ export class DeploymentKeyRepository {
async activateDataEncryptionKey(id: string): Promise<void> {
await this.transactionRunner.run({}, async (ctx) => {
const tx = this.store.managerForContext(ctx);
const tx = this.store.managerFor(ctx);
const target = await tx.findOne(DeploymentKey, {
where: { id, type: 'data_encryption' },
});

4
packages/@n8n/module-cli/src/templates/backend/README.md.template
View File

@ -22,4 +22,6 @@ service, repository, entity, configuration, and testing patterns.
- Keep the module entrypoint in the `./module` export. Do not export it from `src/index.ts`.
Importing the entrypoint runs the `@BackendModule` decorator.
- Use dynamic imports inside the module entrypoint. This keeps disabled modules unloaded.
- Keep TypeORM imports in entity and repository files.
- Keep TypeORM imports in entity and repository files. A repository composes a
file-private `BaseRepository` table and exposes only use-case methods. It does
not extend `BaseRepository`.

34
packages/cli/AGENTS.md
View File

@ -7,11 +7,13 @@ for repo-wide conventions.
TypeORM belongs in the **persistence layer**, not in business logic.
**Allowed to import `@n8n/typeorm`** — files that declare an `@Entity` class or a
repository class that extends TypeORM `Repository` or n8n `BaseRepository`.
Their folder and filename do not affect the rule. Tests and migration tooling use
Code Health exceptions. Helper-only persistence adapters need a narrow,
auditable entry in the `typeorm-persistence-boundary` rule.
**Allowed to import `@n8n/typeorm`** — files that declare an `@Entity` class or
a class that extends TypeORM `Repository` or n8n `BaseRepository`. Their folder
and filename do not affect the rule. The file-private table class of a
composition repository (see below) satisfies this check, so such a repository
needs no exception. Tests and migration tooling use Code Health exceptions.
Helper-only persistence adapters need a narrow, auditable entry in the
`typeorm-persistence-boundary` rule.
**Not allowed** — business logic (services, controllers, public-api handlers,
commands, factories) must not import `@n8n/typeorm` or `@n8n/typeorm/...`
@ -24,8 +26,26 @@ without decoupling anything. Existing leaks of both kinds are tracked in the
shrink-only Code Health baseline. Do not add new baseline entries.
Distinct from that shrink-only ratchet, `src/commands/db/revert.ts` is an exact
exception for CLI migration tooling. Composition-based repositories use exact
exceptions. Repository subclasses need no exception.
exception for CLI migration tooling. Adapters with no `Repository` or
`BaseRepository` subclass in the file at all (for example
`src/modules/data-table/data-table-rows.repository.ts`) also need an exact
`allowedFiles` entry in the rule. A repository with a file-private table class
needs none.
## Repositories
A repository is the boundary that keeps TypeORM out of business logic, so it
**composes** a `BaseRepository` and never extends one. Extending would hand
every injecting service `find`, `save`, `delete`, `createQueryBuilder` and
`.manager` for free, and TypeORM would leak through inferred types that no lint
rule can see. Declare a file-private subclass (the table), hold it as a private
field, and write every data access as a named use-case method that calls
`this.table.managerFor(ctx)` internally. The table is never exported or
registered with DI. See the root AGENTS.md section for the rules and
`src/modules/inbound-auth-core/database/repositories/trusted-source.repository.ts`
for a worked example. Most repositories in this package still extend
`BaseRepository` or `Repository`. They are legacy: do not copy them, and do not
convert them in unrelated PRs.
Need an operator query (`In`, `IsNull`, `FindOptionsWhere`, …)? Add a
use-case-named repository method (plain parameters, domain-shaped return) rather

2
packages/cli/src/modules/inbound-auth-core/__tests__/trusted-source.gate.test.ts
View File

@ -76,7 +76,7 @@ describe('TrustedSourceDbGate', () => {
describe('with a binding', () => {
it('allows an active binding to the same user', async () => {
expect(await gate.authorizeSealed({ userId: 'user-1', grant, binding })).toBe(true);
expect(identityRepository.findBinding).toHaveBeenCalledWith('source-1', 'idp-alice');
expect(identityRepository.findBinding).toHaveBeenCalledWith('source-1', 'idp-alice', {});
});
it.each(['suspended', 'revoked'] as const)('denies a %s binding', async (status) => {

23
packages/cli/src/modules/inbound-auth-core/database/repositories/trusted-source-identity.repository.ts
View File

@ -19,10 +19,6 @@ class TrustedSourceIdentityStore extends BaseRepository<TrustedSourceIdentityEnt
constructor(dataSource: DataSource, transactionRunner: TransactionRunner) {
super(TrustedSourceIdentityEntity, dataSource.manager, transactionRunner);
}
override managerFor(ctx: OperationContext) {
return super.managerFor(ctx);
}
}
export type InsertTrustedSourceIdentityRow = Pick<
@ -46,7 +42,7 @@ export class TrustedSourceIdentityRepository {
async findBinding(
sourceId: string,
subject: string,
ctx: OperationContext = {},
ctx: OperationContext,
): Promise<{ userId: string; status: TrustedSourceIdentityStatus } | null> {
return await this.store.managerFor(ctx).findOne(TrustedSourceIdentityEntity, {
select: ['userId', 'status'],
@ -54,7 +50,7 @@ export class TrustedSourceIdentityRepository {
});
}
async clearByTrustedSourceId(trustedSourceId: string, ctx: OperationContext = {}): Promise<void> {
async clearByTrustedSourceId(trustedSourceId: string, ctx: OperationContext): Promise<void> {
await this.store
.managerFor(ctx)
.delete(TrustedSourceIdentityEntity, { sourceId: trustedSourceId });
@ -64,7 +60,7 @@ export class TrustedSourceIdentityRepository {
async findBySubject(
sourceId: string,
subject: string,
ctx: OperationContext = {},
ctx: OperationContext,
): Promise<TrustedSourceIdentityEntity | null> {
return await this.store.managerFor(ctx).findOne(TrustedSourceIdentityEntity, {
where: { sourceId, subject },
@ -77,18 +73,18 @@ export class TrustedSourceIdentityRepository {
}
/** Reads inside the identity transaction must reuse its connection: the pool may hold one. */
async findUserByEmail(email: string, ctx: OperationContext = {}): Promise<User | null> {
async findUserByEmail(email: string, ctx: OperationContext): Promise<User | null> {
return await this.store
.managerFor(ctx)
.findOne(User, { where: { email }, relations: { role: true } });
}
async globalRoleExists(slug: string, ctx: OperationContext = {}): Promise<boolean> {
async globalRoleExists(slug: string, ctx: OperationContext): Promise<boolean> {
return await this.store.managerFor(ctx).exists(Role, { where: { slug, roleType: 'global' } });
}
/** True when every slug names a project role. Reads inside the identity transaction. */
async projectRolesExist(slugs: string[], ctx: OperationContext = {}): Promise<boolean> {
async projectRolesExist(slugs: string[], ctx: OperationContext): Promise<boolean> {
const unique = [...new Set(slugs)];
if (unique.length === 0) return true;
const count = await this.store
@ -101,7 +97,7 @@ export class TrustedSourceIdentityRepository {
sourceId: string,
subject: string,
seenAt: Date,
ctx: OperationContext = {},
ctx: OperationContext,
): Promise<void> {
await this.store
.managerFor(ctx)
@ -109,10 +105,7 @@ export class TrustedSourceIdentityRepository {
}
/** Writes the binding unless one exists for `(sourceId, subject)`; an existing row wins silently. */
async insertIfAbsent(
row: InsertTrustedSourceIdentityRow,
ctx: OperationContext = {},
): Promise<void> {
async insertIfAbsent(row: InsertTrustedSourceIdentityRow, ctx: OperationContext): Promise<void> {
await this.store
.managerFor(ctx)
.createQueryBuilder()

24
packages/cli/src/modules/inbound-auth-core/database/repositories/trusted-source.repository.ts
View File

@ -15,10 +15,6 @@ class TrustedSourceTable extends BaseRepository<TrustedSourceEntity> {
constructor(dataSource: DataSource, transactionRunner: TransactionRunner) {
super(TrustedSourceEntity, dataSource.manager, transactionRunner);
}
override managerFor(ctx: OperationContext) {
return super.managerFor(ctx);
}
}
export type NewTrustedSourceRow = Pick<
@ -79,15 +75,15 @@ export class TrustedSourceRepository {
this.isPostgres = config.type === 'postgresdb';
}
async findById(id: string, ctx: OperationContext = {}) {
async findById(id: string, ctx: OperationContext) {
return await this.table.managerFor(ctx).findOneBy(TrustedSourceEntity, { id });
}
async findByIssuer(issuer: string, ctx: OperationContext = {}) {
async findByIssuer(issuer: string, ctx: OperationContext) {
return await this.table.managerFor(ctx).findOneBy(TrustedSourceEntity, { issuer });
}
async findAll(ctx: OperationContext = {}) {
async findAll(ctx: OperationContext) {
return await this.table.managerFor(ctx).find(TrustedSourceEntity, { order: { name: 'ASC' } });
}
@ -98,7 +94,7 @@ export class TrustedSourceRepository {
*/
async findDue(
{ errorRetrySeconds, healthyRefreshSeconds, limit }: DueQuery,
ctx: OperationContext = {},
ctx: OperationContext,
): Promise<TrustedSourceEntity[]> {
const errorCutoff = dbNowPlusMsLiteral(this.isPostgres, -errorRetrySeconds * 1000);
const healthyCutoff = dbNowPlusMsLiteral(this.isPostgres, -healthyRefreshSeconds * 1000);
@ -119,7 +115,7 @@ export class TrustedSourceRepository {
}
/** `save`, not `insert`: the `@BeforeInsert` id generator only runs on an entity instance. */
async insertRow(row: NewTrustedSourceRow, ctx: OperationContext = {}) {
async insertRow(row: NewTrustedSourceRow, ctx: OperationContext) {
return await this.table.managerFor(ctx).save(TrustedSourceEntity, this.table.create(row));
}
@ -130,7 +126,7 @@ export class TrustedSourceRepository {
*/
async insertIfAbsent(
row: NewTrustedSourceRow & Pick<TrustedSourceEntity, 'id'>,
ctx: OperationContext = {},
ctx: OperationContext,
) {
await this.table
.managerFor(ctx)
@ -142,11 +138,11 @@ export class TrustedSourceRepository {
.execute();
}
async updateById(id: string, changes: TrustedSourceRowChanges, ctx: OperationContext = {}) {
async updateById(id: string, changes: TrustedSourceRowChanges, ctx: OperationContext) {
await this.table.managerFor(ctx).update(TrustedSourceEntity, { id }, changes);
}
async deleteById(id: string, ctx: OperationContext = {}) {
async deleteById(id: string, ctx: OperationContext) {
await this.table.managerFor(ctx).delete(TrustedSourceEntity, { id });
}
@ -155,7 +151,7 @@ export class TrustedSourceRepository {
id: string,
token: string,
leaseSeconds: number,
ctx: OperationContext = {},
ctx: OperationContext,
): Promise<boolean> {
const staleBefore = dbNowPlusMsLiteral(this.isPostgres, -leaseSeconds * 1000);
const result = await this.table
@ -179,7 +175,7 @@ export class TrustedSourceRepository {
id: string,
token: string,
changes: DiscoveryRowChanges,
ctx: OperationContext = {},
ctx: OperationContext,
): Promise<boolean> {
const result = await this.table
.managerFor(ctx)

2
packages/cli/src/modules/inbound-auth-core/trusted-source.gate.ts
View File

@ -37,7 +37,7 @@ export class TrustedSourceDbGate extends TrustedSourceGate {
}
if (binding) {
const row = await this.identityRepository.findBinding(binding.sourceId, binding.subject);
const row = await this.identityRepository.findBinding(binding.sourceId, binding.subject, {});
if (row?.status !== 'active' || row.userId !== userId) {
this.logger.debug('Sealed identity denied: the binding is not active for this user', {
userId,

119
packages/cli/src/modules/inbound-auth-core/trusted-source.store.ts
View File

@ -81,7 +81,7 @@ export class TrustedSourceDbStore extends TrustedSourceStore {
async getById(id: string): Promise<TrustedSource | undefined> {
return await this.cacheService.get(idKey(id), {
ttl: CACHE_TTL,
refreshFn: async () => await this.load(await this.trustedSourceRepository.findById(id)),
refreshFn: async () => await this.load(await this.trustedSourceRepository.findById(id, {})),
});
}
@ -89,7 +89,7 @@ export class TrustedSourceDbStore extends TrustedSourceStore {
return await this.cacheService.get(issuerKey(issuer), {
ttl: CACHE_TTL,
refreshFn: async () =>
await this.load(await this.trustedSourceRepository.findByIssuer(issuer)),
await this.load(await this.trustedSourceRepository.findByIssuer(issuer, {})),
});
}
@ -109,11 +109,14 @@ export class TrustedSourceDbStore extends TrustedSourceStore {
/** Sources due for discovery, most urgent first; not cached, the task reads it once a minute. */
async listDue(limit: number): Promise<TrustedSource[]> {
const rows = await this.trustedSourceRepository.findDue({
errorRetrySeconds: ERROR_RETRY_SECONDS,
healthyRefreshSeconds: HEALTHY_REFRESH_SECONDS,
limit,
});
const rows = await this.trustedSourceRepository.findDue(
{
errorRetrySeconds: ERROR_RETRY_SECONDS,
healthyRefreshSeconds: HEALTHY_REFRESH_SECONDS,
limit,
},
{},
);
const sources = await Promise.all(rows.map(async (row) => await this.load(row)));
return sources.filter((source): source is TrustedSource => source !== undefined);
}
@ -125,6 +128,7 @@ export class TrustedSourceDbStore extends TrustedSourceStore {
source.id,
token,
DISCOVERY_LEASE_SECONDS,
{},
);
if (!claimed) return null;
await this.invalidateCache(source);
@ -138,11 +142,16 @@ export class TrustedSourceDbStore extends TrustedSourceStore {
result: DiscoveryResult,
): Promise<boolean> {
// Metadata is never cleared: an error keeps the last good documents readable.
const recorded = await this.trustedSourceRepository.recordDiscovery(source.id, token, {
status: result.status,
lastError: result.lastError,
...(result.metadata && { metadata: JSON.stringify(result.metadata) }),
});
const recorded = await this.trustedSourceRepository.recordDiscovery(
source.id,
token,
{
status: result.status,
lastError: result.lastError,
...(result.metadata && { metadata: JSON.stringify(result.metadata) }),
},
{},
);
if (recorded) await this.invalidateCache(source);
return recorded;
}
@ -151,18 +160,21 @@ export class TrustedSourceDbStore extends TrustedSourceStore {
// Persist the parsed, migrated output, not the raw input: stored rows hold the latest version
// with defaults filled in, so a later default change must not alter them.
const config = migrateToLatest(trustedSourceConfigSchemaFor('admin').parse(input.config));
const row = await this.trustedSourceRepository.insertRow({
name: input.name,
issuer: input.issuer,
type: config.authentication.type,
managedBy: 'admin',
status: 'unchecked',
lastError: null,
lastCheckedAt: null,
configVersion: config.version,
config: await this.cipher.encryptV2(config),
metadata: null,
});
const row = await this.trustedSourceRepository.insertRow(
{
name: input.name,
issuer: input.issuer,
type: config.authentication.type,
managedBy: 'admin',
status: 'unchecked',
lastError: null,
lastCheckedAt: null,
configVersion: config.version,
config: await this.cipher.encryptV2(config),
metadata: null,
},
{},
);
await this.invalidateCache(row);
return this.toRuntime(row, config);
}
@ -207,7 +219,7 @@ export class TrustedSourceDbStore extends TrustedSourceStore {
async delete(id: string): Promise<void> {
const row = await this.loadAdminRow(id, 'delete');
await this.trustedSourceRepository.deleteById(id); // identities go with it via FK cascade
await this.trustedSourceRepository.deleteById(id, {}); // identities go with it via FK cascade
await this.invalidateCache(row);
}
@ -218,25 +230,28 @@ export class TrustedSourceDbStore extends TrustedSourceStore {
*/
async seedSystemSource(input: SeedSystemSourceInput): Promise<SeedSystemSourceOutcome> {
const { id, name, issuer } = input;
const existing = await this.trustedSourceRepository.findById(id);
const existing = await this.trustedSourceRepository.findById(id, {});
if (!existing) {
const config = migrateToLatest(trustedSourceConfigSchemaFor('system').parse(input.config));
await this.trustedSourceRepository.insertIfAbsent({
id,
name,
issuer,
type: config.authentication.type,
managedBy: 'system',
status: 'unchecked',
lastError: null,
lastCheckedAt: null,
configVersion: config.version,
config: await this.cipher.encryptV2(config),
metadata: null,
});
await this.trustedSourceRepository.insertIfAbsent(
{
id,
name,
issuer,
type: config.authentication.type,
managedBy: 'system',
status: 'unchecked',
lastError: null,
lastCheckedAt: null,
configVersion: config.version,
config: await this.cipher.encryptV2(config),
metadata: null,
},
{},
);
// `orIgnore` also drops the insert when an admin row holds the name or the issuer.
if (!(await this.trustedSourceRepository.findById(id))) return 'conflict';
if (!(await this.trustedSourceRepository.findById(id, {}))) return 'conflict';
await this.invalidateCache({ id, issuer });
return 'inserted';
}
@ -246,22 +261,26 @@ export class TrustedSourceDbStore extends TrustedSourceStore {
if (existing.issuer === issuer) return 'unchanged';
// Another seeder can move the issuer first; then the row holding it is this one.
const holder = await this.trustedSourceRepository.findByIssuer(issuer);
const holder = await this.trustedSourceRepository.findByIssuer(issuer, {});
if (holder) return holder.id === id ? 'unchanged' : 'conflict';
try {
// The keys do not change, so the source stays usable. Discovery fetches the metadata for the
// new issuer on its next run. A run that started before the move loses its lease.
await this.trustedSourceRepository.updateById(id, {
issuer,
lastCheckedAt: null,
discoveryClaimToken: null,
discoveryClaimedAt: null,
});
await this.trustedSourceRepository.updateById(
id,
{
issuer,
lastCheckedAt: null,
discoveryClaimToken: null,
discoveryClaimedAt: null,
},
{},
);
} catch (error) {
// Another row can take the issuer between the check and the update. A conflict must not
// stop the boot, so classify it by reading the issuer again.
const winner = await this.trustedSourceRepository.findByIssuer(issuer);
const winner = await this.trustedSourceRepository.findByIssuer(issuer, {});
if (!winner) throw error;
return winner.id === id ? 'unchanged' : 'conflict';
}
@ -270,7 +289,7 @@ export class TrustedSourceDbStore extends TrustedSourceStore {
}
private async loadAdminRow(id: string, action: 'update' | 'delete') {
const row = await this.trustedSourceRepository.findById(id);
const row = await this.trustedSourceRepository.findById(id, {});
if (!row) throw new NotFoundError(`Trusted source "${id}" not found`);
if (row.managedBy === 'system') throw new SystemTrustedSourceModificationError(action);
return row;
@ -306,7 +325,7 @@ export class TrustedSourceDbStore extends TrustedSourceStore {
}
private async loadAll(): Promise<TrustedSource[]> {
const rows = await this.trustedSourceRepository.findAll();
const rows = await this.trustedSourceRepository.findAll({});
const sources = await Promise.all(rows.map(async (row) => await this.load(row)));
return sources.filter((source): source is TrustedSource => source !== undefined);
}

17
packages/cli/src/modules/oauth-server/__tests__/oauth-flow.service.api.test.ts
View File

@ -235,13 +235,16 @@ const createIdpSource = async (
};
const bindIdpSubject = async (sourceId: string, userId: string) => {
await Container.get(TrustedSourceIdentityRepository).insertIfAbsent({
sourceId,
subject: IDP_SUBJECT,
userId,
provenance: 'admin',
status: 'active',
});
await Container.get(TrustedSourceIdentityRepository).insertIfAbsent(
{
sourceId,
subject: IDP_SUBJECT,
userId,
provenance: 'admin',
status: 'active',
},
{},
);
};
/** An access token as the external issuer would mint it for the trigger. */

13
packages/cli/test/integration/inbound-auth-core/system-trusted-source.seeder.integration.test.ts
View File

@ -34,11 +34,14 @@ const systemRow = async () => await rows.findOneByOrFail({ id: ID });
const dueIds = async () =>
(
await Container.get(TrustedSourceRepository).findDue({
errorRetrySeconds: ERROR_RETRY_SECONDS,
healthyRefreshSeconds: HEALTHY_REFRESH_SECONDS,
limit: 10,
})
await Container.get(TrustedSourceRepository).findDue(
{
errorRetrySeconds: ERROR_RETRY_SECONDS,
healthyRefreshSeconds: HEALTHY_REFRESH_SECONDS,
limit: 10,
},
{},
)
).map((row) => row.id);
const adminConfig: TrustedSourceConfigInput = {

4
packages/cli/test/integration/inbound-auth-core/trusted-source-identity.service.integration.test.ts
View File

@ -477,9 +477,9 @@ describe('TrustedSourceIdentityService (integration)', () => {
status: 'active',
} as const;
await identities.insertIfAbsent({ ...row, userId: first.id });
await identities.insertIfAbsent({ ...row, userId: first.id }, {});
await expect(
identities.insertIfAbsent({ ...row, userId: second.id }),
identities.insertIfAbsent({ ...row, userId: second.id }, {}),
).resolves.toBeUndefined();
expect(await bindings.count()).toBe(1);

38
scripts/backend-module/backend-module-guide.md
View File

@ -260,22 +260,44 @@ Service-level decorators to be aware of:
Repositories handle database access using `typeorm`, operating on database models ("entities").
A repository is the boundary that keeps TypeORM out of business logic. It therefore **composes** a `BaseRepository` and never extends one. If it extended `BaseRepository`, every service that injects it would also get `find`, `save`, `delete`, `createQueryBuilder` and `.manager` for free, and TypeORM would leak through inferred types that no lint rule can see. Instead the repository holds a file-private subclass (the table) and exposes only named use-case methods that call the table internally.
```ts
import { BaseRepository, OperationContext, TransactionRunner } from '@n8n/db';
import { Service } from '@n8n/di';
import { DataSource } from '@n8n/typeorm';
import { MyFeatureEntity } from './my-feature.entity';
// File-private: never exported, never registered with DI.
class MyFeatureTable extends BaseRepository<MyFeatureEntity> {
constructor(dataSource: DataSource, transactionRunner: TransactionRunner) {
super(MyFeatureEntity, dataSource.manager, transactionRunner);
}
}
@Service()
export class MyFeatureRepository extends Repository<MyFeatureEntity> {
constructor(dataSource: DataSource) {
super(MyFeatureEntity, dataSource.manager);
export class MyFeatureRepository {
private readonly table: MyFeatureTable;
constructor(dataSource: DataSource, transactionRunner: TransactionRunner) {
this.table = new MyFeatureTable(dataSource, transactionRunner);
}
async getSummary() {
return await /* typeorm query on entities */;
async findById(id: string, ctx: OperationContext): Promise<MyFeatureEntity | null> {
return await this.table.managerFor(ctx).findOneBy(MyFeatureEntity, { id });
}
}
```
Repository-level decorators to be aware of:
Rules:
- `@Service()` to make a repository usable by the dependency injection container
- Only the repository is `@Service()`. The table class is not exported.
- Every public method takes plain parameters, returns domain shapes, and takes `ctx: OperationContext` last. Do not give `ctx` a default: a forgotten `ctx` inside a transaction must be a compile error, not a second connection. Pass the entity class explicitly to the manager call (`findOneBy(MyFeatureEntity, …)`), because the repository is not a TypeORM repository for that entity.
- No generic `find`/`save`/`delete` passthrough and no getter for the table.
- A non-transactional caller passes `{}` as `ctx`. Inside `TransactionRunner.run`, thread the `ctx` you received.
See `packages/cli/src/modules/inbound-auth-core/database/repositories/trusted-source.repository.ts` for a real example. Most existing repositories in `packages/cli` still extend `BaseRepository` or `Repository`. They are legacy: do not copy them.
## Entities
@ -345,6 +367,8 @@ Place tests with the module that owns them. A packaged module keeps its tests un
Use `@n8n/backend-test-utils` for shared database and module helpers. Pass packaged module import thunks to `testModules.loadModules`. Use `tscDecoratorTransform` from `@n8n/vitest-config/tsc-decorator-transform` when a package owns TypeORM entities.
A composition repository has no `find`, `save`, or `delete`, so tests cannot seed or assert through `Container.get(MyFeatureRepository)` the way older tests do. Seed and assert through the repository's use-case methods, or through `Container.get(DataSource).getRepository(MyFeatureEntity)` as `packages/cli/src/encryption/__tests__/encryption-bootstrap.service.integration.test.ts` does. Do not add a `find()` passthrough for tests. `testDb.truncate` works unchanged.
## Future work
1. A few aspects of modules continue to be defined outside a module's dir:

31
scripts/backend-module/my-feature.repository.template
View File

@ -1,15 +1,28 @@
import { BaseRepository, OperationContext, TransactionRunner } from '@n8n/db';
import { Service } from '@n8n/di';
import { DataSource, Repository } from '@n8n/typeorm';
import { DataSource } from '@n8n/typeorm';
import { MyFeatureEntity } from './my-feature.entity';
@Service()
export class MyFeatureRepository extends Repository<MyFeatureEntity> {
constructor(dataSource: DataSource) {
super(MyFeatureEntity, dataSource.manager);
}
async getSummary() {
return await Promise.resolve({});
// File-private table: never exported, never registered with DI. Only the
// repository below reaches TypeORM, so nothing leaks into business logic.
class MyFeatureTable extends BaseRepository<MyFeatureEntity> {
constructor(dataSource: DataSource, transactionRunner: TransactionRunner) {
super(MyFeatureEntity, dataSource.manager, transactionRunner);
}
}
@Service()
export class MyFeatureRepository {
private readonly table: MyFeatureTable;
constructor(dataSource: DataSource, transactionRunner: TransactionRunner) {
this.table = new MyFeatureTable(dataSource, transactionRunner);
}
// Every data access is a named use-case method: plain params in, domain
// shapes out, `ctx` last, manager resolved via `this.table.managerFor(ctx)`.
async getSummary(ctx: OperationContext) {
return await this.table.managerFor(ctx).count(MyFeatureEntity);
}
}

2
scripts/backend-module/my-feature.service.template
View File

@ -38,6 +38,6 @@ export class MyFeatureService {
}
async getSummary() {
return await this.myFeatureRepository.getSummary();
return await this.myFeatureRepository.getSummary({});
}
}