Commons Email
  1. Commons Email
  2. EMAIL-51

[email] Improve MultiPartEmail to ease extending

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Labels:
      None
    • 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()

        Activity

        Troy Poppe created issue -
        Hide
        Troy Poppe added a comment -

        Created an attachment (id=14506)
        proposed patch

        Show
        Troy Poppe added a comment - Created an attachment (id=14506) proposed patch
        Hide
        Troy Poppe added a comment -

        (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; + }

        }

        Show
        Troy Poppe added a comment - (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; + } }
        Hide
        Troy Poppe added a comment -

        (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; + }

        }

        Show
        Troy Poppe added a comment - (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; + } }
        Hide
        Troy Poppe added a comment -

        Created an attachment (id=14513)
        proposed patch

        Show
        Troy Poppe added a comment - Created an attachment (id=14513) proposed patch
        Hide
        Troy Poppe added a comment -

        Created an attachment (id=14518)
        proposed patch

        Show
        Troy Poppe added a comment - Created an attachment (id=14518) proposed patch
        Hide
        Troy Poppe added a comment -

        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.

        Show
        Troy Poppe added a comment - 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.
        Hide
        dion gillard added a comment -

        Post 1.0 enhancements

        Show
        dion gillard added a comment - Post 1.0 enhancements
        Hide
        David Eric Pugh added a comment -

        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.

        Show
        David Eric Pugh added a comment - 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.
        Hide
        Troy Poppe added a comment -

        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

        Show
        Troy Poppe added a comment - 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
        Hide
        David Eric Pugh added a comment -

        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.

        Show
        David Eric Pugh added a comment - 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.
        Henri Yandell made changes -
        Field Original Value New Value
        issue.field.bugzillaimportkey 34056 12342120
        Henri Yandell made changes -
        Key COM-1968 EMAIL-51
        Affects Version/s unspecified [ 12311647 ]
        Project Commons [ 12310458 ] Commons Email [ 12310474 ]
        Assignee Jakarta Commons Developers Mailing List [ commons-dev@jakarta.apache.org ]
        Component/s Email [ 12311114 ]
        Siegfried Goeschl made changes -
        Status Resolved [ 5 ] Closed [ 6 ]

          People

          • Assignee:
            Unassigned
            Reporter:
            Troy Poppe
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development