Skip to content

Commit a16d7ee

Browse files
authored
Prevent reserved fields from being registered (#819)
* Improve validation check when applying Featureset * Address PR comments
1 parent 3d25b72 commit a16d7ee

2 files changed

Lines changed: 43 additions & 0 deletions

File tree

core/src/main/java/feast/core/validators/FeatureSetValidator.java

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,17 @@
2222
import feast.proto.core.FeatureSetProto.EntitySpec;
2323
import feast.proto.core.FeatureSetProto.FeatureSet;
2424
import feast.proto.core.FeatureSetProto.FeatureSpec;
25+
import java.util.Arrays;
2526
import java.util.HashSet;
2627
import java.util.List;
2728
import java.util.stream.Collectors;
29+
import org.apache.commons.lang3.StringUtils;
2830

2931
public class FeatureSetValidator {
3032

33+
private static List<String> reservedNames =
34+
Arrays.asList("created_timestamp", "event_timestamp", "ingestion_id", "job_id");
35+
3136
public static void validateSpec(FeatureSet featureSet) {
3237
if (featureSet.getSpec().getProject().isEmpty()) {
3338
throw new IllegalArgumentException("Project name must be provided");
@@ -43,6 +48,7 @@ public static void validateSpec(FeatureSet featureSet) {
4348
checkValidCharacters(featureSet.getSpec().getName(), "name");
4449
checkUniqueColumns(
4550
featureSet.getSpec().getEntitiesList(), featureSet.getSpec().getFeaturesList());
51+
checkReservedColumns(featureSet.getSpec().getFeaturesList());
4652
for (EntitySpec entitySpec : featureSet.getSpec().getEntitiesList()) {
4753
checkValidCharacters(entitySpec.getName(), "entities::name");
4854
}
@@ -64,4 +70,17 @@ private static void checkUniqueColumns(
6470
String.format("fields within a featureset must be unique."));
6571
}
6672
}
73+
74+
private static void checkReservedColumns(List<FeatureSpec> featureSpecs) {
75+
String reservedNamesString = StringUtils.join(reservedNames, ", ");
76+
for (FeatureSpec featureSpec : featureSpecs) {
77+
if (reservedNames.contains(featureSpec.getName())) {
78+
throw new IllegalArgumentException(
79+
String.format(
80+
"Reserved feature names have been used, which are not allowed. These names include %s."
81+
+ "You've just used an invalid name, %s.",
82+
reservedNamesString, featureSpec.getName()));
83+
}
84+
}
85+
}
6786
}

core/src/test/java/feast/core/service/SpecServiceTest.java

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
import java.util.*;
5757
import java.util.Map.Entry;
5858
import java.util.stream.Collectors;
59+
import org.apache.commons.lang3.StringUtils;
5960
import org.junit.Before;
6061
import org.junit.Rule;
6162
import org.junit.Test;
@@ -91,6 +92,7 @@ public class SpecServiceTest {
9192

9293
private SpecService specService;
9394
private List<FeatureSet> featureSets;
95+
private List<FeatureSet> invalidFeatureSets;
9496
private List<Feature> features;
9597
private List<Store> stores;
9698
private Source defaultSource;
@@ -214,6 +216,12 @@ public void setUp() throws InvalidProtocolBufferException {
214216

215217
specService =
216218
new SpecService(featureSetRepository, storeRepository, projectRepository, defaultSource);
219+
220+
Feature invalidFeature1 = TestUtil.CreateFeature("created_timestamp", Enum.INT64);
221+
FeatureSet invalidFeatureSet1 =
222+
TestUtil.CreateFeatureSet(
223+
"f1", "invalid", Arrays.asList(f3e1), Arrays.asList(invalidFeature1));
224+
invalidFeatureSets = Arrays.asList(invalidFeatureSet1);
217225
}
218226

219227
@Test
@@ -277,6 +285,22 @@ public void shouldThrowExceptionGivenMissingFeatureSetName()
277285
specService.getFeatureSet(GetFeatureSetRequest.newBuilder().build());
278286
}
279287

288+
@Test
289+
public void shouldThrowExceptionGivenReservedFeatureName() throws InvalidProtocolBufferException {
290+
List<String> reservedNames =
291+
Arrays.asList("created_timestamp", "event_timestamp", "ingestion_id", "job_id");
292+
String reservedNamesString = StringUtils.join(reservedNames, ", ");
293+
expectedException.expect(IllegalArgumentException.class);
294+
expectedException.expectMessage(
295+
String.format(
296+
"Reserved feature names have been used, which are not allowed. These names include %s."
297+
+ "You've just used an invalid name, %s.",
298+
reservedNamesString, "created_timestamp"));
299+
FeatureSet invalidFeatureSet = invalidFeatureSets.get(0);
300+
301+
specService.applyFeatureSet(invalidFeatureSet.toProto());
302+
}
303+
280304
@Test
281305
public void shouldThrowExceptionGivenMissingFeatureSet() throws InvalidProtocolBufferException {
282306
expectedException.expect(RetrievalException.class);

0 commit comments

Comments
 (0)