Skip to content

Commit

Permalink
Clean up element hack and prepare for easy removal once we're there
Browse files Browse the repository at this point in the history 
Would be nice to finally get rid of this but matrix-org/matrix-react-sdk#9556 is still a thing
  • Loading branch information
@maxmalek
maxmalek committed Mar 3, 2023
1 parent e60c013 commit a95eed0
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 73 deletions.
7 changes: 3 additions & 4 deletions src/maiden/mxsearch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ void MxSearch::rebuildCache(VarCRef src)
_strings.size(), m->size(), m->size() - _strings.size());
}

MxSearch::Matches MxSearch::search(const MxMatcherList& matchers, bool fuzzy, const TwoWayCasefoldMatcher *fullmatch) const
MxSearch::Matches MxSearch::search(const MxMatcherList& matchers) const
{
std::shared_lock lock(mutex);
//-----------------------------------------------------------
Expand All @@ -100,14 +100,13 @@ MxSearch::Matches MxSearch::search(const MxMatcherList& matchers, bool fuzzy, co
for(size_t i = 0; i < N; ++i)
{
int score = mxMatchAndScore_Exact(_strings[i].s, _strings[i].len, matchers.data(), matchers.size());
if(fuzzy)
score += mxMatchAndScore_Fuzzy(_strings[i].s, matchers.data(), matchers.size());
//if(fuzzy)
// score += mxMatchAndScore_Fuzzy(_strings[i].s, matchers.data(), matchers.size());
if(score > 0)
{
Match m;
m.key = _keys[i];
m.score = score;
m.full = fullmatch && fullmatch->match(_strings[i].s, _strings[i].len);
hits.push_back(m);
}
}
Expand Down
5 changes: 2 additions & 3 deletions src/maiden/mxsearch.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ struct MxSearchConfig
// -- below here is not used by mxstore --
std::string avatar_url;
size_t maxsize = 1024; // max. size of search request, json and all
bool fuzzy = false;
//bool fuzzy = false;
bool element_hack = false;
bool debug_dummy_result = false;
};
Expand Down Expand Up @@ -55,7 +55,6 @@ class MxSearch : public EvTreeRebuilt
{
StrRef key; // key in mxstore user table
int score;
bool full; // match is a direct substring of the search term. used only if element_hack is enabled

// highest score first
inline bool operator<(const Match& o) const
Expand All @@ -66,7 +65,7 @@ class MxSearch : public EvTreeRebuilt

typedef std::vector<Match> Matches;

Matches search(const MxMatcherList& matchers, bool fuzzy, const TwoWayCasefoldMatcher *fullmatch) const; // fullmatch is only for the element_hack
Matches search(const MxMatcherList& matchers) const;

// Inherited via EvTreeRebuilt
virtual void onTreeRebuilt(VarCRef src) override;
Expand Down
112 changes: 52 additions & 60 deletions src/maiden/mxservices.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,8 @@ bool MxSearchHandler::init(VarCRef cfg)
}
}

if (VarCRef xfuzzy = cfg.lookup("fuzzy"))
searchcfg.fuzzy = xfuzzy && xfuzzy.asBool();
/*if (VarCRef xfuzzy = cfg.lookup("fuzzy"))
searchcfg.fuzzy = xfuzzy && xfuzzy.asBool();*/

if (VarCRef xeh = cfg.lookup("element_hack"))
searchcfg.element_hack = xeh && xeh.asBool();
Expand Down Expand Up @@ -189,7 +189,7 @@ bool MxSearchHandler::init(VarCRef cfg)
logdebug("MxSearchHandler: max. client request size = %u", (unsigned)searchcfg.maxsize);
logdebug("MxSearchHandler: avatar_url = %s", searchcfg.avatar_url.c_str());
logdebug("MxSearchHandler: displayname = %s", searchcfg.displaynameField.c_str());
logdebug("MxSearchHandler: fuzzy global search = %d", searchcfg.fuzzy);
//logdebug("MxSearchHandler: fuzzy global search = %d", searchcfg.fuzzy);
logdebug("MxSearchHandler: Element substring HACK = %d", searchcfg.element_hack);
logdebug("MxSearchHandler: searching %u fields:", (unsigned)searchcfg.fields.size());
for(MxSearchConfig::Fields::iterator it = searchcfg.fields.begin(); it != searchcfg.fields.end(); ++it)
Expand Down Expand Up @@ -269,7 +269,10 @@ MxSearchResults MxSearchHandler::mergeResults(const MxSearchResults& myresults,

// Fill in missing data, or override if configured to do so
if(overrideDisplayname || hs.displayname.empty())
{
DEBUG_LOG("Override displayname '%s' with '%s'", hs.displayname.c_str(), my.displayname.c_str());
hs.displayname = my.displayname;
}
if(overrideAvatar || hs.avatar.empty())
hs.avatar = my.avatar;
}
Expand All @@ -281,7 +284,24 @@ MxSearchResults MxSearchHandler::mergeResults(const MxSearchResults& myresults,
return res;
}

void MxSearchHandler::doSearch(VarRef dst, const char* term, size_t limit, VarCRef hsResultsArray) const
// Workaround for https://github.com/matrix-org/matrix-react-sdk/pull/9556
void MxSearchHandler::_ApplyElementHack(MxSearchResults& results, const std::string& term)
{
TwoWayCasefoldMatcher fullmatch(term.c_str(), term.length());
const std::string suffix = " // " + term;

const size_t N = results.size();
for(size_t i = 0; i < N; ++i)
{
MxSearchResult& sr = results[i];
bool found = fullmatch.match(sr.displayname.c_str(), sr.displayname.length())
|| fullmatch.match(sr.mxid.c_str(), sr.mxid.length());
if(!found)
sr.displayname += suffix; // trick element's substring check for the search term into always succeeding
}
}

MxSearchHandler::MxSearchResultsEx MxSearchHandler::doSearch(const char* term, size_t limit, VarCRef hsResultsArray) const
{
const std::vector<TwoWayCasefoldMatcher> matchers = mxBuildMatchersForTerm(term);
{
Expand All @@ -292,8 +312,7 @@ void MxSearchHandler::doSearch(VarRef dst, const char* term, size_t limit, VarCR
logdebug("%s", os.str().c_str());
}

TwoWayCasefoldMatcher fullmatch(term, strlen(term));
MxSearch::Matches hits = search.search(matchers, searchcfg.fuzzy, searchcfg.element_hack ? &fullmatch : NULL);
MxSearch::Matches hits = search.search(matchers);
const size_t totalhits = hits.size();

// Go throush results provided by HS, if any
Expand All @@ -309,24 +328,18 @@ void MxSearchHandler::doSearch(VarRef dst, const char* term, size_t limit, VarCR
}

// resolve matches to something readable
MxSearchResults myresults = _sources.formatMatches(*dst.mem, searchcfg, hits.data(), hits.size(), term);
MxSearchResults myresults = _sources.formatMatches(searchcfg, hits.data(), hits.size());

// Now we have up to limit many entries on both sides (HS and ours). Merge both.
MxSearchResults results = mergeResults(myresults, hsresults);
MxSearchResultsEx rx = { mergeResults(myresults, hsresults), false };

// Clip again if too many
if(results.size() > limit)
if(rx.results.size() > limit)
{
results.resize(limit);
rx.results.resize(limit);
limited = true;
}

dst.makeMap().v->map()->clear(*dst.mem); // make sure it's an empty map

dst["limited"] = limited;

const bool useGlobalAvatar = !searchcfg.avatar_url.empty();

if(searchcfg.debug_dummy_result)
{
if(hits.size() && hits.size() == limit)
Expand All @@ -341,20 +354,32 @@ void MxSearchHandler::doSearch(VarRef dst, const char* term, size_t limit, VarCR
MxSearchResult dummy;
dummy.displayname = os.str();
dummy.mxid = "@debug_dummy_result:localhost"; // matrix spec requires this to exist
results.insert(results.begin(), std::move(dummy));
rx.results.insert(rx.results.begin(), std::move(dummy));
}

VarRef ra = dst["results"].makeArray(results.size());
rx.limited = limited;
return rx;
}

void MxSearchHandler::translateResults(VarRef dst, const MxSearchResultsEx& rx) const
{
dst.makeMap().v->map()->clear(*dst.mem); // make sure it's an empty map

dst["limited"] = rx.limited;
const bool useGlobalAvatar = !searchcfg.avatar_url.empty();


for (size_t i = 0; i < results.size(); ++i)
VarRef ra = dst["results"].makeArray(rx.results.size());

for (size_t i = 0; i < rx.results.size(); ++i)
{
VarRef d = ra.at(i).makeMap();
const MxSearchResult& r = results[i];
const MxSearchResult& r = rx.results[i];

VarRef mxid = d["user_id"];
StrRef mxidRef = mxid.setStr(r.mxid.c_str(), r.mxid.length());

if(!r.avatar.empty())
if (!r.avatar.empty())
d["avatar_url"] = r.avatar.c_str();
else if (useGlobalAvatar)
d["avatar_url"] = searchcfg.avatar_url.c_str();
Expand Down Expand Up @@ -445,51 +470,18 @@ int MxSearchHandler::onRequest(BufferedWriteStream& dst, mg_connection* conn, co
xhsResultsArray = xresults;
const size_t n = xresults.size();
logdev("HS found %zu users", n);

if(useHS && n && searchcfg.element_hack)
{
TwoWayCasefoldMatcher fullmatch(term.c_str(), term.length());
Var *a = xresults.v->array_unsafe();

// FIXME: move this elsewhere
const StrRef user_id = hsdata.lookup("user_id", 7);
for(size_t i = 0; i < n; ++i)
{
VarRef entry(hsdata, &a[i]);
bool found = false;
std::string dn;

VarRef xdn = entry["display_name"];
if (xdn.type() == Var::TYPE_STRING)
{
PoolStr ps = xdn.asString();
found = fullmatch.match(ps.s, ps.len);
dn.assign(ps.s, ps.len);
}

if(!found && user_id)
if(VarCRef xid = entry.lookup(user_id))
if(xid.type() == Var::TYPE_STRING)
{
PoolStr ps = xid.asString();
found = fullmatch.match(ps.s, ps.len);
}

if(!found)
{
dn += " // " + term;
xdn.setStr(dn.c_str(), dn.length());
}
}
}
}
else
logerror("HS didn't send results array! (This is against the spec)");
}

MxSearchResultsEx rx = doSearch(term.c_str(), limit, xhsResultsArray);

if(searchcfg.element_hack)
_ApplyElementHack(rx.results, term);

// re-use vars for the search since we don't need those anymore
vars.root().v->makeMap(vars)->clear(vars);
doSearch(vars.root(), term.c_str(), limit, xhsResultsArray);
translateResults(vars.root(), rx);

writeJson(dst, vars.root(), false);
return 0;
Expand Down
9 changes: 8 additions & 1 deletion src/maiden/mxservices.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,15 @@ class MxSearchHandler : public RequestHandler
URLTarget homeserver;

private:
void doSearch(VarRef dst, const char* term, size_t limit, const VarCRef hsResultsArray) const;
struct MxSearchResultsEx
{
MxSearchResults results;
bool limited;
};
MxSearchResultsEx doSearch(const char* term, size_t limit, const VarCRef hsResultsArray) const;
void translateResults(VarRef dst, const MxSearchResultsEx& results) const;
MxSearchResults mergeResults(const MxSearchResults& myresults, const MxSearchResults& hsresults) const;
static void _ApplyElementHack(MxSearchResults& results, const std::string& term);

MxSources& _sources;
};
Expand Down
5 changes: 1 addition & 4 deletions src/maiden/mxsources.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -538,7 +538,7 @@ void MxSources::_Loop_th(MxSources* self, bool buildAsync)
logdebug("MxSources: Background thread exiting");
}

MxSearchResults MxSources::formatMatches(TreeMem& mem, const MxSearchConfig& scfg, const MxSearch::Match* matches, size_t n, const char* term) const
MxSearchResults MxSources::formatMatches(const MxSearchConfig& scfg, const MxSearch::Match* matches, size_t n) const
{
ScopeTimer timer;
MxSearchResults res;
Expand Down Expand Up @@ -567,9 +567,6 @@ MxSearchResults MxSources::formatMatches(TreeMem& mem, const MxSearchConfig& scf
if (const char* dn = xdn->asCString(*locked.ref.mem))
sr.displayname = dn;

if (scfg.element_hack && !matches[i].full && term)
sr.displayname = sr.displayname + " // " + term;

res.push_back(std::move(sr));
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/maiden/mxsources.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class MxSources
DataTree::LockedRef lockedRef() { return _merged.lockedRef(); }

// resolve search matches to actual, ready-to-display search results
MxSearchResults formatMatches(TreeMem& mem, const MxSearchConfig& scfg, const MxSearch::Match* matches, size_t n, const char* term) const;
MxSearchResults formatMatches(const MxSearchConfig& scfg, const MxSearch::Match* matches, size_t n) const;

bool load();
bool save() const;
Expand Down

0 comments on commit a95eed0

Please sign in to comment.