-
Notifications
You must be signed in to change notification settings - Fork 426
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: Generate ID and ObjectType Show Object Methods #3292
base: dev
Are you sure you want to change the base?
Changes from 10 commits
cea5dd3
d47db72
4750632
e051890
86d55bd
cf927ef
495e236
c9cb8d7
8fdc1dd
a3784c0
a1d1067
1c5c00d
590cb48
b0725ae
78a01e3
d5d1aa6
6b4be76
e1a27df
38b8a3e
9f29c42
1310f63
d610978
72fc399
475a570
388abfc
546e125
64fea51
3f44299
8987603
05914c3
3954232
05c44e5
9e757be
8183ff7
268e2d8
c60dec5
df0563b
a8ff21d
a9d50cb
57ebbc3
560cc8b
891ce2b
2d9930e
5f5e263
cae24f6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -40,6 +40,8 @@ type Operation struct { | |
DescribeMapping *Mapping | ||
// ShowByIDFiltering defines a kind of filterings performed in ShowByID operation | ||
ShowByIDFiltering []ShowByIDFiltering | ||
// ResourceHelperMethods contains helper methods for the Interface file (i.e. ID(), ObjectType()) | ||
ResourceHelperMethods []*ResourceHelperMethod | ||
} | ||
|
||
type Mapping struct { | ||
|
@@ -79,6 +81,11 @@ func (s *Operation) withHelperStructs(helperStructs ...*Field) *Operation { | |
return s | ||
} | ||
|
||
func (s *Operation) withObjectInterface(objectInterface *Interface) *Operation { | ||
s.ObjectInterface = objectInterface | ||
return s | ||
} | ||
|
||
func addShowMapping(op *Operation, from, to *Field) { | ||
op.ShowMapping = newMapping("convert", from, to) | ||
} | ||
|
@@ -117,6 +124,7 @@ func (i *Interface) newOperationWithDBMapping( | |
resourceRepresentation *plainStruct, | ||
queryStruct *QueryStruct, | ||
addMappingFunc func(op *Operation, from, to *Field), | ||
resourceHelperMethods ...ResourceHelperMethodKind, | ||
) *Operation { | ||
db := dbRepresentation.IntoField() | ||
res := resourceRepresentation.IntoField() | ||
|
@@ -126,7 +134,10 @@ func (i *Interface) newOperationWithDBMapping( | |
op := newOperation(kind, doc). | ||
withHelperStruct(db). | ||
withHelperStruct(res). | ||
withOptionsStruct(queryStruct.IntoField()) | ||
withOptionsStruct(queryStruct.IntoField()). | ||
withObjectInterface(i). | ||
withResourceHelperMethods(res.Name, resourceHelperMethods...) | ||
|
||
addMappingFunc(op, db, res) | ||
i.Operations = append(i.Operations, op) | ||
return op | ||
|
@@ -156,8 +167,8 @@ func (i *Interface) RevokeOperation(doc string, queryStruct *QueryStruct) *Inter | |
return i.newSimpleOperation(string(OperationKindRevoke), doc, queryStruct) | ||
} | ||
|
||
func (i *Interface) ShowOperation(doc string, dbRepresentation *dbStruct, resourceRepresentation *plainStruct, queryStruct *QueryStruct) *Interface { | ||
i.newOperationWithDBMapping(string(OperationKindShow), doc, dbRepresentation, resourceRepresentation, queryStruct, addShowMapping) | ||
func (i *Interface) ShowOperation(doc string, dbRepresentation *dbStruct, resourceRepresentation *plainStruct, queryStruct *QueryStruct, helperMethods ...ResourceHelperMethodKind) *Interface { | ||
i.newOperationWithDBMapping(string(OperationKindShow), doc, dbRepresentation, resourceRepresentation, queryStruct, addShowMapping, helperMethods...) | ||
return i | ||
} | ||
|
||
|
@@ -170,9 +181,9 @@ func (i *Interface) ShowByIdOperationWithNoFiltering() *Interface { | |
|
||
// ShowByIdOperationWithFiltering adds a ShowByID operation to the interface with filtering. Should be used for objects that implement filtering options e.g. Like or In. | ||
func (i *Interface) ShowByIdOperationWithFiltering(filter ShowByIDFilteringKind, filtering ...ShowByIDFilteringKind) *Interface { | ||
op := newNoSqlOperation(string(OperationKindShowByID)) | ||
op.ObjectInterface = i | ||
op.withFiltering(append([]ShowByIDFilteringKind{filter}, filtering...)...) | ||
op := newNoSqlOperation(string(OperationKindShowByID)). | ||
withObjectInterface(i). | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
withFiltering(append(filtering, filter)...) | ||
i.Operations = append(i.Operations, op) | ||
return i | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,122 @@ | ||
package generator | ||
|
||
import ( | ||
"fmt" | ||
"log" | ||
"slices" | ||
) | ||
|
||
type objectIdentifier string | ||
|
||
const ( | ||
AccountObjectIdentifier objectIdentifier = "AccountObjectIdentifier" | ||
DatabaseObjectIdentifier objectIdentifier = "DatabaseObjectIdentifier" | ||
SchemaObjectIdentifier objectIdentifier = "SchemaObjectIdentifier" | ||
SchemaObjectIdentifierWithArguments objectIdentifier = "SchemaObjectIdentifierWithArguments" | ||
) | ||
|
||
func identifierStringToObjectIdentifier(s string) objectIdentifier { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: pls add basic unit tests. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
switch s { | ||
case "AccountObjectIdentifier": | ||
return AccountObjectIdentifier | ||
case "DatabaseObjectIdentifier": | ||
return DatabaseObjectIdentifier | ||
case "SchemaObjectIdentifier": | ||
return SchemaObjectIdentifier | ||
case "SchemaObjectIdentifierWithArguments": | ||
return SchemaObjectIdentifierWithArguments | ||
default: | ||
return "" | ||
} | ||
} | ||
|
||
type ResourceHelperMethodKind uint | ||
|
||
const ( | ||
ResourceIDHelperMethod ResourceHelperMethodKind = iota | ||
ResourceObjectTypeHelperMethod | ||
) | ||
|
||
type ResourceHelperMethod struct { | ||
Name string | ||
StructName string | ||
ReturnValue string | ||
ReturnType string | ||
} | ||
|
||
func newResourceHelperMethod(name, structName, returnValue string, returnType string) *ResourceHelperMethod { | ||
return &ResourceHelperMethod{ | ||
Name: name, | ||
StructName: structName, | ||
ReturnValue: returnValue, | ||
ReturnType: returnType, | ||
} | ||
} | ||
|
||
var requiredFieldsForIDMethodMapping map[objectIdentifier][]string = map[objectIdentifier][]string{ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. seems all right |
||
AccountObjectIdentifier: {"Name"}, | ||
DatabaseObjectIdentifier: {"Name", "DatabaseName"}, | ||
SchemaObjectIdentifier: {"Name", "DatabaseName", "SchemaName"}, | ||
} | ||
|
||
func hasRequiredFieldsForIDMethod(structName string, helperStructs []*Field, requiredFields ...string) bool { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: we should receive |
||
for _, field := range helperStructs { | ||
if field.Name == structName { | ||
return containsFieldNames(field.Fields, requiredFields...) | ||
} | ||
} | ||
return false | ||
} | ||
|
||
func containsFieldNames(fields []*Field, names ...string) bool { | ||
fieldNames := []string{} | ||
for _, field := range fields { | ||
fieldNames = append(fieldNames, field.Name) | ||
} | ||
|
||
for _, name := range names { | ||
if !slices.Contains(fieldNames, name) { | ||
return false | ||
} | ||
} | ||
return true | ||
} | ||
|
||
func newResourceIDHelperMethod(structName string, helperStructs []*Field, identifierString string) *ResourceHelperMethod { | ||
objectIdentifier := identifierStringToObjectIdentifier(identifierString) | ||
requiredFields, ok := requiredFieldsForIDMethodMapping[objectIdentifier] | ||
if !ok { | ||
log.Printf("WARNING: No required fields mapping defined for identifier %s", objectIdentifier) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: we us |
||
return nil | ||
} | ||
if !hasRequiredFieldsForIDMethod(structName, helperStructs, requiredFields...) { | ||
log.Printf("WARNING: Struct '%s' does not contain needed fields to build ID() helper method. Create the method manually in _ext file or add missing one of required fields: %v.\n", structName, requiredFields) | ||
return nil | ||
} | ||
|
||
var args string | ||
for _, field := range requiredFields { | ||
args += fmt.Sprintf("v.%v, ", field) | ||
} | ||
|
||
returnValue := fmt.Sprintf("New%v(%v)", objectIdentifier, args) | ||
return newResourceHelperMethod("ID", structName, returnValue, string(objectIdentifier)) | ||
} | ||
|
||
func newResourceObjectTypeHelperMethod(structName string) *ResourceHelperMethod { | ||
return newResourceHelperMethod("ObjectType", structName, "ObjectType"+structName, "ObjectType") | ||
} | ||
|
||
func (s *Operation) withResourceHelperMethods(structName string, helperMethods ...ResourceHelperMethodKind) *Operation { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: in general, it's easier to read a file when upstream functions are below functions that call them. So, this function should be moved higher than |
||
for _, helperMethod := range helperMethods { | ||
switch helperMethod { | ||
case ResourceIDHelperMethod: | ||
s.ResourceHelperMethods = append(s.ResourceHelperMethods, newResourceIDHelperMethod(structName, s.HelperStructs, s.ObjectInterface.IdentifierKind)) | ||
case ResourceObjectTypeHelperMethod: | ||
s.ResourceHelperMethods = append(s.ResourceHelperMethods, newResourceObjectTypeHelperMethod(structName)) | ||
default: | ||
log.Println("No resourceHelperMethod found for kind:", helperMethod) | ||
} | ||
} | ||
return s | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resource is bad, because it's coupled with TF resource, and SDK should not be coupled with TF. My proposals are
g.ShowObjectIdMethod
andg.ObjectIdMethod
. I like the former more.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
g.ShowObjectIdMethod and g.ShowObjectTypeMethod seems fine to me as well