Skip to content

Commit

Permalink
[keyserver] Remove holders in upload deleters
Browse files Browse the repository at this point in the history
Summary:
Address [[ https://linear.app/comm/issue/ENG-9352/delete-holder-when-deleting-upload | ENG-9352 ]].
Before calling `DELETE FROM uploads`, we need to select the `extra` column first and remove blob holders if present in the column JSON.

Depends on D13511

Test Plan:
Used local Blob Service for testing.

For deleteUpload, used the `delete_upload` keyserver endpoint (easy way is canceling media message on web, with blob uploads enabled). Called the endpoint and verified that blob associated with the upload was deleted from DDB.

For deleting unassigned uploads: Manually modified MariaDB by setting containers to null and age to be older than 24h. Modified the cron job call to instead run during keyserver startup. Verified that holders were removed.

Reviewers: tomek, ashoat

Reviewed By: ashoat

Differential Revision: https://phab.comm.dev/D13512
  • Loading branch information
barthap committed Oct 2, 2024
1 parent 7d84edf commit 4f2c8b6
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 5 deletions.
31 changes: 27 additions & 4 deletions keyserver/src/deleters/upload-deleters.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,17 @@
import { ServerError } from 'lib/utils/errors.js';

import { dbQuery, SQL } from '../database/database.js';
import { deleteBlob, removeBlobHolders } from '../services/blob.js';
import type { Viewer } from '../session/viewer.js';
import { blobHoldersFromUploadRows } from '../uploads/media-utils.js';

async function deleteUpload(viewer: Viewer, id: string): Promise<void> {
if (!viewer.loggedIn) {
throw new ServerError('not_logged_in');
}

const fetchQuery = SQL`
SELECT uploader, container, user_container AS userContainer
SELECT uploader, container, user_container AS userContainer, extra
FROM uploads
WHERE id = ${id}
`;
Expand All @@ -21,7 +23,7 @@ async function deleteUpload(viewer: Viewer, id: string): Promise<void> {
throw new ServerError('invalid_parameters');
}
const [row] = result;
const { uploader, container, userContainer } = row;
const { uploader, container, userContainer, extra } = row;

if (
uploader.toString() !== viewer.userID ||
Expand All @@ -31,6 +33,14 @@ async function deleteUpload(viewer: Viewer, id: string): Promise<void> {
throw new ServerError('invalid_parameters');
}

const { blobHash, blobHolder } = JSON.parse(extra);
if (blobHash && blobHolder) {
await deleteBlob({
hash: blobHash,
holder: blobHolder,
});
}

const deleteQuery = SQL`
DELETE u, i
FROM uploads u
Expand All @@ -43,14 +53,27 @@ async function deleteUpload(viewer: Viewer, id: string): Promise<void> {
const maxUnassignedUploadAge = 24 * 60 * 60 * 1000;
async function deleteUnassignedUploads(): Promise<void> {
const oldestUnassignedUploadToKeep = Date.now() - maxUnassignedUploadAge;
await dbQuery(SQL`

const holdersQuery = SQL`
SELECT extra
FROM uploads
WHERE container IS NULL
AND user_container IS NULL
AND creation_time < ${oldestUnassignedUploadToKeep}
`;
const [rows] = await dbQuery(holdersQuery);
const blobHolders = blobHoldersFromUploadRows(rows);
await removeBlobHolders(blobHolders);

const deletionQuery = SQL`
DELETE u, i
FROM uploads u
LEFT JOIN ids i ON i.id = u.id
WHERE u.container IS NULL
AND u.user_container IS NULL
AND creation_time < ${oldestUnassignedUploadToKeep}
`);
`;
await dbQuery(deletionQuery);
}

export { deleteUpload, deleteUnassignedUploads };
20 changes: 19 additions & 1 deletion keyserver/src/uploads/media-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
mediaConfig,
} from 'lib/media/file-utils.js';
import { getImageProcessingPlan } from 'lib/media/image-utils.js';
import type { BlobHashAndHolder } from 'lib/types/holder-types.js';
import type { Dimensions } from 'lib/types/media-types.js';
import { deepFileInfoFromData } from 'web/media/file-utils.js';

Expand Down Expand Up @@ -239,4 +240,21 @@ async function convertImage(
};
}

export { getMediaType, validateAndConvert };
function blobHoldersFromUploadRows(
rows: $ReadOnlyArray<Object>,
): $ReadOnlyArray<BlobHashAndHolder> {
const results = [];
for (const { extra } of rows) {
if (!extra) {
continue;
}
const { blobHash, blobHolder } = JSON.parse(extra);
if (blobHash && blobHolder) {
results.push({ blobHash, holder: blobHolder });
}
}

return results;
}

export { blobHoldersFromUploadRows, getMediaType, validateAndConvert };

0 comments on commit 4f2c8b6

Please sign in to comment.