[email] Improve MultiPartEmail to ease extending

• Key: EMAIL-51
• Type: Improvement
• Status: Resolved
• Resolution: Fixed
• Priority: Minor
• Assignee: Unassigned
• Reporter: Troy Poppe

Environment

Operating System: other
Platform: Other

Description

MultiPartEmail currently constructs MimeBodyPart objects in some of the
methods. Extending MultiPartEmail may require that the type of BodyPart
constructed be specified by the subclass.

Replace constructor calls with calls to:
protected BodyPart getBodyPart()
protected MultiPart getMultiPart()

Comments

[Troy Poppe]

Created an attachment (id=14506)
proposed patch

[Troy Poppe]

(From update of attachment 14506)
Index:
c:/projects3.0/commons-email/src/java/org/apache/commons/mail/MultiPartEmail.ja
va
===================================================================
—
c:/projects3.0/commons-email/src/java/org/apache/commons/mail/MultiPartEmail.ja
va (revision 157757)
+++
c:/projects3.0/commons-email/src/java/org/apache/commons/mail/MultiPartEmail.ja
va (working copy)
@@ -19,13 +19,16 @@
import java.io.IOException;
import java.io.InputStream;
import java.net.URL;
+
import javax.activation.DataHandler;
import javax.activation.DataSource;
import javax.activation.FileDataSource;
import javax.activation.URLDataSource;
+import javax.mail.BodyPart;
import javax.mail.MessagingException;
import javax.mail.internet.MimeBodyPart;
import javax.mail.internet.MimeMultipart;
+
import org.apache.commons.lang.StringUtils;

/**
@@ -52,7 +55,7 @@
private MimeMultipart container = null;

/** The message container. */

• private MimeBodyPart primaryBodyPart = null;
  + private BodyPart primaryBodyPart = null;

/** The MIME subtype. */
private String subType = null;
@@ -92,7 +95,7 @@
public Email addPart(String content, String contentType)
throws EmailException
{

• MimeBodyPart bodyPart = new MimeBodyPart();
  + BodyPart bodyPart = createBodyPart();
  try
  {
  bodyPart.setContent(content, contentType);
  @@ -115,7 +118,7 @@
  */
  public Email addPart(MimeMultipart multipart) throws EmailException
  {
• MimeBodyPart bodyPart = new MimeBodyPart();
  + BodyPart bodyPart = createBodyPart();
  try { bodyPart.setContent(multipart); getContainer().addBodyPart(bodyPart); @@ -140,7 +143,7 @@ throw new IllegalStateException("Already initialized"); }

• container = new MimeMultipart();
  + container = createMimeMultipart();
  super.setContent(container);

initialized = true;
@@ -164,7 +167,9 @@
try {
if (StringUtils.isNotEmpty(charset))

{ - getPrimaryBodyPart().setText(msg, charset); +// BROKEN! +// getPrimaryBodyPart().setText(msg, charset); + getPrimaryBodyPart().setText(msg); }

else
{
@@ -192,7 +197,7 @@
// the content for the main body part was actually set. If
not,
// an IOException will be thrown during super.send().

• MimeBodyPart body = this.getPrimaryBodyPart();
  + BodyPart body = this.getPrimaryBodyPart();
  Object content = null;
  try
  {
  @@ -380,7 +385,7 @@ { name = ds.getName(); }
• MimeBodyPart mbp = new MimeBodyPart();
  + BodyPart mbp = createBodyPart();
  try {
  getContainer().addBodyPart(mbp);

@@ -392,7 +397,7 @@
catch (MessagingException me){ throw new EmailException(me); }

• this.boolHasAttachments = true;
  + setBoolHasAttachments(true);

return this;
}
@@ -404,7 +409,7 @@

• @throws EmailException see javax.mail.internet.MimeBodyPart
• for defintions
  */

• protected MimeBodyPart getPrimaryBodyPart() throws MessagingException
  + protected BodyPart getPrimaryBodyPart() throws MessagingException
  {
  if (!initialized)
  {
  @@ -414,7 +419,7 @@
  // Add the first body part to the message. The fist body part must be
  if (this.primaryBodyPart == null) { - primaryBodyPart = new MimeBodyPart(); + primaryBodyPart = createBodyPart(); getContainer().addBodyPart(primaryBodyPart); }

@@ -437,6 +442,23 @@
return container;
}

+ /**
+ *
+ * @return
+ */
+ protected boolean isInitialized()
+ { + return initialized; + }
+
+ /**
+ *
+ * @param b
+ */
+ protected void setIsInitialized(boolean b)
+ { + initialized = b; + }

/**

• @return boolHasAttachments
  @@ -453,5 +475,24 @@ { boolHasAttachments = b; }

  +
  + /**
  + *
  + * @return
  + */
  + protected BodyPart createBodyPart()
  + { + BodyPart bp = new MimeBodyPart(); + return bp; + }

+ /**
+ *
+ * @return
+ */
+ public MimeMultipart createMimeMultipart()
+ { + MimeMultipart mmp = new MimeMultipart(); + return mmp; + }
}

[Troy Poppe]

(From update of attachment 14506)
Index:
c:/projects3.0/commons-email/src/java/org/apache/commons/mail/MultiPartEmail.ja
va
===================================================================
—
c:/projects3.0/commons-email/src/java/org/apache/commons/mail/MultiPartEmail.ja
va (revision 157757)
+++
c:/projects3.0/commons-email/src/java/org/apache/commons/mail/MultiPartEmail.ja
va (working copy)
@@ -19,13 +19,16 @@
import java.io.IOException;
import java.io.InputStream;
import java.net.URL;
+
import javax.activation.DataHandler;
import javax.activation.DataSource;
import javax.activation.FileDataSource;
import javax.activation.URLDataSource;
+import javax.mail.BodyPart;
import javax.mail.MessagingException;
import javax.mail.internet.MimeBodyPart;
import javax.mail.internet.MimeMultipart;
+
import org.apache.commons.lang.StringUtils;

/**
@@ -52,7 +55,7 @@
private MimeMultipart container = null;

/** The message container. */

• private MimeBodyPart primaryBodyPart = null;
  + private BodyPart primaryBodyPart = null;

/** The MIME subtype. */
private String subType = null;
@@ -92,7 +95,7 @@
public Email addPart(String content, String contentType)
throws EmailException
{

• MimeBodyPart bodyPart = new MimeBodyPart();
  + BodyPart bodyPart = createBodyPart();
  try
  {
  bodyPart.setContent(content, contentType);
  @@ -115,7 +118,7 @@
  */
  public Email addPart(MimeMultipart multipart) throws EmailException
  {
• MimeBodyPart bodyPart = new MimeBodyPart();
  + BodyPart bodyPart = createBodyPart();
  try { bodyPart.setContent(multipart); getContainer().addBodyPart(bodyPart); @@ -140,7 +143,7 @@ throw new IllegalStateException("Already initialized"); }

• container = new MimeMultipart();
  + container = createMimeMultipart();
  super.setContent(container);

initialized = true;
@@ -164,7 +167,9 @@
try {
if (StringUtils.isNotEmpty(charset))

{ - getPrimaryBodyPart().setText(msg, charset); +// BROKEN! +// getPrimaryBodyPart().setText(msg, charset); + getPrimaryBodyPart().setText(msg); }

else
{
@@ -192,7 +197,7 @@
// the content for the main body part was actually set. If
not,
// an IOException will be thrown during super.send().

• MimeBodyPart body = this.getPrimaryBodyPart();
  + BodyPart body = this.getPrimaryBodyPart();
  Object content = null;
  try
  {
  @@ -380,7 +385,7 @@ { name = ds.getName(); }
• MimeBodyPart mbp = new MimeBodyPart();
  + BodyPart mbp = createBodyPart();
  try {
  getContainer().addBodyPart(mbp);

@@ -392,7 +397,7 @@
catch (MessagingException me){ throw new EmailException(me); }

• this.boolHasAttachments = true;
  + setBoolHasAttachments(true);

return this;
}
@@ -404,7 +409,7 @@

• @throws EmailException see javax.mail.internet.MimeBodyPart
• for defintions
  */

• protected MimeBodyPart getPrimaryBodyPart() throws MessagingException
  + protected BodyPart getPrimaryBodyPart() throws MessagingException
  {
  if (!initialized)
  {
  @@ -414,7 +419,7 @@
  // Add the first body part to the message. The fist body part must be
  if (this.primaryBodyPart == null) { - primaryBodyPart = new MimeBodyPart(); + primaryBodyPart = createBodyPart(); getContainer().addBodyPart(primaryBodyPart); }

@@ -437,6 +442,23 @@
return container;
}

+ /**
+ *
+ * @return
+ */
+ protected boolean isInitialized()
+ { + return initialized; + }
+
+ /**
+ *
+ * @param b
+ */
+ protected void setIsInitialized(boolean b)
+ { + initialized = b; + }

/**

• @return boolHasAttachments
  @@ -453,5 +475,24 @@ { boolHasAttachments = b; }

  +
  + /**
  + *
  + * @return
  + */
  + protected BodyPart createBodyPart()
  + { + BodyPart bp = new MimeBodyPart(); + return bp; + }

+ /**
+ *
+ * @return
+ */
+ public MimeMultipart createMimeMultipart()
+ { + MimeMultipart mmp = new MimeMultipart(); + return mmp; + }
}

[Troy Poppe]

Created an attachment (id=14513)
proposed patch

[Troy Poppe]

Created an attachment (id=14518)
proposed patch

[Troy Poppe]

Would it be possible to get this patch accepted into v1.0? It doesn't alter
the application, it merely provides subclasses with a means of overriding the
BodyPart type that is constructed.

[dion gillard]

Post 1.0 enhancements

[David Eric Pugh]

In reviewing this patch, I didn't understand this line:

• getPrimaryBodyPart().setText(msg, charset);
  +// BROKEN!
  +// getPrimaryBodyPart().setText(msg, charset);
  + getPrimaryBodyPart().setText(msg);

Why isn't it getPrimaryBodyPart().setText(msg,charset); as well? also, I could not get the patch to apply
cleanly to SVN trunk.

[Troy Poppe]

The reason for the charset change is because I altered the method signature for
getPrimaryBodyPart() to return the superclass:

• protected MimeBodyPart getPrimaryBodyPart() throws MessagingException
  + protected BodyPart getPrimaryBodyPart() throws MessagingException

As a result, the method setText(String, CharSet) doesn't exist in BodyPart, but
it does in MimeBodyPart.

If you're ok with this, and the rest of the changes that I made, I'll go ahead
and make a patch against the trunk, and re-submit.

T

[David Eric Pugh]

Okay, but dropping that signature I think is going to break a lot of usecase, correct? We need to make
sure we aren't making the changes that break all the installations in odd ways by losing charset support.