You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@opennlp.apache.org by "rzo1 (via GitHub)" <gi...@apache.org> on 2023/06/18 10:08:07 UTC
[GitHub] [opennlp] rzo1 opened a new pull request, #541: OPENNLP-1174 - Remove classic format support in feature generator XML config
rzo1 opened a new pull request, #541:
URL: https://github.com/apache/opennlp/pull/541
### For all changes:
- [x ] Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
- [x] Does your PR title start with OPENNLP-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
- [x] Has your PR been rebased against the latest commit within the target branch (typically main)?
- [x] Is your initial contribution a single, squashed commit?
### For code changes:
- [x] Have you ensured that the full suite of tests is executed via mvn clean install at the root opennlp folder?
- [ ] Have you written or updated unit tests to verify your changes?
- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
- [ ] If applicable, have you updated the LICENSE file, including the main LICENSE file in opennlp folder?
- [ ] If applicable, have you updated the NOTICE file, including the main NOTICE file found in opennlp folder?
### For documentation related changes:
- [ ] Have you ensured that the format looks appropriate for the output in which it is rendered?
### Note
I created a paramterized Jenkins job to run the eval tests for a given repository / branch. The removal of the classic xml config format has no (side) effect on the eval tests: https://ci-builds.apache.org/job/OpenNLP/job/eval-tests-configurable/3/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [opennlp] mawiesne commented on a diff in pull request #541: OPENNLP-1174 - Remove classic format support in feature generator XML config
Posted by "mawiesne (via GitHub)" <gi...@apache.org>.
mawiesne commented on code in PR #541:
URL: https://github.com/apache/opennlp/pull/541#discussion_r1233248027
##########
opennlp-tools/src/main/java/opennlp/tools/util/featuregen/GeneratorFactory.java:
##########
@@ -95,25 +87,235 @@
public class GeneratorFactory {
/**
- * The {@link XmlFeatureGeneratorFactory} is responsible to construct
- * an {@link AdaptiveFeatureGenerator} from an given XML {@link Element}
- * which contains all necessary configuration if any.
+ * Creates an {@link AdaptiveFeatureGenerator} from an provided XML descriptor.
+ * <p>
+ * Usually this XML descriptor contains a set of nested feature generators
+ * which are then used to generate the features by one of the opennlp
+ * components.
+ *
+ * @param xmlDescriptorIn the {@link InputStream} from which the descriptor
+ * is read, the stream remains open and must be closed by the caller.
+ * @param resourceManager the resource manager which is used to resolve resources
+ * referenced by a key in the descriptor
+ * @return created feature generators
+ * @throws IOException if an error occurs during reading from the descriptor
+ * {@link InputStream}
*/
- @Deprecated // TODO: (OPENNLP-1174) just remove when back-compat is no longer needed
- interface XmlFeatureGeneratorFactory {
+ public static AdaptiveFeatureGenerator create(InputStream xmlDescriptorIn,
+ FeatureGeneratorResourceProvider resourceManager)
+ throws IOException {
- /**
- * Creates an {@link AdaptiveFeatureGenerator} from a the describing
- * XML element.
- *
- * @param generatorElement the element which contains the configuration
- * @param resourceManager the resource manager which could be used
- * to access referenced resources
- *
- * @return the configured {@link AdaptiveFeatureGenerator}
- */
- AdaptiveFeatureGenerator create(Element generatorElement,
- FeatureGeneratorResourceProvider resourceManager) throws InvalidFormatException;
+ final org.w3c.dom.Document xmlDescriptorDOM = createDOM(xmlDescriptorIn);
+ final Element generatorElement = xmlDescriptorDOM.getDocumentElement();
+
+ return createGenerator(generatorElement, resourceManager);
+ }
+
+ /**
+ * Creates a {@link AdaptiveFeatureGenerator} for the provided element.
+ * To accomplish this it looks up the corresponding factory by the
+ * element tag name. The factory is then responsible for the creation
+ * of the generator from the element.
+ *
+ * @param generatorElement must not be {@code null}
+ * @param resourceManager may be {@code null}
+ * @return a {@link AdaptiveFeatureGenerator}
Review Comment:
Should read "An..." as next word starts with letter A.
##########
opennlp-tools/src/main/java/opennlp/tools/util/featuregen/GeneratorFactory.java:
##########
@@ -95,25 +87,235 @@
public class GeneratorFactory {
/**
- * The {@link XmlFeatureGeneratorFactory} is responsible to construct
- * an {@link AdaptiveFeatureGenerator} from an given XML {@link Element}
- * which contains all necessary configuration if any.
+ * Creates an {@link AdaptiveFeatureGenerator} from an provided XML descriptor.
+ * <p>
+ * Usually this XML descriptor contains a set of nested feature generators
+ * which are then used to generate the features by one of the opennlp
+ * components.
+ *
+ * @param xmlDescriptorIn the {@link InputStream} from which the descriptor
+ * is read, the stream remains open and must be closed by the caller.
+ * @param resourceManager the resource manager which is used to resolve resources
+ * referenced by a key in the descriptor
+ * @return created feature generators
+ * @throws IOException if an error occurs during reading from the descriptor
+ * {@link InputStream}
*/
- @Deprecated // TODO: (OPENNLP-1174) just remove when back-compat is no longer needed
- interface XmlFeatureGeneratorFactory {
+ public static AdaptiveFeatureGenerator create(InputStream xmlDescriptorIn,
+ FeatureGeneratorResourceProvider resourceManager)
+ throws IOException {
- /**
- * Creates an {@link AdaptiveFeatureGenerator} from a the describing
- * XML element.
- *
- * @param generatorElement the element which contains the configuration
- * @param resourceManager the resource manager which could be used
- * to access referenced resources
- *
- * @return the configured {@link AdaptiveFeatureGenerator}
- */
- AdaptiveFeatureGenerator create(Element generatorElement,
- FeatureGeneratorResourceProvider resourceManager) throws InvalidFormatException;
+ final org.w3c.dom.Document xmlDescriptorDOM = createDOM(xmlDescriptorIn);
+ final Element generatorElement = xmlDescriptorDOM.getDocumentElement();
+
+ return createGenerator(generatorElement, resourceManager);
+ }
+
+ /**
+ * Creates a {@link AdaptiveFeatureGenerator} for the provided element.
+ * To accomplish this it looks up the corresponding factory by the
+ * element tag name. The factory is then responsible for the creation
+ * of the generator from the element.
+ *
+ * @param generatorElement must not be {@code null}
+ * @param resourceManager may be {@code null}
+ * @return a {@link AdaptiveFeatureGenerator}
+ */
+ private static AdaptiveFeatureGenerator buildGenerator(Element generatorElement,
+ FeatureGeneratorResourceProvider resourceManager)
+ throws InvalidFormatException {
+
+ if (generatorElement == null) {
+ throw new IllegalArgumentException("generatorElement must not be NULL");
+ }
+
+ final String className = generatorElement.getAttribute("class");
+ if (className.isBlank()) {
+ throw new InvalidFormatException("generator must have class attribute");
+ } else {
+ try {
+ final Class<?> factoryClass = Class.forName(className);
+ try {
+ final Constructor<?> constructor = factoryClass.getConstructor();
+ final AbstractXmlFeatureGeneratorFactory factory =
+ (AbstractXmlFeatureGeneratorFactory) constructor.newInstance();
+ factory.init(generatorElement, resourceManager);
+ return factory.create();
+ } catch (NoSuchMethodException | InvocationTargetException | InstantiationException
+ | IllegalAccessException e) {
+ throw new RuntimeException(e);
+ }
+ } catch (ClassNotFoundException e) {
+ throw new RuntimeException(e);
+ }
+ }
+ }
+
+ /**
+ * Creates a {@link AdaptiveFeatureGenerator} for the provided element.
+ * To accomplish this it looks up the corresponding factory by the
+ * element tag name. The factory is then responsible for the creation
+ * of the generator from the element.
+ *
+ * @param generatorElement must not be {@code null}
+ * @param resourceManager may be {@code null}
+ * @return a {@link AdaptiveFeatureGenerator}
+ */
+ private static AdaptiveFeatureGenerator createGenerator(Element generatorElement,
+ FeatureGeneratorResourceProvider resourceManager)
+ throws InvalidFormatException {
+
+ if (generatorElement == null) {
+ throw new IllegalArgumentException("generatorElement must not be NULL");
Review Comment:
See similar comment above.
##########
opennlp-tools/src/main/java/opennlp/tools/util/featuregen/GeneratorFactory.java:
##########
@@ -95,25 +87,235 @@
public class GeneratorFactory {
/**
- * The {@link XmlFeatureGeneratorFactory} is responsible to construct
- * an {@link AdaptiveFeatureGenerator} from an given XML {@link Element}
- * which contains all necessary configuration if any.
+ * Creates an {@link AdaptiveFeatureGenerator} from an provided XML descriptor.
+ * <p>
+ * Usually this XML descriptor contains a set of nested feature generators
+ * which are then used to generate the features by one of the opennlp
+ * components.
+ *
+ * @param xmlDescriptorIn the {@link InputStream} from which the descriptor
+ * is read, the stream remains open and must be closed by the caller.
+ * @param resourceManager the resource manager which is used to resolve resources
+ * referenced by a key in the descriptor
+ * @return created feature generators
+ * @throws IOException if an error occurs during reading from the descriptor
+ * {@link InputStream}
*/
- @Deprecated // TODO: (OPENNLP-1174) just remove when back-compat is no longer needed
- interface XmlFeatureGeneratorFactory {
+ public static AdaptiveFeatureGenerator create(InputStream xmlDescriptorIn,
+ FeatureGeneratorResourceProvider resourceManager)
+ throws IOException {
- /**
- * Creates an {@link AdaptiveFeatureGenerator} from a the describing
- * XML element.
- *
- * @param generatorElement the element which contains the configuration
- * @param resourceManager the resource manager which could be used
- * to access referenced resources
- *
- * @return the configured {@link AdaptiveFeatureGenerator}
- */
- AdaptiveFeatureGenerator create(Element generatorElement,
- FeatureGeneratorResourceProvider resourceManager) throws InvalidFormatException;
+ final org.w3c.dom.Document xmlDescriptorDOM = createDOM(xmlDescriptorIn);
+ final Element generatorElement = xmlDescriptorDOM.getDocumentElement();
+
+ return createGenerator(generatorElement, resourceManager);
+ }
+
+ /**
+ * Creates a {@link AdaptiveFeatureGenerator} for the provided element.
+ * To accomplish this it looks up the corresponding factory by the
+ * element tag name. The factory is then responsible for the creation
+ * of the generator from the element.
+ *
+ * @param generatorElement must not be {@code null}
+ * @param resourceManager may be {@code null}
+ * @return a {@link AdaptiveFeatureGenerator}
+ */
+ private static AdaptiveFeatureGenerator buildGenerator(Element generatorElement,
+ FeatureGeneratorResourceProvider resourceManager)
+ throws InvalidFormatException {
+
+ if (generatorElement == null) {
+ throw new IllegalArgumentException("generatorElement must not be NULL");
+ }
+
+ final String className = generatorElement.getAttribute("class");
+ if (className.isBlank()) {
+ throw new InvalidFormatException("generator must have class attribute");
+ } else {
+ try {
+ final Class<?> factoryClass = Class.forName(className);
+ try {
+ final Constructor<?> constructor = factoryClass.getConstructor();
+ final AbstractXmlFeatureGeneratorFactory factory =
+ (AbstractXmlFeatureGeneratorFactory) constructor.newInstance();
+ factory.init(generatorElement, resourceManager);
+ return factory.create();
+ } catch (NoSuchMethodException | InvocationTargetException | InstantiationException
+ | IllegalAccessException e) {
+ throw new RuntimeException(e);
+ }
+ } catch (ClassNotFoundException e) {
+ throw new RuntimeException(e);
+ }
+ }
+ }
+
+ /**
+ * Creates a {@link AdaptiveFeatureGenerator} for the provided element.
+ * To accomplish this it looks up the corresponding factory by the
+ * element tag name. The factory is then responsible for the creation
+ * of the generator from the element.
+ *
+ * @param generatorElement must not be {@code null}
+ * @param resourceManager may be {@code null}
+ * @return a {@link AdaptiveFeatureGenerator}
Review Comment:
See similar comment above.
##########
opennlp-tools/src/main/java/opennlp/tools/util/featuregen/GeneratorFactory.java:
##########
@@ -95,25 +87,235 @@
public class GeneratorFactory {
/**
- * The {@link XmlFeatureGeneratorFactory} is responsible to construct
- * an {@link AdaptiveFeatureGenerator} from an given XML {@link Element}
- * which contains all necessary configuration if any.
+ * Creates an {@link AdaptiveFeatureGenerator} from an provided XML descriptor.
+ * <p>
+ * Usually this XML descriptor contains a set of nested feature generators
+ * which are then used to generate the features by one of the opennlp
+ * components.
+ *
+ * @param xmlDescriptorIn the {@link InputStream} from which the descriptor
+ * is read, the stream remains open and must be closed by the caller.
+ * @param resourceManager the resource manager which is used to resolve resources
+ * referenced by a key in the descriptor
+ * @return created feature generators
+ * @throws IOException if an error occurs during reading from the descriptor
+ * {@link InputStream}
*/
- @Deprecated // TODO: (OPENNLP-1174) just remove when back-compat is no longer needed
- interface XmlFeatureGeneratorFactory {
+ public static AdaptiveFeatureGenerator create(InputStream xmlDescriptorIn,
+ FeatureGeneratorResourceProvider resourceManager)
+ throws IOException {
- /**
- * Creates an {@link AdaptiveFeatureGenerator} from a the describing
- * XML element.
- *
- * @param generatorElement the element which contains the configuration
- * @param resourceManager the resource manager which could be used
- * to access referenced resources
- *
- * @return the configured {@link AdaptiveFeatureGenerator}
- */
- AdaptiveFeatureGenerator create(Element generatorElement,
- FeatureGeneratorResourceProvider resourceManager) throws InvalidFormatException;
+ final org.w3c.dom.Document xmlDescriptorDOM = createDOM(xmlDescriptorIn);
+ final Element generatorElement = xmlDescriptorDOM.getDocumentElement();
+
+ return createGenerator(generatorElement, resourceManager);
+ }
+
+ /**
+ * Creates a {@link AdaptiveFeatureGenerator} for the provided element.
Review Comment:
Should read "Creates an..." as next word starts with letter A.
##########
opennlp-tools/src/main/java/opennlp/tools/util/featuregen/GeneratorFactory.java:
##########
@@ -95,25 +87,235 @@
public class GeneratorFactory {
/**
- * The {@link XmlFeatureGeneratorFactory} is responsible to construct
- * an {@link AdaptiveFeatureGenerator} from an given XML {@link Element}
- * which contains all necessary configuration if any.
+ * Creates an {@link AdaptiveFeatureGenerator} from an provided XML descriptor.
+ * <p>
+ * Usually this XML descriptor contains a set of nested feature generators
+ * which are then used to generate the features by one of the opennlp
+ * components.
+ *
+ * @param xmlDescriptorIn the {@link InputStream} from which the descriptor
+ * is read, the stream remains open and must be closed by the caller.
+ * @param resourceManager the resource manager which is used to resolve resources
+ * referenced by a key in the descriptor
+ * @return created feature generators
+ * @throws IOException if an error occurs during reading from the descriptor
+ * {@link InputStream}
*/
- @Deprecated // TODO: (OPENNLP-1174) just remove when back-compat is no longer needed
- interface XmlFeatureGeneratorFactory {
+ public static AdaptiveFeatureGenerator create(InputStream xmlDescriptorIn,
+ FeatureGeneratorResourceProvider resourceManager)
+ throws IOException {
- /**
- * Creates an {@link AdaptiveFeatureGenerator} from a the describing
- * XML element.
- *
- * @param generatorElement the element which contains the configuration
- * @param resourceManager the resource manager which could be used
- * to access referenced resources
- *
- * @return the configured {@link AdaptiveFeatureGenerator}
- */
- AdaptiveFeatureGenerator create(Element generatorElement,
- FeatureGeneratorResourceProvider resourceManager) throws InvalidFormatException;
+ final org.w3c.dom.Document xmlDescriptorDOM = createDOM(xmlDescriptorIn);
+ final Element generatorElement = xmlDescriptorDOM.getDocumentElement();
+
+ return createGenerator(generatorElement, resourceManager);
+ }
+
+ /**
+ * Creates a {@link AdaptiveFeatureGenerator} for the provided element.
+ * To accomplish this it looks up the corresponding factory by the
+ * element tag name. The factory is then responsible for the creation
+ * of the generator from the element.
+ *
+ * @param generatorElement must not be {@code null}
+ * @param resourceManager may be {@code null}
+ * @return a {@link AdaptiveFeatureGenerator}
+ */
+ private static AdaptiveFeatureGenerator buildGenerator(Element generatorElement,
+ FeatureGeneratorResourceProvider resourceManager)
+ throws InvalidFormatException {
+
+ if (generatorElement == null) {
+ throw new IllegalArgumentException("generatorElement must not be NULL");
Review Comment:
Please add to Javadoc that this exception might occur.
##########
opennlp-tools/src/main/java/opennlp/tools/util/featuregen/GeneratorFactory.java:
##########
@@ -95,25 +87,235 @@
public class GeneratorFactory {
/**
- * The {@link XmlFeatureGeneratorFactory} is responsible to construct
- * an {@link AdaptiveFeatureGenerator} from an given XML {@link Element}
- * which contains all necessary configuration if any.
+ * Creates an {@link AdaptiveFeatureGenerator} from an provided XML descriptor.
+ * <p>
+ * Usually this XML descriptor contains a set of nested feature generators
+ * which are then used to generate the features by one of the opennlp
+ * components.
+ *
+ * @param xmlDescriptorIn the {@link InputStream} from which the descriptor
+ * is read, the stream remains open and must be closed by the caller.
+ * @param resourceManager the resource manager which is used to resolve resources
+ * referenced by a key in the descriptor
+ * @return created feature generators
+ * @throws IOException if an error occurs during reading from the descriptor
+ * {@link InputStream}
*/
- @Deprecated // TODO: (OPENNLP-1174) just remove when back-compat is no longer needed
- interface XmlFeatureGeneratorFactory {
+ public static AdaptiveFeatureGenerator create(InputStream xmlDescriptorIn,
+ FeatureGeneratorResourceProvider resourceManager)
+ throws IOException {
- /**
- * Creates an {@link AdaptiveFeatureGenerator} from a the describing
- * XML element.
- *
- * @param generatorElement the element which contains the configuration
- * @param resourceManager the resource manager which could be used
- * to access referenced resources
- *
- * @return the configured {@link AdaptiveFeatureGenerator}
- */
- AdaptiveFeatureGenerator create(Element generatorElement,
- FeatureGeneratorResourceProvider resourceManager) throws InvalidFormatException;
+ final org.w3c.dom.Document xmlDescriptorDOM = createDOM(xmlDescriptorIn);
+ final Element generatorElement = xmlDescriptorDOM.getDocumentElement();
+
+ return createGenerator(generatorElement, resourceManager);
+ }
+
+ /**
+ * Creates a {@link AdaptiveFeatureGenerator} for the provided element.
+ * To accomplish this it looks up the corresponding factory by the
+ * element tag name. The factory is then responsible for the creation
+ * of the generator from the element.
+ *
+ * @param generatorElement must not be {@code null}
+ * @param resourceManager may be {@code null}
+ * @return a {@link AdaptiveFeatureGenerator}
+ */
+ private static AdaptiveFeatureGenerator buildGenerator(Element generatorElement,
+ FeatureGeneratorResourceProvider resourceManager)
+ throws InvalidFormatException {
+
+ if (generatorElement == null) {
+ throw new IllegalArgumentException("generatorElement must not be NULL");
+ }
+
+ final String className = generatorElement.getAttribute("class");
+ if (className.isBlank()) {
+ throw new InvalidFormatException("generator must have class attribute");
+ } else {
+ try {
+ final Class<?> factoryClass = Class.forName(className);
+ try {
+ final Constructor<?> constructor = factoryClass.getConstructor();
+ final AbstractXmlFeatureGeneratorFactory factory =
+ (AbstractXmlFeatureGeneratorFactory) constructor.newInstance();
+ factory.init(generatorElement, resourceManager);
+ return factory.create();
+ } catch (NoSuchMethodException | InvocationTargetException | InstantiationException
+ | IllegalAccessException e) {
+ throw new RuntimeException(e);
+ }
+ } catch (ClassNotFoundException e) {
+ throw new RuntimeException(e);
+ }
+ }
+ }
+
+ /**
+ * Creates a {@link AdaptiveFeatureGenerator} for the provided element.
Review Comment:
Should read "An..." as next word starts with letter A.
##########
opennlp-tools/src/main/java/opennlp/tools/util/featuregen/GeneratorFactory.java:
##########
@@ -95,25 +87,235 @@
public class GeneratorFactory {
/**
- * The {@link XmlFeatureGeneratorFactory} is responsible to construct
- * an {@link AdaptiveFeatureGenerator} from an given XML {@link Element}
- * which contains all necessary configuration if any.
+ * Creates an {@link AdaptiveFeatureGenerator} from an provided XML descriptor.
+ * <p>
+ * Usually this XML descriptor contains a set of nested feature generators
+ * which are then used to generate the features by one of the opennlp
+ * components.
+ *
+ * @param xmlDescriptorIn the {@link InputStream} from which the descriptor
+ * is read, the stream remains open and must be closed by the caller.
+ * @param resourceManager the resource manager which is used to resolve resources
+ * referenced by a key in the descriptor
+ * @return created feature generators
+ * @throws IOException if an error occurs during reading from the descriptor
+ * {@link InputStream}
*/
- @Deprecated // TODO: (OPENNLP-1174) just remove when back-compat is no longer needed
- interface XmlFeatureGeneratorFactory {
+ public static AdaptiveFeatureGenerator create(InputStream xmlDescriptorIn,
+ FeatureGeneratorResourceProvider resourceManager)
+ throws IOException {
- /**
- * Creates an {@link AdaptiveFeatureGenerator} from a the describing
- * XML element.
- *
- * @param generatorElement the element which contains the configuration
- * @param resourceManager the resource manager which could be used
- * to access referenced resources
- *
- * @return the configured {@link AdaptiveFeatureGenerator}
- */
- AdaptiveFeatureGenerator create(Element generatorElement,
- FeatureGeneratorResourceProvider resourceManager) throws InvalidFormatException;
+ final org.w3c.dom.Document xmlDescriptorDOM = createDOM(xmlDescriptorIn);
+ final Element generatorElement = xmlDescriptorDOM.getDocumentElement();
+
+ return createGenerator(generatorElement, resourceManager);
+ }
+
+ /**
+ * Creates a {@link AdaptiveFeatureGenerator} for the provided element.
+ * To accomplish this it looks up the corresponding factory by the
+ * element tag name. The factory is then responsible for the creation
+ * of the generator from the element.
+ *
+ * @param generatorElement must not be {@code null}
+ * @param resourceManager may be {@code null}
+ * @return a {@link AdaptiveFeatureGenerator}
+ */
+ private static AdaptiveFeatureGenerator buildGenerator(Element generatorElement,
+ FeatureGeneratorResourceProvider resourceManager)
+ throws InvalidFormatException {
+
+ if (generatorElement == null) {
+ throw new IllegalArgumentException("generatorElement must not be NULL");
+ }
+
+ final String className = generatorElement.getAttribute("class");
+ if (className.isBlank()) {
+ throw new InvalidFormatException("generator must have class attribute");
+ } else {
+ try {
+ final Class<?> factoryClass = Class.forName(className);
+ try {
+ final Constructor<?> constructor = factoryClass.getConstructor();
+ final AbstractXmlFeatureGeneratorFactory factory =
+ (AbstractXmlFeatureGeneratorFactory) constructor.newInstance();
+ factory.init(generatorElement, resourceManager);
+ return factory.create();
+ } catch (NoSuchMethodException | InvocationTargetException | InstantiationException
+ | IllegalAccessException e) {
+ throw new RuntimeException(e);
+ }
+ } catch (ClassNotFoundException e) {
+ throw new RuntimeException(e);
+ }
+ }
+ }
+
+ /**
+ * Creates a {@link AdaptiveFeatureGenerator} for the provided element.
+ * To accomplish this it looks up the corresponding factory by the
+ * element tag name. The factory is then responsible for the creation
+ * of the generator from the element.
+ *
+ * @param generatorElement must not be {@code null}
+ * @param resourceManager may be {@code null}
+ * @return a {@link AdaptiveFeatureGenerator}
+ */
+ private static AdaptiveFeatureGenerator createGenerator(Element generatorElement,
+ FeatureGeneratorResourceProvider resourceManager)
+ throws InvalidFormatException {
+
+ if (generatorElement == null) {
+ throw new IllegalArgumentException("generatorElement must not be NULL");
+ }
+
+ final String elementName = generatorElement.getTagName();
+
+ // check it is new format?
+ if ("featureGenerators".equals(elementName)) {
+
+ final List<AdaptiveFeatureGenerator> generators = new ArrayList<>();
+ final NodeList childNodes = generatorElement.getChildNodes();
+ for (int i = 0; i < childNodes.getLength(); i++) {
+ final Node childNode = childNodes.item(i);
+ if (childNode instanceof Element elem) {
+ final String type = elem.getTagName();
+ if ("generator".equals(type)) {
+ generators.add(buildGenerator(elem, resourceManager));
+ } else {
+ throw new InvalidFormatException("Unexpected element: " + elementName);
+ }
+ }
+ }
+
+ AdaptiveFeatureGenerator featureGenerator;
+ if (generators.size() == 1) {
+ featureGenerator = generators.get(0);
+ } else if (generators.size() > 1) {
+ featureGenerator = new AggregatedFeatureGenerator(generators.toArray(
+ new AdaptiveFeatureGenerator[0]));
+ } else {
+ throw new InvalidFormatException("featureGenerators must have one or more generators");
+ }
+
+ // disallow manually specifying CachedFeatureGenerator
+ if (featureGenerator instanceof CachedFeatureGenerator) {
+ throw new InvalidFormatException("CachedFeatureGeneratorFactory cannot be specified manually." +
+ "Use cache=\"true\" attribute in featureGenerators element instead.");
+ }
+
+ // check cache usage
+ if (Boolean.parseBoolean(generatorElement.getAttribute("cache"))) {
+ return new CachedFeatureGenerator(featureGenerator);
+ } else {
+ return featureGenerator;
+ }
+ } else {
+ throw new IllegalArgumentException(
+ "[OPENNLP-1174] - Classic configuration format is no longer supported!");
+ }
+ }
+
+ private static org.w3c.dom.Document createDOM(InputStream xmlDescriptorIn)
+ throws IOException {
+
+ final DocumentBuilder documentBuilder = XmlUtil.createDocumentBuilder();
+
+ org.w3c.dom.Document xmlDescriptorDOM;
+
+ try {
+ xmlDescriptorDOM = documentBuilder.parse(xmlDescriptorIn);
+ } catch (SAXException e) {
+ throw new InvalidFormatException("Descriptor is not valid XML!", e);
+ }
+ return xmlDescriptorDOM;
+ }
+
+ public static Map<String, ArtifactSerializer<?>> extractArtifactSerializerMappings(
+ InputStream xmlDescriptorIn) throws IOException {
+
+ final org.w3c.dom.Document xmlDescriptorDOM = createDOM(xmlDescriptorIn);
+ final Element element = xmlDescriptorDOM.getDocumentElement();
+
+ final String elementName = element.getTagName();
+
+ // check it is new format?
+ if ("featureGenerators".equals(elementName)) {
+ return addMappingsFromXmlChildren(element.getChildNodes(), new HashMap<>());
+ } else {
+ throw new IllegalArgumentException(
+ "[OPENNLP-1174] - Classic configuration format is no longer supported!");
+ }
+ }
+
+ private static void extractArtifactSerializerMappings(
+ Map<String, ArtifactSerializer<?>> mapping, Element element) {
+ final String className = element.getAttribute("class");
+ if (!className.isBlank()) {
+ try {
+ final Class<?> factoryClass = Class.forName(className);
+ try {
+ final Constructor<?> constructor = factoryClass.getConstructor();
+ final AbstractXmlFeatureGeneratorFactory factory =
+ (AbstractXmlFeatureGeneratorFactory) constructor.newInstance();
+ factory.init(element, null);
+ final Map<String, ArtifactSerializer<?>> map = factory.getArtifactSerializerMapping();
+ if (map != null) {
+ mapping.putAll(map);
+ }
+ } catch (NoSuchMethodException | InvocationTargetException | InstantiationException
+ | IllegalAccessException e) {
+ throw new RuntimeException(e);
+ } catch (InvalidFormatException ignored) {
+ }
+ } catch (ClassNotFoundException e) {
+ throw new RuntimeException(e);
+ }
+ }
+
+ addMappingsFromXmlChildren(element.getChildNodes(), mapping);
+ }
+
+ private static Map<String, ArtifactSerializer<?>> addMappingsFromXmlChildren(
+ final NodeList nodes, final Map<String, ArtifactSerializer<?>> mapping) {
+ for (int i = 0; i < nodes.getLength(); i++) {
+ if (nodes.item(i) instanceof Element childElem) {
+ if ("generator".equals(childElem.getTagName())) {
+ extractArtifactSerializerMappings(mapping, childElem);
+ }
+ }
+ }
+ return mapping;
+ }
+
+ /**
+ * Provides a list with all the elements in the xml feature descriptor.
+ *
+ * @param xmlDescriptorIn the xml feature descriptor
+ * @return a list containing all elements
+ * @throws IOException if inputstream cannot be open
+ * @throws InvalidFormatException if xml is not well-formed
+ */
+ public static List<Element> getDescriptorElements(InputStream xmlDescriptorIn)
+ throws IOException {
+
+ final List<Element> elements = new ArrayList<>();
+ final org.w3c.dom.Document xmlDescriptorDOM = createDOM(xmlDescriptorIn);
+ final XPath xPath = XPathFactory.newInstance().newXPath();
+ NodeList allElements;
+ try {
+ final XPathExpression exp = xPath.compile("//*");
+ allElements = (NodeList) exp.evaluate(xmlDescriptorDOM.getDocumentElement(), XPathConstants.NODESET);
+ } catch (XPathExpressionException e) {
+ throw new IllegalStateException("The hard coded XPath expression should always be valid!");
Review Comment:
Please add to JavaDoc that this exception can occur.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [opennlp] mawiesne commented on pull request #541: OPENNLP-1174 - Remove classic format support in feature generator XML config
Posted by "mawiesne (via GitHub)" <gi...@apache.org>.
mawiesne commented on PR #541:
URL: https://github.com/apache/opennlp/pull/541#issuecomment-1596086817
Thx @rzo1 for the PR! 🙏
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [opennlp] mawiesne merged pull request #541: OPENNLP-1174 - Remove classic format support in feature generator XML config
Posted by "mawiesne (via GitHub)" <gi...@apache.org>.
mawiesne merged PR #541:
URL: https://github.com/apache/opennlp/pull/541
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [opennlp] rzo1 commented on a diff in pull request #541: OPENNLP-1174 - Remove classic format support in feature generator XML config
Posted by "rzo1 (via GitHub)" <gi...@apache.org>.
rzo1 commented on code in PR #541:
URL: https://github.com/apache/opennlp/pull/541#discussion_r1233251530
##########
opennlp-tools/src/main/java/opennlp/tools/util/featuregen/GeneratorFactory.java:
##########
@@ -95,25 +87,235 @@
public class GeneratorFactory {
/**
- * The {@link XmlFeatureGeneratorFactory} is responsible to construct
- * an {@link AdaptiveFeatureGenerator} from an given XML {@link Element}
- * which contains all necessary configuration if any.
+ * Creates an {@link AdaptiveFeatureGenerator} from an provided XML descriptor.
+ * <p>
+ * Usually this XML descriptor contains a set of nested feature generators
+ * which are then used to generate the features by one of the opennlp
+ * components.
+ *
+ * @param xmlDescriptorIn the {@link InputStream} from which the descriptor
+ * is read, the stream remains open and must be closed by the caller.
+ * @param resourceManager the resource manager which is used to resolve resources
+ * referenced by a key in the descriptor
+ * @return created feature generators
+ * @throws IOException if an error occurs during reading from the descriptor
+ * {@link InputStream}
*/
- @Deprecated // TODO: (OPENNLP-1174) just remove when back-compat is no longer needed
- interface XmlFeatureGeneratorFactory {
+ public static AdaptiveFeatureGenerator create(InputStream xmlDescriptorIn,
+ FeatureGeneratorResourceProvider resourceManager)
+ throws IOException {
- /**
- * Creates an {@link AdaptiveFeatureGenerator} from a the describing
- * XML element.
- *
- * @param generatorElement the element which contains the configuration
- * @param resourceManager the resource manager which could be used
- * to access referenced resources
- *
- * @return the configured {@link AdaptiveFeatureGenerator}
- */
- AdaptiveFeatureGenerator create(Element generatorElement,
- FeatureGeneratorResourceProvider resourceManager) throws InvalidFormatException;
+ final org.w3c.dom.Document xmlDescriptorDOM = createDOM(xmlDescriptorIn);
+ final Element generatorElement = xmlDescriptorDOM.getDocumentElement();
+
+ return createGenerator(generatorElement, resourceManager);
+ }
+
+ /**
+ * Creates a {@link AdaptiveFeatureGenerator} for the provided element.
+ * To accomplish this it looks up the corresponding factory by the
+ * element tag name. The factory is then responsible for the creation
+ * of the generator from the element.
+ *
+ * @param generatorElement must not be {@code null}
+ * @param resourceManager may be {@code null}
+ * @return a {@link AdaptiveFeatureGenerator}
+ */
+ private static AdaptiveFeatureGenerator buildGenerator(Element generatorElement,
+ FeatureGeneratorResourceProvider resourceManager)
+ throws InvalidFormatException {
+
+ if (generatorElement == null) {
+ throw new IllegalArgumentException("generatorElement must not be NULL");
+ }
+
+ final String className = generatorElement.getAttribute("class");
+ if (className.isBlank()) {
+ throw new InvalidFormatException("generator must have class attribute");
+ } else {
+ try {
+ final Class<?> factoryClass = Class.forName(className);
+ try {
+ final Constructor<?> constructor = factoryClass.getConstructor();
+ final AbstractXmlFeatureGeneratorFactory factory =
+ (AbstractXmlFeatureGeneratorFactory) constructor.newInstance();
+ factory.init(generatorElement, resourceManager);
+ return factory.create();
+ } catch (NoSuchMethodException | InvocationTargetException | InstantiationException
+ | IllegalAccessException e) {
+ throw new RuntimeException(e);
+ }
+ } catch (ClassNotFoundException e) {
+ throw new RuntimeException(e);
+ }
+ }
+ }
+
+ /**
+ * Creates a {@link AdaptiveFeatureGenerator} for the provided element.
+ * To accomplish this it looks up the corresponding factory by the
+ * element tag name. The factory is then responsible for the creation
+ * of the generator from the element.
+ *
+ * @param generatorElement must not be {@code null}
+ * @param resourceManager may be {@code null}
+ * @return a {@link AdaptiveFeatureGenerator}
+ */
+ private static AdaptiveFeatureGenerator createGenerator(Element generatorElement,
+ FeatureGeneratorResourceProvider resourceManager)
+ throws InvalidFormatException {
+
+ if (generatorElement == null) {
+ throw new IllegalArgumentException("generatorElement must not be NULL");
+ }
+
+ final String elementName = generatorElement.getTagName();
+
+ // check it is new format?
+ if ("featureGenerators".equals(elementName)) {
+
+ final List<AdaptiveFeatureGenerator> generators = new ArrayList<>();
+ final NodeList childNodes = generatorElement.getChildNodes();
+ for (int i = 0; i < childNodes.getLength(); i++) {
+ final Node childNode = childNodes.item(i);
+ if (childNode instanceof Element elem) {
+ final String type = elem.getTagName();
+ if ("generator".equals(type)) {
+ generators.add(buildGenerator(elem, resourceManager));
+ } else {
+ throw new InvalidFormatException("Unexpected element: " + elementName);
+ }
+ }
+ }
+
+ AdaptiveFeatureGenerator featureGenerator;
+ if (generators.size() == 1) {
+ featureGenerator = generators.get(0);
+ } else if (generators.size() > 1) {
+ featureGenerator = new AggregatedFeatureGenerator(generators.toArray(
+ new AdaptiveFeatureGenerator[0]));
+ } else {
+ throw new InvalidFormatException("featureGenerators must have one or more generators");
+ }
+
+ // disallow manually specifying CachedFeatureGenerator
+ if (featureGenerator instanceof CachedFeatureGenerator) {
+ throw new InvalidFormatException("CachedFeatureGeneratorFactory cannot be specified manually." +
+ "Use cache=\"true\" attribute in featureGenerators element instead.");
+ }
+
+ // check cache usage
+ if (Boolean.parseBoolean(generatorElement.getAttribute("cache"))) {
+ return new CachedFeatureGenerator(featureGenerator);
+ } else {
+ return featureGenerator;
+ }
+ } else {
+ throw new IllegalArgumentException(
+ "[OPENNLP-1174] - Classic configuration format is no longer supported!");
+ }
+ }
+
+ private static org.w3c.dom.Document createDOM(InputStream xmlDescriptorIn)
+ throws IOException {
+
+ final DocumentBuilder documentBuilder = XmlUtil.createDocumentBuilder();
+
+ org.w3c.dom.Document xmlDescriptorDOM;
+
+ try {
+ xmlDescriptorDOM = documentBuilder.parse(xmlDescriptorIn);
+ } catch (SAXException e) {
+ throw new InvalidFormatException("Descriptor is not valid XML!", e);
+ }
+ return xmlDescriptorDOM;
+ }
+
+ public static Map<String, ArtifactSerializer<?>> extractArtifactSerializerMappings(
+ InputStream xmlDescriptorIn) throws IOException {
+
+ final org.w3c.dom.Document xmlDescriptorDOM = createDOM(xmlDescriptorIn);
+ final Element element = xmlDescriptorDOM.getDocumentElement();
+
+ final String elementName = element.getTagName();
+
+ // check it is new format?
+ if ("featureGenerators".equals(elementName)) {
+ return addMappingsFromXmlChildren(element.getChildNodes(), new HashMap<>());
+ } else {
+ throw new IllegalArgumentException(
+ "[OPENNLP-1174] - Classic configuration format is no longer supported!");
+ }
+ }
+
+ private static void extractArtifactSerializerMappings(
+ Map<String, ArtifactSerializer<?>> mapping, Element element) {
+ final String className = element.getAttribute("class");
+ if (!className.isBlank()) {
+ try {
+ final Class<?> factoryClass = Class.forName(className);
+ try {
+ final Constructor<?> constructor = factoryClass.getConstructor();
+ final AbstractXmlFeatureGeneratorFactory factory =
+ (AbstractXmlFeatureGeneratorFactory) constructor.newInstance();
+ factory.init(element, null);
+ final Map<String, ArtifactSerializer<?>> map = factory.getArtifactSerializerMapping();
+ if (map != null) {
+ mapping.putAll(map);
+ }
+ } catch (NoSuchMethodException | InvocationTargetException | InstantiationException
+ | IllegalAccessException e) {
+ throw new RuntimeException(e);
+ } catch (InvalidFormatException ignored) {
+ }
+ } catch (ClassNotFoundException e) {
+ throw new RuntimeException(e);
+ }
+ }
+
+ addMappingsFromXmlChildren(element.getChildNodes(), mapping);
+ }
+
+ private static Map<String, ArtifactSerializer<?>> addMappingsFromXmlChildren(
+ final NodeList nodes, final Map<String, ArtifactSerializer<?>> mapping) {
+ for (int i = 0; i < nodes.getLength(); i++) {
+ if (nodes.item(i) instanceof Element childElem) {
+ if ("generator".equals(childElem.getTagName())) {
+ extractArtifactSerializerMappings(mapping, childElem);
+ }
+ }
+ }
+ return mapping;
+ }
+
+ /**
+ * Provides a list with all the elements in the xml feature descriptor.
+ *
+ * @param xmlDescriptorIn the xml feature descriptor
+ * @return a list containing all elements
+ * @throws IOException if inputstream cannot be open
+ * @throws InvalidFormatException if xml is not well-formed
+ */
+ public static List<Element> getDescriptorElements(InputStream xmlDescriptorIn)
+ throws IOException {
+
+ final List<Element> elements = new ArrayList<>();
+ final org.w3c.dom.Document xmlDescriptorDOM = createDOM(xmlDescriptorIn);
+ final XPath xPath = XPathFactory.newInstance().newXPath();
+ NodeList allElements;
+ try {
+ final XPathExpression exp = xPath.compile("//*");
+ allElements = (NodeList) exp.evaluate(xmlDescriptorDOM.getDocumentElement(), XPathConstants.NODESET);
+ } catch (XPathExpressionException e) {
+ throw new IllegalStateException("The hard coded XPath expression should always be valid!");
Review Comment:
Should be `InvalidFormatException` here, which is already documented in the JavaDoc but code was misbehaving. Changed to `InvalidFormatException`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@opennlp.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org