Skip to content

Commit

Permalink
fix bug structured validation on reapply (#27)
Browse files Browse the repository at this point in the history
Fix bug in validating structured indexes where empty `dependent_fields` and `features` in the `allFields` are not recognized as null when refreshing state
  • Loading branch information
RaynorChavez authored Jul 23, 2024
1 parent cb6b015 commit 59e666a
Show file tree
Hide file tree
Showing 4 changed files with 142 additions and 33 deletions.
4 changes: 2 additions & 2 deletions examples/create_structured_index/create_dependent.tf
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ resource "marqo_index" "example" {
"name" : "multimodal_field",
"type" : "multimodal_combination",
"dependent_fields" : {
"image_field" : 0.8,
"text_field" : 0.1
"imageField" : 0.8,
"textField" : 0.1
},
},
],
Expand Down
6 changes: 3 additions & 3 deletions internal/provider/indices_datasource.go
Original file line number Diff line number Diff line change
Expand Up @@ -285,9 +285,9 @@ func (d *indicesDataSource) Schema(_ context.Context, _ datasource.SchemaRequest
func ConvertMarqoAllFieldInputs(marqoFields []go_marqo.AllFieldInput) []AllFieldInput {
allFieldsConverted := make([]AllFieldInput, len(marqoFields))
for i, field := range marqoFields {
featuresConverted := make([]types.String, len(field.Features))
for j, feature := range field.Features {
featuresConverted[j] = types.StringValue(feature)
featuresConverted := make([]types.String, 0)
for _, feature := range field.Features {
featuresConverted = append(featuresConverted, types.StringValue(feature))
}
dependentFieldsConverted := make(map[string]types.Float64)
for key, value := range field.DependentFields {
Expand Down
73 changes: 45 additions & 28 deletions internal/provider/indices_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,33 +227,32 @@ func StringToInt64(str string) types.Int64 {
func validateAndConstructAllFields(allFieldsInput []AllFieldInput) ([]map[string]interface{}, error) {
var allFields []map[string]interface{}
for _, field := range allFieldsInput {
// Basic validation example. Expand based on full Marqo documentation requirements.
if field.Name.IsNull() || field.Type.IsNull() {
return nil, fmt.Errorf("each field must have a name and type")
}
fieldMap := map[string]interface{}{
"name": field.Name.ValueString(),
"type": field.Type.ValueString(),
"features": []string{}, // Convert types.String to string
"name": field.Name.ValueString(),
"type": field.Type.ValueString(),
"features": []string{},
"dependentFields": map[string]float64{},
}
// Assert the type of "features" before appending
features, ok := fieldMap["features"].([]string)
if !ok {
// Handle the error
features = []string{}
}
for _, feature := range field.Features {
features = append(features, feature.ValueString())

if len(field.Features) > 0 {
features := []string{}
for _, feature := range field.Features {
features = append(features, feature.ValueString())
}
fieldMap["features"] = features
}
fieldMap["features"] = features
fieldMap["dependentFields"] = map[string]float64{}

if len(field.DependentFields) > 0 {
dependentFields := make(map[string]float64)
for key, value := range field.DependentFields {
dependentFields[key] = value.ValueFloat64()
}
fieldMap["dependentFields"] = dependentFields
}

allFields = append(allFields, fieldMap)
}
return allFields, nil
Expand All @@ -264,23 +263,25 @@ func convertAllFieldsToMap(allFieldsInput []AllFieldInput) []map[string]interfac
allFields := []map[string]interface{}{}
for _, field := range allFieldsInput {
fieldMap := map[string]interface{}{
"name": field.Name.ValueString(),
"type": field.Type.ValueString(),
// Add other necessary fields from AllFieldInput struct
"name": field.Name.ValueString(),
"type": field.Type.ValueString(),
"features": []string{},
"dependentFields": map[string]float64{},
}
// Assuming Features is a slice of types.String and needs conversion
features := []string{}
for _, feature := range field.Features {
features = append(features, feature.ValueString())
}
fieldMap["features"] = features

// Convert DependentFields if necessary
dependentFieldsMap := make(map[string]float64)
for key, value := range field.DependentFields {
dependentFieldsMap[key] = value.ValueFloat64()
if len(field.Features) > 0 {
features := []string{}
for _, feature := range field.Features {
features = append(features, feature.ValueString())
}
fieldMap["features"] = features
}
if len(dependentFieldsMap) > 0 {

if len(field.DependentFields) > 0 {
dependentFieldsMap := make(map[string]float64)
for key, value := range field.DependentFields {
dependentFieldsMap[key] = value.ValueFloat64()
}
fieldMap["dependentFields"] = dependentFieldsMap
}

Expand Down Expand Up @@ -384,6 +385,22 @@ func (r *indicesResource) Read(ctx context.Context, req resource.ReadRequest, re
}
}

// Ensure features and dependent_fields are always set
for i := range newState.Settings.AllFields {
if len(newState.Settings.AllFields[i].Features) == 0 {
newState.Settings.AllFields[i].Features = nil
}
if len(newState.Settings.AllFields[i].DependentFields) == 0 {
newState.Settings.AllFields[i].DependentFields = nil
}
}

// Ignore these fields for structured indexes
if newState.Settings.Type.ValueString() == "structured" {
newState.Settings.FilterStringMaxLength = types.Int64Null()
newState.Settings.TreatUrlsAndPointersAsImages = types.BoolNull()
}

// Handle image_preprocessing.patch_method
if newState.Settings.ImagePreprocessing.PatchMethod.ValueString() == "" {
newState.Settings.ImagePreprocessing.PatchMethod = types.StringNull()
Expand Down
92 changes: 92 additions & 0 deletions internal/provider/indices_resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,98 @@ func TestAccResourceIndex(t *testing.T) {
})
}

func TestAccResourceStructuredIndex(t *testing.T) {
structured_index_name := fmt.Sprintf("structured_resource_%s", randomString(8))
resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
ProtoV6ProviderFactories: testAccProtoV6ProviderFactories,
Steps: []resource.TestStep{
// Check if index exists and delete if it does
{
Config: testAccEmptyConfig(),
Check: resource.ComposeTestCheckFunc(
testAccCheckIndexExistsAndDelete(structured_index_name),
),
},
// Create and Read testing
{
Config: testAccResourceStructuredIndexConfig(structured_index_name),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("marqo_index.test", "index_name", structured_index_name),
resource.TestCheckResourceAttr("marqo_index.test", "settings.type", "structured"),
resource.TestCheckResourceAttr("marqo_index.test", "settings.vector_numeric_type", "float"),
resource.TestCheckResourceAttr("marqo_index.test", "settings.model", "open_clip/ViT-L-14/laion2b_s32b_b82k"),
resource.TestCheckResourceAttr("marqo_index.test", "settings.normalize_embeddings", "true"),
resource.TestCheckResourceAttr("marqo_index.test", "settings.inference_type", "marqo.CPU.small"),
resource.TestCheckResourceAttr("marqo_index.test", "settings.number_of_inferences", "1"),
resource.TestCheckResourceAttr("marqo_index.test", "settings.number_of_replicas", "0"),
resource.TestCheckResourceAttr("marqo_index.test", "settings.number_of_shards", "2"),
resource.TestCheckResourceAttr("marqo_index.test", "settings.storage_class", "marqo.basic"),
resource.TestCheckResourceAttr("marqo_index.test", "settings.text_preprocessing.split_length", "2"),
resource.TestCheckResourceAttr("marqo_index.test", "settings.text_preprocessing.split_method", "sentence"),
resource.TestCheckResourceAttr("marqo_index.test", "settings.text_preprocessing.split_overlap", "0"),
resource.TestCheckResourceAttr("marqo_index.test", "settings.ann_parameters.space_type", "prenormalized-angular"),
resource.TestCheckResourceAttr("marqo_index.test", "settings.ann_parameters.parameters.ef_construction", "512"),
resource.TestCheckResourceAttr("marqo_index.test", "settings.ann_parameters.parameters.m", "16"),
testAccCheckIndexIsReady(structured_index_name),
),
},
// Check for no changes on re-apply
{
Config: testAccResourceStructuredIndexConfig(structured_index_name),
PlanOnly: true,
ExpectNonEmptyPlan: false,
},
},
})
}

func testAccResourceStructuredIndexConfig(name string) string {
return fmt.Sprintf(`
resource "marqo_index" "test" {
index_name = "%s"
settings = {
type = "structured"
vector_numeric_type = "float"
all_fields = [
{ "name" : "text_field_0", "type" : "text", "features" : ["lexical_search"] },
{ "name" : "text_field", "type" : "text", "features" : ["lexical_search"] },
{ "name" : "image_field", "type" : "image_pointer" },
{
"name" : "multimodal_field",
"type" : "multimodal_combination",
"dependent_fields" : {
"imageField" : 0.8,
"textField" : 0.1
},
},
]
number_of_inferences = 1
storage_class = "marqo.basic"
number_of_replicas = 0
number_of_shards = 2
tensor_fields = ["multimodal_field"]
model = "open_clip/ViT-L-14/laion2b_s32b_b82k"
normalize_embeddings = true
inference_type = "marqo.CPU.small"
text_preprocessing = {
split_length = 2
split_method = "sentence"
split_overlap = 0
}
image_preprocessing = {}
ann_parameters = {
space_type = "prenormalized-angular"
parameters = {
ef_construction = 512
m = 16
}
}
}
}
`, name)
}

func testAccResourceIndexConfig(name string) string {
return fmt.Sprintf(`
resource "marqo_index" "test" {
Expand Down

0 comments on commit 59e666a

Please sign in to comment.