Skip to content

Limit batch size for attachments streaming to 50 #37

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 23, 2025
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
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@devrev/ts-adaas",
"version": "1.4.1",
"version": "1.4.2",
"description": "Typescript library containing the ADaaS(AirDrop as a Service) control protocol.",
"type": "commonjs",
"main": "./dist/index.js",
Expand Down
138 changes: 87 additions & 51 deletions src/workers/worker-adapter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -173,50 +173,6 @@ describe('WorkerAdapter', () => {
expect(result[0]).toHaveLength(1);
expect(result[1]).toHaveLength(1);
});

it('should handle invalid (0) batch size', async () => {
// Arrange
const mockStream = jest.fn();
const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation();

// Act
const result = await adapter.streamAttachments({
stream: mockStream,
batchSize: 0,
});

// Assert
expect(consoleErrorSpy).toHaveBeenCalled();
expect(result).toEqual({
error: expect.any(Error),
});
expect(result?.error?.message).toContain('Invalid attachments batch size');

// Restore console.error
consoleErrorSpy.mockRestore();
});

it('should handle invalid (negative) batch size', async () => {
// Arrange
const mockStream = jest.fn();
const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation();

// Act
const result = await adapter.streamAttachments({
stream: mockStream,
batchSize: -1,
});

// Assert
expect(consoleErrorSpy).toHaveBeenCalled();
expect(result).toEqual({
error: expect.any(Error),
});
expect(result?.error?.message).toContain('Invalid attachments batch size');

// Restore console.error
consoleErrorSpy.mockRestore();
});
});

describe('defaultAttachmentsIterator', () => {
Expand Down Expand Up @@ -480,7 +436,29 @@ describe('WorkerAdapter', () => {
it('should handle invalid batch size', async () => {
// Arrange
const mockStream = jest.fn();
const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation();
const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation();

// Set up adapter state with artifact IDs
adapter.state.toDevRev = {
attachmentsMetadata: {
artifactIds: ['artifact1'],
lastProcessed: 0,
lastProcessedAttachmentsIdsList: [],
},
};

// Mock getting attachments
adapter['uploader'].getAttachmentsFromArtifactId = jest.fn().mockResolvedValue({
attachments: [
{ url: 'http://example.com/file1.pdf', id: 'attachment1', file_name: 'file1.pdf', parent_id: 'parent1' },
],
});

// Mock the required methods
adapter.initializeRepos = jest.fn();
const mockReducedAttachments = [['batch1']];
adapter['defaultAttachmentsReducer'] = jest.fn().mockReturnValue(mockReducedAttachments);
adapter['defaultAttachmentsIterator'] = jest.fn().mockResolvedValue({});

// Act
const result = await adapter.streamAttachments({
Expand All @@ -489,14 +467,72 @@ describe('WorkerAdapter', () => {
});

// Assert
expect(consoleErrorSpy).toHaveBeenCalled();
expect(result).toEqual({
error: expect.any(Error),
expect(consoleWarnSpy).toHaveBeenCalledWith(
'The specified batch size (0) is invalid. Using 1 instead.'
);

// Verify that the reducer was called with batchSize 50 (not 100)
expect(adapter['defaultAttachmentsReducer']).toHaveBeenCalledWith({
attachments: expect.any(Array),
adapter: adapter,
batchSize: 1,
});
expect(result?.error?.message).toContain('Invalid attachments batch size');

// Restore console.error
consoleErrorSpy.mockRestore();
expect(result).toBeUndefined();

// Restore console.warn
consoleWarnSpy.mockRestore();
});

it('should cap batch size to 50 when batchSize is greater than 50', async () => {
// Arrange
const mockStream = jest.fn();
const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation();

// Set up adapter state with artifact IDs
adapter.state.toDevRev = {
attachmentsMetadata: {
artifactIds: ['artifact1'],
lastProcessed: 0,
lastProcessedAttachmentsIdsList: [],
},
};

// Mock getting attachments
adapter['uploader'].getAttachmentsFromArtifactId = jest.fn().mockResolvedValue({
attachments: [
{ url: 'http://example.com/file1.pdf', id: 'attachment1', file_name: 'file1.pdf', parent_id: 'parent1' },
],
});

// Mock the required methods
adapter.initializeRepos = jest.fn();
const mockReducedAttachments = [['batch1']];
adapter['defaultAttachmentsReducer'] = jest.fn().mockReturnValue(mockReducedAttachments);
adapter['defaultAttachmentsIterator'] = jest.fn().mockResolvedValue({});

// Act
const result = await adapter.streamAttachments({
stream: mockStream,
batchSize: 100, // Set batch size greater than 50
});

// Assert
expect(consoleWarnSpy).toHaveBeenCalledWith(
'The specified batch size (100) is too large. Using 50 instead.'
);

// Verify that the reducer was called with batchSize 50 (not 100)
expect(adapter['defaultAttachmentsReducer']).toHaveBeenCalledWith({
attachments: expect.any(Array),
adapter: adapter,
batchSize: 50, // Should be capped at 50
});

expect(result).toBeUndefined();

// Restore console.warn
consoleWarnSpy.mockRestore();
});

it('should handle empty attachments metadata artifact IDs', async () => {
Expand Down
12 changes: 7 additions & 5 deletions src/workers/worker-adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -952,11 +952,13 @@ export class WorkerAdapter<ConnectorState> {
batchSize?: number;
}): Promise<StreamAttachmentsReturnType> {
if (batchSize <= 0) {
const error = new Error(
`Invalid attachments batch size: ${batchSize}. Batch size must be greater than 0.`
);
console.error(error.message);
return { error };
console.warn(`The specified batch size (${batchSize}) is invalid. Using 1 instead.`);
batchSize = 1;
}

if (batchSize > 50) {
console.warn(`The specified batch size (${batchSize}) is too large. Using 50 instead.`);
batchSize = 50;
}

const repos = [
Expand Down