fix: 保证消息模板引用一致性
This commit is contained in:
parent
3686e82604
commit
1a19730c18
@ -1,6 +1,11 @@
|
|||||||
import { Injectable } from '@nestjs/common';
|
import { Injectable } from '@nestjs/common';
|
||||||
import { InjectRepository } from '@nestjs/typeorm';
|
import { InjectRepository } from '@nestjs/typeorm';
|
||||||
import { Like, Repository, type FindOptionsWhere } from 'typeorm';
|
import {
|
||||||
|
Like,
|
||||||
|
Repository,
|
||||||
|
type EntityManager,
|
||||||
|
type FindOptionsWhere,
|
||||||
|
} from 'typeorm';
|
||||||
import type {
|
import type {
|
||||||
MessageTemplateInput,
|
MessageTemplateInput,
|
||||||
MessageTemplateListQuery,
|
MessageTemplateListQuery,
|
||||||
@ -124,16 +129,31 @@ export class QqbotMessageTemplateService {
|
|||||||
* @throws {SystemMessageContractError} When any non-deleted binding still references it.
|
* @throws {SystemMessageContractError} When any non-deleted binding still references it.
|
||||||
*/
|
*/
|
||||||
async remove(id: string): Promise<boolean> {
|
async remove(id: string): Promise<boolean> {
|
||||||
const current = await this.findActive(id);
|
return this.templateRepository.manager.transaction(
|
||||||
const referenceCount = await this.bindingRepository.count({
|
/** Holds the template write lock until the reference check and soft deletion commit together. */
|
||||||
|
async (manager) => {
|
||||||
|
const templateRepository = manager.getRepository(QqbotMessageTemplate);
|
||||||
|
const bindingRepository = manager.getRepository(
|
||||||
|
QqbotMessagePublishBinding,
|
||||||
|
);
|
||||||
|
const current = await templateRepository.findOne({
|
||||||
|
lock: { mode: 'pessimistic_write' },
|
||||||
|
where: { id, isDeleted: false },
|
||||||
|
});
|
||||||
|
if (!current) throw new SystemMessageContractError('template_invalid');
|
||||||
|
|
||||||
|
const referenceCount = await bindingRepository.count({
|
||||||
where: { isDeleted: false, templateId: id },
|
where: { isDeleted: false, templateId: id },
|
||||||
});
|
});
|
||||||
if (referenceCount > 0)
|
if (referenceCount > 0) {
|
||||||
throw new SystemMessageContractError('template_invalid');
|
throw new SystemMessageContractError('template_invalid');
|
||||||
|
}
|
||||||
current.enabled = false;
|
current.enabled = false;
|
||||||
current.isDeleted = true;
|
current.isDeleted = true;
|
||||||
await this.templateRepository.save(current);
|
await templateRepository.save(current);
|
||||||
return true;
|
return true;
|
||||||
|
},
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@ -154,7 +174,12 @@ export class QqbotMessageTemplateService {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Enforces the reusable template gate before a publish binding is created or enabled.
|
* Enforces the template lock order before a publish binding is created, updated, or revived.
|
||||||
|
*
|
||||||
|
* Callers must invoke this inside their current transaction before saving a non-deleted
|
||||||
|
* binding, and must retain that transaction through the binding save and commit. Disabled
|
||||||
|
* bindings also require the lock because they still create a live template reference.
|
||||||
|
* @param manager - Current binding-write transaction manager; never pass a global manager.
|
||||||
* @param templateId - Candidate template's string Snowflake identity.
|
* @param templateId - Candidate template's string Snowflake identity.
|
||||||
* @param sourceKey - Source subscribed by the binding.
|
* @param sourceKey - Source subscribed by the binding.
|
||||||
* @param bindingEnabled - Whether the binding is being created or enabled for delivery.
|
* @param bindingEnabled - Whether the binding is being created or enabled for delivery.
|
||||||
@ -162,11 +187,13 @@ export class QqbotMessageTemplateService {
|
|||||||
* @throws {SystemMessageContractError} With `template_invalid` for every unavailable binding choice.
|
* @throws {SystemMessageContractError} With `template_invalid` for every unavailable binding choice.
|
||||||
*/
|
*/
|
||||||
async requireAvailableForBinding(
|
async requireAvailableForBinding(
|
||||||
|
manager: EntityManager,
|
||||||
templateId: string,
|
templateId: string,
|
||||||
sourceKey: string,
|
sourceKey: string,
|
||||||
bindingEnabled: boolean,
|
bindingEnabled: boolean,
|
||||||
): Promise<QqbotMessageTemplate> {
|
): Promise<QqbotMessageTemplate> {
|
||||||
const template = await this.templateRepository.findOne({
|
const template = await manager.getRepository(QqbotMessageTemplate).findOne({
|
||||||
|
lock: { mode: 'pessimistic_write' },
|
||||||
where: { id: templateId, isDeleted: false },
|
where: { id: templateId, isDeleted: false },
|
||||||
});
|
});
|
||||||
if (!template || template.sourceKey !== sourceKey) {
|
if (!template || template.sourceKey !== sourceKey) {
|
||||||
|
|||||||
@ -1,4 +1,4 @@
|
|||||||
import type { Repository } from 'typeorm';
|
import type { EntityManager, FindOptionsWhere, Repository } from 'typeorm';
|
||||||
import { KtDateTime } from '../../../../src/common';
|
import { KtDateTime } from '../../../../src/common';
|
||||||
import { SystemMessageTemplateRendererService } from '../../../../src/modules/qqbot/core/application/message-push/system-message-template-renderer.service';
|
import { SystemMessageTemplateRendererService } from '../../../../src/modules/qqbot/core/application/message-push/system-message-template-renderer.service';
|
||||||
import { QqbotMessageTemplateService } from '../../../../src/modules/qqbot/core/application/message-push/qqbot-message-template.service';
|
import { QqbotMessageTemplateService } from '../../../../src/modules/qqbot/core/application/message-push/qqbot-message-template.service';
|
||||||
@ -28,6 +28,45 @@ function template(
|
|||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** Creates a publish-binding fixture used to prove live-reference filtering. */
|
||||||
|
function binding(
|
||||||
|
overrides: Partial<QqbotMessagePublishBinding> = {},
|
||||||
|
): QqbotMessagePublishBinding {
|
||||||
|
return {
|
||||||
|
accountId: '200',
|
||||||
|
activeKey: '200:300',
|
||||||
|
createId: jest.fn(),
|
||||||
|
createTime: NOW as never,
|
||||||
|
enabled: true,
|
||||||
|
id: '400',
|
||||||
|
isDeleted: false,
|
||||||
|
selfId: '123456',
|
||||||
|
subscriptionId: '300',
|
||||||
|
templateId: '100',
|
||||||
|
updateTime: NOW as never,
|
||||||
|
...overrides,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Reads the value held by TypeORM's private Like find operator for fake filtering. */
|
||||||
|
function likeValue(value: unknown): string | undefined {
|
||||||
|
if (typeof value !== 'object' || value === null) return undefined;
|
||||||
|
const candidate = value as { _value?: unknown };
|
||||||
|
return typeof candidate._value === 'string' ? candidate._value : undefined;
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Builds a promise gate whose resolution is controlled by the test. */
|
||||||
|
function deferred(): {
|
||||||
|
promise: Promise<void>;
|
||||||
|
resolve: () => void;
|
||||||
|
} {
|
||||||
|
let resolve!: () => void;
|
||||||
|
const promise = new Promise<void>((next) => {
|
||||||
|
resolve = next;
|
||||||
|
});
|
||||||
|
return { promise, resolve };
|
||||||
|
}
|
||||||
|
|
||||||
/** Registers the source definition used by template lifecycle tests. */
|
/** Registers the source definition used by template lifecycle tests. */
|
||||||
function registry(): SystemMessageSourceRegistry {
|
function registry(): SystemMessageSourceRegistry {
|
||||||
const value = new SystemMessageSourceRegistry();
|
const value = new SystemMessageSourceRegistry();
|
||||||
@ -72,10 +111,37 @@ function registry(): SystemMessageSourceRegistry {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/** Constructs a template service with narrow repository fakes for unit-level lifecycle checks. */
|
/** Constructs a template service with narrow repository fakes for unit-level lifecycle checks. */
|
||||||
function setup(items: QqbotMessageTemplate[] = [template()]) {
|
function setup(
|
||||||
|
items: QqbotMessageTemplate[] = [template()],
|
||||||
|
bindings: QqbotMessagePublishBinding[] = [],
|
||||||
|
) {
|
||||||
const templateRepository = {
|
const templateRepository = {
|
||||||
create: jest.fn((input) => template(input)),
|
create: jest.fn((input) => template(input)),
|
||||||
findAndCount: jest.fn().mockResolvedValue([items, items.length]),
|
findAndCount: jest.fn(
|
||||||
|
async ({
|
||||||
|
skip = 0,
|
||||||
|
take = items.length,
|
||||||
|
where,
|
||||||
|
}: {
|
||||||
|
skip?: number;
|
||||||
|
take?: number;
|
||||||
|
where: FindOptionsWhere<QqbotMessageTemplate>;
|
||||||
|
}) => {
|
||||||
|
const namePattern = likeValue(where.name);
|
||||||
|
const name = namePattern?.slice(1, -1);
|
||||||
|
const filtered = items.filter((item) => {
|
||||||
|
if (item.isDeleted !== where.isDeleted) return false;
|
||||||
|
if (where.enabled !== undefined && item.enabled !== where.enabled) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
if (where.sourceKey && item.sourceKey !== where.sourceKey) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
return !name || item.name.includes(name);
|
||||||
|
});
|
||||||
|
return [filtered.slice(skip, skip + take), filtered.length];
|
||||||
|
},
|
||||||
|
),
|
||||||
findOne: jest.fn(
|
findOne: jest.fn(
|
||||||
async ({ where: { id } }) =>
|
async ({ where: { id } }) =>
|
||||||
items.find((item) => item.id === id && !item.isDeleted) ?? null,
|
items.find((item) => item.id === id && !item.isDeleted) ?? null,
|
||||||
@ -83,30 +149,53 @@ function setup(items: QqbotMessageTemplate[] = [template()]) {
|
|||||||
save: jest.fn(async (item) => item),
|
save: jest.fn(async (item) => item),
|
||||||
} as unknown as jest.Mocked<Repository<QqbotMessageTemplate>>;
|
} as unknown as jest.Mocked<Repository<QqbotMessageTemplate>>;
|
||||||
const bindingRepository = {
|
const bindingRepository = {
|
||||||
count: jest.fn().mockResolvedValue(0),
|
count: jest.fn(
|
||||||
|
async ({ where: { isDeleted, templateId } }) =>
|
||||||
|
bindings.filter(
|
||||||
|
(item) =>
|
||||||
|
item.isDeleted === isDeleted && item.templateId === templateId,
|
||||||
|
).length,
|
||||||
|
),
|
||||||
} as unknown as jest.Mocked<Repository<QqbotMessagePublishBinding>>;
|
} as unknown as jest.Mocked<Repository<QqbotMessagePublishBinding>>;
|
||||||
|
const manager = {
|
||||||
|
getRepository: jest.fn((entity) => {
|
||||||
|
if (entity === QqbotMessageTemplate) return templateRepository;
|
||||||
|
if (entity === QqbotMessagePublishBinding) return bindingRepository;
|
||||||
|
throw new Error('unexpected repository');
|
||||||
|
}),
|
||||||
|
} as unknown as jest.Mocked<EntityManager>;
|
||||||
|
Object.assign(templateRepository, {
|
||||||
|
manager: {
|
||||||
|
transaction: jest.fn((callback) => callback(manager)),
|
||||||
|
},
|
||||||
|
});
|
||||||
const service = new QqbotMessageTemplateService(
|
const service = new QqbotMessageTemplateService(
|
||||||
templateRepository,
|
templateRepository,
|
||||||
bindingRepository,
|
bindingRepository,
|
||||||
registry(),
|
registry(),
|
||||||
new SystemMessageTemplateRendererService(),
|
new SystemMessageTemplateRendererService(),
|
||||||
);
|
);
|
||||||
return { bindingRepository, service, templateRepository };
|
return { bindingRepository, manager, service, templateRepository };
|
||||||
}
|
}
|
||||||
|
|
||||||
describe('QqbotMessageTemplateService', () => {
|
describe('QqbotMessageTemplateService', () => {
|
||||||
it('pages undeleted templates with filters, source names, and every live binding reference', async () => {
|
it('pages undeleted templates with filters, source names, and every live binding reference', async () => {
|
||||||
const { bindingRepository, service, templateRepository } = setup([
|
const { bindingRepository, service, templateRepository } = setup(
|
||||||
|
[
|
||||||
template({ id: '100' }),
|
template({ id: '100' }),
|
||||||
template({ id: '101', name: '另一模板' }),
|
template({ id: '101', isDeleted: true }),
|
||||||
]);
|
template({ id: '102', enabled: false }),
|
||||||
bindingRepository.count.mockResolvedValueOnce(2).mockResolvedValueOnce(1);
|
template({ id: '103', sourceKey: 'other.source' }),
|
||||||
|
template({ id: '104', name: '名称不匹配' }),
|
||||||
|
],
|
||||||
|
[binding({ enabled: false }), binding({ id: '401', isDeleted: true })],
|
||||||
|
);
|
||||||
|
|
||||||
await expect(
|
await expect(
|
||||||
service.page({
|
service.page({
|
||||||
enabled: true,
|
enabled: true,
|
||||||
name: '端口',
|
name: '端口',
|
||||||
pageNo: 2,
|
pageNo: 1,
|
||||||
pageSize: 10,
|
pageSize: 10,
|
||||||
sourceKey: SOURCE_KEY,
|
sourceKey: SOURCE_KEY,
|
||||||
}),
|
}),
|
||||||
@ -114,19 +203,23 @@ describe('QqbotMessageTemplateService', () => {
|
|||||||
items: [
|
items: [
|
||||||
expect.objectContaining({
|
expect.objectContaining({
|
||||||
id: '100',
|
id: '100',
|
||||||
referenceCount: 2,
|
|
||||||
sourceName: 'STUN 端口变化',
|
|
||||||
}),
|
|
||||||
expect.objectContaining({
|
|
||||||
id: '101',
|
|
||||||
referenceCount: 1,
|
referenceCount: 1,
|
||||||
sourceName: 'STUN 端口变化',
|
sourceName: 'STUN 端口变化',
|
||||||
}),
|
}),
|
||||||
],
|
],
|
||||||
total: 2,
|
total: 1,
|
||||||
});
|
});
|
||||||
expect(templateRepository.findAndCount).toHaveBeenCalledWith(
|
expect(templateRepository.findAndCount).toHaveBeenCalledWith(
|
||||||
expect.objectContaining({ skip: 10, take: 10 }),
|
expect.objectContaining({
|
||||||
|
skip: 0,
|
||||||
|
take: 10,
|
||||||
|
where: {
|
||||||
|
enabled: true,
|
||||||
|
isDeleted: false,
|
||||||
|
name: expect.objectContaining({ _type: 'like', _value: '%端口%' }),
|
||||||
|
sourceKey: SOURCE_KEY,
|
||||||
|
},
|
||||||
|
}),
|
||||||
);
|
);
|
||||||
expect(bindingRepository.count).toHaveBeenNthCalledWith(1, {
|
expect(bindingRepository.count).toHaveBeenNthCalledWith(1, {
|
||||||
where: { isDeleted: false, templateId: '100' },
|
where: { isDeleted: false, templateId: '100' },
|
||||||
@ -205,7 +298,7 @@ describe('QqbotMessageTemplateService', () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
it('blocks soft deletion when any non-deleted binding references the template, including disabled bindings', async () => {
|
it('blocks soft deletion when any non-deleted binding references the template, including disabled bindings', async () => {
|
||||||
const { bindingRepository, service, templateRepository } = setup();
|
const { bindingRepository, manager, service, templateRepository } = setup();
|
||||||
bindingRepository.count.mockResolvedValueOnce(1).mockResolvedValueOnce(0);
|
bindingRepository.count.mockResolvedValueOnce(1).mockResolvedValueOnce(0);
|
||||||
|
|
||||||
await expect(service.remove('100')).rejects.toMatchObject({
|
await expect(service.remove('100')).rejects.toMatchObject({
|
||||||
@ -215,28 +308,132 @@ describe('QqbotMessageTemplateService', () => {
|
|||||||
expect(templateRepository.save).toHaveBeenLastCalledWith(
|
expect(templateRepository.save).toHaveBeenLastCalledWith(
|
||||||
expect.objectContaining({ enabled: false, isDeleted: true }),
|
expect.objectContaining({ enabled: false, isDeleted: true }),
|
||||||
);
|
);
|
||||||
|
expect(templateRepository.manager.transaction).toHaveBeenCalledTimes(2);
|
||||||
|
expect(manager.getRepository).toHaveBeenCalledWith(QqbotMessageTemplate);
|
||||||
|
expect(templateRepository.findOne).toHaveBeenLastCalledWith({
|
||||||
|
lock: { mode: 'pessimistic_write' },
|
||||||
|
where: { id: '100', isDeleted: false },
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
it('gates publish bindings by source, deletion, enabled state, and current template validity', async () => {
|
it('gates publish bindings by source, deletion, enabled state, and current template validity', async () => {
|
||||||
const current = template();
|
const current = template();
|
||||||
const { service } = setup([current]);
|
const { manager, service, templateRepository } = setup([current]);
|
||||||
|
|
||||||
await expect(
|
await expect(
|
||||||
service.requireAvailableForBinding('100', SOURCE_KEY, true),
|
service.requireAvailableForBinding(manager, '100', SOURCE_KEY, true),
|
||||||
).resolves.toBe(current);
|
).resolves.toBe(current);
|
||||||
current.enabled = false;
|
current.enabled = false;
|
||||||
await expect(
|
await expect(
|
||||||
service.requireAvailableForBinding('100', SOURCE_KEY, true),
|
service.requireAvailableForBinding(manager, '100', SOURCE_KEY, true),
|
||||||
).rejects.toMatchObject({ code: 'template_invalid' });
|
).rejects.toMatchObject({ code: 'template_invalid' });
|
||||||
await expect(
|
await expect(
|
||||||
service.requireAvailableForBinding('100', SOURCE_KEY, false),
|
service.requireAvailableForBinding(manager, '100', SOURCE_KEY, false),
|
||||||
).resolves.toBe(current);
|
).resolves.toBe(current);
|
||||||
await expect(
|
await expect(
|
||||||
service.requireAvailableForBinding('100', 'other.source', false),
|
service.requireAvailableForBinding(manager, '100', 'other.source', false),
|
||||||
).rejects.toMatchObject({ code: 'template_invalid' });
|
).rejects.toMatchObject({ code: 'template_invalid' });
|
||||||
current.isDeleted = true;
|
current.isDeleted = true;
|
||||||
await expect(
|
await expect(
|
||||||
service.requireAvailableForBinding('100', SOURCE_KEY, false),
|
service.requireAvailableForBinding(manager, '100', SOURCE_KEY, false),
|
||||||
).rejects.toMatchObject({ code: 'template_invalid' });
|
).rejects.toMatchObject({ code: 'template_invalid' });
|
||||||
|
expect(templateRepository.findOne).toHaveBeenCalledWith({
|
||||||
|
lock: { mode: 'pessimistic_write' },
|
||||||
|
where: { id: '100', isDeleted: false },
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it('holds the binding template lock through commit so deletion re-counts the committed live binding', async () => {
|
||||||
|
const current = template();
|
||||||
|
const bindings: QqbotMessagePublishBinding[] = [];
|
||||||
|
const bindingHasLock = deferred();
|
||||||
|
const allowBindingCommit = deferred();
|
||||||
|
let lockTail = Promise.resolve();
|
||||||
|
let deleteCountStarted = false;
|
||||||
|
|
||||||
|
/** Creates a transaction manager whose pessimistic lock is held until callback completion. */
|
||||||
|
const createTransactionManager = (): EntityManager => {
|
||||||
|
let releaseTemplateLock: (() => void) | undefined;
|
||||||
|
const acquireTemplateLock = async (): Promise<void> => {
|
||||||
|
const previous = lockTail;
|
||||||
|
const lockReleased = deferred();
|
||||||
|
lockTail = lockReleased.promise;
|
||||||
|
await previous;
|
||||||
|
releaseTemplateLock = lockReleased.resolve;
|
||||||
|
};
|
||||||
|
const manager = {
|
||||||
|
getRepository: jest.fn((entity) => {
|
||||||
|
if (entity === QqbotMessageTemplate) {
|
||||||
|
return {
|
||||||
|
findOne: jest.fn(async (options) => {
|
||||||
|
if (options.lock?.mode === 'pessimistic_write') {
|
||||||
|
await acquireTemplateLock();
|
||||||
|
}
|
||||||
|
return current.isDeleted ? null : current;
|
||||||
|
}),
|
||||||
|
save: jest.fn(async (item) => item),
|
||||||
|
};
|
||||||
|
}
|
||||||
|
if (entity === QqbotMessagePublishBinding) {
|
||||||
|
return {
|
||||||
|
count: jest.fn(async () => {
|
||||||
|
deleteCountStarted = true;
|
||||||
|
return bindings.filter(
|
||||||
|
(item) => !item.isDeleted && item.templateId === current.id,
|
||||||
|
).length;
|
||||||
|
}),
|
||||||
|
};
|
||||||
|
}
|
||||||
|
throw new Error('unexpected repository');
|
||||||
|
}),
|
||||||
|
} as unknown as EntityManager;
|
||||||
|
Object.assign(manager, {
|
||||||
|
transaction: async <T>(
|
||||||
|
callback: (currentManager: EntityManager) => Promise<T>,
|
||||||
|
) => {
|
||||||
|
try {
|
||||||
|
return await callback(manager);
|
||||||
|
} finally {
|
||||||
|
releaseTemplateLock?.();
|
||||||
|
}
|
||||||
|
},
|
||||||
|
});
|
||||||
|
return manager;
|
||||||
|
};
|
||||||
|
const transactionManager = createTransactionManager();
|
||||||
|
const templateRepository = {
|
||||||
|
manager: transactionManager,
|
||||||
|
} as unknown as Repository<QqbotMessageTemplate>;
|
||||||
|
const service = new QqbotMessageTemplateService(
|
||||||
|
templateRepository,
|
||||||
|
{} as Repository<QqbotMessagePublishBinding>,
|
||||||
|
registry(),
|
||||||
|
new SystemMessageTemplateRendererService(),
|
||||||
|
);
|
||||||
|
|
||||||
|
const bindingTransaction = transactionManager.transaction(
|
||||||
|
async (manager) => {
|
||||||
|
await service.requireAvailableForBinding(
|
||||||
|
manager,
|
||||||
|
'100',
|
||||||
|
SOURCE_KEY,
|
||||||
|
true,
|
||||||
|
);
|
||||||
|
bindingHasLock.resolve();
|
||||||
|
await allowBindingCommit.promise;
|
||||||
|
bindings.push(binding());
|
||||||
|
},
|
||||||
|
);
|
||||||
|
await bindingHasLock.promise;
|
||||||
|
|
||||||
|
const deletion = service.remove('100');
|
||||||
|
await Promise.resolve();
|
||||||
|
expect(deleteCountStarted).toBe(false);
|
||||||
|
|
||||||
|
allowBindingCommit.resolve();
|
||||||
|
await bindingTransaction;
|
||||||
|
await expect(deletion).rejects.toMatchObject({ code: 'template_invalid' });
|
||||||
|
expect(current.isDeleted).toBe(false);
|
||||||
|
expect(bindings).toHaveLength(1);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user