From 09d96bff35288bdf50ceab4b5b2206819ae0933c Mon Sep 17 00:00:00 2001 From: Martin Wiesner Date: Sat, 6 Jul 2024 19:06:54 +0200 Subject: [PATCH] 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 --- .../DictionaryFeatureGeneratorFactory.java | 35 +++++--- .../util/featuregen/GeneratorFactory.java | 6 +- .../util/featuregen/GeneratorFactoryTest.java | 88 ++++++++++--------- .../tools/util/featuregen/DictionaryTest.xml | 32 +++++++ ...tDictionarySerializerMappingExtraction.xml | 8 +- 5 files changed, 110 insertions(+), 59 deletions(-) create mode 100644 opennlp-tools/src/test/resources/opennlp/tools/util/featuregen/DictionaryTest.xml diff --git a/opennlp-tools/src/main/java/opennlp/tools/util/featuregen/DictionaryFeatureGeneratorFactory.java b/opennlp-tools/src/main/java/opennlp/tools/util/featuregen/DictionaryFeatureGeneratorFactory.java index 72b253b51..a28305334 100644 --- a/opennlp-tools/src/main/java/opennlp/tools/util/featuregen/DictionaryFeatureGeneratorFactory.java +++ b/opennlp-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> getArtifactSerializerMapping() throws InvalidFormatException { Map> 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 index 7dccaed64..dcb9ccc23 100644 --- 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 index dd25f11eb..8fb08bea8 100644 --- 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 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 expectedGenerators = new ArrayList<>(); - expectedGenerators.add(OutcomePriorFeatureGenerator.class.getName()); + Collection 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> 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> 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 new file mode 100644 index 000000000..fa9946113 --- /dev/null +++ b/opennlp-tools/src/test/resources/opennlp/tools/util/featuregen/DictionaryTest.xml @@ -0,0 +1,32 @@ + + + + + + + S. + + + z. B. + + + z.B. + + diff --git a/opennlp-tools/src/test/resources/opennlp/tools/util/featuregen/TestDictionarySerializerMappingExtraction.xml b/opennlp-tools/src/test/resources/opennlp/tools/util/featuregen/TestDictionarySerializerMappingExtraction.xml index 8c68c8bf1..a7076dcfb 100644 --- a/opennlp-tools/src/test/resources/opennlp/tools/util/featuregen/TestDictionarySerializerMappingExtraction.xml +++ b/opennlp-tools/src/test/resources/opennlp/tools/util/featuregen/TestDictionarySerializerMappingExtraction.xml @@ -17,8 +17,8 @@ under the License. --> - - - test.dictionary - + + + opennlp/tools/util/featuregen/DictionaryTest.xml +