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/07/14 00:45:07 UTC

[GitHub] [nifi] emiliosetiadarma opened a new pull request, #6204: NIFI-9758 implemented additional Dynamic Properties for JavaMail session

emiliosetiadarma opened a new pull request, #6204:
URL: https://github.com/apache/nifi/pull/6204

   <!-- 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-9758](https://issues.apache.org/jira/browse/NIFI-9758)
   
   These dynamic properties allow the `mail.smtp.timeout` property for the `JavaMail` session to be set.
   
   # Tracking
   
   Please complete the following tracking steps prior to pull request creation.
   
   ### Issue Tracking
   
   - [x] [Apache NiFi Jira](https://issues.apache.org/jira/browse/NIFI) issue created
   
   ### Pull Request Tracking
   
   - [x] Pull Request title starts with Apache NiFi Jira issue number, such as `NIFI-00000`
   - [x] Pull Request commit message starts with Apache NiFi Jira issue number, as such `NIFI-00000`
   
   ### Pull Request Formatting
   
   - [x] Pull Request based on current revision of the `main` branch
   - [x] 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
   
   - [x] Build completed using `mvn clean install -P contrib-check`
     - [x] JDK 8
     - [x] JDK 11
     - [x] 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] emiliosetiadarma commented on a diff in pull request #6204: NIFI-9758 implemented additional Dynamic Properties for JavaMail session

Posted by GitBox <gi...@apache.org>.
emiliosetiadarma commented on code in PR #6204:
URL: https://github.com/apache/nifi/pull/6204#discussion_r921704746


##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/PutEmail.java:
##########
@@ -305,6 +315,18 @@ protected List<PropertyDescriptor> getSupportedPropertyDescriptors() {
         return properties;
     }
 
+    @Override
+    protected PropertyDescriptor getSupportedDynamicPropertyDescriptor(final String propertyDescriptorName) {
+        return new PropertyDescriptor.Builder()
+                .name(propertyDescriptorName)
+                .description("Mail properties to be set to the JavaMail Session")

Review Comment:
   Making the changes



-- 
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] emiliosetiadarma commented on a diff in pull request #6204: NIFI-9758 implemented additional Dynamic Properties for JavaMail session

Posted by GitBox <gi...@apache.org>.
emiliosetiadarma commented on code in PR #6204:
URL: https://github.com/apache/nifi/pull/6204#discussion_r921705901


##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/PutEmail.java:
##########
@@ -523,4 +555,57 @@ private InternetAddress[] toInetAddresses(final ProcessContext context, final Fl
     protected void send(final Message msg) throws MessagingException {
         Transport.send(msg);
     }
+
+    /**
+     * Utility function to return a Map of optional {@code mail._____} properties that were set dynamically.
+     * @param context the ProcessContext
+     * @return a Map with optional {@code mail._____} properties
+     */
+    private Map<String, String> getDynamicMailProperties(final ProcessContext context) {
+        final Map<String, String> dynamicMailProperties = new HashMap<>();
+
+        for (final Map.Entry<PropertyDescriptor, String> entry: context.getProperties().entrySet()) {
+            final PropertyDescriptor descriptor = entry.getKey();
+            if (descriptor.isDynamic()) {
+                final String name = descriptor.getName();
+                final Matcher matcher = MAIL_PROPERTY_PATTERN.matcher(name);
+                if (matcher.matches()) {
+                    dynamicMailProperties.put(name, entry.getValue());
+                }
+            }
+        }
+
+        return dynamicMailProperties;
+    }
+
+    private static class DynamicMailPropertyValidator implements Validator {
+        @Override
+        public ValidationResult validate(String subject, String input, ValidationContext context) {
+            final Matcher matcher = MAIL_PROPERTY_PATTERN.matcher(subject);
+            if (!matcher.matches()) {
+                return new ValidationResult.Builder()
+                        .input(input)
+                        .subject(subject)
+                        .valid(false)
+                        .explanation(String.format("[%s] is an invalid mail._____ property", subject))
+                        .build();
+            }
+
+            if (propertyToContext.containsKey(subject)) {
+                return new ValidationResult.Builder()
+                        .input(input)
+                        .subject(subject)
+                        .valid(false)
+                        .explanation(String.format("[%s] overwrites required existing properties", subject))

Review Comment:
   Making the changes



-- 
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] emiliosetiadarma commented on a diff in pull request #6204: NIFI-9758 implemented additional Dynamic Properties for JavaMail session

Posted by GitBox <gi...@apache.org>.
emiliosetiadarma commented on code in PR #6204:
URL: https://github.com/apache/nifi/pull/6204#discussion_r921706355


##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestPutEmail.java:
##########
@@ -299,4 +299,51 @@ public void testOutgoingMessageWithFlowfileContent() throws Exception {
         assertEquals("anotherbcc@apache.org", message.getRecipients(RecipientType.BCC)[1].toString());
     }
 
+    @Test
+    public void testValidDynamicMailProperties() throws Exception {
+        runner.setProperty(PutEmail.SMTP_HOSTNAME, "smtp-host");
+        runner.setProperty(PutEmail.HEADER_XMAILER, "TestingNiFi");
+        runner.setProperty(PutEmail.FROM, "test@apache.org,from@apache.org");
+        runner.setProperty(PutEmail.MESSAGE, "${body}");
+        runner.setProperty(PutEmail.TO, "recipient@apache.org,another@apache.org");
+        runner.setProperty(PutEmail.CC, "recipientcc@apache.org,anothercc@apache.org");
+        runner.setProperty(PutEmail.BCC, "recipientbcc@apache.org,anotherbcc@apache.org");
+        runner.setProperty(PutEmail.CONTENT_AS_MESSAGE, "${sendContent}");
+
+        runner.setProperty("mail.smtp.timeout", "sample dynamic mail property");
+        runner.setProperty("mail.smtp.writetimeout", "sample dynamic mail property");
+        runner.setProperty("mail.smtp.connectiontimeout", "sample dynamic mail property");
+        runner.setProperty("mail.smtp.auth.xoauth2.disable", "sample dynamic mail property");

Review Comment:
   Will make the changes



-- 
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] emiliosetiadarma commented on a diff in pull request #6204: NIFI-9758 implemented additional Dynamic Properties for JavaMail session

Posted by GitBox <gi...@apache.org>.
emiliosetiadarma commented on code in PR #6204:
URL: https://github.com/apache/nifi/pull/6204#discussion_r921705793


##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/PutEmail.java:
##########
@@ -523,4 +555,57 @@ private InternetAddress[] toInetAddresses(final ProcessContext context, final Fl
     protected void send(final Message msg) throws MessagingException {
         Transport.send(msg);
     }
+
+    /**
+     * Utility function to return a Map of optional {@code mail._____} properties that were set dynamically.
+     * @param context the ProcessContext
+     * @return a Map with optional {@code mail._____} properties
+     */
+    private Map<String, String> getDynamicMailProperties(final ProcessContext context) {
+        final Map<String, String> dynamicMailProperties = new HashMap<>();
+
+        for (final Map.Entry<PropertyDescriptor, String> entry: context.getProperties().entrySet()) {
+            final PropertyDescriptor descriptor = entry.getKey();
+            if (descriptor.isDynamic()) {
+                final String name = descriptor.getName();
+                final Matcher matcher = MAIL_PROPERTY_PATTERN.matcher(name);
+                if (matcher.matches()) {
+                    dynamicMailProperties.put(name, entry.getValue());
+                }
+            }
+        }
+
+        return dynamicMailProperties;
+    }
+
+    private static class DynamicMailPropertyValidator implements Validator {
+        @Override
+        public ValidationResult validate(String subject, String input, ValidationContext context) {
+            final Matcher matcher = MAIL_PROPERTY_PATTERN.matcher(subject);
+            if (!matcher.matches()) {
+                return new ValidationResult.Builder()
+                        .input(input)
+                        .subject(subject)
+                        .valid(false)
+                        .explanation(String.format("[%s] is an invalid mail._____ property", subject))

Review Comment:
   Making the changes



-- 
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] emiliosetiadarma commented on a diff in pull request #6204: NIFI-9758 implemented additional Dynamic Properties for JavaMail session

Posted by GitBox <gi...@apache.org>.
emiliosetiadarma commented on code in PR #6204:
URL: https://github.com/apache/nifi/pull/6204#discussion_r921704086


##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/PutEmail.java:
##########
@@ -79,10 +82,17 @@
 @Tags({"email", "put", "notify", "smtp"})
 @InputRequirement(Requirement.INPUT_REQUIRED)
 @CapabilityDescription("Sends an e-mail to configured recipients for each incoming FlowFile")
+@DynamicProperty(name = "mail._____",

Review Comment:
   Thanks for the input! Will make the changes!



-- 
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] emiliosetiadarma commented on a diff in pull request #6204: NIFI-9758 implemented additional Dynamic Properties for JavaMail session

Posted by GitBox <gi...@apache.org>.
emiliosetiadarma commented on code in PR #6204:
URL: https://github.com/apache/nifi/pull/6204#discussion_r921704827


##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/PutEmail.java:
##########
@@ -79,10 +82,17 @@
 @Tags({"email", "put", "notify", "smtp"})
 @InputRequirement(Requirement.INPUT_REQUIRED)
 @CapabilityDescription("Sends an e-mail to configured recipients for each incoming FlowFile")

Review Comment:
   That makes sense, adding it!



-- 
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 #6204: NIFI-9758 implemented additional Dynamic Properties for JavaMail session

Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on code in PR #6204:
URL: https://github.com/apache/nifi/pull/6204#discussion_r921462808


##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/PutEmail.java:
##########
@@ -305,6 +315,18 @@ protected List<PropertyDescriptor> getSupportedPropertyDescriptors() {
         return properties;
     }
 
+    @Override
+    protected PropertyDescriptor getSupportedDynamicPropertyDescriptor(final String propertyDescriptorName) {
+        return new PropertyDescriptor.Builder()
+                .name(propertyDescriptorName)
+                .description("Mail properties to be set to the JavaMail Session")

Review Comment:
   ```suggestion
                   .description("SMTP property passed to the Mail Session")
   ```



##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/PutEmail.java:
##########
@@ -79,10 +82,17 @@
 @Tags({"email", "put", "notify", "smtp"})
 @InputRequirement(Requirement.INPUT_REQUIRED)
 @CapabilityDescription("Sends an e-mail to configured recipients for each incoming FlowFile")
+@DynamicProperty(name = "mail._____",
+        value = "Value for a specific property to be set in the JavaMail Session object",
+        description = "The values specified in this dynamic property will be set in the JavaMail Session object. " +

Review Comment:
   ```suggestion
           description = "Dynamic property names that will be passed to the Mail Session. " +
   ```



##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/PutEmail.java:
##########
@@ -79,10 +82,17 @@
 @Tags({"email", "put", "notify", "smtp"})
 @InputRequirement(Requirement.INPUT_REQUIRED)
 @CapabilityDescription("Sends an e-mail to configured recipients for each incoming FlowFile")

Review Comment:
   The annotation `@SupportsSensitiveDynamicProperties` should be added to allow optional property encryption for properties such as Proxy passwords.



##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/PutEmail.java:
##########
@@ -464,6 +486,16 @@ private Properties getMailPropertiesFromFlowFile(final ProcessContext context, f
             }
         }
 
+        for (final PropertyDescriptor descriptor : context.getProperties().keySet()) {
+            if (descriptor.isDynamic()) {
+                final String flowFileValue = context.getProperty(descriptor).evaluateAttributeExpressions(flowFile).getValue();
+                // Nullable values are not allowed, so filter out
+                if (null != flowFileValue) {
+                    properties.setProperty(descriptor.getName(), flowFileValue);

Review Comment:
   Recommend renaming `flowFileValue` to `mailPropertyValue`:
   ```suggestion
                   final String mailPropertyValue = context.getProperty(descriptor).evaluateAttributeExpressions(flowFile).getValue();
                   // Nullable values are not allowed, so filter out
                   if (mailPropertyValue != null) {
                       properties.setProperty(descriptor.getName(), mailPropertyValue);
   ```



##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestPutEmail.java:
##########
@@ -299,4 +299,51 @@ public void testOutgoingMessageWithFlowfileContent() throws Exception {
         assertEquals("anotherbcc@apache.org", message.getRecipients(RecipientType.BCC)[1].toString());
     }
 
+    @Test
+    public void testValidDynamicMailProperties() throws Exception {
+        runner.setProperty(PutEmail.SMTP_HOSTNAME, "smtp-host");
+        runner.setProperty(PutEmail.HEADER_XMAILER, "TestingNiFi");
+        runner.setProperty(PutEmail.FROM, "test@apache.org,from@apache.org");
+        runner.setProperty(PutEmail.MESSAGE, "${body}");
+        runner.setProperty(PutEmail.TO, "recipient@apache.org,another@apache.org");
+        runner.setProperty(PutEmail.CC, "recipientcc@apache.org,anothercc@apache.org");
+        runner.setProperty(PutEmail.BCC, "recipientbcc@apache.org,anotherbcc@apache.org");
+        runner.setProperty(PutEmail.CONTENT_AS_MESSAGE, "${sendContent}");
+
+        runner.setProperty("mail.smtp.timeout", "sample dynamic mail property");
+        runner.setProperty("mail.smtp.writetimeout", "sample dynamic mail property");
+        runner.setProperty("mail.smtp.connectiontimeout", "sample dynamic mail property");
+        runner.setProperty("mail.smtp.auth.xoauth2.disable", "sample dynamic mail property");

Review Comment:
   Setting a single property seems sufficient for testing.



##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/PutEmail.java:
##########
@@ -79,10 +82,17 @@
 @Tags({"email", "put", "notify", "smtp"})
 @InputRequirement(Requirement.INPUT_REQUIRED)
 @CapabilityDescription("Sends an e-mail to configured recipients for each incoming FlowFile")
+@DynamicProperty(name = "mail._____",

Review Comment:
   Instead of using underscores as a placeholder, recommend using `propertyName`:
   ```suggestion
   @DynamicProperty(name = "mail.propertyName",
   ```



##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/PutEmail.java:
##########
@@ -79,10 +82,17 @@
 @Tags({"email", "put", "notify", "smtp"})
 @InputRequirement(Requirement.INPUT_REQUIRED)
 @CapabilityDescription("Sends an e-mail to configured recipients for each incoming FlowFile")
+@DynamicProperty(name = "mail._____",
+        value = "Value for a specific property to be set in the JavaMail Session object",
+        description = "The values specified in this dynamic property will be set in the JavaMail Session object. " +
+                "Possible properties can be found in: https://javaee.github.io/javamail/docs/api/com/sun/mail/smtp/package-summary.html.",
+        expressionLanguageScope = ExpressionLanguageScope.FLOWFILE_ATTRIBUTES)
 @SystemResourceConsideration(resource = SystemResource.MEMORY, description = "The entirety of the FlowFile's content (as a String object) "
         + "will be read into memory in case the property to use the flow file content as the email body is set to true.")
 public class PutEmail extends AbstractProcessor {
 
+    private static final Pattern MAIL_PROPERTY_PATTERN = Pattern.compile("^mail\\.([a-z0-9\\.]+)$");

Review Comment:
   This could be adjusted to require `mail.smtp` with an optional `s`:
   ```suggestion
       private static final Pattern MAIL_PROPERTY_PATTERN = Pattern.compile("^mail\\.smtps?\\.([a-z0-9\\.]+)$");
   ```



##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/PutEmail.java:
##########
@@ -523,4 +555,57 @@ private InternetAddress[] toInetAddresses(final ProcessContext context, final Fl
     protected void send(final Message msg) throws MessagingException {
         Transport.send(msg);
     }
+
+    /**
+     * Utility function to return a Map of optional {@code mail._____} properties that were set dynamically.
+     * @param context the ProcessContext
+     * @return a Map with optional {@code mail._____} properties
+     */
+    private Map<String, String> getDynamicMailProperties(final ProcessContext context) {
+        final Map<String, String> dynamicMailProperties = new HashMap<>();
+
+        for (final Map.Entry<PropertyDescriptor, String> entry: context.getProperties().entrySet()) {
+            final PropertyDescriptor descriptor = entry.getKey();
+            if (descriptor.isDynamic()) {
+                final String name = descriptor.getName();
+                final Matcher matcher = MAIL_PROPERTY_PATTERN.matcher(name);
+                if (matcher.matches()) {
+                    dynamicMailProperties.put(name, entry.getValue());
+                }
+            }
+        }
+
+        return dynamicMailProperties;
+    }
+
+    private static class DynamicMailPropertyValidator implements Validator {
+        @Override
+        public ValidationResult validate(String subject, String input, ValidationContext context) {
+            final Matcher matcher = MAIL_PROPERTY_PATTERN.matcher(subject);
+            if (!matcher.matches()) {
+                return new ValidationResult.Builder()
+                        .input(input)
+                        .subject(subject)
+                        .valid(false)
+                        .explanation(String.format("[%s] is an invalid mail._____ property", subject))

Review Comment:
   ```suggestion
                           .explanation(String.format("[%s] does not start with mail.smtp", subject))
   ```



##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/PutEmail.java:
##########
@@ -523,4 +555,57 @@ private InternetAddress[] toInetAddresses(final ProcessContext context, final Fl
     protected void send(final Message msg) throws MessagingException {
         Transport.send(msg);
     }
+
+    /**
+     * Utility function to return a Map of optional {@code mail._____} properties that were set dynamically.
+     * @param context the ProcessContext
+     * @return a Map with optional {@code mail._____} properties
+     */
+    private Map<String, String> getDynamicMailProperties(final ProcessContext context) {
+        final Map<String, String> dynamicMailProperties = new HashMap<>();
+
+        for (final Map.Entry<PropertyDescriptor, String> entry: context.getProperties().entrySet()) {
+            final PropertyDescriptor descriptor = entry.getKey();
+            if (descriptor.isDynamic()) {
+                final String name = descriptor.getName();
+                final Matcher matcher = MAIL_PROPERTY_PATTERN.matcher(name);
+                if (matcher.matches()) {
+                    dynamicMailProperties.put(name, entry.getValue());
+                }
+            }
+        }
+
+        return dynamicMailProperties;
+    }
+
+    private static class DynamicMailPropertyValidator implements Validator {
+        @Override
+        public ValidationResult validate(String subject, String input, ValidationContext context) {
+            final Matcher matcher = MAIL_PROPERTY_PATTERN.matcher(subject);
+            if (!matcher.matches()) {
+                return new ValidationResult.Builder()
+                        .input(input)
+                        .subject(subject)
+                        .valid(false)
+                        .explanation(String.format("[%s] is an invalid mail._____ property", subject))
+                        .build();
+            }
+
+            if (propertyToContext.containsKey(subject)) {
+                return new ValidationResult.Builder()
+                        .input(input)
+                        .subject(subject)
+                        .valid(false)
+                        .explanation(String.format("[%s] overwrites required existing properties", subject))
+                        .build();
+            }
+
+            return new ValidationResult.Builder()
+                    .subject(subject)
+                    .input(input)
+                    .valid(true)
+                    .explanation("Valid mail._____ property that does not overwrite existing required properties")

Review Comment:
   ```suggestion
                       .explanation("Valid mail.smtp property found")
   ```



##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/PutEmail.java:
##########
@@ -523,4 +555,57 @@ private InternetAddress[] toInetAddresses(final ProcessContext context, final Fl
     protected void send(final Message msg) throws MessagingException {
         Transport.send(msg);
     }
+
+    /**
+     * Utility function to return a Map of optional {@code mail._____} properties that were set dynamically.
+     * @param context the ProcessContext
+     * @return a Map with optional {@code mail._____} properties
+     */
+    private Map<String, String> getDynamicMailProperties(final ProcessContext context) {
+        final Map<String, String> dynamicMailProperties = new HashMap<>();
+
+        for (final Map.Entry<PropertyDescriptor, String> entry: context.getProperties().entrySet()) {
+            final PropertyDescriptor descriptor = entry.getKey();
+            if (descriptor.isDynamic()) {
+                final String name = descriptor.getName();
+                final Matcher matcher = MAIL_PROPERTY_PATTERN.matcher(name);
+                if (matcher.matches()) {
+                    dynamicMailProperties.put(name, entry.getValue());
+                }
+            }
+        }
+
+        return dynamicMailProperties;
+    }

Review Comment:
   This method does not appear to be used, can it be removed?



##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/PutEmail.java:
##########
@@ -523,4 +555,57 @@ private InternetAddress[] toInetAddresses(final ProcessContext context, final Fl
     protected void send(final Message msg) throws MessagingException {
         Transport.send(msg);
     }
+
+    /**
+     * Utility function to return a Map of optional {@code mail._____} properties that were set dynamically.
+     * @param context the ProcessContext
+     * @return a Map with optional {@code mail._____} properties
+     */
+    private Map<String, String> getDynamicMailProperties(final ProcessContext context) {
+        final Map<String, String> dynamicMailProperties = new HashMap<>();
+
+        for (final Map.Entry<PropertyDescriptor, String> entry: context.getProperties().entrySet()) {
+            final PropertyDescriptor descriptor = entry.getKey();
+            if (descriptor.isDynamic()) {
+                final String name = descriptor.getName();
+                final Matcher matcher = MAIL_PROPERTY_PATTERN.matcher(name);
+                if (matcher.matches()) {
+                    dynamicMailProperties.put(name, entry.getValue());
+                }
+            }
+        }
+
+        return dynamicMailProperties;
+    }
+
+    private static class DynamicMailPropertyValidator implements Validator {
+        @Override
+        public ValidationResult validate(String subject, String input, ValidationContext context) {
+            final Matcher matcher = MAIL_PROPERTY_PATTERN.matcher(subject);
+            if (!matcher.matches()) {
+                return new ValidationResult.Builder()
+                        .input(input)
+                        .subject(subject)
+                        .valid(false)
+                        .explanation(String.format("[%s] is an invalid mail._____ property", subject))
+                        .build();
+            }
+
+            if (propertyToContext.containsKey(subject)) {
+                return new ValidationResult.Builder()
+                        .input(input)
+                        .subject(subject)
+                        .valid(false)
+                        .explanation(String.format("[%s] overwrites required existing properties", subject))

Review Comment:
   ```suggestion
                           .explanation(String.format("[%s] overwrites standard properties", subject))
   ```



##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestPutEmail.java:
##########
@@ -299,4 +299,51 @@ public void testOutgoingMessageWithFlowfileContent() throws Exception {
         assertEquals("anotherbcc@apache.org", message.getRecipients(RecipientType.BCC)[1].toString());
     }
 
+    @Test
+    public void testValidDynamicMailProperties() throws Exception {
+        runner.setProperty(PutEmail.SMTP_HOSTNAME, "smtp-host");
+        runner.setProperty(PutEmail.HEADER_XMAILER, "TestingNiFi");
+        runner.setProperty(PutEmail.FROM, "test@apache.org,from@apache.org");
+        runner.setProperty(PutEmail.MESSAGE, "${body}");
+        runner.setProperty(PutEmail.TO, "recipient@apache.org,another@apache.org");
+        runner.setProperty(PutEmail.CC, "recipientcc@apache.org,anothercc@apache.org");
+        runner.setProperty(PutEmail.BCC, "recipientbcc@apache.org,anotherbcc@apache.org");
+        runner.setProperty(PutEmail.CONTENT_AS_MESSAGE, "${sendContent}");

Review Comment:
   Instead of duplicating all of these setProperty calls, recommend creating a shared method named `setRequiredProperties()` and calling in each test method.



-- 
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 #6204: NIFI-9758 implemented additional Dynamic Properties for JavaMail session

Posted by GitBox <gi...@apache.org>.
exceptionfactory closed pull request #6204: NIFI-9758 implemented additional Dynamic Properties for JavaMail session
URL: https://github.com/apache/nifi/pull/6204


-- 
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] emiliosetiadarma commented on a diff in pull request #6204: NIFI-9758 implemented additional Dynamic Properties for JavaMail session

Posted by GitBox <gi...@apache.org>.
emiliosetiadarma commented on code in PR #6204:
URL: https://github.com/apache/nifi/pull/6204#discussion_r921705204


##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/PutEmail.java:
##########
@@ -79,10 +82,17 @@
 @Tags({"email", "put", "notify", "smtp"})
 @InputRequirement(Requirement.INPUT_REQUIRED)
 @CapabilityDescription("Sends an e-mail to configured recipients for each incoming FlowFile")
+@DynamicProperty(name = "mail._____",
+        value = "Value for a specific property to be set in the JavaMail Session object",
+        description = "The values specified in this dynamic property will be set in the JavaMail Session object. " +
+                "Possible properties can be found in: https://javaee.github.io/javamail/docs/api/com/sun/mail/smtp/package-summary.html.",
+        expressionLanguageScope = ExpressionLanguageScope.FLOWFILE_ATTRIBUTES)
 @SystemResourceConsideration(resource = SystemResource.MEMORY, description = "The entirety of the FlowFile's content (as a String object) "
         + "will be read into memory in case the property to use the flow file content as the email body is set to true.")
 public class PutEmail extends AbstractProcessor {
 
+    private static final Pattern MAIL_PROPERTY_PATTERN = Pattern.compile("^mail\\.([a-z0-9\\.]+)$");

Review Comment:
   Making the changes



-- 
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] emiliosetiadarma commented on a diff in pull request #6204: NIFI-9758 implemented additional Dynamic Properties for JavaMail session

Posted by GitBox <gi...@apache.org>.
emiliosetiadarma commented on code in PR #6204:
URL: https://github.com/apache/nifi/pull/6204#discussion_r921706220


##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/PutEmail.java:
##########
@@ -523,4 +555,57 @@ private InternetAddress[] toInetAddresses(final ProcessContext context, final Fl
     protected void send(final Message msg) throws MessagingException {
         Transport.send(msg);
     }
+
+    /**
+     * Utility function to return a Map of optional {@code mail._____} properties that were set dynamically.
+     * @param context the ProcessContext
+     * @return a Map with optional {@code mail._____} properties
+     */
+    private Map<String, String> getDynamicMailProperties(final ProcessContext context) {
+        final Map<String, String> dynamicMailProperties = new HashMap<>();
+
+        for (final Map.Entry<PropertyDescriptor, String> entry: context.getProperties().entrySet()) {
+            final PropertyDescriptor descriptor = entry.getKey();
+            if (descriptor.isDynamic()) {
+                final String name = descriptor.getName();
+                final Matcher matcher = MAIL_PROPERTY_PATTERN.matcher(name);
+                if (matcher.matches()) {
+                    dynamicMailProperties.put(name, entry.getValue());
+                }
+            }
+        }
+
+        return dynamicMailProperties;
+    }

Review Comment:
   Oh yeah my bad forgot to remove this from my first implementation, removing it now!



-- 
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] emiliosetiadarma commented on a diff in pull request #6204: NIFI-9758 implemented additional Dynamic Properties for JavaMail session

Posted by GitBox <gi...@apache.org>.
emiliosetiadarma commented on code in PR #6204:
URL: https://github.com/apache/nifi/pull/6204#discussion_r921705549


##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/PutEmail.java:
##########
@@ -464,6 +486,16 @@ private Properties getMailPropertiesFromFlowFile(final ProcessContext context, f
             }
         }
 
+        for (final PropertyDescriptor descriptor : context.getProperties().keySet()) {
+            if (descriptor.isDynamic()) {
+                final String flowFileValue = context.getProperty(descriptor).evaluateAttributeExpressions(flowFile).getValue();
+                // Nullable values are not allowed, so filter out
+                if (null != flowFileValue) {
+                    properties.setProperty(descriptor.getName(), flowFileValue);

Review Comment:
   Making the changes



-- 
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] emiliosetiadarma commented on a diff in pull request #6204: NIFI-9758 implemented additional Dynamic Properties for JavaMail session

Posted by GitBox <gi...@apache.org>.
emiliosetiadarma commented on code in PR #6204:
URL: https://github.com/apache/nifi/pull/6204#discussion_r921705995


##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/PutEmail.java:
##########
@@ -523,4 +555,57 @@ private InternetAddress[] toInetAddresses(final ProcessContext context, final Fl
     protected void send(final Message msg) throws MessagingException {
         Transport.send(msg);
     }
+
+    /**
+     * Utility function to return a Map of optional {@code mail._____} properties that were set dynamically.
+     * @param context the ProcessContext
+     * @return a Map with optional {@code mail._____} properties
+     */
+    private Map<String, String> getDynamicMailProperties(final ProcessContext context) {
+        final Map<String, String> dynamicMailProperties = new HashMap<>();
+
+        for (final Map.Entry<PropertyDescriptor, String> entry: context.getProperties().entrySet()) {
+            final PropertyDescriptor descriptor = entry.getKey();
+            if (descriptor.isDynamic()) {
+                final String name = descriptor.getName();
+                final Matcher matcher = MAIL_PROPERTY_PATTERN.matcher(name);
+                if (matcher.matches()) {
+                    dynamicMailProperties.put(name, entry.getValue());
+                }
+            }
+        }
+
+        return dynamicMailProperties;
+    }
+
+    private static class DynamicMailPropertyValidator implements Validator {
+        @Override
+        public ValidationResult validate(String subject, String input, ValidationContext context) {
+            final Matcher matcher = MAIL_PROPERTY_PATTERN.matcher(subject);
+            if (!matcher.matches()) {
+                return new ValidationResult.Builder()
+                        .input(input)
+                        .subject(subject)
+                        .valid(false)
+                        .explanation(String.format("[%s] is an invalid mail._____ property", subject))
+                        .build();
+            }
+
+            if (propertyToContext.containsKey(subject)) {
+                return new ValidationResult.Builder()
+                        .input(input)
+                        .subject(subject)
+                        .valid(false)
+                        .explanation(String.format("[%s] overwrites required existing properties", subject))
+                        .build();
+            }
+
+            return new ValidationResult.Builder()
+                    .subject(subject)
+                    .input(input)
+                    .valid(true)
+                    .explanation("Valid mail._____ property that does not overwrite existing required properties")

Review Comment:
   Making the changes



-- 
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] emiliosetiadarma commented on a diff in pull request #6204: NIFI-9758 implemented additional Dynamic Properties for JavaMail session

Posted by GitBox <gi...@apache.org>.
emiliosetiadarma commented on code in PR #6204:
URL: https://github.com/apache/nifi/pull/6204#discussion_r921707219


##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestPutEmail.java:
##########
@@ -299,4 +299,51 @@ public void testOutgoingMessageWithFlowfileContent() throws Exception {
         assertEquals("anotherbcc@apache.org", message.getRecipients(RecipientType.BCC)[1].toString());
     }
 
+    @Test
+    public void testValidDynamicMailProperties() throws Exception {
+        runner.setProperty(PutEmail.SMTP_HOSTNAME, "smtp-host");
+        runner.setProperty(PutEmail.HEADER_XMAILER, "TestingNiFi");
+        runner.setProperty(PutEmail.FROM, "test@apache.org,from@apache.org");
+        runner.setProperty(PutEmail.MESSAGE, "${body}");
+        runner.setProperty(PutEmail.TO, "recipient@apache.org,another@apache.org");
+        runner.setProperty(PutEmail.CC, "recipientcc@apache.org,anothercc@apache.org");
+        runner.setProperty(PutEmail.BCC, "recipientbcc@apache.org,anotherbcc@apache.org");
+        runner.setProperty(PutEmail.CONTENT_AS_MESSAGE, "${sendContent}");

Review Comment:
   Will make the required changes



-- 
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] emiliosetiadarma commented on a diff in pull request #6204: NIFI-9758 implemented additional Dynamic Properties for JavaMail session

Posted by GitBox <gi...@apache.org>.
emiliosetiadarma commented on code in PR #6204:
URL: https://github.com/apache/nifi/pull/6204#discussion_r921704468


##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/PutEmail.java:
##########
@@ -79,10 +82,17 @@
 @Tags({"email", "put", "notify", "smtp"})
 @InputRequirement(Requirement.INPUT_REQUIRED)
 @CapabilityDescription("Sends an e-mail to configured recipients for each incoming FlowFile")
+@DynamicProperty(name = "mail._____",
+        value = "Value for a specific property to be set in the JavaMail Session object",
+        description = "The values specified in this dynamic property will be set in the JavaMail Session object. " +

Review Comment:
   Making the changes



-- 
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