Prevent reserved fields from being registered (#819)

* Improve validation check when applying Featureset * Address PR comments
feast-dev · Jun 23, 2020 · a16d7ee · a16d7ee
1 parent 3d25b72
commit a16d7ee
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 0 deletions.
diff --git a/core/src/main/java/feast/core/validators/FeatureSetValidator.java b/core/src/main/java/feast/core/validators/FeatureSetValidator.java
@@ -22,12 +22,17 @@
 import feast.proto.core.FeatureSetProto.EntitySpec;
 import feast.proto.core.FeatureSetProto.FeatureSet;
 import feast.proto.core.FeatureSetProto.FeatureSpec;
+import java.util.Arrays;
 import java.util.HashSet;
 import java.util.List;
 import java.util.stream.Collectors;
+import org.apache.commons.lang3.StringUtils;
 
 public class FeatureSetValidator {
 
+  private static List<String> reservedNames =
+      Arrays.asList("created_timestamp", "event_timestamp", "ingestion_id", "job_id");
+
   public static void validateSpec(FeatureSet featureSet) {
     if (featureSet.getSpec().getProject().isEmpty()) {
       throw new IllegalArgumentException("Project name must be provided");
@@ -43,6 +48,7 @@ public static void validateSpec(FeatureSet featureSet) {
     checkValidCharacters(featureSet.getSpec().getName(), "name");
     checkUniqueColumns(
         featureSet.getSpec().getEntitiesList(), featureSet.getSpec().getFeaturesList());
+    checkReservedColumns(featureSet.getSpec().getFeaturesList());
     for (EntitySpec entitySpec : featureSet.getSpec().getEntitiesList()) {
       checkValidCharacters(entitySpec.getName(), "entities::name");
     }
@@ -64,4 +70,17 @@ private static void checkUniqueColumns(
           String.format("fields within a featureset must be unique."));
     }
   }
+
+  private static void checkReservedColumns(List<FeatureSpec> featureSpecs) {
+    String reservedNamesString = StringUtils.join(reservedNames, ", ");
+    for (FeatureSpec featureSpec : featureSpecs) {
+      if (reservedNames.contains(featureSpec.getName())) {
+        throw new IllegalArgumentException(
+            String.format(
+                "Reserved feature names have been used, which are not allowed. These names include %s."
+                    + "You've just used an invalid name, %s.",
+                reservedNamesString, featureSpec.getName()));
+      }
+    }
+  }
 }
diff --git a/core/src/test/java/feast/core/service/SpecServiceTest.java b/core/src/test/java/feast/core/service/SpecServiceTest.java
@@ -56,6 +56,7 @@
 import java.util.*;
 import java.util.Map.Entry;
 import java.util.stream.Collectors;
+import org.apache.commons.lang3.StringUtils;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
@@ -91,6 +92,7 @@ public class SpecServiceTest {
 
   private SpecService specService;
   private List<FeatureSet> featureSets;
+  private List<FeatureSet> invalidFeatureSets;
   private List<Feature> features;
   private List<Store> stores;
   private Source defaultSource;
@@ -214,6 +216,12 @@ public void setUp() throws InvalidProtocolBufferException {
 
     specService =
         new SpecService(featureSetRepository, storeRepository, projectRepository, defaultSource);
+
+    Feature invalidFeature1 = TestUtil.CreateFeature("created_timestamp", Enum.INT64);
+    FeatureSet invalidFeatureSet1 =
+        TestUtil.CreateFeatureSet(
+            "f1", "invalid", Arrays.asList(f3e1), Arrays.asList(invalidFeature1));
+    invalidFeatureSets = Arrays.asList(invalidFeatureSet1);
   }
 
   @Test
@@ -277,6 +285,22 @@ public void shouldThrowExceptionGivenMissingFeatureSetName()
     specService.getFeatureSet(GetFeatureSetRequest.newBuilder().build());
   }
 
+  @Test
+  public void shouldThrowExceptionGivenReservedFeatureName() throws InvalidProtocolBufferException {
+    List<String> reservedNames =
+        Arrays.asList("created_timestamp", "event_timestamp", "ingestion_id", "job_id");
+    String reservedNamesString = StringUtils.join(reservedNames, ", ");
+    expectedException.expect(IllegalArgumentException.class);
+    expectedException.expectMessage(
+        String.format(
+            "Reserved feature names have been used, which are not allowed. These names include %s."
+                + "You've just used an invalid name, %s.",
+            reservedNamesString, "created_timestamp"));
+    FeatureSet invalidFeatureSet = invalidFeatureSets.get(0);
+
+    specService.applyFeatureSet(invalidFeatureSet.toProto());
+  }
+
   @Test
   public void shouldThrowExceptionGivenMissingFeatureSet() throws InvalidProtocolBufferException {
     expectedException.expect(RetrievalException.class);