Resolve comments from ODATA PR and add UTs for selectTree

backend/pkg/database/gorm/odata.go

@@ -43,12 +43,12 @@ var schemaMetas = map[string]odatasql.SchemaMeta{
 			"scheduled": odatasql.FieldMeta{
 				FieldType:             odatasql.ComplexFieldType,
 				ComplexFieldSchemas:   []string{"SingleScheduleScanConfig"},
-				DescriminatorProperty: "objectType",
+				DiscriminatorProperty: "objectType",
 			},
 			"scope": odatasql.FieldMeta{
 				FieldType:             odatasql.ComplexFieldType,
 				ComplexFieldSchemas:   []string{"AwsScanScope"},
-				DescriminatorProperty: "objectType",
+				DiscriminatorProperty: "objectType",
 			},
 		},
 	},

backend/pkg/database/gorm/resttodb.go

@@ -140,7 +140,7 @@ func ConvertToDBScan(scan models.Scan) (Scan, error) {
 	if scan.Id != nil {
 		scanUUID, err = uuid.FromString(*scan.Id)
 		if err != nil {
-			return ret, fmt.Errorf("failed to convert scanID %v to uuid: %v", scan.Id, err)
+			return ret, fmt.Errorf("failed to convert scanID %v to uuid: %v", *scan.Id, err)
 		}
 	}
 

backend/pkg/database/gorm/scan_config.go

@@ -163,9 +163,9 @@ func (s *ScanConfigsTableHandler) CreateScanConfig(scanConfig models.ScanConfig)
 	return sc, nil
 }
 
-func getExistingScanConfigbyID(db *gorm.DB, scanConfig models.ScanConfig) (ScanConfig, error) {
+func getExistingScanConfigByID(db *gorm.DB, scanConfigID models.ScanConfigID) (ScanConfig, error) {
 	var dbScanConfig ScanConfig
-	filter := fmt.Sprintf("id eq '%s'", *scanConfig.Id)
+	filter := fmt.Sprintf("id eq '%s'", scanConfigID)
 	err := ODataQuery(db, "ScanConfig", &filter, nil, nil, nil, nil, false, &dbScanConfig)
 	if err != nil {
 		if errors.Is(err, gorm.ErrRecordNotFound) {
@@ -177,7 +177,7 @@ func getExistingScanConfigbyID(db *gorm.DB, scanConfig models.ScanConfig) (ScanC
 }
 
 func (s *ScanConfigsTableHandler) SaveScanConfig(scanConfig models.ScanConfig) (models.ScanConfig, error) {
-	dbScanConfig, err := getExistingScanConfigbyID(s.DB, scanConfig)
+	dbScanConfig, err := getExistingScanConfigByID(s.DB, *scanConfig.Id)
 	if err != nil {
 		return models.ScanConfig{}, err
 	}
@@ -205,7 +205,7 @@ func (s *ScanConfigsTableHandler) SaveScanConfig(scanConfig models.ScanConfig) (
 }
 
 func (s *ScanConfigsTableHandler) UpdateScanConfig(scanConfig models.ScanConfig) (models.ScanConfig, error) {
-	dbScanConfig, err := getExistingScanConfigbyID(s.DB, scanConfig)
+	dbScanConfig, err := getExistingScanConfigByID(s.DB, *scanConfig.Id)
 	if err != nil {
 		return models.ScanConfig{}, err
 	}

backend/pkg/database/odatasql/query.go

@@ -177,11 +177,11 @@ func buildSelectFields(schemaMetas map[string]SchemaMeta, field FieldMeta, ident
 			schema := schemaMetas[schemaName]
 
 			parts := []string{}
-			if field.DescriminatorProperty != "" {
-				parts = append(parts, fmt.Sprintf("'%s', '%s'", field.DescriminatorProperty, schemaName))
+			if field.DiscriminatorProperty != "" {
+				parts = append(parts, fmt.Sprintf("'%s', '%s'", field.DiscriminatorProperty, schemaName))
 			}
 			for key, fm := range schema.Fields {
-				if field.DescriminatorProperty != "" && key == field.DescriminatorProperty {
+				if field.DiscriminatorProperty != "" && key == field.DiscriminatorProperty {
 					continue
 				}
 
@@ -216,10 +216,13 @@ func buildSelectFields(schemaMetas map[string]SchemaMeta, field FieldMeta, ident
 		// descriminator, this would be a developer error. Might be
 		// avoidable if we create a schema builder thing instead of
 		// just defining it as a variable.
-		// if field.DescriminatorProperty == "" {
+		// if field.DiscriminatorProperty == "" {
 		// }
 
-		return fmt.Sprintf("(SELECT %s.value FROM JSON_EACH(JSON_ARRAY(%s)) AS %s WHERE %s.value -> '$.%s' = %s -> '%s.%s')", identifier, strings.Join(objects, ","), identifier, identifier, field.DescriminatorProperty, source, path, field.DescriminatorProperty)
+		return fmt.Sprintf(
+			"(SELECT %s.value FROM JSON_EACH(JSON_ARRAY(%s)) AS %s WHERE %s.value -> '$.%s' = %s -> '%s.%s')",
+			identifier, strings.Join(objects, ","), identifier,
+			identifier, field.DiscriminatorProperty, source, path, field.DiscriminatorProperty)
 
 	case RelationshipFieldType:
 		if st == nil || !st.expand {

backend/pkg/database/odatasql/query_test.go

@@ -159,14 +159,14 @@ var carSchemaMetas = map[string]SchemaMeta{
 			"MainStereo": {
 				FieldType:             ComplexFieldType,
 				ComplexFieldSchemas:   []string{"CDPlayer", "Radio"},
-				DescriminatorProperty: "ObjectType",
+				DiscriminatorProperty: "ObjectType",
 			},
 			"OtherStereos": {
 				FieldType: CollectionFieldType,
 				CollectionItemMeta: &FieldMeta{
 					FieldType:             ComplexFieldType,
 					ComplexFieldSchemas:   []string{"CDPlayer", "Radio"},
-					DescriminatorProperty: "ObjectType",
+					DiscriminatorProperty: "ObjectType",
 				},
 			},
 			"Manufacturer": {

backend/pkg/database/odatasql/schema.go

@@ -33,7 +33,7 @@ type FieldMeta struct {
 
 	// Fields for complex field types
 	ComplexFieldSchemas   []string
-	DescriminatorProperty string
+	DiscriminatorProperty string
 
 	// Fields for relationship and relationship collection types
 	RelationshipSchema   string

backend/pkg/database/odatasql/selectTree.go

@@ -40,9 +40,8 @@ func newSelectTree() *selectNode {
 
 // nolint:gocognit,cyclop
 func (st *selectNode) insert(path []*godata.Token, filter *godata.GoDataFilterQuery, sel *godata.GoDataSelectQuery, subExpand *godata.GoDataExpandQuery, expand bool) error {
-	// If path length == 0 then we've reach the bottom of the path and now
-	// we need to save the filter/select and process any sub
-	// selects/expands
+	// If path length == 0 then we've reach the bottom of the path, now we
+	// need to save the filter/select and process any sub selects/expands
 	if len(path) == 0 {
 		if st.filter != nil {
 			return fmt.Errorf("filter for field specified twice")
@@ -57,7 +56,7 @@ func (st *selectNode) insert(path []*godata.Token, filter *godata.GoDataFilterQu
 			}
 
 			// Parse $select using ParseExpandString because godata.ParseSelectString
-			// is a nieve implementation and doesn't handle query options properly
+			// is a naive implementation and doesn't handle query options properly
 			childSelections, err := godata.ParseExpandString(context.TODO(), sel.RawValue)
 			if err != nil {
 				return fmt.Errorf("failed to parse select: %w", err)

backend/pkg/database/odatasql/selectTree_test.go

@@ -0,0 +1,96 @@
+// Copyright © 2023 Cisco Systems, Inc. and its affiliates.
+// All rights reserved.
+//
+// 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
+//
+//     http://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 odatasql
+
+import (
+	"context"
+	"testing"
+
+	"github.com/CiscoM31/godata"
+)
+
+func Test_SelectTree(t *testing.T) {
+	basicExpand, _ := godata.ParseExpandString(context.TODO(), "Manufacturer")
+	expandWithNestedFilter, _ := godata.ParseExpandString(context.TODO(), "Manufacturer($filter=foo eq 'bar')")
+	expandWithNestedSelect, _ := godata.ParseExpandString(context.TODO(), "Manufacturer($select=Name)")
+	expandWithNestedSelectAndFilter, _ := godata.ParseExpandString(context.TODO(), "Manufacturer($select=Names($filter=foo eq 'bar'))")
+
+	tests := []struct {
+		name        string
+		selectQuery *godata.GoDataSelectQuery
+		expandQuery *godata.GoDataExpandQuery
+		wantErr     bool
+	}{
+		{
+			name:        "Basic Select",
+			selectQuery: &godata.GoDataSelectQuery{RawValue: "ModelName,Engine/Options"},
+		},
+		{
+			name:        "Basic Expand",
+			expandQuery: basicExpand,
+		},
+		{
+			name:        "Basic Expand and Select",
+			selectQuery: &godata.GoDataSelectQuery{RawValue: "ModelName,Engine/Options"},
+			expandQuery: basicExpand,
+		},
+		{
+			name: "Select with filter in two places",
+			selectQuery: &godata.GoDataSelectQuery{
+				RawValue: "Engine/Options($filter=Foo eq 'bar'),Engine($select=Options($filter=Id eq 'id'))",
+			},
+			expandQuery: nil,
+			wantErr:     true,
+		},
+		{
+			name:        "Select with empty string",
+			selectQuery: &godata.GoDataSelectQuery{RawValue: ""},
+			wantErr:     true,
+		},
+		{
+			name:        "Expand with nested filter",
+			expandQuery: expandWithNestedFilter,
+		},
+		{
+			name:        "Expand with nested select",
+			expandQuery: expandWithNestedSelect,
+		},
+		{
+			name:        "Expand with nested select and filter",
+			expandQuery: expandWithNestedSelectAndFilter,
+		},
+		{
+			name:        "Selection specified for same field in multiple forms",
+			selectQuery: &godata.GoDataSelectQuery{RawValue: "Engine/Options,Engine($select=Options)"},
+			wantErr:     true,
+		},
+		{
+			name:        "Expand is not allowed inside $select",
+			selectQuery: &godata.GoDataSelectQuery{RawValue: "Engine($expand=Foo)"},
+			wantErr:     true,
+		},
+	}
+
+	for _, test := range tests {
+		selectTree := newSelectTree()
+		err := selectTree.insert(nil, nil, test.selectQuery, test.expandQuery, false)
+		if err != nil && !test.wantErr {
+			t.Errorf("unexpected error for %v: %v", test.name, err)
+		} else if err == nil && test.wantErr {
+			t.Errorf("expected error for %v but got nil", test.name)
+		}
+	}
+}