From f156f8361a0c366d66c48cab75bcdfe48b62c4d0 Mon Sep 17 00:00:00 2001
From: Khor Shu Heng <32997938+khorshuheng@users.noreply.github.com>
Date: Mon, 30 Nov 2020 18:01:50 +0800
Subject: [PATCH] Fix Feature Table not updated on new feature addition (#1197)

Signed-off-by: Khor Shu Heng <khor.heng@gojek.com>

Co-authored-by: Khor Shu Heng <khor.heng@gojek.com>
---
 common-test/pom.xml                           |  6 ++---
 .../main/java/feast/common/util/TestUtil.java |  5 ++--
 .../java/feast/core/model/FeatureTable.java   |  4 +--
 .../feast/core/service/SpecServiceIT.java     | 26 +++++++++++++++++++
 4 files changed, 34 insertions(+), 7 deletions(-)

diff --git a/common-test/pom.xml b/common-test/pom.xml
index 563129cf063..62fd6ae00f2 100644
--- a/common-test/pom.xml
+++ b/common-test/pom.xml
@@ -106,17 +106,17 @@
         <dependency>
             <groupId>org.testcontainers</groupId>
             <artifactId>junit-jupiter</artifactId>
-            <version>1.14.3</version>
+            <version>1.15.0</version>
         </dependency>
         <dependency>
             <groupId>org.testcontainers</groupId>
             <artifactId>postgresql</artifactId>
-            <version>1.14.3</version>
+            <version>1.15.0</version>
         </dependency>
         <dependency>
             <groupId>org.testcontainers</groupId>
             <artifactId>kafka</artifactId>
-            <version>1.14.3</version>
+            <version>1.15.0</version>
         </dependency>
         <dependency>
             <groupId>org.junit.jupiter</groupId>
diff --git a/common-test/src/main/java/feast/common/util/TestUtil.java b/common-test/src/main/java/feast/common/util/TestUtil.java
index 142c5e4850d..ee355d3766f 100644
--- a/common-test/src/main/java/feast/common/util/TestUtil.java
+++ b/common-test/src/main/java/feast/common/util/TestUtil.java
@@ -62,11 +62,12 @@ public static boolean compareFeatureTableSpec(FeatureTableSpec spec, FeatureTabl
             .toBuilder()
             .clearFeatures()
             .addAllFeatures(
-                spec.getFeaturesList().stream()
+                otherSpec.getFeaturesList().stream()
                     .sorted(Comparator.comparing(FeatureSpecV2::getName))
                     .collect(Collectors.toSet()))
             .clearEntities()
-            .addAllEntities(spec.getEntitiesList().stream().sorted().collect(Collectors.toSet()))
+            .addAllEntities(
+                otherSpec.getEntitiesList().stream().sorted().collect(Collectors.toSet()))
             .build();
 
     return spec.equals(otherSpec);
diff --git a/core/src/main/java/feast/core/model/FeatureTable.java b/core/src/main/java/feast/core/model/FeatureTable.java
index 8c62df64867..72ad77f1b3e 100644
--- a/core/src/main/java/feast/core/model/FeatureTable.java
+++ b/core/src/main/java/feast/core/model/FeatureTable.java
@@ -340,8 +340,8 @@ public boolean equals(Object o) {
     return getName().equals(other.getName())
         && getProject().equals(other.getProject())
         && getLabelsJSON().equals(other.getLabelsJSON())
-        && getFeatures().containsAll(other.getFeatures())
-        && getEntities().containsAll(other.getEntities())
+        && getFeatures().equals(other.getFeatures())
+        && getEntities().equals(other.getEntities())
         && getMaxAgeSecs() == getMaxAgeSecs()
         && Optional.ofNullable(getBatchSource()).equals(Optional.ofNullable(other.getBatchSource()))
         && Optional.ofNullable(getStreamSource())
diff --git a/core/src/test/java/feast/core/service/SpecServiceIT.java b/core/src/test/java/feast/core/service/SpecServiceIT.java
index 8d56de606b0..40d6aa1a7aa 100644
--- a/core/src/test/java/feast/core/service/SpecServiceIT.java
+++ b/core/src/test/java/feast/core/service/SpecServiceIT.java
@@ -1158,6 +1158,32 @@ public void shouldUpdateFeatureTableOnFeatureTypeChange() {
       assertTrue(TestUtil.compareFeatureTableSpec(updatedTable.getSpec(), updatedSpec));
     }
 
+    @Test
+    public void shouldUpdateFeatureTableOnFeatureAddition() {
+      FeatureTableProto.FeatureTableSpec updatedSpec =
+          DataGenerator.createFeatureTableSpec(
+                  "featuretable1",
+                  Arrays.asList("entity1", "entity2"),
+                  new HashMap<>() {
+                    {
+                      put("feature1", ValueProto.ValueType.Enum.STRING);
+                      put("feature2", ValueProto.ValueType.Enum.FLOAT);
+                      put("feature3", ValueProto.ValueType.Enum.FLOAT);
+                    }
+                  },
+                  7200,
+                  ImmutableMap.of("feat_key2", "feat_value2"))
+              .toBuilder()
+              .setBatchSource(
+                  DataGenerator.createFileDataSourceSpec("file:///path/to/file", "ts_col", ""))
+              .build();
+
+      FeatureTableProto.FeatureTable updatedTable =
+          apiClient.applyFeatureTable("default", updatedSpec);
+
+      assertTrue(TestUtil.compareFeatureTableSpec(updatedTable.getSpec(), updatedSpec));
+    }
+
     @Test
     public void shouldNotUpdateIfNoChanges() {
       FeatureTableProto.FeatureTable table = apiClient.applyFeatureTable("default", getTestSpec());