Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 66 additions & 0 deletions src/commands/delete.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
*/

import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import type { Config } from '@oclif/core';
import { DeleteCommand } from './delete';
import { FirestoreOps } from '../domain/firestore-ops';
import { BatchProcessor } from '../domain/batch-processor';
Expand Down Expand Up @@ -85,6 +86,71 @@ describe('DeleteCommand', () => {
const batchProcessor = new BatchProcessor(mockFirestore as any);
expect(batchProcessor.deleteCollection).toBeDefined();
});

it('should support recursive document deletion via BatchProcessor', () => {
const batchProcessor = new BatchProcessor(mockFirestore as any);
expect(batchProcessor.deleteDocument).toBeDefined();
});

it('should classify a document path as recursive-eligible (no collection-only restriction)', () => {
const firestoreOps = new FirestoreOps(mockFirestore as any);
const isDocument = firestoreOps.isDocumentPath('users/user123');
const isCollection = firestoreOps.isCollectionPath('users/user123');

// The old validation rejected `recursive && !isCollection`. A document
// path must remain a valid target for --recursive.
expect(isDocument).toBe(true);
expect(isCollection).toBe(false);
});

it('should route document path + --recursive to BatchProcessor.deleteDocument (recursiveDelete equivalent)', async () => {
const mockDocRef = {
delete: vi.fn().mockResolvedValue(undefined),
listCollections: vi.fn().mockResolvedValue([]),
};
mockFirestore.doc.mockReturnValue(mockDocRef);

const deleteDocumentSpy = vi.spyOn(BatchProcessor.prototype, 'deleteDocument');

const command = new DeleteCommand([], {} as Config);
const promptService = new PromptService();

await (command as any).deleteDocumentRecursive(
mockFirestore,
'users/user123',
true,
promptService
);

// The old CLI-side restriction ("--recursive can only be used with
// collection paths") is gone: a document path + --recursive now
// reaches BatchProcessor.deleteDocument, which deletes the document
// reference and all of its subcollections.
expect(deleteDocumentSpy).toHaveBeenCalledWith('users/user123', expect.any(Function));
expect(mockDocRef.delete).toHaveBeenCalled();

deleteDocumentSpy.mockRestore();
});

it('should still route collection path + --recursive to BatchProcessor.deleteCollection (regression)', async () => {
const mockCollectionRef = {
limit: vi.fn().mockReturnValue({
get: vi.fn().mockResolvedValue({ empty: true, size: 0, docs: [] }),
}),
};
mockFirestore.collection.mockReturnValue(mockCollectionRef);

const deleteCollectionSpy = vi.spyOn(BatchProcessor.prototype, 'deleteCollection');

const command = new DeleteCommand([], {} as Config);
const promptService = new PromptService();

await (command as any).deleteCollection(mockFirestore, 'users', true, promptService);

expect(deleteCollectionSpy).toHaveBeenCalledWith('users', expect.any(Function));

deleteCollectionSpy.mockRestore();
});
});

describe('confirmation prompt', () => {
Expand Down
62 changes: 52 additions & 10 deletions src/commands/delete.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ export class DeleteCommand extends BaseCommand {
command: '<%= config.bin %> delete users/user123/orders --recursive --yes',
description: 'Delete all documents in a subcollection',
},
{
command: '<%= config.bin %> delete users/user123 --recursive --yes',
description: 'Delete a document and all its subcollections',
},
];

static override args = {
Expand Down Expand Up @@ -85,17 +89,14 @@ export class DeleteCommand extends BaseCommand {
const isDocument = firestoreOps.isDocumentPath(path);
const isCollection = firestoreOps.isCollectionPath(path);

if (recursive && !isCollection) {
this.handleError(
'--recursive can only be used with collection paths',
'validation'
);
return;
}

if (isDocument) {
// Single document deletion
await this.deleteDocument(firestoreOps, path, skipConfirm, promptService);
if (recursive) {
// Recursive document deletion (document + subcollections)
await this.deleteDocumentRecursive(firestore, path, skipConfirm, promptService);
} else {
// Single document deletion
await this.deleteDocument(firestoreOps, path, skipConfirm, promptService);
}
} else if (isCollection) {
if (recursive) {
// Recursive collection deletion
Expand Down Expand Up @@ -187,4 +188,45 @@ export class DeleteCommand extends BaseCommand {
console.log(`${result.value.deletedCount} ${t('msg.documentsDeleted')}`);
}
}

/**
* Delete a document and its subcollections recursively
*/
private async deleteDocumentRecursive(
firestore: any,
path: string,
skipConfirm: boolean,
promptService: PromptService
): Promise<void> {
const batchProcessor = new BatchProcessor(firestore);

const confirmCallback = async (): Promise<boolean> => {
if (skipConfirm) {
return true;
}

const confirmResult = await promptService.confirm(
`${t('prompt.confirmDelete')} "${path}" (${t('prompt.confirmDeleteDocumentRecursive')})`
);

if (confirmResult.isErr()) {
return false;
}

return confirmResult.value;
};

const result = await batchProcessor.deleteDocument(path, confirmCallback);

if (result.isErr()) {
this.handleError(result.error.message, 'firestore');
return;
}

if (result.value.deletedCount === 0) {
console.log('Cancelled');
} else {
console.log(`${result.value.deletedCount} ${t('msg.documentsDeleted')}`);
}
}
}
48 changes: 48 additions & 0 deletions src/domain/batch-processor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ describe('BatchProcessor', () => {
const mockDocRef: any = {
path: 'users/user1',
listCollections: vi.fn().mockResolvedValue([]),
delete: vi.fn().mockResolvedValue(undefined),
};

mockDocRef.get = vi.fn().mockResolvedValue({
Expand Down Expand Up @@ -322,6 +323,53 @@ describe('BatchProcessor', () => {
});
});

describe('deleteDocument', () => {
it('should delete a document with no subcollections', async () => {
const confirmCallback = vi.fn().mockResolvedValue(true);

const result = await batchProcessor.deleteDocument('users/user1', confirmCallback);

expect(result.isOk()).toBe(true);
if (result.isOk()) {
expect(result.value.deletedCount).toBe(1);
}
expect(confirmCallback).toHaveBeenCalled();
expect(mockFirestore.doc).toHaveBeenCalledWith('users/user1');
const docRef = vi.mocked(mockFirestore.doc).mock.results[0].value;
expect(docRef.delete).toHaveBeenCalled();
});

it('should delete a document and its subcollections recursively', async () => {
const confirmCallback = vi.fn().mockResolvedValue(true);
const docRef = vi.mocked(mockFirestore.doc)();
vi.mocked(docRef.listCollections).mockResolvedValue([
{ path: 'users/user1/orders' } as never,
]);

const result = await batchProcessor.deleteDocument('users/user1', confirmCallback);

expect(result.isOk()).toBe(true);
if (result.isOk()) {
// 1 for the document itself + whatever the subcollection delete reported
expect(result.value.deletedCount).toBeGreaterThanOrEqual(1);
}
expect(docRef.delete).toHaveBeenCalled();
});

it('should not delete if confirmation is denied', async () => {
const confirmCallback = vi.fn().mockResolvedValue(false);
const docRef = vi.mocked(mockFirestore.doc)();

const result = await batchProcessor.deleteDocument('users/user1', confirmCallback);

expect(result.isOk()).toBe(true);
if (result.isOk()) {
expect(result.value.deletedCount).toBe(0);
}
expect(docRef.delete).not.toHaveBeenCalled();
});
});

describe('batch operations', () => {
it('should process in batches of specified size', async () => {
// Create a large dataset file
Expand Down
46 changes: 46 additions & 0 deletions src/domain/batch-processor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -344,4 +344,50 @@ export class BatchProcessor {
});
}
}

/**
* Delete a document and all its subcollections recursively
*/
async deleteDocument(
path: string,
confirmCallback: () => Promise<boolean>
): Promise<Result<DeleteResult, BatchProcessorError>> {
try {
// Ask for confirmation
const confirmed = await confirmCallback();
if (!confirmed) {
return ok({ deletedCount: 0 });
}

let deletedCount = 0;
const docRef = this.firestore.doc(path);

// Recursively delete subcollections
const subcollections = await docRef.listCollections();
for (const subcol of subcollections) {
const subResult = await this.deleteCollection(
subcol.path,
async () => true // Auto-confirm for subcollections
);

if (subResult.isOk()) {
deletedCount += subResult.value.deletedCount;
}
}

// Delete the document itself
await docRef.delete();
deletedCount += 1;

return ok({ deletedCount });
} catch (error) {
return err({
type: 'FIRESTORE_ERROR',
message: `ドキュメントの削除に失敗しました: ${
error instanceof Error ? error.message : String(error)
}`,
originalError: error instanceof Error ? error : new Error(String(error)),
});
}
}
}
3 changes: 3 additions & 0 deletions src/shared/i18n.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ export interface Messages {
// Prompts
'prompt.confirmDelete': string;
'prompt.confirmDeleteCollection': string;
'prompt.confirmDeleteDocumentRecursive': string;

// Export/Import runtime messages
'msg.exporting': string;
Expand Down Expand Up @@ -382,6 +383,7 @@ const jaMessages: Messages = {
// Prompts
'prompt.confirmDelete': 'を削除しますか?',
'prompt.confirmDeleteCollection': 'コレクション内の全ドキュメントを削除しますか?',
'prompt.confirmDeleteDocumentRecursive': 'サブコレクションを含めて削除しますか?',

// Export/Import runtime messages
'msg.exporting': 'コレクションをエクスポートしています',
Expand Down Expand Up @@ -683,6 +685,7 @@ const enMessages: Messages = {
// Prompts
'prompt.confirmDelete': 'Are you sure you want to delete',
'prompt.confirmDeleteCollection': 'Delete all documents in collection?',
'prompt.confirmDeleteDocumentRecursive': 'including all subcollections?',

// Export/Import runtime messages
'msg.exporting': 'Exporting collection',
Expand Down
Loading