Refactoring and cleanup in preparation for changing namespaces

docs/hugo/content/contributing/generator-overview.md

@@ -63,7 +63,7 @@ One reason for this is to allow the creation of multiple pipelines (currently we
 
 Pipeline stages are instances of `pipeline.Stage`. Each has a factory method that returns a `pipeline.Stage` instance. In operation, each accepts a `pipeline.State` containing the current object model and transforms it into a new state, that is passed to the next stage in turn. If a stage returns an error, the pipeline run is aborted.
 
-New stages should use the `MakeStage()` function. You'll see some older stages that predate a structural change use the deprecated `MakeLegacyStage()` factory instead; these older stages are slowly being migrated and `MakeLegacyStage()` will be deleted when this is complete.
+New stages should use the `NewStage()` function. You'll see some older stages that predate a structural change use the deprecated `NewLegacyStage()` factory instead; these older stages are slowly being migrated and `NewLegacyStage()` will be deleted when this is complete.
 
 ## Code Generation
 

hack/crossplane/azure-crossplane.yaml

@@ -133,7 +133,7 @@ typeTransformers:
     property: Copy
     ifType:
       group: deploymenttemplate
-      version: v1alpha1api20190401
+      version: "2019-04-01"
       name: ResourceCopy
       optional: true
     remove: true
@@ -142,7 +142,7 @@ typeTransformers:
     property: Copy
     ifType:
       group: definitions
-      version: v1alpha1api
+      version: "*"
       name: ResourceCopy
       optional: true
     remove: true

v2/azure-arm.yaml

@@ -181,7 +181,7 @@ typeTransformers:
     property: Copy
     ifType:
       group: deploymenttemplate
-      version: v1alpha1api20190401
+      version: "2019-04-01"
       name: ResourceCopy
       optional: true
     remove: true
@@ -190,7 +190,7 @@ typeTransformers:
     property: Copy
     ifType:
       group: definitions
-      version: v1alpha1api
+      version: "*"
       name: ResourceCopy
       optional: true
     remove: true

v2/tools/generator/internal/astmodel/interface_implementer.go

@@ -90,7 +90,8 @@ func (i InterfaceImplementer) AsDeclarations(
 		})
 
 		for _, f := range functions {
-			result = append(result, f.AsFunc(codeGenerationContext, typeName))
+			decl := generateMethodDeclForFunction(typeName, f, codeGenerationContext)
+			result = append(result, decl)
 		}
 	}
 

v2/tools/generator/internal/astmodel/local_package_reference.go

@@ -16,43 +16,26 @@ type LocalPackageReference struct {
 	localPathPrefix string
 	group           string
 	version         string
+	versionPrefix   string
 }
 
 var (
 	_ PackageReference = LocalPackageReference{}
 	_ fmt.Stringer     = LocalPackageReference{}
 )
 
-const generatorVersionPrefix string = "v1alpha1api"
+const GeneratorVersionPrefix string = "v1alpha1api"
 
 // MakeLocalPackageReference Creates a new local package reference from a group and version
-func MakeLocalPackageReference(prefix string, group string, version string) LocalPackageReference {
+func MakeLocalPackageReference(prefix string, group string, versionPrefix string, version string) LocalPackageReference {
 	return LocalPackageReference{
 		localPathPrefix: prefix,
 		group:           group,
-		version:         version,
+		versionPrefix:   versionPrefix,
+		version:         sanitizePackageName(version),
 	}
 }
 
-// CreateLocalPackageNameFromVersion transforms a version string (2018-06-01) into a package
-// name (v1alpha1api20180601)
-func CreateLocalPackageNameFromVersion(version string) string {
-	return generatorVersionPrefix + sanitizePackageName(version)
-}
-
-// sanitizePackageName removes all non-alphanum characters and converts to lower case
-func sanitizePackageName(input string) string {
-	var builder []rune = make([]rune, 0, len(input))
-
-	for _, r := range input {
-		if unicode.IsLetter(r) || unicode.IsNumber(r) {
-			builder = append(builder, unicode.ToLower(rune(r)))
-		}
-	}
-
-	return string(builder)
-}
-
 // LocalPathPrefix returns the prefix (everything up to the group name)
 func (pr LocalPackageReference) LocalPathPrefix() string {
 	return pr.localPathPrefix
@@ -65,17 +48,17 @@ func (pr LocalPackageReference) Group() string {
 
 // Version returns the version of this local reference
 func (pr LocalPackageReference) Version() string {
-	return pr.version
+	return pr.versionPrefix + pr.version
 }
 
 // PackageName returns the package name of this reference
 func (pr LocalPackageReference) PackageName() string {
-	return pr.version
+	return pr.Version()
 }
 
 // PackagePath returns the fully qualified package path
 func (pr LocalPackageReference) PackagePath() string {
-	url := pr.localPathPrefix + "/" + pr.group + "/" + pr.version
+	url := pr.localPathPrefix + "/" + pr.group + "/" + pr.PackageName()
 	return url
 }
 
@@ -87,6 +70,7 @@ func (pr LocalPackageReference) Equals(ref PackageReference) bool {
 
 	if other, ok := ref.(LocalPackageReference); ok {
 		return pr.localPathPrefix == other.localPathPrefix &&
+			pr.versionPrefix == other.versionPrefix &&
 			pr.version == other.version &&
 			pr.group == other.group
 	}
@@ -100,10 +84,18 @@ func (pr LocalPackageReference) String() string {
 }
 
 // IsPreview returns true if this package reference is a preview
+// We don't check the version prefix (which contains the version of the generator) as that may contain alpha or beta
+// even if the ARM version is not preview.
 func (pr LocalPackageReference) IsPreview() bool {
 	return containsPreviewVersionLabel(strings.ToLower(pr.version))
 }
 
+// WithVersionPrefix returns a new LocalPackageReference with a different version prefix
+func (pr LocalPackageReference) WithVersionPrefix(prefix string) LocalPackageReference {
+	pr.versionPrefix = prefix
+	return pr
+}
+
 // IsLocalPackageReference returns true if the supplied reference is a local one
 func IsLocalPackageReference(ref PackageReference) bool {
 	_, ok := ref.(LocalPackageReference)
@@ -112,5 +104,18 @@ func IsLocalPackageReference(ref PackageReference) bool {
 
 // GroupVersion returns the group and version of this local reference.
 func (pr LocalPackageReference) GroupVersion() (string, string, bool) {
-	return pr.group, pr.version, true
+	return pr.group, pr.Version(), true
+}
+
+// sanitizePackageName removes all non-alphanumeric characters and converts to lower case
+func sanitizePackageName(input string) string {
+	var builder = make([]rune, 0, len(input))
+
+	for _, r := range input {
+		if unicode.IsLetter(r) || unicode.IsNumber(r) {
+			builder = append(builder, unicode.ToLower(r))
+		}
+	}
+
+	return string(builder)
 }

v2/tools/generator/internal/astmodel/local_package_reference_test.go

@@ -12,28 +12,30 @@ import (
 )
 
 func makeTestLocalPackageReference(group string, version string) LocalPackageReference {
-	return MakeLocalPackageReference("github.com/Azure/azure-service-operator/v2/api", group, version)
+	// We use a fixed path and version prefixes to ensure consistency across testing
+	return MakeLocalPackageReference("github.com/Azure/azure-service-operator/v2/api", group, "v", version)
 }
 
 func TestMakeLocalPackageReference_GivenGroupAndPackage_ReturnsInstanceWithProperties(t *testing.T) {
 	t.Parallel()
 
 	cases := []struct {
-		name  string
-		group string
-		pkg   string
+		name    string
+		group   string
+		version string
+		pkg     string
 	}{
-		{"Networking", "microsoft.networking", "v20200901"},
-		{"Batch (new)", "microsoft.batch", "v20200901"},
-		{"Batch (old)", "microsoft.batch", "v20150101"},
+		{"Networking", "microsoft.networking", "2020-09-01", "v20200901"},
+		{"Batch (new)", "microsoft.batch", "2020-09-01", "v20200901"},
+		{"Batch (old)", "microsoft.batch", "2015-01-01", "v20150101"},
 	}
 	for _, c := range cases {
 		c := c
 		t.Run(c.name, func(t *testing.T) {
 			t.Parallel()
 			g := NewGomegaWithT(t)
 
-			ref := makeTestLocalPackageReference(c.group, c.pkg)
+			ref := makeTestLocalPackageReference(c.group, c.version)
 			grp := ref.Group()
 			pkg := ref.PackageName()
 
@@ -49,24 +51,28 @@ func TestLocalPackageReferences_ReturnExpectedProperties(t *testing.T) {
 	cases := []struct {
 		name         string
 		group        string
+		version      string
 		pkg          string
 		expectedPath string
 	}{
 		{
 			"Networking",
 			"microsoft.networking",
+			"2020-09-01",
 			"v20200901",
 			"github.com/Azure/azure-service-operator/v2/api/microsoft.networking/v20200901",
 		},
 		{
 			"Batch (new)",
 			"microsoft.batch",
+			"2020-09-01",
 			"v20200901",
 			"github.com/Azure/azure-service-operator/v2/api/microsoft.batch/v20200901",
 		},
 		{
 			"Batch (old)",
 			"microsoft.batch",
+			"2015-01-01",
 			"v20150101",
 			"github.com/Azure/azure-service-operator/v2/api/microsoft.batch/v20150101",
 		},
@@ -77,7 +83,7 @@ func TestLocalPackageReferences_ReturnExpectedProperties(t *testing.T) {
 			t.Parallel()
 			g := NewGomegaWithT(t)
 
-			ref := makeTestLocalPackageReference(c.group, c.pkg)
+			ref := makeTestLocalPackageReference(c.group, c.version)
 			grp := ref.Group()
 			g.Expect(ref.PackageName()).To(Equal(c.pkg))
 			g.Expect(ref.PackagePath()).To(Equal(c.expectedPath))
@@ -144,9 +150,7 @@ func TestLocalPackageReferenceIsPreview(t *testing.T) {
 			t.Parallel()
 			g := NewGomegaWithT(t)
 
-			ref := makeTestLocalPackageReference(
-				"microsoft.storage",
-				CreateLocalPackageNameFromVersion(c.version))
+			ref := makeTestLocalPackageReference("microsoft.storage", c.version)
 
 			g.Expect(ref.IsPreview()).To(Equal(c.isPreview))
 		})

v2/tools/generator/internal/astmodel/object_type.go

@@ -81,7 +81,7 @@ func (objectType *ObjectType) generateMethodDecls(codeGenerationContext *CodeGen
 	var result []dst.Decl
 
 	for _, f := range objectType.Functions() {
-		funcDef := f.AsFunc(codeGenerationContext, typeName)
+		funcDef := generateMethodDeclForFunction(typeName, f, codeGenerationContext)
 		result = append(result, funcDef)
 	}
 
@@ -453,6 +453,15 @@ func (objectType *ObjectType) WithFunction(function Function) *ObjectType {
 	return result
 }
 
+// WithoutFunctions creates a new ObjectType with no functions (useful for testing)
+func (objectType *ObjectType) WithoutFunctions() *ObjectType {
+	// Create a copy to preserve immutability
+	result := objectType.copy()
+	result.functions = make(map[string]Function)
+
+	return result
+}
+
 // WithInterface creates a new ObjectType that's a copy with an interface implementation attached
 func (objectType *ObjectType) WithInterface(iface *InterfaceImplementation) *ObjectType {
 	// Create a copy of objectType to preserve immutability

v2/tools/generator/internal/astmodel/package_reference.go

@@ -266,15 +266,8 @@ func (v *versionComparer) isPreviewVersionLabel(identifier string) (int, bool) {
 // special set, and if so returns its true. If the passed identifier does not contain one,
 // returns false.
 func containsPreviewVersionLabel(identifier string) bool {
-	// Work out where the API version starts in the identifier
-	// (We don't want to return preview just because the operator itself is in preview)
-	startOfAPIVersion := 0
-	if strings.HasPrefix(identifier, generatorVersionPrefix) {
-		startOfAPIVersion = len(generatorVersionPrefix)
-	}
-
 	for _, id := range previewVersionLabels {
-		if strings.LastIndex(identifier, id) > startOfAPIVersion {
+		if strings.LastIndex(identifier, id) > 0 {
 			return true
 		}
 	}

v2/tools/generator/internal/astmodel/property_reference_test.go

@@ -43,7 +43,7 @@ func TestPropertyReference_IsEmpty_ReturnsExpectedResult(t *testing.T) {
 func TestPropertyReference_String_ReturnsExpectedResult(t *testing.T) {
 	t.Parallel()
 	g := NewGomegaWithT(t)
-	pkg := makeTestLocalPackageReference("Demo", "v1")
+	pkg := makeTestLocalPackageReference("Demo", "1")
 	declaringType := MakeTypeName(pkg, "Person")
 	property := PropertyName("FullName")
 

v2/tools/generator/internal/astmodel/resource_type.go

@@ -225,6 +225,15 @@ func (resource *ResourceType) WithFunction(function Function) *ResourceType {
 	return result
 }
 
+// WithoutFunctions creates a new Resource with no functions (useful for testing)
+func (resource *ResourceType) WithoutFunctions() *ResourceType {
+	// Create a copy to preserve immutability
+	result := resource.copy()
+	result.functions = make(map[string]Function)
+
+	return result
+}
+
 // WithTestCase creates a new Resource that's a copy with an additional test case included
 func (resource *ResourceType) WithTestCase(testcase TestCase) *ResourceType {
 	result := resource.copy()
@@ -596,7 +605,7 @@ func (resource *ResourceType) generateMethodDecls(codeGenerationContext *CodeGen
 	var result []dst.Decl
 
 	for _, f := range resource.Functions() {
-		funcDef := f.AsFunc(codeGenerationContext, typeName)
+		funcDef := generateMethodDeclForFunction(typeName, f, codeGenerationContext)
 		result = append(result, funcDef)
 	}
 
@@ -717,3 +726,23 @@ func (resource *ResourceType) WriteDebugDescription(builder *strings.Builder, de
 	resource.status.WriteDebugDescription(builder, definitions)
 	builder.WriteString("]")
 }
+
+// generateMethodDeclForFunction generates the AST for a function; if a panic occurs, the identity of the type and
+// function being generated will be wrapped around the existing panic details to aid in debugging.
+func generateMethodDeclForFunction(
+	typeName TypeName,
+	f Function,
+	codeGenerationContext *CodeGenerationContext) *dst.FuncDecl {
+
+	defer func() {
+		if err := recover(); err != nil {
+			panic(fmt.Sprintf(
+				"generating method declaration for %s.%s: %s",
+				typeName.Name(),
+				f.Name(),
+				err))
+		}
+	}()
+
+	return f.AsFunc(codeGenerationContext, typeName)
+}