Differential D7117 Diff 23920 keyserver/src/database/search_utils.js

Changeset View

Standalone View

keyserver/src/database/search_utils.js

// @flow		// @flow

import natural from 'natural';		import natural from 'natural';

import type { RawMessageInfo } from 'lib/types/message-types';		import type { RawMessageInfo } from 'lib/types/message-types';
import { messageTypes } from 'lib/types/message-types.js';		import { messageTypes } from 'lib/types/message-types.js';

import { dbQuery, SQL } from '../database/database.js';		import { dbQuery, SQL } from '../database/database.js';

async function processMessagesForSearch(		async function processMessagesForSearch(
messages: $ReadOnlyArray<RawMessageInfo>,		messages: $ReadOnlyArray<RawMessageInfo \| ProcessedForSearchRow>,
): Promise<void> {		): Promise<void> {
const processedMessages = [];		const processedMessages = [];

for (const msg of messages) {		for (const msg of messages) {
if (		if (
msg.type !== messageTypes.TEXT &&		msg.type !== messageTypes.TEXT &&
msg.type !== messageTypes.EDIT_MESSAGE		msg.type !== messageTypes.EDIT_MESSAGE
) {		) {
Show All 20 Lines	await dbQuery(SQL`
INSERT INTO search (original_message_id, message_id, processed_content)		INSERT INTO search (original_message_id, message_id, processed_content)
VALUES ${processedMessages}		VALUES ${processedMessages}
ON DUPLICATE KEY UPDATE		ON DUPLICATE KEY UPDATE
message_id = VALUE(message_id),		message_id = VALUE(message_id),
processed_content = VALUE(processed_content);		processed_content = VALUE(processed_content);
`);		`);
}		}

export { processMessagesForSearch };		type ProcessedForSearchRowText = {
		+type: 0,
		+id: string,
		+text: string,
		};
		type ProcessedForSearchRowEdit = {
		+type: 20,
		+id: string,
		+targetMessageID: string,
		+text: string,
		};
		type ProcessedForSearchRow =
		\| ProcessedForSearchRowText
		\| ProcessedForSearchRowEdit;

		function processRowsForSearch(
		rows: $ReadOnlyArray<any>,
		): $ReadOnlyArray<ProcessedForSearchRow> {
		const results = [];
		for (const row of rows) {
		if (row.type === messageTypes.TEXT) {
		results.push({ type: row.type, id: row.id, text: row.content });
		} else if (row.type === messageTypes.EDIT_MESSAGE) {
		results.push({
		type: row.type,
		id: row.id,
		targetMessageID: row.target_message,
		text: row.content,
		});
		}
		}
		return results;
		}

		const pageSize = 1001;

		async function processMessagesInDBForSearch(): Promise<void> {
		let last_id = 0;
		ashoatUnsubmitted Not Done Inline Actions You are breaking variable naming conventions. We never name variables like this in the JS codebase... it always looks like `lastID`. Please try to maintain consistency! ashoat: You are breaking variable naming conventions. We never name variables like this in the JS…
		let messages = [];
		ashoatUnsubmitted Not Done Inline Actions `let` is somewhat of an antipattern, and `do-while` is an unusual construction. Can you revise this code to use a simple `while (true) {` look with a conditional `break` inside of it? I think that approach will have less "indirection" and will consequenly be more readable ashoat: `let` is somewhat of an antipattern, and `do-while` is an unusual construction. Can you revise…
		inkaAuthorUnsubmitted Done Inline Actions We have an eslint rule `no-constant-condition` that disallows constant expressions in conditions. We do not ignore this rule anywhere in the code. It is turned on by "extends": [ "eslint:recommended", in our .eslintrc.json And we use a do-while in one other place in the code here Should I change it anyway? inka: We have an eslint rule `no-constant-condition` that disallows constant expressions in…
		ashoatUnsubmitted Not Done Inline Actions I just put up D7173 to address this. Can you rebase on top of that and try again? ashoat: I just put up D7173 to address this. Can you rebase on top of that and try again?
		inkaAuthorUnsubmitted Done Inline Actions I cannot rebase until @kuba rebases his stack, because I need to be rebased on a patch of his diffs. I will ask him to do that. inka: I cannot rebase until @kuba rebases his stack, because I need to be rebased on a patch of his…

		do {
		[messages] = await dbQuery(SQL`
		SELECT id, type, content, target_message FROM messages WHERE (type=0 OR type=20)
		AND id > ${last_id} ORDER BY id LIMIT ${pageSize}
		ashoatUnsubmitted Not Done Inline Actions Lots of issues with this query: You have lines longer than 80 chars. Please always check this before submitting a diff going forward, as it leads to unnecessary review churn! You should always use constants from JS. Please look at how other queries are constructed Your query format does not look anything like how we typically format SQL queries You have trailing spaces This should be: SELECT id, type, content, target_message FROM messages WHERE (type = {messageTypes.TEXT} OR type = {messageTypes.EDIT_MESSAGE}) AND id > ${lastID} ORDER BY id LIMIT ${pageSize} ashoat: Lots of issues with this query: 1. You have lines longer than 80 chars. Please always check…
		`);
		bartekUnsubmitted Not Done Inline Actions Could the message types `0` and `20` be extracted as constants (similarly to pageSize), or at least add a comment? bartek: Could the message types `0` and `20` be extracted as constants (similarly to pageSize), or at…

		let truncatedMessages;
		if (messages.length < pageSize) {
		truncatedMessages = messages;
		} else {
		truncatedMessages = messages.slice(0, -1);
		}

		const processed_rows = processRowsForSearch(truncatedMessages);
		ashoatUnsubmitted Not Done Inline Actions Once again, inconsistent variable naming conventions ashoat: Once again, inconsistent variable naming conventions

		await processMessagesForSearch(processed_rows);

		last_id = truncatedMessages[truncatedMessages.length - 1].id;
		} while (messages.length === pageSize);
		}

		export { processMessagesForSearch, processMessagesInDBForSearch };