diff --git a/hack/generator/pkg/astbuilder/builder.go b/hack/generator/pkg/astbuilder/builder.go index d304bf8481d..b05ecac867b 100644 --- a/hack/generator/pkg/astbuilder/builder.go +++ b/hack/generator/pkg/astbuilder/builder.go @@ -303,9 +303,10 @@ func ReturnNoError() dst.Stmt { // // errors.Wrap(err, ) // -func WrappedErrorf(template string, args ...interface{}) dst.Expr { +// (actual package name will be used, which will usually be 'errors') +func WrappedErrorf(errorsPackage string, template string, args ...interface{}) dst.Expr { return CallQualifiedFunc( - "errors", + errorsPackage, "Wrap", dst.NewIdent("err"), StringLiteralf(template, args...)) diff --git a/hack/generator/pkg/astmodel/known_locals_set.go b/hack/generator/pkg/astmodel/known_locals_set.go index 266e6c6c584..d65a94f4b0e 100644 --- a/hack/generator/pkg/astmodel/known_locals_set.go +++ b/hack/generator/pkg/astmodel/known_locals_set.go @@ -79,6 +79,7 @@ func (locals *KnownLocalsSet) Add(local string) { locals.names[name] = struct{}{} } +// HasName returns true if the specified name exists in the set, false otherwise func (locals *KnownLocalsSet) HasName(name string) bool { _, ok := locals.names[name] return ok diff --git a/hack/generator/pkg/astmodel/optional_type.go b/hack/generator/pkg/astmodel/optional_type.go index 785de60e28a..423a374fa9b 100644 --- a/hack/generator/pkg/astmodel/optional_type.go +++ b/hack/generator/pkg/astmodel/optional_type.go @@ -104,7 +104,7 @@ func (optional *OptionalType) BaseType() Type { // String implements fmt.Stringer func (optional *OptionalType) String() string { - return fmt.Sprintf("(optional: %s)", optional.element.String()) + return fmt.Sprintf("optional(%s)", optional.element.String()) } // Unwrap returns the type contained within the wrapper type diff --git a/hack/generator/pkg/astmodel/package_reference.go b/hack/generator/pkg/astmodel/package_reference.go index 6875a4a0eca..6a246ced6a1 100644 --- a/hack/generator/pkg/astmodel/package_reference.go +++ b/hack/generator/pkg/astmodel/package_reference.go @@ -48,15 +48,19 @@ func PackageAsLocalPackage(pkg PackageReference) (LocalPackageReference, error) func SortPackageReferencesByPathAndVersion(packages []PackageReference) { sort.Slice(packages, func(i, j int) bool { - comparer := versionComparer{ - left: []rune(packages[i].PackagePath()), - right: []rune(packages[j].PackagePath()), - } - compare := comparer.Compare() - return compare < 0 + return ComparePathAndVersion(packages[i].PackagePath(), packages[j].PackagePath()) }) } +// ComparePathAndVersion compares two paths containing versions and returns true if left should go before right +func ComparePathAndVersion(left string, right string) bool { + comparer := versionComparer{ + left: []rune(left), + right: []rune(right), + } + return comparer.Compare() < 0 +} + // versionComparer captures our state while doing an alphanumeric version comparision // We need separate indexes for each side because we're doing a numeric comparison, which will // compare "100" and "0100" as equal (leading zeros are not significant) diff --git a/hack/generator/pkg/astmodel/package_reference_set.go b/hack/generator/pkg/astmodel/package_reference_set.go index 88a27c9fc71..933382e0ba6 100644 --- a/hack/generator/pkg/astmodel/package_reference_set.go +++ b/hack/generator/pkg/astmodel/package_reference_set.go @@ -43,6 +43,11 @@ func (set *PackageReferenceSet) Remove(ref PackageReference) { delete(set.references, ref) } +// Clear removes everything from the set +func (set *PackageReferenceSet) Clear() { + set.references = make(map[PackageReference]struct{}) +} + // Contains allows checking to see if an import is included func (set *PackageReferenceSet) Contains(ref PackageReference) bool { _, ok := set.references[ref] diff --git a/hack/generator/pkg/astmodel/resource_type.go b/hack/generator/pkg/astmodel/resource_type.go index c7fe081a153..86d73ca7f0b 100644 --- a/hack/generator/pkg/astmodel/resource_type.go +++ b/hack/generator/pkg/astmodel/resource_type.go @@ -362,6 +362,10 @@ func (resource *ResourceType) RequiredPackageReferences() *PackageReferenceSet { references.Merge(resource.status.RequiredPackageReferences()) } + for _, fn := range resource.functions { + references.Merge(fn.RequiredPackageReferences()) + } + // Interface imports references.Merge(resource.InterfaceImplementer.RequiredPackageReferences()) diff --git a/hack/generator/pkg/astmodel/std_references.go b/hack/generator/pkg/astmodel/std_references.go index b99a9800ad2..0bc5791f304 100644 --- a/hack/generator/pkg/astmodel/std_references.go +++ b/hack/generator/pkg/astmodel/std_references.go @@ -7,14 +7,14 @@ package astmodel var ( // References to standard Go Libraries - ErrorsReference PackageReference = MakeExternalPackageReference("errors") - FmtReference PackageReference = MakeExternalPackageReference("fmt") - JsonReference PackageReference = MakeExternalPackageReference("encoding/json") - ReflectReference PackageReference = MakeExternalPackageReference("reflect") - TestingReference PackageReference = MakeExternalPackageReference("testing") + ErrorsReference = MakeExternalPackageReference("errors") + FmtReference = MakeExternalPackageReference("fmt") + JsonReference = MakeExternalPackageReference("encoding/json") + ReflectReference = MakeExternalPackageReference("reflect") + TestingReference = MakeExternalPackageReference("testing") // References to our Libraries - GenRuntimeReference PackageReference = MakeExternalPackageReference(genRuntimePathPrefix) + GenRuntimeReference = MakeExternalPackageReference(genRuntimePathPrefix) // References to other libraries APIExtensionsReference = MakeExternalPackageReference("k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1") @@ -23,19 +23,20 @@ var ( APIMachineryRuntimeReference = MakeExternalPackageReference("k8s.io/apimachinery/pkg/runtime") ClientGoSchemeReference = MakeExternalPackageReference("k8s.io/client-go/kubernetes/scheme") ControllerRuntimeAdmission = MakeExternalPackageReference("sigs.k8s.io/controller-runtime/pkg/webhook/admission") + GitHubErrorsReference = MakeExternalPackageReference("github.com/pkg/errors") // References to libraries used for testing - CmpReference PackageReference = MakeExternalPackageReference("github.com/google/go-cmp/cmp") - CmpOptsReference PackageReference = MakeExternalPackageReference("github.com/google/go-cmp/cmp/cmpopts") - DiffReference PackageReference = MakeExternalPackageReference("github.com/kylelemons/godebug/diff") - GopterReference PackageReference = MakeExternalPackageReference("github.com/leanovate/gopter") - GopterGenReference PackageReference = MakeExternalPackageReference("github.com/leanovate/gopter/gen") - GopterPropReference PackageReference = MakeExternalPackageReference("github.com/leanovate/gopter/prop") - GomegaReference PackageReference = MakeExternalPackageReference("github.com/onsi/gomega") - PrettyReference PackageReference = MakeExternalPackageReference("github.com/kr/pretty") + CmpReference = MakeExternalPackageReference("github.com/google/go-cmp/cmp") + CmpOptsReference = MakeExternalPackageReference("github.com/google/go-cmp/cmp/cmpopts") + DiffReference = MakeExternalPackageReference("github.com/kylelemons/godebug/diff") + GopterReference = MakeExternalPackageReference("github.com/leanovate/gopter") + GopterGenReference = MakeExternalPackageReference("github.com/leanovate/gopter/gen") + GopterPropReference = MakeExternalPackageReference("github.com/leanovate/gopter/prop") + GomegaReference = MakeExternalPackageReference("github.com/onsi/gomega") + PrettyReference = MakeExternalPackageReference("github.com/kr/pretty") // Imports with specified names - GomegaImport PackageImport = NewPackageImport(GomegaReference).WithName(".") + GomegaImport = NewPackageImport(GomegaReference).WithName(".") // Type names ResourceReferenceTypeName = MakeTypeName(GenRuntimeReference, "ResourceReference") diff --git a/hack/generator/pkg/astmodel/storage_conversion_context.go b/hack/generator/pkg/astmodel/storage_conversion_context.go index b53f6c727f3..7c60ca449d4 100644 --- a/hack/generator/pkg/astmodel/storage_conversion_context.go +++ b/hack/generator/pkg/astmodel/storage_conversion_context.go @@ -12,21 +12,40 @@ type StorageConversionContext struct { types Types // functionName is the name of the function we're currently generating functionName string + // knownLocals is a reference to our set of local variables + // (Pointer because it's a reference type, not because it's optional) + knownLocals *KnownLocalsSet } // NewStorageConversionContext creates a new instance of a StorageConversionContext -func NewStorageConversionContext(types Types) *StorageConversionContext { +func NewStorageConversionContext(types Types, idFactory IdentifierFactory) *StorageConversionContext { return &StorageConversionContext{ - types: types, + types: types, + knownLocals: NewKnownLocalsSet(idFactory), } } +// WithFunctionName returns a new context with the specified function name included func (c *StorageConversionContext) WithFunctionName(name string) *StorageConversionContext { - result := NewStorageConversionContext(c.types) + result := c.clone() result.functionName = name return result } +// WithKnownLocals returns a new context with the specified known locals set referenced +func (c *StorageConversionContext) WithKnownLocals(knownLocals *KnownLocalsSet) *StorageConversionContext { + result := c.clone() + result.knownLocals = knownLocals + 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 *StorageConversionContext) NestedContext() *StorageConversionContext { + result := c.clone() + return result +} + // ResolveType resolves a type that might be a type name into both the name and the actual // type it references, returning true iff it was a TypeName that could be resolved func (c *StorageConversionContext) ResolveType(t Type) (TypeName, Type, bool) { @@ -42,3 +61,22 @@ func (c *StorageConversionContext) ResolveType(t Type) (TypeName, Type, bool) { return name, actualType, true } + +// TryCreateLocal returns true if the specified local was successfully created, false if it already existed +func (c *StorageConversionContext) TryCreateLocal(local string) bool { + if c.knownLocals.HasName(local) { + return false + } + + c.knownLocals.Add(local) + return true +} + +// clone returns a new independent copy of this context +func (c *StorageConversionContext) clone() *StorageConversionContext { + return &StorageConversionContext{ + types: c.types, + functionName: c.functionName, + knownLocals: c.knownLocals.Clone(), + } +} diff --git a/hack/generator/pkg/astmodel/storage_conversion_endpoint.go b/hack/generator/pkg/astmodel/storage_conversion_endpoint.go index 220035593ca..d26d211210d 100644 --- a/hack/generator/pkg/astmodel/storage_conversion_endpoint.go +++ b/hack/generator/pkg/astmodel/storage_conversion_endpoint.go @@ -6,8 +6,6 @@ package astmodel import ( - "strings" - "github.com/gobuffalo/flect" ) @@ -28,7 +26,7 @@ func NewStorageConversionEndpoint( knownLocals *KnownLocalsSet) *StorageConversionEndpoint { return &StorageConversionEndpoint{ theType: theType, - name: strings.ToLower(name), + name: name, knownLocals: knownLocals, } } diff --git a/hack/generator/pkg/astmodel/storage_conversion_factories.go b/hack/generator/pkg/astmodel/storage_conversion_factories.go index e25627323ad..d9fdd6350fa 100644 --- a/hack/generator/pkg/astmodel/storage_conversion_factories.go +++ b/hack/generator/pkg/astmodel/storage_conversion_factories.go @@ -7,6 +7,7 @@ package astmodel import ( "go/token" + "strings" "github.com/Azure/azure-service-operator/hack/generator/pkg/astbuilder" "github.com/dave/dst" @@ -48,6 +49,7 @@ func init() { assignObjectTypeFromObjectType, // Known types assignKnownReferenceFromKnownReference, + assignResourceReferenceFromResourceReference, // Meta-conversions assignFromOptionalType, // Must go before assignToOptionalType so we generate the right zero values assignToOptionalType, @@ -123,10 +125,18 @@ func createTypeConversion( } } + // No conversion found, we need to generate a useful error message + + var debugDescriptionOfDestination strings.Builder + destinationEndpoint.Type().WriteDebugDescription(&debugDescriptionOfDestination, conversionContext.types) + + var debugDescriptionOfSource strings.Builder + sourceEndpoint.Type().WriteDebugDescription(&debugDescriptionOfSource, conversionContext.types) + err := errors.Errorf( "no conversion found to assign %q from %q", - destinationEndpoint.name, - sourceEndpoint.name) + debugDescriptionOfDestination.String(), + debugDescriptionOfSource.String()) return nil, err } @@ -200,8 +210,14 @@ func assignFromOptionalType( } // Require a conversion between the unwrapped type and our source + // We supply a nested context as any code generated by the conversion will be within the if + // statement (a nested scope) and we don't want any variables declared in that scope to leak + // out elsewhere. unwrappedEndpoint := sourceEndpoint.WithType(sourceOptional.element) - conversion, _ := createTypeConversion(unwrappedEndpoint, destinationEndpoint, conversionContext) + conversion, _ := createTypeConversion( + unwrappedEndpoint, + destinationEndpoint, + conversionContext.NestedContext()) if conversion == nil { return nil } @@ -513,9 +529,14 @@ func assignArrayFromArray( } // Require a conversion between the array types + // We supply a nested context as any code generated by the conversion will be within the loop + // (a nested scope) and we don't want any variables declared in that scope to leak out elsewhere. unwrappedSourceEndpoint := sourceEndpoint.WithType(sourceArray.element) unwrappedDestinationEndpoint := destinationEndpoint.WithType(destinationArray.element) - conversion, _ := createTypeConversion(unwrappedSourceEndpoint, unwrappedDestinationEndpoint, conversionContext) + conversion, _ := createTypeConversion( + unwrappedSourceEndpoint, + unwrappedDestinationEndpoint, + conversionContext.NestedContext()) if conversion == nil { return nil } @@ -592,9 +613,14 @@ func assignMapFromMap( } // Require a conversion between the map items + // We supply a nested context as any code generated by the conversion will be within the loop + // (a nested scope) and we don't want any variables declared in that scope to leak out elsewhere. unwrappedSourceEndpoint := sourceEndpoint.WithType(sourceMap.value) unwrappedDestinationEndpoint := destinationEndpoint.WithType(destinationMap.value) - conversion, _ := createTypeConversion(unwrappedSourceEndpoint, unwrappedDestinationEndpoint, conversionContext) + conversion, _ := createTypeConversion( + unwrappedSourceEndpoint, + unwrappedDestinationEndpoint, + conversionContext.NestedContext()) if conversion == nil { return nil } @@ -804,9 +830,18 @@ func assignObjectTypeFromObjectType( return func(reader dst.Expr, writer func(dst.Expr) []dst.Stmt, generationContext *CodeGenerationContext) []dst.Stmt { + // We have to do this at render time in order to ensure the first conversion generated + // declares 'err', not a later one + tok := token.ASSIGN + if conversionContext.TryCreateLocal("err") { + tok = token.DEFINE + } + localId := dst.NewIdent(copyVar) errLocal := dst.NewIdent("err") + errorsPackageName := generationContext.MustGetImportedPackageName(GitHubErrorsReference) + declaration := astbuilder.LocalVariableDeclaration(copyVar, createTypeDeclaration(destinationName, generationContext), "") // If our reader is a dereference, we strip that off (because we need a pointer), else we @@ -820,20 +855,24 @@ func assignObjectTypeFromObjectType( var conversion dst.Stmt if destinationName.PackageReference.Equals(generationContext.CurrentPackage()) { + // Destination is our current type conversion = astbuilder.SimpleAssignment( errLocal, - token.DEFINE, + tok, astbuilder.CallExpr(localId, conversionContext.functionName, actualReader)) + } else { + // Destination is another type conversion = astbuilder.SimpleAssignment( errLocal, - token.DEFINE, - astbuilder.CallExpr(reader, conversionContext.functionName, localId)) + tok, + astbuilder.CallExpr(reader, conversionContext.functionName, astbuilder.AddrOf(localId))) } checkForError := astbuilder.ReturnIfNotNil( errLocal, astbuilder.WrappedErrorf( + errorsPackageName, "populating %s from %s, calling %s()", destinationEndpoint.name, sourceEndpoint.name, conversionContext.functionName)) @@ -874,7 +913,7 @@ func assignKnownReferenceFromKnownReference( } // Require source to be a KnownResourceReference - if sourceName.Name() != "KnownResourceReference" { + if sourceName.Name() != "KnownResourceReference" && sourceName.PackageReference.Equals(GenRuntimeReference) { return nil } @@ -888,6 +927,52 @@ func assignKnownReferenceFromKnownReference( } } +// assignResourceReferenceFromResourceReference will generate a direct assignment if both types are genruntime.ResourceReference +// +// = .Copy() +// +func assignResourceReferenceFromResourceReference( + sourceEndpoint *StorageConversionEndpoint, + destinationEndpoint *StorageConversionEndpoint, + _ *StorageConversionContext) StorageTypeConversion { + + // Require source to be non-optional + if _, sourceIsOptional := AsOptionalType(sourceEndpoint.Type()); sourceIsOptional { + return nil + } + + // Require destination to be non-optional + if _, destinationIsOptional := AsOptionalType(destinationEndpoint.Type()); destinationIsOptional { + return nil + } + + // Require source to be a named type + sourceName, sourceIsName := AsTypeName(sourceEndpoint.Type()) + if !sourceIsName { + return nil + } + + // Require destination to be a named type + destinationName, destinationIsName := AsTypeName(destinationEndpoint.Type()) + if !destinationIsName { + return nil + } + + // Require source to be a ResourceReference + if sourceName.Name() != "ResourceReference" && sourceName.PackageReference.Equals(GenRuntimeReference) { + return nil + } + + // Require destination to be a ResourceReference + if destinationName.Name() != "ResourceReference" && destinationName.PackageReference.Equals(GenRuntimeReference) { + return nil + } + + return func(reader dst.Expr, writer func(dst.Expr) []dst.Stmt, generationContext *CodeGenerationContext) []dst.Stmt { + return writer(astbuilder.CallExpr(reader, "Copy")) + } +} + func createTypeDeclaration(name TypeName, generationContext *CodeGenerationContext) dst.Expr { if name.PackageReference.Equals(generationContext.CurrentPackage()) { return dst.NewIdent(name.Name()) diff --git a/hack/generator/pkg/astmodel/storage_conversion_function.go b/hack/generator/pkg/astmodel/storage_conversion_function.go index e7be50137b9..eec9529d46c 100644 --- a/hack/generator/pkg/astmodel/storage_conversion_function.go +++ b/hack/generator/pkg/astmodel/storage_conversion_function.go @@ -9,27 +9,20 @@ import ( "fmt" "go/token" "sort" + "strings" "github.com/Azure/azure-service-operator/hack/generator/pkg/astbuilder" "github.com/dave/dst" "github.com/pkg/errors" - kerrors "k8s.io/apimachinery/pkg/util/errors" ) -// StoragePropertyConversion represents a function that generates the correct AST to convert a single property value -// Different functions will be used, depending on the types of the properties to be converted. -// source is an expression for the source value that will be read. -// destination is an expression the target value that will be written. -type StoragePropertyConversion func( - source dst.Expr, destination dst.Expr, generationContext *CodeGenerationContext) []dst.Stmt - // StorageConversionFunction represents a function that performs conversions for storage versions type StorageConversionFunction struct { // name of this conversion function name string - // otherType is the type we are converting to (or from). This will be a type which is "closer" + // 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. - otherType TypeDefinition + otherDefinition TypeDefinition // conversions is a map of all property conversions we are going to use, keyed by name of the // receiver property conversions map[string]StoragePropertyConversion @@ -43,13 +36,20 @@ type StorageConversionFunction struct { conversionContext *StorageConversionContext } +// StoragePropertyConversion represents a function that generates the correct AST to convert a single property value +// Different functions will be used, depending on the types of the properties to be converted. +// source is an expression for the source value that will be read. +// destination is an expression the target value that will be written. +type StoragePropertyConversion func( + source dst.Expr, destination dst.Expr, generationContext *CodeGenerationContext) []dst.Stmt + // StorageConversionDirection specifies the direction of conversion we're implementing with this function type StorageConversionDirection int const ( - // Indicates the conversion is from the passed other instance, populating the receiver + // ConvertFrom indicates the conversion is from the passed other instance, populating the receiver ConvertFrom = StorageConversionDirection(1) - // Indicate the conversion is to the passed other type, populating other + // ConvertTo indicates the conversion is to the passed other type, populating other ConvertTo = StorageConversionDirection(2) ) @@ -59,23 +59,23 @@ var _ Function = &StorageConversionFunction{} // NewStorageConversionFromFunction creates a new StorageConversionFunction to convert from the specified source func NewStorageConversionFromFunction( receiver TypeDefinition, - otherType TypeDefinition, + otherDefinition TypeDefinition, idFactory IdentifierFactory, conversionContext *StorageConversionContext, ) (*StorageConversionFunction, error) { result := &StorageConversionFunction{ - otherType: otherType, + otherDefinition: otherDefinition, idFactory: idFactory, conversionDirection: ConvertFrom, conversions: make(map[string]StoragePropertyConversion), knownLocals: NewKnownLocalsSet(idFactory), } - version := idFactory.CreateIdentifier(otherType.Name().PackageReference.PackageName(), Exported) + version := idFactory.CreateIdentifier(otherDefinition.Name().PackageReference.PackageName(), Exported) result.name = "ConvertFrom" + version - result.conversionContext = conversionContext.WithFunctionName(result.name) + result.conversionContext = conversionContext.WithFunctionName(result.name).WithKnownLocals(result.knownLocals) - err := result.createConversions(receiver) + err := result.createConversions(receiver, conversionContext.types) if err != nil { return nil, errors.Wrapf(err, "creating '%s()'", result.name) } @@ -86,23 +86,23 @@ func NewStorageConversionFromFunction( // NewStorageConversionToFunction creates a new StorageConversionFunction to convert to the specified destination func NewStorageConversionToFunction( receiver TypeDefinition, - otherType TypeDefinition, + otherDefinition TypeDefinition, idFactory IdentifierFactory, conversionContext *StorageConversionContext, ) (*StorageConversionFunction, error) { result := &StorageConversionFunction{ - otherType: otherType, + otherDefinition: otherDefinition, idFactory: idFactory, conversionDirection: ConvertTo, conversions: make(map[string]StoragePropertyConversion), knownLocals: NewKnownLocalsSet(idFactory), } - version := idFactory.CreateIdentifier(otherType.Name().PackageReference.PackageName(), Exported) + version := idFactory.CreateIdentifier(otherDefinition.Name().PackageReference.PackageName(), Exported) result.name = "ConvertTo" + version - result.conversionContext = conversionContext.WithFunctionName(result.name) + result.conversionContext = conversionContext.WithFunctionName(result.name).WithKnownLocals(result.knownLocals) - err := result.createConversions(receiver) + err := result.createConversions(receiver, conversionContext.types) if err != nil { return nil, errors.Wrapf(err, "creating '%s()'", result.name) } @@ -118,15 +118,15 @@ func (fn *StorageConversionFunction) Name() string { // RequiredPackageReferences returns the set of package references required by this function func (fn *StorageConversionFunction) RequiredPackageReferences() *PackageReferenceSet { result := NewPackageReferenceSet( - ErrorsReference, - fn.otherType.Name().PackageReference) + GitHubErrorsReference, + fn.otherDefinition.Name().PackageReference) return result } // References returns the set of types referenced by this function func (fn *StorageConversionFunction) References() TypeNameSet { - return NewTypeNameSet(fn.otherType.Name()) + return NewTypeNameSet(fn.otherDefinition.Name()) } // Equals checks to see if the supplied function is the same as this one @@ -163,31 +163,35 @@ func (fn *StorageConversionFunction) AsFunc(generationContext *CodeGenerationCon switch fn.conversionDirection { case ConvertFrom: parameterName = "source" - description = fmt.Sprintf("populates our %s from the provided source %s", receiver.Name(), fn.otherType.Name().Name()) + description = fmt.Sprintf("populates our %s from the provided source %s", receiver.Name(), fn.otherDefinition.Name().Name()) case ConvertTo: parameterName = "destination" - description = fmt.Sprintf("populates the provided destination %s from our %s", fn.otherType.Name().Name(), receiver.Name()) + description = fmt.Sprintf("populates the provided destination %s from our %s", fn.otherDefinition.Name().Name(), receiver.Name()) default: panic(fmt.Sprintf("unexpected conversion direction %q", fn.conversionDirection)) } + // Create a sensible name for our receiver receiverName := fn.idFactory.CreateIdentifier(receiver.Name(), NotExported) + // We always use a pointer receiver so we can modify it + receiverType := NewOptionalType(receiver).AsType(generationContext) + funcDetails := &astbuilder.FuncDetails{ ReceiverIdent: receiverName, - ReceiverType: receiver.AsType(generationContext), + ReceiverType: receiverType, Name: fn.Name(), Body: fn.generateBody(receiverName, parameterName, generationContext), } - parameterPackage := generationContext.MustGetImportedPackageName(fn.otherType.Name().PackageReference) + parameterPackage := generationContext.MustGetImportedPackageName(fn.otherDefinition.Name().PackageReference) funcDetails.AddParameter( parameterName, &dst.StarExpr{ X: &dst.SelectorExpr{ X: dst.NewIdent(parameterPackage), - Sel: dst.NewIdent(fn.otherType.Name().Name()), + Sel: dst.NewIdent(fn.otherDefinition.Name().Name()), }, }) @@ -276,29 +280,38 @@ func (fn *StorageConversionFunction) generateAssignments( // createConversions iterates through the properties on our receiver type, matching them up with // our other type and generating conversions where possible -func (fn *StorageConversionFunction) createConversions(receiver TypeDefinition) error { +func (fn *StorageConversionFunction) createConversions(receiver TypeDefinition, types Types) error { - receiverObject, ok := AsObjectType(receiver.Type()) + receiverContainer, ok := fn.asPropertyContainer(receiver.theType) if !ok { + var typeDescription strings.Builder + receiver.Type().WriteDebugDescription(&typeDescription, types) + return errors.Errorf( - "expected TypeDefinition %q to wrap receiver object type, but found %q", + "expected receiver TypeDefinition %q to be either resource or object type, but found %q", receiver.name.String(), - receiver.Type()) + typeDescription.String()) } - var otherObject *ObjectType - otherObject, ok = AsObjectType(fn.otherType.Type()) + otherContainer, ok := fn.asPropertyContainer(fn.otherDefinition.theType) if !ok { - return errors.Errorf("expected TypeDefinition %q to wrap object type, but none found", fn.otherType.Name().String()) + var typeDescription strings.Builder + fn.otherDefinition.Type().WriteDebugDescription(&typeDescription, types) + + return errors.Errorf( + "expected other TypeDefinition %q to be either resource or object type, but found %q", + fn.otherDefinition.Name().String(), + typeDescription.String()) } - var errs []error + receiverProperties := fn.createPropertyMap(receiverContainer) + otherProperties := fn.createPropertyMap(otherContainer) // Flag receiver name as used fn.knownLocals.Add(receiver.Name().Name()) - for _, receiverProperty := range receiverObject.Properties() { - otherProperty, ok := otherObject.Property(receiverProperty.propertyName) + for _, receiverProperty := range receiverProperties { + otherProperty, ok := otherProperties[receiverProperty.propertyName] //TODO: Handle renames if ok { var conv StoragePropertyConversion @@ -313,19 +326,38 @@ func (fn *StorageConversionFunction) createConversions(receiver TypeDefinition) } if err != nil { - // An error was returned; this can happen even if a conversion was created as well. - errs = append(errs, err) - continue - } - - if conv != nil { + // An error was returned, we abort creating conversions for this object + return errors.Wrapf(err, "creating conversion for property %q of %q", receiverProperty.PropertyName(), receiver.Name()) + } else if conv != nil { // A conversion was created, keep it for later fn.conversions[string(receiverProperty.propertyName)] = conv } } } - return kerrors.NewAggregate(errs) + return nil +} + +// asPropertyContainer converts a type into a property container +func (fn *StorageConversionFunction) asPropertyContainer(theType Type) (PropertyContainer, bool) { + switch t := theType.(type) { + case PropertyContainer: + return t, true + case MetaType: + return fn.asPropertyContainer(t.Unwrap()) + default: + return nil, false + } +} + +// createPropertyMap extracts the properties from a PropertyContainer and returns them as a map +func (fn *StorageConversionFunction) createPropertyMap(container PropertyContainer) map[PropertyName]*PropertyDefinition { + result := make(map[PropertyName]*PropertyDefinition) + for _, p := range container.Properties() { + result[p.PropertyName()] = p + } + + return result } // createPropertyConversion tries to create a property conversion between the two provided properties, using all of the @@ -344,7 +376,7 @@ func (fn *StorageConversionFunction) createPropertyConversion( if err != nil { return nil, errors.Wrapf( err, - "trying to assign %q (%s) from %q (%s)", + "trying to assign %q [%s] by converting from %q [%s]", destinationProperty.PropertyName(), destinationProperty.PropertyType(), sourceProperty.PropertyName(), diff --git a/hack/generator/pkg/astmodel/storage_conversion_function_test.go b/hack/generator/pkg/astmodel/storage_conversion_function_test.go index 3c3a60e046e..ba5740668bb 100644 --- a/hack/generator/pkg/astmodel/storage_conversion_function_test.go +++ b/hack/generator/pkg/astmodel/storage_conversion_function_test.go @@ -167,7 +167,7 @@ func RunTestStorageConversionFunction_AsFunc(c *StorageConversionPropertyTestCas idFactory := NewIdentifierFactory() - conversionContext := NewStorageConversionContext(c.types) + conversionContext := NewStorageConversionContext(c.types, idFactory) currentType, ok := AsObjectType(c.currentObject.Type()) g.Expect(ok).To(BeTrue()) diff --git a/hack/generator/pkg/astmodel/storage_package_reference.go b/hack/generator/pkg/astmodel/storage_package_reference.go index daf4102ce4e..dd36b5489a7 100644 --- a/hack/generator/pkg/astmodel/storage_package_reference.go +++ b/hack/generator/pkg/astmodel/storage_package_reference.go @@ -33,7 +33,7 @@ func MakeStoragePackageReference(local LocalPackageReference) StoragePackageRefe // String returns the string representation of the package reference func (s StoragePackageReference) String() string { - return fmt.Sprintf("storage:%v", s.PackagePath()) + return fmt.Sprintf("storage:%s/%s", s.group, s.version) } // IsPreview returns true if this package reference is a preview diff --git a/hack/generator/pkg/astmodel/testdata/ConvertBetweenAliasAndAliasType.golden b/hack/generator/pkg/astmodel/testdata/ConvertBetweenAliasAndAliasType.golden index 2b8ca78e9b3..28318dde7e3 100644 --- a/hack/generator/pkg/astmodel/testdata/ConvertBetweenAliasAndAliasType.golden +++ b/hack/generator/pkg/astmodel/testdata/ConvertBetweenAliasAndAliasType.golden @@ -10,7 +10,7 @@ type Person struct { } // ConvertFromVNext populates our Person from the provided source Person -func (person Person) ConvertFromVNext(source *vNext.Person) error { +func (person *Person) ConvertFromVNext(source *vNext.Person) error { // age person.age = Age(source.age) @@ -20,7 +20,7 @@ func (person Person) ConvertFromVNext(source *vNext.Person) error { } // ConvertToVNext populates the provided destination Person from our Person -func (person Person) ConvertToVNext(destination *vNext.Person) error { +func (person *Person) ConvertToVNext(destination *vNext.Person) error { // age destination.age = vNext.Age(person.age) diff --git a/hack/generator/pkg/astmodel/testdata/ConvertBetweenAliasAndBaseType.golden b/hack/generator/pkg/astmodel/testdata/ConvertBetweenAliasAndBaseType.golden index 5507c3c28ef..e52686a36a9 100644 --- a/hack/generator/pkg/astmodel/testdata/ConvertBetweenAliasAndBaseType.golden +++ b/hack/generator/pkg/astmodel/testdata/ConvertBetweenAliasAndBaseType.golden @@ -10,7 +10,7 @@ type Person struct { } // ConvertFromVNext populates our Person from the provided source Person -func (person Person) ConvertFromVNext(source *vNext.Person) error { +func (person *Person) ConvertFromVNext(source *vNext.Person) error { // age person.age = Age(source.age) @@ -20,7 +20,7 @@ func (person Person) ConvertFromVNext(source *vNext.Person) error { } // ConvertToVNext populates the provided destination Person from our Person -func (person Person) ConvertToVNext(destination *vNext.Person) error { +func (person *Person) ConvertToVNext(destination *vNext.Person) error { // age destination.age = int(person.age) diff --git a/hack/generator/pkg/astmodel/testdata/ConvertBetweenAliasAndOptionalAliasType.golden b/hack/generator/pkg/astmodel/testdata/ConvertBetweenAliasAndOptionalAliasType.golden index d4507a7f398..b9286182b77 100644 --- a/hack/generator/pkg/astmodel/testdata/ConvertBetweenAliasAndOptionalAliasType.golden +++ b/hack/generator/pkg/astmodel/testdata/ConvertBetweenAliasAndOptionalAliasType.golden @@ -10,7 +10,7 @@ type Person struct { } // ConvertFromVNext populates our Person from the provided source Person -func (person Person) ConvertFromVNext(source *vNext.Person) error { +func (person *Person) ConvertFromVNext(source *vNext.Person) error { // age if source.age != nil { @@ -24,7 +24,7 @@ func (person Person) ConvertFromVNext(source *vNext.Person) error { } // ConvertToVNext populates the provided destination Person from our Person -func (person Person) ConvertToVNext(destination *vNext.Person) error { +func (person *Person) ConvertToVNext(destination *vNext.Person) error { // age age := vNext.Age(person.age) diff --git a/hack/generator/pkg/astmodel/testdata/ConvertBetweenEnumAndBaseType.golden b/hack/generator/pkg/astmodel/testdata/ConvertBetweenEnumAndBaseType.golden index a4b7c8fa7da..151d85d3d40 100644 --- a/hack/generator/pkg/astmodel/testdata/ConvertBetweenEnumAndBaseType.golden +++ b/hack/generator/pkg/astmodel/testdata/ConvertBetweenEnumAndBaseType.golden @@ -10,7 +10,7 @@ type Person struct { } // ConvertFromVNext populates our Person from the provided source Person -func (person Person) ConvertFromVNext(source *vNext.Person) error { +func (person *Person) ConvertFromVNext(source *vNext.Person) error { // sku person.sku = string(source.sku) @@ -20,7 +20,7 @@ func (person Person) ConvertFromVNext(source *vNext.Person) error { } // ConvertToVNext populates the provided destination Person from our Person -func (person Person) ConvertToVNext(destination *vNext.Person) error { +func (person *Person) ConvertToVNext(destination *vNext.Person) error { // sku destination.sku = vNext.Bucket(person.sku) diff --git a/hack/generator/pkg/astmodel/testdata/ConvertBetweenEnumAndOptionalBaseType.golden b/hack/generator/pkg/astmodel/testdata/ConvertBetweenEnumAndOptionalBaseType.golden index 87f22d64883..979dbdb1c79 100644 --- a/hack/generator/pkg/astmodel/testdata/ConvertBetweenEnumAndOptionalBaseType.golden +++ b/hack/generator/pkg/astmodel/testdata/ConvertBetweenEnumAndOptionalBaseType.golden @@ -10,7 +10,7 @@ type Person struct { } // ConvertFromVNext populates our Person from the provided source Person -func (person Person) ConvertFromVNext(source *vNext.Person) error { +func (person *Person) ConvertFromVNext(source *vNext.Person) error { // sku sku := string(source.sku) @@ -21,7 +21,7 @@ func (person Person) ConvertFromVNext(source *vNext.Person) error { } // ConvertToVNext populates the provided destination Person from our Person -func (person Person) ConvertToVNext(destination *vNext.Person) error { +func (person *Person) ConvertToVNext(destination *vNext.Person) error { // sku if person.sku != nil { diff --git a/hack/generator/pkg/astmodel/testdata/ConvertBetweenOptionalAliasAndBaseType.golden b/hack/generator/pkg/astmodel/testdata/ConvertBetweenOptionalAliasAndBaseType.golden index b6388df860c..ed4ebb45e34 100644 --- a/hack/generator/pkg/astmodel/testdata/ConvertBetweenOptionalAliasAndBaseType.golden +++ b/hack/generator/pkg/astmodel/testdata/ConvertBetweenOptionalAliasAndBaseType.golden @@ -10,7 +10,7 @@ type Person struct { } // ConvertFromVNext populates our Person from the provided source Person -func (person Person) ConvertFromVNext(source *vNext.Person) error { +func (person *Person) ConvertFromVNext(source *vNext.Person) error { // age age := Age(source.age) @@ -21,7 +21,7 @@ func (person Person) ConvertFromVNext(source *vNext.Person) error { } // ConvertToVNext populates the provided destination Person from our Person -func (person Person) ConvertToVNext(destination *vNext.Person) error { +func (person *Person) ConvertToVNext(destination *vNext.Person) error { // age if person.age != nil { diff --git a/hack/generator/pkg/astmodel/testdata/ConvertBetweenOptionalAliasAndOptionalAliasType.golden b/hack/generator/pkg/astmodel/testdata/ConvertBetweenOptionalAliasAndOptionalAliasType.golden index 126514118e4..c28d0a4aa96 100644 --- a/hack/generator/pkg/astmodel/testdata/ConvertBetweenOptionalAliasAndOptionalAliasType.golden +++ b/hack/generator/pkg/astmodel/testdata/ConvertBetweenOptionalAliasAndOptionalAliasType.golden @@ -10,7 +10,7 @@ type Person struct { } // ConvertFromVNext populates our Person from the provided source Person -func (person Person) ConvertFromVNext(source *vNext.Person) error { +func (person *Person) ConvertFromVNext(source *vNext.Person) error { // age if source.age != nil { @@ -25,7 +25,7 @@ func (person Person) ConvertFromVNext(source *vNext.Person) error { } // ConvertToVNext populates the provided destination Person from our Person -func (person Person) ConvertToVNext(destination *vNext.Person) error { +func (person *Person) ConvertToVNext(destination *vNext.Person) error { // age if person.age != nil { diff --git a/hack/generator/pkg/astmodel/testdata/ConvertBetweenOptionalAliasAndOptionalBaseType.golden b/hack/generator/pkg/astmodel/testdata/ConvertBetweenOptionalAliasAndOptionalBaseType.golden index 45671365c74..6e79ac84a49 100644 --- a/hack/generator/pkg/astmodel/testdata/ConvertBetweenOptionalAliasAndOptionalBaseType.golden +++ b/hack/generator/pkg/astmodel/testdata/ConvertBetweenOptionalAliasAndOptionalBaseType.golden @@ -10,7 +10,7 @@ type Person struct { } // ConvertFromVNext populates our Person from the provided source Person -func (person Person) ConvertFromVNext(source *vNext.Person) error { +func (person *Person) ConvertFromVNext(source *vNext.Person) error { // age if source.age != nil { @@ -25,7 +25,7 @@ func (person Person) ConvertFromVNext(source *vNext.Person) error { } // ConvertToVNext populates the provided destination Person from our Person -func (person Person) ConvertToVNext(destination *vNext.Person) error { +func (person *Person) ConvertToVNext(destination *vNext.Person) error { // age if person.age != nil { diff --git a/hack/generator/pkg/astmodel/testdata/ConvertBetweenOptionalEnumAndBaseType.golden b/hack/generator/pkg/astmodel/testdata/ConvertBetweenOptionalEnumAndBaseType.golden index c4c12af18e7..7fd311fc495 100644 --- a/hack/generator/pkg/astmodel/testdata/ConvertBetweenOptionalEnumAndBaseType.golden +++ b/hack/generator/pkg/astmodel/testdata/ConvertBetweenOptionalEnumAndBaseType.golden @@ -10,7 +10,7 @@ type Person struct { } // ConvertFromVNext populates our Person from the provided source Person -func (person Person) ConvertFromVNext(source *vNext.Person) error { +func (person *Person) ConvertFromVNext(source *vNext.Person) error { // sku if source.sku != nil { @@ -24,7 +24,7 @@ func (person Person) ConvertFromVNext(source *vNext.Person) error { } // ConvertToVNext populates the provided destination Person from our Person -func (person Person) ConvertToVNext(destination *vNext.Person) error { +func (person *Person) ConvertToVNext(destination *vNext.Person) error { // sku sku := vNext.Bucket(person.sku) diff --git a/hack/generator/pkg/astmodel/testdata/ConvertBetweenOptionalEnumAndOptionalBaseType.golden b/hack/generator/pkg/astmodel/testdata/ConvertBetweenOptionalEnumAndOptionalBaseType.golden index 7e6ab95cfba..2d1755dc949 100644 --- a/hack/generator/pkg/astmodel/testdata/ConvertBetweenOptionalEnumAndOptionalBaseType.golden +++ b/hack/generator/pkg/astmodel/testdata/ConvertBetweenOptionalEnumAndOptionalBaseType.golden @@ -10,7 +10,7 @@ type Person struct { } // ConvertFromVNext populates our Person from the provided source Person -func (person Person) ConvertFromVNext(source *vNext.Person) error { +func (person *Person) ConvertFromVNext(source *vNext.Person) error { // sku if source.sku != nil { @@ -25,7 +25,7 @@ func (person Person) ConvertFromVNext(source *vNext.Person) error { } // ConvertToVNext populates the provided destination Person from our Person -func (person Person) ConvertToVNext(destination *vNext.Person) error { +func (person *Person) ConvertToVNext(destination *vNext.Person) error { // sku if person.sku != nil { diff --git a/hack/generator/pkg/astmodel/testdata/ConvertBetweenOptionalEnumAndOptionalEnum.golden b/hack/generator/pkg/astmodel/testdata/ConvertBetweenOptionalEnumAndOptionalEnum.golden index 1cae259eaf9..47c9b8367bf 100644 --- a/hack/generator/pkg/astmodel/testdata/ConvertBetweenOptionalEnumAndOptionalEnum.golden +++ b/hack/generator/pkg/astmodel/testdata/ConvertBetweenOptionalEnumAndOptionalEnum.golden @@ -10,7 +10,7 @@ type Person struct { } // ConvertFromVNext populates our Person from the provided source Person -func (person Person) ConvertFromVNext(source *vNext.Person) error { +func (person *Person) ConvertFromVNext(source *vNext.Person) error { // release if source.release != nil { @@ -25,7 +25,7 @@ func (person Person) ConvertFromVNext(source *vNext.Person) error { } // ConvertToVNext populates the provided destination Person from our Person -func (person Person) ConvertToVNext(destination *vNext.Person) error { +func (person *Person) ConvertToVNext(destination *vNext.Person) error { // release if person.release != nil { diff --git a/hack/generator/pkg/astmodel/testdata/ConvertBetweenOptionalObjectAndOptionalObject.golden b/hack/generator/pkg/astmodel/testdata/ConvertBetweenOptionalObjectAndOptionalObject.golden index e8591f75d64..71879f7e4d4 100644 --- a/hack/generator/pkg/astmodel/testdata/ConvertBetweenOptionalObjectAndOptionalObject.golden +++ b/hack/generator/pkg/astmodel/testdata/ConvertBetweenOptionalObjectAndOptionalObject.golden @@ -3,14 +3,17 @@ // Licensed under the MIT license. package vCurrent -import "github.com/Azure/azure-service-operator/hack/generated/_apis/Verification/vNext" +import ( + "github.com/Azure/azure-service-operator/hack/generated/_apis/Verification/vNext" + "github.com/pkg/errors" +) type Person struct { role *Release `json:"role"` } // ConvertFromVNext populates our Person from the provided source Person -func (person Person) ConvertFromVNext(source *vNext.Person) error { +func (person *Person) ConvertFromVNext(source *vNext.Person) error { // role if source.role != nil { @@ -29,12 +32,12 @@ func (person Person) ConvertFromVNext(source *vNext.Person) error { } // ConvertToVNext populates the provided destination Person from our Person -func (person Person) ConvertToVNext(destination *vNext.Person) error { +func (person *Person) ConvertToVNext(destination *vNext.Person) error { // role if person.role != nil { var role vNext.Release - err := (*person.role).ConvertToVNext(role) + err := (*person.role).ConvertToVNext(&role) if err != nil { return errors.Wrap(err, "populating role from role, calling ConvertToVNext()") } diff --git a/hack/generator/pkg/astmodel/testdata/ConvertBetweenRequiredEnumAndOptionalEnum.golden b/hack/generator/pkg/astmodel/testdata/ConvertBetweenRequiredEnumAndOptionalEnum.golden index 766fa5c81a3..155f25746c1 100644 --- a/hack/generator/pkg/astmodel/testdata/ConvertBetweenRequiredEnumAndOptionalEnum.golden +++ b/hack/generator/pkg/astmodel/testdata/ConvertBetweenRequiredEnumAndOptionalEnum.golden @@ -10,7 +10,7 @@ type Person struct { } // ConvertFromVNext populates our Person from the provided source Person -func (person Person) ConvertFromVNext(source *vNext.Person) error { +func (person *Person) ConvertFromVNext(source *vNext.Person) error { // release if source.release != nil { @@ -25,7 +25,7 @@ func (person Person) ConvertFromVNext(source *vNext.Person) error { } // ConvertToVNext populates the provided destination Person from our Person -func (person Person) ConvertToVNext(destination *vNext.Person) error { +func (person *Person) ConvertToVNext(destination *vNext.Person) error { // release release := Bucket(person.release) diff --git a/hack/generator/pkg/astmodel/testdata/ConvertBetweenRequiredEnumAndRequiredEnum.golden b/hack/generator/pkg/astmodel/testdata/ConvertBetweenRequiredEnumAndRequiredEnum.golden index 0feb7688144..4ed51d4b221 100644 --- a/hack/generator/pkg/astmodel/testdata/ConvertBetweenRequiredEnumAndRequiredEnum.golden +++ b/hack/generator/pkg/astmodel/testdata/ConvertBetweenRequiredEnumAndRequiredEnum.golden @@ -10,7 +10,7 @@ type Person struct { } // ConvertFromVNext populates our Person from the provided source Person -func (person Person) ConvertFromVNext(source *vNext.Person) error { +func (person *Person) ConvertFromVNext(source *vNext.Person) error { // release release := Bucket(source.release) @@ -21,7 +21,7 @@ func (person Person) ConvertFromVNext(source *vNext.Person) error { } // ConvertToVNext populates the provided destination Person from our Person -func (person Person) ConvertToVNext(destination *vNext.Person) error { +func (person *Person) ConvertToVNext(destination *vNext.Person) error { // release release := Bucket(person.release) diff --git a/hack/generator/pkg/astmodel/testdata/ConvertBetweenRequiredObjectAndOptionalObject.golden b/hack/generator/pkg/astmodel/testdata/ConvertBetweenRequiredObjectAndOptionalObject.golden index ec385c54422..d949d0863ca 100644 --- a/hack/generator/pkg/astmodel/testdata/ConvertBetweenRequiredObjectAndOptionalObject.golden +++ b/hack/generator/pkg/astmodel/testdata/ConvertBetweenRequiredObjectAndOptionalObject.golden @@ -3,14 +3,17 @@ // Licensed under the MIT license. package vCurrent -import "github.com/Azure/azure-service-operator/hack/generated/_apis/Verification/vNext" +import ( + "github.com/Azure/azure-service-operator/hack/generated/_apis/Verification/vNext" + "github.com/pkg/errors" +) type Person struct { role Release `json:"role"` } // ConvertFromVNext populates our Person from the provided source Person -func (person Person) ConvertFromVNext(source *vNext.Person) error { +func (person *Person) ConvertFromVNext(source *vNext.Person) error { // role if source.role != nil { @@ -29,11 +32,11 @@ func (person Person) ConvertFromVNext(source *vNext.Person) error { } // ConvertToVNext populates the provided destination Person from our Person -func (person Person) ConvertToVNext(destination *vNext.Person) error { +func (person *Person) ConvertToVNext(destination *vNext.Person) error { // role var role vNext.Release - err := person.role.ConvertToVNext(role) + err := person.role.ConvertToVNext(&role) if err != nil { return errors.Wrap(err, "populating role from role, calling ConvertToVNext()") } diff --git a/hack/generator/pkg/astmodel/testdata/ConvertBetweenRequiredObjectAndRequiredObject.golden b/hack/generator/pkg/astmodel/testdata/ConvertBetweenRequiredObjectAndRequiredObject.golden index 30b0e2f3a71..13652e676ab 100644 --- a/hack/generator/pkg/astmodel/testdata/ConvertBetweenRequiredObjectAndRequiredObject.golden +++ b/hack/generator/pkg/astmodel/testdata/ConvertBetweenRequiredObjectAndRequiredObject.golden @@ -3,14 +3,17 @@ // Licensed under the MIT license. package vCurrent -import "github.com/Azure/azure-service-operator/hack/generated/_apis/Verification/vNext" +import ( + "github.com/Azure/azure-service-operator/hack/generated/_apis/Verification/vNext" + "github.com/pkg/errors" +) type Person struct { role Release `json:"role"` } // ConvertFromVNext populates our Person from the provided source Person -func (person Person) ConvertFromVNext(source *vNext.Person) error { +func (person *Person) ConvertFromVNext(source *vNext.Person) error { // role var role Release @@ -25,11 +28,11 @@ func (person Person) ConvertFromVNext(source *vNext.Person) error { } // ConvertToVNext populates the provided destination Person from our Person -func (person Person) ConvertToVNext(destination *vNext.Person) error { +func (person *Person) ConvertToVNext(destination *vNext.Person) error { // role var role vNext.Release - err := person.role.ConvertToVNext(role) + err := person.role.ConvertToVNext(&role) if err != nil { return errors.Wrap(err, "populating role from role, calling ConvertToVNext()") } diff --git a/hack/generator/pkg/astmodel/testdata/NastyTest.golden b/hack/generator/pkg/astmodel/testdata/NastyTest.golden index 5df4413eb1b..5680232274f 100644 --- a/hack/generator/pkg/astmodel/testdata/NastyTest.golden +++ b/hack/generator/pkg/astmodel/testdata/NastyTest.golden @@ -10,7 +10,7 @@ type Person struct { } // ConvertFromVNext populates our Person from the provided source Person -func (person Person) ConvertFromVNext(source *vNext.Person) error { +func (person *Person) ConvertFromVNext(source *vNext.Person) error { // nasty nastyMap := make(map[string][]map[string]bool) @@ -38,7 +38,7 @@ func (person Person) ConvertFromVNext(source *vNext.Person) error { } // ConvertToVNext populates the provided destination Person from our Person -func (person Person) ConvertToVNext(destination *vNext.Person) error { +func (person *Person) ConvertToVNext(destination *vNext.Person) error { // nasty nastyMap := make(map[string][]map[string]bool) diff --git a/hack/generator/pkg/astmodel/testdata/SetArrayOfOptionalFromArrayOfRequired.golden b/hack/generator/pkg/astmodel/testdata/SetArrayOfOptionalFromArrayOfRequired.golden index ebd8214beca..3be053b478f 100644 --- a/hack/generator/pkg/astmodel/testdata/SetArrayOfOptionalFromArrayOfRequired.golden +++ b/hack/generator/pkg/astmodel/testdata/SetArrayOfOptionalFromArrayOfRequired.golden @@ -10,7 +10,7 @@ type Person struct { } // ConvertFromVNext populates our Person from the provided source Person -func (person Person) ConvertFromVNext(source *vNext.Person) error { +func (person *Person) ConvertFromVNext(source *vNext.Person) error { // scores scoreList := make([]*int, len(source.scores)) @@ -26,7 +26,7 @@ func (person Person) ConvertFromVNext(source *vNext.Person) error { } // ConvertToVNext populates the provided destination Person from our Person -func (person Person) ConvertToVNext(destination *vNext.Person) error { +func (person *Person) ConvertToVNext(destination *vNext.Person) error { // scores scoreList := make([]int, len(person.scores)) diff --git a/hack/generator/pkg/astmodel/testdata/SetArrayOfRequiredFromArrayOfOptional.golden b/hack/generator/pkg/astmodel/testdata/SetArrayOfRequiredFromArrayOfOptional.golden index ee4dbac985a..9e1b25fd151 100644 --- a/hack/generator/pkg/astmodel/testdata/SetArrayOfRequiredFromArrayOfOptional.golden +++ b/hack/generator/pkg/astmodel/testdata/SetArrayOfRequiredFromArrayOfOptional.golden @@ -10,7 +10,7 @@ type Person struct { } // ConvertFromVNext populates our Person from the provided source Person -func (person Person) ConvertFromVNext(source *vNext.Person) error { +func (person *Person) ConvertFromVNext(source *vNext.Person) error { // scores scoreList := make([]int, len(source.scores)) @@ -30,7 +30,7 @@ func (person Person) ConvertFromVNext(source *vNext.Person) error { } // ConvertToVNext populates the provided destination Person from our Person -func (person Person) ConvertToVNext(destination *vNext.Person) error { +func (person *Person) ConvertToVNext(destination *vNext.Person) error { // scores scoreList := make([]*int, len(person.scores)) diff --git a/hack/generator/pkg/astmodel/testdata/SetArrayOfRequiredFromArrayOfRequired.golden b/hack/generator/pkg/astmodel/testdata/SetArrayOfRequiredFromArrayOfRequired.golden index 3900b5bbdfc..381c1f08ad0 100644 --- a/hack/generator/pkg/astmodel/testdata/SetArrayOfRequiredFromArrayOfRequired.golden +++ b/hack/generator/pkg/astmodel/testdata/SetArrayOfRequiredFromArrayOfRequired.golden @@ -10,7 +10,7 @@ type Person struct { } // ConvertFromVNext populates our Person from the provided source Person -func (person Person) ConvertFromVNext(source *vNext.Person) error { +func (person *Person) ConvertFromVNext(source *vNext.Person) error { // scores scoreList := make([]int, len(source.scores)) @@ -26,7 +26,7 @@ func (person Person) ConvertFromVNext(source *vNext.Person) error { } // ConvertToVNext populates the provided destination Person from our Person -func (person Person) ConvertToVNext(destination *vNext.Person) error { +func (person *Person) ConvertToVNext(destination *vNext.Person) error { // scores scoreList := make([]int, len(person.scores)) diff --git a/hack/generator/pkg/astmodel/testdata/SetIntFromInt.golden b/hack/generator/pkg/astmodel/testdata/SetIntFromInt.golden index 7ae92611bae..31899886562 100644 --- a/hack/generator/pkg/astmodel/testdata/SetIntFromInt.golden +++ b/hack/generator/pkg/astmodel/testdata/SetIntFromInt.golden @@ -10,7 +10,7 @@ type Person struct { } // ConvertFromVNext populates our Person from the provided source Person -func (person Person) ConvertFromVNext(source *vNext.Person) error { +func (person *Person) ConvertFromVNext(source *vNext.Person) error { // age person.age = source.age @@ -20,7 +20,7 @@ func (person Person) ConvertFromVNext(source *vNext.Person) error { } // ConvertToVNext populates the provided destination Person from our Person -func (person Person) ConvertToVNext(destination *vNext.Person) error { +func (person *Person) ConvertToVNext(destination *vNext.Person) error { // age destination.age = person.age diff --git a/hack/generator/pkg/astmodel/testdata/SetIntFromOptionalInt.golden b/hack/generator/pkg/astmodel/testdata/SetIntFromOptionalInt.golden index a058e477d08..211d8c670a0 100644 --- a/hack/generator/pkg/astmodel/testdata/SetIntFromOptionalInt.golden +++ b/hack/generator/pkg/astmodel/testdata/SetIntFromOptionalInt.golden @@ -10,7 +10,7 @@ type Person struct { } // ConvertFromVNext populates our Person from the provided source Person -func (person Person) ConvertFromVNext(source *vNext.Person) error { +func (person *Person) ConvertFromVNext(source *vNext.Person) error { // age if source.age != nil { @@ -24,7 +24,7 @@ func (person Person) ConvertFromVNext(source *vNext.Person) error { } // ConvertToVNext populates the provided destination Person from our Person -func (person Person) ConvertToVNext(destination *vNext.Person) error { +func (person *Person) ConvertToVNext(destination *vNext.Person) error { // age age := person.age diff --git a/hack/generator/pkg/astmodel/testdata/SetMapOfOptionalFromMapOfRequired.golden b/hack/generator/pkg/astmodel/testdata/SetMapOfOptionalFromMapOfRequired.golden index 2c05e78a4be..94d098ce7e7 100644 --- a/hack/generator/pkg/astmodel/testdata/SetMapOfOptionalFromMapOfRequired.golden +++ b/hack/generator/pkg/astmodel/testdata/SetMapOfOptionalFromMapOfRequired.golden @@ -10,7 +10,7 @@ type Person struct { } // ConvertFromVNext populates our Person from the provided source Person -func (person Person) ConvertFromVNext(source *vNext.Person) error { +func (person *Person) ConvertFromVNext(source *vNext.Person) error { // ratings ratingMap := make(map[string]*int) @@ -26,7 +26,7 @@ func (person Person) ConvertFromVNext(source *vNext.Person) error { } // ConvertToVNext populates the provided destination Person from our Person -func (person Person) ConvertToVNext(destination *vNext.Person) error { +func (person *Person) ConvertToVNext(destination *vNext.Person) error { // ratings ratingMap := make(map[string]int) diff --git a/hack/generator/pkg/astmodel/testdata/SetMapOfRequiredFromMapOfOptional.golden b/hack/generator/pkg/astmodel/testdata/SetMapOfRequiredFromMapOfOptional.golden index 22cdadf802e..68b71fe3fd2 100644 --- a/hack/generator/pkg/astmodel/testdata/SetMapOfRequiredFromMapOfOptional.golden +++ b/hack/generator/pkg/astmodel/testdata/SetMapOfRequiredFromMapOfOptional.golden @@ -10,7 +10,7 @@ type Person struct { } // ConvertFromVNext populates our Person from the provided source Person -func (person Person) ConvertFromVNext(source *vNext.Person) error { +func (person *Person) ConvertFromVNext(source *vNext.Person) error { // ratings ratingMap := make(map[string]int) @@ -30,7 +30,7 @@ func (person Person) ConvertFromVNext(source *vNext.Person) error { } // ConvertToVNext populates the provided destination Person from our Person -func (person Person) ConvertToVNext(destination *vNext.Person) error { +func (person *Person) ConvertToVNext(destination *vNext.Person) error { // ratings ratingMap := make(map[string]*int) diff --git a/hack/generator/pkg/astmodel/testdata/SetMapOfRequiredFromMapOfRequired.golden b/hack/generator/pkg/astmodel/testdata/SetMapOfRequiredFromMapOfRequired.golden index 076df0f79e5..d0e76e80b33 100644 --- a/hack/generator/pkg/astmodel/testdata/SetMapOfRequiredFromMapOfRequired.golden +++ b/hack/generator/pkg/astmodel/testdata/SetMapOfRequiredFromMapOfRequired.golden @@ -10,7 +10,7 @@ type Person struct { } // ConvertFromVNext populates our Person from the provided source Person -func (person Person) ConvertFromVNext(source *vNext.Person) error { +func (person *Person) ConvertFromVNext(source *vNext.Person) error { // ratings ratingMap := make(map[string]int) @@ -26,7 +26,7 @@ func (person Person) ConvertFromVNext(source *vNext.Person) error { } // ConvertToVNext populates the provided destination Person from our Person -func (person Person) ConvertToVNext(destination *vNext.Person) error { +func (person *Person) ConvertToVNext(destination *vNext.Person) error { // ratings ratingMap := make(map[string]int) diff --git a/hack/generator/pkg/astmodel/testdata/SetOptionalStringFromOptionalString.golden b/hack/generator/pkg/astmodel/testdata/SetOptionalStringFromOptionalString.golden index 02cdbfdf93d..c8c4c671fb0 100644 --- a/hack/generator/pkg/astmodel/testdata/SetOptionalStringFromOptionalString.golden +++ b/hack/generator/pkg/astmodel/testdata/SetOptionalStringFromOptionalString.golden @@ -10,7 +10,7 @@ type Person struct { } // ConvertFromVNext populates our Person from the provided source Person -func (person Person) ConvertFromVNext(source *vNext.Person) error { +func (person *Person) ConvertFromVNext(source *vNext.Person) error { // name if source.name != nil { @@ -25,7 +25,7 @@ func (person Person) ConvertFromVNext(source *vNext.Person) error { } // ConvertToVNext populates the provided destination Person from our Person -func (person Person) ConvertToVNext(destination *vNext.Person) error { +func (person *Person) ConvertToVNext(destination *vNext.Person) error { // name if person.name != nil { diff --git a/hack/generator/pkg/astmodel/testdata/SetOptionalStringFromString.golden b/hack/generator/pkg/astmodel/testdata/SetOptionalStringFromString.golden index e6b25a16f81..e434b99a6bc 100644 --- a/hack/generator/pkg/astmodel/testdata/SetOptionalStringFromString.golden +++ b/hack/generator/pkg/astmodel/testdata/SetOptionalStringFromString.golden @@ -10,7 +10,7 @@ type Person struct { } // ConvertFromVNext populates our Person from the provided source Person -func (person Person) ConvertFromVNext(source *vNext.Person) error { +func (person *Person) ConvertFromVNext(source *vNext.Person) error { // name name := source.name @@ -21,7 +21,7 @@ func (person Person) ConvertFromVNext(source *vNext.Person) error { } // ConvertToVNext populates the provided destination Person from our Person -func (person Person) ConvertToVNext(destination *vNext.Person) error { +func (person *Person) ConvertToVNext(destination *vNext.Person) error { // name if person.name != nil { diff --git a/hack/generator/pkg/astmodel/testdata/SetStringFromOptionalString.golden b/hack/generator/pkg/astmodel/testdata/SetStringFromOptionalString.golden index 0d6a47494fe..14163fd5aa9 100644 --- a/hack/generator/pkg/astmodel/testdata/SetStringFromOptionalString.golden +++ b/hack/generator/pkg/astmodel/testdata/SetStringFromOptionalString.golden @@ -10,7 +10,7 @@ type Person struct { } // ConvertFromVNext populates our Person from the provided source Person -func (person Person) ConvertFromVNext(source *vNext.Person) error { +func (person *Person) ConvertFromVNext(source *vNext.Person) error { // name if source.name != nil { @@ -24,7 +24,7 @@ func (person Person) ConvertFromVNext(source *vNext.Person) error { } // ConvertToVNext populates the provided destination Person from our Person -func (person Person) ConvertToVNext(destination *vNext.Person) error { +func (person *Person) ConvertToVNext(destination *vNext.Person) error { // name name := person.name diff --git a/hack/generator/pkg/astmodel/testdata/SetStringFromString.golden b/hack/generator/pkg/astmodel/testdata/SetStringFromString.golden index 6a16de0f0fc..61e578b5bfc 100644 --- a/hack/generator/pkg/astmodel/testdata/SetStringFromString.golden +++ b/hack/generator/pkg/astmodel/testdata/SetStringFromString.golden @@ -10,7 +10,7 @@ type Person struct { } // ConvertFromVNext populates our Person from the provided source Person -func (person Person) ConvertFromVNext(source *vNext.Person) error { +func (person *Person) ConvertFromVNext(source *vNext.Person) error { // name person.name = source.name @@ -20,7 +20,7 @@ func (person Person) ConvertFromVNext(source *vNext.Person) error { } // ConvertToVNext populates the provided destination Person from our Person -func (person Person) ConvertToVNext(destination *vNext.Person) error { +func (person *Person) ConvertToVNext(destination *vNext.Person) error { // name destination.name = person.name diff --git a/hack/generator/pkg/astmodel/type_flag.go b/hack/generator/pkg/astmodel/type_flag.go index d76b0d5bf2e..bd1b98febfc 100644 --- a/hack/generator/pkg/astmodel/type_flag.go +++ b/hack/generator/pkg/astmodel/type_flag.go @@ -29,6 +29,7 @@ func (f TypeFlag) ApplyTo(t Type) *FlaggedType { // RemoveFrom applies the tag to the provided type func (f TypeFlag) RemoveFrom(t Type) (Type, error) { + removeFlag := func(it *FlaggedType) Type { return it.WithoutFlag(f) } @@ -40,11 +41,15 @@ func (f TypeFlag) RemoveFrom(t Type) (Type, error) { return visitor.Visit(t, nil) } -// IsOn returns true if t is a flagged type that has this flag -func (f TypeFlag) IsOn(t Type) bool { - if ft, ok := t.(*FlaggedType); ok { - return ft.HasFlag(f) - } +// IsOn returns true if t is or contains a flagged type that has this flag +func (f TypeFlag) IsOn(theType Type) bool { + switch t := theType.(type) { + case *FlaggedType: + return t.HasFlag(f) - return false + case MetaType: + return f.IsOn(t.Unwrap()) + default: + return false + } } diff --git a/hack/generator/pkg/astmodel/type_visitor.go b/hack/generator/pkg/astmodel/type_visitor.go index 554d5c9980e..b4f344efc92 100644 --- a/hack/generator/pkg/astmodel/type_visitor.go +++ b/hack/generator/pkg/astmodel/type_visitor.go @@ -178,12 +178,12 @@ func IdentityVisitOfObjectType(this *TypeVisitor, it *ObjectType, ctx interface{ func IdentityVisitOfMapType(this *TypeVisitor, it *MapType, ctx interface{}) (Type, error) { visitedKey, err := this.Visit(it.key, ctx) if err != nil { - return nil, errors.Wrapf(err, "failed to visit map key type %T", it.key) + return nil, errors.Wrapf(err, "failed to visit map key type %q", it.key) } visitedValue, err := this.Visit(it.value, ctx) if err != nil { - return nil, errors.Wrapf(err, "failed to visit map value type %T", it.value) + return nil, errors.Wrapf(err, "failed to visit map value type %q", it.value) } if visitedKey == it.key && visitedValue == it.value { @@ -202,7 +202,7 @@ func IdentityVisitOfEnumType(_ *TypeVisitor, it *EnumType, _ interface{}) (Type, func IdentityVisitOfOptionalType(this *TypeVisitor, it *OptionalType, ctx interface{}) (Type, error) { visitedElement, err := this.Visit(it.element, ctx) if err != nil { - return nil, errors.Wrapf(err, "failed to visit optional element type %T", it.element) + return nil, errors.Wrapf(err, "failed to visit optional element type %q", it.element) } if visitedElement == it.element { @@ -215,12 +215,12 @@ func IdentityVisitOfOptionalType(this *TypeVisitor, it *OptionalType, ctx interf func IdentityVisitOfResourceType(this *TypeVisitor, it *ResourceType, ctx interface{}) (Type, error) { visitedSpec, err := this.Visit(it.spec, ctx) if err != nil { - return nil, errors.Wrapf(err, "failed to visit resource spec type %T", it.spec) + return nil, errors.Wrapf(err, "failed to visit resource spec type %q", it.spec) } visitedStatus, err := this.Visit(it.status, ctx) if err != nil { - return nil, errors.Wrapf(err, "failed to visit resource status type %T", it.status) + return nil, errors.Wrapf(err, "failed to visit resource status type %q", it.status) } if visitedSpec == it.spec && visitedStatus == it.status { @@ -296,7 +296,7 @@ func typeSlicesFastEqual(t1 []Type, t2 []Type) bool { func IdentityVisitOfFlaggedType(this *TypeVisitor, ft *FlaggedType, ctx interface{}) (Type, error) { nt, err := this.Visit(ft.element, ctx) if err != nil { - return nil, errors.Wrapf(err, "failed to visit flagged type %T", ft.element) + return nil, errors.Wrapf(err, "failed to visit flagged type %q", ft.element) } if nt == ft.element { @@ -309,7 +309,7 @@ func IdentityVisitOfFlaggedType(this *TypeVisitor, ft *FlaggedType, ctx interfac func IdentityVisitOfValidatedType(this *TypeVisitor, v *ValidatedType, ctx interface{}) (Type, error) { nt, err := this.Visit(v.element, ctx) if err != nil { - return nil, errors.Wrapf(err, "failed to visit validated type %T", v.element) + return nil, errors.Wrapf(err, "failed to visit validated type %q", v.element) } if nt == v.element { @@ -322,7 +322,7 @@ func IdentityVisitOfValidatedType(this *TypeVisitor, v *ValidatedType, ctx inter func IdentityVisitOfErroredType(this *TypeVisitor, e *ErroredType, ctx interface{}) (Type, error) { nt, err := this.Visit(e.inner, ctx) if err != nil { - return nil, errors.Wrapf(err, "failed to visit errored type %T", e.inner) + return nil, errors.Wrapf(err, "failed to visit errored type %q", e.inner) } if nt == e.inner { diff --git a/hack/generator/pkg/astmodel/types.go b/hack/generator/pkg/astmodel/types.go index 51376989707..330f100f8bf 100644 --- a/hack/generator/pkg/astmodel/types.go +++ b/hack/generator/pkg/astmodel/types.go @@ -7,7 +7,6 @@ package astmodel import ( "fmt" - "github.com/pkg/errors" ) @@ -204,3 +203,22 @@ func (types Types) ResolveResourceStatusDefinition( // preserve outer spec name return resourceStatusDef.WithName(statusName), nil } + +// Process applies a func to transform all members of this set of type definitions, returning a new set of type +// definitions containing the results of the transfomration, or possibly an error +// Only definitions returned by the func will be included in the results of the function. The func may return a nil +// TypeDefinition if it doesn't want to include anything in the output set. +func (types Types) Process(transformation func(definition TypeDefinition) (*TypeDefinition, error)) (Types, error) { + result := make(Types) + + for _, def := range types { + d, err := transformation(def) + if err != nil { + return nil, err + } else if d != nil { + result.Add(*d) + } + } + + return result, nil +} diff --git a/hack/generator/pkg/astmodel/validated_type.go b/hack/generator/pkg/astmodel/validated_type.go index 1047675099e..fb9d0d2bdd2 100644 --- a/hack/generator/pkg/astmodel/validated_type.go +++ b/hack/generator/pkg/astmodel/validated_type.go @@ -176,9 +176,9 @@ func (v *ValidatedType) AsType(_ *CodeGenerationContext) dst.Expr { panic("Should never happen: validated types must either be named (handled by 'name types for CRDs' pipeline stage) or be directly under properties (handled by PropertyDefinition.AsField)") } -// AsZero panics because validated types should always be named +// AsZero returns the zero for our underlying type func (v *ValidatedType) AsZero(types Types, ctx *CodeGenerationContext) dst.Expr { - panic("Should never happen: validated types must either be named (handled by 'name types for CRDs' pipeline stage) or be directly under properties (handled by PropertyDefinition.AsField)") + return v.element.AsZero(types, ctx) } func (v *ValidatedType) References() TypeNameSet { diff --git a/hack/generator/pkg/codegen/code_generator.go b/hack/generator/pkg/codegen/code_generator.go index b2fc7cf58ee..7ab9d4b6b0b 100644 --- a/hack/generator/pkg/codegen/code_generator.go +++ b/hack/generator/pkg/codegen/code_generator.go @@ -152,7 +152,7 @@ func createAllPipelineStages(idFactory astmodel.IdentifierFactory, configuration addCrossplaneEmbeddedResourceSpec(idFactory).UsedFor(CrossplaneTarget), addCrossplaneEmbeddedResourceStatus(idFactory).UsedFor(CrossplaneTarget), - createStorageTypes().UsedFor(ARMTarget), // TODO: For now only used for ARM + createStorageTypes(idFactory).UsedFor(ARMTarget), // TODO: For now only used for ARM simplifyDefinitions(), injectJsonSerializationTests(idFactory).UsedFor(ARMTarget), diff --git a/hack/generator/pkg/codegen/pipeline_create_storage_types.go b/hack/generator/pkg/codegen/pipeline_create_storage_types.go index 50940aedee5..2ebb33b24fc 100644 --- a/hack/generator/pkg/codegen/pipeline_create_storage_types.go +++ b/hack/generator/pkg/codegen/pipeline_create_storage_types.go @@ -7,247 +7,68 @@ package codegen import ( "context" - "fmt" - "github.com/Azure/azure-service-operator/hack/generator/pkg/astmodel" + "github.com/Azure/azure-service-operator/hack/generator/pkg/codegen/storage" kerrors "k8s.io/apimachinery/pkg/util/errors" + "k8s.io/klog/v2" ) // createStorageTypes returns a pipeline stage that creates dedicated storage types for each resource and nested object. // Storage versions are created for *all* API versions to allow users of older versions of the operator to easily // upgrade. This is of course a bit odd for the first release, but defining the approach from day one is useful. -func createStorageTypes() PipelineStage { +func createStorageTypes(idFactory astmodel.IdentifierFactory) PipelineStage { return MakePipelineStage( "createStorage", "Create storage versions of CRD types", func(ctx context.Context, types astmodel.Types) (astmodel.Types, error) { - storageTypes := make(astmodel.Types) - visitor := makeStorageTypesVisitor(types) - vc := makeStorageTypesVisitorContext() - var errs []error - for _, d := range types { - d := d + // Create a factory for each group (aka service) and divvy up the types + factories := make(map[string]*storage.StorageTypeFactory) + for name, def := range types { - if astmodel.ARMFlag.IsOn(d.Type()) { - // Skip ARM definitions, we don't need to create storage variants of those + ref, ok := name.PackageReference.AsLocalPackage() + if !ok { + // Skip definitions from non-local packages + // (should never happen) + klog.Warningf("Skipping storage type generation for unexpected non-local package reference %q", name.PackageReference) continue } - if _, ok := types.ResolveEnumDefinition(&d); ok { - // Skip Enum definitions as we use the base type for storage + factory, ok := factories[ref.Group()] + if !ok { + klog.V(3).Infof("Creating storage factory for %s", ref.Group()) + factory = storage.NewStorageTypeFactory(ref.Group(), idFactory) + factories[ref.Group()] = factory + } + + if astmodel.ARMFlag.IsOn(def.Type()) { + // skip ARM types as they don't need storage variants continue } - def, err := visitor.VisitDefinition(d, vc) + factory.Add(def) + } + + // Collect up all the results + result := make(astmodel.Types) + var errs []error + for _, factory := range factories { + t, err := factory.Types() if err != nil { errs = append(errs, err) continue } - finalDef := def.WithDescription(descriptionForStorageVariant(d)) - storageTypes[finalDef.Name()] = finalDef + result.AddTypes(t) } - if len(errs) > 0 { - err := kerrors.NewAggregate(errs) + err := kerrors.NewAggregate(errs) + if err != nil { return nil, err } - types.AddTypes(storageTypes) - - return types, nil + unmodified := types.Except(result) + result.AddTypes(unmodified) + return result, nil }) } - -// makeStorageTypesVisitor returns a TypeVisitor to do the creation of dedicated storage types -func makeStorageTypesVisitor(types astmodel.Types) astmodel.TypeVisitor { - factory := &StorageTypeFactory{ - types: types, - } - - visitor := astmodel.TypeVisitorBuilder{ - VisitValidatedType: factory.stripValidations, - VisitTypeName: factory.remapNames, - VisitObjectType: factory.convertObjects, - VisitResourceType: factory.convertResources, - VisitFlaggedType: factory.skipArmTypes, - }.Build() - - factory.visitor = visitor - factory.propertyConversions = []propertyConversion{ - factory.preserveKubernetesResourceStorageProperties, - factory.convertPropertiesForStorage, - } - - return visitor -} - -type StorageTypeFactory struct { - types astmodel.Types - propertyConversions []propertyConversion - visitor astmodel.TypeVisitor -} - -// A property conversion accepts a property definition and optionally applies a conversion to make -// the property suitable for use on a storage type. Conversions return nil if they decline to -// convert, deferring the conversion to another. -type propertyConversion = func(property *astmodel.PropertyDefinition, ctx StorageTypesVisitorContext) (*astmodel.PropertyDefinition, error) - -func (factory *StorageTypeFactory) stripValidations(this *astmodel.TypeVisitor, v *astmodel.ValidatedType, ctx interface{}) (astmodel.Type, error) { - // strip all type validations from storage types, - // act as if they do not exist - return this.Visit(v.ElementType(), ctx) -} - -func (factory *StorageTypeFactory) remapNames(_ *astmodel.TypeVisitor, name astmodel.TypeName, ctx interface{}) (astmodel.Type, error) { - visitorContext := ctx.(StorageTypesVisitorContext) - - // Resolve the type name to the actual referenced type - actualDefinition, actualDefinitionFound := factory.types[name] - - // Check for property specific handling - if visitorContext.property != nil && actualDefinitionFound { - if et, ok := actualDefinition.Type().(*astmodel.EnumType); ok { - // Property type refers to an enum, so we use the base type instead - return et.BaseType(), nil - } - } - - // Map the type name into our storage namespace - localRef, ok := name.PackageReference.AsLocalPackage() - if !ok { - return name, nil - } - - storageRef := astmodel.MakeStoragePackageReference(localRef) - visitedName := astmodel.MakeTypeName(storageRef, name.Name()) - return visitedName, nil -} - -func (factory *StorageTypeFactory) convertResources( - this *astmodel.TypeVisitor, - resource *astmodel.ResourceType, - ctx interface{}) (astmodel.Type, error) { - - // storage resource types do not need defaulter/validator interface, they have no webhooks - return resource.WithoutInterface(astmodel.DefaulterInterfaceName).WithoutInterface(astmodel.ValidatorInterfaceName), nil -} - -func (factory *StorageTypeFactory) convertObjects( - _ *astmodel.TypeVisitor, - object *astmodel.ObjectType, - ctx interface{}) (astmodel.Type, error) { - visitorContext := ctx.(StorageTypesVisitorContext) - objectContext := visitorContext.forObject(object) - - var errs []error - properties := object.Properties() - for i, prop := range properties { - p, err := factory.makeStorageProperty(prop, objectContext) - if err != nil { - errs = append(errs, err) - } else { - properties[i] = p - } - } - - if len(errs) > 0 { - err := kerrors.NewAggregate(errs) - return nil, err - } - - objectType := astmodel.NewObjectType().WithProperties(properties...) - return astmodel.StorageFlag.ApplyTo(objectType), nil -} - -// makeStorageProperty applies a conversion to make a variant of the property for use when -// serializing to storage -func (factory *StorageTypeFactory) makeStorageProperty( - prop *astmodel.PropertyDefinition, - objectContext StorageTypesVisitorContext) (*astmodel.PropertyDefinition, error) { - for _, conv := range factory.propertyConversions { - p, err := conv(prop, objectContext.forProperty(prop)) - if err != nil { - // Something went wrong, return the error - return nil, err - } - if p != nil { - // We have the conversion we need, return it promptly - return p, nil - } - } - - return nil, fmt.Errorf("failed to find a conversion for property %v", prop.PropertyName()) -} - -// preserveKubernetesResourceStorageProperties preserves properties required by the KubernetesResource interface as they're always required -func (factory *StorageTypeFactory) preserveKubernetesResourceStorageProperties( - prop *astmodel.PropertyDefinition, - _ StorageTypesVisitorContext) (*astmodel.PropertyDefinition, error) { - if astmodel.IsKubernetesResourceProperty(prop.PropertyName()) { - // Keep these unchanged - return prop, nil - } - - // No opinion, defer to another conversion - return nil, nil -} - -func (factory *StorageTypeFactory) convertPropertiesForStorage( - prop *astmodel.PropertyDefinition, - objectContext StorageTypesVisitorContext) (*astmodel.PropertyDefinition, error) { - propertyType, err := factory.visitor.Visit(prop.PropertyType(), objectContext) - if err != nil { - return nil, err - } - - p := prop.WithType(propertyType). - MakeOptional(). - WithDescription("") - - return p, nil -} - -func (factory *StorageTypeFactory) skipArmTypes( - tv *astmodel.TypeVisitor, - flaggedType *astmodel.FlaggedType, - ctx interface{}) (astmodel.Type, error) { - if flaggedType.HasFlag(astmodel.ARMFlag) { - // We don't want to do anything with ARM types - return flaggedType, nil - } - - return astmodel.IdentityVisitOfFlaggedType(tv, flaggedType, ctx) -} - -func descriptionForStorageVariant(definition astmodel.TypeDefinition) []string { - pkg := definition.Name().PackageReference.PackageName() - - result := []string{ - fmt.Sprintf("Storage version of %v.%v", pkg, definition.Name().Name()), - } - result = append(result, definition.Description()...) - - return result -} - -type StorageTypesVisitorContext struct { - object *astmodel.ObjectType - property *astmodel.PropertyDefinition -} - -func makeStorageTypesVisitorContext() StorageTypesVisitorContext { - return StorageTypesVisitorContext{} -} - -func (context StorageTypesVisitorContext) forObject(object *astmodel.ObjectType) StorageTypesVisitorContext { - context.object = object - context.property = nil - return context -} - -func (context StorageTypesVisitorContext) forProperty(property *astmodel.PropertyDefinition) StorageTypesVisitorContext { - context.property = property - return context -} diff --git a/hack/generator/pkg/codegen/pipeline_ensure_type_has_arm_type.go b/hack/generator/pkg/codegen/pipeline_ensure_type_has_arm_type.go index dd52baeebde..47b911a566e 100644 --- a/hack/generator/pkg/codegen/pipeline_ensure_type_has_arm_type.go +++ b/hack/generator/pkg/codegen/pipeline_ensure_type_has_arm_type.go @@ -21,13 +21,13 @@ func ensureARMTypeExistsForEveryResource() PipelineStage { "ensureArmTypeExistsForEveryType", "Ensure that an ARM type for every top level resource spec/status exists", func(ctx context.Context, types astmodel.Types) (astmodel.Types, error) { - return types, validateAllTypesHaveARMType(types) + return types, validateExpectedTypesHaveARMType(types) }) } -// validateAllTypesHaveARMType returns an error containing details about all +// validateExpectedTypesHaveARMType returns an error containing details about all // types which do not have a matching ARM type. -func validateAllTypesHaveARMType(definitions astmodel.Types) error { +func validateExpectedTypesHaveARMType(definitions astmodel.Types) error { findARMType := func(t astmodel.Type) error { name, ok := astmodel.AsTypeName(t) if !ok { @@ -45,7 +45,13 @@ func validateAllTypesHaveARMType(definitions astmodel.Types) error { var errs []error - for _, def := range definitions { + for name, def := range definitions { + + if astmodel.IsStoragePackageReference(name.PackageReference) { + // Don't need ARM types within Storage Packages + continue + } + if resourceType, ok := definitions.ResolveResourceType(def.Type()); ok { err := findARMType(resourceType.SpecType()) diff --git a/hack/generator/pkg/codegen/pipeline_mark_storage_version.go b/hack/generator/pkg/codegen/pipeline_mark_storage_version.go index bdb98bbadca..119bea869f6 100644 --- a/hack/generator/pkg/codegen/pipeline_mark_storage_version.go +++ b/hack/generator/pkg/codegen/pipeline_mark_storage_version.go @@ -72,6 +72,15 @@ func groupResourcesByVersion(types astmodel.Types) (map[unversionedName][]astmod result := make(map[unversionedName][]astmodel.TypeDefinition) for _, def := range types { + + // TODO: Remove this + // We want to explicitly avoid storage types for now, as we want to tag the latest API version to avoid a bug + // Once we get the full conversion functions in place, and a mapping that allows us to find correct API version + // within the controller, we can delete this + if astmodel.IsStoragePackageReference(def.Name().PackageReference) { + continue + } + if astmodel.IsResourceDefinition(def) { name, err := getUnversionedName(def.Name()) if err != nil { @@ -86,7 +95,9 @@ func groupResourcesByVersion(types astmodel.Types) (map[unversionedName][]astmod // order each set of resources by package name (== by version as these are sortable dates) for _, slice := range result { sort.Slice(slice, func(i, j int) bool { - return slice[i].Name().PackageReference.PackageName() < slice[j].Name().PackageReference.PackageName() + return astmodel.ComparePathAndVersion( + slice[i].Name().PackageReference.PackagePath(), + slice[j].Name().PackageReference.PackagePath()) }) } diff --git a/hack/generator/pkg/codegen/storage/function_injector.go b/hack/generator/pkg/codegen/storage/function_injector.go new file mode 100644 index 00000000000..8b238c715c1 --- /dev/null +++ b/hack/generator/pkg/codegen/storage/function_injector.go @@ -0,0 +1,47 @@ +/* + * Copyright (c) Microsoft Corporation. + * Licensed under the MIT license. + */ + +package storage + +import "github.com/Azure/azure-service-operator/hack/generator/pkg/astmodel" + +// FunctionInjector is a utility for injecting function definitions into resources and objects +type FunctionInjector struct { + // visitor is used to do the actual injection + visitor astmodel.TypeVisitor +} + +// NewFunctionInjector creates a new function injector for modifying resources and objects +func NewFunctionInjector() *FunctionInjector { + result := &FunctionInjector{} + + result.visitor = astmodel.TypeVisitorBuilder{ + VisitObjectType: result.injectFunctionIntoObject, + VisitResourceType: result.injectFunctionIntoResource, + }.Build() + + return result +} + +// Inject modifies the passed type definition by injecting the passed function +func (fi *FunctionInjector) Inject(def astmodel.TypeDefinition, fn astmodel.Function) (astmodel.TypeDefinition, error) { + return fi.visitor.VisitDefinition(def, fn) +} + +// injectFunctionIntoObject takes the function provided as a context and includes it on the +// provided object type +func (_ *FunctionInjector) injectFunctionIntoObject( + _ *astmodel.TypeVisitor, ot *astmodel.ObjectType, ctx interface{}) (astmodel.Type, error) { + fn := ctx.(astmodel.Function) + return ot.WithFunction(fn), nil +} + +// injectFunctionIntoResource takes the function provided as a context and includes it on the +// provided resource type +func (_ *FunctionInjector) injectFunctionIntoResource( + _ *astmodel.TypeVisitor, rt *astmodel.ResourceType, ctx interface{}) (astmodel.Type, error) { + fn := ctx.(astmodel.Function) + return rt.WithFunction(fn), nil +} diff --git a/hack/generator/pkg/codegen/storage/hub_version_marker.go b/hack/generator/pkg/codegen/storage/hub_version_marker.go new file mode 100644 index 00000000000..b28e4e72243 --- /dev/null +++ b/hack/generator/pkg/codegen/storage/hub_version_marker.go @@ -0,0 +1,36 @@ +/* + * Copyright (c) Microsoft Corporation. + * Licensed under the MIT license. + */ + +package storage + +import "github.com/Azure/azure-service-operator/hack/generator/pkg/astmodel" + +// HubVersionMarker is a utility for marking resource types as "hub" versions +type HubVersionMarker struct { + // visitor is used to do the actual marking + visitor astmodel.TypeVisitor +} + +// NewHubVersionMarker returns a new hub version marker for flagging resource types +func NewHubVersionMarker() *HubVersionMarker { + result := &HubVersionMarker{} + + result.visitor = astmodel.TypeVisitorBuilder{ + VisitResourceType: result.markResourceAsStorageVersion, + }.Build() + + return result +} + +// MarkAsStorageVersion marks the supplied type definition as the storage version +func (m *HubVersionMarker) MarkAsStorageVersion(def astmodel.TypeDefinition) (astmodel.TypeDefinition, error) { + return m.visitor.VisitDefinition(def, nil) +} + +// markResourceAsStorageVersion marks the supplied resource as the canonical hub (storage) version +func (m *HubVersionMarker) markResourceAsStorageVersion( + _ *astmodel.TypeVisitor, rt *astmodel.ResourceType, _ interface{}) (astmodel.Type, error) { + return rt.MarkAsStorageVersion(), nil +} diff --git a/hack/generator/pkg/codegen/storage/property_converter.go b/hack/generator/pkg/codegen/storage/property_converter.go new file mode 100644 index 00000000000..8a48c09c2c0 --- /dev/null +++ b/hack/generator/pkg/codegen/storage/property_converter.go @@ -0,0 +1,182 @@ +/* + * Copyright (c) Microsoft Corporation. + * Licensed under the MIT license. + */ + +package storage + +import ( + "fmt" + "github.com/Azure/azure-service-operator/hack/generator/pkg/astmodel" + "github.com/pkg/errors" + "strings" +) + +// PropertyConverter is used to convert the properties of object inputTypes as required for storage variants +type PropertyConverter struct { + // visitor is used to apply the modification + visitor astmodel.TypeVisitor + // types contains all the types for this group + types astmodel.Types + // propertyConversions is an ordered list of all our conversion rules for creating storage variants + propertyConversions []propertyConversion +} + +// NewPropertyConverter creates a new property converter for modifying object properties +func NewPropertyConverter(types astmodel.Types) *PropertyConverter { + result := &PropertyConverter{ + types: types, + } + + result.propertyConversions = []propertyConversion{ + result.preserveKubernetesResourceStorageProperties, + result.preserveResourceReferenceProperties, + result.defaultPropertyConversion, + } + + result.visitor = astmodel.TypeVisitorBuilder{ + VisitEnumType: result.useBaseTypeForEnumerations, + VisitValidatedType: result.stripAllValidations, + VisitTypeName: result.shortCircuitNamesOfSimpleTypes, + }.Build() + + return result +} + +// ConvertProperty applies our conversion rules to a specific property +func (p *PropertyConverter) ConvertProperty(property *astmodel.PropertyDefinition) (*astmodel.PropertyDefinition, error) { + + for _, conv := range p.propertyConversions { + prop, err := conv(property) + if err != nil { + // Something went wrong, return the error + return nil, err + } + if prop != nil { + // We have the conversion we need, return it promptly + return prop, nil + } + } + + // No conversion found + + var typeDescription strings.Builder + property.PropertyType().WriteDebugDescription(&typeDescription, p.types) + + return nil, fmt.Errorf( + "failed to find a conversion for property %v (%v)", property.PropertyName(), typeDescription.String()) +} + +// stripAllValidations removes all validations +func (p *PropertyConverter) stripAllValidations( + this *astmodel.TypeVisitor, v *astmodel.ValidatedType, ctx interface{}) (astmodel.Type, error) { + // strip all type validations from storage property inputTypes + // act as if they do not exist + return this.Visit(v.ElementType(), ctx) +} + +// useBaseTypeForEnumerations replaces an enumeration with its underlying base type +func (p *PropertyConverter) useBaseTypeForEnumerations( + tv *astmodel.TypeVisitor, et *astmodel.EnumType, ctx interface{}) (astmodel.Type, error) { + return tv.Visit(et.BaseType(), ctx) +} + +// shortCircuitNamesOfSimpleTypes redirects or replaces TypeNames +// o If a TypeName points into an API package, it is redirected into the appropriate storage package +// o If a TypeName references an enumeration, it is replaced with the underlying type of the enumeration as our +// storage types don't use enumerations, they use primitive types +// o If a TypeName references an alias for a primitive type (these are used to specify validations), it is replace +// with the primitive type +func (p *PropertyConverter) shortCircuitNamesOfSimpleTypes( + tv *astmodel.TypeVisitor, tn astmodel.TypeName, ctx interface{}) (astmodel.Type, error) { + + actualType, err := p.types.FullyResolve(tn) + if err != nil { + // Can't resolve to underlying type, give up + return nil, err + } + + _, isObject := astmodel.AsObjectType(actualType) + _, isResource := astmodel.AsResourceType(actualType) + + if isObject || isResource { + // We have an object or a resource, redirect to our storage package if we can + if storageName, ok := p.tryConvertToStoragePackage(tn); ok { + return storageName, nil + } + + // Otherwise just keep the name + return tn, nil + } + + // Replace the name with the underlying type + return tv.Visit(actualType, ctx) +} + +func (_ *PropertyConverter) tryConvertToStoragePackage(name astmodel.TypeName) (astmodel.TypeName, bool) { + // Map the type name into our storage package + localRef, ok := name.PackageReference.AsLocalPackage() + if !ok { + return astmodel.TypeName{}, false + } + + storageRef := astmodel.MakeStoragePackageReference(localRef) + visitedName := astmodel.MakeTypeName(storageRef, name.Name()) + return visitedName, true +} + +// A property conversion accepts a property definition and optionally applies a conversion to make +// the property suitable for use on a storage type. Conversions return nil if they decline to +// convert, deferring the conversion to another. +type propertyConversion = func(property *astmodel.PropertyDefinition) (*astmodel.PropertyDefinition, error) + +// preserveKubernetesResourceStorageProperties preserves properties required by the +// KubernetesResource interface as they're always required exactly as declared +func (p *PropertyConverter) preserveKubernetesResourceStorageProperties( + prop *astmodel.PropertyDefinition) (*astmodel.PropertyDefinition, error) { + + if astmodel.IsKubernetesResourceProperty(prop.PropertyName()) { + // Keep these unchanged + return prop, nil + } + + // Not a kubernetes type, defer to another conversion + return nil, nil +} + +// preserveResourceReferenceProperties preserves properties required by the +// KubernetesResource interface as they're always required exactly as declared +func (p *PropertyConverter) preserveResourceReferenceProperties( + prop *astmodel.PropertyDefinition) (*astmodel.PropertyDefinition, error) { + + propertyType := prop.PropertyType() + if opt, ok := astmodel.AsOptionalType(propertyType); ok { + if opt.Element().Equals(astmodel.ResourceReferenceTypeName) { + // Keep these unchanged + return prop, nil + } + } + + if propertyType.Equals(astmodel.ResourceReferenceTypeName) { + // Keep these unchanged + return prop, nil + } + + // Not a resource reference property, defer to another conversion + return nil, nil +} + +func (p *PropertyConverter) defaultPropertyConversion( + property *astmodel.PropertyDefinition) (*astmodel.PropertyDefinition, error) { + + propertyType, err := p.visitor.Visit(property.PropertyType(), nil) + if err != nil { + return nil, errors.Wrapf(err, "converting property %q", property.PropertyName()) + } + + newProperty := property.WithType(propertyType). + MakeOptional(). + WithDescription("") + + return newProperty, nil +} diff --git a/hack/generator/pkg/codegen/storage/storage_type_factory.go b/hack/generator/pkg/codegen/storage/storage_type_factory.go new file mode 100644 index 00000000000..903b22e0f2a --- /dev/null +++ b/hack/generator/pkg/codegen/storage/storage_type_factory.go @@ -0,0 +1,189 @@ +/* + * Copyright (c) Microsoft Corporation. + * Licensed under the MIT license. + */ + +package storage + +import ( + "github.com/Azure/azure-service-operator/hack/generator/pkg/astmodel" + "github.com/pkg/errors" + "k8s.io/klog/v2" +) + +// StorageTypeFactory is used to create storage inputTypes for a specific api group +type StorageTypeFactory struct { + group string // Name of the group we're handling (used mostly for logging) + inputTypes astmodel.Types // All the types for this group + outputTypes astmodel.Types // All the types created/modified by this factory + idFactory astmodel.IdentifierFactory // Factory for creating identifiers + typeConverter *TypeConverter // a utility type type visitor used to create storage variants + functionInjector *FunctionInjector // a utility used to inject functions into definitions + resourceHubMarker *HubVersionMarker // a utility used to mark resources as Storage Versions + conversionMap map[astmodel.PackageReference]astmodel.PackageReference // Map of conversion links for creating our conversion graph +} + +// NewStorageTypeFactory creates a new instance of StorageTypeFactory ready for use +func NewStorageTypeFactory(group string, idFactory astmodel.IdentifierFactory) *StorageTypeFactory { + + types := make(astmodel.Types) + + result := &StorageTypeFactory{ + group: group, + inputTypes: types, + idFactory: idFactory, + conversionMap: make(map[astmodel.PackageReference]astmodel.PackageReference), + functionInjector: NewFunctionInjector(), + resourceHubMarker: NewHubVersionMarker(), + typeConverter: NewTypeConverter(types), + } + + return result +} + +// Add the supplied type definition to this factory +func (f *StorageTypeFactory) Add(def astmodel.TypeDefinition) { + f.inputTypes.Add(def) +} + +// Types returns inputTypes contained by the factory, including all new storage variants and modified +// api inputTypes. If any errors occur during processing, they're returned here. +func (f *StorageTypeFactory) Types() (astmodel.Types, error) { + if len(f.outputTypes) == 0 { + err := f.process() + if err != nil { + return nil, err + } + } + + return f.outputTypes, nil +} + +func (f *StorageTypeFactory) process() error { + + f.outputTypes = make(astmodel.Types) + + // Create storage variants and stash them in outputTypes so injectConversions can find them + storageVariants, err := f.inputTypes.Process(f.createStorageVariant) + if err != nil { + return err + } + + f.outputTypes.AddTypes(storageVariants) + + // Inject conversions into our original types and stash them in outputTypes + inputTypesWithConversions, err := f.inputTypes.Process(f.injectConversions) + if err != nil { + return err + } + + f.outputTypes.AddTypes(inputTypesWithConversions) + + // Inject conversions into our storage variants and replace the definitions in outputTypes + storageVariantsWithConversions, err := storageVariants.Process(f.injectConversions) + if err != nil { + return err + } + + for _, d := range storageVariantsWithConversions { + f.outputTypes[d.Name()] = d + } + + // Add anything missing into outputTypes + f.outputTypes.AddTypes(f.inputTypes.Except(f.outputTypes)) + + return nil +} + +// createStorageVariant takes an existing object definition and creates a storage variant in a +// related package. +// def is the api definition on which to base the storage variant +// visitor is a type visitor that will do the creation +func (f *StorageTypeFactory) createStorageVariant(definition astmodel.TypeDefinition) (*astmodel.TypeDefinition, error) { + name := definition.Name() + _, isObject := astmodel.AsObjectType(definition.Type()) + _, isResource := astmodel.AsResourceType(definition.Type()) + if !isObject && !isResource { + // just skip it + klog.V(4).Infof("Skipping %s as no storage variant needed", name) + return nil, nil + } + + klog.V(3).Infof("Creating storage variant of %s", name) + + storageDef, err := f.typeConverter.ConvertDefinition(definition) + if err != nil { + return nil, errors.Wrapf(err, "creating storage variant for %q", name) + } + + // Add API-Package -> Storage-Package link into the conversion map + f.conversionMap[name.PackageReference] = storageDef.Name().PackageReference + + return &storageDef, nil +} + +// injectConversions modifies the named type by injecting the required conversion methods using +// the conversionMap we've previously established +func (f *StorageTypeFactory) injectConversions(definition astmodel.TypeDefinition) (*astmodel.TypeDefinition, error) { + name := definition.Name() + _, isObject := astmodel.AsObjectType(definition.Type()) + _, isResource := astmodel.AsResourceType(definition.Type()) + if !isObject && !isResource { + // just skip it + klog.V(4).Infof("Skipping %s as no conversion functions needed", name) + return nil, nil + } + + klog.V(3).Infof("Injecting conversion functions into %s", name) + + // Find the definition we want to convert to/from + nextPackage, ok := f.conversionMap[name.PackageReference] + if !ok { + // No next package, so nothing to do + // (this is expected if we have the hub storage package) + // Flag the type as needing to be flagged as the storage version + //TODO: Restore this - currently disabled until we get all the conversion functions injected + //hubDefintion, err := f.resourceHubMarker.markResourceAsStorageVersion(definition) + //if err != nil { + // return nil, errors.Wrapf(err, "marking %q as hub version", name) + //} + //return &hubDefintion, nil + return nil, nil + } + + nextName := astmodel.MakeTypeName(nextPackage, name.Name()) + nextDef, ok := f.outputTypes[nextName] + if !ok { + // No next type so nothing to do + // (this is expected if the type is discontinued or we're looking at the hub type) + return nil, nil + } + + // Create conversion functions + knownTypes := make(astmodel.Types) + knownTypes.AddTypes(f.inputTypes) + knownTypes.AddTypes(f.outputTypes) + conversionContext := astmodel.NewStorageConversionContext(knownTypes, f.idFactory) + + convertFromFn, err := astmodel.NewStorageConversionFromFunction(definition, nextDef, f.idFactory, conversionContext) + if err != nil { + return nil, errors.Wrapf(err, "creating ConvertFrom() function for %q", name) + } + + convertToFn, err := astmodel.NewStorageConversionToFunction(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) + } + + updatedDefinition, err = f.functionInjector.Inject(updatedDefinition, convertToFn) + if err != nil { + return nil, errors.Wrapf(err, "failed to inject ConvertFrom function into %q", name) + } + + return &updatedDefinition, nil +} diff --git a/hack/generator/pkg/codegen/storage/type_converter.go b/hack/generator/pkg/codegen/storage/type_converter.go new file mode 100644 index 00000000000..f4457419d35 --- /dev/null +++ b/hack/generator/pkg/codegen/storage/type_converter.go @@ -0,0 +1,151 @@ +/* + * Copyright (c) Microsoft Corporation. + * Licensed under the MIT license. + */ + +package storage + +import ( + "fmt" + "github.com/Azure/azure-service-operator/hack/generator/pkg/astmodel" + "github.com/pkg/errors" + kerrors "k8s.io/apimachinery/pkg/util/errors" +) + +// TypeConverter is used to create a storage variant of an API type +type TypeConverter struct { + // visitor used to apply the modification + visitor astmodel.TypeVisitor + // types contains all the types for this group + types astmodel.Types + // propertyConverter is used to modify properties + propertyConverter *PropertyConverter +} + +// NewTypeConverter creates a new instance of the utility type +func NewTypeConverter(types astmodel.Types) *TypeConverter { + result := &TypeConverter{ + types: types, + propertyConverter: NewPropertyConverter(types), + } + + result.visitor = astmodel.TypeVisitorBuilder{ + VisitObjectType: result.convertObjectType, + VisitResourceType: result.convertResourceType, + VisitTypeName: result.redirectTypeNamesToStoragePackage, + VisitValidatedType: result.stripAllValidations, + VisitFlaggedType: result.stripAllFlags, + }.Build() + + return result +} + +// ConvertDefinition applies our type conversion to a specific type definition +func (t *TypeConverter) ConvertDefinition(def astmodel.TypeDefinition) (astmodel.TypeDefinition, error) { + result, err := t.visitor.VisitDefinition(def, nil) + if err != nil { + return astmodel.TypeDefinition{}, errors.Wrapf(err, "converting %q for storage variant", def.Name()) + } + + description := t.descriptionForStorageVariant(def) + result = result.WithDescription(description) + + return result, nil +} + +/* + * Functions used by the typeConverter TypeVisitor + */ + +// convertResourceType creates a storage variation of a resource type +func (t *TypeConverter) convertResourceType( + tv *astmodel.TypeVisitor, + resource *astmodel.ResourceType, + ctx interface{}) (astmodel.Type, error) { + + // storage resource types do not need defaulter/validator interfaces, they have no webhooks + modifiedResource := resource.WithoutInterface(astmodel.DefaulterInterfaceName). + WithoutInterface(astmodel.ValidatorInterfaceName) + + return astmodel.IdentityVisitOfResourceType(tv, modifiedResource, ctx) +} + +// convertObjectType creates a storage variation of an object type +func (t *TypeConverter) convertObjectType( + _ *astmodel.TypeVisitor, object *astmodel.ObjectType, _ interface{}) (astmodel.Type, error) { + + var errs []error + properties := object.Properties() + for i, prop := range properties { + p, err := t.propertyConverter.ConvertProperty(prop) + if err != nil { + errs = append(errs, errors.Wrapf(err, "property %s", prop.PropertyName())) + } else { + properties[i] = p + } + } + + if len(errs) > 0 { + err := kerrors.NewAggregate(errs) + return nil, err + } + + objectType := astmodel.NewObjectType().WithProperties(properties...) + return astmodel.StorageFlag.ApplyTo(objectType), nil +} + +// redirectTypeNamesToStoragePackage modifies TypeNames to reference the current storage package +func (t *TypeConverter) redirectTypeNamesToStoragePackage( + _ *astmodel.TypeVisitor, name astmodel.TypeName, _ interface{}) (astmodel.Type, error) { + if result, ok := t.tryConvertToStoragePackage(name); ok { + return result, nil + } + + return name, nil +} + +// stripAllValidations removes all validations +func (t *TypeConverter) stripAllValidations( + this *astmodel.TypeVisitor, v *astmodel.ValidatedType, ctx interface{}) (astmodel.Type, error) { + // strip all type validations from storage types, + // act as if they do not exist + return this.Visit(v.ElementType(), ctx) +} + +// stripAllFlags removes all flags +func (t *TypeConverter) stripAllFlags( + tv *astmodel.TypeVisitor, + flaggedType *astmodel.FlaggedType, + ctx interface{}) (astmodel.Type, error) { + if flaggedType.HasFlag(astmodel.ARMFlag) { + // We don't want to do anything with ARM types + return flaggedType, nil + } + + return astmodel.IdentityVisitOfFlaggedType(tv, flaggedType, ctx) +} + +func (_ *TypeConverter) tryConvertToStoragePackage(name astmodel.TypeName) (astmodel.TypeName, bool) { + // Map the type name into our storage package + localRef, ok := name.PackageReference.AsLocalPackage() + if !ok { + return astmodel.TypeName{}, false + } + + storageRef := astmodel.MakeStoragePackageReference(localRef) + visitedName := astmodel.MakeTypeName(storageRef, name.Name()) + return visitedName, true +} + +// descriptionForStorageVariant creates a description for a storage variant, indicating which +// original type it is based upon +func (_ *TypeConverter) descriptionForStorageVariant(definition astmodel.TypeDefinition) []string { + pkg := definition.Name().PackageReference.PackageName() + + result := []string{ + fmt.Sprintf("Storage version of %v.%v", pkg, definition.Name().Name()), + } + result = append(result, definition.Description()...) + + return result +}