fix: apply feature set should authorize default project if project is…

… not set.
feast-dev · Aug 3, 2020 · 041083a · 041083a
1 parent 3590339
commit 041083a
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 19 deletions.
diff --git a/core/src/main/java/feast/core/grpc/CoreServiceImpl.java b/core/src/main/java/feast/core/grpc/CoreServiceImpl.java
@@ -30,6 +30,7 @@
 import feast.core.service.StatsService;
 import feast.proto.core.CoreServiceGrpc.CoreServiceImplBase;
 import feast.proto.core.CoreServiceProto.*;
+import feast.proto.core.FeatureSetProto.FeatureSet;
 import io.grpc.Status;
 import io.grpc.StatusRuntimeException;
 import io.grpc.stub.StreamObserver;
@@ -187,9 +188,10 @@ public void applyFeatureSet(
     String projectId = null;
 
     try {
-      projectId = request.getFeatureSet().getSpec().getProject();
+      FeatureSet featureSet = specService.imputeProjectName(request.getFeatureSet());
+      projectId = featureSet.getSpec().getProject();
       authorizationService.authorizeRequest(SecurityContextHolder.getContext(), projectId);
-      ApplyFeatureSetResponse response = specService.applyFeatureSet(request.getFeatureSet());
+      ApplyFeatureSetResponse response = specService.applyFeatureSet(featureSet);
       responseObserver.onNext(response);
       responseObserver.onCompleted();
     } catch (org.hibernate.exception.ConstraintViolationException e) {

diff --git a/core/src/main/java/feast/core/service/SpecService.java b/core/src/main/java/feast/core/service/SpecService.java
@@ -312,15 +312,6 @@ public ListStoresResponse listStores(ListStoresRequest.Filter filter) {
   @Transactional
   public ApplyFeatureSetResponse applyFeatureSet(FeatureSetProto.FeatureSet newFeatureSet)
       throws InvalidProtocolBufferException {
-    // Autofill default project if not specified
-    if (newFeatureSet.getSpec().getProject().isEmpty()) {
-      newFeatureSet =
-          newFeatureSet
-              .toBuilder()
-              .setSpec(newFeatureSet.getSpec().toBuilder().setProject(Project.DEFAULT_NAME).build())
-              .build();
-    }
-
     // Validate incoming feature set
     FeatureSetValidator.validateSpec(newFeatureSet);
 
@@ -383,6 +374,21 @@ public ApplyFeatureSetResponse applyFeatureSet(FeatureSetProto.FeatureSet newFea
         .build();
   }
 
+  /**
+   * sets project to 'default' if project is not specified in featureSet
+   *
+   * @param featureSet Feature set which needs to be imputed with default project.
+   */
+  public FeatureSetProto.FeatureSet imputeProjectName(FeatureSetProto.FeatureSet featureSet) {
+    if (featureSet.getSpec().getProject().isEmpty()) {
+      return featureSet
+          .toBuilder()
+          .setSpec(featureSet.getSpec().toBuilder().setProject(Project.DEFAULT_NAME).build())
+          .build();
+    }
+    return featureSet;
+  }
+
   /**
    * UpdateStore updates the repository with the new given store.
    *

diff --git a/core/src/test/java/feast/core/auth/CoreServiceAuthTest.java b/core/src/test/java/feast/core/auth/CoreServiceAuthTest.java
@@ -102,6 +102,9 @@ public void shouldNotApplyFeatureSetIfNotProjectMember() throws InvalidProtocolB
 
     StreamRecorder<ApplyFeatureSetResponse> responseObserver = StreamRecorder.create();
     FeatureSetProto.FeatureSet incomingFeatureSet = newDummyFeatureSet("f2", 1, project).toProto();
+    doReturn(incomingFeatureSet)
+        .when(specService)
+        .imputeProjectName(any(FeatureSetProto.FeatureSet.class));
     FeatureSetProto.FeatureSetSpec incomingFeatureSetSpec =
         incomingFeatureSet.getSpec().toBuilder().build();
     FeatureSetProto.FeatureSet spec =

diff --git a/core/src/test/java/feast/core/service/SpecServiceTest.java b/core/src/test/java/feast/core/service/SpecServiceTest.java
@@ -606,7 +606,7 @@ public void applyFeatureSetShouldCreateProjectWhenNotAlreadyExists()
   }
 
   @Test
-  public void applyFeatureSetShouldUsedDefaultProjectIfUnspecified()
+  public void imputeProjectNameShouldSetDefaultProjectIfUnspecified()
       throws InvalidProtocolBufferException {
     Feature f3f1 = TestUtil.CreateFeature("f3f1", Enum.INT64);
     Feature f3f2 = TestUtil.CreateFeature("f3f2", Enum.INT64);
@@ -616,13 +616,9 @@ public void applyFeatureSetShouldUsedDefaultProjectIfUnspecified()
     FeatureSetProto.FeatureSet incomingFeatureSet =
         TestUtil.CreateFeatureSet("f3", "", Arrays.asList(f3e1), Arrays.asList(f3f2, f3f1))
             .toProto();
-    ApplyFeatureSetResponse applyFeatureSetResponse =
-        specService.applyFeatureSet(incomingFeatureSet);
-    assertThat(applyFeatureSetResponse.getStatus(), equalTo(Status.CREATED));
-
-    assertThat(
-        applyFeatureSetResponse.getFeatureSet().getSpec().getProject(),
-        equalTo(Project.DEFAULT_NAME));
+    FeatureSetProto.FeatureSet imputedFeatureSet =
+        specService.imputeProjectName(incomingFeatureSet);
+    assertThat(imputedFeatureSet.getSpec().getProject(), equalTo(Project.DEFAULT_NAME));
   }
 
   @Test