Skip to content
This repository has been archived by the owner on Apr 14, 2023. It is now read-only.

Commit

Permalink
feat(#1675): Add quotes in validation messages
Browse files Browse the repository at this point in the history
... so that it is clearer for users to interpret.
  • Loading branch information
matthewdunsdon committed Jul 15, 2020
1 parent b48e150 commit e900e81
Show file tree
Hide file tree
Showing 38 changed files with 167 additions and 150 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.List;
import java.util.stream.Collectors;
import java.util.stream.Stream;
Expand Down Expand Up @@ -77,6 +78,24 @@ public static ValidationResult combine(Stream<ValidationResult> validationResult
});
return new ValidationResult(isValid.stream().allMatch(o -> o), errors);
}

public static String quote(Object value)
{
if (value instanceof Collection)
{
Collection<Object> values = (Collection<Object>) value;
return values.stream().map(ValidationResult::quote).collect(Collectors.joining(", "));
} else if (value instanceof String)
{
return String.format("'%s'", ((String) value).replace("'", "\\'"));
} else if (value == null)
{
return "NULL";
} else
{
return value.toString();
}
}
}


Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ Feature: User can create data across multiple fields for all combinations availa
And bar is in set:
| 2 |
| null |
Then the profile is invalid with error "Values must be specified | Field: bar | Constraint: inSet"
Then the profile is invalid with error "Values must be specified | Field: 'bar' | Constraint: 'inSet'"

Scenario: Running an exhaustive combination strategy should be successful
Given the following non nullable fields exist:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ Feature: User can specify that a value is equalTo a required value
Given there is a non nullable field foo
And foo has type "string"
And foo is equal to null
Then the profile is invalid with error "Values must be specified | Field: foo | Constraint: equalTo"
Then the profile is invalid with error "Values must be specified | Field: 'foo' | Constraint: 'equalTo'"

Scenario: Running an 'equalTo' of an invalid date value should fail with an error message
Given there is a non nullable field foo
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ Feature: User can specify that a field value belongs to a set of predetermined o
And foo is in set:
| null |
| 1 |
Then the profile is invalid with error "Values must be specified | Field: foo | Constraint: inSet"
Then the profile is invalid with error "Values must be specified | Field: 'foo' | Constraint: 'inSet'"
And no data is created

Scenario: Running an 'inSet' request that includes multiples of the same entry should be successful.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -241,4 +241,4 @@ Feature: User can specify that a datetime date is after, but not equal to, a spe

Scenario: Running a 'after' request that specifies the highest valid system date should be unsuccessful
Given foo is after 10000-01-01T00:00:00.000Z
Then the profile is invalid with error containing "Dates must be between 0001-01-01T00:00:00.000Z and 9999-12-31T23:59:59.999Z | Field: foo | Constraint: after"
Then the profile is invalid with error containing "Dates must be between 0001-01-01T00:00:00.000Z and 9999-12-31T23:59:59.999Z | Field: 'foo' | Constraint: 'after'"
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ Feature: User can specify that a datetime date is more than, or the same as, a s

Scenario: Running afterOrAt request that includes datetime field with date and time (YYYY-MM-DDTHH:MM:SS) values that has invalid year should fail
Given foo is after or at 0000-01-10T00:00:00.000Z
Then the profile is invalid with error containing "Dates must be between 0001-01-01T00:00:00.000Z and 9999-12-31T23:59:59.999Z | Field: foo | Constraint: afterOrAt"
Then the profile is invalid with error containing "Dates must be between 0001-01-01T00:00:00.000Z and 9999-12-31T23:59:59.999Z | Field: 'foo' | Constraint: 'afterOrAt'"
And no data is created

Scenario: Running afterOrAt request that includes datetime field with date and time (YYYY-MM-DDTHH:MM:SS) values that has leap year
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,4 +108,4 @@ Feature: User can specify that a datetime date is lower than, but not equal to,

Scenario: Running a 'before' request that specifies the highest valid system date should be unsuccessful
Given foo is before 0000-01-01T00:00:00.000Z
Then the profile is invalid with error containing "Dates must be between 0001-01-01T00:00:00.000Z and 9999-12-31T23:59:59.999Z | Field: foo | Constraint: before"
Then the profile is invalid with error containing "Dates must be between 0001-01-01T00:00:00.000Z and 9999-12-31T23:59:59.999Z | Field: 'foo' | Constraint: 'before'"
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ Feature: User can specify that a datetime date is lower than, or the same as, a

Scenario: Running beforeOrAt request that includes datetime field with date and time (YYYY-MM-DDTHH:MM:SS) values that has invalid year should fail
Given foo is before or at 0000-01-10T00:00:00.000Z
Then the profile is invalid with error containing "Dates must be between 0001-01-01T00:00:00.000Z and 9999-12-31T23:59:59.999Z | Field: foo | Constraint: beforeOrAt"
Then the profile is invalid with error containing "Dates must be between 0001-01-01T00:00:00.000Z and 9999-12-31T23:59:59.999Z | Field: 'foo' | Constraint: 'beforeOrAt'"
And no data is created

Scenario: Running beforeOrAt request against a non-contradicting beforeOrAt constraint should be successful
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,5 +134,5 @@ Feature: User can specify that datetime fields are granular to a certain unit

Scenario: Applying an invalid datetime granularTo constraint fails with an appropriate error
Given foo is granular to "decades"
Then the profile is invalid with error "Granularity decades is not supported | Field: foo | Constraint: granularTo"
Then the profile is invalid with error "Granularity 'decades' is not supported | Field: 'foo' | Constraint: 'granularTo'"
And no data is created
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,14 @@
import com.scottlogic.datahelix.generator.profile.dtos.constraints.relations.InMapConstraintDTO;
import com.scottlogic.datahelix.generator.profile.dtos.constraints.relations.RelationalConstraintDTO;
import com.scottlogic.datahelix.generator.profile.services.FieldService;
import com.scottlogic.datahelix.generator.profile.validators.profile.constraints.relations.InMapConstraintValidator;
import com.scottlogic.datahelix.generator.profile.validators.profile.constraints.grammatical.NotConstraintValidator;
import com.scottlogic.datahelix.generator.profile.validators.profile.constraints.relations.RelationalConstraintValidator;
import com.scottlogic.datahelix.generator.profile.validators.profile.constraints.atomic.*;
import com.scottlogic.datahelix.generator.profile.validators.profile.constraints.capabilities.DateTimeGranularityValidator;
import com.scottlogic.datahelix.generator.profile.validators.profile.constraints.grammatical.AllOfConstraintValidator;
import com.scottlogic.datahelix.generator.profile.validators.profile.constraints.grammatical.AnyOfConstraintValidator;
import com.scottlogic.datahelix.generator.profile.validators.profile.constraints.grammatical.ConditionalConstraintValidator;
import com.scottlogic.datahelix.generator.profile.validators.profile.constraints.grammatical.NotConstraintValidator;
import com.scottlogic.datahelix.generator.profile.validators.profile.constraints.relations.InMapConstraintValidator;
import com.scottlogic.datahelix.generator.profile.validators.profile.constraints.relations.RelationalConstraintValidator;

import java.util.List;

Expand All @@ -62,7 +62,7 @@ protected ConstraintValidator(List<FieldDTO> fields)

protected String getErrorInfo(T constraint)
{
return " | Constraint: " + constraint.getType().propertyName;
return String.format(" | Constraint: %s", ValidationResult.quote(constraint.getType().propertyName));
}

protected static ValidationResult validateConstraint(ConstraintDTO dto, List<FieldDTO> fields)
Expand Down Expand Up @@ -157,9 +157,9 @@ protected ValidationResult validateGranularity(T dto, String field, Object value
switch (fieldType)
{
case BOOLEAN:
return ValidationResult.failure("Granularity " + value + " is not supported for boolean fields" + getErrorInfo(dto));
return ValidationResult.failure(String.format("Granularity %s is not supported for boolean fields%s", ValidationResult.quote(value), getErrorInfo(dto)));
case STRING:
return ValidationResult.failure("Granularity " + value + " is not supported for string fields" + getErrorInfo(dto));
return ValidationResult.failure(String.format("Granularity %s is not supported for string fields%s", ValidationResult.quote(value), getErrorInfo(dto)));
case DATETIME:
return new DateTimeGranularityValidator(getErrorInfo(dto)).validate((String) value);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,10 @@ private ValidationResult nameMustBeSpecified(FieldDTO field)

private ValidationResult typeMustBeSpecified(FieldDTO field)
{
String fieldName = field.name == null ? "Unnamed field" : field.name;
String fieldName = field.name == null ? "Unnamed" : ValidationResult.quote(field.name);
return field.type != null
? ValidationResult.success()
: ValidationResult.failure("Field type must be specified | Field: " + fieldName);
: ValidationResult.failure(String.format("Field type must be specified | Field: %s", fieldName));
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,16 +73,15 @@ private ValidationResult fieldsMustBeUnique(List<FieldDTO> fields)

fields.forEach(field ->
{
if(!fieldNames.add(field.name))
if (!fieldNames.add(field.name))
{
duplicateFieldNames.add(field.name);
}
});

return duplicateFieldNames.isEmpty()
? ValidationResult.success()
: ValidationResult.failure("Field names must be unique | Duplicates: "
+ String.join(", ", duplicateFieldNames));
: ValidationResult.failure(String.format("Field names must be unique | Duplicates: %s", ValidationResult.quote(duplicateFieldNames)));
}

private ValidationResult fieldsMustBeValid(List<FieldDTO> fields)
Expand Down Expand Up @@ -113,7 +112,7 @@ private ValidationResult uniqueFieldsMustNotBeInIfStatements(ProfileDTO dto)
.collect(Collectors.toSet());

List<String> errors = uniqueFields.stream().filter(ifConstraintFields::contains)
.map(f -> "Unique field "+f+" cannot be referenced in IF statement")
.map(f -> String.format("Unique field %s cannot be referenced in IF statement", ValidationResult.quote(f)))
.collect(Collectors.toList());

return errors.isEmpty()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,37 +35,37 @@ abstract class AtomicConstraintValidator<T extends AtomicConstraintDTO> extends
@Override
protected String getErrorInfo(T atomicConstraint)
{
return " | Field: " + atomicConstraint.field + super.getErrorInfo(atomicConstraint);
return String.format(" | Field: %s%s", ValidationResult.quote(atomicConstraint.field), super.getErrorInfo(atomicConstraint));
}

ValidationResult fieldTypeMustMatchValueType(T dto, FieldType expectedFieldType)
{
FieldType fieldType = getFieldType(dto.field);
if (expectedFieldType != fieldType)
{
return ValidationResult.failure("Expected field type " + expectedFieldType + " doesn't match field type " + fieldType + getErrorInfo(dto));
return ValidationResult.failure(String.format("Expected field type %s doesn't match field type %s%s", ValidationResult.quote(expectedFieldType), ValidationResult.quote(fieldType), getErrorInfo(dto)));
}
return ValidationResult.success();
}

ValidationResult fieldTypeMustMatchValueType(T dto, Object value)
{
if(value == null)
if (value == null)
{
return ValidationResult.failure("Values must be specified" + getErrorInfo(dto));
}
FieldType fieldType = getFieldType(dto.field);
if(value instanceof Boolean && fieldType != FieldType.BOOLEAN)
if (value instanceof Boolean && fieldType != FieldType.BOOLEAN)
{
return ValidationResult.failure("Value " + value + " must be a boolean" + getErrorInfo(dto));
return ValidationResult.failure(String.format("Value %s must be a boolean%s", ValidationResult.quote(value), getErrorInfo(dto)));
}
if (!(value instanceof Number || value instanceof String && isNumber((String)value)) && fieldType == FieldType.NUMERIC)
if (!(value instanceof Number || value instanceof String && isNumber((String) value)) && fieldType == FieldType.NUMERIC)
{
return ValidationResult.failure("Value " + value + " must be a number" + getErrorInfo(dto));
return ValidationResult.failure(String.format("Value %s must be a number%s", ValidationResult.quote(value), getErrorInfo(dto)));
}
if (value instanceof Number && fieldType != FieldType.NUMERIC)
{
return ValidationResult.failure("Value " + value + " must be a string" + getErrorInfo(dto));
return ValidationResult.failure(String.format("Value %s must be a string%s", ValidationResult.quote(value), getErrorInfo(dto)));
}
return ValidationResult.success();
}
Expand All @@ -76,8 +76,7 @@ private static boolean isNumber(String s)
{
Double.parseDouble(s);
return true;
}
catch (NumberFormatException | NullPointerException e)
} catch (NumberFormatException | NullPointerException e)
{
return false;
}
Expand All @@ -94,7 +93,7 @@ ValidationResult fieldMustBeValid(T dto)
Optional<FieldDTO> field = fields.stream().filter(f -> f.name.equals(fieldName)).findFirst();
if (!field.isPresent())
{
return ValidationResult.failure(fieldName + " must be defined in fields" + getErrorInfo(dto));
return ValidationResult.failure(String.format("%s must be defined in fields%s", ValidationResult.quote(fieldName), getErrorInfo(dto)));
}
return ValidationResult.success();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public final ValidationResult validate(String value)
}
} catch (Exception e)
{
return ValidationResult.failure("Granularity " + value + " is not supported" + errorInfo);
return ValidationResult.failure(String.format("Granularity %s is not supported%s", ValidationResult.quote(value), errorInfo));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ private ValidationResult fieldMustBeValid(InMapConstraintDTO dto)
Optional<FieldDTO> field = fields.stream().filter(f -> f.name.equals(dto.field)).findFirst();
if (!field.isPresent())
{
return ValidationResult.failure(dto.field + " must be defined in fields" + getErrorInfo(dto));
return ValidationResult.failure(String.format("%s must be defined in fields%s", ValidationResult.quote(dto.field), getErrorInfo(dto)));
}
return ValidationResult.success();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,18 +48,18 @@ public ValidationResult validate(T dto)
Optional<FieldDTO> field = fields.stream().filter(f -> f.name.equals(fieldName)).findFirst();
if (!field.isPresent())
{
return ValidationResult.failure(fieldName + " must be defined in fields" + getErrorInfo(dto));
return ValidationResult.failure(String.format("%s must be defined in fields%s", ValidationResult.quote(fieldName), getErrorInfo(dto)));
}
Optional<FieldDTO> otherField = fields.stream().filter(f -> f.name.equals(otherFieldName)).findFirst();
if (!otherField.isPresent())
{
return ValidationResult.failure(otherFieldName + " must be defined in fields" + getErrorInfo(dto));
return ValidationResult.failure(String.format("%s must be defined in fields%s", ValidationResult.quote(otherFieldName), getErrorInfo(dto)));
}
FieldType fieldType = getFieldType(field.get().name);
FieldType otherFieldType = getFieldType(otherField.get().name);
if (fieldType != otherFieldType)
{
return ValidationResult.failure("Field type " + fieldName + " doesn't match related field type " + otherFieldName + getErrorInfo(dto));
return ValidationResult.failure(String.format("Field type %s doesn't match related field type %s%s", ValidationResult.quote(fieldName), ValidationResult.quote(otherFieldName), getErrorInfo(dto)));
}
if (dto.offsetUnit != null && !dto.offsetUnit.isEmpty())
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@
import com.scottlogic.datahelix.generator.profile.validators.ReadRelationshipsValidator;
import com.scottlogic.datahelix.generator.profile.validators.profile.ProfileValidator;
import org.junit.Assert;
import org.junit.Ignore;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -603,7 +602,8 @@ public void shouldRejectNonPowerOfTenNumericGranularityConstraint() {
}

@Test
public void shouldRejectEqualToWithNullValue() {
public void shouldRejectEqualToWithNullValue()
{
givenJson(
"{" +
" \"fields\": [ { \"name\": \"foo\", \"type\": \"datetime\" } ]," +
Expand All @@ -612,23 +612,25 @@ public void shouldRejectEqualToWithNullValue() {
" ]" +
"}");

expectValidationException("Values must be specified | Field: foo | Constraint: equalTo");
expectValidationException("Values must be specified | Field: 'foo' | Constraint: 'equalTo'");
}

@Test
public void shouldRejectLessThanWithNullValue() {
public void shouldRejectLessThanWithNullValue()
{
givenJson(
"{" +
" \"fields\": [ { \"name\": \"foo\", \"type\": \"integer\" } ]," +
" \"constraints\": [" +
" { \"field\": \"foo\", \"lessThan\": null }" +
" ]" +
"}");
expectValidationException("Number must be specified | Field: foo | Constraint: lessThan");
expectValidationException("Number must be specified | Field: 'foo' | Constraint: 'lessThan'");
}

@Test
public void shouldRejectInSetWithANullValue() {
public void shouldRejectInSetWithANullValue()
{
givenJson(
"{" +
" \"fields\": [ { \"name\": \"foo\", \"type\": \"datetime\" } ]," +
Expand All @@ -637,11 +639,12 @@ public void shouldRejectInSetWithANullValue() {
" ]" +
"}");

expectValidationException("Values must be specified | Field: foo | Constraint: inSet");
expectValidationException("Values must be specified | Field: 'foo' | Constraint: 'inSet'");
}

@Test
public void shouldRejectInSetSetToNull() {
public void shouldRejectInSetSetToNull()
{
givenJson(
"{" +
" \"fields\": [ { \"name\": \"foo\", \"type\": \"datetime\" } ]," +
Expand All @@ -650,7 +653,7 @@ public void shouldRejectInSetSetToNull() {
" ]" +
"}");

expectValidationException("In set values must be specified | Field: foo | Constraint: inSet");
expectValidationException("In set values must be specified | Field: 'foo' | Constraint: 'inSet'");
}

@Test
Expand Down

0 comments on commit e900e81

Please sign in to comment.