◐ Shell
reader mode source ↗
Skip to content
Merged

Add more validation in feature spec registration #11

Show file tree
Hide file tree
Changes from all commits
File filter
Conversations
Jump to
Diff view
Apply and reload
Show whitespace
Diff view
Apply and reload
52 changes: 35 additions & 17 deletions core/src/main/java/feast/core/validators/SpecValidator.java
View file Edit file Delete file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,14 @@

package feast.core.validators;

import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
import static feast.core.validators.Matchers.checkLowerSnakeCase;

import com.google.common.base.Strings;
import com.google.common.collect.Lists;
import feast.core.dao.EntityInfoRepository;
import feast.core.dao.FeatureGroupInfoRepository;
import feast.core.dao.FeatureInfoRepository;
import feast.core.dao.StorageInfoRepository;
import feast.core.model.FeatureGroupInfo;
import feast.core.storage.BigQueryStorageManager;
import feast.core.storage.BigTableStorageManager;
import feast.core.storage.PostgresStorageManager;
Expand All @@ -38,9 +35,16 @@
import feast.specs.ImportSpecProto.Field;
import feast.specs.ImportSpecProto.ImportSpec;
import feast.specs.StorageSpecProto.StorageSpec;
import java.util.Arrays;
import java.util.Optional;
import org.springframework.beans.factory.annotation.Autowired;

public class SpecValidator {

Expand All @@ -50,14 +54,14 @@ public class SpecValidator {
private FeatureInfoRepository featureInfoRepository;
private static final String FILE_ERROR_STORE_TYPE = "file.json";

private String[] supportedStorageTypes =
new String[]{
BigQueryStorageManager.TYPE,
BigTableStorageManager.TYPE,
PostgresStorageManager.TYPE,
RedisStorageManager.TYPE,
BigQueryStorageManager.TYPE,
FILE_ERROR_STORE_TYPE
};

@Autowired
Expand Down Expand Up @@ -126,12 +130,21 @@ public void validateFeatureSpec(FeatureSpec spec) throws IllegalArgumentExceptio
warehouseStoreId =
warehouseStoreId.equals("") ? group.getWarehouseStore().getId() : warehouseStoreId;
}
checkArgument(
storageInfoRepository.existsById(servingStoreId),
Strings.lenientFormat("Serving store with id %s does not exist", servingStoreId));
checkArgument(
storageInfoRepository.existsById(warehouseStoreId),
Strings.lenientFormat("Warehouse store with id %s does not exist", warehouseStoreId));

} catch (NullPointerException | IllegalArgumentException e) {
throw new IllegalArgumentException(
Expand Down Expand Up @@ -186,7 +199,12 @@ public void validateStorageSpec(StorageSpec spec) throws IllegalArgumentExceptio
try {
checkArgument(!spec.getId().equals(""), "Id field cannot be empty");
Matchers.checkUpperSnakeCase(spec.getId(), "Id");
checkArgument(Arrays.asList(supportedStorageTypes).contains(spec.getType()));
} catch (NullPointerException | IllegalArgumentException e) {
throw new IllegalArgumentException(
Strings.lenientFormat(
Expand Down Expand Up @@ -275,4 +293,4 @@ private void checkBigqueryImportSpecOption(ImportSpec spec) throws IllegalArgume
Strings.lenientFormat("Invalid options: %s", e.getMessage()));
}
}
}
114 changes: 105 additions & 9 deletions core/src/test/java/feast/core/validators/SpecValidatorTest.java
View file Edit file Delete file
Open in desktop
Original file line number Diff line number Diff line change
@@ -17,6 +17,9 @@

package feast.core.validators;

import feast.core.dao.EntityInfoRepository;
import feast.core.dao.FeatureGroupInfoRepository;
import feast.core.dao.FeatureInfoRepository;
Expand All @@ -33,16 +36,13 @@
import feast.specs.ImportSpecProto.Schema;
import feast.specs.StorageSpecProto.StorageSpec;
import feast.types.GranularityProto.Granularity;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
import org.mockito.Mockito;

import java.util.Optional;

import static org.mockito.Mockito.when;

public class SpecValidatorTest {
private FeatureInfoRepository featureInfoRepository;
private FeatureGroupInfoRepository featureGroupInfoRepository;
Expand Down Expand Up @@ -329,7 +329,9 @@ public void featureSpecWithoutServingStoreShouldInheritServingStoreIdFromGroup()
FeatureGroupInfo fgi = new FeatureGroupInfo();
StorageInfo redis1 = new StorageInfo();
redis1.setId("REDIS1");
fgi.setServingStore(redis1);
when(featureGroupInfoRepository.existsById("group")).thenReturn(true);
when(featureGroupInfoRepository.findById("group")).thenReturn(Optional.of(fgi));
SpecValidator validator =
Expand Up @@ -358,17 +360,25 @@ public void featureSpecWithoutServingStoreShouldInheritServingStoreIdFromGroup()

@Test
public void featureSpecWithoutExistingWarehouseStoreShouldThrowIllegalArgumentException() {
when(entityInfoRepository.existsById("entity")).thenReturn(true);
when(storageInfoRepository.existsById("REDIS1")).thenReturn(true);
when(storageInfoRepository.existsById("REDIS2")).thenReturn(false);
SpecValidator validator =
new SpecValidator(
storageInfoRepository,
entityInfoRepository,
featureGroupInfoRepository,
featureInfoRepository);
DataStore servingStore = DataStore.newBuilder().setId("REDIS1").build();
DataStore warehouseStore = DataStore.newBuilder().setId("REDIS2").build();
DataStores dataStores =
DataStores.newBuilder().setServing(servingStore).setWarehouse(warehouseStore).build();
FeatureSpec input =
Expand All @@ -382,7 +392,93 @@ public void featureSpecWithoutExistingWarehouseStoreShouldThrowIllegalArgumentEx
.setDataStores(dataStores)
.build();
exception.expect(IllegalArgumentException.class);
exception.expectMessage("Warehouse store with id REDIS2 does not exist");
validator.validateFeatureSpec(input);
}

Expand Down
Toggle all file notes Toggle all file annotations