Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

LUCENE-10558: expose stream-based Kuromoji resource constructors #867

Closed
wants to merge 1 commit into from
Closed

LUCENE-10558: expose stream-based Kuromoji resource constructors #867

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 7 additions & 1 deletion lucene/analysis/kuromoji/src/java/org/apache/lucene/analysis/ja/dict/ConnectionCosts.java
View file
Open in desktop
Expand Up @@ -40,7 +40,13 @@ private ConnectionCosts() throws IOException {
this(ConnectionCosts::getClassResource);
}

private ConnectionCosts(IOSupplier<InputStream> connectionCostResource) throws IOException {
/**
* Create a {@link ConnectionCosts} from an input stream supplier.
*
* @param connectionCostResource supplies a stream where connection costs resource can be read
* @throws IOException if the supplied stream could not be read
*/
public ConnectionCosts(IOSupplier<InputStream> connectionCostResource) throws IOException {
super(
connectionCostResource, DictionaryConstants.CONN_COSTS_HEADER, DictionaryConstants.VERSION);
}
Expand Down
11 changes: 10 additions & 1 deletion ...ne/analysis/kuromoji/src/java/org/apache/lucene/analysis/ja/dict/TokenInfoDictionary.java
View file
Open in desktop
Expand Up @@ -66,7 +66,16 @@ private TokenInfoDictionary() throws IOException {
() -> getClassResource(FST_FILENAME_SUFFIX));
}

private TokenInfoDictionary(
/**
* Create a {@link ConnectionCosts} from an input stream supplier.
*
* @param targetMapResource supplies a stream where the target map can be read
* @param posResource supplies a stream where the pos resource can be read
* @param dictResource supplies a stream where the dictionary can be read
* @param fstResource supplies a stream where the FST can be read
* @throws IOException if the supplied stream could not be read
*/
public TokenInfoDictionary(
IOSupplier<InputStream> targetMapResource,
IOSupplier<InputStream> posResource,
IOSupplier<InputStream> dictResource,
Expand Down
22 changes: 15 additions & 7 deletions lucene/analysis/kuromoji/src/java/org/apache/lucene/analysis/ja/dict/UnknownDictionary.java
View file
Open in desktop
Expand Up @@ -52,18 +52,26 @@ private UnknownDictionary() throws IOException {
() -> getClassResource(DICT_FILENAME_SUFFIX));
}

private UnknownDictionary(
IOSupplier<InputStream> targetMapResource,
IOSupplier<InputStream> posResource,
IOSupplier<InputStream> dictResource)
/**
* Create a {@link UnknownDictionary} from an external resource path.
*
* @param targetMap supplier for stream containing target map
* @param posDict supplier for stream containing POS dictionary
* @param dict supplier for stream containing dictionary entries
* @throws IOException if a stream could not be read
*/
public UnknownDictionary(
IOSupplier<InputStream> targetMap,
IOSupplier<InputStream> posDict,
IOSupplier<InputStream> dict)
Comment on lines +64 to +66
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you explain why the parameter names should be changed? It's now inconsistent with other constructors (e.g. TokenInfoDictionary).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think he did this because "Resource" is a bit strange, as it is no longer classpath based.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's another day, I can no longer confirm nor deny, but Uwe's explanation makes sense to me :) If we keep this change, I'd be fine with the resource naming too, although it does have that classpath connotation?

throws IOException {
super(
targetMapResource,
dictResource,
targetMap,
dict,
DictionaryConstants.TARGETMAP_HEADER,
DictionaryConstants.DICT_HEADER,
DictionaryConstants.VERSION);
this.morphAtts = new UnknownMorphData(buffer, posResource);
this.morphAtts = new UnknownMorphData(buffer, posDict);
}

private static InputStream getClassResource(String suffix) throws IOException {
Expand Down