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("[email protected]",
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, "[email protected],[email protected]");
+ runner.setProperty(PutEmail.MESSAGE, "${body}");
+ runner.setProperty(PutEmail.TO,
"[email protected],[email protected]");
+ runner.setProperty(PutEmail.CC,
"[email protected],[email protected]");
+ runner.setProperty(PutEmail.BCC,
"[email protected],[email protected]");
+ 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("[email protected]",
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, "[email protected],[email protected]");
+ runner.setProperty(PutEmail.MESSAGE, "${body}");
+ runner.setProperty(PutEmail.TO,
"[email protected],[email protected]");
+ runner.setProperty(PutEmail.CC,
"[email protected],[email protected]");
+ runner.setProperty(PutEmail.BCC,
"[email protected],[email protected]");
+ 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
