-
Notifications
You must be signed in to change notification settings - Fork 2
[PB-6472]: feat/track-user-mail-usage #220
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
base: feat/mail-bucket-provisioning
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,8 @@ | ||
| import { Request, Response } from 'express'; | ||
| import { Logger } from 'winston'; | ||
| import { EmailIsAlreadyInUseError, InvalidDataFormatError, UserAlreadyExistsError, UserNotFoundError, UsersUsecase } from '../../../core'; | ||
| import { EmailIsAlreadyInUseError, InvalidDataFormatError, UserAlreadyExistsError, UserNotFoundError, UserSpaceSnapshot, UsersUsecase } from '../../../core'; | ||
| import { BucketEntriesUsecase } from '../../../core/bucketEntries/usecase'; | ||
| import { BucketNotFoundError } from '../../../core/buckets/usecase'; | ||
|
|
||
| import { GatewayUsecase } from '../../../core/gateway/Usecase'; | ||
| import { EventBus, EventBusEvents, UserStorageChangedPayload } from '../../eventBus'; | ||
|
|
@@ -16,6 +17,13 @@ type DeleteFilesInBulkResponse = { | |
| type CreateBucketBody = { name: string }; | ||
| type CreateBucketResponse = { id: string; name: string }; | ||
|
|
||
| type SetBucketUsageBody = { usedSpaceBytes: number }; | ||
|
|
||
| const OBJECT_ID_PATTERN = /^[a-f0-9]{24}$/i; | ||
|
|
||
| const isValidUsedSpaceBytes = (value: unknown): value is number => | ||
| typeof value === 'number' && Number.isFinite(value) && value >= 0; | ||
|
|
||
| export class HTTPGatewayController { | ||
| constructor( | ||
| private gatewayUsecase: GatewayUsecase, | ||
|
|
@@ -165,6 +173,44 @@ export class HTTPGatewayController { | |
| } | ||
| } | ||
|
|
||
| async setBucketUsage( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I do not know if it fits you but, isn't it better to set this on any upload so when you use the upload endpoint for mail, the computation for that bucket is set?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. similar to the above comment #220 (comment), mail doesnt go through the normal upload path |
||
| req: Request<{ uuid: string; id: string }, {}, Partial<SetBucketUsageBody>, {}>, | ||
| res: Response<UserSpaceSnapshot | { message: string }> | ||
| ) { | ||
| const { uuid, id } = req.params; | ||
| const { usedSpaceBytes } = req.body; | ||
|
|
||
| if (!uuid || !id || !OBJECT_ID_PATTERN.test(id)) { | ||
| return res.status(400).send({ message: 'Invalid params' }); | ||
| } | ||
|
|
||
| if (!isValidUsedSpaceBytes(usedSpaceBytes)) { | ||
| return res | ||
| .status(400) | ||
| .send({ message: 'usedSpaceBytes must be a non-negative number' }); | ||
| } | ||
|
|
||
| try { | ||
| const snapshot = await this.usersUsecase.setBucketUsage(uuid, id, usedSpaceBytes); | ||
|
|
||
| return res.status(200).send(snapshot); | ||
| } catch (err) { | ||
| if (err instanceof UserNotFoundError || err instanceof BucketNotFoundError) { | ||
| return res.status(404).send({ message: err.message }); | ||
| } | ||
|
|
||
| this.logger.error( | ||
| '[GATEWAY/BUCKET_USAGE] Error setting usage for bucket %s of user %s: %s. %s', | ||
| id, | ||
| uuid, | ||
| (err as Error).message, | ||
| (err as Error).stack || 'NO STACK' | ||
| ); | ||
|
|
||
| return res.status(500).send({ message: 'Internal server error' }); | ||
| } | ||
| } | ||
|
|
||
| async deleteUserBucket( | ||
| req: Request<{ uuid: string; id: string }>, | ||
| res: Response<{ message: string }> | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why? The user's
totalUsedSpaceBytesalready includes thebucketsUsedSpaceBytesUh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's the main issue here. The network needs to know how much space is consumed by mail. However mail blobs (bodies and attachments) are managed and stored by stalwart that upload path does not come through the network. I though about leveraging sieve scripts, event hooks or similar to also store the metadata on here, though that would create the problem of mantaining both
as it stands mail just reports the its usage which would not be part of totalUsedSpaceBytes as its contents did not go through the normal upload flow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We cannot allow people to use more space than paid, that's against the business costs to operate. We bill for the space they use. If we are not controlling that at all, we have a operational issue. The totalUsage of the user should be the real total. In fact, if that is not respected, Drive may consume more space than it should, which is again, a hidden overcost