Skip to content

Conversation

@apsantiso
Copy link
Contributor

@apsantiso apsantiso commented Dec 12, 2025

Adds support for empty files (size = 0) during file creation and replacement. Empty files now have null fileId values instead of requiring a network file identifier, reducing unnecessary network storage usage.

Changes

  • Updated CreateFileDto and ReplaceFileDto to make fileId optional and conditionally required only when size > 0
  • Implemented empty file limit checking via MaxZeroSizeFiles feature limit
  • Updated file creation and replacement logic to set fileId = null when size = 0
  • Modified file replacement lock mechanism to handle empty file scenarios:
    • Uses newFileId when replacing empty file with actual file
    • Uses oldFileId when replacing actual file with empty file
    • Falls back to uuid only when both files are empty. No usage change and no record in usages table is created as we always check if delta = 0 before creating a record in the usages table.

Depends on:
#824

Comment on lines +1194 to +1178
const lockFileId =
newFileData.fileId || oldFileData.fileId || newFileData.uuid;
const lockKey = `file-size-change:${lockFileId}`;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Defensive lock key selection to prevent race conditions during file replacement:

Scenario 1: oldFileId exists, newFileId is null (replacing file with empty file)
- lockKey = oldFileId
- delta = oldSize - 0
- Usage replacement IS created

Scenario 2: oldFileId is null, newFileId is null (replacing empty file with empty file)
- lockKey = uuid (fallback)
- delta = 0 - 0
- NO usage replacement created (safe - no actual usage change)

Scenario 3: oldFileId is null, newFileId exists (replacing empty file with actual file)
- lockKey = newFileId
- delta = 0 - newSize
- Usage replacement IS created

I know it is somehow inconsistent but this is the only way we have (for the time being) to protect us from duplicated usage records when allowing empty files without modifying too much things, let's see if there's a better way when I take the ticket I created today @sg-gs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Scenario 2 is just a fallback as inside the service we always check if delta != 0. We never create a replacement usage record with delta = 0.

@apsantiso apsantiso self-assigned this Dec 12, 2025
@apsantiso apsantiso requested a review from sg-gs December 16, 2025 09:26
Base automatically changed from feat/allow-empty-file-ids-constraint to master December 16, 2025 09:31
@apsantiso apsantiso force-pushed the feat/allow-empty-files branch from 8ec988b to 42027f1 Compare December 16, 2025 09:33
@apsantiso apsantiso added the enhancement New feature or request label Dec 16, 2025
]);

if (
!limit ||
Copy link
Member

Choose a reason for hiding this comment

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

For free users, it would be a good idea to throw a 402 for the frontend (upgrade to get more features)

@larryrider
Copy link
Contributor

Hey @apsantiso! Shouldn't the fileId from file.dto be modified as string | undefined too? 🤔
For what I see it should be:

  • file.dto
  • create-file.dto (already modified)
  • replace-file.dto (already modified)

@apsantiso
Copy link
Contributor Author

apsantiso commented Dec 16, 2025

Hey @apsantiso! Shouldn't the fileId from file.dto be modified as string | undefined too? 🤔 For what I see it should be:

  • file.dto
  • create-file.dto (already modified)
  • replace-file.dto (already modified)

True, modified @larryrider

@apsantiso apsantiso force-pushed the feat/allow-empty-files branch from 15621b6 to b57b8f8 Compare December 16, 2025 12:17
@sonarqubecloud
Copy link

@apsantiso apsantiso requested a review from sg-gs December 16, 2025 13:16
@apsantiso apsantiso merged commit 169da17 into master Dec 16, 2025
13 checks passed
@apsantiso apsantiso deleted the feat/allow-empty-files branch December 16, 2025 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request ready-for-preview

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants