[services] Backup - Blob Put Client - Update put API
ClosedPublic
Actions

services/backup/blob_client/src/lib.rs
13 ↗	(On Diff #16164)	Do you think this comment is useful? For me, if names start with `put` it is obvious that these are a part of `put`, but feel free to keep it if it's useful for you.
15 ↗	(On Diff #16164)	It would be great if we could avoid having `unsafe` at function level, so that safe code could call it. I prefer having `unsafe` sections as small as possible, so maybe we can have an unsafe block in the function instead of the whole unsafe function?

This revision is now accepted and ready to land.Sep 5 2022, 2:13 AM

• karol added inline comments.Sep 5 2022, 5:59 AM

services/backup/blob_client/src/lib.rs
13 ↗	(On Diff #16164)	For me, it looks better once there's also the `get` section, but I can remove this, not a problem.
15 ↗	(On Diff #16164)	Rust complains: pointer argument requires that the function be marked unsafe I assume we have to mark these functions `unsafe`. I don't fully understand your concern, they're about to be called from c++ exclusively and I didn't notice any impact of `unsafe` on how we call them in c++ really. And I don't think we'll want to use them from rust.

update comment

remove comment

Harbormaster completed remote builds in B11905: Diff 16348.Sep 6 2022, 1:28 AM

Harbormaster completed remote builds in B11906: Diff 16349.

tomek added inline comments.Sep 6 2022, 6:40 AM

services/backup/blob_client/src/lib.rs
15 ↗	(On Diff #16164)	Ok, that surprising. I thought that only dereferencing a raw pointer is unsafe. https://doc.rust-lang.org/book/ch19-01-unsafe-rust.html#unsafe-superpowers So maybe what happens here is that Rust automatically dereferences a raw pointer when it is a function argument... I don't fully understand your concern, they're about to be called from c++ exclusively and I didn't notice any impact of unsafe on how we call them in c++ really. The issue is not on C++ side but on the Rust one. If we mark the function as unsafe, we can use unsafe operations in the whole function. According to the book: People are fallible, and mistakes will happen, but by requiring these five unsafe operations to be inside blocks annotated with unsafe you’ll know that any errors related to memory safety must be within an unsafe block. Keep unsafe blocks small; you’ll be thankful later when you investigate memory bugs. It is a good practice to keep unsafe blocks as small as possible.

tomek mentioned this in D5009: [services] Backup - Blob Put Client - Add initialize.Sep 6 2022, 6:51 AM

• karol added inline comments.Sep 8 2022, 1:14 AM

services/backup/blob_client/src/lib.rs
15 ↗	(On Diff #16164)	Discussed offline. I understand that it's better to make `unsafe` areas as small as possible but here we have no choice, I'm afraid.

rebase

Harbormaster completed remote builds in B11981: Diff 16436.Sep 8 2022, 4:06 AM

Closed by commit rCOMM0e63f2b49414: [services] Backup - Blob Put Client - Update put API (authored by • karol). · Explain WhySep 8 2022, 4:10 AM

This revision was automatically updated to reflect the committed changes.

• karol added a commit: rCOMM0e63f2b49414: [services] Backup - Blob Put Client - Update put API.

Revision Contents
Changeset List

Path

Size

services/

backup/

blob_client/

src/

lib.rs

11 lines

put_client.rs

17 lines

Diff 16349

View Options

services/backup/blob_client/src/lib.rs

Show All 26 Lines
import type { UpdatesForCurrentSession } from './update-creator.js';		import type { UpdatesForCurrentSession } from './update-creator.js';
import { createUpdates } from './update-creator.js';		import { createUpdates } from './update-creator.js';
import {		import {
dbQuery,		dbQuery,
SQL,		SQL,
appendSQLArray,		appendSQLArray,
mergeOrConditions,		mergeOrConditions,
} from '../database/database.js';		} from '../database/database.js';
		import { processMessagesForSearch } from '../database/search-utils.js';
import {		import {
fetchMessageInfoForLocalID,		fetchMessageInfoForLocalID,
fetchMessageInfoByID,		fetchMessageInfoByID,
} from '../fetchers/message-fetchers.js';		} from '../fetchers/message-fetchers.js';
import { fetchOtherSessionsForViewer } from '../fetchers/session-fetchers.js';		import { fetchOtherSessionsForViewer } from '../fetchers/session-fetchers.js';
import { fetchServerThreadInfos } from '../fetchers/thread-fetchers.js';		import { fetchServerThreadInfos } from '../fetchers/thread-fetchers.js';
import { sendPushNotifs, sendRescindNotifs } from '../push/send.js';		import { sendPushNotifs, sendRescindNotifs } from '../push/send.js';
import { handleAsyncPromise } from '../responders/handlers.js';		import { handleAsyncPromise } from '../responders/handlers.js';
▲ Show 20 Lines • Show All 240 Lines • ▼ Show 20 Lines	if (updatedThreads.length > 0) {
await createUpdates(updates);		await createUpdates(updates);
}		}
}		}

// Handles:		// Handles:
// (1) Sending push notifs		// (1) Sending push notifs
// (2) Setting threads to unread and generating corresponding UpdateInfos		// (2) Setting threads to unread and generating corresponding UpdateInfos
// (3) Publishing to Redis so that active sockets pass on new messages		// (3) Publishing to Redis so that active sockets pass on new messages
		// (4) Processing messages for search
async function postMessageSend(		async function postMessageSend(
viewer: Viewer,		viewer: Viewer,
threadsToMessageIndices: Map<string, number[]>,		threadsToMessageIndices: Map<string, number[]>,
subthreadPermissionsToCheck: Set<string>,		subthreadPermissionsToCheck: Set<string>,
messageInfos: RawMessageInfo[],		messageInfos: RawMessageInfo[],
messageDatas: MessageData[],		messageDatas: MessageData[],
updatesForCurrentSession: UpdatesForCurrentSession,		updatesForCurrentSession: UpdatesForCurrentSession,
) {		) {
		const processForSearch = processMessagesForSearch(messageInfos);

let joinIndex = 0;		let joinIndex = 0;
let subthreadSelects = '';		let subthreadSelects = '';
const subthreadJoins = [];		const subthreadJoins = [];
for (const subthread of subthreadPermissionsToCheck) {		for (const subthread of subthreadPermissionsToCheck) {
const index = joinIndex++;		const index = joinIndex++;
subthreadSelects += `		subthreadSelects += `
,		,
stm${index}.permissions AS subthread${subthread}_permissions,		stm${index}.permissions AS subthread${subthread}_permissions,
▲ Show 20 Lines • Show All 186 Lines • ▼ Show 20 Lines	) {
const latestMessages = flattenLatestMessagesPerUser(latestMessagesPerUser);		const latestMessages = flattenLatestMessagesPerUser(latestMessagesPerUser);

const [pushInfo, rescindInfo] = await Promise.all([		const [pushInfo, rescindInfo] = await Promise.all([
promiseAll(userPushInfoPromises),		promiseAll(userPushInfoPromises),
promiseAll(userRescindInfoPromises),		promiseAll(userRescindInfoPromises),
createReadStatusUpdates(latestMessages),		createReadStatusUpdates(latestMessages),
redisPublish(viewer, messageInfosPerUser, updatesForCurrentSession),		redisPublish(viewer, messageInfosPerUser, updatesForCurrentSession),
updateLatestMessages(latestMessages),		updateLatestMessages(latestMessages),
		processForSearch,
]);		]);

await Promise.all([		await Promise.all([
sendPushNotifs(_pickBy(Boolean)(pushInfo)),		sendPushNotifs(_pickBy(Boolean)(pushInfo)),
sendRescindNotifs(_pickBy(Boolean)(rescindInfo)),		sendRescindNotifs(_pickBy(Boolean)(rescindInfo)),
]);		]);
}		}

▲ Show 20 Lines • Show All 174 Lines • Show Last 20 Lines

View Options

services/backup/blob_client/src/put_client.rs

Show All 26 Lines
import type { UpdatesForCurrentSession } from './update-creator.js';		import type { UpdatesForCurrentSession } from './update-creator.js';
import { createUpdates } from './update-creator.js';		import { createUpdates } from './update-creator.js';
import {		import {
dbQuery,		dbQuery,
SQL,		SQL,
appendSQLArray,		appendSQLArray,
mergeOrConditions,		mergeOrConditions,
} from '../database/database.js';		} from '../database/database.js';
		import { processMessagesForSearch } from '../database/search-utils.js';
import {		import {
fetchMessageInfoForLocalID,		fetchMessageInfoForLocalID,
fetchMessageInfoByID,		fetchMessageInfoByID,
} from '../fetchers/message-fetchers.js';		} from '../fetchers/message-fetchers.js';
import { fetchOtherSessionsForViewer } from '../fetchers/session-fetchers.js';		import { fetchOtherSessionsForViewer } from '../fetchers/session-fetchers.js';
import { fetchServerThreadInfos } from '../fetchers/thread-fetchers.js';		import { fetchServerThreadInfos } from '../fetchers/thread-fetchers.js';
import { sendPushNotifs, sendRescindNotifs } from '../push/send.js';		import { sendPushNotifs, sendRescindNotifs } from '../push/send.js';
import { handleAsyncPromise } from '../responders/handlers.js';		import { handleAsyncPromise } from '../responders/handlers.js';
▲ Show 20 Lines • Show All 240 Lines • ▼ Show 20 Lines	if (updatedThreads.length > 0) {
await createUpdates(updates);		await createUpdates(updates);
}		}
}		}

// Handles:		// Handles:
// (1) Sending push notifs		// (1) Sending push notifs
// (2) Setting threads to unread and generating corresponding UpdateInfos		// (2) Setting threads to unread and generating corresponding UpdateInfos
// (3) Publishing to Redis so that active sockets pass on new messages		// (3) Publishing to Redis so that active sockets pass on new messages
		// (4) Processing messages for search
async function postMessageSend(		async function postMessageSend(
viewer: Viewer,		viewer: Viewer,
threadsToMessageIndices: Map<string, number[]>,		threadsToMessageIndices: Map<string, number[]>,
subthreadPermissionsToCheck: Set<string>,		subthreadPermissionsToCheck: Set<string>,
messageInfos: RawMessageInfo[],		messageInfos: RawMessageInfo[],
messageDatas: MessageData[],		messageDatas: MessageData[],
updatesForCurrentSession: UpdatesForCurrentSession,		updatesForCurrentSession: UpdatesForCurrentSession,
) {		) {
		const processForSearch = processMessagesForSearch(messageInfos);

let joinIndex = 0;		let joinIndex = 0;
let subthreadSelects = '';		let subthreadSelects = '';
const subthreadJoins = [];		const subthreadJoins = [];
for (const subthread of subthreadPermissionsToCheck) {		for (const subthread of subthreadPermissionsToCheck) {
const index = joinIndex++;		const index = joinIndex++;
subthreadSelects += `		subthreadSelects += `
,		,
stm${index}.permissions AS subthread${subthread}_permissions,		stm${index}.permissions AS subthread${subthread}_permissions,
▲ Show 20 Lines • Show All 186 Lines • ▼ Show 20 Lines	) {
const latestMessages = flattenLatestMessagesPerUser(latestMessagesPerUser);		const latestMessages = flattenLatestMessagesPerUser(latestMessagesPerUser);

const [pushInfo, rescindInfo] = await Promise.all([		const [pushInfo, rescindInfo] = await Promise.all([
promiseAll(userPushInfoPromises),		promiseAll(userPushInfoPromises),
promiseAll(userRescindInfoPromises),		promiseAll(userRescindInfoPromises),
createReadStatusUpdates(latestMessages),		createReadStatusUpdates(latestMessages),
redisPublish(viewer, messageInfosPerUser, updatesForCurrentSession),		redisPublish(viewer, messageInfosPerUser, updatesForCurrentSession),
updateLatestMessages(latestMessages),		updateLatestMessages(latestMessages),
		processForSearch,
]);		]);

await Promise.all([		await Promise.all([
sendPushNotifs(_pickBy(Boolean)(pushInfo)),		sendPushNotifs(_pickBy(Boolean)(pushInfo)),
sendRescindNotifs(_pickBy(Boolean)(rescindInfo)),		sendRescindNotifs(_pickBy(Boolean)(rescindInfo)),
]);		]);
}		}

▲ Show 20 Lines • Show All 174 Lines • Show Last 20 Lines

[services] Backup - Blob Put Client - Update put APIClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 16349

services/backup/blob_client/src/lib.rs

services/backup/blob_client/src/put_client.rs

[services] Backup - Blob Put Client - Update put API
ClosedPublic
Actions

Revision Contents
Changeset List