Skip to content

Commit

Permalink
Simplify Property Conversion Context (#1562)
Browse files Browse the repository at this point in the history
* Rename constructor to NewPropertyConversionContext()

* Use Name() instead of storing it

* Introduce WithDirection() and consume when creating functions

* Don't provide direction when creating contexts

* Code gardening
  • Loading branch information
theunrepentantgeek authored Jun 17, 2021
1 parent 69e8589 commit cac6e83
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 24 deletions.
12 changes: 5 additions & 7 deletions hack/generator/pkg/codegen/storage/storage_type_factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,28 +165,26 @@ func (f *StorageTypeFactory) injectConversions(definition astmodel.TypeDefinitio
knownTypes := make(astmodel.Types)
knownTypes.AddTypes(f.inputTypes)
knownTypes.AddTypes(f.outputTypes)
conversionFromContext := conversions.NewStorageConversionContext(knownTypes, conversions.ConvertFrom, f.idFactory)
conversionContext := conversions.NewPropertyConversionContext(knownTypes, f.idFactory)

convertFromFn, err := conversions.NewPropertyAssignmentFromFunction(definition, nextDef, f.idFactory, conversionFromContext)
convertFromFn, err := conversions.NewPropertyAssignmentFromFunction(definition, nextDef, f.idFactory, conversionContext)
if err != nil {
return nil, errors.Wrapf(err, "creating ConvertFrom() function for %q", name)
}

conversionToContext := conversions.NewStorageConversionContext(knownTypes, conversions.ConvertTo, f.idFactory)

convertToFn, err := conversions.NewPropertyAssignmentToFunction(definition, nextDef, f.idFactory, conversionToContext)
convertToFn, err := conversions.NewPropertyAssignmentToFunction(definition, nextDef, f.idFactory, conversionContext)
if err != nil {
return nil, errors.Wrapf(err, "creating ConvertTo() function for %q", name)
}

updatedDefinition, err := f.functionInjector.Inject(definition, convertFromFn)
if err != nil {
return nil, errors.Wrapf(err, "failed to inject ConvertFrom function into %q", name)
return nil, errors.Wrapf(err, "failed to inject %s function into %q", convertFromFn.Name(), name)
}

updatedDefinition, err = f.functionInjector.Inject(updatedDefinition, convertToFn)
if err != nil {
return nil, errors.Wrapf(err, "failed to inject ConvertFrom function into %q", name)
return nil, errors.Wrapf(err, "failed to inject %s function into %q", convertToFn.Name(), name)
}

return &updatedDefinition, nil
Expand Down
20 changes: 10 additions & 10 deletions hack/generator/pkg/conversions/property_assignment_function.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ import (
// PropertyAssignmentFunction represents a function that assigns all the properties from one resource or object to
// another. Performs a single step of the conversions required to/from the hub version.
type PropertyAssignmentFunction struct {
// name of this conversion function
name string
// otherDefinition is the type we are converting to (or from). This will be a type which is "closer"
// to the hub storage type, making this a building block of the final conversion.
otherDefinition astmodel.TypeDefinition
Expand Down Expand Up @@ -65,12 +63,13 @@ func NewPropertyAssignmentFromFunction(
// TODO: Bevan will improve how this is done (avoid hard-coding "source")
result.knownLocals.Add("source")

result.name = nameOfPropertyAssignmentFunction(otherDefinition.Name(), ConvertFrom, idFactory)
result.conversionContext = conversionContext.WithFunctionName(result.name).WithKnownLocals(result.knownLocals)
result.conversionContext = conversionContext.WithFunctionName(result.Name()).
WithKnownLocals(result.knownLocals).
WithDirection(ConvertFrom)

err := result.createConversions(receiver)
if err != nil {
return nil, errors.Wrapf(err, "creating '%s()'", result.name)
return nil, errors.Wrapf(err, "creating '%s()'", result.Name())
}

return result, nil
Expand All @@ -94,20 +93,21 @@ func NewPropertyAssignmentToFunction(
// TODO: Bevan will improve how this is done (avoid hard-coding "destination")
result.knownLocals.Add("destination")

result.name = nameOfPropertyAssignmentFunction(otherDefinition.Name(), ConvertTo, idFactory)
result.conversionContext = conversionContext.WithFunctionName(result.name).WithKnownLocals(result.knownLocals)
result.conversionContext = conversionContext.WithFunctionName(result.Name()).
WithKnownLocals(result.knownLocals).
WithDirection(ConvertTo)

err := result.createConversions(receiver)
if err != nil {
return nil, errors.Wrapf(err, "creating '%s()'", result.name)
return nil, errors.Wrapf(err, "creating '%s()'", result.Name())
}

return result, nil
}

// Name returns the name of this function
func (fn *PropertyAssignmentFunction) Name() string {
return fn.name
return nameOfPropertyAssignmentFunction(fn.otherDefinition.Name(), fn.direction, fn.idFactory)
}

// RequiredPackageReferences returns the set of package references required by this function
Expand All @@ -127,7 +127,7 @@ func (fn *PropertyAssignmentFunction) References() astmodel.TypeNameSet {
// Equals checks to see if the supplied function is the same as this one
func (fn *PropertyAssignmentFunction) Equals(f astmodel.Function) bool {
if other, ok := f.(*PropertyAssignmentFunction); ok {
if fn.name != other.name {
if fn.Name() != other.Name() {
// Different name means not-equal
return false
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,12 +204,11 @@ func runTestPropertyAssignmentFunction_AsFunc(c *StorageConversionPropertyTestCa
currentType, ok := astmodel.AsObjectType(c.currentObject.Type())
g.Expect(ok).To(BeTrue())

conversionFromContext := NewStorageConversionContext(c.types, ConvertFrom, idFactory)
convertFrom, errs := NewPropertyAssignmentFromFunction(c.currentObject, c.otherObject, idFactory, conversionFromContext)
conversionContext := NewPropertyConversionContext(c.types, idFactory)
convertFrom, errs := NewPropertyAssignmentFromFunction(c.currentObject, c.otherObject, idFactory, conversionContext)
g.Expect(errs).To(BeNil())

conversionToContext := NewStorageConversionContext(c.types, ConvertTo, idFactory)
convertTo, errs := NewPropertyAssignmentToFunction(c.currentObject, c.otherObject, idFactory, conversionToContext)
convertTo, errs := NewPropertyAssignmentToFunction(c.currentObject, c.otherObject, idFactory, conversionContext)
g.Expect(errs).To(BeNil())

receiverDefinition := c.currentObject.WithType(currentType.WithFunction(convertFrom).WithFunction(convertTo))
Expand Down
12 changes: 9 additions & 3 deletions hack/generator/pkg/conversions/property_conversion_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,10 @@ type PropertyConversionContext struct {
idFactory astmodel.IdentifierFactory
}

// NewStorageConversionContext creates a new instance of a PropertyConversionContext
func NewStorageConversionContext(types astmodel.Types, direction Direction, idFactory astmodel.IdentifierFactory) *PropertyConversionContext {
// NewPropertyConversionContext creates a new instance of a PropertyConversionContext
func NewPropertyConversionContext(types astmodel.Types, idFactory astmodel.IdentifierFactory) *PropertyConversionContext {
return &PropertyConversionContext{
types: types,
direction: direction,
idFactory: idFactory,
knownLocals: astmodel.NewKnownLocalsSet(idFactory),
}
Expand Down Expand Up @@ -57,6 +56,13 @@ func (c *PropertyConversionContext) WithKnownLocals(knownLocals *astmodel.KnownL
return result
}

// WithDirection returns a new context with the specified direction
func (c *PropertyConversionContext) WithDirection(dir Direction) *PropertyConversionContext {
result := c.clone()
result.direction = dir
return result
}

// NestedContext returns a new context with a cloned knownLocals so that nested blocks
// can declare and reuse locals independently of each other.
func (c *PropertyConversionContext) NestedContext() *PropertyConversionContext {
Expand Down

0 comments on commit cac6e83

Please sign in to comment.