apache / apache/lucene

add extra safety to concrete codec implementations [LUCENE-3560]

Open
#4,634 7 comments 0 reactions 0 assignees View on GitHub
affects-version:4.0-ALPHA legacy-jira-priority:Major type:enhancement
Dominant language
Java
Stars
3.6k
Forks
1.4k
Avg merge
2d 11h
Merged PRs (30d)
88

Description

In #4564, we reorganized the codec model, and a key part of this is that Codecs are "safer"
and don't rely upon client-side configuration: IndexReader doesn't take Codec or anything of that
nature, only IndexWriter.

Instead for "read" all codecs are initialized from the classpath via a no-arg ctor from Java's
Service Provider Mechanism.

So, although Codecs can still take parameters in the constructors, be subclassable, etc (for passing
to IndexWriter), this enforces that they must write any configuration information they need into
the index, so that we don't have a flimsy API.

I think we should go even further, for additional safety. Any methods on our concrete codecs that
are not intended to be subclassed should be final, and we should add assertions to verify this.

For example, SimpleText's files() implementation should be final. If you want to make an extension
of simpletext that has additional files, then this is a different index format and should have a
different name!

Note: This doesn't stop extensibility, only stupid mistakes.
For example, this means that Lucene40Codec's postingsFormat() implementation is final, even though
it offers a configurable "hook" (getPostingsFormatForField) for you to specify per-field postings
formats (which it writes into a .per file into the index, so that it knows how to read each field).

```Java
private final PostingsFormat postingsFormat = new PerFieldPostingsFormat() {
`@Override`
public PostingsFormat getPostingsFormatForField(String field) {
return Lucene40Codec.this.getPostingsFormatForField(field);
}
};

...

`@Override`
public final PostingsFormat postingsFormat() {
return postingsFormat;
}

...

/** Returns the postings format that should be used for writing
* new segments of field.
*
* The default implementation always returns "Lucene40"
*/
public PostingsFormat getPostingsFormatForField(String field) {
return defaultFormat;
}
```

---
Migrated from [LUCENE-3560](https://issues.apache.org/jira/browse/LUCENE-3560) by Robert Muir (@rmuir)
Attachments: [LUCENE-3560.patch](https://apache.github.io/lucene-jira-archive/attachments/LUCENE-3560/LUCENE-3560.patch)

Contributor guide

Open the contributing guide

Research direction

Start by locating the concrete codec implementations, especially SimpleText and Lucene40Codec, then inspect their files() and postingsFormat() entry points. Compare methods intended as extension hooks, such as getPostingsFormatForField, with methods that should be final. Done means the applicable concrete implementations enforce those boundaries and include assertions verifying them.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
search
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.