Refactor common module Feature ref (#814)

Co-authored-by: Terence <terence.limxp@go-jek.com>
feast-dev · Jun 22, 2020 · 46e3e09 · 46e3e09
1 parent 7ce74c6
commit 46e3e09
Show file tree

Hide file tree

Showing 6 changed files with 28 additions and 18 deletions.
diff --git a/common/src/main/java/feast/common/models/Feature.java b/common/src/main/java/feast/common/models/Feature.java
@@ -22,19 +22,32 @@ public class Feature {
 
   /**
    * Accepts FeatureReference object and returns its reference in String
+   * "featureset_name:feature_name".
+   *
+   * @param featureReference {@link FeatureReference}
+   * @return String format of FeatureReference
+   */
+  public static String getFeatureStringRef(FeatureReference featureReference) {
+    String ref = featureReference.getName();
+    if (!featureReference.getFeatureSet().isEmpty()) {
+      ref = featureReference.getFeatureSet() + ":" + ref;
+    }
+    return ref;
+  }
+
+  /**
+   * Accepts FeatureReference object and returns its reference with project included in String, eg.
    * "project/featureset_name:feature_name".
    *
    * @param featureReference {@link FeatureReference}
-   * @param ignoreProject Flag whether to return FeatureReference with project name
    * @return String format of FeatureReference
    */
-  public static String getFeatureStringRef(
-      FeatureReference featureReference, boolean ignoreProject) {
+  public static String getFeatureStringWithProjectRef(FeatureReference featureReference) {
     String ref = featureReference.getName();
     if (!featureReference.getFeatureSet().isEmpty()) {
       ref = featureReference.getFeatureSet() + ":" + ref;
     }
-    if (!featureReference.getProject().isEmpty() && !ignoreProject) {
+    if (!featureReference.getProject().isEmpty()) {
       ref = featureReference.getProject() + "/" + ref;
     }
     return ref;

diff --git a/common/src/test/java/feast/common/models/FeaturesTest.java b/common/src/test/java/feast/common/models/FeaturesTest.java
@@ -88,9 +88,8 @@ public void shouldReturnFeatureStringRef() {
             .setName(featureSetSpec.getFeatures(0).getName())
             .build();
 
-    String actualFeatureStringRef = Feature.getFeatureStringRef(featureReference, false);
-    String actualFeatureIgnoreProjectStringRef =
-        Feature.getFeatureStringRef(featureReference, true);
+    String actualFeatureStringRef = Feature.getFeatureStringWithProjectRef(featureReference);
+    String actualFeatureIgnoreProjectStringRef = Feature.getFeatureStringRef(featureReference);
     String expectedFeatureStringRef = "project1/featureSetWithConstraints:feature1";
     String expectedFeatureIgnoreProjectStringRef = "featureSetWithConstraints:feature1";
 

diff --git a/sdk/java/src/main/java/com/gojek/feast/FeastClient.java b/sdk/java/src/main/java/com/gojek/feast/FeastClient.java
@@ -151,7 +151,7 @@ public List<Row> getOnlineFeatures(
                   // Strip project from string Feature References from returned from serving
                   FeatureReference featureRef =
                       RequestUtil.parseFeatureRef(fieldName, true).build();
-                  stripFieldName = Feature.getFeatureStringRef(featureRef, true);
+                  stripFieldName = Feature.getFeatureStringRef(featureRef);
                   row.set(
                       stripFieldName,
                       fieldValues.getFieldsMap().get(fieldName),

diff --git a/sdk/java/src/test/java/com/gojek/feast/RequestUtilTest.java b/sdk/java/src/test/java/com/gojek/feast/RequestUtilTest.java
@@ -76,9 +76,7 @@ void renderFeatureRef_ShouldReturnFeatureRefString(
             .map(ref -> ref.toBuilder().clearProject().build())
             .collect(Collectors.toList());
     List<String> actual =
-        input.stream()
-            .map(ref -> Feature.getFeatureStringRef(ref, true))
-            .collect(Collectors.toList());
+        input.stream().map(ref -> Feature.getFeatureStringRef(ref)).collect(Collectors.toList());
     assertEquals(expected.size(), actual.size());
     for (int i = 0; i < expected.size(); i++) {
       assertEquals(expected.get(i), actual.get(i));

diff --git a/serving/src/main/java/feast/serving/service/OnlineServingService.java b/serving/src/main/java/feast/serving/service/OnlineServingService.java
@@ -175,7 +175,7 @@ private static Map<String, Value> unpackValueMap(
                   Collectors.toMap(
                       featureRowField -> {
                         FeatureReference featureRef = nameRefMap.get(featureRowField.getName());
-                        return Feature.getFeatureStringRef(featureRef, false);
+                        return Feature.getFeatureStringWithProjectRef(featureRef);
                       },
                       featureRowField -> {
                         // drop feature values with an age outside feature set's max age.
@@ -188,7 +188,7 @@ private static Map<String, Value> unpackValueMap(
     // create empty values for features specified in request but not present in feature row.
     Set<String> missingFeatures =
         nameRefMap.values().stream()
-            .map(ref -> Feature.getFeatureStringRef(ref, false))
+            .map(ref -> Feature.getFeatureStringWithProjectRef(ref))
             .collect(Collectors.toSet());
     missingFeatures.removeAll(valueMap.keySet());
     missingFeatures.forEach(refString -> valueMap.put(refString, Value.newBuilder().build()));

diff --git a/serving/src/main/java/feast/serving/specs/CachedSpecService.java b/serving/src/main/java/feast/serving/specs/CachedSpecService.java
@@ -16,7 +16,7 @@
  */
 package feast.serving.specs;
 
-import static feast.common.models.Feature.getFeatureStringRef;
+import static feast.common.models.Feature.getFeatureStringWithProjectRef;
 import static feast.common.models.FeatureSet.getFeatureSetStringRef;
 import static java.util.stream.Collectors.groupingBy;
 
@@ -117,18 +117,18 @@ public List<FeatureSetRequest> getFeatureSets(List<FeatureReference> featureRefe
             featureReference -> {
               // map feature reference to coresponding feature set name
               String fsName =
-                  featureToFeatureSetMapping.get(getFeatureStringRef(featureReference, false));
+                  featureToFeatureSetMapping.get(getFeatureStringWithProjectRef(featureReference));
               if (fsName == null) {
                 throw new SpecRetrievalException(
                     String.format(
                         "Unable to find Feature Set for the given Feature Reference: %s",
-                        getFeatureStringRef(featureReference, false)));
+                        getFeatureStringWithProjectRef(featureReference)));
               } else if (fsName == FEATURE_SET_CONFLICT_FLAG) {
                 throw new SpecRetrievalException(
                     String.format(
                         "Given Feature Reference is amibigous as it matches multiple Feature Sets: %s."
                             + "Please specify a more specific Feature Reference (ie specify the project or feature set)",
-                        getFeatureStringRef(featureReference, false)));
+                        getFeatureStringWithProjectRef(featureReference)));
               }
               return Pair.of(fsName, featureReference);
             })
@@ -291,6 +291,6 @@ private Pair<String, String> generateFeatureToFeatureSetMapping(
       featureRef = featureRef.clearFeatureSet();
     }
     return Pair.of(
-        getFeatureStringRef(featureRef.build(), false), getFeatureSetStringRef(featureSetSpec));
+        getFeatureStringWithProjectRef(featureRef.build()), getFeatureSetStringRef(featureSetSpec));
   }
 }