Page MenuHomePhabricator

[keyserver] Process new messages for search
Needs RevisionPublic

Authored by inka on Wed, Mar 15, 4:31 AM.
Referenced Files
Unknown Object (File)
Thu, Mar 16, 7:04 PM
Unknown Object (File)
Thu, Mar 16, 7:04 PM
Unknown Object (File)
Thu, Mar 16, 2:31 AM



tokenizeAndStem splits the text into words, removes stopwords, and stemms the remaining words. It returns an array. The second paramether is a boolean tellinng whether to keep the stopwords. source

Test Plan

Tested that when a new message is created, a proper field appears in the search table. Tested that when a message is edited (by passing an artificial edit message to processMessagesForSearch
function) a proper field is edited in the search table.

Diff Detail

rCOMM Comm
No Lint Coverage
No Test Coverage

Event Timeline

inka requested review of this revision.Wed, Mar 15, 4:46 AM
36–38 ↗(On Diff #23751)
  1. Please format this like other SQL queries. Note the indentation we use in the codebase, and note the spacing we use in the codebase
  2. Please make sure you don't have any lines longer than 80 chars

Address review and add early return when there is nothing to insert

ashoat requested changes to this revision.Mon, Mar 20, 2:14 PM

Looks good, but a couple notes in message-creator.js


We should only pass newMessageInfos in here. The messages that are in returnMessageInfos but not newMessageInfos are messages that have already been created. Those messages should have already been indexed

This brings to mind a question: is the indexing process idempotent? Meaning, if I index the same message twice, will it be the same as if I indexed that message once?


I don't think we should block the return on this. Search indexing is usually implemented as a "post-processing step"... the user creating the message shouldn't need to wait on the search indexing to complete for the endpoint to return.

Instead, I think we should include this in postMessageSendPromise. Can you move the call to processMessagesForSearch into postMessageSend? You can use the messageInfos parameter (stripLocalIDs should have no effect on indexing I think)


Can you name this file search-utils.js to match the naming convention in the codebase?

This revision now requires changes to proceed.Mon, Mar 20, 2:14 PM