From 873f6edd66b49fd9d533090c6c6fbb9e3976b9d9 Mon Sep 17 00:00:00 2001 From: Terence Date: Mon, 22 Jun 2020 14:21:11 +0800 Subject: [PATCH] Refactor common module Feature ref --- .../java/feast/common/models/Feature.java | 21 +++++++++++++++---- .../feast/common/models/FeaturesTest.java | 5 ++--- .../java/com/gojek/feast/FeastClient.java | 2 +- .../java/com/gojek/feast/RequestUtilTest.java | 4 +--- .../serving/service/OnlineServingService.java | 4 ++-- .../serving/specs/CachedSpecService.java | 10 ++++----- 6 files changed, 28 insertions(+), 18 deletions(-) diff --git a/common/src/main/java/feast/common/models/Feature.java b/common/src/main/java/feast/common/models/Feature.java index 413193b79aa..1d7fc43ba3c 100644 --- 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 index 10d62878bad..a4426ad21c0 100644 --- 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 index 00e0541e7c3..a7f24e5282b 100644 --- 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 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 index 0b34a4d4bb3..11e15c6cec3 100644 --- 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 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 index dbf15877c5f..37b3e2bd707 100644 --- 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 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 unpackValueMap( // create empty values for features specified in request but not present in feature row. Set 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 index ed5458d974a..b479eb35cd7 100644 --- 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 getFeatureSets(List 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 generateFeatureToFeatureSetMapping( featureRef = featureRef.clearFeatureSet(); } return Pair.of( - getFeatureStringRef(featureRef.build(), false), getFeatureSetStringRef(featureSetSpec)); + getFeatureStringWithProjectRef(featureRef.build()), getFeatureSetStringRef(featureSetSpec)); } }