----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31587/#review74705 -----------------------------------------------------------
security-admin/src/main/java/org/apache/ranger/rest/RangerServiceValidator.java <https://reviews.apache.org/r/31587/#comment121372> If validator stores the state of the last validate call (in _failureDetails) i.e. a validator object is expected to be used to validate only one action on a single object, it might be better to take the "service" and action also as a constructor parameters; and support only a generic validation method like validate(). Other option is not to store the state of the validation and have each validateFor*() variation to return the validation details via exception or return value. security-admin/src/main/java/org/apache/ranger/rest/RangerServiceValidator.java <https://reviews.apache.org/r/31587/#comment121373> validateFor*() methods seem to stop at (or report) only the one (or first) failure. It will be useful to find as many issues as possible and return in one call - like 1) name not specified 2) service-def/service/policy/condition do not exist 3) no value provided for required configuration(s) 4) invalid value for configuration(s) security-admin/src/main/java/org/apache/ranger/rest/RangerServiceValidator.java <https://reviews.apache.org/r/31587/#comment121371> Shouldn't the field be "id" here? security-admin/src/main/java/org/apache/ranger/rest/RangerServiceValidator.java <https://reviews.apache.org/r/31587/#comment121374> For return messages, use <== (instead of ==>). Review other such usage as well. - Madhan Neethiraj On March 1, 2015, 5:44 a.m., Alok Lal wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31587/ > ----------------------------------------------------------- > > (Updated March 1, 2015, 5:44 a.m.) > > > Review request for ranger. > > > Bugs: RANGER-278 > https://issues.apache.org/jira/browse/RANGER-278 > > > Repository: ranger > > > Description > ------- > > Validation test for services > > - Junits were added for ServiceREST only for create. Update and delete are > not testable currently without non-trivial refactoring. Hence, there is no > direct test of ServiceREST functionality. Of course, validator methods > called during update and delete have their junits. > > Fixes unrelaed to this bug: > > - Some pom files have hardcoded version in them. Version for child pom > should be inherited from that of parent pom as it the case for rest of child > poms. > - ServiceDBStore was missing Apache license boiler plate. > - LdapUserGroupBuilder file had an unused import that was causing a compile > error. > - Fixed maven build warnings due to missing findbugs version and unused/stale > eclipse m2e plugin. > > > Diffs > ----- > > pom.xml b2a955d > ranger-util/pom.xml 047008a > security-admin/pom.xml 6d313b6 > security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java > 0cb1707 > > security-admin/src/main/java/org/apache/ranger/rest/RangerServiceValidator.java > PRE-CREATION > security-admin/src/main/java/org/apache/ranger/rest/RangerValidator.java > PRE-CREATION > > security-admin/src/main/java/org/apache/ranger/rest/RangerValidatorFactory.java > PRE-CREATION > security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java > d4851cc > > security-admin/src/main/java/org/apache/ranger/rest/ValidationFailureDetails.java > PRE-CREATION > > security-admin/src/main/java/org/apache/ranger/rest/ValidationFailureDetailsBuilder.java > PRE-CREATION > > security-admin/src/test/java/org/apache/ranger/rest/TestRangerServiceValidator.java > PRE-CREATION > > security-admin/src/test/java/org/apache/ranger/rest/TestServiceRESTForValidation.java > PRE-CREATION > > security-admin/src/test/java/org/apache/ranger/rest/TestServiceValidator.java > PRE-CREATION > > security-admin/src/test/java/org/apache/ranger/rest/ValidationTestUtils.java > PRE-CREATION > > ugsync/src/main/java/org/apache/ranger/ldapusersync/process/LdapUserGroupBuilder.java > f2fbf02 > > Diff: https://reviews.apache.org/r/31587/diff/ > > > Testing > ------- > > Added new junits for create/update/delete validation of services > Manual excercise of REST api to validate response and error messages(s). > > > Thanks, > > Alok Lal > >
