From 54d708a8566c56669c9b45b81359065343e9bc45 Mon Sep 17 00:00:00 2001 From: Ramzi Maalej Date: Wed, 30 Mar 2016 14:43:38 -0400 Subject: [PATCH 1/2] RM | Adding test to ValidationErrors and Fixing EmptyStackException --- .gitignore | 2 + .../core/validation/ValidationErrors.java | 22 +++-- .../core/validation/ValidationErrorsTest.java | 87 +++++++++++++++++++ 3 files changed, 104 insertions(+), 7 deletions(-) create mode 100644 src/test/java/org/owasp/esapi/core/validation/ValidationErrorsTest.java diff --git a/.gitignore b/.gitignore index 8b8a078..98913d1 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,6 @@ +# Java Files *.class +target/ # Package Files # *.jar diff --git a/src/main/java/org/owasp/esapi/core/validation/ValidationErrors.java b/src/main/java/org/owasp/esapi/core/validation/ValidationErrors.java index 2e26698..612ddde 100644 --- a/src/main/java/org/owasp/esapi/core/validation/ValidationErrors.java +++ b/src/main/java/org/owasp/esapi/core/validation/ValidationErrors.java @@ -1,12 +1,9 @@ package org.owasp.esapi.core.validation; import java.io.Serializable; -import java.util.List; -import java.util.Map; -import java.util.Stack; +import java.util.*; import java.util.concurrent.ConcurrentHashMap; -@SuppressWarnings("UnusedDeclaration") public class ValidationErrors implements Serializable { private static final long serialVersionUID = 20131112L; @@ -31,12 +28,23 @@ public ValidationError nextError(String context) { if (errorsForContext == null) { return null; } - - return errorsForContext.pop(); + try { + return errorsForContext.pop(); + } catch (EmptyStackException ese) { + return null; + } } public List getErrors(String context) { - return errors.get(context); + List errorListToReturn = new ArrayList(); + + List errorListForContext = errors.get(context); + + if(null != errorListForContext) { + errorListToReturn.addAll(errorListForContext); + } + + return Collections.unmodifiableList(errorListToReturn); } public void clear() { diff --git a/src/test/java/org/owasp/esapi/core/validation/ValidationErrorsTest.java b/src/test/java/org/owasp/esapi/core/validation/ValidationErrorsTest.java new file mode 100644 index 0000000..104b7b3 --- /dev/null +++ b/src/test/java/org/owasp/esapi/core/validation/ValidationErrorsTest.java @@ -0,0 +1,87 @@ +package org.owasp.esapi.core.validation; + +import org.testng.annotations.BeforeTest; +import org.testng.annotations.Test; + +import java.util.List; + +import static org.hamcrest.CoreMatchers.nullValue; +import static org.hamcrest.core.Is.is; +import static org.hamcrest.core.IsNull.notNullValue; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.fail; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +@Test +public class ValidationErrorsTest { + + public static final String REGISTRATION = "registration"; + public static final String INVALID_EMAIL_ADDRESS_ERROR_MSG = "invalid email address"; + public static final String EMAIL = "email"; + private ValidationErrors validationErrors; + + @BeforeTest + public void setup() { + this.validationErrors = new ValidationErrors(); + } + + @Test + public void testAddError() { + ValidationError validationError = mock(ValidationError.class); + when(validationError.getField()).thenReturn(EMAIL); + when(validationError.getMessage()).thenReturn(INVALID_EMAIL_ADDRESS_ERROR_MSG); + + validationErrors.addError(REGISTRATION, validationError); + List errors = validationErrors.getErrors(REGISTRATION); + + assertThat(errors, notNullValue()); + assertThat(errors.size(), is(1)); + assertThat(errors.get(0), is(validationError)); + } + + @Test + public void testReturnedErrorsListIsUnmodifiable() { + ValidationError validationError = mock(ValidationError.class); + when(validationError.getField()).thenReturn(EMAIL); + when(validationError.getMessage()).thenReturn(null); + + validationErrors.addError(REGISTRATION, validationError); + List errors = validationErrors.getErrors(REGISTRATION); + + try { + errors.clear(); + fail("Should throw an exception as this is an unmodifiable list"); + } catch (UnsupportedOperationException e) { + } + } + + @Test + public void testReturnedErrorsListIsEmptyForNonExistingContext() { + List errors = validationErrors.getErrors(REGISTRATION); + + assertThat(errors, notNullValue()); + assertThat(errors.size(), is(0)); + } + + @Test void testReturnNextError() { + ValidationError inputError = mock(ValidationError.class); + validationErrors.addError(REGISTRATION, inputError); + validationErrors.nextError(REGISTRATION); + + ValidationError nextError = validationErrors.nextError(REGISTRATION); + assertThat(nextError, nullValue()); + } + + @Test + public void testClearingErrorList() { + ValidationError validationError = mock(ValidationError.class); + when(validationError.getField()).thenReturn(EMAIL); + when(validationError.getMessage()).thenReturn(INVALID_EMAIL_ADDRESS_ERROR_MSG); + + validationErrors.addError(REGISTRATION, validationError); + validationErrors.clear(); + + assertThat(validationErrors.getErrors(REGISTRATION).size(), is(0)); + } +} From eea63196b1e13348cc22e0d808b31d023732088c Mon Sep 17 00:00:00 2001 From: Ramzi Maalej Date: Wed, 30 Mar 2016 14:44:39 -0400 Subject: [PATCH 2/2] RM | Adding default ValidationError implementation --- .../validation/impl/ValidationErrorImpl.java | 47 +++++++++++++++++++ .../impl/ValidationErrorImplTest.java | 42 +++++++++++++++++ 2 files changed, 89 insertions(+) create mode 100644 src/main/java/org/owasp/esapi/core/validation/impl/ValidationErrorImpl.java create mode 100644 src/test/java/org/owasp/esapi/core/validation/impl/ValidationErrorImplTest.java diff --git a/src/main/java/org/owasp/esapi/core/validation/impl/ValidationErrorImpl.java b/src/main/java/org/owasp/esapi/core/validation/impl/ValidationErrorImpl.java new file mode 100644 index 0000000..0a663e9 --- /dev/null +++ b/src/main/java/org/owasp/esapi/core/validation/impl/ValidationErrorImpl.java @@ -0,0 +1,47 @@ +package org.owasp.esapi.core.validation.impl; + +import org.owasp.esapi.core.validation.ValidationError; + +public final class ValidationErrorImpl implements ValidationError { + protected static final String FIELD_NULL_EMPTY_ERROR = "Field cannot be null or empty"; + + private String field; + private String message; + + public ValidationErrorImpl(String field, String message) { + if(null == field || field.isEmpty()) { + throw new IllegalArgumentException(FIELD_NULL_EMPTY_ERROR); + } + this.field = field; + this.message = message; + } + + @Override + public String getField() { + return field; + } + + @Override + public String getMessage() { + return message; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + + ValidationErrorImpl that = (ValidationErrorImpl) o; + + if (!field.equals(that.field)) return false; + return message.equals(that.message); + + } + + @Override + public int hashCode() { + int result = field.hashCode(); + result = 31 * result + message.hashCode(); + return result; + } +} diff --git a/src/test/java/org/owasp/esapi/core/validation/impl/ValidationErrorImplTest.java b/src/test/java/org/owasp/esapi/core/validation/impl/ValidationErrorImplTest.java new file mode 100644 index 0000000..37d1940 --- /dev/null +++ b/src/test/java/org/owasp/esapi/core/validation/impl/ValidationErrorImplTest.java @@ -0,0 +1,42 @@ +package org.owasp.esapi.core.validation.impl; + +import org.owasp.esapi.core.validation.ValidationError; +import org.testng.annotations.Test; + +import static org.hamcrest.core.IsEqual.equalTo; +import static org.hamcrest.core.IsNull.notNullValue; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.fail; + +@Test +public class ValidationErrorImplTest { + + public static final String FIELD_EMAIL = "email"; + public static final String MSG_EMAIL = "Invalid email address"; + + @Test + public void testCreateValidationErrorSuccessfully() { + ValidationError validationError = new ValidationErrorImpl(FIELD_EMAIL, MSG_EMAIL); + assertThat(validationError, notNullValue()); + assertThat(validationError.getField(), equalTo(FIELD_EMAIL)); + assertThat(validationError.getMessage(), equalTo(MSG_EMAIL)); + } + + @Test + public void testThrowsErrorWhenFieldIsEmpty() { + try { + new ValidationErrorImpl(null, MSG_EMAIL); + fail("Should throw exception"); + } catch (IllegalArgumentException e) { + assertThat(e.getMessage(), equalTo(ValidationErrorImpl.FIELD_NULL_EMPTY_ERROR)); + } + } + + @Test + public void testEquality() { + ValidationError validationError = new ValidationErrorImpl(FIELD_EMAIL, MSG_EMAIL); + ValidationError validationError2 = new ValidationErrorImpl(FIELD_EMAIL, MSG_EMAIL); + assertThat(validationError, equalTo(validationError2)); + assertThat(validationError.hashCode(), equalTo(validationError2.hashCode())); + } +}