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
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { TemporalFileDeleter } from '../../../../../context/storage/TemporalFile
import { TemporalFile } from '../../../../../context/storage/TemporalFiles/domain/TemporalFile';
import { FirstsFileSearcher } from '../../../../../context/virtual-drive/files/application/search/FirstsFileSearcher';
import { File, FileAttributes } from '../../../../../context/virtual-drive/files/domain/File';
import { ContentsId } from '../../../../../apps/main/database/entities/DriveFile';
import { FileStatuses } from '../../../../../context/virtual-drive/files/domain/FileStatus';
import { FuseCodes } from '../../../../../apps/drive/fuse/callbacks/FuseCodes';
import { call, calls } from '../../../../../../tests/vitest/utils.helper';
Expand Down Expand Up @@ -78,7 +79,7 @@ describe('release', () => {
const temporalFile = createTemporalFile('/Documents/report.pdf');
finder.run.mockResolvedValue(temporalFile);
fileSearcher.run.mockResolvedValue(undefined);
uploader.run.mockResolvedValue('contents-id-123');
uploader.run.mockResolvedValue('contents-id-123' as ContentsId);

const { data, error } = await release({ path: '/Documents/report.pdf', processName: 'cat', container });

Expand All @@ -92,7 +93,7 @@ describe('release', () => {
const existingFile = File.from(fileAttrs);
finder.run.mockResolvedValue(temporalFile);
fileSearcher.run.mockResolvedValue(existingFile);
uploader.run.mockResolvedValue('new-contents-id');
uploader.run.mockResolvedValue('new-contents-id' as ContentsId);

const { data, error } = await release({ path: '/Documents/report.pdf', processName: 'cat', container });

Expand All @@ -114,6 +115,37 @@ describe('release', () => {
expect(error?.code).toBe(FuseCodes.EIO);
call(deleter.run).toStrictEqual('/Documents/report.pdf');
});

it('should skip the second release when an upload for the same path is already in progress', async () => {
const temporalFile = createTemporalFile('/Documents/report.pdf');
finder.run.mockResolvedValue(temporalFile);
fileSearcher.run.mockResolvedValue(undefined);

// First release never resolves during the test — simulates a long in-progress upload
let resolveFirstUpload!: () => void;
uploader.run.mockReturnValue(
new Promise<ContentsId>((resolve) => {
resolveFirstUpload = () => resolve('contents-id-123' as ContentsId);
}),
);

const first = release({ path: '/Documents/report.pdf', processName: 'proc-a', container });

await Promise.resolve();

const { data: data2, error: error2 } = await release({
path: '/Documents/report.pdf',
processName: 'proc-b',
container,
});

expect(error2).toBeUndefined();
expect(data2).toBeUndefined();
calls(uploader.run).toHaveLength(1);

resolveFirstUpload();
await first;
Comment on lines +146 to +147
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need this on the test?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is intended to clear the set that stores uploads in progress, so they don't remain there and interfere with other tests.

});
});

describe('when finder throws an unexpected error', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,22 @@ import { TemporalFileUploader } from '../../../../../context/storage/TemporalFil
import { TemporalFileDeleter } from '../../../../../context/storage/TemporalFiles/application/deletion/TemporalFileDeleter';
import { FirstsFileSearcher } from '../../../../../context/virtual-drive/files/application/search/FirstsFileSearcher';
import { FileStatuses } from '../../../../../context/virtual-drive/files/domain/FileStatus';

type Props = {
path: string;
processName: string;
container: Container;
};

// v.2.6.0
// Esteban Galvis Triana
// For files with unusual extensions or when the system has to figure
// out which app to use to open them—two file descriptors end up being created:
// one for metadata and one for the actual content.
// The issue is that when each descriptor closes, it triggers a release,
// resulting in a duplicate request to create the file remotely.
const uploadsInProgress = new Set<string>();

export async function release({ path, processName, container }: Props): Promise<Result<void, FuseError>> {
try {
const temporalFile = await container.get(TemporalFileByPathFinder).run(path);
Expand All @@ -27,19 +38,27 @@ export async function release({ path, processName, container }: Props): Promise<
return { data: undefined };
}

const existingFile = await container.get(FirstsFileSearcher).run({ path, status: FileStatuses.EXISTS });
const replaces = existingFile
? { contentsId: existingFile.contentsId, name: existingFile.name, extension: existingFile.type }
: undefined;
if (uploadsInProgress.has(path)) {
logger.debug({ msg: '[Release] Upload already in progress, skipping duplicate release', path, processName });
return { data: undefined };
}
uploadsInProgress.add(path);

try {
const existingFile = await container.get(FirstsFileSearcher).run({ path, status: FileStatuses.EXISTS });
const replaces = existingFile
? { contentsId: existingFile.contentsId, name: existingFile.name, extension: existingFile.type }
: undefined;

await container.get(TemporalFileUploader).run(temporalFile, replaces);
logger.debug({ msg: '[Release] Temporal file uploaded', path, processName });
return { data: undefined };
} catch (uploadError) {
logger.error({ msg: '[Release] Upload failed, deleting temporal file', error: uploadError, path, processName });
await container.get(TemporalFileDeleter).run(path);
return { error: new FuseIOError('Upload failed due to insufficient storage or network issues.') };
} finally {
uploadsInProgress.delete(path);
}
} catch (err: unknown) {
logger.error({ msg: '[Release] Unexpected error', error: err, path, processName });
Expand Down
Loading