Skip to content

Commit

Permalink
Merge pull request Netflix#56 from allenxwang/master
Browse files Browse the repository at this point in the history
Changing the PropertyChangeValidator interface to throw ValidationException instead of return boolean so that the message can be more descriptive and ease handling of validation failures.
  • Loading branch information
allenxwang committed Dec 19, 2012
2 parents adf8f0f + 73dbcd7 commit 4978d78
Show file tree
Hide file tree
Showing 6 changed files with 15 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ private void addOrChangeProperty(String name, Object newValue, final Configurati
}
}
} catch (ValidationException e) {
log.error("Validation failed for property " + name, e);
log.warn("Validation failed for property " + name, e);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -506,12 +506,11 @@ private void notifyCallbacks() {
private void validate(String newValue) {
for (PropertyChangeValidator v: validators) {
try {
if (!v.validate(newValue)) {
throw new ValidationException("Validation failed from validator "
+ v.getClass().getName() + " for value " + newValue);
}
v.validate(newValue);
} catch (ValidationException e) {
throw e;
} catch (Throwable e) {
throw new ValidationException("Unexpected exception from validator " + v.getClass().getName(), e);
throw new ValidationException("Unexpected exception during validation", e);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,8 @@ public void run() {
});
this.prop.addValidator(new PropertyChangeValidator() {
@Override
public boolean validate(String newValue) {
return PropertyWrapper.this.validate(newValue);
public void validate(String newValue) {
PropertyWrapper.this.validate(newValue);
}
});
}
Expand Down Expand Up @@ -110,8 +110,7 @@ protected void propertyChanged(V newValue) {
// by default, do nothing
}

protected boolean validate(String newValue) {
return true;
protected void validate(String newValue) {
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@

public interface PropertyChangeValidator {

public boolean validate(String newValue);
public void validate(String newValue) throws ValidationException;
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.junit.Test;

import com.netflix.config.sources.URLConfigurationSource;
import com.netflix.config.validation.ValidationException;

import static org.junit.Assert.*;

Expand Down Expand Up @@ -103,11 +104,10 @@ public void run() {

DynamicIntProperty validatedProp = new DynamicIntProperty("abc", 0) {
@Override
public boolean validate(String newValue) {
if (Integer.parseInt(newValue) >= 0) {
return true;
public void validate(String newValue) {
if (Integer.parseInt(newValue) < 0) {
throw new ValidationException("Cannot be negative");
}
return false;
}
};
assertEquals(0, validatedProp.get());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

import org.junit.Test;

import com.netflix.config.ConcurrentCompositeConfiguration;
import com.netflix.config.ConfigurationManager;
import com.netflix.config.DynamicStringProperty;
import com.netflix.config.validation.ValidationException;
Expand All @@ -14,8 +13,8 @@ public class ValidationTest {
@Test
public void testValidation() {
DynamicStringProperty prop = new DynamicStringProperty("abc", "default") {
public boolean validate(String newValue) {
return false;
public void validate(String newValue) {
throw new ValidationException("failed");
}
};
try {
Expand Down

0 comments on commit 4978d78

Please sign in to comment.