You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by mb...@apache.org on 2017/08/18 19:10:41 UTC

svn commit: r1805459 - in /ofbiz/ofbiz-framework/trunk: applications/order/servicedef/ applications/order/src/main/java/org/apache/ofbiz/order/order/ framework/base/src/main/java/org/apache/ofbiz/base/util/ framework/common/src/main/java/org/apache/ofb...

Author: mbrohl
Date: Fri Aug 18 19:10:41 2017
New Revision: 1805459

URL: http://svn.apache.org/viewvc?rev=1805459&view=rev
Log:
Improved: EmailServices.sendMailFromScreen improved to take multiple 
attachments with appropriate type along with several email fixes.
(OFBIZ-9395)

This patch fixes a number of issues combined in a patch because of the 
code dependencies.

1. it enables to add BCC adress(es) to service 
OrderServices.sendOrderNotificationScreens to oversteer 
ProductStoreEmailSetting of BCC the same was as for CC.

2. a method UtilValidate.isEmailList(String) is added to check a 
comma separated list of email addresses, used for example to check the 
String passed to the new BCC field for an Order-Notification.

3. there are improvements in EmailServices.sendMailFromScreen. The 
attachment type of MailAttachments is now not only .pdf but depends on 
the specific file. This has not been the case before - the mime type was
always hard coded as .pdf. The same goes for the bodyPart content-type 
which is now set to the passed content type or the default text/html 
type. Before this was also always set to text/html. 
Additionally, an attachment that has the mime-type text/plain is not 
rendered with the fop-renderer anymore but with a simple text-renderer. 
Therefore it is possible to send an CSV file as attachment now.

The patch also refactors some catch-Blocks in the 
EmailServices.sendMailFromScreen by using multi-catch since the e
xception handling is always the same.

Thanks Martin Becker for reporting and providing the patch.

Modified:
    ofbiz/ofbiz-framework/trunk/applications/order/servicedef/services.xml
    ofbiz/ofbiz-framework/trunk/applications/order/src/main/java/org/apache/ofbiz/order/order/OrderServices.java
    ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilValidate.java
    ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbiz/common/email/EmailServices.java

Modified: ofbiz/ofbiz-framework/trunk/applications/order/servicedef/services.xml
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/order/servicedef/services.xml?rev=1805459&r1=1805458&r2=1805459&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/applications/order/servicedef/services.xml (original)
+++ ofbiz/ofbiz-framework/trunk/applications/order/servicedef/services.xml Fri Aug 18 19:10:41 2017
@@ -34,6 +34,7 @@ under the License.
         <attribute name="body" type="String" mode="OUT" optional="true"/>
         <attribute name="sendTo" type="String" mode="IN" optional="true"/>
         <attribute name="sendCc" type="String" mode="IN" optional="true"/>
+        <attribute name="sendBcc" type="String" mode="IN" optional="true"/>
         <attribute name="note" type="String" mode="IN" optional="true"/>
         <attribute name="temporaryAnonymousUserLogin" type="org.apache.ofbiz.entity.GenericValue" mode="IN" optional="true"/>
         <attribute name="messageWrapper" type="org.apache.ofbiz.service.mail.MimeMessageWrapper" mode="OUT" optional="true"/>

Modified: ofbiz/ofbiz-framework/trunk/applications/order/src/main/java/org/apache/ofbiz/order/order/OrderServices.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/order/src/main/java/org/apache/ofbiz/order/order/OrderServices.java?rev=1805459&r1=1805458&r2=1805459&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/applications/order/src/main/java/org/apache/ofbiz/order/order/OrderServices.java (original)
+++ ofbiz/ofbiz-framework/trunk/applications/order/src/main/java/org/apache/ofbiz/order/order/OrderServices.java Fri Aug 18 19:10:41 2017
@@ -83,6 +83,9 @@ import org.apache.ofbiz.service.ServiceU
 
 import com.ibm.icu.util.Calendar;
 
+import org.apache.commons.lang.StringUtils;
+import org.apache.fop.apps.MimeConstants;
+
 /**
  * Order Processing Services
  */
@@ -2533,8 +2536,10 @@ public class OrderServices {
         GenericValue userLogin = (GenericValue) context.get("userLogin");
         String orderId = (String) context.get("orderId");
         String orderItemSeqId = (String) context.get("orderItemSeqId");
+        String shipGroupSeqId = (String) context.get("shipGroupSeqId");
         String sendTo = (String) context.get("sendTo");
         String sendCc = (String) context.get("sendCc");
+        String sendBcc = (String) context.get("sendBcc");
         String note = (String) context.get("note");
         String screenUri = (String) context.get("screenUri");
         GenericValue temporaryAnonymousUserLogin = (GenericValue) context.get("temporaryAnonymousUserLogin");
@@ -2589,10 +2594,17 @@ public class OrderServices {
             String xslfoAttachScreenLocation = productStoreEmail.getString("xslfoAttachScreenLocation");
             sendMap.put("xslfoAttachScreenLocation", xslfoAttachScreenLocation);
             // add attachmentName param to get an attachment namend "[oderId].pdf" instead of default "Details.pdf"
-            sendMap.put("attachmentName", orderId + ".pdf");
+            sendMap.put("attachmentName", (UtilValidate.isNotEmpty(shipGroupSeqId) ? orderId + "-" + StringUtils.stripStart(shipGroupSeqId, "0") : orderId) + ".pdf");
+            sendMap.put("attachmentType", MimeConstants.MIME_PDF);
         } else {
             sendMap.put("bodyScreenUri", screenUri);
         }
+        
+        if (context.containsKey("xslfoAttachScreenLocationList")) {
+            sendMap.put("xslfoAttachScreenLocationList", context.get("xslfoAttachScreenLocationList"));
+            sendMap.put("attachmentNameList", context.get("attachmentNameList"));
+            sendMap.put("attachmentTypeList", context.get("attachmentTypeList"));
+        }
 
         // website
         sendMap.put("webSiteId", orderHeader.get("webSiteId"));
@@ -2636,6 +2648,7 @@ public class OrderServices {
             bodyParameters.put("partyId", placingParty.get("partyId"));
         }
         bodyParameters.put("note", note);
+        bodyParameters.put("shipGroupSeqId", shipGroupSeqId);
         sendMap.put("bodyParameters", bodyParameters);
         sendMap.put("userLogin",userLogin);
 
@@ -2657,6 +2670,10 @@ public class OrderServices {
             sendMap.put("sendCc", productStoreEmail.get("ccAddress"));
         }
 
+        if ((sendBcc != null) && UtilValidate.isEmailList(sendBcc)) {
+            sendMap.put("sendBcc", sendBcc);
+        }
+
         // send the notification
         Map<String, Object> sendResp = null;
         try {

Modified: ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilValidate.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilValidate.java?rev=1805459&r1=1805458&r2=1805459&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilValidate.java (original)
+++ ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilValidate.java Fri Aug 18 19:10:41 2017
@@ -691,6 +691,20 @@ public final class UtilValidate {
         if (isEmpty(s)) return defaultEmptyOK;
         return EmailValidator.getInstance().isValid(s);
     }
+    
+    /** 
+     * Checks a String for a valid Email-List seperated by ",".
+     */
+    public static boolean isEmailList(String s) {
+        if (isEmpty(s)) return defaultEmptyOK;
+        String[] emails = s.split(",");
+        for (String email : emails) {
+            if (!EmailValidator.getInstance().isValid(email)) {
+                return false;
+            }
+        }
+        return true;
+    }
 
     /** isUrl returns true if the string contains ://
      * @param s String to validate

Modified: ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbiz/common/email/EmailServices.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbiz/common/email/EmailServices.java?rev=1805459&r1=1805458&r2=1805459&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbiz/common/email/EmailServices.java (original)
+++ ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbiz/common/email/EmailServices.java Fri Aug 18 19:10:41 2017
@@ -51,6 +51,7 @@ import javax.mail.internet.MimeMultipart
 import javax.xml.parsers.ParserConfigurationException;
 import javax.xml.transform.stream.StreamSource;
 
+import org.apache.fop.apps.FOPException;
 import org.apache.fop.apps.Fop;
 import org.apache.fop.apps.MimeConstants;
 import org.apache.ofbiz.base.util.Debug;
@@ -438,6 +439,12 @@ public class EmailServices {
         if (UtilValidate.isNotEmpty(xslfoAttachScreenLocationListParam)) xslfoAttachScreenLocationList.addAll(xslfoAttachScreenLocationListParam);
         if (UtilValidate.isNotEmpty(attachmentNameListParam)) attachmentNameList.addAll(attachmentNameListParam);
         
+        List<String> attachmentTypeList = new LinkedList<String>();
+        String attachmentTypeParam = (String) serviceContext.remove("attachmentType");
+        List<String> attachmentTypeListParam = UtilGenerics.checkList(serviceContext.remove("attachmentTypeList"));
+        if (UtilValidate.isNotEmpty(attachmentTypeParam)) attachmentTypeList.add(attachmentTypeParam);
+        if (UtilValidate.isNotEmpty(attachmentTypeListParam)) attachmentTypeList.addAll(attachmentTypeListParam);
+        
         Locale locale = (Locale) serviceContext.get("locale");
         Map<String, Object> bodyParameters = UtilGenerics.checkMap(serviceContext.remove("bodyParameters"));
         if (bodyParameters == null) {
@@ -504,9 +511,9 @@ public class EmailServices {
             List<Map<String, ? extends Object>> bodyParts = new LinkedList<Map<String, ? extends Object>>();
             if (bodyText != null) {
                 bodyText = FlexibleStringExpander.expandString(bodyText, screenContext,  locale);
-                bodyParts.add(UtilMisc.<String, Object>toMap("content", bodyText, "type", "text/html"));
+                bodyParts.add(UtilMisc.<String, Object>toMap("content", bodyText, "type", UtilValidate.isNotEmpty(contentType) ? contentType : "text/html"));
             } else {
-                bodyParts.add(UtilMisc.<String, Object>toMap("content", bodyWriter.toString(), "type", "text/html"));
+                bodyParts.add(UtilMisc.<String, Object>toMap("content", bodyWriter.toString(), "type", UtilValidate.isNotEmpty(contentType) ? contentType : "text/html"));
             }
             
             for (int i = 0; i < xslfoAttachScreenLocationList.size(); i++) {
@@ -515,39 +522,51 @@ public class EmailServices {
                 if (UtilValidate.isNotEmpty(attachmentNameList) && attachmentNameList.size() >= i) {
                     attachmentName = attachmentNameList.get(i);
                 }
+
+                String attachmentType = MimeConstants.MIME_PDF;
+                if (UtilValidate.isNotEmpty(attachmentTypeList) && attachmentTypeList.size() >= i) {
+                    attachmentType = attachmentTypeList.get(i);
+                }
+                
                 isMultiPart = true;
                 // start processing fo pdf attachment
                 try {
                     Writer writer = new StringWriter();
-                    MapStack<String> screenContextAtt = MapStack.create();
                     // substitute the freemarker variables...
-                    ScreenStringRenderer foScreenStringRenderer = new MacroScreenRenderer(EntityUtilProperties.getPropertyValue("widget", "screenfop.name", dctx.getDelegator()),
-                            EntityUtilProperties.getPropertyValue("widget", "screenfop.screenrenderer", dctx.getDelegator()));
+                    ScreenStringRenderer foScreenStringRenderer = null;
+                    if(MimeConstants.MIME_PLAIN_TEXT.equals(attachmentType)){
+                        foScreenStringRenderer = new MacroScreenRenderer(EntityUtilProperties.getPropertyValue("widget", "screentext.name", dctx.getDelegator()),
+                                EntityUtilProperties.getPropertyValue("widget", "screentext.screenrenderer", dctx.getDelegator()));   
+                    }else{
+                        foScreenStringRenderer = new MacroScreenRenderer(EntityUtilProperties.getPropertyValue("widget", "screenfop.name", dctx.getDelegator()),
+                                EntityUtilProperties.getPropertyValue("widget", "screenfop.screenrenderer", dctx.getDelegator()));
+                    } 
                     ScreenRenderer screensAtt = new ScreenRenderer(writer, screenContext, foScreenStringRenderer);
                     screensAtt.populateContextForService(dctx, bodyParameters);
-                    screenContextAtt.putAll(bodyParameters);
                     screensAtt.render(xslfoAttachScreenLocation);
 
-                    // create the input stream for the generation
-                    StreamSource src = new StreamSource(new StringReader(writer.toString()));
-
                     // create the output stream for the generation
                     ByteArrayOutputStream baos = new ByteArrayOutputStream();
 
-                    Fop fop = ApacheFopWorker.createFopInstance(baos, MimeConstants.MIME_PDF);
-                    ApacheFopWorker.transform(src, null, fop);
+                    if (MimeConstants.MIME_PLAIN_TEXT.equals(attachmentType)) {
+                        baos.write(writer.toString().getBytes());
+                    } else {
+                        // create the input stream for the generation
+                        StreamSource src = new StreamSource(new StringReader(writer.toString()));
+                        Fop fop = ApacheFopWorker.createFopInstance(baos, attachmentType);
+                        ApacheFopWorker.transform(src, null, fop);
+                    }
 
-                    // and generate the PDF
+                    // and generate the attachment
                     baos.flush();
                     baos.close();
 
                     // store in the list of maps for sendmail....
-                    bodyParts.add(UtilMisc.<String, Object> toMap("content", baos.toByteArray(), "type", "application/pdf", "filename",
-                            attachmentName));
-                } catch (Exception e) {
-                    Debug.logError(e, "Error rendering PDF attachment for email: " + e.toString(), module);
-                    return ServiceUtil.returnError(UtilProperties.getMessage(resource, "CommonEmailSendRenderingScreenPdfError",
-                            UtilMisc.toMap("errorString", e.toString()), locale));
+                    bodyParts.add(UtilMisc.<String, Object>toMap("content", baos.toByteArray(), "type", attachmentType, "filename", attachmentName));
+                    
+                } catch (GeneralException|IOException|SAXException|ParserConfigurationException |TemplateException ge) {
+                    Debug.logError(ge, "Error rendering PDF attachment for email: " + ge.toString(), module);
+                    return ServiceUtil.returnError(UtilProperties.getMessage(resource, "CommonEmailSendRenderingScreenPdfError", UtilMisc.toMap("errorString", ge.toString()), locale));
                 }
                 
                 serviceContext.put("bodyParts", bodyParts);



Re: svn commit: r1805459 - in /ofbiz/ofbiz-framework/trunk: applications/order/servicedef/ applications/order/src/main/java/org/apache/ofbiz/order/order/ framework/base/src/main/java/org/apache/ofbiz/base/util/ framework/common/src/main/java/org/apache/ofb...

Posted by Jacques Le Roux <ja...@les7arts.com>.
Thanks Michael,

I hope you the best for your new location

Cheers

Jacques


Le 01/10/2017 à 12:45, Michael Brohl a écrit :
> Hi Jacques,
>
> sorry for the delay, we were in the middle of our movement to a new ecomify headquarter.
>
> This is now fixed in r1810260, thanks for spotting!
>
> Regards,
>
> Michael
>
>
> Am 27.09.17 um 14:12 schrieb Jacques Le Roux:
>> Hi Michael,
>>
>> You introduced attachmentType in this commit but the service sendMailFromScreen does not declare this parameter, nor any other service, same for 
>> attachmentTypeList.
>>
>> Please check
>>
>> Thanks
>>
>> Jacques
>>
>>
>> Le 18/08/2017 à 21:10, mbrohl@apache.org a écrit :
>>> Author: mbrohl
>>> Date: Fri Aug 18 19:10:41 2017
>>> New Revision: 1805459
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1805459&view=rev
>>> Log:
>>> Improved: EmailServices.sendMailFromScreen improved to take multiple
>>> attachments with appropriate type along with several email fixes.
>>> (OFBIZ-9395)
>>>
>>> This patch fixes a number of issues combined in a patch because of the
>>> code dependencies.
>>>
>>> 1. it enables to add BCC adress(es) to service
>>> OrderServices.sendOrderNotificationScreens to oversteer
>>> ProductStoreEmailSetting of BCC the same was as for CC.
>>>
>>> 2. a method UtilValidate.isEmailList(String) is added to check a
>>> comma separated list of email addresses, used for example to check the
>>> String passed to the new BCC field for an Order-Notification.
>>>
>>> 3. there are improvements in EmailServices.sendMailFromScreen. The
>>> attachment type of MailAttachments is now not only .pdf but depends on
>>> the specific file. This has not been the case before - the mime type was
>>> always hard coded as .pdf. The same goes for the bodyPart content-type
>>> which is now set to the passed content type or the default text/html
>>> type. Before this was also always set to text/html.
>>> Additionally, an attachment that has the mime-type text/plain is not
>>> rendered with the fop-renderer anymore but with a simple text-renderer.
>>> Therefore it is possible to send an CSV file as attachment now.
>>>
>>> The patch also refactors some catch-Blocks in the
>>> EmailServices.sendMailFromScreen by using multi-catch since the e
>>> xception handling is always the same.
>>>
>>> Thanks Martin Becker for reporting and providing the patch.
>>>
>>> Modified:
>>> ofbiz/ofbiz-framework/trunk/applications/order/servicedef/services.xml
>>> ofbiz/ofbiz-framework/trunk/applications/order/src/main/java/org/apache/ofbiz/order/order/OrderServices.java
>>> ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilValidate.java
>>> ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbiz/common/email/EmailServices.java
>>>
>>> Modified: ofbiz/ofbiz-framework/trunk/applications/order/servicedef/services.xml
>>> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/order/servicedef/services.xml?rev=1805459&r1=1805458&r2=1805459&view=diff
>>> ==============================================================================
>>> --- ofbiz/ofbiz-framework/trunk/applications/order/servicedef/services.xml (original)
>>> +++ ofbiz/ofbiz-framework/trunk/applications/order/servicedef/services.xml Fri Aug 18 19:10:41 2017
>>> @@ -34,6 +34,7 @@ under the License.
>>>           <attribute name="body" type="String" mode="OUT" optional="true"/>
>>>           <attribute name="sendTo" type="String" mode="IN" optional="true"/>
>>>           <attribute name="sendCc" type="String" mode="IN" optional="true"/>
>>> +        <attribute name="sendBcc" type="String" mode="IN" optional="true"/>
>>>           <attribute name="note" type="String" mode="IN" optional="true"/>
>>>           <attribute name="temporaryAnonymousUserLogin" type="org.apache.ofbiz.entity.GenericValue" mode="IN" optional="true"/>
>>>           <attribute name="messageWrapper" type="org.apache.ofbiz.service.mail.MimeMessageWrapper" mode="OUT" optional="true"/>
>>>
>>> Modified: ofbiz/ofbiz-framework/trunk/applications/order/src/main/java/org/apache/ofbiz/order/order/OrderServices.java
>>> URL: 
>>> http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/order/src/main/java/org/apache/ofbiz/order/order/OrderServices.java?rev=1805459&r1=1805458&r2=1805459&view=diff
>>> ==============================================================================
>>> --- ofbiz/ofbiz-framework/trunk/applications/order/src/main/java/org/apache/ofbiz/order/order/OrderServices.java (original)
>>> +++ ofbiz/ofbiz-framework/trunk/applications/order/src/main/java/org/apache/ofbiz/order/order/OrderServices.java Fri Aug 18 19:10:41 2017
>>> @@ -83,6 +83,9 @@ import org.apache.ofbiz.service.ServiceU
>>>     import com.ibm.icu.util.Calendar;
>>>   +import org.apache.commons.lang.StringUtils;
>>> +import org.apache.fop.apps.MimeConstants;
>>> +
>>>   /**
>>>    * Order Processing Services
>>>    */
>>> @@ -2533,8 +2536,10 @@ public class OrderServices {
>>>           GenericValue userLogin = (GenericValue) context.get("userLogin");
>>>           String orderId = (String) context.get("orderId");
>>>           String orderItemSeqId = (String) context.get("orderItemSeqId");
>>> +        String shipGroupSeqId = (String) context.get("shipGroupSeqId");
>>>           String sendTo = (String) context.get("sendTo");
>>>           String sendCc = (String) context.get("sendCc");
>>> +        String sendBcc = (String) context.get("sendBcc");
>>>           String note = (String) context.get("note");
>>>           String screenUri = (String) context.get("screenUri");
>>>           GenericValue temporaryAnonymousUserLogin = (GenericValue) context.get("temporaryAnonymousUserLogin");
>>> @@ -2589,10 +2594,17 @@ public class OrderServices {
>>>               String xslfoAttachScreenLocation = productStoreEmail.getString("xslfoAttachScreenLocation");
>>>               sendMap.put("xslfoAttachScreenLocation", xslfoAttachScreenLocation);
>>>               // add attachmentName param to get an attachment namend "[oderId].pdf" instead of default "Details.pdf"
>>> -            sendMap.put("attachmentName", orderId + ".pdf");
>>> +            sendMap.put("attachmentName", (UtilValidate.isNotEmpty(shipGroupSeqId) ? orderId + "-" + StringUtils.stripStart(shipGroupSeqId, "0") 
>>> : orderId) + ".pdf");
>>> +            sendMap.put("attachmentType", MimeConstants.MIME_PDF);
>>>           } else {
>>>               sendMap.put("bodyScreenUri", screenUri);
>>>           }
>>> +
>>> +        if (context.containsKey("xslfoAttachScreenLocationList")) {
>>> +            sendMap.put("xslfoAttachScreenLocationList", context.get("xslfoAttachScreenLocationList"));
>>> +            sendMap.put("attachmentNameList", context.get("attachmentNameList"));
>>> +            sendMap.put("attachmentTypeList", context.get("attachmentTypeList"));
>>> +        }
>>>             // website
>>>           sendMap.put("webSiteId", orderHeader.get("webSiteId"));
>>> @@ -2636,6 +2648,7 @@ public class OrderServices {
>>>               bodyParameters.put("partyId", placingParty.get("partyId"));
>>>           }
>>>           bodyParameters.put("note", note);
>>> +        bodyParameters.put("shipGroupSeqId", shipGroupSeqId);
>>>           sendMap.put("bodyParameters", bodyParameters);
>>>           sendMap.put("userLogin",userLogin);
>>>   @@ -2657,6 +2670,10 @@ public class OrderServices {
>>>               sendMap.put("sendCc", productStoreEmail.get("ccAddress"));
>>>           }
>>>   +        if ((sendBcc != null) && UtilValidate.isEmailList(sendBcc)) {
>>> +            sendMap.put("sendBcc", sendBcc);
>>> +        }
>>> +
>>>           // send the notification
>>>           Map<String, Object> sendResp = null;
>>>           try {
>>>
>>> Modified: ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilValidate.java
>>> URL: 
>>> http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilValidate.java?rev=1805459&r1=1805458&r2=1805459&view=diff
>>> ==============================================================================
>>> --- ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilValidate.java (original)
>>> +++ ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilValidate.java Fri Aug 18 19:10:41 2017
>>> @@ -691,6 +691,20 @@ public final class UtilValidate {
>>>           if (isEmpty(s)) return defaultEmptyOK;
>>>           return EmailValidator.getInstance().isValid(s);
>>>       }
>>> +
>>> +    /**
>>> +     * Checks a String for a valid Email-List seperated by ",".
>>> +     */
>>> +    public static boolean isEmailList(String s) {
>>> +        if (isEmpty(s)) return defaultEmptyOK;
>>> +        String[] emails = s.split(",");
>>> +        for (String email : emails) {
>>> +            if (!EmailValidator.getInstance().isValid(email)) {
>>> +                return false;
>>> +            }
>>> +        }
>>> +        return true;
>>> +    }
>>>         /** isUrl returns true if the string contains ://
>>>        * @param s String to validate
>>>
>>> Modified: ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbiz/common/email/EmailServices.java
>>> URL: 
>>> http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbiz/common/email/EmailServices.java?rev=1805459&r1=1805458&r2=1805459&view=diff
>>> ==============================================================================
>>> --- ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbiz/common/email/EmailServices.java (original)
>>> +++ ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbiz/common/email/EmailServices.java Fri Aug 18 19:10:41 2017
>>> @@ -51,6 +51,7 @@ import javax.mail.internet.MimeMultipart
>>>   import javax.xml.parsers.ParserConfigurationException;
>>>   import javax.xml.transform.stream.StreamSource;
>>>   +import org.apache.fop.apps.FOPException;
>>>   import org.apache.fop.apps.Fop;
>>>   import org.apache.fop.apps.MimeConstants;
>>>   import org.apache.ofbiz.base.util.Debug;
>>> @@ -438,6 +439,12 @@ public class EmailServices {
>>>           if (UtilValidate.isNotEmpty(xslfoAttachScreenLocationListParam)) xslfoAttachScreenLocationList.addAll(xslfoAttachScreenLocationListParam);
>>>           if (UtilValidate.isNotEmpty(attachmentNameListParam)) attachmentNameList.addAll(attachmentNameListParam);
>>>           +        List<String> attachmentTypeList = new LinkedList<String>();
>>> +        String attachmentTypeParam = (String) serviceContext.remove("attachmentType");
>>> +        List<String> attachmentTypeListParam = UtilGenerics.checkList(serviceContext.remove("attachmentTypeList"));
>>> +        if (UtilValidate.isNotEmpty(attachmentTypeParam)) attachmentTypeList.add(attachmentTypeParam);
>>> +        if (UtilValidate.isNotEmpty(attachmentTypeListParam)) attachmentTypeList.addAll(attachmentTypeListParam);
>>> +
>>>           Locale locale = (Locale) serviceContext.get("locale");
>>>           Map<String, Object> bodyParameters = UtilGenerics.checkMap(serviceContext.remove("bodyParameters"));
>>>           if (bodyParameters == null) {
>>> @@ -504,9 +511,9 @@ public class EmailServices {
>>>               List<Map<String, ? extends Object>> bodyParts = new LinkedList<Map<String, ? extends Object>>();
>>>               if (bodyText != null) {
>>>                   bodyText = FlexibleStringExpander.expandString(bodyText, screenContext, locale);
>>> -                bodyParts.add(UtilMisc.<String, Object>toMap("content", bodyText, "type", "text/html"));
>>> +                bodyParts.add(UtilMisc.<String, Object>toMap("content", bodyText, "type", UtilValidate.isNotEmpty(contentType) ? contentType : 
>>> "text/html"));
>>>               } else {
>>> -                bodyParts.add(UtilMisc.<String, Object>toMap("content", bodyWriter.toString(), "type", "text/html"));
>>> +                bodyParts.add(UtilMisc.<String, Object>toMap("content", bodyWriter.toString(), "type", UtilValidate.isNotEmpty(contentType) ? 
>>> contentType : "text/html"));
>>>               }
>>>                             for (int i = 0; i < xslfoAttachScreenLocationList.size(); i++) {
>>> @@ -515,39 +522,51 @@ public class EmailServices {
>>>                   if (UtilValidate.isNotEmpty(attachmentNameList) && attachmentNameList.size() >= i) {
>>>                       attachmentName = attachmentNameList.get(i);
>>>                   }
>>> +
>>> +                String attachmentType = MimeConstants.MIME_PDF;
>>> +                if (UtilValidate.isNotEmpty(attachmentTypeList) && attachmentTypeList.size() >= i) {
>>> +                    attachmentType = attachmentTypeList.get(i);
>>> +                }
>>> +
>>>                   isMultiPart = true;
>>>                   // start processing fo pdf attachment
>>>                   try {
>>>                       Writer writer = new StringWriter();
>>> -                    MapStack<String> screenContextAtt = MapStack.create();
>>>                       // substitute the freemarker variables...
>>> -                    ScreenStringRenderer foScreenStringRenderer = new MacroScreenRenderer(EntityUtilProperties.getPropertyValue("widget", 
>>> "screenfop.name", dctx.getDelegator()),
>>> - EntityUtilProperties.getPropertyValue("widget", "screenfop.screenrenderer", dctx.getDelegator()));
>>> +                    ScreenStringRenderer foScreenStringRenderer = null;
>>> + if(MimeConstants.MIME_PLAIN_TEXT.equals(attachmentType)){
>>> +                        foScreenStringRenderer = new MacroScreenRenderer(EntityUtilProperties.getPropertyValue("widget", "screentext.name", 
>>> dctx.getDelegator()),
>>> + EntityUtilProperties.getPropertyValue("widget", "screentext.screenrenderer", dctx.getDelegator()));
>>> +                    }else{
>>> +                        foScreenStringRenderer = new MacroScreenRenderer(EntityUtilProperties.getPropertyValue("widget", "screenfop.name", 
>>> dctx.getDelegator()),
>>> + EntityUtilProperties.getPropertyValue("widget", "screenfop.screenrenderer", dctx.getDelegator()));
>>> +                    }
>>>                       ScreenRenderer screensAtt = new ScreenRenderer(writer, screenContext, foScreenStringRenderer);
>>> screensAtt.populateContextForService(dctx, bodyParameters);
>>> -                    screenContextAtt.putAll(bodyParameters);
>>> screensAtt.render(xslfoAttachScreenLocation);
>>>   -                    // create the input stream for the generation
>>> -                    StreamSource src = new StreamSource(new StringReader(writer.toString()));
>>> -
>>>                       // create the output stream for the generation
>>>                       ByteArrayOutputStream baos = new ByteArrayOutputStream();
>>>   -                    Fop fop = ApacheFopWorker.createFopInstance(baos, MimeConstants.MIME_PDF);
>>> -                    ApacheFopWorker.transform(src, null, fop);
>>> +                    if (MimeConstants.MIME_PLAIN_TEXT.equals(attachmentType)) {
>>> + baos.write(writer.toString().getBytes());
>>> +                    } else {
>>> +                        // create the input stream for the generation
>>> +                        StreamSource src = new StreamSource(new StringReader(writer.toString()));
>>> +                        Fop fop = ApacheFopWorker.createFopInstance(baos, attachmentType);
>>> +                        ApacheFopWorker.transform(src, null, fop);
>>> +                    }
>>>   -                    // and generate the PDF
>>> +                    // and generate the attachment
>>>                       baos.flush();
>>>                       baos.close();
>>>                         // store in the list of maps for sendmail....
>>> -                    bodyParts.add(UtilMisc.<String, Object> toMap("content", baos.toByteArray(), "type", "application/pdf", "filename",
>>> -                            attachmentName));
>>> -                } catch (Exception e) {
>>> -                    Debug.logError(e, "Error rendering PDF attachment for email: " + e.toString(), module);
>>> -                    return ServiceUtil.returnError(UtilProperties.getMessage(resource, "CommonEmailSendRenderingScreenPdfError",
>>> -                            UtilMisc.toMap("errorString", e.toString()), locale));
>>> +                    bodyParts.add(UtilMisc.<String, Object>toMap("content", baos.toByteArray(), "type", attachmentType, "filename", 
>>> attachmentName));
>>> +
>>> +                } catch (GeneralException|IOException|SAXException|ParserConfigurationException |TemplateException ge) {
>>> +                    Debug.logError(ge, "Error rendering PDF attachment for email: " + ge.toString(), module);
>>> +                    return ServiceUtil.returnError(UtilProperties.getMessage(resource, "CommonEmailSendRenderingScreenPdfError", 
>>> UtilMisc.toMap("errorString", ge.toString()), locale));
>>>                   }
>>> serviceContext.put("bodyParts", bodyParts);
>>>
>>>
>>>
>>
>


Re: svn commit: r1805459 - in /ofbiz/ofbiz-framework/trunk: applications/order/servicedef/ applications/order/src/main/java/org/apache/ofbiz/order/order/ framework/base/src/main/java/org/apache/ofbiz/base/util/ framework/common/src/main/java/org/apache/ofb...

Posted by Michael Brohl <mi...@ecomify.de>.
Hi Jacques,

sorry for the delay, we were in the middle of our movement to a new 
ecomify headquarter.

This is now fixed in r1810260, thanks for spotting!

Regards,

Michael


Am 27.09.17 um 14:12 schrieb Jacques Le Roux:
> Hi Michael,
>
> You introduced attachmentType in this commit but the service 
> sendMailFromScreen does not declare this parameter, nor any other 
> service, same for attachmentTypeList.
>
> Please check
>
> Thanks
>
> Jacques
>
>
> Le 18/08/2017 à 21:10, mbrohl@apache.org a écrit :
>> Author: mbrohl
>> Date: Fri Aug 18 19:10:41 2017
>> New Revision: 1805459
>>
>> URL: http://svn.apache.org/viewvc?rev=1805459&view=rev
>> Log:
>> Improved: EmailServices.sendMailFromScreen improved to take multiple
>> attachments with appropriate type along with several email fixes.
>> (OFBIZ-9395)
>>
>> This patch fixes a number of issues combined in a patch because of the
>> code dependencies.
>>
>> 1. it enables to add BCC adress(es) to service
>> OrderServices.sendOrderNotificationScreens to oversteer
>> ProductStoreEmailSetting of BCC the same was as for CC.
>>
>> 2. a method UtilValidate.isEmailList(String) is added to check a
>> comma separated list of email addresses, used for example to check the
>> String passed to the new BCC field for an Order-Notification.
>>
>> 3. there are improvements in EmailServices.sendMailFromScreen. The
>> attachment type of MailAttachments is now not only .pdf but depends on
>> the specific file. This has not been the case before - the mime type was
>> always hard coded as .pdf. The same goes for the bodyPart content-type
>> which is now set to the passed content type or the default text/html
>> type. Before this was also always set to text/html.
>> Additionally, an attachment that has the mime-type text/plain is not
>> rendered with the fop-renderer anymore but with a simple text-renderer.
>> Therefore it is possible to send an CSV file as attachment now.
>>
>> The patch also refactors some catch-Blocks in the
>> EmailServices.sendMailFromScreen by using multi-catch since the e
>> xception handling is always the same.
>>
>> Thanks Martin Becker for reporting and providing the patch.
>>
>> Modified:
>> ofbiz/ofbiz-framework/trunk/applications/order/servicedef/services.xml
>> ofbiz/ofbiz-framework/trunk/applications/order/src/main/java/org/apache/ofbiz/order/order/OrderServices.java
>> ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilValidate.java
>> ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbiz/common/email/EmailServices.java
>>
>> Modified: 
>> ofbiz/ofbiz-framework/trunk/applications/order/servicedef/services.xml
>> URL: 
>> http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/order/servicedef/services.xml?rev=1805459&r1=1805458&r2=1805459&view=diff
>> ============================================================================== 
>>
>> --- 
>> ofbiz/ofbiz-framework/trunk/applications/order/servicedef/services.xml 
>> (original)
>> +++ 
>> ofbiz/ofbiz-framework/trunk/applications/order/servicedef/services.xml 
>> Fri Aug 18 19:10:41 2017
>> @@ -34,6 +34,7 @@ under the License.
>>           <attribute name="body" type="String" mode="OUT" 
>> optional="true"/>
>>           <attribute name="sendTo" type="String" mode="IN" 
>> optional="true"/>
>>           <attribute name="sendCc" type="String" mode="IN" 
>> optional="true"/>
>> +        <attribute name="sendBcc" type="String" mode="IN" 
>> optional="true"/>
>>           <attribute name="note" type="String" mode="IN" 
>> optional="true"/>
>>           <attribute name="temporaryAnonymousUserLogin" 
>> type="org.apache.ofbiz.entity.GenericValue" mode="IN" optional="true"/>
>>           <attribute name="messageWrapper" 
>> type="org.apache.ofbiz.service.mail.MimeMessageWrapper" mode="OUT" 
>> optional="true"/>
>>
>> Modified: 
>> ofbiz/ofbiz-framework/trunk/applications/order/src/main/java/org/apache/ofbiz/order/order/OrderServices.java
>> URL: 
>> http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/order/src/main/java/org/apache/ofbiz/order/order/OrderServices.java?rev=1805459&r1=1805458&r2=1805459&view=diff
>> ============================================================================== 
>>
>> --- 
>> ofbiz/ofbiz-framework/trunk/applications/order/src/main/java/org/apache/ofbiz/order/order/OrderServices.java 
>> (original)
>> +++ 
>> ofbiz/ofbiz-framework/trunk/applications/order/src/main/java/org/apache/ofbiz/order/order/OrderServices.java 
>> Fri Aug 18 19:10:41 2017
>> @@ -83,6 +83,9 @@ import org.apache.ofbiz.service.ServiceU
>>     import com.ibm.icu.util.Calendar;
>>   +import org.apache.commons.lang.StringUtils;
>> +import org.apache.fop.apps.MimeConstants;
>> +
>>   /**
>>    * Order Processing Services
>>    */
>> @@ -2533,8 +2536,10 @@ public class OrderServices {
>>           GenericValue userLogin = (GenericValue) 
>> context.get("userLogin");
>>           String orderId = (String) context.get("orderId");
>>           String orderItemSeqId = (String) 
>> context.get("orderItemSeqId");
>> +        String shipGroupSeqId = (String) context.get("shipGroupSeqId");
>>           String sendTo = (String) context.get("sendTo");
>>           String sendCc = (String) context.get("sendCc");
>> +        String sendBcc = (String) context.get("sendBcc");
>>           String note = (String) context.get("note");
>>           String screenUri = (String) context.get("screenUri");
>>           GenericValue temporaryAnonymousUserLogin = (GenericValue) 
>> context.get("temporaryAnonymousUserLogin");
>> @@ -2589,10 +2594,17 @@ public class OrderServices {
>>               String xslfoAttachScreenLocation = 
>> productStoreEmail.getString("xslfoAttachScreenLocation");
>>               sendMap.put("xslfoAttachScreenLocation", 
>> xslfoAttachScreenLocation);
>>               // add attachmentName param to get an attachment namend 
>> "[oderId].pdf" instead of default "Details.pdf"
>> -            sendMap.put("attachmentName", orderId + ".pdf");
>> +            sendMap.put("attachmentName", 
>> (UtilValidate.isNotEmpty(shipGroupSeqId) ? orderId + "-" + 
>> StringUtils.stripStart(shipGroupSeqId, "0") : orderId) + ".pdf");
>> +            sendMap.put("attachmentType", MimeConstants.MIME_PDF);
>>           } else {
>>               sendMap.put("bodyScreenUri", screenUri);
>>           }
>> +
>> +        if (context.containsKey("xslfoAttachScreenLocationList")) {
>> +            sendMap.put("xslfoAttachScreenLocationList", 
>> context.get("xslfoAttachScreenLocationList"));
>> +            sendMap.put("attachmentNameList", 
>> context.get("attachmentNameList"));
>> +            sendMap.put("attachmentTypeList", 
>> context.get("attachmentTypeList"));
>> +        }
>>             // website
>>           sendMap.put("webSiteId", orderHeader.get("webSiteId"));
>> @@ -2636,6 +2648,7 @@ public class OrderServices {
>>               bodyParameters.put("partyId", 
>> placingParty.get("partyId"));
>>           }
>>           bodyParameters.put("note", note);
>> +        bodyParameters.put("shipGroupSeqId", shipGroupSeqId);
>>           sendMap.put("bodyParameters", bodyParameters);
>>           sendMap.put("userLogin",userLogin);
>>   @@ -2657,6 +2670,10 @@ public class OrderServices {
>>               sendMap.put("sendCc", productStoreEmail.get("ccAddress"));
>>           }
>>   +        if ((sendBcc != null) && UtilValidate.isEmailList(sendBcc)) {
>> +            sendMap.put("sendBcc", sendBcc);
>> +        }
>> +
>>           // send the notification
>>           Map<String, Object> sendResp = null;
>>           try {
>>
>> Modified: 
>> ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilValidate.java
>> URL: 
>> http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilValidate.java?rev=1805459&r1=1805458&r2=1805459&view=diff
>> ============================================================================== 
>>
>> --- 
>> ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilValidate.java 
>> (original)
>> +++ 
>> ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilValidate.java 
>> Fri Aug 18 19:10:41 2017
>> @@ -691,6 +691,20 @@ public final class UtilValidate {
>>           if (isEmpty(s)) return defaultEmptyOK;
>>           return EmailValidator.getInstance().isValid(s);
>>       }
>> +
>> +    /**
>> +     * Checks a String for a valid Email-List seperated by ",".
>> +     */
>> +    public static boolean isEmailList(String s) {
>> +        if (isEmpty(s)) return defaultEmptyOK;
>> +        String[] emails = s.split(",");
>> +        for (String email : emails) {
>> +            if (!EmailValidator.getInstance().isValid(email)) {
>> +                return false;
>> +            }
>> +        }
>> +        return true;
>> +    }
>>         /** isUrl returns true if the string contains ://
>>        * @param s String to validate
>>
>> Modified: 
>> ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbiz/common/email/EmailServices.java
>> URL: 
>> http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbiz/common/email/EmailServices.java?rev=1805459&r1=1805458&r2=1805459&view=diff
>> ============================================================================== 
>>
>> --- 
>> ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbiz/common/email/EmailServices.java 
>> (original)
>> +++ 
>> ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbiz/common/email/EmailServices.java 
>> Fri Aug 18 19:10:41 2017
>> @@ -51,6 +51,7 @@ import javax.mail.internet.MimeMultipart
>>   import javax.xml.parsers.ParserConfigurationException;
>>   import javax.xml.transform.stream.StreamSource;
>>   +import org.apache.fop.apps.FOPException;
>>   import org.apache.fop.apps.Fop;
>>   import org.apache.fop.apps.MimeConstants;
>>   import org.apache.ofbiz.base.util.Debug;
>> @@ -438,6 +439,12 @@ public class EmailServices {
>>           if 
>> (UtilValidate.isNotEmpty(xslfoAttachScreenLocationListParam)) 
>> xslfoAttachScreenLocationList.addAll(xslfoAttachScreenLocationListParam);
>>           if (UtilValidate.isNotEmpty(attachmentNameListParam)) 
>> attachmentNameList.addAll(attachmentNameListParam);
>>           +        List<String> attachmentTypeList = new 
>> LinkedList<String>();
>> +        String attachmentTypeParam = (String) 
>> serviceContext.remove("attachmentType");
>> +        List<String> attachmentTypeListParam = 
>> UtilGenerics.checkList(serviceContext.remove("attachmentTypeList"));
>> +        if (UtilValidate.isNotEmpty(attachmentTypeParam)) 
>> attachmentTypeList.add(attachmentTypeParam);
>> +        if (UtilValidate.isNotEmpty(attachmentTypeListParam)) 
>> attachmentTypeList.addAll(attachmentTypeListParam);
>> +
>>           Locale locale = (Locale) serviceContext.get("locale");
>>           Map<String, Object> bodyParameters = 
>> UtilGenerics.checkMap(serviceContext.remove("bodyParameters"));
>>           if (bodyParameters == null) {
>> @@ -504,9 +511,9 @@ public class EmailServices {
>>               List<Map<String, ? extends Object>> bodyParts = new 
>> LinkedList<Map<String, ? extends Object>>();
>>               if (bodyText != null) {
>>                   bodyText = 
>> FlexibleStringExpander.expandString(bodyText, screenContext, locale);
>> -                bodyParts.add(UtilMisc.<String, 
>> Object>toMap("content", bodyText, "type", "text/html"));
>> +                bodyParts.add(UtilMisc.<String, 
>> Object>toMap("content", bodyText, "type", 
>> UtilValidate.isNotEmpty(contentType) ? contentType : "text/html"));
>>               } else {
>> -                bodyParts.add(UtilMisc.<String, 
>> Object>toMap("content", bodyWriter.toString(), "type", "text/html"));
>> +                bodyParts.add(UtilMisc.<String, 
>> Object>toMap("content", bodyWriter.toString(), "type", 
>> UtilValidate.isNotEmpty(contentType) ? contentType : "text/html"));
>>               }
>>                             for (int i = 0; i < 
>> xslfoAttachScreenLocationList.size(); i++) {
>> @@ -515,39 +522,51 @@ public class EmailServices {
>>                   if (UtilValidate.isNotEmpty(attachmentNameList) && 
>> attachmentNameList.size() >= i) {
>>                       attachmentName = attachmentNameList.get(i);
>>                   }
>> +
>> +                String attachmentType = MimeConstants.MIME_PDF;
>> +                if (UtilValidate.isNotEmpty(attachmentTypeList) && 
>> attachmentTypeList.size() >= i) {
>> +                    attachmentType = attachmentTypeList.get(i);
>> +                }
>> +
>>                   isMultiPart = true;
>>                   // start processing fo pdf attachment
>>                   try {
>>                       Writer writer = new StringWriter();
>> -                    MapStack<String> screenContextAtt = 
>> MapStack.create();
>>                       // substitute the freemarker variables...
>> -                    ScreenStringRenderer foScreenStringRenderer = 
>> new 
>> MacroScreenRenderer(EntityUtilProperties.getPropertyValue("widget", 
>> "screenfop.name", dctx.getDelegator()),
>> - EntityUtilProperties.getPropertyValue("widget", 
>> "screenfop.screenrenderer", dctx.getDelegator()));
>> +                    ScreenStringRenderer foScreenStringRenderer = null;
>> + if(MimeConstants.MIME_PLAIN_TEXT.equals(attachmentType)){
>> +                        foScreenStringRenderer = new 
>> MacroScreenRenderer(EntityUtilProperties.getPropertyValue("widget", 
>> "screentext.name", dctx.getDelegator()),
>> + EntityUtilProperties.getPropertyValue("widget", 
>> "screentext.screenrenderer", dctx.getDelegator()));
>> +                    }else{
>> +                        foScreenStringRenderer = new 
>> MacroScreenRenderer(EntityUtilProperties.getPropertyValue("widget", 
>> "screenfop.name", dctx.getDelegator()),
>> + EntityUtilProperties.getPropertyValue("widget", 
>> "screenfop.screenrenderer", dctx.getDelegator()));
>> +                    }
>>                       ScreenRenderer screensAtt = new 
>> ScreenRenderer(writer, screenContext, foScreenStringRenderer);
>>                       screensAtt.populateContextForService(dctx, 
>> bodyParameters);
>> -                    screenContextAtt.putAll(bodyParameters);
>> screensAtt.render(xslfoAttachScreenLocation);
>>   -                    // create the input stream for the generation
>> -                    StreamSource src = new StreamSource(new 
>> StringReader(writer.toString()));
>> -
>>                       // create the output stream for the generation
>>                       ByteArrayOutputStream baos = new 
>> ByteArrayOutputStream();
>>   -                    Fop fop = 
>> ApacheFopWorker.createFopInstance(baos, MimeConstants.MIME_PDF);
>> -                    ApacheFopWorker.transform(src, null, fop);
>> +                    if 
>> (MimeConstants.MIME_PLAIN_TEXT.equals(attachmentType)) {
>> + baos.write(writer.toString().getBytes());
>> +                    } else {
>> +                        // create the input stream for the generation
>> +                        StreamSource src = new StreamSource(new 
>> StringReader(writer.toString()));
>> +                        Fop fop = 
>> ApacheFopWorker.createFopInstance(baos, attachmentType);
>> +                        ApacheFopWorker.transform(src, null, fop);
>> +                    }
>>   -                    // and generate the PDF
>> +                    // and generate the attachment
>>                       baos.flush();
>>                       baos.close();
>>                         // store in the list of maps for sendmail....
>> -                    bodyParts.add(UtilMisc.<String, Object> 
>> toMap("content", baos.toByteArray(), "type", "application/pdf", 
>> "filename",
>> -                            attachmentName));
>> -                } catch (Exception e) {
>> -                    Debug.logError(e, "Error rendering PDF 
>> attachment for email: " + e.toString(), module);
>> -                    return 
>> ServiceUtil.returnError(UtilProperties.getMessage(resource, 
>> "CommonEmailSendRenderingScreenPdfError",
>> -                            UtilMisc.toMap("errorString", 
>> e.toString()), locale));
>> +                    bodyParts.add(UtilMisc.<String, 
>> Object>toMap("content", baos.toByteArray(), "type", attachmentType, 
>> "filename", attachmentName));
>> +
>> +                } catch 
>> (GeneralException|IOException|SAXException|ParserConfigurationException 
>> |TemplateException ge) {
>> +                    Debug.logError(ge, "Error rendering PDF 
>> attachment for email: " + ge.toString(), module);
>> +                    return 
>> ServiceUtil.returnError(UtilProperties.getMessage(resource, 
>> "CommonEmailSendRenderingScreenPdfError", 
>> UtilMisc.toMap("errorString", ge.toString()), locale));
>>                   }
>> serviceContext.put("bodyParts", bodyParts);
>>
>>
>>
>


Re: svn commit: r1805459 - in /ofbiz/ofbiz-framework/trunk: applications/order/servicedef/ applications/order/src/main/java/org/apache/ofbiz/order/order/ framework/base/src/main/java/org/apache/ofbiz/base/util/ framework/common/src/main/java/org/apache/ofb...

Posted by Jacques Le Roux <ja...@les7arts.com>.
Hi Michael,

You introduced attachmentType in this commit but the service sendMailFromScreen does not declare this parameter, nor any other service, same for 
attachmentTypeList.

Please check

Thanks

Jacques


Le 18/08/2017 à 21:10, mbrohl@apache.org a écrit :
> Author: mbrohl
> Date: Fri Aug 18 19:10:41 2017
> New Revision: 1805459
>
> URL: http://svn.apache.org/viewvc?rev=1805459&view=rev
> Log:
> Improved: EmailServices.sendMailFromScreen improved to take multiple
> attachments with appropriate type along with several email fixes.
> (OFBIZ-9395)
>
> This patch fixes a number of issues combined in a patch because of the
> code dependencies.
>
> 1. it enables to add BCC adress(es) to service
> OrderServices.sendOrderNotificationScreens to oversteer
> ProductStoreEmailSetting of BCC the same was as for CC.
>
> 2. a method UtilValidate.isEmailList(String) is added to check a
> comma separated list of email addresses, used for example to check the
> String passed to the new BCC field for an Order-Notification.
>
> 3. there are improvements in EmailServices.sendMailFromScreen. The
> attachment type of MailAttachments is now not only .pdf but depends on
> the specific file. This has not been the case before - the mime type was
> always hard coded as .pdf. The same goes for the bodyPart content-type
> which is now set to the passed content type or the default text/html
> type. Before this was also always set to text/html.
> Additionally, an attachment that has the mime-type text/plain is not
> rendered with the fop-renderer anymore but with a simple text-renderer.
> Therefore it is possible to send an CSV file as attachment now.
>
> The patch also refactors some catch-Blocks in the
> EmailServices.sendMailFromScreen by using multi-catch since the e
> xception handling is always the same.
>
> Thanks Martin Becker for reporting and providing the patch.
>
> Modified:
>      ofbiz/ofbiz-framework/trunk/applications/order/servicedef/services.xml
>      ofbiz/ofbiz-framework/trunk/applications/order/src/main/java/org/apache/ofbiz/order/order/OrderServices.java
>      ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilValidate.java
>      ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbiz/common/email/EmailServices.java
>
> Modified: ofbiz/ofbiz-framework/trunk/applications/order/servicedef/services.xml
> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/order/servicedef/services.xml?rev=1805459&r1=1805458&r2=1805459&view=diff
> ==============================================================================
> --- ofbiz/ofbiz-framework/trunk/applications/order/servicedef/services.xml (original)
> +++ ofbiz/ofbiz-framework/trunk/applications/order/servicedef/services.xml Fri Aug 18 19:10:41 2017
> @@ -34,6 +34,7 @@ under the License.
>           <attribute name="body" type="String" mode="OUT" optional="true"/>
>           <attribute name="sendTo" type="String" mode="IN" optional="true"/>
>           <attribute name="sendCc" type="String" mode="IN" optional="true"/>
> +        <attribute name="sendBcc" type="String" mode="IN" optional="true"/>
>           <attribute name="note" type="String" mode="IN" optional="true"/>
>           <attribute name="temporaryAnonymousUserLogin" type="org.apache.ofbiz.entity.GenericValue" mode="IN" optional="true"/>
>           <attribute name="messageWrapper" type="org.apache.ofbiz.service.mail.MimeMessageWrapper" mode="OUT" optional="true"/>
>
> Modified: ofbiz/ofbiz-framework/trunk/applications/order/src/main/java/org/apache/ofbiz/order/order/OrderServices.java
> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/applications/order/src/main/java/org/apache/ofbiz/order/order/OrderServices.java?rev=1805459&r1=1805458&r2=1805459&view=diff
> ==============================================================================
> --- ofbiz/ofbiz-framework/trunk/applications/order/src/main/java/org/apache/ofbiz/order/order/OrderServices.java (original)
> +++ ofbiz/ofbiz-framework/trunk/applications/order/src/main/java/org/apache/ofbiz/order/order/OrderServices.java Fri Aug 18 19:10:41 2017
> @@ -83,6 +83,9 @@ import org.apache.ofbiz.service.ServiceU
>   
>   import com.ibm.icu.util.Calendar;
>   
> +import org.apache.commons.lang.StringUtils;
> +import org.apache.fop.apps.MimeConstants;
> +
>   /**
>    * Order Processing Services
>    */
> @@ -2533,8 +2536,10 @@ public class OrderServices {
>           GenericValue userLogin = (GenericValue) context.get("userLogin");
>           String orderId = (String) context.get("orderId");
>           String orderItemSeqId = (String) context.get("orderItemSeqId");
> +        String shipGroupSeqId = (String) context.get("shipGroupSeqId");
>           String sendTo = (String) context.get("sendTo");
>           String sendCc = (String) context.get("sendCc");
> +        String sendBcc = (String) context.get("sendBcc");
>           String note = (String) context.get("note");
>           String screenUri = (String) context.get("screenUri");
>           GenericValue temporaryAnonymousUserLogin = (GenericValue) context.get("temporaryAnonymousUserLogin");
> @@ -2589,10 +2594,17 @@ public class OrderServices {
>               String xslfoAttachScreenLocation = productStoreEmail.getString("xslfoAttachScreenLocation");
>               sendMap.put("xslfoAttachScreenLocation", xslfoAttachScreenLocation);
>               // add attachmentName param to get an attachment namend "[oderId].pdf" instead of default "Details.pdf"
> -            sendMap.put("attachmentName", orderId + ".pdf");
> +            sendMap.put("attachmentName", (UtilValidate.isNotEmpty(shipGroupSeqId) ? orderId + "-" + StringUtils.stripStart(shipGroupSeqId, "0") : orderId) + ".pdf");
> +            sendMap.put("attachmentType", MimeConstants.MIME_PDF);
>           } else {
>               sendMap.put("bodyScreenUri", screenUri);
>           }
> +
> +        if (context.containsKey("xslfoAttachScreenLocationList")) {
> +            sendMap.put("xslfoAttachScreenLocationList", context.get("xslfoAttachScreenLocationList"));
> +            sendMap.put("attachmentNameList", context.get("attachmentNameList"));
> +            sendMap.put("attachmentTypeList", context.get("attachmentTypeList"));
> +        }
>   
>           // website
>           sendMap.put("webSiteId", orderHeader.get("webSiteId"));
> @@ -2636,6 +2648,7 @@ public class OrderServices {
>               bodyParameters.put("partyId", placingParty.get("partyId"));
>           }
>           bodyParameters.put("note", note);
> +        bodyParameters.put("shipGroupSeqId", shipGroupSeqId);
>           sendMap.put("bodyParameters", bodyParameters);
>           sendMap.put("userLogin",userLogin);
>   
> @@ -2657,6 +2670,10 @@ public class OrderServices {
>               sendMap.put("sendCc", productStoreEmail.get("ccAddress"));
>           }
>   
> +        if ((sendBcc != null) && UtilValidate.isEmailList(sendBcc)) {
> +            sendMap.put("sendBcc", sendBcc);
> +        }
> +
>           // send the notification
>           Map<String, Object> sendResp = null;
>           try {
>
> Modified: ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilValidate.java
> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilValidate.java?rev=1805459&r1=1805458&r2=1805459&view=diff
> ==============================================================================
> --- ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilValidate.java (original)
> +++ ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/UtilValidate.java Fri Aug 18 19:10:41 2017
> @@ -691,6 +691,20 @@ public final class UtilValidate {
>           if (isEmpty(s)) return defaultEmptyOK;
>           return EmailValidator.getInstance().isValid(s);
>       }
> +
> +    /**
> +     * Checks a String for a valid Email-List seperated by ",".
> +     */
> +    public static boolean isEmailList(String s) {
> +        if (isEmpty(s)) return defaultEmptyOK;
> +        String[] emails = s.split(",");
> +        for (String email : emails) {
> +            if (!EmailValidator.getInstance().isValid(email)) {
> +                return false;
> +            }
> +        }
> +        return true;
> +    }
>   
>       /** isUrl returns true if the string contains ://
>        * @param s String to validate
>
> Modified: ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbiz/common/email/EmailServices.java
> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbiz/common/email/EmailServices.java?rev=1805459&r1=1805458&r2=1805459&view=diff
> ==============================================================================
> --- ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbiz/common/email/EmailServices.java (original)
> +++ ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbiz/common/email/EmailServices.java Fri Aug 18 19:10:41 2017
> @@ -51,6 +51,7 @@ import javax.mail.internet.MimeMultipart
>   import javax.xml.parsers.ParserConfigurationException;
>   import javax.xml.transform.stream.StreamSource;
>   
> +import org.apache.fop.apps.FOPException;
>   import org.apache.fop.apps.Fop;
>   import org.apache.fop.apps.MimeConstants;
>   import org.apache.ofbiz.base.util.Debug;
> @@ -438,6 +439,12 @@ public class EmailServices {
>           if (UtilValidate.isNotEmpty(xslfoAttachScreenLocationListParam)) xslfoAttachScreenLocationList.addAll(xslfoAttachScreenLocationListParam);
>           if (UtilValidate.isNotEmpty(attachmentNameListParam)) attachmentNameList.addAll(attachmentNameListParam);
>           
> +        List<String> attachmentTypeList = new LinkedList<String>();
> +        String attachmentTypeParam = (String) serviceContext.remove("attachmentType");
> +        List<String> attachmentTypeListParam = UtilGenerics.checkList(serviceContext.remove("attachmentTypeList"));
> +        if (UtilValidate.isNotEmpty(attachmentTypeParam)) attachmentTypeList.add(attachmentTypeParam);
> +        if (UtilValidate.isNotEmpty(attachmentTypeListParam)) attachmentTypeList.addAll(attachmentTypeListParam);
> +
>           Locale locale = (Locale) serviceContext.get("locale");
>           Map<String, Object> bodyParameters = UtilGenerics.checkMap(serviceContext.remove("bodyParameters"));
>           if (bodyParameters == null) {
> @@ -504,9 +511,9 @@ public class EmailServices {
>               List<Map<String, ? extends Object>> bodyParts = new LinkedList<Map<String, ? extends Object>>();
>               if (bodyText != null) {
>                   bodyText = FlexibleStringExpander.expandString(bodyText, screenContext,  locale);
> -                bodyParts.add(UtilMisc.<String, Object>toMap("content", bodyText, "type", "text/html"));
> +                bodyParts.add(UtilMisc.<String, Object>toMap("content", bodyText, "type", UtilValidate.isNotEmpty(contentType) ? contentType : "text/html"));
>               } else {
> -                bodyParts.add(UtilMisc.<String, Object>toMap("content", bodyWriter.toString(), "type", "text/html"));
> +                bodyParts.add(UtilMisc.<String, Object>toMap("content", bodyWriter.toString(), "type", UtilValidate.isNotEmpty(contentType) ? contentType : "text/html"));
>               }
>               
>               for (int i = 0; i < xslfoAttachScreenLocationList.size(); i++) {
> @@ -515,39 +522,51 @@ public class EmailServices {
>                   if (UtilValidate.isNotEmpty(attachmentNameList) && attachmentNameList.size() >= i) {
>                       attachmentName = attachmentNameList.get(i);
>                   }
> +
> +                String attachmentType = MimeConstants.MIME_PDF;
> +                if (UtilValidate.isNotEmpty(attachmentTypeList) && attachmentTypeList.size() >= i) {
> +                    attachmentType = attachmentTypeList.get(i);
> +                }
> +
>                   isMultiPart = true;
>                   // start processing fo pdf attachment
>                   try {
>                       Writer writer = new StringWriter();
> -                    MapStack<String> screenContextAtt = MapStack.create();
>                       // substitute the freemarker variables...
> -                    ScreenStringRenderer foScreenStringRenderer = new MacroScreenRenderer(EntityUtilProperties.getPropertyValue("widget", "screenfop.name", dctx.getDelegator()),
> -                            EntityUtilProperties.getPropertyValue("widget", "screenfop.screenrenderer", dctx.getDelegator()));
> +                    ScreenStringRenderer foScreenStringRenderer = null;
> +                    if(MimeConstants.MIME_PLAIN_TEXT.equals(attachmentType)){
> +                        foScreenStringRenderer = new MacroScreenRenderer(EntityUtilProperties.getPropertyValue("widget", "screentext.name", dctx.getDelegator()),
> +                                EntityUtilProperties.getPropertyValue("widget", "screentext.screenrenderer", dctx.getDelegator()));
> +                    }else{
> +                        foScreenStringRenderer = new MacroScreenRenderer(EntityUtilProperties.getPropertyValue("widget", "screenfop.name", dctx.getDelegator()),
> +                                EntityUtilProperties.getPropertyValue("widget", "screenfop.screenrenderer", dctx.getDelegator()));
> +                    }
>                       ScreenRenderer screensAtt = new ScreenRenderer(writer, screenContext, foScreenStringRenderer);
>                       screensAtt.populateContextForService(dctx, bodyParameters);
> -                    screenContextAtt.putAll(bodyParameters);
>                       screensAtt.render(xslfoAttachScreenLocation);
>   
> -                    // create the input stream for the generation
> -                    StreamSource src = new StreamSource(new StringReader(writer.toString()));
> -
>                       // create the output stream for the generation
>                       ByteArrayOutputStream baos = new ByteArrayOutputStream();
>   
> -                    Fop fop = ApacheFopWorker.createFopInstance(baos, MimeConstants.MIME_PDF);
> -                    ApacheFopWorker.transform(src, null, fop);
> +                    if (MimeConstants.MIME_PLAIN_TEXT.equals(attachmentType)) {
> +                        baos.write(writer.toString().getBytes());
> +                    } else {
> +                        // create the input stream for the generation
> +                        StreamSource src = new StreamSource(new StringReader(writer.toString()));
> +                        Fop fop = ApacheFopWorker.createFopInstance(baos, attachmentType);
> +                        ApacheFopWorker.transform(src, null, fop);
> +                    }
>   
> -                    // and generate the PDF
> +                    // and generate the attachment
>                       baos.flush();
>                       baos.close();
>   
>                       // store in the list of maps for sendmail....
> -                    bodyParts.add(UtilMisc.<String, Object> toMap("content", baos.toByteArray(), "type", "application/pdf", "filename",
> -                            attachmentName));
> -                } catch (Exception e) {
> -                    Debug.logError(e, "Error rendering PDF attachment for email: " + e.toString(), module);
> -                    return ServiceUtil.returnError(UtilProperties.getMessage(resource, "CommonEmailSendRenderingScreenPdfError",
> -                            UtilMisc.toMap("errorString", e.toString()), locale));
> +                    bodyParts.add(UtilMisc.<String, Object>toMap("content", baos.toByteArray(), "type", attachmentType, "filename", attachmentName));
> +
> +                } catch (GeneralException|IOException|SAXException|ParserConfigurationException |TemplateException ge) {
> +                    Debug.logError(ge, "Error rendering PDF attachment for email: " + ge.toString(), module);
> +                    return ServiceUtil.returnError(UtilProperties.getMessage(resource, "CommonEmailSendRenderingScreenPdfError", UtilMisc.toMap("errorString", ge.toString()), locale));
>                   }
>                   
>                   serviceContext.put("bodyParts", bodyParts);
>
>
>