|
[
Permlink
| « Hide
]
Jukka Zitting added a comment - 01/Jul/06 02:37 PM
See a related email thread at http://article.gmane.org/gmane.comp.apache.jackrabbit.devel/7609
Attached a proposal patch containing a mostly complete implementation of the TextExtractor idea I discussed briefly on the mailing list. This covers just a part of this issue, but should simplify further work considerably.
The attached patch (jackrabbit-extractor-r420472.patch) contains just the TextExtractor interface and related classes placed in org.apache.jackrabbit.extractor. I chose to place them outside of o.a.j.core as they have no Jackrabbit dependencies, and would probably make a good contribution to Apache Lucene once battle-tested. I'll continue with a separate patch that backwards-compatibly replaces the current TextFilter usage in o.a.j.core.query[.lucene], and with some class diagrams that give a quick overview before diving into the javadocs. Attached patch (jackrabbit-query-r420472.patch) to integrate the TextExtractor proposal with Jackrabbit core.
Looks very good.
Some minor suggestions: - NodeIndexer.addBinaryValue() is protected to allow subclasses to override it but it uses the private method getValue(). Thus getValue() should be protected final in order to be usable for a subclass. - Extracting text should be deferred to the time when the lucene Document acutally requests character from Reader that is assigned to a Field. See http://issues.apache.org/jira/browse/JCR-264. Marcel:
> NodeIndexer.addBinaryValue() is protected to allow subclasses to override it but it uses the private > method getValue(). Thus getValue() should be protected final in order to be usable for a subclass. OK. > Extracting text should be deferred to the time when the lucene Document acutally requests character > from Reader that is assigned to a Field. See http://issues.apache.org/jira/browse/JCR-264. I think it would make more design sense to try to postpone the creation of the Document instances instead of delaying text extraction. But I'm not too familiar with the details, so I'm OK with adding lazy reading to the mix. In any case I think it's best to layer the lazy reading on top of the TextExtractor interface instead of below it. A utility class like the following could achieve this as long as the given InputStream remains valid until the document has been read. class TextExtractorReader extends Reader { private final TextExtractor extractor; private final InputStream stream; private final String type; private final String encoding; private Reader reader; public TextExtractorReader( TextExtractor extractor, InputStream stream, String type, String encoding) { this.extractor = extractor; this.stream = stream; this.type = type; this.encoding = encoding; this.reader = null; } public int read(char[] buffer, int offset, int length) throws IOException { if (reader == null) { reader = extractor.extractText(stream, type, encoding); } return reader.read(buffer, offset, length); } public void close() throws IOException { if (reader != null) { reader.close(); } else { stream.close(); } } } I can update the query patch accordingly. Jukka wrote:
> I think it would make more design sense to try to postpone the creation of the Document instances > instead of delaying text extraction. But I'm not too familiar with the details, so I'm OK with adding lazy > reading to the mix. In any case I think it's best to layer the lazy reading on top of the TextExtractor interface > instead of below it. A utility class like the following could achieve this as long as the given InputStream > remains valid until the document has been read. Yes, you are right. I thought I could get away with the dirty solution ;) While going through your patch I was actually also thinking about a design that should create the document only when it is really added to the index. For now we can maybe use the TextExtractorReader you proposed and then in a next step change the design to create the Document in a later stage of the indexing process. > Yes, you are right. I thought I could get away with the dirty solution ;)
Well, as they say, do the simplest thing that could possibly work! > For now we can maybe use the TextExtractorReader you proposed and then in a next step > change the design to create the Document in a later stage of the indexing process. Sounds good. Attached a revised patch (jackrabbit-query-r421461.patch) that uses the TextExtractorReader solution. I would like to get this change into the next major release (1.3) and propose the following changes:
- Create a new module jackrabbit-text-extractors which will initially contain the jackrabbit-extractor patch provided by Jukka - Migrate the jackrabbit-text-filters into the new extractors module - Add jackrabbit-text-filters as dependency to jackrabbit-core - Remove the jackrabbit-text-filters module and do not create releases anymore for this module. Jackrabbit would still support existing releases of jackrabbit-text-filters but the interface TextFilter will be deprecated (see Jukkas' patch) and developers are encouraged to use the new TextExtractor interface. Does this make sense? In addition to the previously mentioned steps I also implemented the deferred text extraction as discussed with Jukka in this jira issue. The helper class TextExtractorReader has been removed again.
Fixed in revision: 489112 If noone objects within a week I will remove the now obsolete module jackrabbit-index-filters. |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
 |
|
Improvement
Closed
Minor
Enhance indexing of binary content
