Discussion point: SourceDocument design

[lintool]

Follow up to #1047

We need to rethink the indexing pipeline re: "contents" and "raw", because there's a lot of confusion, mostly because the design has evolved over time...

In the Lucene document, we've agreed (and this is clear) that "contents" field should be the content as indexed for searching, and "raw" should be the raw source document.

What about SourceDocument then? Currently, it only has content() method, and it's underspecified as to what it should provide. The Generator is responsible for calling content() and pulling out the necessary parts to populate the Lucene "contents" field and "raw" field.

Does this design make sense? The alternative is that SourceDocument has contents() and raw() methods, i.e., it "knows" its own contents and raw form.

One argument against this is when we have one-to-many SourceDocument to Lucene Document mappings - like in COVID19 (see #1044), where we take a single SourceDocument, which may be full text, and index it at the paragraph level (i.e., multiple Lucene Documents)

@nikhilro @edwinzhng thoughts?

If we stick to the original design, then it's a matter of clarifying in documentation. If we adopted the alternative design, it's going to be a painful refactoring job...

[nikhilro]

I'm about to create a PR which could serve as an example

[nikhilro]

One argument against this is when we have one-to-many SourceDocument to Lucene Document mappings - like in COVID19 (see #1044), where we take a single SourceDocument, which may be full text, and index it at the paragraph level (i.e., multiple Lucene Documents)

This doesn't hold because the way I implemented there is one-to-one mapping

alternative is that SourceDocument has contents() and raw() methods, i.e., it "knows" its own contents and raw form.

I like this. Also, there is an ongoing confusion because we use contents some places and content other. In source document, it's content. So, ultimately I'd vote in favor of each SourceDocument having content and raw.

[edwinzhng]

I like the separate fields too for contents and raw, allowing the SourceDocument to build it's own separation makes things more organized imo

[nikhilro]

For posterity, meat of the resolution in this comment in PR #1049: #1049 (comment)

[lintool]

Okay, this will be a fairly painful refactoring to get everything in this format, so we'll punt it for later.

[lintool]

Actually, if we take this design to the logical conclusion, should the generator also be folded into the collection?

That is, each SourceDocument knows how to generate its own corresponding Lucene document? Or, do we like the separation between Lucene and non-Lucene code?

[nikhilro]

I don't even see the need for any refactoring because there is an implicit assumption that content == raw unless SourceDocument for that Collection specifies raw, no?

That is, each SourceDocument knows how to generate its own corresponding Lucene document? Or, do we like the separation between Lucene and non-Lucene code?

I like the separation between Lucene and non-Lucene code. It's nice and clean

[lintool]

I don't even see the need for any refactoring because there is an implicit assumption that content == raw unless SourceDocument for that Collection specifies raw, no?

I see, I suppose we can change SourceDocument to abstract, and make one dispatch to the other, overridable by subclasses if need be?

[nikhilro]

I see, I suppose we can change SourceDocument to abstract, and make one dispatch to the other, overridable by subclasses if need be?

I don't know Java too well but that sounds reasonable. cc @edwinzhng