Fix: use an array instead of map to maintain the order of the keys (#253

)

* Fix: use an array instead of map to maintain the order

* bit more optimization and generated protos

example/feature_demo/demo_multi_file.proto

@@ -9,7 +9,7 @@ option go_package = "github.com/infobloxopen/protoc-gen-gorm/example/feature_dem
    option (gorm.opts) = {
      ormable: true,
    };
-   string id = 1; // string id
+   string id = 1;
  }
 
 message BlogPost {

example/feature_demo/demo_types.proto

@@ -76,7 +76,7 @@ message TestTypes {
   repeated gorm.types.JSONValue several_values = 14;
 }
 
-// TypeWithID demonstrates some basic assocation behavior
+// TypeWithID demonstrates some basic association behavior
 message TypeWithID {
   // Again we use the 'ormable' option, but also include an extra field
   // using the 'include' option. Any number of fields can be defined this way
@@ -119,7 +119,7 @@ message TypeWithID {
 
 // MultiaccountTypeWithID demonstrates the generated multi-account support
 message MultiaccountTypeWithID {
-  // here we use the multi_account option to generate auth integration in
+  // here we use the multi-account option to generate auth integration in
   // the ORM layer, and an assumed "account_id" column
   option (gorm.opts) = {
     ormable: true,

example/postgres_arrays/postgres_arrays.pb.gorm.go

@@ -4,7 +4,6 @@ import (
 	context "context"
 	fmt "fmt"
 	gateway "github.com/infobloxopen/atlas-app-toolkit/gateway"
-	gorm1 "github.com/infobloxopen/atlas-app-toolkit/gorm"
 	errors "github.com/infobloxopen/protoc-gen-gorm/errors"
 	pq "github.com/lib/pq"
 	field_mask "google.golang.org/genproto/protobuf/field_mask"
@@ -165,9 +164,6 @@ func DefaultReadExample(ctx context.Context, in *Example, db *gorm.DB) (*Example
 			return nil, err
 		}
 	}
-	if db, err = gorm1.ApplyFieldSelection(ctx, db, nil, &ExampleORM{}); err != nil {
-		return nil, err
-	}
 	if hook, ok := interface{}(&ormObj).(ExampleORMWithBeforeReadFind); ok {
 		if db, err = hook.BeforeReadFind(ctx, db); err != nil {
 			return nil, err
@@ -443,10 +439,6 @@ func DefaultListExample(ctx context.Context, db *gorm.DB) ([]*Example, error) {
 			return nil, err
 		}
 	}
-	db, err = gorm1.ApplyCollectionOperators(ctx, db, &ExampleORM{}, &Example{}, nil, nil, nil, nil)
-	if err != nil {
-		return nil, err
-	}
 	if hook, ok := interface{}(&ormObj).(ExampleORMWithBeforeListFind); ok {
 		if db, err = hook.BeforeListFind(ctx, db); err != nil {
 			return nil, err

example/postgres_arrays/postgres_arrays.proto

@@ -9,6 +9,7 @@ option go_package = "github.com/infobloxopen/protoc-gen-gorm/example/postgres_ar
 message Example {
     option (gorm.opts) = {ormable: true};
 
+    // id for example
     string id = 1 [(gorm.field).tag = {type: "uuid" primary_key: true}];
     string description = 2;
     repeated bool array_of_bools = 20;

example/user/user.pb.gorm.go

@@ -3036,6 +3036,9 @@ func DefaultReadTask(ctx context.Context, in *Task, db *gorm.DB) (*Task, error)
 	if err != nil {
 		return nil, err
 	}
+	if ormObj.Id == nil || *ormObj.Id == "" {
+		return nil, errors.EmptyIdError
+	}
 	if hook, ok := interface{}(&ormObj).(TaskORMWithBeforeReadApplyQuery); ok {
 		if db, err = hook.BeforeReadApplyQuery(ctx, db); err != nil {
 			return nil, err
@@ -3077,6 +3080,9 @@ func DefaultDeleteTask(ctx context.Context, in *Task, db *gorm.DB) error {
 	if err != nil {
 		return err
 	}
+	if ormObj.Id == nil || *ormObj.Id == "" {
+		return errors.EmptyIdError
+	}
 	if hook, ok := interface{}(&ormObj).(TaskORMWithBeforeDelete_); ok {
 		if db, err = hook.BeforeDelete_(ctx, db); err != nil {
 			return err
@@ -3439,10 +3445,10 @@ func DefaultDeleteDepartment(ctx context.Context, in *Department, db *gorm.DB) e
 	if err != nil {
 		return err
 	}
-	if ormObj.Name == "" {
+	if ormObj.Id == 0 {
 		return errors.EmptyIdError
 	}
-	if ormObj.Id == 0 {
+	if ormObj.Name == "" {
 		return errors.EmptyIdError
 	}
 	if hook, ok := interface{}(&ormObj).(DepartmentORMWithBeforeDelete_); ok {
@@ -3509,7 +3515,7 @@ func DefaultListDepartment(ctx context.Context, db *gorm.DB) ([]*Department, err
 		}
 	}
 	db = db.Where(&ormObj)
-	db = db.Order("name, id")
+	db = db.Order("id, name")
 	ormResponse := []DepartmentORM{}
 	if err := db.Find(&ormResponse).Error; err != nil {
 		return nil, err

example/user/user.proto

@@ -98,6 +98,7 @@ message Department {
     option (gorm.opts) = {
         ormable: true,
     };
+    // department name and id are composite primary keys
     string name = 1 [(gorm.field).tag = {primary_key: true}];
     int64 id = 2 [(gorm.field).tag = {primary_key: true}];
 }

plugin/plugin.go

@@ -98,6 +98,11 @@ var optionalTypes = map[string]string{
 	"bool":    "*bool",
 }
 
+type pkFieldObjs struct {
+	name  string
+	field *Field
+}
+
 const (
 	protoTypeTimestamp = "Timestamp" // last segment, first will be *google_protobufX
 	protoTypeDuration  = "Duration"
@@ -579,20 +584,35 @@ func (b *ORMBuilder) findPrimaryKey(ormable *OrmableType) (string, *Field) {
 	panic("no primary_key")
 }
 
-func (b *ORMBuilder) getPrimaryKeys(ormable *OrmableType) map[string]*Field {
-	mapPK := make(map[string]*Field)
+// getPrimaryKeys returns a sorted list of primary key field objects
+func (b *ORMBuilder) getPrimaryKeys(ormable *OrmableType) []pkFieldObjs {
+	var (
+		fieldobjs []pkFieldObjs
+	)
+
 	for fieldName, field := range ormable.Fields {
 		if field.GetTag().GetPrimaryKey() {
-			mapPK[fieldName] = field
+			fieldobjs = append(fieldobjs, pkFieldObjs{
+				fieldName,
+				field,
+			})
 		}
 	}
-	// consider field name "id" as well
-	for fieldName, field := range ormable.Fields {
-		if strings.ToLower(fieldName) == "id" {
-			mapPK[fieldName] = field
+	sort.Slice(fieldobjs, func(i, j int) bool {
+		return fieldobjs[i].name < fieldobjs[j].name
+	})
+	// if no primary key is found, use the field named "id"
+	if len(fieldobjs) == 0 {
+		for fieldName, field := range ormable.Fields {
+			if strings.ToLower(fieldName) == "id" {
+				fieldobjs = append(fieldobjs, pkFieldObjs{
+					fieldName,
+					field,
+				})
+			}
 		}
 	}
-	return mapPK
+	return fieldobjs
 }
 
 func (b *ORMBuilder) getOrmable(typeName string) *OrmableType {
@@ -1851,12 +1871,12 @@ func (b *ORMBuilder) generateReadHandler(message *protogen.Message, g *protogen.
 	g.P(`return nil, err`)
 	g.P(`}`)
 
-	mapPK := b.getPrimaryKeys(ormable)
-	for k, f := range mapPK {
-		if strings.Contains(f.TypeName, "*") {
-			g.P(`if ormObj.`, k, ` == nil || *ormObj.`, k, ` == `, b.guessZeroValue(f.TypeName, g), ` {`)
+	pkfieldMapping := b.getPrimaryKeys(ormable)
+	for _, pkfieldObj := range pkfieldMapping {
+		if strings.Contains(pkfieldObj.field.TypeName, "*") {
+			g.P(`if ormObj.`, pkfieldObj.name, ` == nil || *ormObj.`, pkfieldObj.name, ` == `, b.guessZeroValue(pkfieldObj.field.TypeName, g), ` {`)
 		} else {
-			g.P(`if ormObj.`, k, ` == `, b.guessZeroValue(f.TypeName, g), ` {`)
+			g.P(`if ormObj.`, pkfieldObj.name, ` == `, b.guessZeroValue(pkfieldObj.field.TypeName, g), ` {`)
 		}
 		g.P(`return nil, `, "errors", `.EmptyIdError`)
 		g.P(`}`)
@@ -1992,12 +2012,12 @@ func (b *ORMBuilder) generateDeleteHandler(message *protogen.Message, g *protoge
 	g.P(`}`)
 
 	ormable := b.getOrmable(typeName)
-	mapPKs := b.getPrimaryKeys(ormable)
-	for pkName, pk := range mapPKs {
-		if strings.Contains(pk.TypeName, "*") {
-			g.P(`if ormObj.`, pkName, ` == nil || *ormObj.`, pkName, ` == `, b.guessZeroValue(pk.TypeName, g), ` {`)
+	pkFieldMapping := b.getPrimaryKeys(ormable)
+	for _, pkFieldObj := range pkFieldMapping {
+		if strings.Contains(pkFieldObj.field.TypeName, "*") {
+			g.P(`if ormObj.`, pkFieldObj.name, ` == nil || *ormObj.`, pkFieldObj.name, ` == `, b.guessZeroValue(pkFieldObj.field.TypeName, g), ` {`)
 		} else {
-			g.P(`if ormObj.`, pkName, ` == `, b.guessZeroValue(pk.TypeName, g), `{`)
+			g.P(`if ormObj.`, pkFieldObj.name, ` == `, b.guessZeroValue(pkFieldObj.field.TypeName, g), `{`)
 		}
 		g.P(`return `, generateImport("EmptyIdError", gerrorsImport, g))
 		g.P(`}`)
@@ -2672,12 +2692,12 @@ func (b *ORMBuilder) generateListHandler(message *protogen.Message, g *protogen.
 
 	// TODO handle composite primary keys order considering priority tag
 	if b.hasCompositePrimaryKey(ormable) {
-		pksMap := b.getPrimaryKeys(ormable)
+		pkFieldMapping := b.getPrimaryKeys(ormable)
 		var columns []string
-		for fieldName, field := range pksMap {
-			column := field.GetTag().GetColumn()
+		for _, pkFieldObj := range pkFieldMapping {
+			column := pkFieldObj.field.GetTag().GetColumn()
 			if len(column) == 0 {
-				column = gschema.NamingStrategy{SingularTable: true}.TableName(fieldName)
+				column = gschema.NamingStrategy{SingularTable: true}.TableName(pkFieldObj.name)
 			}
 			columns = append(columns, column)
 		}