You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2022/12/19 21:17:55 UTC
[GitHub] [nifi] dan-s1 opened a new pull request, #6798: [NIFI-6498] Set error event listener on both the TransformerFactory a…
dan-s1 opened a new pull request, #6798:
URL: https://github.com/apache/nifi/pull/6798
…nd Transformer to enable logging errors when processing transformation instructions and when running the transformation itself. Also added ability to log errors from messages that are emitted with <xsl:message/>.
<!-- 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. -->
# Summary
[NIFI-6498](https://issues.apache.org/jira/browse/NIFI-6498)
# Tracking
Please complete the following tracking steps prior to pull request creation.
### Issue Tracking
- [ ] [Apache NiFi Jira](https://issues.apache.org/jira/browse/NIFI) issue created
### Pull Request Tracking
- [ ] Pull Request title starts with Apache NiFi Jira issue number, such as `NIFI-00000`
- [ ] Pull Request commit message starts with Apache NiFi Jira issue number, as such `NIFI-00000`
### Pull Request Formatting
- [ ] Pull Request based on current revision of the `main` branch
- [ ] Pull Request refers to a feature branch with one commit containing changes
# Verification
Please indicate the verification steps performed prior to pull request creation.
### Build
- [ ] Build completed using `mvn clean install -P contrib-check`
- [ ] JDK 8
- [ ] JDK 11
- [ ] JDK 17
### Licensing
- [ ] New dependencies are compatible with the [Apache License 2.0](https://apache.org/licenses/LICENSE-2.0) according to the [License Policy](https://www.apache.org/legal/resolved.html)
- [ ] New dependencies are documented in applicable `LICENSE` and `NOTICE` files
### Documentation
- [ ] Documentation formatting appears as expected in rendered files
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] dan-s1 commented on pull request #6798: [NIFI-6498] Set error event listener on both the TransformerFactory a…
Posted by GitBox <gi...@apache.org>.
dan-s1 commented on PR #6798:
URL: https://github.com/apache/nifi/pull/6798#issuecomment-1361586482
@exceptionfactory I just learnt something about the TestRunner that we could access logging that occurred during running a processor. Do you think I should add assertions to the unit tests I added that confirms there was logging when ever a compile time or runtime error occurs when transforming XML?
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on a diff in pull request #6798: [NIFI-6498] Set error event listener on both the TransformerFactory a…
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on code in PR #6798:
URL: https://github.com/apache/nifi/pull/6798#discussion_r1053708567
##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/TransformXml.java:
##########
@@ -372,4 +382,27 @@ private Source getSecureSource(final StreamSource streamSource) throws Transform
throw new TransformerConfigurationException("XSLT Source Stream Reader creation failed", e);
}
}
+}
+
+class ErrorListenerLogger implements ErrorListener {
+ private final ComponentLog logger;
+
+ ErrorListenerLogger(ComponentLog logger) {
+ this.logger = logger;
+ }
+
+ @Override
+ public void warning(TransformerException exception) throws TransformerException {
+ logger.warn(exception.getMessageAndLocation());
+ }
+
+ @Override
+ public void error(TransformerException exception) throws TransformerException {
+ logger.error(exception.getMessageAndLocation());
+ }
+
+ @Override
+ public void fatalError(TransformerException exception) throws TransformerException {
Review Comment:
Yes, thanks!
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] dan-s1 commented on pull request #6798: [NIFI-6498] Set error event listener on both the TransformerFactory a…
Posted by GitBox <gi...@apache.org>.
dan-s1 commented on PR #6798:
URL: https://github.com/apache/nifi/pull/6798#issuecomment-1361674649
@exceptionfactory I added the assertions. Please let me know if that is enough or if there should be more. Thanks!
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] dan-s1 commented on pull request #6798: [NIFI-6498] Set error event listener on both the TransformerFactory a…
Posted by GitBox <gi...@apache.org>.
dan-s1 commented on PR #6798:
URL: https://github.com/apache/nifi/pull/6798#issuecomment-1359669064
@exceptionfactory They actually do assert behavior when terminate is used whether the transform succeeds (terminate=no) or fails (terminate=yes).
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on pull request #6798: [NIFI-6498] Set error event listener on both the TransformerFactory a…
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on PR #6798:
URL: https://github.com/apache/nifi/pull/6798#issuecomment-1359749445
Thanks @dan-s1, I was expecting something related to the message output, but on further review, I see the success and failure output scenarios, which seem worth maintaining.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on pull request #6798: [NIFI-6498] Set error event listener on both the TransformerFactory a…
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on PR #6798:
URL: https://github.com/apache/nifi/pull/6798#issuecomment-1358829602
Thanks for the reply @dan-s1.
Although the `testMessage` methods include the `xsl:message` examples, they do not appear to assert any particular behavior, which makes sense given that it just ends up writing messages. For that reason, it seems best to remove those test methods and related files.
Reviewing the [Saxon Configuration Features](https://www.saxonica.com/html/documentation10/configuration/config-features.html) there is a reference to a message emitter class named `MessageWarner` that notifies the standard JAXP `ErrorListener`. Introducing this feature would still place a direct relationship to the Saxon implementation, so it still seems like it would be best to avoid for now, since `xsl:message` is a defined element used for testing.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory closed pull request #6798: [NIFI-6498] Set error event listener on both the TransformerFactory a…
Posted by GitBox <gi...@apache.org>.
exceptionfactory closed pull request #6798: [NIFI-6498] Set error event listener on both the TransformerFactory a…
URL: https://github.com/apache/nifi/pull/6798
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] dan-s1 commented on pull request #6798: [NIFI-6498] Set error event listener on both the TransformerFactory a…
Posted by GitBox <gi...@apache.org>.
dan-s1 commented on PR #6798:
URL: https://github.com/apache/nifi/pull/6798#issuecomment-1358810614
@exceptionfactory Thank you for taking the time and reviewing this PR. I would just like to clarify what you would like me to remove. I understand you would like me to remove method setMessageReceiver which has the Saxon specific code. Would you like me also to remove unit tests testMessageTerminate and testMessageNonTerminate also or should I keep them as they still demonstrate behavior that TransformXml exhibits when xsl:message is used with terminate and without terminate?
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] dan-s1 commented on pull request #6798: [NIFI-6498] Set error event listener on both the TransformerFactory a…
Posted by GitBox <gi...@apache.org>.
dan-s1 commented on PR #6798:
URL: https://github.com/apache/nifi/pull/6798#issuecomment-1359922428
@exceptionfactory So i ended up only removing the method setMessageReceiver. With that, the only issue which remains is when there is a warning from xsl:message for some reason its not redirected to use the ErrorListener configured. I did find the following [Saxon issue](https://saxonica.plan.io/issues/2581) which details a similar issue but not the exact one where it gives a lot of history behind Saxon's implementation on handling xsl:message.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on a diff in pull request #6798: [NIFI-6498] Set error event listener on both the TransformerFactory a…
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on code in PR #6798:
URL: https://github.com/apache/nifi/pull/6798#discussion_r1053670702
##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/TransformXml.java:
##########
@@ -372,4 +382,27 @@ private Source getSecureSource(final StreamSource streamSource) throws Transform
throw new TransformerConfigurationException("XSLT Source Stream Reader creation failed", e);
}
}
+}
+
+class ErrorListenerLogger implements ErrorListener {
+ private final ComponentLog logger;
+
+ ErrorListenerLogger(ComponentLog logger) {
+ this.logger = logger;
+ }
+
+ @Override
+ public void warning(TransformerException exception) throws TransformerException {
+ logger.warn(exception.getMessageAndLocation());
+ }
+
+ @Override
+ public void error(TransformerException exception) throws TransformerException {
+ logger.error(exception.getMessageAndLocation());
Review Comment:
```suggestion
logger.error(exception.getMessageAndLocation(), exception);
```
##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/TransformXml.java:
##########
@@ -372,4 +382,27 @@ private Source getSecureSource(final StreamSource streamSource) throws Transform
throw new TransformerConfigurationException("XSLT Source Stream Reader creation failed", e);
}
}
+}
+
+class ErrorListenerLogger implements ErrorListener {
Review Comment:
This can be `private` and `static`:
```suggestion
private static class ErrorListenerLogger implements ErrorListener {
```
##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/TransformXml.java:
##########
@@ -372,4 +382,27 @@ private Source getSecureSource(final StreamSource streamSource) throws Transform
throw new TransformerConfigurationException("XSLT Source Stream Reader creation failed", e);
}
}
+}
+
+class ErrorListenerLogger implements ErrorListener {
+ private final ComponentLog logger;
+
+ ErrorListenerLogger(ComponentLog logger) {
+ this.logger = logger;
+ }
+
+ @Override
+ public void warning(TransformerException exception) throws TransformerException {
+ logger.warn(exception.getMessageAndLocation());
Review Comment:
```suggestion
logger.warn(exception.getMessageAndLocation(), exception);
```
##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestTransformXml.java:
##########
@@ -297,4 +297,47 @@ public void testTransformSecureProcessingEnabledXslWithEntity() throws IOExcepti
transformed.assertContentEquals("<?xml version=\"1.0\" encoding=\"UTF-8\"?>");
}
+
+ @Test
+ public void testNonMatchingTemplateTag() throws IOException {
+ final TestRunner runner = TestRunners.newTestRunner(new TransformXml());
+ runner.setProperty("header", "Test for mod");
+ runner.setProperty(TransformXml.XSLT_FILE_NAME, "src/test/resources/TestTransformXml/nonMatchingEndTag.xsl");
+
+ final Map<String, String> attributes = new HashMap<>();
+ runner.enqueue(Paths.get("src/test/resources/TestTransformXml/math.xml"), attributes);
Review Comment:
```suggestion
runner.enqueue(Paths.get("src/test/resources/TestTransformXml/math.xml"));
```
##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestTransformXml.java:
##########
@@ -297,4 +297,47 @@ public void testTransformSecureProcessingEnabledXslWithEntity() throws IOExcepti
transformed.assertContentEquals("<?xml version=\"1.0\" encoding=\"UTF-8\"?>");
}
+
+ @Test
+ public void testNonMatchingTemplateTag() throws IOException {
+ final TestRunner runner = TestRunners.newTestRunner(new TransformXml());
+ runner.setProperty("header", "Test for mod");
+ runner.setProperty(TransformXml.XSLT_FILE_NAME, "src/test/resources/TestTransformXml/nonMatchingEndTag.xsl");
+
+ final Map<String, String> attributes = new HashMap<>();
+ runner.enqueue(Paths.get("src/test/resources/TestTransformXml/math.xml"), attributes);
+ runner.run();
+
+ runner.assertAllFlowFilesTransferred(TransformXml.REL_FAILURE);
+ }
+
+ @Test
+ public void testMessageTerminate() throws IOException {
+ final TestRunner runner = TestRunners.newTestRunner(new TransformXml());
+ runner.setProperty("header", "Test message terminate");
+ runner.setProperty(TransformXml.XSLT_FILE_NAME, "src/test/resources/TestTransformXml/employeeMessageTerminate.xsl");
+
+ final Map<String, String> attributes = new HashMap<>();
+ runner.enqueue(Paths.get("src/test/resources/TestTransformXml/employee.xml"), attributes);
Review Comment:
```suggestion
runner.enqueue(Paths.get("src/test/resources/TestTransformXml/employee.xml"));
```
##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestTransformXml.java:
##########
@@ -297,4 +297,47 @@ public void testTransformSecureProcessingEnabledXslWithEntity() throws IOExcepti
transformed.assertContentEquals("<?xml version=\"1.0\" encoding=\"UTF-8\"?>");
}
+
+ @Test
+ public void testNonMatchingTemplateTag() throws IOException {
+ final TestRunner runner = TestRunners.newTestRunner(new TransformXml());
+ runner.setProperty("header", "Test for mod");
+ runner.setProperty(TransformXml.XSLT_FILE_NAME, "src/test/resources/TestTransformXml/nonMatchingEndTag.xsl");
+
+ final Map<String, String> attributes = new HashMap<>();
+ runner.enqueue(Paths.get("src/test/resources/TestTransformXml/math.xml"), attributes);
+ runner.run();
+
+ runner.assertAllFlowFilesTransferred(TransformXml.REL_FAILURE);
+ }
+
+ @Test
+ public void testMessageTerminate() throws IOException {
+ final TestRunner runner = TestRunners.newTestRunner(new TransformXml());
+ runner.setProperty("header", "Test message terminate");
+ runner.setProperty(TransformXml.XSLT_FILE_NAME, "src/test/resources/TestTransformXml/employeeMessageTerminate.xsl");
+
+ final Map<String, String> attributes = new HashMap<>();
+ runner.enqueue(Paths.get("src/test/resources/TestTransformXml/employee.xml"), attributes);
+ runner.run();
+
+ runner.assertAllFlowFilesTransferred(TransformXml.REL_FAILURE);
+ }
+
+ @Test
+ public void testMessageNonTerminate() throws IOException {
+ final TestRunner runner = TestRunners.newTestRunner(new TransformXml());
+ runner.setProperty("header", "Test message non terminate");
+ runner.setProperty(TransformXml.XSLT_FILE_NAME, "src/test/resources/TestTransformXml/employeeMessageNonTerminate.xsl");
+
+ final Map<String, String> attributes = new HashMap<>();
+ runner.enqueue(Paths.get("src/test/resources/TestTransformXml/employee.xml"), attributes);
Review Comment:
```suggestion
runner.enqueue(Paths.get("src/test/resources/TestTransformXml/employee.xml"));
```
##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/TransformXml.java:
##########
@@ -372,4 +382,27 @@ private Source getSecureSource(final StreamSource streamSource) throws Transform
throw new TransformerConfigurationException("XSLT Source Stream Reader creation failed", e);
}
}
+}
+
+class ErrorListenerLogger implements ErrorListener {
+ private final ComponentLog logger;
+
+ ErrorListenerLogger(ComponentLog logger) {
+ this.logger = logger;
+ }
+
+ @Override
+ public void warning(TransformerException exception) throws TransformerException {
Review Comment:
```suggestion
public void warning(TransformerException exception) {
```
##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/TransformXml.java:
##########
@@ -372,4 +382,27 @@ private Source getSecureSource(final StreamSource streamSource) throws Transform
throw new TransformerConfigurationException("XSLT Source Stream Reader creation failed", e);
}
}
+}
+
+class ErrorListenerLogger implements ErrorListener {
+ private final ComponentLog logger;
+
+ ErrorListenerLogger(ComponentLog logger) {
+ this.logger = logger;
+ }
+
+ @Override
+ public void warning(TransformerException exception) throws TransformerException {
+ logger.warn(exception.getMessageAndLocation());
+ }
+
+ @Override
+ public void error(TransformerException exception) throws TransformerException {
+ logger.error(exception.getMessageAndLocation());
+ }
+
+ @Override
+ public void fatalError(TransformerException exception) throws TransformerException {
Review Comment:
```suggestion
public void fatalError(TransformerException exception) {
```
##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/TransformXml.java:
##########
@@ -372,4 +382,27 @@ private Source getSecureSource(final StreamSource streamSource) throws Transform
throw new TransformerConfigurationException("XSLT Source Stream Reader creation failed", e);
}
}
+}
+
+class ErrorListenerLogger implements ErrorListener {
+ private final ComponentLog logger;
+
+ ErrorListenerLogger(ComponentLog logger) {
+ this.logger = logger;
+ }
+
+ @Override
+ public void warning(TransformerException exception) throws TransformerException {
+ logger.warn(exception.getMessageAndLocation());
+ }
+
+ @Override
+ public void error(TransformerException exception) throws TransformerException {
Review Comment:
```suggestion
public void error(TransformerException exception) {
```
##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/TransformXml.java:
##########
@@ -372,4 +382,27 @@ private Source getSecureSource(final StreamSource streamSource) throws Transform
throw new TransformerConfigurationException("XSLT Source Stream Reader creation failed", e);
}
}
+}
+
+class ErrorListenerLogger implements ErrorListener {
+ private final ComponentLog logger;
+
+ ErrorListenerLogger(ComponentLog logger) {
+ this.logger = logger;
+ }
+
+ @Override
+ public void warning(TransformerException exception) throws TransformerException {
+ logger.warn(exception.getMessageAndLocation());
+ }
+
+ @Override
+ public void error(TransformerException exception) throws TransformerException {
+ logger.error(exception.getMessageAndLocation());
+ }
+
+ @Override
+ public void fatalError(TransformerException exception) throws TransformerException {
+ logger.log(LogLevel.FATAL, exception.getMessageAndLocation());
Review Comment:
```suggestion
logger.log(LogLevel.FATAL, exception.getMessageAndLocation(), exception);
```
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] dan-s1 commented on pull request #6798: [NIFI-6498] Set error event listener on both the TransformerFactory a…
Posted by GitBox <gi...@apache.org>.
dan-s1 commented on PR #6798:
URL: https://github.com/apache/nifi/pull/6798#issuecomment-1362034962
@exceptionfactory I believe I took care of all of your requests.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on a diff in pull request #6798: [NIFI-6498] Set error event listener on both the TransformerFactory a…
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on code in PR #6798:
URL: https://github.com/apache/nifi/pull/6798#discussion_r1053696166
##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/TransformXml.java:
##########
@@ -372,4 +382,27 @@ private Source getSecureSource(final StreamSource streamSource) throws Transform
throw new TransformerConfigurationException("XSLT Source Stream Reader creation failed", e);
}
}
+}
+
+class ErrorListenerLogger implements ErrorListener {
+ private final ComponentLog logger;
+
+ ErrorListenerLogger(ComponentLog logger) {
+ this.logger = logger;
+ }
+
+ @Override
+ public void warning(TransformerException exception) throws TransformerException {
+ logger.warn(exception.getMessageAndLocation());
+ }
+
+ @Override
+ public void error(TransformerException exception) throws TransformerException {
+ logger.error(exception.getMessageAndLocation());
+ }
+
+ @Override
+ public void fatalError(TransformerException exception) throws TransformerException {
Review Comment:
Yes, but if it isn't thrown, it doesn't need to be declared on the implementing method.
In this particular case, on further review, `fatalError` should throw the exception to prevent further processing.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on pull request #6798: [NIFI-6498] Set error event listener on both the TransformerFactory a…
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on PR #6798:
URL: https://github.com/apache/nifi/pull/6798#issuecomment-1361645012
> @exceptionfactory Do you mean just asserting for example getErrorMessages on the MockComponentLog is not empty? I thought it also may be helpful to have the contents of the message similar to in the ticket
>
> > Error on line 17 column 5 SXXP0003: Error reported by XML parser: The element type "xsl:tomplate" must be terminated by the matching end-tag "</xsl:tomplate>".
>
> in order to demonstrate the issue was fixed.
Asserting an exact message makes the unit test more brittle when it comes to library changes. In this particular case, asserting that the message contains the word `tomplate` seems like a good approach.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] dan-s1 commented on pull request #6798: [NIFI-6498] Set error event listener on both the TransformerFactory a…
Posted by GitBox <gi...@apache.org>.
dan-s1 commented on PR #6798:
URL: https://github.com/apache/nifi/pull/6798#issuecomment-1361636187
@exceptionfactory Do you mean just asserting for example getErrorMessages on the MockComponentLog is not empty? I thought it also may be helpful to have the contents of the message similar to in the ticket
> Error on line 17 column 5 SXXP0003: Error reported by XML parser: The element type "xsl:tomplate" must be terminated by the matching end-tag "</xsl:tomplate>".
in order to demonstrate the issue was fixed.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] dan-s1 commented on a diff in pull request #6798: [NIFI-6498] Set error event listener on both the TransformerFactory a…
Posted by GitBox <gi...@apache.org>.
dan-s1 commented on code in PR #6798:
URL: https://github.com/apache/nifi/pull/6798#discussion_r1054634880
##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestTransformXml.java:
##########
@@ -297,4 +301,61 @@ public void testTransformSecureProcessingEnabledXslWithEntity() throws IOExcepti
transformed.assertContentEquals("<?xml version=\"1.0\" encoding=\"UTF-8\"?>");
}
+
+ @Test
+ public void testNonMatchingTemplateTag() throws IOException {
+ final TestRunner runner = TestRunners.newTestRunner(new TransformXml());
+ runner.setProperty("header", "Test for mod");
+ runner.setProperty(TransformXml.XSLT_FILE_NAME, "src/test/resources/TestTransformXml/nonMatchingEndTag.xsl");
+
+ runner.enqueue(Paths.get("src/test/resources/TestTransformXml/math.xml"));
+ runner.run();
+
+ runner.assertAllFlowFilesTransferred(TransformXml.REL_FAILURE);
+ MockComponentLog logger = runner.getLogger();
+ assertFalse(logger.getErrorMessages().isEmpty());
+ assertTrue(logger.getErrorMessages().size() >= 1);
+ String firstMessage = logger.getErrorMessages().get(0).getMsg();
+ assertTrue(firstMessage.contains("xsl:template"));
+ assertTrue(firstMessage.contains("Line#"));
+ assertTrue(firstMessage.contains("Column#"));
Review Comment:
No problem. I thought asserting the existence of the Line# and Column# would be helpful as it addresses the request in the ticket to have them included and that they end up as part of a bulletin in the UI.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] dan-s1 commented on a diff in pull request #6798: [NIFI-6498] Set error event listener on both the TransformerFactory a…
Posted by GitBox <gi...@apache.org>.
dan-s1 commented on code in PR #6798:
URL: https://github.com/apache/nifi/pull/6798#discussion_r1053680263
##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/TransformXml.java:
##########
@@ -372,4 +382,27 @@ private Source getSecureSource(final StreamSource streamSource) throws Transform
throw new TransformerConfigurationException("XSLT Source Stream Reader creation failed", e);
}
}
+}
+
+class ErrorListenerLogger implements ErrorListener {
+ private final ComponentLog logger;
+
+ ErrorListenerLogger(ComponentLog logger) {
+ this.logger = logger;
+ }
+
+ @Override
+ public void warning(TransformerException exception) throws TransformerException {
+ logger.warn(exception.getMessageAndLocation());
+ }
+
+ @Override
+ public void error(TransformerException exception) throws TransformerException {
+ logger.error(exception.getMessageAndLocation());
+ }
+
+ @Override
+ public void fatalError(TransformerException exception) throws TransformerException {
Review Comment:
Isn't `throws TransformerException` part of the signature of the method defined in the interface?
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] dan-s1 commented on a diff in pull request #6798: [NIFI-6498] Set error event listener on both the TransformerFactory a…
Posted by GitBox <gi...@apache.org>.
dan-s1 commented on code in PR #6798:
URL: https://github.com/apache/nifi/pull/6798#discussion_r1053680263
##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/TransformXml.java:
##########
@@ -372,4 +382,27 @@ private Source getSecureSource(final StreamSource streamSource) throws Transform
throw new TransformerConfigurationException("XSLT Source Stream Reader creation failed", e);
}
}
+}
+
+class ErrorListenerLogger implements ErrorListener {
+ private final ComponentLog logger;
+
+ ErrorListenerLogger(ComponentLog logger) {
+ this.logger = logger;
+ }
+
+ @Override
+ public void warning(TransformerException exception) throws TransformerException {
+ logger.warn(exception.getMessageAndLocation());
+ }
+
+ @Override
+ public void error(TransformerException exception) throws TransformerException {
+ logger.error(exception.getMessageAndLocation());
+ }
+
+ @Override
+ public void fatalError(TransformerException exception) throws TransformerException {
Review Comment:
Isn't `throws TransformerException` part of the signature of the method interface?
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] dan-s1 commented on a diff in pull request #6798: [NIFI-6498] Set error event listener on both the TransformerFactory a…
Posted by GitBox <gi...@apache.org>.
dan-s1 commented on code in PR #6798:
URL: https://github.com/apache/nifi/pull/6798#discussion_r1053707406
##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/TransformXml.java:
##########
@@ -372,4 +382,27 @@ private Source getSecureSource(final StreamSource streamSource) throws Transform
throw new TransformerConfigurationException("XSLT Source Stream Reader creation failed", e);
}
}
+}
+
+class ErrorListenerLogger implements ErrorListener {
+ private final ComponentLog logger;
+
+ ErrorListenerLogger(ComponentLog logger) {
+ this.logger = logger;
+ }
+
+ @Override
+ public void warning(TransformerException exception) throws TransformerException {
+ logger.warn(exception.getMessageAndLocation());
+ }
+
+ @Override
+ public void error(TransformerException exception) throws TransformerException {
+ logger.error(exception.getMessageAndLocation());
+ }
+
+ @Override
+ public void fatalError(TransformerException exception) throws TransformerException {
Review Comment:
So I just rethrow the exception argument like below ?
`throw exception;`
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] dan-s1 commented on pull request #6798: [NIFI-6498] Set error event listener on both the TransformerFactory a…
Posted by GitBox <gi...@apache.org>.
dan-s1 commented on PR #6798:
URL: https://github.com/apache/nifi/pull/6798#issuecomment-1361455007
@exceptionfactory I made the changes you requested. I am not sure though what to do as two of the ci builds timed out. Please advise. Thanks!
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on pull request #6798: [NIFI-6498] Set error event listener on both the TransformerFactory a…
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on PR #6798:
URL: https://github.com/apache/nifi/pull/6798#issuecomment-1361599105
> Do you think I should add assertions to the unit tests I added that confirms there was logging when ever a compile time or runtime error occurs when transforming XML?
If you are inclined to add a check for a log at a given level, that would be a helpful addition. It is not necessary to check the content of the message, but check for a warning or error log as appropriate could be useful.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on a diff in pull request #6798: [NIFI-6498] Set error event listener on both the TransformerFactory a…
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on code in PR #6798:
URL: https://github.com/apache/nifi/pull/6798#discussion_r1054627413
##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestTransformXml.java:
##########
@@ -297,4 +301,61 @@ public void testTransformSecureProcessingEnabledXslWithEntity() throws IOExcepti
transformed.assertContentEquals("<?xml version=\"1.0\" encoding=\"UTF-8\"?>");
}
+
+ @Test
+ public void testNonMatchingTemplateTag() throws IOException {
+ final TestRunner runner = TestRunners.newTestRunner(new TransformXml());
+ runner.setProperty("header", "Test for mod");
+ runner.setProperty(TransformXml.XSLT_FILE_NAME, "src/test/resources/TestTransformXml/nonMatchingEndTag.xsl");
+
+ runner.enqueue(Paths.get("src/test/resources/TestTransformXml/math.xml"));
+ runner.run();
+
+ runner.assertAllFlowFilesTransferred(TransformXml.REL_FAILURE);
+ MockComponentLog logger = runner.getLogger();
+ assertFalse(logger.getErrorMessages().isEmpty());
+ assertTrue(logger.getErrorMessages().size() >= 1);
+ String firstMessage = logger.getErrorMessages().get(0).getMsg();
+ assertTrue(firstMessage.contains("xsl:template"));
+ assertTrue(firstMessage.contains("Line#"));
+ assertTrue(firstMessage.contains("Column#"));
+ }
+
+ @Test
+ public void testMessageTerminate() throws IOException {
+ final TestRunner runner = TestRunners.newTestRunner(new TransformXml());
+ runner.setProperty("header", "Test message terminate");
+ runner.setProperty(TransformXml.XSLT_FILE_NAME, "src/test/resources/TestTransformXml/employeeMessageTerminate.xsl");
+
+ runner.enqueue(Paths.get("src/test/resources/TestTransformXml/employee.xml"));
+ runner.run();
+
+ runner.assertAllFlowFilesTransferred(TransformXml.REL_FAILURE);
+ MockComponentLog logger = runner.getLogger();
+ assertFalse(logger.getErrorMessages().isEmpty());
+ assertTrue(logger.getErrorMessages().size() >= 1);
+ String firstMessage = logger.getErrorMessages().get(0).getMsg();
+ assertTrue(firstMessage.contains("xsl:message"));
+ assertTrue(firstMessage.contains("Line#"));
+ assertTrue(firstMessage.contains("Column#"));
Review Comment:
```suggestion
String firstMessage = logger.getErrorMessages().get(0).getMsg();
assertTrue(firstMessage.contains("xsl:message"));
```
##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestTransformXml.java:
##########
@@ -297,4 +301,61 @@ public void testTransformSecureProcessingEnabledXslWithEntity() throws IOExcepti
transformed.assertContentEquals("<?xml version=\"1.0\" encoding=\"UTF-8\"?>");
}
+
+ @Test
+ public void testNonMatchingTemplateTag() throws IOException {
+ final TestRunner runner = TestRunners.newTestRunner(new TransformXml());
+ runner.setProperty("header", "Test for mod");
+ runner.setProperty(TransformXml.XSLT_FILE_NAME, "src/test/resources/TestTransformXml/nonMatchingEndTag.xsl");
+
+ runner.enqueue(Paths.get("src/test/resources/TestTransformXml/math.xml"));
+ runner.run();
+
+ runner.assertAllFlowFilesTransferred(TransformXml.REL_FAILURE);
+ MockComponentLog logger = runner.getLogger();
+ assertFalse(logger.getErrorMessages().isEmpty());
+ assertTrue(logger.getErrorMessages().size() >= 1);
+ String firstMessage = logger.getErrorMessages().get(0).getMsg();
+ assertTrue(firstMessage.contains("xsl:template"));
+ assertTrue(firstMessage.contains("Line#"));
+ assertTrue(firstMessage.contains("Column#"));
Review Comment:
Recommend simplifying the assertions as follows:
```suggestion
String firstMessage = logger.getErrorMessages().get(0).getMsg();
assertTrue(firstMessage.contains("xsl:template"));
```
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] dan-s1 commented on pull request #6798: [NIFI-6498] Set error event listener on both the TransformerFactory a…
Posted by GitBox <gi...@apache.org>.
dan-s1 commented on PR #6798:
URL: https://github.com/apache/nifi/pull/6798#issuecomment-1361602295
@exceptionfactory Ok great! I will see if I can add that.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org