diff --git a/core/src/main/java/feast/core/service/SpecService.java b/core/src/main/java/feast/core/service/SpecService.java index e4b1ce5c43..e7a317b13c 100644 --- a/core/src/main/java/feast/core/service/SpecService.java +++ b/core/src/main/java/feast/core/service/SpecService.java @@ -98,7 +98,7 @@ public GetFeatureSetResponse getFeatureSet(GetFeatureSetRequest request) private FeatureSet getFeatureSet(String projectName, String featureSetName) { // Validate input arguments - checkValidCharacters(featureSetName, "featureSetName"); + checkValidCharacters(featureSetName, "featureset"); if (featureSetName.isEmpty()) { throw new IllegalArgumentException("No feature set name provided"); @@ -151,8 +151,8 @@ public ListFeatureSetsResponse listFeatureSets(ListFeatureSetsRequest.Filter fil "Invalid listFeatureSetRequest, missing arguments. Must provide feature set name:"); } - checkValidCharactersAllowAsterisk(name, "featureSetName"); - checkValidCharactersAllowAsterisk(project, "projectName"); + checkValidCharactersAllowAsterisk(name, "featureset"); + checkValidCharactersAllowAsterisk(project, "project"); // Autofill default project if project not specified if (project.isEmpty()) { @@ -239,7 +239,7 @@ public ListFeaturesResponse listFeatures(ListFeaturesRequest.Filter filter) { List entities = filter.getEntitiesList(); Map labels = filter.getLabelsMap(); - checkValidCharactersAllowAsterisk(project, "projectName"); + checkValidCharactersAllowAsterisk(project, "project"); // Autofill default project if project not specified if (project.isEmpty()) { diff --git a/core/src/main/java/feast/core/validators/FeatureSetValidator.java b/core/src/main/java/feast/core/validators/FeatureSetValidator.java index 3433523661..8787d75f69 100644 --- a/core/src/main/java/feast/core/validators/FeatureSetValidator.java +++ b/core/src/main/java/feast/core/validators/FeatureSetValidator.java @@ -45,15 +45,15 @@ public static void validateSpec(FeatureSet featureSet) { } checkValidCharacters(featureSet.getSpec().getProject(), "project"); - checkValidCharacters(featureSet.getSpec().getName(), "name"); + checkValidCharacters(featureSet.getSpec().getName(), "featureset"); checkUniqueColumns( featureSet.getSpec().getEntitiesList(), featureSet.getSpec().getFeaturesList()); checkReservedColumns(featureSet.getSpec().getFeaturesList()); for (EntitySpec entitySpec : featureSet.getSpec().getEntitiesList()) { - checkValidCharacters(entitySpec.getName(), "entities::name"); + checkValidCharacters(entitySpec.getName(), "entity"); } for (FeatureSpec featureSpec : featureSet.getSpec().getFeaturesList()) { - checkValidCharacters(featureSpec.getName(), "features::name"); + checkValidCharacters(featureSpec.getName(), "feature"); if (featureSpec.getLabelsMap().containsKey("")) { throw new IllegalArgumentException("Feature label keys must not be empty"); } diff --git a/core/src/main/java/feast/core/validators/Matchers.java b/core/src/main/java/feast/core/validators/Matchers.java index 87e2b126f0..ba12a83a79 100644 --- a/core/src/main/java/feast/core/validators/Matchers.java +++ b/core/src/main/java/feast/core/validators/Matchers.java @@ -22,52 +22,56 @@ public class Matchers { private static Pattern UPPER_SNAKE_CASE_REGEX = Pattern.compile("^[A-Z0-9]+(_[A-Z0-9]+)*$"); private static Pattern LOWER_SNAKE_CASE_REGEX = Pattern.compile("^[a-z0-9]+(_[a-z0-9]+)*$"); - private static Pattern VALID_CHARACTERS_REGEX = Pattern.compile("^[a-zA-Z0-9\\-_]*$"); + private static Pattern VALID_CHARACTERS_REGEX = Pattern.compile("^[a-zA-Z_][a-zA-Z0-9_]*$"); private static Pattern VALID_CHARACTERS_REGEX_WITH_ASTERISK_WILDCARD = Pattern.compile("^[a-zA-Z0-9\\-_*]*$"); - private static String ERROR_MESSAGE_TEMPLATE = "invalid value for field %s: %s"; + private static String ERROR_MESSAGE_TEMPLATE = "invalid value for %s resource, %s: %s"; - public static void checkUpperSnakeCase(String input, String fieldName) + public static void checkUpperSnakeCase(String input, String resource) throws IllegalArgumentException { if (!UPPER_SNAKE_CASE_REGEX.matcher(input).matches()) { throw new IllegalArgumentException( String.format( ERROR_MESSAGE_TEMPLATE, - fieldName, + resource, + input, "argument must be in upper snake case, and cannot include any special characters.")); } } - public static void checkLowerSnakeCase(String input, String fieldName) + public static void checkLowerSnakeCase(String input, String resource) throws IllegalArgumentException { if (!LOWER_SNAKE_CASE_REGEX.matcher(input).matches()) { throw new IllegalArgumentException( String.format( ERROR_MESSAGE_TEMPLATE, - fieldName, + resource, + input, "argument must be in lower snake case, and cannot include any special characters.")); } } - public static void checkValidCharacters(String input, String fieldName) + public static void checkValidCharacters(String input, String resource) throws IllegalArgumentException { if (!VALID_CHARACTERS_REGEX.matcher(input).matches()) { throw new IllegalArgumentException( String.format( ERROR_MESSAGE_TEMPLATE, - fieldName, - "argument must only contain alphanumeric characters, dashes and underscores.")); + resource, + input, + "argument must only contain alphanumeric characters and underscores.")); } } - public static void checkValidCharactersAllowAsterisk(String input, String fieldName) + public static void checkValidCharactersAllowAsterisk(String input, String resource) throws IllegalArgumentException { if (!VALID_CHARACTERS_REGEX_WITH_ASTERISK_WILDCARD.matcher(input).matches()) { throw new IllegalArgumentException( String.format( ERROR_MESSAGE_TEMPLATE, - fieldName, + resource, + input, "argument must only contain alphanumeric characters, dashes, underscores, or an asterisk.")); } } diff --git a/core/src/test/java/feast/core/service/SpecServiceIT.java b/core/src/test/java/feast/core/service/SpecServiceIT.java index 75933d3ee2..697ad424d3 100644 --- a/core/src/test/java/feast/core/service/SpecServiceIT.java +++ b/core/src/test/java/feast/core/service/SpecServiceIT.java @@ -228,6 +228,30 @@ public void shouldThrowExceptionGivenReservedFeatureName() { reservedNamesString, "event_timestamp"))); } + @Test + public void shouldThrowExceptionGivenFeatureSetWithDash() { + StatusRuntimeException exc = + assertThrows( + StatusRuntimeException.class, + () -> + apiClient.simpleApplyFeatureSet( + DataGenerator.createFeatureSet( + DataGenerator.getDefaultSource(), + "project", + "dash-name", + ImmutableMap.of("entity", ValueProto.ValueType.Enum.STRING), + ImmutableMap.of("test_string", ValueProto.ValueType.Enum.STRING)))); + + assertThat( + exc.getMessage(), + equalTo( + String.format( + "INTERNAL: invalid value for %s resource, %s: %s", + "featureset", + "dash-name", + "argument must only contain alphanumeric characters and underscores."))); + } + @Test public void shouldReturnFeatureSetIfFeatureSetHasNotChanged() { FeatureSetProto.FeatureSet featureSet = apiClient.getFeatureSet("default", "fs1"); diff --git a/core/src/test/java/feast/core/validators/MatchersTest.java b/core/src/test/java/feast/core/validators/MatchersTest.java index 774e58c7a8..eb9d987cbd 100644 --- a/core/src/test/java/feast/core/validators/MatchersTest.java +++ b/core/src/test/java/feast/core/validators/MatchersTest.java @@ -30,13 +30,13 @@ public class MatchersTest { @Test public void checkUpperSnakeCaseShouldPassForLegitUpperSnakeCase() { String in = "REDIS_DB"; - checkUpperSnakeCase(in, "someField"); + checkUpperSnakeCase(in, "featureset"); } @Test public void checkUpperSnakeCaseShouldPassForLegitUpperSnakeCaseWithNumbers() { String in = "REDIS1"; - checkUpperSnakeCase(in, "someField"); + checkUpperSnakeCase(in, "featureset"); } @Test @@ -44,17 +44,18 @@ public void checkUpperSnakeCaseShouldThrowIllegalArgumentExceptionWithFieldForIn exception.expect(IllegalArgumentException.class); exception.expectMessage( Strings.lenientFormat( - "invalid value for field %s: %s", - "someField", + "invalid value for %s resource, %s: %s", + "featureset", + "redis", "argument must be in upper snake case, and cannot include any special characters.")); String in = "redis"; - checkUpperSnakeCase(in, "someField"); + checkUpperSnakeCase(in, "featureset"); } @Test public void checkLowerSnakeCaseShouldPassForLegitLowerSnakeCase() { String in = "feature_name_v1"; - checkLowerSnakeCase(in, "someField"); + checkLowerSnakeCase(in, "feature"); } @Test @@ -62,10 +63,11 @@ public void checkLowerSnakeCaseShouldThrowIllegalArgumentExceptionWithFieldForIn exception.expect(IllegalArgumentException.class); exception.expectMessage( Strings.lenientFormat( - "invalid value for field %s: %s", - "someField", + "invalid value for %s resource, %s: %s", + "feature", + "Invalid_feature name", "argument must be in lower snake case, and cannot include any special characters.")); String in = "Invalid_feature name"; - checkLowerSnakeCase(in, "someField"); + checkLowerSnakeCase(in, "feature"); } }