Hi all, Review notes and discussed changes/ improvements in Patch review - part 2 are illustrated below
*APIMANAGER-2565* - Add debug logs instead of existing info logs - Handle null workflow references *APIMANAGER-2649* - Remove generic exception handling *APPM-206* - In APPManagerConfigurationServiceComponent, append file path into error and debug logs when reading the config file (Make error and debug logs more context aware) *APPM-98* - Remove generic exceptions - Add error logs to indicate the failure update of SSO provider *APPM-198* - Get rid of the debug logs and add if debug *APPM-202* - Enhance deleteApp() functionality by code reuse *APPM-196* - Imporvement : change AbstractRequest to Request *APPM-166* - Fix license issues - Remove connection rollbacks in ApiMgtDAO database SELECT queries - Enhance NumberFormatException handling in APIProviderHostObject - In some places in XacmlEntitlementServiceImpl and ApiProviderHostObejct, generic exceptions caught and just logged, thus enhance exception handling. On Thu, Dec 18, 2014 at 10:12 AM, Ruwan Yatawara <[email protected]> wrote: > > General > > - > > Make the log parameters in debug logs, context aware and releavent. > - > > Add null checks in all host object methods added. > - > > Add description for parameters, in code comments. > - > > Remove all log.info from jaggery code > - > > Get rid of unused imports > - > > Use standard variable names for parameters like synapse context > > > ProviderHostObject > > - > > change method name from getAssociatedAppsName to getAssociatedAppNames > > > > - > > in iswebappExist method, string comparison need to be done against the > constant, not the other way around. > > > > - > > saveEntitlementPolicyPartials / saveEntitlementPolicyPartials > - > > Add null checks to arguments recieved from native array as it will > be problematic when these functions are used as APIs > - > > Fix length to 120 > > > > AppMgtDao > > - > > SaveEntitlementPolicyPartials / UpdateEntitlementPolicyPartials / > DeleteEntitlementPolicyPartials > - > > Use Integer.parseInt instead of complex value of statement, then > throw a parseException > - > > Throw exception E1 instead of E in the catch block > > > > - > > getSharedEntitlementPolicyPartialsList > - > > Get rid of connection.commit as it is a select query that is > executed > > > > - > > Check if validationResult function is really needed > - addApplication method, replace == with equals > > > > Entitlement JS > > - > > In update scenario, include a comment for the section when building > associated app list > - > > When getting the policy partials, get the policy partials per app in > single db call from db instead compiling a list in UI > > > > APPM-222 > > - > > change method name getAPISubscriptionCountByApp to > getSubscriptionCountByApp > - > > define return parameter in the java doc comment and list what is > returned in the map > - > > The loop to populate subscriptions does not seem to run when the sql > is executed so get rid of that section after checking > - > > Get rid of the UI specific formatting logic executed at the backend > (Adding spaces and formatting) and handle said logic in the UI. > - > > getAllWebapps method instantiate all the variables before starting the > while loop > - > > In the DB query get rid of CURRENT_TIMESTAMP as it not a standard ANSI > sql > > > > APPM-247 > > - > > api.getPolicyPartiallist() is not a list so change the variable name > to something else. > > > > APPM-167 > > - > > addURITemplate : get rid of the byte i =0 section and the logic to > increment same add regular, create the prepared statement by inserting the > values as 1,2,3 > - > > AppID you do not have to cast to string again > - > > In the EntitlemntPolicy Class get rid of the generic exceptions and > Add all the java doc comments > > > AppMgtDao > > - > > For the delete app method, get rid of the call made from DAO layer to > Impl, introduce a hostobject method and call the impl instead > > > > APPM-206 > > - > > AppManagerConfigurationServiceComponent, introduce isDebug enabled > blocks > - > > Add useful error messages. > > > > > > Thanks and Regards, > > Ruwan Yatawara > > Senior Software Engineer, > WSO2 Inc. > > email : [email protected] > mobile : +94 77 9110413 > blog : http://thoughts.ruwan-ace.com/ > www: :http://wso2.com > > -- Thilini Shanika Software Engineer WSO2, Inc.; http://wso2.com 20, Palmgrove Avenue, Colombo 3 E-mail: [email protected]
_______________________________________________ Dev mailing list [email protected] http://wso2.org/cgi-bin/mailman/listinfo/dev
