You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ranger.apache.org by ma...@apache.org on 2015/08/11 00:35:53 UTC
[02/14] incubator-ranger git commit: RANGER-584 User friendly error
messages for service validation error failures
RANGER-584 User friendly error messages for service validation error failures
Project: http://git-wip-us.apache.org/repos/asf/incubator-ranger/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-ranger/commit/d1e2f3b7
Tree: http://git-wip-us.apache.org/repos/asf/incubator-ranger/tree/d1e2f3b7
Diff: http://git-wip-us.apache.org/repos/asf/incubator-ranger/diff/d1e2f3b7
Branch: refs/heads/tag-policy
Commit: d1e2f3b7e14a803ad4d9f117cf8f483e51c25d55
Parents: e22fbb1
Author: Alok Lal <al...@hortonworks.com>
Authored: Thu Jul 30 15:33:42 2015 -0700
Committer: Alok Lal <al...@hortonworks.com>
Committed: Fri Jul 31 13:39:40 2015 -0700
----------------------------------------------------------------------
.../plugin/errors/ValidationErrorCode.java | 76 +++++++++
.../validation/RangerServiceValidator.java | 159 ++++++++++++-------
.../plugin/errors/TestValidationErrorCode.java | 72 +++++++++
.../TestValidationFailureDetails.java | 3 -
4 files changed, 248 insertions(+), 62 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/d1e2f3b7/agents-common/src/main/java/org/apache/ranger/plugin/errors/ValidationErrorCode.java
----------------------------------------------------------------------
diff --git a/agents-common/src/main/java/org/apache/ranger/plugin/errors/ValidationErrorCode.java b/agents-common/src/main/java/org/apache/ranger/plugin/errors/ValidationErrorCode.java
new file mode 100644
index 0000000..77d16f5
--- /dev/null
+++ b/agents-common/src/main/java/org/apache/ranger/plugin/errors/ValidationErrorCode.java
@@ -0,0 +1,76 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.ranger.plugin.errors;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+
+import java.text.MessageFormat;
+import java.util.Arrays;
+
+public enum ValidationErrorCode {
+
+ SERVICE_VALIDATION_ERR_UNSUPPORTED_ACTION(1001, "Internal error: unsupported action[{0}]; isValid(Long) is only supported for DELETE"),
+ SERVICE_VALIDATION_ERR_MISSING_FIELD(1002, "Internal error: missing field[{0}]"),
+ SERVICE_VALIDATION_ERR_NULL_SERVICE_OBJECT(1003, "Internal error: service object passed in was null"),
+ SERVICE_VALIDATION_ERR_EMPTY_SERVICE_ID(1004, "Internal error: service id was null/empty/blank"),
+ SERVICE_VALIDATION_ERR_INVALID_SERVICE_ID(1005, "No service found for id [{0}]"),
+ SERVICE_VALIDATION_ERR_INVALID_SERVICE_NAME(1006, "Service name[{0}] was null/empty/blank"),
+ SERVICE_VALIDATION_ERR_SERVICE_NAME_CONFICT(1007, "service with the name[{0}] already exists"),
+ SERVICE_VALIDATION_ERR_ID_NAME_CONFLICT(1008, "id/name conflict: another service already exists with name[{0}], its id is [{1}]"),
+ SERVICE_VALIDATION_ERR_MISSING_SERVICE_DEF(1009, "service def [{0}] was null/empty/blank"),
+ SERVICE_VALIDATION_ERR_INVALID_SERVICE_DEF(1010, "service def named[{0}] not found"),
+ SERVICE_VALIDATION_ERR_REQUIRED_PARM_MISSING(1011, "required configuration parameter is missing; missing parameters: {0}"),
+ ;
+
+
+ private static final Log LOG = LogFactory.getLog(ValidationErrorCode.class);
+
+ final int _errorCode;
+ final String _template;
+
+ ValidationErrorCode(int errorCode, String template) {
+ _errorCode = errorCode;
+ _template = template;
+ }
+
+ public String getMessage(Object... items) {
+ if (LOG.isDebugEnabled()) {
+ LOG.debug(String.format("<== ValidationErrorCode.getMessage(%s)", Arrays.toString(items)));
+ }
+
+ MessageFormat mf = new MessageFormat(_template);
+ String result = mf.format(items);
+
+ if (LOG.isDebugEnabled()) {
+ LOG.debug(String.format("<== ValidationErrorCode.getMessage(%s): %s", Arrays.toString(items), result));
+ }
+ return result;
+ }
+
+ public int getErrorCode() {
+ return _errorCode;
+ }
+
+ @Override
+ public String toString() {
+ return String.format("Code: %d, template: %s", _errorCode, _template);
+ }
+}
http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/d1e2f3b7/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceValidator.java
----------------------------------------------------------------------
diff --git a/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceValidator.java b/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceValidator.java
index 615e385..3cfaa3e 100644
--- a/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceValidator.java
+++ b/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceValidator.java
@@ -26,6 +26,7 @@ import java.util.Set;
import org.apache.commons.lang.StringUtils;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
+import org.apache.ranger.plugin.errors.ValidationErrorCode;
import org.apache.ranger.plugin.model.RangerService;
import org.apache.ranger.plugin.model.RangerServiceDef;
import org.apache.ranger.plugin.store.ServiceStore;
@@ -67,16 +68,21 @@ public class RangerServiceValidator extends RangerValidator {
boolean valid = true;
if (action != Action.DELETE) {
- failures.add(new ValidationFailureDetailsBuilder()
- .isAnInternalError()
- .becauseOf("unsupported action[" + action + "]; isValid(Long) is only supported for DELETE")
- .build());
+ ValidationErrorCode error = ValidationErrorCode.SERVICE_VALIDATION_ERR_UNSUPPORTED_ACTION;
+ failures.add(new RangerServiceValidationErrorBuilder()
+ .isAnInternalError()
+ .errorCode(error.getErrorCode())
+ .becauseOf(error.getMessage(action))
+ .build());
valid = false;
} else if (id == null) {
- failures.add(new ValidationFailureDetailsBuilder()
- .field("id")
- .isMissing()
- .build());
+ ValidationErrorCode error = ValidationErrorCode.SERVICE_VALIDATION_ERR_MISSING_FIELD;
+ failures.add(new RangerServiceValidationErrorBuilder()
+ .field("id")
+ .isMissing()
+ .errorCode(error.getErrorCode())
+ .becauseOf(error.getMessage(id))
+ .build());
valid = false;
} else if (getService(id) == null) {
if (LOG.isDebugEnabled()) {
@@ -100,32 +106,34 @@ public class RangerServiceValidator extends RangerValidator {
boolean valid = true;
if (service == null) {
- String message = "service object passed in was null";
- LOG.debug(message);
- failures.add(new ValidationFailureDetailsBuilder()
- .field("service")
- .isMissing()
- .becauseOf(message)
- .build());
+ ValidationErrorCode error = ValidationErrorCode.SERVICE_VALIDATION_ERR_NULL_SERVICE_OBJECT;
+ failures.add(new RangerServiceValidationErrorBuilder()
+ .field("service")
+ .isMissing()
+ .errorCode(error.getErrorCode())
+ .becauseOf(error.getMessage())
+ .build());
valid = false;
} else {
Long id = service.getId();
if (action == Action.UPDATE) { // id is ignored for CREATE
if (id == null) {
- String message = "service id was null/empty/blank";
- LOG.debug(message);
- failures.add(new ValidationFailureDetailsBuilder()
- .field("id")
- .isMissing()
- .becauseOf(message)
- .build());
+ ValidationErrorCode error = ValidationErrorCode.SERVICE_VALIDATION_ERR_EMPTY_SERVICE_ID;
+ failures.add(new RangerServiceValidationErrorBuilder()
+ .field("id")
+ .isMissing()
+ .errorCode(error.getErrorCode())
+ .becauseOf(error.getMessage())
+ .build());
valid = false;
} else if (getService(id) == null) {
- failures.add(new ValidationFailureDetailsBuilder()
- .field("id")
- .isSemanticallyIncorrect()
- .becauseOf("no service exists with id[" + id +"]")
- .build());
+ ValidationErrorCode error = ValidationErrorCode.SERVICE_VALIDATION_ERR_INVALID_SERVICE_ID;
+ failures.add(new RangerServiceValidationErrorBuilder()
+ .field("id")
+ .isSemanticallyIncorrect()
+ .errorCode(error.getErrorCode())
+ .becauseOf(error.getMessage(id))
+ .build());
valid = false;
}
}
@@ -133,48 +141,56 @@ public class RangerServiceValidator extends RangerValidator {
boolean nameSpecified = StringUtils.isNotBlank(name);
RangerServiceDef serviceDef = null;
if (!nameSpecified) {
- String message = "service name[" + name + "] was null/empty/blank";
- LOG.debug(message);
- failures.add(new ValidationFailureDetailsBuilder()
- .field("name")
- .isMissing()
- .becauseOf(message)
- .build());
+ ValidationErrorCode error = ValidationErrorCode.SERVICE_VALIDATION_ERR_INVALID_SERVICE_NAME;
+ failures.add(new RangerServiceValidationErrorBuilder()
+ .field("name")
+ .isMissing()
+ .errorCode(error.getErrorCode())
+ .becauseOf(error.getMessage(name))
+ .build());
valid = false;
} else {
RangerService otherService = getService(name);
if (otherService != null && action == Action.CREATE) {
- failures.add(new ValidationFailureDetailsBuilder()
- .field("name")
- .isSemanticallyIncorrect()
- .becauseOf("service with the name[" + name + "] already exists")
- .build());
+ ValidationErrorCode error = ValidationErrorCode.SERVICE_VALIDATION_ERR_SERVICE_NAME_CONFICT;
+ failures.add(new RangerServiceValidationErrorBuilder()
+ .field("name")
+ .isSemanticallyIncorrect()
+ .errorCode(error.getErrorCode())
+ .becauseOf(error.getMessage(name))
+ .build());
valid = false;
} else if (otherService != null && otherService.getId() !=null && !otherService.getId().equals(id)) {
- failures.add(new ValidationFailureDetailsBuilder()
- .field("id/name")
- .isSemanticallyIncorrect()
- .becauseOf("id/name conflict: another service already exists with name[" + name + "], its id is [" + otherService.getId() + "]")
- .build());
+ ValidationErrorCode error = ValidationErrorCode.SERVICE_VALIDATION_ERR_ID_NAME_CONFLICT;
+ failures.add(new RangerServiceValidationErrorBuilder()
+ .field("id/name")
+ .isSemanticallyIncorrect()
+ .errorCode(error.getErrorCode())
+ .becauseOf(error.getMessage(name, otherService.getId()))
+ .build());
valid = false;
}
}
String type = service.getType();
boolean typeSpecified = StringUtils.isNotBlank(type);
if (!typeSpecified) {
- failures.add(new ValidationFailureDetailsBuilder()
- .field("type")
- .isMissing()
- .becauseOf("service def [" + type + "] was null/empty/blank")
- .build());
+ ValidationErrorCode error = ValidationErrorCode.SERVICE_VALIDATION_ERR_MISSING_SERVICE_DEF;
+ failures.add(new RangerServiceValidationErrorBuilder()
+ .field("type")
+ .isMissing()
+ .errorCode(error.getErrorCode())
+ .becauseOf(error.getMessage(type))
+ .build());
valid = false;
} else {
serviceDef = getServiceDef(type);
if (serviceDef == null) {
- failures.add(new ValidationFailureDetailsBuilder()
- .field("type")
- .isSemanticallyIncorrect()
- .becauseOf("service def named[" + type + "] not found")
+ ValidationErrorCode error = ValidationErrorCode.SERVICE_VALIDATION_ERR_INVALID_SERVICE_DEF;
+ failures.add(new RangerServiceValidationErrorBuilder()
+ .field("type")
+ .isSemanticallyIncorrect()
+ .errorCode(error.getErrorCode())
+ .becauseOf(error.getMessage(type))
.build());
valid = false;
}
@@ -185,12 +201,14 @@ public class RangerServiceValidator extends RangerValidator {
Set<String> inputParameters = getServiceConfigParameters(service);
Set<String> missingParameters = Sets.difference(reqiredParameters, inputParameters);
if (!missingParameters.isEmpty()) {
- failures.add(new ValidationFailureDetailsBuilder()
- .field("configuration")
- .subField(missingParameters.iterator().next()) // we return any one parameter!
- .isMissing()
- .becauseOf("required configuration parameter is missing; missing parameters: " + missingParameters)
- .build());
+ ValidationErrorCode error = ValidationErrorCode.SERVICE_VALIDATION_ERR_REQUIRED_PARM_MISSING;
+ failures.add(new RangerServiceValidationErrorBuilder()
+ .field("configuration")
+ .subField(missingParameters.iterator().next()) // we return any one parameter!
+ .isMissing()
+ .errorCode(error.getErrorCode())
+ .becauseOf(error.getMessage(missingParameters))
+ .build());
valid = false;
}
}
@@ -201,4 +219,27 @@ public class RangerServiceValidator extends RangerValidator {
}
return valid;
}
+
+ static class RangerServiceValidationErrorBuilder extends ValidationFailureDetailsBuilder {
+
+ @Override
+ ValidationFailureDetails build() {
+ return new RangerPolicyValidationFailure(_errorCode, _fieldName, _subFieldName, _missing, _semanticError, _internalError, _reason);
+ }
+ }
+
+ static class RangerPolicyValidationFailure extends ValidationFailureDetails {
+
+ public RangerPolicyValidationFailure(int errorCode, String fieldName, String subFieldName, boolean missing, boolean semanticError, boolean internalError, String reason) {
+ super(errorCode, fieldName, subFieldName, missing, semanticError, internalError, reason);
+ }
+
+ // TODO remove and move to baseclass when all 3 move to new message framework
+ @Override
+ public String toString() {
+ LOG.debug("RangerServiceValidationFailure.toString");
+ return String.format("%s: %d, %s", "Policy validation failure", _errorCode, _reason);
+ }
+ }
+
}
http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/d1e2f3b7/agents-common/src/test/java/org/apache/ranger/plugin/errors/TestValidationErrorCode.java
----------------------------------------------------------------------
diff --git a/agents-common/src/test/java/org/apache/ranger/plugin/errors/TestValidationErrorCode.java b/agents-common/src/test/java/org/apache/ranger/plugin/errors/TestValidationErrorCode.java
new file mode 100644
index 0000000..d6b2d16
--- /dev/null
+++ b/agents-common/src/test/java/org/apache/ranger/plugin/errors/TestValidationErrorCode.java
@@ -0,0 +1,72 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.ranger.plugin.errors;
+
+import com.google.common.collect.ImmutableSet;
+import junit.framework.TestCase;
+
+import java.util.HashSet;
+import java.util.Set;
+
+/**
+ * Created by alal on 7/30/15.
+ */
+public class TestValidationErrorCode extends TestCase {
+
+
+ public void testUserMessage() throws Exception {
+ ValidationErrorCode errorCode = ValidationErrorCode.SERVICE_VALIDATION_ERR_UNSUPPORTED_ACTION;
+ String aParameter = "FOO";
+ String expected = errorCode._template.replace("{0}", aParameter);
+ assertEquals(expected, errorCode.getMessage(aParameter));
+ }
+
+ /**
+ * tests if template has any trivial template variable problems, e.g. if template has {3} in it then it
+ * better also have {0}, {1} and {2} in it else MessageFormat output might be unexpected.
+ *
+ * This check is far from perfect. It may give false alarms if the message itself contains strings of the form {1}
+ * which have been escaped using single quotes. If that happens we would have to make this test smarter.
+ */
+ public void testTemplates() {
+
+ // we check up to 5 substitution variables. If there are more than 5 then you probably have a different set of problems
+ Set<ValidationErrorCode> may = ImmutableSet.copyOf(ValidationErrorCode.values());
+
+ // set of enums that must not hvae any subsequent placeholders in it
+ Set<ValidationErrorCode> mustNot = new HashSet<ValidationErrorCode>();
+
+ for (int i = 0; i < 5; i++) {
+ String token = String.format("{%d", i);
+ // check which ones should not have anymore substition varabile placehoders in them, {0}, {1}, etc.
+ for (ValidationErrorCode anEnum : may) {
+ if (!anEnum._template.contains(token)) {
+ // if template does not have {1} then it surely must not have {2}, {3}, etc.
+ mustNot.add(anEnum);
+ }
+ }
+ // check for incorrectly numbers substition variable placeholders
+ for (ValidationErrorCode anEnum : mustNot) {
+ assertFalse(anEnum.toString() + ": contains " + token + ". Check for wongly numberd substition variable placeholders.",
+ anEnum._template.contains(token));
+ }
+ }
+ }
+}
\ No newline at end of file
http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/d1e2f3b7/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestValidationFailureDetails.java
----------------------------------------------------------------------
diff --git a/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestValidationFailureDetails.java b/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestValidationFailureDetails.java
index cf929c6..815d41c 100644
--- a/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestValidationFailureDetails.java
+++ b/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestValidationFailureDetails.java
@@ -27,9 +27,6 @@ import java.util.regex.Pattern;
import static org.junit.Assert.assertEquals;
-/**
- * Created by alal on 6/17/15.
- */
public class TestValidationFailureDetails {
@Test