fix: race condition and delete checking for templates
This commit is contained in:
1 parent
6c2d2c48bf
commit
696b79c5d7
18 files changed
+334
-51
No files matched your search
@@ -1,33 +1,53 @@
|
||||
import { describe, it, expect, beforeEach, vi } from 'vitest';
|
||||
import { TemplateEngine } from '../../src/main/engine/TemplateEngine';
|
||||
import { posts, tags, templates } from '../../src/main/database/schema';
|
||||
|
||||
const mockTemplates = new Map<string, any>();
|
||||
const mockPosts = new Map<string, any>();
|
||||
const mockTags = new Map<string, any>();
|
||||
const mockFiles = new Map<string, string>();
|
||||
|
||||
function createSelectChain() {
|
||||
function createSelectChain(dataSource: () => any[]) {
|
||||
return {
|
||||
from: vi.fn().mockReturnThis(),
|
||||
from: vi.fn(function (this: any, table: any) {
|
||||
if (table === posts) {
|
||||
this.all = vi.fn(() => Promise.resolve(Array.from(mockPosts.values())));
|
||||
} else if (table === tags) {
|
||||
this.all = vi.fn(() => Promise.resolve(Array.from(mockTags.values())));
|
||||
}
|
||||
return this;
|
||||
}),
|
||||
where: vi.fn().mockReturnThis(),
|
||||
orderBy: vi.fn().mockReturnThis(),
|
||||
all: vi.fn().mockImplementation(() => Promise.resolve(Array.from(mockTemplates.values()))),
|
||||
all: vi.fn().mockImplementation(() => Promise.resolve(dataSource())),
|
||||
get: vi.fn().mockImplementation(() => Promise.resolve(undefined)),
|
||||
};
|
||||
}
|
||||
|
||||
function createDrizzleMock() {
|
||||
return {
|
||||
select: vi.fn(() => createSelectChain()),
|
||||
select: vi.fn(() => createSelectChain(() => Array.from(mockTemplates.values()))),
|
||||
insert: vi.fn(() => ({
|
||||
values: vi.fn((data: any) => {
|
||||
mockTemplates.set(data.id, data);
|
||||
return Promise.resolve();
|
||||
}),
|
||||
})),
|
||||
update: vi.fn(() => ({
|
||||
update: vi.fn((table?: any) => ({
|
||||
set: vi.fn((updates: any) => ({
|
||||
where: vi.fn(async () => {
|
||||
for (const [templateId, existing] of mockTemplates.entries()) {
|
||||
mockTemplates.set(templateId, { ...existing, ...updates });
|
||||
if (table === posts) {
|
||||
for (const [postId, existing] of mockPosts.entries()) {
|
||||
mockPosts.set(postId, { ...existing, ...updates });
|
||||
}
|
||||
} else if (table === tags) {
|
||||
for (const [tagId, existing] of mockTags.entries()) {
|
||||
mockTags.set(tagId, { ...existing, ...updates });
|
||||
}
|
||||
} else {
|
||||
for (const [templateId, existing] of mockTemplates.entries()) {
|
||||
mockTemplates.set(templateId, { ...existing, ...updates });
|
||||
}
|
||||
}
|
||||
}),
|
||||
})),
|
||||
@@ -101,9 +121,11 @@ describe('TemplateEngine', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
mockTemplates.clear();
|
||||
mockPosts.clear();
|
||||
mockTags.clear();
|
||||
mockFiles.clear();
|
||||
(globalThis as any).__mockTemplateFiles = mockFiles;
|
||||
vi.mocked(mockLocalDb.select).mockImplementation(() => createSelectChain());
|
||||
vi.mocked(mockLocalDb.select).mockImplementation(() => createSelectChain(() => Array.from(mockTemplates.values())));
|
||||
|
||||
templateEngine = new TemplateEngine();
|
||||
templateEngine.setProjectContext('default', '/mock/userData/projects/default');
|
||||
@@ -170,9 +192,9 @@ describe('TemplateEngine', () => {
|
||||
content: '<footer>Footer</footer>',
|
||||
});
|
||||
|
||||
const deleted = await templateEngine.deleteTemplate(created.id);
|
||||
const result = await templateEngine.deleteTemplate(created.id);
|
||||
|
||||
expect(deleted).toBe(true);
|
||||
expect(result).toEqual({ deleted: true });
|
||||
expect(mockTemplates.has(created.id)).toBe(false);
|
||||
expect(mockFiles.has('/mock/userData/projects/default/templates/delete_me.liquid')).toBe(false);
|
||||
});
|
||||
@@ -389,4 +411,110 @@ describe('TemplateEngine', () => {
|
||||
expect(found).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('referential integrity', () => {
|
||||
it('getTemplateReferences returns empty arrays when no references exist', async () => {
|
||||
const refs = await templateEngine.getTemplateReferences('custom_post');
|
||||
expect(refs).toEqual({ postIds: [], tagIds: [] });
|
||||
});
|
||||
|
||||
it('getTemplateReferences returns referencing post and tag IDs', async () => {
|
||||
mockPosts.set('post-1', { id: 'post-1', projectId: 'default', templateSlug: 'custom_post' });
|
||||
mockTags.set('tag-1', { id: 'tag-1', projectId: 'default', postTemplateSlug: 'custom_post' });
|
||||
|
||||
const refs = await templateEngine.getTemplateReferences('custom_post');
|
||||
expect(refs.postIds).toContain('post-1');
|
||||
expect(refs.tagIds).toContain('tag-1');
|
||||
});
|
||||
|
||||
it('deleteTemplate blocks deletion when references exist and force is not set', async () => {
|
||||
const created = await templateEngine.createTemplate({
|
||||
title: 'Referenced Template',
|
||||
kind: 'post',
|
||||
content: '<main>Referenced</main>',
|
||||
});
|
||||
|
||||
mockPosts.set('post-1', { id: 'post-1', projectId: 'default', templateSlug: created.slug });
|
||||
|
||||
const result = await templateEngine.deleteTemplate(created.id);
|
||||
|
||||
expect(result.deleted).toBe(false);
|
||||
expect(result.references?.postIds).toContain('post-1');
|
||||
expect(mockTemplates.has(created.id)).toBe(true);
|
||||
});
|
||||
|
||||
it('deleteTemplate with force clears references and deletes', async () => {
|
||||
const created = await templateEngine.createTemplate({
|
||||
title: 'Force Delete',
|
||||
kind: 'post',
|
||||
content: '<main>Force</main>',
|
||||
});
|
||||
|
||||
mockPosts.set('post-1', { id: 'post-1', projectId: 'default', templateSlug: created.slug });
|
||||
mockTags.set('tag-1', { id: 'tag-1', projectId: 'default', postTemplateSlug: created.slug });
|
||||
|
||||
const result = await templateEngine.deleteTemplate(created.id, { force: true });
|
||||
|
||||
expect(result.deleted).toBe(true);
|
||||
expect(mockTemplates.has(created.id)).toBe(false);
|
||||
expect(mockPosts.get('post-1').templateSlug).toBeNull();
|
||||
expect(mockTags.get('tag-1').postTemplateSlug).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('slug cascade on rename', () => {
|
||||
it('cascades slug changes to posts.templateSlug on template rename', async () => {
|
||||
const created = await templateEngine.createTemplate({
|
||||
title: 'Original Name',
|
||||
kind: 'post',
|
||||
content: '<main>Content</main>',
|
||||
});
|
||||
|
||||
mockPosts.set('post-1', { id: 'post-1', projectId: 'default', templateSlug: created.slug });
|
||||
|
||||
await templateEngine.updateTemplate(created.id, { title: 'Renamed Template' });
|
||||
|
||||
expect(mockPosts.get('post-1').templateSlug).toBe('renamed_template');
|
||||
});
|
||||
|
||||
it('cascades slug changes to tags.postTemplateSlug on template rename', async () => {
|
||||
const created = await templateEngine.createTemplate({
|
||||
title: 'Original Name',
|
||||
kind: 'post',
|
||||
content: '<main>Content</main>',
|
||||
});
|
||||
|
||||
mockTags.set('tag-1', { id: 'tag-1', projectId: 'default', postTemplateSlug: created.slug });
|
||||
|
||||
await templateEngine.updateTemplate(created.id, { title: 'Renamed Template' });
|
||||
|
||||
expect(mockTags.get('tag-1').postTemplateSlug).toBe('renamed_template');
|
||||
});
|
||||
});
|
||||
|
||||
describe('race condition protection', () => {
|
||||
it('rolls back DB on file operation failure during update', async () => {
|
||||
const created = await templateEngine.createTemplate({
|
||||
title: 'Rollback Test',
|
||||
kind: 'post',
|
||||
content: '<main>Original</main>',
|
||||
});
|
||||
|
||||
const originalSlug = created.slug;
|
||||
const originalTitle = created.title;
|
||||
|
||||
// Make fs.rename throw to simulate file operation failure
|
||||
const fsModule = await import('fs/promises');
|
||||
vi.mocked(fsModule.rename).mockRejectedValueOnce(new Error('EPERM'));
|
||||
|
||||
await expect(
|
||||
templateEngine.updateTemplate(created.id, { title: 'Should Fail' }),
|
||||
).rejects.toThrow('EPERM');
|
||||
|
||||
// DB should have been rolled back to original values
|
||||
const row = mockTemplates.get(created.id);
|
||||
expect(row.slug).toBe(originalSlug);
|
||||
expect(row.title).toBe(originalTitle);
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -2844,12 +2844,21 @@ describe('IPC Handlers', () => {
|
||||
|
||||
describe('templates:delete', () => {
|
||||
it('should call TemplateEngine.deleteTemplate with id', async () => {
|
||||
mockTemplateEngine.deleteTemplate.mockResolvedValue(true);
|
||||
mockTemplateEngine.deleteTemplate.mockResolvedValue({ deleted: true });
|
||||
|
||||
const result = await invokeHandler('templates:delete', 'template-1');
|
||||
|
||||
expect(mockTemplateEngine.deleteTemplate).toHaveBeenCalledWith('template-1');
|
||||
expect(result).toBe(true);
|
||||
expect(mockTemplateEngine.deleteTemplate).toHaveBeenCalledWith('template-1', undefined);
|
||||
expect(result).toEqual({ deleted: true });
|
||||
});
|
||||
|
||||
it('should forward force option to TemplateEngine.deleteTemplate', async () => {
|
||||
mockTemplateEngine.deleteTemplate.mockResolvedValue({ deleted: true });
|
||||
|
||||
const result = await invokeHandler('templates:delete', 'template-1', { force: true });
|
||||
|
||||
expect(mockTemplateEngine.deleteTemplate).toHaveBeenCalledWith('template-1', { force: true });
|
||||
expect(result).toEqual({ deleted: true });
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -47,7 +47,7 @@ describe('TemplatesView', () => {
|
||||
templates: {
|
||||
create: vi.fn(),
|
||||
update: vi.fn(),
|
||||
delete: vi.fn(),
|
||||
delete: vi.fn().mockResolvedValue({ deleted: true }),
|
||||
get: vi.fn().mockResolvedValue({ ...mockTemplate }),
|
||||
getAll: vi.fn().mockResolvedValue([]),
|
||||
getEnabledByKind: vi.fn().mockResolvedValue([]),
|
||||
@@ -180,7 +180,7 @@ describe('TemplatesView', () => {
|
||||
});
|
||||
|
||||
it('deletes template and closes tab', async () => {
|
||||
const deleteMock = vi.fn().mockResolvedValue(true);
|
||||
const deleteMock = vi.fn().mockResolvedValue({ deleted: true });
|
||||
(window as any).electronAPI.templates.delete = deleteMock;
|
||||
|
||||
useAppStore.setState({
|
||||
|
||||
@@ -67,7 +67,7 @@ describe('pythonApiContractV1', () => {
|
||||
|
||||
it('contains semantic version metadata for compatibility checks', () => {
|
||||
expect(BDS_PYTHON_API_CONTRACT_V1).toMatchObject({
|
||||
version: '1.8.0',
|
||||
version: '1.9.0',
|
||||
generatedAt: expect.any(String),
|
||||
});
|
||||
});
|
||||
|
||||
+1
-1
@@ -152,7 +152,7 @@ Object.defineProperty(globalThis, 'window', {
|
||||
get: vi.fn().mockResolvedValue(null),
|
||||
create: vi.fn().mockResolvedValue(null),
|
||||
update: vi.fn().mockResolvedValue(null),
|
||||
delete: vi.fn().mockResolvedValue(false),
|
||||
delete: vi.fn().mockResolvedValue({ deleted: true }),
|
||||
validate: vi.fn().mockResolvedValue({ valid: true, errors: [] }),
|
||||
rebuildFromFiles: vi.fn().mockResolvedValue(undefined),
|
||||
},
|
||||
|
||||
Reference in new issue
Block a user