Make Projects optional & Update Feature References (#693)

* Squash and rebase on master

Squashed commits:
* Update e2e test verify that users can retrieve from multiple featuresets in batch serving
* Add e2e test verify that users can retrieve from multiple featuresets in online serving
* Remove and Reserve version field in FeatureReference in ServingService proto
* Clarify comment in SpecService docs
* Update BQ Retriever to only namespace returned features with featureset only when explictly specified in feature reference.
* Configure BQ Retriever SQL queries to namespace returned feature column with feature set name
* Resolve unresolved rebase markers in Python's SDK client docs
* Squash and Rebase on master
* history:
* Fix batch e2e failing as new SDK does not accept feature refs with projects.
* Fix e2e batch tests: remove rebase markers
* Fix keyerror in end to end tests
* Fix e2e test that still has removed versions
* Correct spelling of omitted in SpecService
* Squash and rebase on version removal PR
* Fix java lint
* Remove comments in JobServiceTest that do not comply with JavaDoc
* Clarify naming of unit test in SpecServiceTest
* Fix python linting
* Refactor AccessManagementService's archiveProject() to use guard clauses
* Update Go SDK to strip the project part of string feature references returned from serving
* Update Java SDK to strip the project part of string feature references returned from serving
* Update Python SDK to strip the project part of string feature references returned from serving
* Fix java unit tests afeter adding check for duplicate feature references
* Remove trailing whitespace to satisfy python lint.
* Fix typo in e2e
* Add check that multiple feature references in get online/batch features don't refer to the same feature
* Fix end to end tests referencing the same feature in different references
* Fix typo in call to get_feature_set in e2e tests.
* Fix issue in e2e test where client persisted wrong project. Should used default instead.
* Remove support of projects in string Feature References in java SDK
* Remove support of projects in string Feature References in go SDK
* Remove support of projects in string Feature References in python SDK
* Fix python SDK linting failures
* Update documentation in CoreService proto to document that field names are unique with a featureset
* Update E2E tests to check default project and updated feature reference functionality
* Fixed Python SDK's FeatureSetRef rendering with missing project
* Make Python's SDK client's archive_project() revert to default project on archive.
* Make Feast Core throw an error when the user attempts to archive the default project
* Apply spotless formatting to java SDK's RequestUtil
* Updated python SDK's client's to parse feature refs without projects and with feature sets
* Update Go SDK buildFeatureRefs() to parse feature refs without projects and with feature sets
* Regenrate go protos for Go SDK using make compile-protos-go
* Changes Java SDK's FeastClient defaultProject to project which overrides project in feature refs.
* Updated Java SDK's RequestUtil to parse string feature refs without project/with feature set.
* Refactor CachedSpecService to support different variations of feature references.
* Update RefUtil's generateFeatureStringRef to FeatureReference with featureset name field
* Add missing final to make core Project models's DEFAULT_NAME into a constant.
* Added feature_set_name field to ServingService's FeatureReference proto
* Remove projects from JobUpdateTask's unit tests
* Remove projects from ServingServiceGRpcControllerTest's unit tests.
* Remove projects from OnlineServingService's unit tests.
* Update CachedSpecService's getFeatureToFeatureSetMapping() to match Feature References with no project.
* Update RefUtil's generateFeatureStringRef() to properly render refs without a project
* Document in CoreService proto that specifying project in list, get, update featureset is optional
* Update SpecService's applyFeatureSet() to auto default project if not specified.
* Update SpecService's listFeatureSets() to autofill default project if project unspecified.
* Remove comment as FeatureSet's id is no longer a String.
* Update SpecService's getFeatureSet() to autofill default project if project unspecified.
* Config AccessManagementService to create default project in constructor
* Document that FeatureReference's project field is optional.
* Allow CachedSpecService's getFeatureSets() to match FeatureReferences with no project by autofilling default project
* Make CachedSpecService's featureSetCacheLoader a local var instead of private property as its only used once.

* Change SpecServiceTest to use global default project constant.

* Make AccessManagementService's archiveProject() throw UnimplementOperation error when trying to archive default project.

* Correct incorrect documentation on source being ignored on Core's ApplyFeatureSet

* Use ValueProto.Value in java SDK's FeastClient lambda instead of generic Object

* Clarify how the ignore project param of feature ref to string() worked in docs in SDKs

* Correct missing test to check for empty feature refs in java SDK.

* Clarified ref_protos to feature_ref_protos to make code clearer

* Rename ServingService's FeatureReference proto 'feature_set_name` to `feature_set`

* Remove nested function strip_project() with loop with python SDK's client's get_online_features()

* Remove extra version paramter in python sdk test's TestFeatureRef

* Change CachedSpecService's "hasProject == true" to more concise "!hasProject"

* Removed setFeatureSet() calls to OnlineServingServiceTest as it does not need to be set in the unit test

* Make client.set_project() without args reset project to default in python SDK

* Updated e2e to test for feature set inference for feature ref without feature set specified

* Remove unused field max_age in FeatureReference

* Fix typo in e2e

* Fix error in e2e tests regarding string splits

* Optimise Serving's CachedSpecService populateCache() by directly assigning map.

* Remove final in CachedSpecService to allow assignment in featureToFeatureSetMapping

* Remove unnescessary copy of list in QueryTemplater

Co-authored-by: Zhu Zhanyan <zhu.zhanyan@gojek.com>

core/src/main/java/feast/core/grpc/CoreServiceImpl.java

@@ -183,6 +183,17 @@ public void archiveProject(
       accessManagementService.archiveProject(request.getName());
       responseObserver.onNext(ArchiveProjectResponse.getDefaultInstance());
       responseObserver.onCompleted();
+    } catch (IllegalArgumentException e) {
+      log.error("Recieved an invalid request on calling archiveProject method:", e);
+      responseObserver.onError(
+          Status.INVALID_ARGUMENT
+              .withDescription(e.getMessage())
+              .withCause(e)
+              .asRuntimeException());
+    } catch (UnsupportedOperationException e) {
+      log.error("Attempted to archive an unsupported project:", e);
+      responseObserver.onError(
+          Status.UNIMPLEMENTED.withDescription(e.getMessage()).withCause(e).asRuntimeException());
     } catch (Exception e) {
       log.error("Exception has occurred in the createProject method: ", e);
       responseObserver.onError(

core/src/main/java/feast/core/model/FeatureSet.java

@@ -39,7 +39,6 @@
     uniqueConstraints = @UniqueConstraint(columnNames = {"name", "project_name"}))
 public class FeatureSet extends AbstractTimestampEntity {
 
-  // Id of the featureSet, defined as project/feature_set_name:feature_set_version
   @Id @GeneratedValue private long id;
 
   // Name of the featureSet

core/src/main/java/feast/core/service/AccessManagementService.java

@@ -28,12 +28,15 @@
 @Slf4j
 @Service
 public class AccessManagementService {
-
   private ProjectRepository projectRepository;
 
   @Autowired
   public AccessManagementService(ProjectRepository projectRepository) {
     this.projectRepository = projectRepository;
+    // create default project if it does not yet exist.
+    if (!projectRepository.existsById(Project.DEFAULT_NAME)) {
+      this.createProject(Project.DEFAULT_NAME);
+    }
   }
 
   /**
@@ -61,6 +64,9 @@ public void archiveProject(String name) {
     if (!project.isPresent()) {
       throw new IllegalArgumentException(String.format("Could not find project: \"%s\"", name));
     }
+    if (name.equals(Project.DEFAULT_NAME)) {
+      throw new UnsupportedOperationException("Archiving the default project is not allowed.");
+    }
     Project p = project.get();
     p.setArchived(true);
     projectRepository.saveAndFlush(p);

core/src/main/java/feast/core/service/SpecService.java

@@ -80,7 +80,8 @@ public SpecService(
   /**
    * Get a feature set matching the feature name and version and project. The feature set name and
    * project are required, but version can be omitted by providing 0 for its value. If the version
-   * is omitted, the latest feature set will be provided.
+   * is omitted, the latest feature set will be provided. If the project is omitted, the default
+   * would be used.
    *
    * @param request: GetFeatureSetRequest Request containing filter parameters.
    * @return Returns a GetFeatureSetResponse containing a feature set..
@@ -94,8 +95,9 @@ public GetFeatureSetResponse getFeatureSet(GetFeatureSetRequest request)
     if (request.getName().isEmpty()) {
       throw new IllegalArgumentException("No feature set name provided");
     }
+    // Autofill default project if project is not specified
     if (request.getProject().isEmpty()) {
-      throw new IllegalArgumentException("No project provided");
+      request = request.toBuilder().setProject(Project.DEFAULT_NAME).build();
     }
 
     FeatureSet featureSet;
@@ -117,7 +119,8 @@ public GetFeatureSetResponse getFeatureSet(GetFeatureSetRequest request)
    * projects.
    *
    * <p>Project name can be explicitly provided, or an asterisk can be provided to match all
-   * projects. It is not possible to provide a combination of asterisks/wildcards and text.
+   * projects. It is not possible to provide a combination of asterisks/wildcards and text. If the
+   * project name is omitted, the default project would be used.
    *
    * <p>The feature set name in the filter accepts an asterisk as a wildcard. All matching feature
    * sets will be returned. Regex is not supported. Explicitly defining a feature set name is not
@@ -131,14 +134,19 @@ public ListFeatureSetsResponse listFeatureSets(ListFeatureSetsRequest.Filter fil
     String name = filter.getFeatureSetName();
     String project = filter.getProject();
 
-    if (project.isEmpty() || name.isEmpty()) {
+    if (name.isEmpty()) {
       throw new IllegalArgumentException(
-          "Invalid listFeatureSetRequest, missing arguments. Must provide project and feature set name.");
+          "Invalid listFeatureSetRequest, missing arguments. Must provide feature set name:");
     }
 
     checkValidCharactersAllowAsterisk(name, "featureSetName");
     checkValidCharactersAllowAsterisk(project, "projectName");
 
+    // Autofill default project if project not specified
+    if (project.isEmpty()) {
+      project = Project.DEFAULT_NAME;
+    }
+
     List<FeatureSet> featureSets = new ArrayList<FeatureSet>() {};
 
     if (project.contains("*")) {
@@ -227,12 +235,21 @@ public ListStoresResponse listStores(ListStoresRequest.Filter filter) {
    *
    * <p>This function is idempotent. If no changes are detected in the incoming featureSet's schema,
    * this method will update the incoming featureSet spec with the latest version stored in the
-   * repository, and return that.
+   * repository, and return that. If project is not specified in the given featureSet, will assign
+   * the featureSet to the'default' project.
    *
    * @param newFeatureSet Feature set that will be created or updated.
    */
   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);

core/src/test/java/feast/core/job/JobUpdateTaskTest.java

@@ -53,8 +53,7 @@ public class JobUpdateTaskTest {
 
   private static final FeatureSetProto.FeatureSet.Builder fsBuilder =
       FeatureSetProto.FeatureSet.newBuilder().setMeta(FeatureSetMeta.newBuilder());
-  private static final FeatureSetSpec.Builder specBuilder =
-      FeatureSetSpec.newBuilder().setProject("project1");
+  private static final FeatureSetSpec.Builder specBuilder = FeatureSetSpec.newBuilder();
 
   @Mock private JobManager jobManager;
 

core/src/test/java/feast/core/service/AccessManagementServiceTest.java

@@ -0,0 +1,74 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ * Copyright 2018-2019 The Feast Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package feast.core.service;
+
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+import static org.mockito.MockitoAnnotations.initMocks;
+
+import feast.core.dao.ProjectRepository;
+import feast.core.model.Project;
+import java.util.Optional;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.mockito.Mock;
+
+public class AccessManagementServiceTest {
+  @Rule public ExpectedException expectedException = ExpectedException.none();
+  // mocks
+  @Mock private ProjectRepository projectRepository;
+  // dummy models
+  private Project defaultProject;
+  private Project testProject;
+
+  // test target
+  private AccessManagementService accessService;
+
+  @Before
+  public void setup() {
+    initMocks(this);
+    // setup dummy models for testing
+    this.defaultProject = new Project(Project.DEFAULT_NAME);
+    this.testProject = new Project("project");
+    // setup test target
+    when(this.projectRepository.existsById(Project.DEFAULT_NAME)).thenReturn(false);
+    this.accessService = new AccessManagementService(this.projectRepository);
+  }
+
+  @Test
+  public void testDefaultProjectCreateInConstructor() {
+    verify(this.projectRepository).saveAndFlush(this.defaultProject);
+  }
+
+  @Test
+  public void testArchiveProject() {
+    when(this.projectRepository.findById("project")).thenReturn(Optional.of(this.testProject));
+    this.accessService.archiveProject("project");
+    this.testProject.setArchived(true);
+    verify(this.projectRepository).saveAndFlush(this.testProject);
+    // reset archived flag
+    this.testProject.setArchived(false);
+  }
+
+  @Test
+  public void shouldNotArchiveDefaultProject() {
+    expectedException.expect(IllegalArgumentException.class);
+    this.accessService.archiveProject(Project.DEFAULT_NAME);
+  }
+}

core/src/test/java/feast/core/service/JobServiceTest.java

@@ -69,7 +69,6 @@ public class JobServiceTest {
   // test target
   public JobService jobService;
 
-  /* unit test setup */
   @Before
   public void setup() {
     initMocks(this);
@@ -107,7 +106,6 @@ public void setup() {
         new JobService(this.jobRepository, this.specService, Arrays.asList(this.jobManager));
   }
 
-  // setup fake spec service
   public void setupSpecService() {
     try {
       ListFeatureSetsResponse response =
@@ -124,7 +122,6 @@ public void setupSpecService() {
     }
   }
 
-  // setup fake job repository
   public void setupJobRepository() {
     when(this.jobRepository.findById(this.job.getId())).thenReturn(Optional.of(this.job));
     when(this.jobRepository.findByStoreName(this.dataStore.getName()))
@@ -134,14 +131,12 @@ public void setupJobRepository() {
     when(this.jobRepository.findAll()).thenReturn(Arrays.asList(this.job));
   }
 
-  // TODO: setup fake job manager
   public void setupJobManager() {
     when(this.jobManager.getRunnerType()).thenReturn(Runner.DATAFLOW);
     when(this.jobManager.restartJob(this.job))
         .thenReturn(this.newDummyJob(this.job.getId(), this.job.getExtId(), JobStatus.PENDING));
   }
 
-  // dummy model constructorss
   private FeatureSet newDummyFeatureSet(String name, int version, String project) {
     Feature feature = TestObjectFactory.CreateFeature(name + "_feature", Enum.INT64);
     Entity entity = TestObjectFactory.CreateEntity(name + "_entity", Enum.STRING);
@@ -203,7 +198,6 @@ private List<ListFeatureSetsRequest.Filter> newDummyListRequestFilters() {
             .build());
   }
 
-  /* unit tests */
   private ListIngestionJobsResponse tryListJobs(ListIngestionJobsRequest request) {
     ListIngestionJobsResponse response = null;
     try {
@@ -216,7 +210,6 @@ private ListIngestionJobsResponse tryListJobs(ListIngestionJobsRequest request)
     return response;
   }
 
-  // list jobs
   @Test
   public void testListJobsById() {
     ListIngestionJobsRequest.Filter filter =
@@ -275,7 +268,6 @@ public void testListIngestionJobByFeatureSetReference() {
     assertThat(this.tryListJobs(request).getJobs(0), equalTo(this.ingestionJob));
   }
 
-  // stop jobs
   private StopIngestionJobResponse tryStopJob(
       StopIngestionJobRequest request, boolean expectError) {
     StopIngestionJobResponse response = null;

core/src/test/java/feast/core/service/SpecServiceTest.java

@@ -1,6 +1,6 @@
 /*
  * SPDX-License-Identifier: Apache-2.0
- * Copyright 2018-2019 The Feast Authors
+ * Copyright 2018-2020 The Feast Authors
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -34,6 +34,7 @@
 import feast.proto.core.CoreServiceProto.ApplyFeatureSetResponse;
 import feast.proto.core.CoreServiceProto.ApplyFeatureSetResponse.Status;
 import feast.proto.core.CoreServiceProto.GetFeatureSetRequest;
+import feast.proto.core.CoreServiceProto.GetFeatureSetResponse;
 import feast.proto.core.CoreServiceProto.ListFeatureSetsRequest.Filter;
 import feast.proto.core.CoreServiceProto.ListFeatureSetsResponse;
 import feast.proto.core.CoreServiceProto.ListStoresRequest;
@@ -105,11 +106,13 @@ public void setUp() {
     Feature f3f1 = TestObjectFactory.CreateFeature("f3f1", Enum.INT64);
     Feature f3f2 = TestObjectFactory.CreateFeature("f3f2", Enum.INT64);
     Entity f3e1 = TestObjectFactory.CreateEntity("f3e1", Enum.STRING);
-    FeatureSet featureSet3v1 =
+    FeatureSet featureSet3 =
         TestObjectFactory.CreateFeatureSet(
             "f3", "project1", Arrays.asList(f3e1), Arrays.asList(f3f2, f3f1));
 
-    featureSets = Arrays.asList(featureSet1, featureSet2);
+    FeatureSet featureSet4 = newDummyFeatureSet("f4", Project.DEFAULT_NAME);
+    featureSets = Arrays.asList(featureSet1, featureSet2, featureSet3, featureSet4);
+
     when(featureSetRepository.findAll()).thenReturn(featureSets);
     when(featureSetRepository.findAllByOrderByNameAsc()).thenReturn(featureSets);
     when(featureSetRepository.findFeatureSetByNameAndProject_Name("f1", "project1"))
@@ -160,15 +163,6 @@ public void shouldGetAllFeatureSetsIfOnlyWildcardsProvided()
     assertThat(actual, equalTo(expected));
   }
 
-  @Test
-  public void listFeatureSetShouldFailIfFeatureSetProvidedWithoutProject()
-      throws InvalidProtocolBufferException {
-    expectedException.expect(IllegalArgumentException.class);
-    expectedException.expectMessage(
-        "Invalid listFeatureSetRequest, missing arguments. Must provide project and feature set name.");
-    specService.listFeatureSets(Filter.newBuilder().setFeatureSetName("f1").build());
-  }
-
   @Test
   public void shouldGetAllFeatureSetsMatchingNameWithWildcardSearch()
       throws InvalidProtocolBufferException {
@@ -511,6 +505,26 @@ public void applyFeatureSetShouldCreateProjectWhenNotAlreadyExists()
         equalTo(incomingFeatureSet.getSpec().getProject()));
   }
 
+  @Test
+  public void applyFeatureSetShouldUsedDefaultProjectIfUnspecified()
+      throws InvalidProtocolBufferException {
+    Feature f3f1 = TestObjectFactory.CreateFeature("f3f1", Enum.INT64);
+    Feature f3f2 = TestObjectFactory.CreateFeature("f3f2", Enum.INT64);
+    Entity f3e1 = TestObjectFactory.CreateEntity("f3e1", Enum.STRING);
+
+    // In protov3, unspecified project defaults to ""
+    FeatureSetProto.FeatureSet incomingFeatureSet =
+        TestObjectFactory.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));
+  }
+
   @Test
   public void applyFeatureSetShouldFailWhenProjectIsArchived()
       throws InvalidProtocolBufferException {
@@ -661,10 +675,20 @@ public void shouldDoNothingIfNoChange() throws InvalidProtocolBufferException {
   }
 
   @Test
-  public void shouldFailIfGetFeatureSetWithoutProject() throws InvalidProtocolBufferException {
-    expectedException.expect(IllegalArgumentException.class);
-    expectedException.expectMessage("No project provided");
-    specService.getFeatureSet(GetFeatureSetRequest.newBuilder().setName("f1").build());
+  public void getOrListFeatureSetShouldUseDefaultProjectIfProjectUnspecified()
+      throws InvalidProtocolBufferException {
+    when(featureSetRepository.findFeatureSetByNameAndProject_Name("f4", Project.DEFAULT_NAME))
+        .thenReturn(featureSets.get(3));
+    FeatureSet expected = featureSets.get(3);
+    // check getFeatureSet()
+    GetFeatureSetResponse getResponse =
+        specService.getFeatureSet(GetFeatureSetRequest.newBuilder().setName("f4").build());
+    assertThat(getResponse.getFeatureSet(), equalTo(expected.toProto()));
+
+    // check listFeatureSets()
+    ListFeatureSetsResponse listResponse =
+        specService.listFeatureSets(Filter.newBuilder().setFeatureSetName("f4").build());
+    assertThat(listResponse.getFeatureSetsList(), equalTo(Arrays.asList(expected.toProto())));
   }
 
   private FeatureSet newDummyFeatureSet(String name, String project) {