+1, well appreciated, Jacques. Regards,
Michael Am 29.03.17 um 17:30 schrieb Jacques Le Roux:
Hi Taher,OK, I got back to this and finally understood what happened. I was fed up by the "swallowed exceptions" convo and was in a bull mindset[1].So when you rightly wrote >I believe the @SuppressWarnings tags are used incorrectly in this commit >and in some cases hide the root problem.I did not notice I wrote "Fixed some unchecked casts by using UtilMisc.toMap" in my commit comment. Of course this makes no sense and I thought about using UtilGenerics.checkMap and alike. I was sure I used that, which I agree is worrying :D... Age certainly That's why I answered "Please specify which ones you see not fit", being sure I did right.Then you answered >All of them.This is where all derailed. Because not all changes was concerned. And then, still being in bull mindset, I did not put sufficiently attention to your last answer.>Well, if you think using UtilGenerics or removing unused variables or >refactoring code to remove unnecessary warnings is FUD, then I have no >comment.Because I sincerely though I used UtilGenerics. I somehow got mislead by Eclipse. It also hides the warning when you use UtilMisc.toMap so I did not notice and copied it everywhere. But anyway it's not an excuse and I should have noticed the issue with List, which you reported in you 1st answer :/About removing unused variables, I did not because one is really used (modelService in createFlexibleReportFromMasterServiceWorkflow). And I think the other (imageHandler in BirtUtil) will maybe used later, I put a TODOThere is though one that we can surely get rid of: private GenericValue userLogin; in ReportDesignGenerator which is really useless So I have fixed it all at, r1789381 Thanks for your review, and please accept my apologies. Jacques[1] Note: I believe I'm totally right about the "swallowed exceptions" convo and I'll try to prove it since I'm asked so.Le 28/03/2017 à 05:47, Jacques Le Roux a écrit :Thanks for your detailed analysis. And yes this sentence cynical. I know exactly why I did so in each case, contrary as what you seem to think.Jacques Le 27/03/2017 à 23:05, Taher Alkhateeb a écrit :Well, if you think using UtilGenerics or removing unused variables or refactoring code to remove unnecessary warnings is FUD, then I have no comment. On Mon, Mar 27, 2017 at 4:26 PM, Jacques Le Roux < [email protected]> wrote:This if FUD Jacques Le 27/03/2017 à 13:40, Taher Alkhateeb a écrit :All of them. On Mon, Mar 27, 2017 at 2:29 PM, Jacques Le Roux < [email protected]> wrote: Please specify which ones you see not fitThanks Jacques Le 27/03/2017 à 12:14, Taher Alkhateeb a écrit : cand in some cases hide the root problem. On Mon, Mar 27, 2017 at 12:54 PM, <[email protected]> wrote: Author: jlerouxDate: Mon Mar 27 09:54:18 2017 New Revision: 1788869 URL: http://svn.apache.org/viewvc?rev=1788869&view=rev Log: No functional changes. Fixes some unchecked casts by using UtilMisc.toMap Adds some @SuppressWarnings("unchecked") Adds few @SuppressWarnings("unused") Cleans imports Completes and fixes Javadoc in BirtUtil class Modified: ofbiz/ofbiz-plugins/trunk/birt/src/main/java/org/apache/ ofbiz/birt/flexible/BirtMasterReportServices.java ofbiz/ofbiz-plugins/trunk/birt/src/main/java/org/apache/ ofbiz/birt/flexible/BirtServices.java ofbiz/ofbiz-plugins/trunk/birt/src/main/java/org/apache/ ofbiz/birt/flexible/BirtUtil.java ofbiz/ofbiz-plugins/trunk/birt/src/main/java/org/apache/ ofbiz/birt/flexible/ReportDesignGenerator.java Modified: ofbiz/ofbiz-plugins/trunk/birt/src/main/java/org/apache/ ofbiz/birt/flexible/BirtMasterReportServices.java URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-plugins/trunk/ birt/src/main/java/org/apache/ofbiz/birt/flexible/ BirtMasterReportServices.java?rev=1788869&r1=1788868&r2=1788 869&view=diff ============================================================ ================== --- ofbiz/ofbiz-plugins/trunk/birt/src/main/java/org/apache/ ofbiz/birt/flexible/BirtMasterReportServices.java (original) +++ ofbiz/ofbiz-plugins/trunk/birt/src/main/java/org/apache/ofbiz/birt/flexible/BirtMasterReportServices.java Mon Mar 27 09:54:182017 @@ -9,6 +9,7 @@ import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Set; + import org.apache.ofbiz.base.util.UtilDateTime; import org.apache.ofbiz.base.util.UtilMisc; import org.apache.ofbiz.base.util.UtilProperties; @@ -32,7 +33,7 @@ public class BirtMasterReportServices { public static final String resource = "BirtUiLabels";public static final String resource_error = "BirtErrorUiLabels";- // The following funtion are flexible service as example for reporting + // The following methods are flexible service as example for reportingpublic static Map<String, Object> workEffortPerPersonPrepareDate(DispatchContext dctx, Map<String, Object> context) {Map<String, String> dataMap = UtilMisc.toMap("lastName","name", "firstName", "name", "hours", "floating-point", "fromDate", "date-time", "thruDate", "date-time"); LinkedHashMap<String, String> filterMap = new LinkedHashMap<String, String>(); @@ -57,7 +58,7 @@ public class BirtMasterReportServices {public static Map<String, Object> workEffortPerPerson(DispatchContext dctx, Map<String, Object> context) { Delegator delegator = (Delegator) dctx.getDelegator(); IReportContext reportContext = (IReportContext) context.get("reportContext"); - Map<String, Object> parameters = (Map<String, Object>) reportContext.getParameterValue("parameters"); + Map<String, Object> parameters = UtilMisc.<String, Object>toMap(reportContext.getParameterValue("parameters")); List<GenericValue> listWorkEffortTime = null;if (UtilValidate.isEmpty(parameters.get("firstName")) &&UtilValidate.isEmpty(parameters.get("lastName"))) { @@ -146,7 +147,7 @@ public class BirtMasterReportServices { Delegator delegator = (Delegator) dctx.getDelegator(); Locale locale = (Locale) context.get("locale"); IReportContext reportContext = (IReportContext) context.get("reportContext"); - Map<String, Object> parameters = (Map<String, Object>) reportContext.getParameterValue("parameters"); + Map<String, Object> parameters = UtilMisc.<String, Object>toMap(reportContext.getParameterValue("parameters")); List<GenericValue> listTurnOver = null; List<Map<String, Object>> listInvoiceEditable = new ArrayList<Map<String, Object>>(); @@ -176,7 +177,7 @@ public class BirtMasterReportServices {if (parameters.get("productCategoryId") instanceofString) { String productCategoryId = (String) parameters.get(" productCategoryId"); productCategoryList.add(productCategoryId); - } else { + } else if (parameters.get("productStoreId") instanceof String) { productCategoryList = (List<String>) parameters.get(" productCategoryId"); } // getting productIds in these categories @@ -200,7 +201,7 @@ public class BirtMasterReportServices {if (parameters.get("productStoreId") instanceofString) { String productStoreId = (String) parameters.get(" productStoreId"); productStoreList.add(productStoreId); - } else { + } else if (parameters.get("productStoreId") instanceof List) { productStoreList = (List<String>) parameters.get(" productStoreId"); } // getting list of invoice Ids linked to these productStore @@ -259,7 +260,7 @@ public class BirtMasterReportServices { // adding missing fields for (GenericValue invoice : listTurnOver) {- Map<String, Object> invoiceEditableTemp = (Map<String,Object>) invoice.clone(); + Map<String, Object> invoiceEditableTemp = UtilMisc.<String, Object>toMap(invoice.clone()); invoiceEditableTemp.remove("GenericEntity"); Map<String, Object> invoiceEditable = new HashMap<String, Object>(); invoiceEditable.putAll(invoiceEditableTemp); Modified: ofbiz/ofbiz-plugins/trunk/birt/src/main/java/org/apache/ ofbiz/birt/flexible/BirtServices.java URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-plugins/trunk/ birt/src/main/java/org/apache/ofbiz/birt/flexible/ BirtServices.java?rev=1788869&r1=1788868&r2=1788869&view=diff ============================================================ ================== --- ofbiz/ofbiz-plugins/trunk/birt/src/main/java/org/apache/ ofbiz/birt/flexible/BirtServices.java (original) +++ ofbiz/ofbiz-plugins/trunk/birt/src/main/java/org/apache/ ofbiz/birt/flexible/BirtServices.java Mon Mar 27 09:54:18 2017 @@ -163,7 +163,7 @@ public class BirtServices { Locale locale = (Locale) context.get("locale"); GenericValue userLogin = (GenericValue) context.get("userLogin"); String entityViewName = (String) reportContext. getParameterValue("modelElementName"); - Map<String, Object> inputFields = (Map<String, Object>) reportContext.getParameterValue("parameters"); + Map<String, Object> inputFields = UtilMisc.<String, Object>toMap(reportContext.getParameterValue("parameters"));Map<String, Object> resultPerformFind = new HashMap<String,Object>(); Map<String, Object> resultToBirt = null; List<GenericValue> list = null; @@ -337,18 +337,18 @@ public class BirtServices {if (ServiceUtil.isError(resultMapsForGeneration)) {return ServiceUtil.returnError( ServiceUtil.getErrorMessage(resultMapsForGeneration)); } - Map<String, String> dataMap = (Map<String, String>) resultMapsForGeneration.get("dataMap"); + Map<String, String> dataMap = UtilMisc.<String, String>toMap( resultMapsForGeneration.get("dataMap")); Map<String, String> fieldDisplayLabels = null; if (UtilValidate.isNotEmpty(resul tMapsForGeneration.get("fieldDisplayLabels"))) { - fieldDisplayLabels = (Map<String, String>) resultMapsForGeneration.get("fieldDisplayLabels");+ fieldDisplayLabels = UtilMisc.<String, String>toMap(resultMapsForGeneration.get("fieldDisplayLabels")); } Map<String, String> filterMap = null; if (UtilValidate.isNotEmpty(resul tMapsForGeneration.get("filterMap"))) { - filterMap = (Map<String, String>) resultMapsForGeneration.get("filterMap"); + filterMap = UtilMisc.<String, String>toMap( resultMapsForGeneration.get("filterMap")); } Map<String, String> filterDisplayLabels = null; if (UtilValidate.isNotEmpty(resul tMapsForGeneration.get("filterDisplayLabels"))) { - filterDisplayLabels = (Map<String, String>) resultMapsForGeneration.get("filterDisplayLabels");+ filterDisplayLabels = UtilMisc.<String, String>toMap(resultMapsForGeneration.get("filterDisplayLabels")); }contentId = BirtWorker.recordReportContent(delegator,dispatcher, context); // callPerformFindFromBirt is the customMethod for Entity workflow @@ -410,6 +410,7 @@ public class BirtServices {serviceName = customMethodName + "PrepareFields";} try { + @SuppressWarnings("unused")ModelService modelService = dctx.getModelService(serviceName); } catch (GenericServiceException e) {return ServiceUtil.returnError("No service definewith name " + serviceName); //TODO labelise @@ -417,10 +418,10 @@ public class BirtServices {contentId = BirtWorker.recordReportContent(delegator,dispatcher, context); String rptDesignFileName = BirtUtil. resolveRptDesignFilePathFromContent(delegator, contentId); Map<String, Object> resultService = dispatcher.runSync(serviceName, UtilMisc.toMap("locale", locale, "userLogin", userLogin)); - Map<String, String> dataMap = (Map<String, String>) resultService.get("dataMap"); - Map<String, String> filterMap = (Map<String, String>) resultService.get("filterMap"); - Map<String, String> fieldDisplayLabels = (Map<String, String>) resultService.get("fieldDisplayLabels");- Map<String, String> filterDisplayLabels = (Map<String,String>) resultService.get("filterDisplayLabels"); + Map<String, String> dataMap = UtilMisc.<String, String>toMap(resultService.get("dataMap")); + Map<String, String> filterMap = UtilMisc.<String, String>toMap(resultService.get("filterMap"));+ Map<String, String> fieldDisplayLabels = UtilMisc.<String,String>toMap(resultService.get("fieldDisplayLabels")); + Map<String, String> filterDisplayLabels = UtilMisc.<String, String>toMap(resultService.get("filterDisplayLabels")); Map<String, Object> resultGeneration = dispatcher.runSync("createFlexibleReport", UtilMisc.toMap( "locale", locale, "dataMap", dataMap, @@ -685,6 +686,7 @@ public class BirtServices { if (UtilValidate.isNotEmpty(designStored.getBody())) { SlotHandle bodyStored = designStored.getBody(); + @SuppressWarnings("unchecked") Iterator<DesignElementHandle> iter = bodyStored.iterator(); while (iter.hasNext()) { try { @@ -718,6 +720,7 @@ public class BirtServices { //copy cube SlotHandle cubesFromUser = designFromUser.getCubes(); + @SuppressWarnings("unchecked") Iterator<DesignElementHandle> iterCube = cubesFromUser.iterator(); while (iterCube.hasNext()) { @@ -733,6 +736,7 @@ public class BirtServices { // copy body SlotHandle bodyFromUser = designFromUser.getBody(); + @SuppressWarnings("unchecked") Iterator<DesignElementHandle> iter = bodyFromUser.iterator(); while (iter.hasNext()) { @@ -748,6 +752,7 @@ public class BirtServices { // deleting simple master page from design stored try { + @SuppressWarnings("unchecked") List<DesignElementHandle> listMasterPagesStored = designStored.getMasterPages().getContents(); for (Object masterPage : listMasterPagesStored) {if (masterPage instanceof SimpleMasterPageHandle) {@@ -756,6 +761,7 @@ public class BirtServices { }// adding simple master page => tous ces casts et autres instanceof... c'est laid, mais c'est tellement galère que quand jetrouve une solution qui marche... :s + @SuppressWarnings("unchecked") List<DesignElementHandle> listMasterPages = designFromUser.getMasterPages().getContents();for (DesignElementHandle masterPage : listMasterPages) { if (masterPage instanceof SimpleMasterPageHandle) {@@ -784,12 +790,14 @@ public class BirtServices { // getting style names from stored report List<String> listStyleNames = new ArrayList<String>(); + @SuppressWarnings("unchecked") Iterator<DesignElementHandle> iterStored = stylesStored.iterator(); while (iterStored.hasNext()) { DesignElementHandle item = (DesignElementHandle) iterStored.next(); listStyleNames.add(item.getName()); } + @SuppressWarnings("unchecked") Iterator<DesignElementHandle> iterUser = stylesFromUser.iterator();// adding to styles those which are not already presentModified: ofbiz/ofbiz-plugins/trunk/birt/src/main/java/org/apache/ ofbiz/birt/flexible/BirtUtil.java URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-plugins/trunk/ birt/src/main/java/org/apache/ofbiz/birt/flexible/BirtUtil. java?rev=1788869&r1=1788868&r2=1788869&view=diff ============================================================ ================== --- ofbiz/ofbiz-plugins/trunk/birt/src/main/java/org/apache/ ofbiz/birt/flexible/BirtUtil.java (original) +++ ofbiz/ofbiz-plugins/trunk/birt/src/main/java/org/apache/ ofbiz/birt/flexible/BirtUtil.java Mon Mar 27 09:54:18 2017 @@ -18,25 +18,15 @@ ************************************************************ *******************/ package org.apache.ofbiz.birt.flexible; -import java.io.OutputStream; -import java.io.StringWriter; -import java.sql.SQLException; import java.util.List; -import java.util.Locale; import java.util.Map; -import javax.servlet.ServletContext; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; -import javax.servlet.http.HttpSession; + import org.apache.commons.collections4.MapUtils; -import org.apache.ofbiz.base.util.Debug; import org.apache.ofbiz.base.util.GeneralException; import org.apache.ofbiz.base.util.StringUtil; -import org.apache.ofbiz.base.util.UtilGenerics; import org.apache.ofbiz.base.util.UtilMisc; import org.apache.ofbiz.base.util.UtilProperties; import org.apache.ofbiz.base.util.UtilValidate; -import org.apache.ofbiz.base.util.template.FreeMarkerWorker; import org.apache.ofbiz.entity.Delegator; import org.apache.ofbiz.entity.GenericEntityException; import org.apache.ofbiz.entity.GenericValue; @@ -44,19 +34,7 @@ import org.apache.ofbiz.entity.condition import org.apache.ofbiz.entity.condition.EntityConditionList; import org.apache.ofbiz.entity.condition.EntityExpr; import org.apache.ofbiz.entity.util.EntityQuery; -import org.apache.ofbiz.entity.util.EntityUtil; -import org.apache.ofbiz.security.Security; -import org.apache.ofbiz.service.GenericServiceException; -import org.apache.ofbiz.service.LocalDispatcher; -import org.eclipse.birt.report.engine.api.EXCELRenderOption; -import org.eclipse.birt.report.engine.api.EngineException; -import org.eclipse.birt.report.engine.api.HTMLRenderOption;import org.eclipse.birt.report.engine.api.HTMLServerImageHandler;-import org.eclipse.birt.report.engine.api.IPDFRenderOption; -import org.eclipse.birt.report.engine.api.IReportEngine; -import org.eclipse.birt.report.engine.api.IReportRunnable; -import org.eclipse.birt.report.engine.api.IRunAndRenderTask; -import org.eclipse.birt.report.engine.api.PDFRenderOption; import org.eclipse.birt.report.engine.api.RenderOption;import org.eclipse.birt.report.model.api.elements.DesignChoiceConstants; @@ -64,6 +42,7 @@ public final class BirtUtil {public final static String module = BirtUtil.class.getName();+ @SuppressWarnings("unused")private final static HTMLServerImageHandler imageHandler = newHTMLServerImageHandler(); private final static Map<String, String> entityFieldTypeBirtTypeMap = MapUtils.unmodifiableMap(UtilMisc.toMap("id", DesignChoiceConstants.COLUMN_DATA_TYPE_STRING,@@ -148,8 +127,7 @@ public final class BirtUtil { /*** Return birt field type corresponding to given entity fieldtype * @param entityFieldType - * @return - * @throws GeneralException+ * @return birt field type corresponding to given entity fieldtype */ public static String convertFieldTypeToBirtType(String entityFieldType) { if (UtilValidate.isEmpty(entityFieldType)) { @@ -159,10 +137,9 @@ public final class BirtUtil { } /**- * Return birt parameter type corresponding to given entity fieldtype+ * Return birt parameter type corresponding to given entity fieldtype * @param entityFieldType - * @return - * @throws GeneralException+ * @return birt parameter type corresponding to given entity fieldtype */public static String convertFieldTypeToBirtParameterType(StringentityFieldType) { if (UtilValidate.isEmpty(entityFieldType)) { @@ -174,8 +151,7 @@ public final class BirtUtil { /** * Return true if mime type related to a contentType is supported by Birt * @param contentType - * @return - * @throws GeneralException+ * @return true if mime type related to a contentType is supportedby Birt */public static boolean isSupportedMimeType(String contentType) { return mimeTypeOutputFormatMap.containsKey(contentType);@@ -184,7 +160,7 @@ public final class BirtUtil { /*** Return mime type related to a contentType supported by Birt* @param contentType - * @return+ * @return mime type related to a contentType supported by Birt* @throws GeneralException */public static String getMimeTypeOutputFormat(String contentType)throws GeneralException { @@ -195,9 +171,8 @@ public final class BirtUtil { } /**- * return extension file related to a contentType supported byBirt * @param contentType - * @return+ * return extension file related to a contentType supported byBirt * @throws GeneralException */ public static String getMimeTypeFileExtension(String contentType) throws GeneralException { @@ -210,7 +185,7 @@ public final class BirtUtil {* second from content.properties content.upload.path.prefix* and add birtReptDesign directory * default OFBIZ_HOME/runtime/uploads/birtRptDesign/ - * @return+ * @return template path location where rptDesign file is stored*/ public static String resolveTemplatePathLocation() {String templatePathLocation = UtilProperties.getPropertyValue("birt", "rptDesign.output.path"); @@ -231,7 +206,7 @@ public final class BirtUtil {* With the reporting contentId element resolve the path torptDesign linked * @param delegator * @param contentId - * @return + * @return path to rptDesign file * @throws GenericEntityException */ public static String resolveRptDesignFilePathFromCo ntent(Delegator delegator, String contentId) throws GenericEntityException { @@ -253,7 +228,7 @@ public final class BirtUtil { /*** remove all non unicode alphanumeric and replace space by _* @param reportName - * @return + * @return spaces replaced by underscore */ public static String encodeReportName(String reportName) { if (UtilValidate.isEmpty(reportName)) return ""; Modified: ofbiz/ofbiz-plugins/trunk/birt/src/main/java/org/apache/ ofbiz/birt/flexible/ReportDesignGenerator.java URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-plugins/trunk/ birt/src/main/java/org/apache/ofbiz/birt/flexible/ReportDesignGenerator.java?rev=1788869&r1=1788868&r2=1788869&view=diff============================================================ ================== --- ofbiz/ofbiz-plugins/trunk/birt/src/main/java/org/apache/ ofbiz/birt/flexible/ReportDesignGenerator.java (original) +++ ofbiz/ofbiz-plugins/trunk/birt/src/main/java/org/apache/ ofbiz/birt/flexible/ReportDesignGenerator.java Mon Mar 27 09:54:18 2017 @@ -1,10 +1,10 @@ package org.apache.ofbiz.birt.flexible; -import com.ibm.icu.util.ULocale; import java.io.IOException; import java.util.LinkedHashMap; import java.util.Locale; import java.util.Map; + import org.apache.ofbiz.base.util.Debug; import org.apache.ofbiz.base.util.GeneralException; import org.apache.ofbiz.base.util.UtilProperties; @@ -40,6 +40,8 @@ import org.eclipse.birt.report.model.api import org.eclipse.birt.report.model.api.elements.structures. ResultSetColumn; import org.eclipse.birt.report.model.elements.ReportItem; +import com.ibm.icu.util.ULocale; + /*** Report Design Generator Object - Handles flexible report designGeneration from Master. */ @@ -60,10 +62,12 @@ public class ReportDesignGenerator { private Map<String, String> filterDisplayLabels; private String rptDesignName; private boolean generateFilters = false; + @SuppressWarnings("unused") private GenericValue userLogin;public static final String resource_error = "BirtErrorUiLabels";+ @SuppressWarnings("unchecked") public ReportDesignGenerator(Map<String, Object> context, DispatchContext dctx) throws GeneralException, SemanticException { locale = (Locale) context.get("locale");dataMap = (Map<String, String>) context.get("dataMap");
smime.p7s
Description: S/MIME Cryptographic Signature
