[ 
https://issues.apache.org/jira/browse/OPENNLP-1590?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17863516#comment-17863516
 ] 

ASF GitHub Bot commented on OPENNLP-1590:
-----------------------------------------

rzo1 commented on code in PR #633:
URL: https://github.com/apache/opennlp/pull/633#discussion_r1667397156


##########
opennlp-tools/src/main/java/opennlp/tools/util/featuregen/DictionaryFeatureGeneratorFactory.java:
##########
@@ -31,30 +33,39 @@
 public class DictionaryFeatureGeneratorFactory
     extends GeneratorFactory.AbstractXmlFeatureGeneratorFactory {
 
+  private static final String DICT = "dict";
+
   public DictionaryFeatureGeneratorFactory() {
     super();
   }
 
   @Override
   public AdaptiveFeatureGenerator create() throws InvalidFormatException {
-    // if resourceManager is null, we don't instantiate
-    if (resourceManager == null) {
-      return null;
-    }
-
-    String dictResourceKey = getStr("dict");
-    Object dictResource = resourceManager.getResource(dictResourceKey);
-    if (!(dictResource instanceof Dictionary)) {
-      throw new InvalidFormatException("No dictionary resource for key: " + 
dictResourceKey);
+    Dictionary dict;
+    if (resourceManager == null) { // load the dictionary directly
+      String dictResourcePath = getStr(DICT);
+      ClassLoader cl = Thread.currentThread().getContextClassLoader();
+      try (InputStream is = cl.getResourceAsStream(dictResourcePath)) {

Review Comment:
   `is` can be NULL after this call (if `dictResourcePath` isn't found). Didn't 
Look into create(...) how it is handled.



##########
opennlp-tools/src/main/java/opennlp/tools/util/featuregen/DictionaryFeatureGeneratorFactory.java:
##########
@@ -31,30 +33,39 @@
 public class DictionaryFeatureGeneratorFactory
     extends GeneratorFactory.AbstractXmlFeatureGeneratorFactory {
 
+  private static final String DICT = "dict";
+
   public DictionaryFeatureGeneratorFactory() {
     super();
   }
 
   @Override
   public AdaptiveFeatureGenerator create() throws InvalidFormatException {
-    // if resourceManager is null, we don't instantiate
-    if (resourceManager == null) {
-      return null;
-    }
-
-    String dictResourceKey = getStr("dict");
-    Object dictResource = resourceManager.getResource(dictResourceKey);
-    if (!(dictResource instanceof Dictionary)) {
-      throw new InvalidFormatException("No dictionary resource for key: " + 
dictResourceKey);
+    Dictionary dict;
+    if (resourceManager == null) { // load the dictionary directly
+      String dictResourcePath = getStr(DICT);
+      ClassLoader cl = Thread.currentThread().getContextClassLoader();
+      try (InputStream is = cl.getResourceAsStream(dictResourcePath)) {
+        dict = ((DictionarySerializer) 
getArtifactSerializerMapping().get(dictResourcePath)).create(is);
+      } catch (IOException e) {
+        throw new InvalidFormatException("No dictionary resource at: " + 
dictResourcePath, e);
+      }
+    } else { // get the dictionary via a resourceManager lookup
+      String dictResourceKey = getStr(DICT);
+      Object dictResource = resourceManager.getResource(dictResourceKey);
+      if (dictResource instanceof Dictionary) {

Review Comment:
   Since we are Java 17, we can omit the cast in the subsequent line by inline 
casting in the instanceof.





> Clear open TODO in GenericFactoryTest
> -------------------------------------
>
>                 Key: OPENNLP-1590
>                 URL: https://issues.apache.org/jira/browse/OPENNLP-1590
>             Project: OpenNLP
>          Issue Type: Test
>    Affects Versions: 2.3.3
>            Reporter: Martin Wiesner
>            Assignee: Martin Wiesner
>            Priority: Minor
>             Fix For: 2.4.0
>
>
> The test case _testDictionaryArtifactToSerializerMappingExtraction()_ in 
> _GeneratorFactoryTest_ has a TODO note that can be cleared by adjusting the 
> test case slightly, as well as the related inconsistent implementation 
> class(es).



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to