Page MenuHomePhabricator

Lightly modernize FundInitiativeSearchEngine
ClosedPublic

Authored by epriestley on Sep 7 2017, 5:06 PM.

Details

Summary

Ref T12819. Prepares for Ferret engine support.

Test Plan

Queried for various initiatives.

Diff Detail

Repository
rP Phabricator
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

epriestley created this revision.Sep 7 2017, 5:06 PM
chad accepted this revision.Sep 7 2017, 5:25 PM

So for Ferret is there anything day to day we need to know, or is everything automatic in modern code?

This revision is now accepted and ready to land.Sep 7 2017, 5:25 PM

If you don't care about the document showing up in global search, you don't need to do anything.

If you want it to appear in global search, you must currently implement both FerretInterface and FulltextInterface (previously, you only needed to implement FulltextInterface). These may fold into a single interface later, but I'm not yet sure if we can get rid of ElasticSearch completely or not.

To implement FerretInterface, you need to write 4 extra classes (Engine, Document, Field, Ngram) and create 3 tables (Document, Field, Ngram). This is a lot of busywork but we don't have any real tools to avoid it. That's what I'm going through and doing now: adding the interface, 4 classes, and 3 tables to everything which currently implements FulltextInterface so that it can also implement FerretInterface. There are about 15 global document types so I'm creating ~45 new tables and ~60 new classes.

In some cases, some of the "modern magic" (both new Ferret magic, and older non-Ferret magic) won't work on older code. That was the case here, since the Query (D18553) and SearchEngine (here) were using some older stuff. For example, Fund did not previously support search by "Projects" or "Subscribers". Modernizing the Query and SearchEngine lets the application use all the magic: subscriber queries, project queries, and now Ferret engine queries.

The one special case is that Ferret queries can cause a join to a table with an id column, which previously wouldn't happen automatically. This results in an "ambiguous column id" error, and is resolved by defining getPrimaryTableAlias() on the Query. This is generally good practice and most applications do already, but it didn't hurt anything before so not everything did it.

This is a lot of busywork but we don't have any real tools to avoid it.

This is only mostly true -- we do have the ability to programmatically define schemata, and we do it today for edges (not everything which uses edges needs an explicit edge table object). We could do it for the Ferret tables, but we still need the interfaces, engines, and migrations. Still, maybe I'll take at least a brief look at this since it might reduce the amount of copy/pasting and general pain I'm setting myself up for.

chad added a comment.Sep 7 2017, 5:49 PM

I can also copy-pasta if this is something needed for each application.

I'm still hitting some special cases and there are only ~12 more of them, but if it looks like it's just copy/paste left I'll give you whatever's left of the list.