OPENNLP-1590 Clear open TODO in GenericFactoryTest

- adjusts DictionaryFeatureGeneratorFactory to handle situations without a ResourceManager instance at runtime - fixes a missing Exception type in catch block of GeneratorFactory#buildGenerator which wasn't handled correctly - clears TODO in GeneratorFactoryTest - adds another test case to demonstrate that a descriptively declared dictionary is loaded for the creation of a DictionaryFeatureGeneratorFactory - adds a related test resource
apache · Jul 6, 2024 · 09d96bf · 09d96bf
1 parent fddd5ae
commit 09d96bf
Show file tree

Hide file tree

Showing 5 changed files with 110 additions and 59 deletions.
diff --git a/...-tools/src/main/java/opennlp/tools/util/featuregen/DictionaryFeatureGeneratorFactory.java b/...-tools/src/main/java/opennlp/tools/util/featuregen/DictionaryFeatureGeneratorFactory.java
@@ -17,6 +17,8 @@
 
 package opennlp.tools.util.featuregen;
 
+import java.io.IOException;
+import java.io.InputStream;
 import java.util.HashMap;
 import java.util.Map;
 
@@ -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) {
+        dict = (Dictionary) dictResource;
+      } else {
+        throw new InvalidFormatException("No dictionary resource for key: " + dictResourceKey);
+      }
     }
-
-    return new DictionaryFeatureGenerator(getStr("prefix"), (Dictionary) dictResource);
+    return new DictionaryFeatureGenerator(dict);
   }
 
   @Override
   public Map<String, ArtifactSerializer<?>> getArtifactSerializerMapping() throws InvalidFormatException {
     Map<String, ArtifactSerializer<?>> mapping = new HashMap<>();
-    mapping.put(getStr("dict"), new DictionarySerializer());
+    mapping.put(getStr(DICT), new DictionarySerializer());
     return mapping;
   }
 }
diff --git a/opennlp-tools/src/main/java/opennlp/tools/util/featuregen/GeneratorFactory.java b/opennlp-tools/src/main/java/opennlp/tools/util/featuregen/GeneratorFactory.java
@@ -144,8 +144,8 @@ private static AdaptiveFeatureGenerator buildGenerator(Element generatorElement,
               (AbstractXmlFeatureGeneratorFactory) constructor.newInstance();
           factory.init(generatorElement, resourceManager);
           return factory.create();
-        } catch (NoSuchMethodException | InvocationTargetException | InstantiationException
-                 | IllegalAccessException e) {
+        } catch (NoSuchMethodException | InvocationTargetException | InstantiationException |
+                 InvalidFormatException | IllegalAccessException e) {
           throw new RuntimeException(e);
         }
       } catch (ClassNotFoundException e) {
@@ -351,8 +351,8 @@ final void init(Element element, FeatureGeneratorResourceProvider resourceManage
           if (type.equals("generator")) {
             String key = "generator#" + generators.size();
             AdaptiveFeatureGenerator afg = buildGenerator(elem, resourceManager);
-            generators.add(afg);
             if (afg != null) {
+              generators.add(afg);
               args.put(key, afg);
             }
           } else {

diff --git a/opennlp-tools/src/test/java/opennlp/tools/util/featuregen/GeneratorFactoryTest.java b/opennlp-tools/src/test/java/opennlp/tools/util/featuregen/GeneratorFactoryTest.java
@@ -61,6 +61,21 @@ public void createFeatures(List<String> features, String[] tokens, int index,
     }
   }
 
+  /**
+   * Tests the creation from a descriptor which contains an unkown element.
+   * The creation should fail with an {@link InvalidFormatException}
+   */
+  @Test
+  void testCreationWithUnkownElement() {
+
+    Assertions.assertThrows(IOException.class, () -> {
+      try (InputStream descIn = getClass().getResourceAsStream(
+              "/opennlp/tools/util/featuregen/FeatureGeneratorConfigWithUnkownElement.xml")) {
+        GeneratorFactory.create(descIn, null);
+      }
+    });
+  }
+
   @Test
   void testCreationWithTokenClassFeatureGenerator() throws Exception {
     InputStream generatorDescriptorIn = getClass().getResourceAsStream(
@@ -81,62 +96,55 @@ void testCreationWithTokenClassFeatureGenerator() throws Exception {
 
   @Test
   void testCreationWihtSimpleDescriptor() throws Exception {
-    InputStream generatorDescriptorIn = getClass().getResourceAsStream(
-        "/opennlp/tools/util/featuregen/TestFeatureGeneratorConfig.xml");
+    try (InputStream generatorDescriptorIn = getClass().getResourceAsStream(
+        "/opennlp/tools/util/featuregen/TestFeatureGeneratorConfig.xml")) {
 
-    // If this fails the generator descriptor could not be found
-    // at the expected location
-    Assertions.assertNotNull(generatorDescriptorIn);
+      // If this fails the generator descriptor could not be found
+      // at the expected location
+      Assertions.assertNotNull(generatorDescriptorIn);
 
-    Collection<String> expectedGenerators = new ArrayList<>();
-    expectedGenerators.add(OutcomePriorFeatureGenerator.class.getName());
+      Collection<String> expectedGenerators = new ArrayList<>();
+      expectedGenerators.add(OutcomePriorFeatureGenerator.class.getName());
 
-    AggregatedFeatureGenerator aggregatedGenerator =
-        (AggregatedFeatureGenerator) GeneratorFactory.create(generatorDescriptorIn, null);
+      AggregatedFeatureGenerator aggregatedGenerator =
+              (AggregatedFeatureGenerator) GeneratorFactory.create(generatorDescriptorIn, null);
 
 
-    for (AdaptiveFeatureGenerator generator : aggregatedGenerator.getGenerators()) {
+      for (AdaptiveFeatureGenerator generator : aggregatedGenerator.getGenerators()) {
 
-      expectedGenerators.remove(generator.getClass().getName());
+        expectedGenerators.remove(generator.getClass().getName());
 
-      // if of kind which requires parameters check that
-    }
+        // if of kind which requires parameters check that
+      }
 
-    // If this fails not all expected generators were found and
-    // removed from the expected generators collection
-    Assertions.assertEquals(0, expectedGenerators.size());
+      // If this fails not all expected generators were found and
+      // removed from the expected generators collection
+      Assertions.assertEquals(0, expectedGenerators.size());
+    }
   }
 
-  /**
-   * Tests the creation from a descriptor which contains an unkown element.
-   * The creation should fail with an {@link InvalidFormatException}
-   */
   @Test
-  void testCreationWithUnkownElement() {
-
-    Assertions.assertThrows(IOException.class, () -> {
-
-      try (InputStream descIn = getClass().getResourceAsStream(
-          "/opennlp/tools/util/featuregen/FeatureGeneratorConfigWithUnkownElement.xml")) {
-        GeneratorFactory.create(descIn, null);
-      }
-    });
+  void testCreationWithDictionaryFeatureGenerator() throws IOException {
+    try (InputStream descIn = getClass().getResourceAsStream(
+            "/opennlp/tools/util/featuregen/TestDictionarySerializerMappingExtraction.xml")) {
+      AdaptiveFeatureGenerator generator = GeneratorFactory.create(descIn, null);
+      Assertions.assertInstanceOf(DictionaryFeatureGenerator.class,
+              ((CachedFeatureGenerator) generator).getCachedFeatureGenerator());
+    }
   }
 
   @Test
   void testDictionaryArtifactToSerializerMappingExtraction() throws IOException {
 
-    InputStream descIn = getClass().getResourceAsStream(
-        "/opennlp/tools/util/featuregen/TestDictionarySerializerMappingExtraction.xml");
-
-    Map<String, ArtifactSerializer<?>> mapping =
-        GeneratorFactory.extractArtifactSerializerMappings(descIn);
-
-    Assertions.assertInstanceOf(DictionarySerializer.class, mapping.get("test.dictionary"));
-    // TODO: if make the following effective, the test fails.
-    // this is strange because DictionaryFeatureGeneratorFactory cast dictResource to Dictionary...
-    //Assert.assertTrue(mapping.get("test.dictionary") instanceof
-    //    opennlp.tools.dictionary.Dictionary);
+    try (InputStream descIn = getClass().getResourceAsStream(
+        "/opennlp/tools/util/featuregen/TestDictionarySerializerMappingExtraction.xml")) {
+      Map<String, ArtifactSerializer<?>> mapping =
+          GeneratorFactory.extractArtifactSerializerMappings(descIn);
+      Assertions.assertNotNull(mapping);
+      Assertions.assertEquals(1, mapping.size());
+      ArtifactSerializer<?> result = mapping.get("opennlp/tools/util/featuregen/DictionaryTest.xml");
+      Assertions.assertInstanceOf(DictionarySerializer.class, result);
+    }
   }
 
   @Test

diff --git a/opennlp-tools/src/test/resources/opennlp/tools/util/featuregen/DictionaryTest.xml b/opennlp-tools/src/test/resources/opennlp/tools/util/featuregen/DictionaryTest.xml
@@ -0,0 +1,32 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<!--
+   Licensed to the Apache Software Foundation (ASF) under one
+   or more contributor license agreements.  See the NOTICE file
+   distributed with this work for additional information
+   regarding copyright ownership.  The ASF licenses this file
+   to you under the Apache License, Version 2.0 (the
+   "License"); you may not use this file except in compliance
+   with the License.  You may obtain a copy of the License at
+
+     http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing,
+   software distributed under the License is distributed on an
+   "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+   KIND, either express or implied.  See the License for the
+   specific language governing permissions and limitations
+   under the License.
+-->
+
+<dictionary case_sensitive="false">
+  <entry>
+    <token>S.</token>
+  </entry>
+  <entry>
+    <token>z. B.</token>
+  </entry>
+  <entry>
+    <token>z.B.</token>
+  </entry>
+</dictionary>
diff --git a/...est/resources/opennlp/tools/util/featuregen/TestDictionarySerializerMappingExtraction.xml b/...est/resources/opennlp/tools/util/featuregen/TestDictionarySerializerMappingExtraction.xml
@@ -17,8 +17,8 @@
 	under the License.
 -->
 
-<featureGenerators name="test">
-  <generator class="opennlp.tools.util.featuregen.DictionaryFeatureGeneratorFactory">
-    <str name="dict">test.dictionary</str>
-  </generator>
+<featureGenerators cache="true" name="test">
+    <generator class="opennlp.tools.util.featuregen.DictionaryFeatureGeneratorFactory">
+      <str name="dict">opennlp/tools/util/featuregen/DictionaryTest.xml</str>
+    </generator>
 </featureGenerators>