Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

REPORT-848: added try catch statements to catch NullPointerException #170

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion api/src/main/resources/messages.properties
Original file line number Diff line number Diff line change
Expand Up @@ -765,4 +765,7 @@ reporting.createNewLogicDataSet=Create New Logic DataSet
reporting.previewLimitedToRandomCohort=Results have been limited to a random cohort of {0} patients
reporting.affectedReportDefinitions.label=Affected Reports Definitions
reporting.loadAffectedReportDefinitions.label=Load Affected Report Definitions
reporting.noAffectedReportDefs.label=No Affected Report Defifnitions!
reporting.noAffectedReportDefs.label=No Affected Report Defifnitions!

reporting.nullPropertyValues.error=Empty Property values Entered!
reporting.nullRnderType.error=No render Type Selected!
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@
import java.util.Set;

import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpSession;

import org.apache.commons.lang.StringUtils;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.hibernate.PropertyValueException;
import org.openmrs.api.context.Context;
import org.openmrs.module.htmlwidgets.web.handler.WidgetHandler;
import org.openmrs.module.reporting.report.ReportDesign;
Expand Down Expand Up @@ -51,7 +53,7 @@ public ReportDesignFormController() { }
*/
@RequestMapping("/module/reporting/reports/saveReportDesign")
@SuppressWarnings("unchecked")
public String saveReportDesign(ModelMap model, HttpServletRequest request,
public String saveReportDesign(ModelMap model, HttpServletRequest request, HttpSession sesion,
@RequestParam(required=false, value="uuid") String uuid,
@RequestParam(required=true, value="name") String name,
@RequestParam(required=false, value="description") String description,
Expand All @@ -62,7 +64,7 @@ public String saveReportDesign(ModelMap model, HttpServletRequest request,
) {

ReportService rs = Context.getService(ReportService.class);

try {
ReportDesign design = null;
if (StringUtils.isNotEmpty(uuid)) {
design = rs.getReportDesignByUuid(uuid);
Expand Down Expand Up @@ -128,6 +130,10 @@ else if (successUrl.startsWith(pathToRemove)) {
}
design = rs.saveReportDesign(design);
return "redirect:" + successUrl;
}catch(PropertyValueException e) {
sesion.setAttribute(WebConstants.OPENMRS_ERROR_ATTR,"reporting.nullPropertyValues.error");
return "module/reporting/reports/renderers/defaultReportDesignEditor";
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,16 @@
import org.openmrs.module.reporting.report.service.ReportService;
import org.openmrs.module.reporting.web.controller.mapping.renderers.RendererMappingHandler;
import org.openmrs.util.HandlerUtil;
import org.openmrs.web.WebConstants;
import org.springframework.stereotype.Controller;
import org.springframework.ui.ModelMap;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestParam;

import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import javax.servlet.http.HttpSession;

import java.io.IOException;
import java.util.ArrayList;
import java.util.LinkedHashMap;
Expand Down Expand Up @@ -103,12 +106,12 @@ public ModelMap manageReportDesigns(ModelMap model,
* to edit a reportDefinition based on its rendererType.
*/
@RequestMapping("/module/reporting/reports/renderers/editReportDesign")
public String editReportDesign(ModelMap model,
public String editReportDesign(ModelMap model, HttpSession sesion,
@RequestParam(required=true, value="type") Class<? extends ReportRenderer> type,
@RequestParam(required=false, value="reportDesignUuid") String reportDesignUuid,
@RequestParam(required=false, value="reportDefinitionUuid") String reportDefinitionUuid,
@RequestParam(required=false, value="returnUrl") String returnUrl) {

try {
if ( !ObjectUtil.isNull(type) ) {
try {
RendererMappingHandler handler = HandlerUtil.getPreferredHandler(RendererMappingHandler.class, type);
Expand Down Expand Up @@ -137,6 +140,10 @@ public String editReportDesign(ModelMap model,
url.append("&returnUrl=" + returnUrl);
}
return "redirect:"+url.toString();
}catch(NullPointerException e){
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it was only here , where an empty parameter passed to the controller throws a NPE

sesion.setAttribute(WebConstants.OPENMRS_ERROR_ATTR,"reporting.nullRnderType.error");
return "module/reporting/reports/manageReportDesigns";
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,15 @@
package org.openmrs.module.reporting.web.reports.renderers;

import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpSession;

import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Properties;

import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.hibernate.PropertyValueException;
import org.apache.commons.lang.StringUtils;

import org.springframework.stereotype.Controller;
Expand Down Expand Up @@ -96,7 +99,7 @@ else if (successUrl.startsWith(pathToRemove)) {
* @throws InstantiationException
*/
@RequestMapping("/module/reporting/reports/renderers/saveDelimitedTextReportDesign")
public String saveDelimitedTextReportDesign(ModelMap model, HttpServletRequest request,
public String saveDelimitedTextReportDesign(ModelMap model, HttpServletRequest request, HttpSession sesion,
@RequestParam(required=false, value="uuid") String uuid,
@RequestParam(required=true, value="name") String name,
@RequestParam(required=false, value="description") String description,
Expand All @@ -110,7 +113,7 @@ public String saveDelimitedTextReportDesign(ModelMap model, HttpServletRequest r
ReportService rs = Context.getService(ReportService.class);
ReportDesign design = null;
Properties delimiters = new Properties();

try {
if (StringUtils.isNotEmpty(uuid)) {
design = rs.getReportDesignByUuid(uuid);
}
Expand Down Expand Up @@ -148,5 +151,10 @@ else if (successUrl.startsWith(pathToRemove)) {
}
design = rs.saveReportDesign(design);
return "redirect:" + successUrl;
}catch(PropertyValueException e) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mozzy11 , what happens when a NullPointerException is thrown instead?

sesion.setAttribute(WebConstants.OPENMRS_ERROR_ATTR,"reporting.nullPropertyValues.error");
return "module/reporting/reports/renderers/delimitedTextReportRenderer";

}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,11 @@
import java.util.Set;

import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpSession;

import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.hibernate.PropertyValueException;
import org.apache.commons.lang.StringUtils;

import org.openmrs.module.reporting.report.renderer.XlsReportRenderer;
Expand Down Expand Up @@ -111,7 +113,7 @@ else if (successUrl.startsWith(pathToRemove)) {
*/
@SuppressWarnings("unchecked")
@RequestMapping("/module/reporting/reports/renderers/saveExcelReportRenderer")
public String saveExcelReportRenderer(ModelMap model, HttpServletRequest request,
public String saveExcelReportRenderer(ModelMap model, HttpServletRequest request, HttpSession sesion,
@RequestParam(required=false, value="uuid") String uuid,
@RequestParam(required=true, value="name") String name,
@RequestParam(required=false, value="description") String description,
Expand All @@ -126,7 +128,7 @@ public String saveExcelReportRenderer(ModelMap model, HttpServletRequest request
) throws ClassNotFoundException, InstantiationException, IllegalAccessException {
ReportService rs = Context.getService(ReportService.class);
ReportDesign design = null;

try {
if (StringUtils.isNotEmpty(uuid)) {
design = rs.getReportDesignByUuid(uuid);
}
Expand Down Expand Up @@ -213,5 +215,9 @@ else if (successUrl.startsWith(pathToRemove)) {
}
design = rs.saveReportDesign(design);
return "redirect:" + successUrl;
}catch(PropertyValueException e) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same applies here and anywhere else in this PR

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did several testing of that , and instead of a Null Pointer Exception being throen when an empty parameter is passed to the controller, its a PropertyValueEception that is thrown . i think the NPE is already manged by the util methods used there in that specific controller where i was catching the propertValueException

sesion.setAttribute(WebConstants.OPENMRS_ERROR_ATTR,"reporting.nullPropertyValues.error");
return "module/reporting/reports/renderers/excelReportRenderer";
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,11 @@
package org.openmrs.module.reporting.web.reports.renderers;

import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpSession;

import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.hibernate.PropertyValueException;
import org.apache.commons.lang.StringUtils;

import org.springframework.stereotype.Controller;
Expand Down Expand Up @@ -79,7 +81,7 @@ else if (successUrl.startsWith(pathToRemove)) {
* Saves report design
*/
@RequestMapping("/module/reporting/reports/renderers/saveNonConfigurableReportRenderer")
public String saveNonConfigurableReportRenderer(ModelMap model, HttpServletRequest request,
public String saveNonConfigurableReportRenderer(ModelMap model, HttpServletRequest request,HttpSession sesion,
@RequestParam(required=false, value="uuid") String uuid,
@RequestParam(required=true, value="name") String name,
@RequestParam(required=false, value="description") String description,
Expand All @@ -89,7 +91,7 @@ public String saveNonConfigurableReportRenderer(ModelMap model, HttpServletReque
){
ReportService rs = Context.getService(ReportService.class);
ReportDesign design = null;

try {
if (StringUtils.isNotEmpty(uuid)) {
design = rs.getReportDesignByUuid(uuid);
}
Expand All @@ -112,5 +114,9 @@ else if (successUrl.startsWith(pathToRemove)) {
}
design = rs.saveReportDesign(design);
return "redirect:" + successUrl;
}catch(PropertyValueException e){
sesion.setAttribute(WebConstants.OPENMRS_ERROR_ATTR,"reporting.nullPropertyValues.error");
return "module/reporting/reports/renderers/nonConfigurableReportRenderer";
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
package org.openmrs.module.reporting.web.reports.renderers;

import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpSession;

import java.io.ByteArrayOutputStream;
import java.io.UnsupportedEncodingException;
Expand All @@ -24,6 +25,7 @@

import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.hibernate.PropertyValueException;
import org.apache.commons.lang.StringUtils;
import org.springframework.stereotype.Controller;
import org.springframework.ui.ModelMap;
Expand Down Expand Up @@ -125,7 +127,7 @@ else if (successUrl.startsWith(pathToRemove)) {
* @throws UnsupportedEncodingException
*/
@RequestMapping("/module/reporting/reports/renderers/saveTextTemplateReportRendererDesign")
public String saveTextTemplateReportRendererDesign(ModelMap model, HttpServletRequest request,
public String saveTextTemplateReportRendererDesign(ModelMap model, HttpServletRequest request, HttpSession sesion,
@RequestParam(required=false, value="uuid") String uuid,
@RequestParam(required=true, value="name") String name,
@RequestParam(required=false, value="description") String description,
Expand All @@ -138,7 +140,7 @@ public String saveTextTemplateReportRendererDesign(ModelMap model, HttpServletRe
ReportService rs = Context.getService(ReportService.class);
ReportDesign design = null;
ReportDesignResource designResource = new ReportDesignResource();

try {
if (StringUtils.isNotEmpty(uuid)) {
design = rs.getReportDesignByUuid(uuid);
}
Expand Down Expand Up @@ -171,6 +173,10 @@ else if (successUrl.startsWith(pathToRemove)) {

design = rs.saveReportDesign(design);
return "redirect:" + successUrl;
}catch(PropertyValueException e){
sesion.setAttribute(WebConstants.OPENMRS_ERROR_ATTR,"reporting.nullPropertyValues.error");
return "module/reporting/reports/renderers/textTemplateReportRenderer";
}
}

@ModelAttribute( "expSupportedTypes" )
Expand Down