fix: compressArticle used hard-coded indices, PSU rendered as NIC when GPU absent
compressArticle assumed a fixed segment layout [model,cpu,mem,gpu,disk,net,psu,support].
Build() skips empty segments, so without GPU the PSU slot shifted to index 5. Step 2 of
compression called compressNetSegment on the PSU value ("2x1kW") which returned "2xNIC".
Replace positional indexing with namedSeg{group,value} tagged segments; compressArticle
now looks up each segment by group name via findSegGroup(), regardless of array length.
Regression test TestBuild_CompressArticle_NoGPU_PSUNotNIC reproduces the exact config
from OPS-2445 (NF5280M6 + 2xPSU, no GPU) that triggered the bug.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -22,24 +22,29 @@ type BuildResult struct {
|
|||||||
}
|
}
|
||||||
|
|
||||||
var (
|
var (
|
||||||
reMemGiB = regexp.MustCompile(`(?i)(\d+)\s*(GB|G)`)
|
reMemGiB = regexp.MustCompile(`(?i)(\d+)\s*(GB|G)`)
|
||||||
reMemTiB = regexp.MustCompile(`(?i)(\d+)\s*(TB|T)`)
|
reMemTiB = regexp.MustCompile(`(?i)(\d+)\s*(TB|T)`)
|
||||||
reCapacityT = regexp.MustCompile(`(?i)(\d+(?:[.,]\d+)?)T`)
|
reCapacityT = regexp.MustCompile(`(?i)(\d+(?:[.,]\d+)?)T`)
|
||||||
reCapacityG = regexp.MustCompile(`(?i)(\d+(?:[.,]\d+)?)G`)
|
reCapacityG = regexp.MustCompile(`(?i)(\d+(?:[.,]\d+)?)G`)
|
||||||
rePortSpeed = regexp.MustCompile(`(?i)(\d+)p(\d+)(GbE|G)`)
|
rePortSpeed = regexp.MustCompile(`(?i)(\d+)p(\d+)(GbE|G)`)
|
||||||
rePortFC = regexp.MustCompile(`(?i)(\d+)pFC(\d+)`)
|
rePortFC = regexp.MustCompile(`(?i)(\d+)pFC(\d+)`)
|
||||||
reWatts = regexp.MustCompile(`(?i)(\d{3,5})\s*W`)
|
reWatts = regexp.MustCompile(`(?i)(\d{3,5})\s*W`)
|
||||||
)
|
)
|
||||||
|
|
||||||
|
type namedSeg struct {
|
||||||
|
group string // "MODEL","CPU","MEM","GPU","DISK","NET","PSU","SUPPORT"
|
||||||
|
value string
|
||||||
|
}
|
||||||
|
|
||||||
func Build(local *localdb.LocalDB, items []models.ConfigItem, opts BuildOptions) (BuildResult, error) {
|
func Build(local *localdb.LocalDB, items []models.ConfigItem, opts BuildOptions) (BuildResult, error) {
|
||||||
segments := make([]string, 0, 8)
|
segs := make([]namedSeg, 0, 8)
|
||||||
warnings := make([]string, 0)
|
warnings := make([]string, 0)
|
||||||
|
|
||||||
model := NormalizeServerModel(opts.ServerModel)
|
model := NormalizeServerModel(opts.ServerModel)
|
||||||
if model == "" {
|
if model == "" {
|
||||||
return BuildResult{}, fmt.Errorf("server_model required")
|
return BuildResult{}, fmt.Errorf("server_model required")
|
||||||
}
|
}
|
||||||
segments = append(segments, model)
|
segs = append(segs, namedSeg{"MODEL", model})
|
||||||
|
|
||||||
lotNames := make([]string, 0, len(items))
|
lotNames := make([]string, 0, len(items))
|
||||||
for _, it := range items {
|
for _, it := range items {
|
||||||
@@ -55,41 +60,39 @@ func Build(local *localdb.LocalDB, items []models.ConfigItem, opts BuildOptions)
|
|||||||
return BuildResult{}, err
|
return BuildResult{}, err
|
||||||
}
|
}
|
||||||
|
|
||||||
cpuSeg := buildCPUSegment(items, cats)
|
if cpuSeg := buildCPUSegment(items, cats); cpuSeg != "" {
|
||||||
if cpuSeg != "" {
|
segs = append(segs, namedSeg{"CPU", cpuSeg})
|
||||||
segments = append(segments, cpuSeg)
|
|
||||||
}
|
}
|
||||||
memSeg, memWarn := buildMemSegment(items, cats)
|
memSeg, memWarn := buildMemSegment(items, cats)
|
||||||
if memWarn != "" {
|
if memWarn != "" {
|
||||||
warnings = append(warnings, memWarn)
|
warnings = append(warnings, memWarn)
|
||||||
}
|
}
|
||||||
if memSeg != "" {
|
if memSeg != "" {
|
||||||
segments = append(segments, memSeg)
|
segs = append(segs, namedSeg{"MEM", memSeg})
|
||||||
}
|
}
|
||||||
gpuSeg := buildGPUSegment(items, cats)
|
if gpuSeg := buildGPUSegment(items, cats); gpuSeg != "" {
|
||||||
if gpuSeg != "" {
|
segs = append(segs, namedSeg{"GPU", gpuSeg})
|
||||||
segments = append(segments, gpuSeg)
|
|
||||||
}
|
}
|
||||||
diskSeg, diskWarn := buildDiskSegment(items, cats)
|
diskSeg, diskWarn := buildDiskSegment(items, cats)
|
||||||
if diskWarn != "" {
|
if diskWarn != "" {
|
||||||
warnings = append(warnings, diskWarn)
|
warnings = append(warnings, diskWarn)
|
||||||
}
|
}
|
||||||
if diskSeg != "" {
|
if diskSeg != "" {
|
||||||
segments = append(segments, diskSeg)
|
segs = append(segs, namedSeg{"DISK", diskSeg})
|
||||||
}
|
}
|
||||||
netSeg, netWarn := buildNetSegment(items, cats)
|
netSeg, netWarn := buildNetSegment(items, cats)
|
||||||
if netWarn != "" {
|
if netWarn != "" {
|
||||||
warnings = append(warnings, netWarn)
|
warnings = append(warnings, netWarn)
|
||||||
}
|
}
|
||||||
if netSeg != "" {
|
if netSeg != "" {
|
||||||
segments = append(segments, netSeg)
|
segs = append(segs, namedSeg{"NET", netSeg})
|
||||||
}
|
}
|
||||||
psuSeg, psuWarn := buildPSUSegment(items, cats)
|
psuSeg, psuWarn := buildPSUSegment(items, cats)
|
||||||
if psuWarn != "" {
|
if psuWarn != "" {
|
||||||
warnings = append(warnings, psuWarn)
|
warnings = append(warnings, psuWarn)
|
||||||
}
|
}
|
||||||
if psuSeg != "" {
|
if psuSeg != "" {
|
||||||
segments = append(segments, psuSeg)
|
segs = append(segs, namedSeg{"PSU", psuSeg})
|
||||||
}
|
}
|
||||||
|
|
||||||
if strings.TrimSpace(opts.SupportCode) != "" {
|
if strings.TrimSpace(opts.SupportCode) != "" {
|
||||||
@@ -97,12 +100,12 @@ func Build(local *localdb.LocalDB, items []models.ConfigItem, opts BuildOptions)
|
|||||||
if !isSupportCodeValid(code) {
|
if !isSupportCodeValid(code) {
|
||||||
return BuildResult{}, fmt.Errorf("invalid_support_code")
|
return BuildResult{}, fmt.Errorf("invalid_support_code")
|
||||||
}
|
}
|
||||||
segments = append(segments, code)
|
segs = append(segs, namedSeg{"SUPPORT", code})
|
||||||
}
|
}
|
||||||
|
|
||||||
article := strings.Join(segments, "-")
|
article := strings.Join(namedSegsValues(segs), "-")
|
||||||
if len([]rune(article)) > 80 {
|
if len([]rune(article)) > 80 {
|
||||||
article = compressArticle(segments)
|
article = compressArticle(segs)
|
||||||
warnings = append(warnings, "compressed")
|
warnings = append(warnings, "compressed")
|
||||||
}
|
}
|
||||||
if len([]rune(article)) > 80 {
|
if len([]rune(article)) > 80 {
|
||||||
@@ -112,6 +115,23 @@ func Build(local *localdb.LocalDB, items []models.ConfigItem, opts BuildOptions)
|
|||||||
return BuildResult{Article: article, Warnings: warnings}, nil
|
return BuildResult{Article: article, Warnings: warnings}, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func namedSegsValues(segs []namedSeg) []string {
|
||||||
|
out := make([]string, len(segs))
|
||||||
|
for i, s := range segs {
|
||||||
|
out[i] = s.value
|
||||||
|
}
|
||||||
|
return out
|
||||||
|
}
|
||||||
|
|
||||||
|
func findSegGroup(segs []namedSeg, group string) int {
|
||||||
|
for i, s := range segs {
|
||||||
|
if s.group == group {
|
||||||
|
return i
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return -1
|
||||||
|
}
|
||||||
|
|
||||||
func isSupportCodeValid(code string) bool {
|
func isSupportCodeValid(code string) bool {
|
||||||
if len(code) < 3 {
|
if len(code) < 3 {
|
||||||
return false
|
return false
|
||||||
@@ -484,60 +504,50 @@ func atoi(v string) int {
|
|||||||
return n
|
return n
|
||||||
}
|
}
|
||||||
|
|
||||||
func compressArticle(segments []string) string {
|
func compressArticle(segs []namedSeg) string {
|
||||||
if len(segments) == 0 {
|
if len(segs) == 0 {
|
||||||
return ""
|
return ""
|
||||||
}
|
}
|
||||||
normalized := make([]string, 0, len(segments))
|
for i, s := range segs {
|
||||||
for _, s := range segments {
|
segs[i].value = strings.ReplaceAll(s.value, "GbE", "G")
|
||||||
normalized = append(normalized, strings.ReplaceAll(s, "GbE", "G"))
|
|
||||||
}
|
}
|
||||||
segments = normalized
|
article := strings.Join(namedSegsValues(segs), "-")
|
||||||
article := strings.Join(segments, "-")
|
|
||||||
if len([]rune(article)) <= 80 {
|
if len([]rune(article)) <= 80 {
|
||||||
return article
|
return article
|
||||||
}
|
}
|
||||||
|
|
||||||
// segment order: model, cpu, mem, gpu, disk, net, psu, support
|
|
||||||
index := func(i int) (int, bool) {
|
|
||||||
if i >= 0 && i < len(segments) {
|
|
||||||
return i, true
|
|
||||||
}
|
|
||||||
return -1, false
|
|
||||||
}
|
|
||||||
|
|
||||||
// 1) remove PSU
|
// 1) remove PSU
|
||||||
if i, ok := index(6); ok {
|
if i := findSegGroup(segs, "PSU"); i >= 0 {
|
||||||
segments = append(segments[:i], segments[i+1:]...)
|
segs = append(segs[:i], segs[i+1:]...)
|
||||||
article = strings.Join(segments, "-")
|
article = strings.Join(namedSegsValues(segs), "-")
|
||||||
if len([]rune(article)) <= 80 {
|
if len([]rune(article)) <= 80 {
|
||||||
return article
|
return article
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// 2) compress NET/HBA/HCA
|
// 2) compress NET/HBA/HCA
|
||||||
if i, ok := index(5); ok {
|
if i := findSegGroup(segs, "NET"); i >= 0 {
|
||||||
segments[i] = compressNetSegment(segments[i])
|
segs[i].value = compressNetSegment(segs[i].value)
|
||||||
article = strings.Join(segments, "-")
|
article = strings.Join(namedSegsValues(segs), "-")
|
||||||
if len([]rune(article)) <= 80 {
|
if len([]rune(article)) <= 80 {
|
||||||
return article
|
return article
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// 3) compress DISK
|
// 3) compress DISK
|
||||||
if i, ok := index(4); ok {
|
if i := findSegGroup(segs, "DISK"); i >= 0 {
|
||||||
segments[i] = compressDiskSegment(segments[i])
|
segs[i].value = compressDiskSegment(segs[i].value)
|
||||||
article = strings.Join(segments, "-")
|
article = strings.Join(namedSegsValues(segs), "-")
|
||||||
if len([]rune(article)) <= 80 {
|
if len([]rune(article)) <= 80 {
|
||||||
return article
|
return article
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// 4) compress GPU to vendor only (GPU_NV)
|
// 4) compress GPU to vendor only (GPU_NV)
|
||||||
if i, ok := index(3); ok {
|
if i := findSegGroup(segs, "GPU"); i >= 0 {
|
||||||
segments[i] = compressGPUSegment(segments[i])
|
segs[i].value = compressGPUSegment(segs[i].value)
|
||||||
}
|
}
|
||||||
return strings.Join(segments, "-")
|
return strings.Join(namedSegsValues(segs), "-")
|
||||||
}
|
}
|
||||||
|
|
||||||
func compressNetSegment(seg string) string {
|
func compressNetSegment(seg string) string {
|
||||||
|
|||||||
@@ -61,6 +61,79 @@ func TestBuild_ParsesNetAndPSU(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestBuild_CompressArticle_NoGPU_PSUNotNIC reproduces the bug where 2 PSUs produced
|
||||||
|
// "2xNIC" in the article because compressArticle used hard-coded indices that assumed
|
||||||
|
// GPU was always present.
|
||||||
|
func TestBuild_CompressArticle_NoGPU_PSUNotNIC(t *testing.T) {
|
||||||
|
local, err := localdb.New(filepath.Join(t.TempDir(), "local.db"))
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("init local db: %v", err)
|
||||||
|
}
|
||||||
|
t.Cleanup(func() { _ = local.Close() })
|
||||||
|
|
||||||
|
if err := local.SaveLocalPricelist(&localdb.LocalPricelist{
|
||||||
|
ServerID: 2,
|
||||||
|
Source: "estimate",
|
||||||
|
Version: "S-2026-05-19-001",
|
||||||
|
Name: "test",
|
||||||
|
CreatedAt: time.Now(),
|
||||||
|
SyncedAt: time.Now(),
|
||||||
|
}); err != nil {
|
||||||
|
t.Fatalf("save local pricelist: %v", err)
|
||||||
|
}
|
||||||
|
localPL, err := local.GetLocalPricelistByServerID(2)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("get local pricelist: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
if err := local.SaveLocalPricelistItems([]localdb.LocalPricelistItem{
|
||||||
|
{PricelistID: localPL.ID, LotName: "CPU_INTEL_8358", LotCategory: "CPU", Price: 1},
|
||||||
|
{PricelistID: localPL.ID, LotName: "MEM_DDR4_64G_3200", LotCategory: "MEM", Price: 1},
|
||||||
|
{PricelistID: localPL.ID, LotName: "SSD_SATA_0.4T", LotCategory: "SSD", Price: 1},
|
||||||
|
{PricelistID: localPL.ID, LotName: "SSD_SATA_0.9T", LotCategory: "SSD", Price: 1},
|
||||||
|
{PricelistID: localPL.ID, LotName: "HDD_SATA_16T", LotCategory: "HDD", Price: 1},
|
||||||
|
{PricelistID: localPL.ID, LotName: "NIC_2p25G_MCX512A", LotCategory: "NIC", Price: 1},
|
||||||
|
{PricelistID: localPL.ID, LotName: "HBA_2pFC32_Gen6", LotCategory: "HBA", Price: 1},
|
||||||
|
{PricelistID: localPL.ID, LotName: "NIC_4p1G_I350", LotCategory: "NIC", Price: 1},
|
||||||
|
{PricelistID: localPL.ID, LotName: "PS_1500W_Platinum", LotCategory: "PS", Price: 1},
|
||||||
|
}); err != nil {
|
||||||
|
t.Fatalf("save local items: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// PS_1500W → "2x1.5kW" (7 chars) brings uncompressed article to 81 chars, triggering
|
||||||
|
// compressArticle. Before the fix, compressArticle used hard-coded index 5 for NET, but
|
||||||
|
// without GPU the PSU sits at index 5, so compressNetSegment("2x1.5kW") returned "2xNIC".
|
||||||
|
items := models.ConfigItems{
|
||||||
|
{LotName: "CPU_INTEL_8358", Quantity: 2},
|
||||||
|
{LotName: "MEM_DDR4_64G_3200", Quantity: 16}, // 1024 GiB = 1T
|
||||||
|
{LotName: "SSD_SATA_0.4T", Quantity: 2},
|
||||||
|
{LotName: "SSD_SATA_0.9T", Quantity: 4},
|
||||||
|
{LotName: "HDD_SATA_16T", Quantity: 6},
|
||||||
|
{LotName: "NIC_2p25G_MCX512A", Quantity: 1},
|
||||||
|
{LotName: "HBA_2pFC32_Gen6", Quantity: 1},
|
||||||
|
{LotName: "NIC_4p1G_I350", Quantity: 1},
|
||||||
|
{LotName: "PS_1500W_Platinum", Quantity: 2},
|
||||||
|
}
|
||||||
|
result, err := Build(local, items, BuildOptions{
|
||||||
|
ServerModel: "NF5280M6",
|
||||||
|
ServerPricelist: &localPL.ServerID,
|
||||||
|
})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("build article: %v", err)
|
||||||
|
}
|
||||||
|
if len([]rune(result.Article)) > 80 {
|
||||||
|
t.Fatalf("article too long (%d): %s", len([]rune(result.Article)), result.Article)
|
||||||
|
}
|
||||||
|
// PSU segment must not be mis-labeled as NIC during compression
|
||||||
|
// The correct behaviour: PSU is dropped, NET stays as-is or compressed to HBA/NIC labels
|
||||||
|
// Before the fix: article ended with "-2xNIC" (PSU turned into NIC)
|
||||||
|
// After the fix: article must not contain a standalone "NIC" that came from PSU wattage
|
||||||
|
if strings.HasSuffix(result.Article, "-2xNIC") {
|
||||||
|
t.Fatalf("PSU mis-labeled as NIC in article: %s", result.Article)
|
||||||
|
}
|
||||||
|
t.Logf("article: %s (warnings: %v)", result.Article, result.Warnings)
|
||||||
|
}
|
||||||
|
|
||||||
func contains(s, sub string) bool {
|
func contains(s, sub string) bool {
|
||||||
return strings.Contains(s, sub)
|
return strings.Contains(s, sub)
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user