Skip to content

Commit 96937f1

Browse files
Fix report case sensitivity (#4433)
* Add tests for desired behaviour * Modify report to handle inconstencies * Add missing t.Parallel() call
1 parent 1b3c4a9 commit 96937f1

File tree

2 files changed

+133
-16
lines changed

2 files changed

+133
-16
lines changed

v2/tools/generator/internal/codegen/pipeline/report_resource_versions.go

+53-16
Original file line numberDiff line numberDiff line change
@@ -175,15 +175,15 @@ func (report *ResourceVersionsReport) summarize(definitions astmodel.TypeDefinit
175175
name := rsrc.Name()
176176
pkg := name.PackageReference()
177177

178-
defType := astmodel.MustBeResourceType(rsrc.Type())
179-
armVersion := strings.Trim(defType.APIVersionEnumValue().Value, "\"")
180-
item := report.createItem(name, defType.ARMType(), armVersion)
181-
182178
if astmodel.IsStoragePackageReference(pkg) {
183179
// Skip storage versions - they're an implementation detail
184180
continue
185181
}
186182

183+
defType := astmodel.MustBeResourceType(rsrc.Type())
184+
armVersion := strings.Trim(defType.APIVersionEnumValue().Value, "\"")
185+
item := report.createItem(name, defType.ARMType(), armVersion)
186+
187187
report.addItem(item)
188188
}
189189

@@ -741,18 +741,45 @@ func (report *ResourceVersionsReport) writeFragment(name string, data any, buffe
741741
return nil
742742
}
743743

744-
// groupTitle returns the title to use for the given group, based on the first resource found in that group
744+
type reportMetadataQuality int
745+
746+
const (
747+
poor reportMetadataQuality = 10
748+
good reportMetadataQuality = 20
749+
better reportMetadataQuality = 30
750+
best reportMetadataQuality = 40
751+
)
752+
753+
// groupTitle returns metadata to use for the given group.
754+
// group is the name of the group to generate metadata for.
755+
// items is the set of items in the group.
756+
// Some resource-providers (incl alertsmanagement) are inconsistent with letter casing, so we
757+
// look for the best information we have.
745758
func (report *ResourceVersionsReport) groupInfo(
746759
group string,
747760
items set.Set[ResourceVersionsReportResourceItem],
748761
) *ResourceVersionsReportGroupInfo {
749-
caser := cases.Title(language.English)
762+
caser := cases.Title(language.English, cases.NoLower)
750763
result := &ResourceVersionsReportGroupInfo{
751764
Group: group,
752765
Title: caser.String(group),
753766
Provider: caser.String(group),
754767
}
768+
currentQuality := poor
769+
770+
saveCandidate := func(
771+
title string,
772+
provider string,
773+
quality reportMetadataQuality,
774+
) {
775+
if quality > currentQuality {
776+
result.Title = title
777+
result.Provider = provider
778+
currentQuality = quality
779+
}
780+
}
755781

782+
// Scan through available resources to find better values for Title & Provider
756783
for item := range items {
757784
if item.armType == "" {
758785
// Didn't find a resource, keep looking
@@ -761,17 +788,27 @@ func (report *ResourceVersionsReport) groupInfo(
761788

762789
// Slice off the part before the first "/" to get the Provider name
763790
parts := strings.Split(item.armType, "/")
764-
if len(parts) == 0 {
765-
// String doesn't look like a resource type, keep looking
766-
continue
791+
provider := parts[0]
792+
793+
const prefix = "Microsoft."
794+
if strings.HasPrefix(provider, prefix) {
795+
// If the provider starts with exactly "Microsoft." (with a case-sensitive check),
796+
// that's our best bet for a properly cased provider name
797+
saveCandidate(
798+
strings.TrimPrefix(provider, prefix),
799+
provider,
800+
best)
801+
} else if strings.HasPrefix(strings.ToLower(provider), strings.ToLower(prefix)) {
802+
// If the provider starts with "Microsoft." (but doesn't match the case exactly),
803+
// that's not so good, but better than the defaults
804+
saveCandidate(
805+
caser.String(provider[len(prefix):]),
806+
caser.String(provider),
807+
better)
808+
} else {
809+
// Just use what we have
810+
saveCandidate(provider, provider, good)
767811
}
768-
769-
// Store the provider name
770-
result.Provider = parts[0]
771-
772-
// Remove the "Microsoft." prefix to make the title
773-
result.Title = strings.TrimPrefix(result.Provider, "Microsoft.")
774-
break
775812
}
776813

777814
return result

v2/tools/generator/internal/codegen/pipeline/report_resource_versions_test.go

+80
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
. "github.com/onsi/gomega"
1313
"github.com/sebdah/goldie/v2"
1414

15+
"github.com/Azure/azure-service-operator/v2/internal/set"
1516
"github.com/Azure/azure-service-operator/v2/tools/generator/internal/astmodel"
1617
"github.com/Azure/azure-service-operator/v2/tools/generator/internal/config"
1718
"github.com/Azure/azure-service-operator/v2/tools/generator/internal/test"
@@ -95,3 +96,82 @@ func TestGolden_ReportAllResourceVersions(t *testing.T) {
9596

9697
gold.Assert(t, t.Name(), []byte(buffer.String()))
9798
}
99+
100+
func TestResourceVersionsReportGroupInfo_GivenGroup_ReturnsExpectedResult(t *testing.T) {
101+
t.Parallel()
102+
103+
storagePkg := test.MakeLocalPackageReference("storage", "v20230101")
104+
105+
storageAccount := ResourceVersionsReportResourceItem{
106+
name: astmodel.MakeInternalTypeName(storagePkg, "StorageAccount"),
107+
armType: "Microsoft.Storage/storageAccounts",
108+
armVersion: "2023-01-01",
109+
supportedFrom: "v2.0.0",
110+
}
111+
112+
alertsManagementPkg := test.MakeLocalPackageReference("alertsmanagement", "v20210401")
113+
114+
smartDetector := ResourceVersionsReportResourceItem{
115+
name: astmodel.MakeInternalTypeName(alertsManagementPkg, "SmartDetector"),
116+
armType: "microsoft.alertsManagement/smartDetectorAlertRules",
117+
armVersion: "2021-04-01",
118+
supportedFrom: "v2.11.0",
119+
}
120+
121+
prometheusRuleGroup := ResourceVersionsReportResourceItem{
122+
name: astmodel.MakeInternalTypeName(alertsManagementPkg, "PrometheusRuleGroup"),
123+
armType: "Microsoft.AlertsManagement/prometheusRuleGroups",
124+
}
125+
126+
cases := map[string]struct {
127+
group string
128+
items set.Set[ResourceVersionsReportResourceItem]
129+
expectedGroup string
130+
expectedProvider string
131+
expectedTitle string
132+
}{
133+
"StorageAccount": {
134+
group: "storage",
135+
items: set.Make(storageAccount),
136+
expectedGroup: "storage",
137+
expectedProvider: "Microsoft.Storage",
138+
expectedTitle: "Storage",
139+
},
140+
"SmartDetector": {
141+
group: "alertsmanagement",
142+
items: set.Make(smartDetector),
143+
expectedGroup: "alertsmanagement",
144+
expectedProvider: "Microsoft.alertsManagement",
145+
expectedTitle: "AlertsManagement",
146+
},
147+
"PrometheusRuleGroup": {
148+
group: "alertsmanagement",
149+
items: set.Make(prometheusRuleGroup),
150+
expectedGroup: "alertsmanagement",
151+
expectedProvider: "Microsoft.AlertsManagement",
152+
expectedTitle: "AlertsManagement",
153+
},
154+
"Prefers Correct Case": {
155+
group: "alertsmanagement",
156+
items: set.Make(smartDetector, prometheusRuleGroup),
157+
expectedGroup: "alertsmanagement",
158+
expectedProvider: "Microsoft.AlertsManagement",
159+
expectedTitle: "AlertsManagement",
160+
},
161+
}
162+
163+
for name, c := range cases {
164+
t.Run(name, func(t *testing.T) {
165+
t.Parallel()
166+
g := NewGomegaWithT(t)
167+
168+
report := &ResourceVersionsReport{} // empty report
169+
170+
info := report.groupInfo(c.group, c.items)
171+
g.Expect(info).ToNot(BeNil())
172+
g.Expect(info.Group).To(Equal(c.expectedGroup))
173+
g.Expect(info.Provider).To(Equal(c.expectedProvider))
174+
g.Expect(info.Title).To(Equal(c.expectedTitle))
175+
})
176+
}
177+
}

0 commit comments

Comments
 (0)