You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by trixpan <gi...@git.apache.org> on 2016/09/30 12:14:10 UTC
[GitHub] nifi pull request #1083: NIFI-2709 - Refactor ConsumeIMAP and ConsumePOP3 to...
GitHub user trixpan opened a pull request:
https://github.com/apache/nifi/pull/1083
NIFI-2709 - Refactor ConsumeIMAP and ConsumePOP3 to generate RFC2822
compliant flowfiles
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/trixpan/nifi NIFI-2709
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/nifi/pull/1083.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1083
----
commit 4fc0779960480b6d8e81372506201e2a9a9bae29
Author: Andre F de Miranda <tr...@users.noreply.github.com>
Date: 2016-09-30T09:00:59Z
NIFI-2709 - Refactor ConsumeIMAP and ConsumePOP3 to generate RFC2822
compliant flowfiles
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #1083: NIFI-2709 - Refactor ConsumeIMAP and ConsumePOP3 to genera...
Posted by trixpan <gi...@git.apache.org>.
Github user trixpan commented on the issue:
https://github.com/apache/nifi/pull/1083
@bbende this should solve the IMAP issue.
Notes:
1. This PR reads the content of the message into memory, in contrast with the original design that relied on InputStream to write the content into the FlowFile. This is due to the fact that JavaMail doesn't seem to have an inputstream method capable of providing an RFC2822 message. In case anyone has an idea on how to improve that, I am keen to incorporate into this PR.
2. The PR includes a GreenMail based junit and required dependency to ease debugging. Since I am not sure if we want to refactor the jUnits already in place (they seem to work well) with GreenMail based jUnits I aded very little coverage. Let me know your preference and I will either remove the new jUnit or to refactor @olegz original jUnit to use GreenMail based unit testing.
3. This will fail contrib-check. To be addressed once we decide what to do with the jUnit tests mentioned above.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request #1083: NIFI-2709 - Refactor ConsumeIMAP and ConsumePOP3 to...
Posted by olegz <gi...@git.apache.org>.
Github user olegz commented on a diff in the pull request:
https://github.com/apache/nifi/pull/1083#discussion_r81533566
--- Diff: nifi-nar-bundles/nifi-email-bundle/nifi-email-processors/src/main/java/org/apache/nifi/processors/email/AbstractEmailProcessor.java ---
@@ -348,14 +345,11 @@ private void transfer(Message emailMessage, ProcessContext context, ProcessSessi
long start = System.nanoTime();
FlowFile flowFile = processSession.create();
- flowFile = processSession.append(flowFile, new OutputStreamCallback() {
- @Override
- public void process(final OutputStream out) throws IOException {
- try {
- StreamUtils.copy(emailMessage.getInputStream(), out);
- } catch (MessagingException e) {
- throw new IOException(e);
- }
+ flowFile = processSession.append(flowFile, out -> {
--- End diff --
actually my bad, i've mistaken it with JMS. Indeed they were introduced for 1.0 only, so all is good.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request #1083: NIFI-2709 - Refactor ConsumeIMAP and ConsumePOP3 to...
Posted by olegz <gi...@git.apache.org>.
Github user olegz commented on a diff in the pull request:
https://github.com/apache/nifi/pull/1083#discussion_r81532172
--- Diff: nifi-nar-bundles/nifi-email-bundle/nifi-email-processors/src/main/java/org/apache/nifi/processors/email/AbstractEmailProcessor.java ---
@@ -348,14 +345,11 @@ private void transfer(Message emailMessage, ProcessContext context, ProcessSessi
long start = System.nanoTime();
FlowFile flowFile = processSession.create();
- flowFile = processSession.append(flowFile, new OutputStreamCallback() {
- @Override
- public void process(final OutputStream out) throws IOException {
- try {
- StreamUtils.copy(emailMessage.getInputStream(), out);
- } catch (MessagingException e) {
- throw new IOException(e);
- }
+ flowFile = processSession.append(flowFile, out -> {
--- End diff --
The email support was introduced prior to 1.0, so if this patch only meant for 1.0, then it's fine, but at the moment the JIRA is not versioned and I assume the fix should be applied on both branches 0.* and master
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request #1083: NIFI-2709 - Refactor ConsumeIMAP and ConsumePOP3 to...
Posted by olegz <gi...@git.apache.org>.
Github user olegz commented on a diff in the pull request:
https://github.com/apache/nifi/pull/1083#discussion_r81530213
--- Diff: nifi-nar-bundles/nifi-email-bundle/nifi-email-processors/src/main/java/org/apache/nifi/processors/email/AbstractEmailProcessor.java ---
@@ -348,14 +345,11 @@ private void transfer(Message emailMessage, ProcessContext context, ProcessSessi
long start = System.nanoTime();
FlowFile flowFile = processSession.create();
- flowFile = processSession.append(flowFile, new OutputStreamCallback() {
- @Override
- public void process(final OutputStream out) throws IOException {
- try {
- StreamUtils.copy(emailMessage.getInputStream(), out);
- } catch (MessagingException e) {
- throw new IOException(e);
- }
+ flowFile = processSession.append(flowFile, out -> {
--- End diff --
This makes it incompatible with Java 7
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request #1083: NIFI-2709 - Refactor ConsumeIMAP and ConsumePOP3 to...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/nifi/pull/1083
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #1083: NIFI-2709 - Refactor ConsumeIMAP and ConsumePOP3 to genera...
Posted by trixpan <gi...@git.apache.org>.
Github user trixpan commented on the issue:
https://github.com/apache/nifi/pull/1083
@olegz any feedback or :+1: ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #1083: NIFI-2709 - Refactor ConsumeIMAP and ConsumePOP3 to genera...
Posted by trixpan <gi...@git.apache.org>.
Github user trixpan commented on the issue:
https://github.com/apache/nifi/pull/1083
@olegz thanks.
I wasn't sure if the comment was referring to the Java 8 or the whole thing. Thanks for clarifying and thank you for reviewing.
Cheers
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #1083: NIFI-2709 - Refactor ConsumeIMAP and ConsumePOP3 to genera...
Posted by trixpan <gi...@git.apache.org>.
Github user trixpan commented on the issue:
https://github.com/apache/nifi/pull/1083
User reveals patch works as expected:
https://community.hortonworks.com/comments/59274/view.html
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #1083: NIFI-2709 - Refactor ConsumeIMAP and ConsumePOP3 to genera...
Posted by trixpan <gi...@git.apache.org>.
Github user trixpan commented on the issue:
https://github.com/apache/nifi/pull/1083
@bbende code should pass contrib-check and be ready for review.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #1083: NIFI-2709 - Refactor ConsumeIMAP and ConsumePOP3 to genera...
Posted by trixpan <gi...@git.apache.org>.
Github user trixpan commented on the issue:
https://github.com/apache/nifi/pull/1083
content no long loaded into memory. except for the issue around jUnits (2 & 3), it should be ready to review
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request #1083: NIFI-2709 - Refactor ConsumeIMAP and ConsumePOP3 to...
Posted by trixpan <gi...@git.apache.org>.
Github user trixpan commented on a diff in the pull request:
https://github.com/apache/nifi/pull/1083#discussion_r81531370
--- Diff: nifi-nar-bundles/nifi-email-bundle/nifi-email-processors/src/main/java/org/apache/nifi/processors/email/AbstractEmailProcessor.java ---
@@ -348,14 +345,11 @@ private void transfer(Message emailMessage, ProcessContext context, ProcessSessi
long start = System.nanoTime();
FlowFile flowFile = processSession.create();
- flowFile = processSession.append(flowFile, new OutputStreamCallback() {
- @Override
- public void process(final OutputStream out) throws IOException {
- try {
- StreamUtils.copy(emailMessage.getInputStream(), out);
- } catch (MessagingException e) {
- throw new IOException(e);
- }
+ flowFile = processSession.append(flowFile, out -> {
--- End diff --
having said that, if necessary, happy to try to avoid using the lambda
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request #1083: NIFI-2709 - Refactor ConsumeIMAP and ConsumePOP3 to...
Posted by trixpan <gi...@git.apache.org>.
Github user trixpan commented on a diff in the pull request:
https://github.com/apache/nifi/pull/1083#discussion_r81530739
--- Diff: nifi-nar-bundles/nifi-email-bundle/nifi-email-processors/src/main/java/org/apache/nifi/processors/email/AbstractEmailProcessor.java ---
@@ -348,14 +345,11 @@ private void transfer(Message emailMessage, ProcessContext context, ProcessSessi
long start = System.nanoTime();
FlowFile flowFile = processSession.create();
- flowFile = processSession.append(flowFile, new OutputStreamCallback() {
- @Override
- public void process(final OutputStream out) throws IOException {
- try {
- StreamUtils.copy(emailMessage.getInputStream(), out);
- } catch (MessagingException e) {
- throw new IOException(e);
- }
+ flowFile = processSession.append(flowFile, out -> {
--- End diff --
NiFi 1.0 requires Java 8 or above?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request #1083: NIFI-2709 - Refactor ConsumeIMAP and ConsumePOP3 to...
Posted by olegz <gi...@git.apache.org>.
Github user olegz commented on a diff in the pull request:
https://github.com/apache/nifi/pull/1083#discussion_r81530413
--- Diff: nifi-nar-bundles/nifi-email-bundle/nifi-email-processors/src/test/java/org/apache/nifi/processors/email/TestConsumeEmail.java ---
@@ -0,0 +1,180 @@
+/*
+ * 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.
+ */
+
+package org.apache.nifi.processors.email;
+
+import com.icegreen.greenmail.user.GreenMailUser;
+import com.icegreen.greenmail.util.GreenMail;
+import com.icegreen.greenmail.util.ServerSetupTest;
+import org.apache.nifi.util.MockFlowFile;
+import org.apache.nifi.util.TestRunner;
+import org.apache.nifi.util.TestRunners;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.Assert;
+import org.springframework.integration.mail.AbstractMailReceiver;
+
+import javax.mail.Message;
+import javax.mail.MessagingException;
+import javax.mail.Session;
+import javax.mail.internet.InternetAddress;
+import javax.mail.internet.MimeMessage;
+import java.lang.reflect.Field;
+import java.util.List;
+import java.util.Properties;
+
+import static org.junit.Assert.assertEquals;
+
+
+public class TestConsumeEmail {
--- End diff --
Why was the name of the class changed?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request #1083: NIFI-2709 - Refactor ConsumeIMAP and ConsumePOP3 to...
Posted by trixpan <gi...@git.apache.org>.
Github user trixpan commented on a diff in the pull request:
https://github.com/apache/nifi/pull/1083#discussion_r81533345
--- Diff: nifi-nar-bundles/nifi-email-bundle/nifi-email-processors/src/main/java/org/apache/nifi/processors/email/AbstractEmailProcessor.java ---
@@ -348,14 +345,11 @@ private void transfer(Message emailMessage, ProcessContext context, ProcessSessi
long start = System.nanoTime();
FlowFile flowFile = processSession.create();
- flowFile = processSession.append(flowFile, new OutputStreamCallback() {
- @Override
- public void process(final OutputStream out) throws IOException {
- try {
- StreamUtils.copy(emailMessage.getInputStream(), out);
- } catch (MessagingException e) {
- throw new IOException(e);
- }
+ flowFile = processSession.append(flowFile, out -> {
--- End diff --
@olegz happy to follow you lead but are you sure it isn't 1.0 only?
If I recall correctly the email processors were being ironed out few days prior to 1.0 getting released.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi pull request #1083: NIFI-2709 - Refactor ConsumeIMAP and ConsumePOP3 to...
Posted by trixpan <gi...@git.apache.org>.
Github user trixpan commented on a diff in the pull request:
https://github.com/apache/nifi/pull/1083#discussion_r81531262
--- Diff: nifi-nar-bundles/nifi-email-bundle/nifi-email-processors/src/test/java/org/apache/nifi/processors/email/TestConsumeEmail.java ---
@@ -0,0 +1,180 @@
+/*
+ * 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.
+ */
+
+package org.apache.nifi.processors.email;
+
+import com.icegreen.greenmail.user.GreenMailUser;
+import com.icegreen.greenmail.util.GreenMail;
+import com.icegreen.greenmail.util.ServerSetupTest;
+import org.apache.nifi.util.MockFlowFile;
+import org.apache.nifi.util.TestRunner;
+import org.apache.nifi.util.TestRunners;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.Assert;
+import org.springframework.integration.mail.AbstractMailReceiver;
+
+import javax.mail.Message;
+import javax.mail.MessagingException;
+import javax.mail.Session;
+import javax.mail.internet.InternetAddress;
+import javax.mail.internet.MimeMessage;
+import java.lang.reflect.Field;
+import java.util.List;
+import java.util.Properties;
+
+import static org.junit.Assert.assertEquals;
+
+
+public class TestConsumeEmail {
--- End diff --
The initial commit introduced a temporary test unit based on GreenMail, to avoid overlap, I gave it a different name. After user confirmed patch worked, I migrated some of the test you had created into the new jUnit.
This allowed me to run debug the two test units while I was developing the final iteration of the test unit.
I guess Test*.java was used was this naming convention used in other processors of the package, happy to rename back to the original if necessary.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #1083: NIFI-2709 - Refactor ConsumeIMAP and ConsumePOP3 to genera...
Posted by olegz <gi...@git.apache.org>.
Github user olegz commented on the issue:
https://github.com/apache/nifi/pull/1083
@trixpan I thought I replied (3 hrs ago). Anyway it may not have been clear. I am ok, all is good.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---