fix: wrap GROUP_CONCAT with COALESCE to prevent NULL scan on products with no pans

FindProductSummary and FindColorDetail aggregate pan.position via GROUP_CONCAT
with a LEFT JOIN. When a product has no pans the join produces all-NULL rows,
causing GROUP_CONCAT to return NULL, which cannot be scanned into a Go string —
making the entire summary query fail silently and the frontend show an empty table.

Fix: wrap both GROUP_CONCAT(pan.position) calls with COALESCE(..., '').

Also removes CostPrice from the generated InvProduct model (field dropped from schema).

Adds integration tests TestFindProductSummaryNoPans and TestFindColorDetailNoPans
to guard against this class of NULL-scan regression.
This commit is contained in:
kae 2026-06-16 03:30:39 +09:00 committed by Chever John
parent 541dd5e4c8
commit 4759295ce1
3 changed files with 351 additions and 7 deletions

View File

@ -151,8 +151,8 @@ func (m *customInvProductModel) FindProductSummary(ctx context.Context, tenantId
COUNT(DISTINCT pan.pan_id) AS total_pan_count, COUNT(DISTINCT pan.pan_id) AS total_pan_count,
COUNT(bolt.bolt_id) AS total_bolt_count, COUNT(bolt.bolt_id) AS total_bolt_count,
COALESCE(SUM(bolt.length_m), 0) AS total_length_m, COALESCE(SUM(bolt.length_m), 0) AS total_length_m,
GROUP_CONCAT(DISTINCT p.color) AS colors, COALESCE(GROUP_CONCAT(DISTINCT p.color), '') AS colors,
GROUP_CONCAT(DISTINCT pan.position ORDER BY pan.position) AS locations COALESCE(GROUP_CONCAT(DISTINCT pan.position ORDER BY pan.position), '') AS locations
FROM ` + m.table + ` p FROM ` + m.table + ` p
LEFT JOIN ` + "`inv_product_pan`" + ` pan ON pan.product_id = p.product_id LEFT JOIN ` + "`inv_product_pan`" + ` pan ON pan.product_id = p.product_id
LEFT JOIN ` + "`inv_product_bolt`" + ` bolt ON bolt.pan_id = pan.pan_id LEFT JOIN ` + "`inv_product_bolt`" + ` bolt ON bolt.pan_id = pan.pan_id
@ -204,7 +204,7 @@ func (m *customInvProductModel) FindColorDetail(ctx context.Context, tenantId, p
COUNT(DISTINCT pan.pan_id) AS pan_count, COUNT(DISTINCT pan.pan_id) AS pan_count,
COUNT(bolt.bolt_id) AS bolt_count, COUNT(bolt.bolt_id) AS bolt_count,
COALESCE(SUM(bolt.length_m), 0) AS total_length_m, COALESCE(SUM(bolt.length_m), 0) AS total_length_m,
GROUP_CONCAT(DISTINCT pan.position ORDER BY pan.position) AS locations COALESCE(GROUP_CONCAT(DISTINCT pan.position ORDER BY pan.position), '') AS locations
FROM ` + m.table + ` p FROM ` + m.table + ` p
LEFT JOIN ` + "`inv_product_pan`" + ` pan ON pan.product_id = p.product_id LEFT JOIN ` + "`inv_product_pan`" + ` pan ON pan.product_id = p.product_id
LEFT JOIN ` + "`inv_product_bolt`" + ` bolt ON bolt.pan_id = pan.pan_id LEFT JOIN ` + "`inv_product_bolt`" + ` bolt ON bolt.pan_id = pan.pan_id

View File

@ -52,7 +52,6 @@ type (
Spec string `db:"spec"` Spec string `db:"spec"`
Color string `db:"color"` Color string `db:"color"`
SalesPrice float64 `db:"sales_price"` SalesPrice float64 `db:"sales_price"`
CostPrice float64 `db:"cost_price"`
Remark sql.NullString `db:"remark"` Remark sql.NullString `db:"remark"`
Status int64 `db:"status"` Status int64 `db:"status"`
TenantId string `db:"tenant_id"` TenantId string `db:"tenant_id"`
@ -147,8 +146,8 @@ func (m *defaultInvProductModel) Insert(ctx context.Context, data *InvProduct) (
invProductProductIdKey := fmt.Sprintf("%s%v", cacheInvProductProductIdPrefix, data.ProductId) invProductProductIdKey := fmt.Sprintf("%s%v", cacheInvProductProductIdPrefix, data.ProductId)
invProductProductNameKey := fmt.Sprintf("%s%v", cacheInvProductProductNamePrefix, data.ProductName) invProductProductNameKey := fmt.Sprintf("%s%v", cacheInvProductProductNamePrefix, data.ProductName)
ret, err := m.ExecCtx(ctx, func(ctx context.Context, conn sqlx.SqlConn) (result sql.Result, err error) { ret, err := m.ExecCtx(ctx, func(ctx context.Context, conn sqlx.SqlConn) (result sql.Result, err error) {
query := fmt.Sprintf("insert into %s (%s) values (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)", m.table, invProductRowsExpectAutoSet) query := fmt.Sprintf("insert into %s (%s) values (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)", m.table, invProductRowsExpectAutoSet)
return conn.ExecCtx(ctx, query, data.ProductId, data.ProductName, data.ImageUrl, data.Spec, data.Color, data.SalesPrice, data.CostPrice, data.Remark, data.Status, data.TenantId, data.DeletedAt) return conn.ExecCtx(ctx, query, data.ProductId, data.ProductName, data.ImageUrl, data.Spec, data.Color, data.SalesPrice, data.Remark, data.Status, data.TenantId, data.DeletedAt)
}, invProductIdKey, invProductProductIdKey, invProductProductNameKey) }, invProductIdKey, invProductProductIdKey, invProductProductNameKey)
return ret, err return ret, err
} }
@ -164,7 +163,7 @@ func (m *defaultInvProductModel) Update(ctx context.Context, newData *InvProduct
invProductProductNameKey := fmt.Sprintf("%s%v", cacheInvProductProductNamePrefix, data.ProductName) invProductProductNameKey := fmt.Sprintf("%s%v", cacheInvProductProductNamePrefix, data.ProductName)
_, err = m.ExecCtx(ctx, func(ctx context.Context, conn sqlx.SqlConn) (result sql.Result, err error) { _, err = m.ExecCtx(ctx, func(ctx context.Context, conn sqlx.SqlConn) (result sql.Result, err error) {
query := fmt.Sprintf("update %s set %s where `id` = ?", m.table, invProductRowsWithPlaceHolder) query := fmt.Sprintf("update %s set %s where `id` = ?", m.table, invProductRowsWithPlaceHolder)
return conn.ExecCtx(ctx, query, newData.ProductId, newData.ProductName, newData.ImageUrl, newData.Spec, newData.Color, newData.SalesPrice, newData.CostPrice, newData.Remark, newData.Status, newData.TenantId, newData.DeletedAt, newData.Id) return conn.ExecCtx(ctx, query, newData.ProductId, newData.ProductName, newData.ImageUrl, newData.Spec, newData.Color, newData.SalesPrice, newData.Remark, newData.Status, newData.TenantId, newData.DeletedAt, newData.Id)
}, invProductIdKey, invProductProductIdKey, invProductProductNameKey) }, invProductIdKey, invProductProductIdKey, invProductProductNameKey)
return err return err
} }

View File

@ -0,0 +1,345 @@
package model
// Integration tests for InvProduct model against the real dev database.
// These tests guard against model/schema drift (e.g. missing or extra columns).
//
// Run with the dev DB reachable:
// go test ./model/ -run TestInvProduct -v
// Skip in CI where DB is unavailable:
// go test ./model/ -short
import (
"context"
"database/sql"
"fmt"
"testing"
"time"
"github.com/zeromicro/go-zero/core/stores/cache"
"github.com/zeromicro/go-zero/core/stores/redis"
"github.com/zeromicro/go-zero/core/stores/sqlx"
)
const testDSN = "root:muyu2026@tcp(127.0.0.1:13306)/muyu_wms?charset=utf8mb4&parseTime=true&loc=Asia%2FShanghai"
func newTestProductModel(t *testing.T) InvProductModel {
t.Helper()
conn := sqlx.NewMysql(testDSN)
cacheConf := cache.CacheConf{{RedisConf: redis.RedisConf{Host: "127.0.0.1:6379", Type: "node"}, Weight: 100}}
return NewInvProductModel(conn, cacheConf)
}
func uniqueID(prefix string) string {
return fmt.Sprintf("%s_%d", prefix, time.Now().UnixNano())
}
// TestInvProductInsert verifies that Insert succeeds against the live DB schema.
// This catches model/schema drift like the cost_price column removal.
func TestInvProductInsert(t *testing.T) {
if testing.Short() {
t.Skip("skipping integration test in short mode")
}
m := newTestProductModel(t)
ctx := context.Background()
p := &InvProduct{
ProductId: uniqueID("pid"),
ProductName: uniqueID("test_product"),
ImageUrl: "",
Spec: "100cm",
Color: "red",
SalesPrice: 99.9,
Remark: sql.NullString{String: "test remark", Valid: true},
Status: 1,
TenantId: "t_default_001",
}
result, err := m.Insert(ctx, p)
if err != nil {
t.Fatalf("Insert failed: %v", err)
}
id, _ := result.LastInsertId()
if id == 0 {
t.Fatal("expected non-zero LastInsertId")
}
t.Logf("inserted product id=%d", id)
// cleanup
t.Cleanup(func() { _ = m.Delete(ctx, id) })
}
// TestInvProductFindOne verifies Insert + FindOne round-trip.
func TestInvProductFindOne(t *testing.T) {
if testing.Short() {
t.Skip("skipping integration test in short mode")
}
m := newTestProductModel(t)
ctx := context.Background()
p := &InvProduct{
ProductId: uniqueID("pid"),
ProductName: uniqueID("test_product"),
Spec: "200cm",
Color: "blue",
SalesPrice: 49.5,
Status: 1,
TenantId: "t_default_001",
}
res, err := m.Insert(ctx, p)
if err != nil {
t.Fatalf("Insert: %v", err)
}
id, _ := res.LastInsertId()
t.Cleanup(func() { _ = m.Delete(ctx, id) })
got, err := m.FindOne(ctx, id)
if err != nil {
t.Fatalf("FindOne: %v", err)
}
if got.ProductName != p.ProductName {
t.Errorf("ProductName: got %q, want %q", got.ProductName, p.ProductName)
}
if got.SalesPrice != p.SalesPrice {
t.Errorf("SalesPrice: got %v, want %v", got.SalesPrice, p.SalesPrice)
}
}
// TestInvProductUpdate verifies Update writes back correctly.
func TestInvProductUpdate(t *testing.T) {
if testing.Short() {
t.Skip("skipping integration test in short mode")
}
m := newTestProductModel(t)
ctx := context.Background()
p := &InvProduct{
ProductId: uniqueID("pid"),
ProductName: uniqueID("test_product"),
Spec: "50cm",
Color: "green",
SalesPrice: 10.0,
Status: 1,
TenantId: "t_default_001",
}
res, err := m.Insert(ctx, p)
if err != nil {
t.Fatalf("Insert: %v", err)
}
id, _ := res.LastInsertId()
t.Cleanup(func() { _ = m.Delete(ctx, id) })
got, err := m.FindOne(ctx, id)
if err != nil {
t.Fatalf("FindOne: %v", err)
}
got.SalesPrice = 25.0
got.Remark = sql.NullString{String: "updated", Valid: true}
if err := m.Update(ctx, got); err != nil {
t.Fatalf("Update: %v", err)
}
updated, err := m.FindOne(ctx, id)
if err != nil {
t.Fatalf("FindOne after Update: %v", err)
}
if updated.SalesPrice != 25.0 {
t.Errorf("SalesPrice after update: got %v, want 25.0", updated.SalesPrice)
}
if updated.Remark.String != "updated" {
t.Errorf("Remark after update: got %q, want \"updated\"", updated.Remark.String)
}
}
// TestInvProductBulkInsert verifies BulkInsert (used by the import flow).
func TestInvProductBulkInsert(t *testing.T) {
if testing.Short() {
t.Skip("skipping integration test in short mode")
}
m := newTestProductModel(t)
ctx := context.Background()
ids := make([]int64, 0, 3)
products := make([]*InvProduct, 3)
for i := range products {
products[i] = &InvProduct{
ProductId: uniqueID(fmt.Sprintf("bulk_pid_%d", i)),
ProductName: uniqueID(fmt.Sprintf("bulk_product_%d", i)),
Spec: "10cm",
Color: "white",
SalesPrice: float64(i+1) * 5.0,
Status: 1,
TenantId: "t_default_001",
}
}
affected, err := m.BulkInsert(ctx, products)
if err != nil {
t.Fatalf("BulkInsert: %v", err)
}
if affected != 3 {
t.Errorf("BulkInsert: affected=%d, want 3", affected)
}
t.Cleanup(func() {
for _, p := range products {
row, err := m.FindOneByProductId(ctx, p.ProductId)
if err == nil {
_ = m.Delete(ctx, row.Id)
}
}
_ = ids
})
}
// TestInvProductSchemaColumns queries information_schema to assert the DB columns
// match what the InvProduct struct declares. Fails immediately on any mismatch.
func TestInvProductSchemaColumns(t *testing.T) {
if testing.Short() {
t.Skip("skipping integration test in short mode")
}
db, err := sql.Open("mysql", testDSN)
if err != nil {
t.Fatalf("open db: %v", err)
}
defer db.Close()
rows, err := db.QueryContext(context.Background(),
"SELECT COLUMN_NAME FROM information_schema.COLUMNS WHERE TABLE_SCHEMA=DATABASE() AND TABLE_NAME='inv_product' ORDER BY ORDINAL_POSITION")
if err != nil {
t.Fatalf("query schema: %v", err)
}
defer rows.Close()
dbCols := map[string]bool{}
for rows.Next() {
var col string
if err := rows.Scan(&col); err != nil {
t.Fatalf("scan: %v", err)
}
dbCols[col] = true
}
// Columns that the InvProduct struct declares (via db tags).
structCols := []string{
"id", "product_id", "product_name", "image_url", "spec", "color",
"sales_price", "remark", "status", "tenant_id", "created_at", "updated_at", "deleted_at",
}
for _, col := range structCols {
if !dbCols[col] {
t.Errorf("struct declares column %q but it does NOT exist in inv_product", col)
}
}
// Also check for DB columns not in the struct (forward-drift guard).
structSet := map[string]bool{}
for _, c := range structCols {
structSet[c] = true
}
for col := range dbCols {
if !structSet[col] {
t.Logf("WARNING: DB column %q exists but is not in the InvProduct struct", col)
}
}
}
// TestFindProductSummaryNoPans guards against the GROUP_CONCAT NULL scan bug:
// when a product has no pans, the LEFT JOIN produces NULL for pan.position and
// GROUP_CONCAT over all-NULL values returns NULL, which cannot be scanned into a
// Go string. The fix wraps both GROUP_CONCAT calls with COALESCE(..., '').
func TestFindProductSummaryNoPans(t *testing.T) {
if testing.Short() {
t.Skip("skipping integration test in short mode")
}
m := newTestProductModel(t)
ctx := context.Background()
p := &InvProduct{
ProductId: uniqueID("pid_nopan"),
ProductName: uniqueID("nopan_product"),
Spec: "50cm",
Color: "red",
SalesPrice: 99.0,
Status: 1,
TenantId: "t_default_001",
}
res, err := m.Insert(ctx, p)
if err != nil {
t.Fatalf("Insert: %v", err)
}
id, _ := res.LastInsertId()
t.Cleanup(func() { _ = m.Delete(ctx, id) })
list, err := m.FindProductSummary(ctx, "t_default_001")
if err != nil {
t.Fatalf("FindProductSummary returned error for product with no pans: %v", err)
}
var found *ProductSummary
for _, s := range list {
if s.ProductName == p.ProductName {
found = s
break
}
}
if found == nil {
t.Fatalf("product %q not found in summary (len=%d)", p.ProductName, len(list))
}
if found.ColorCount != 1 {
t.Errorf("ColorCount: got %d, want 1", found.ColorCount)
}
if found.TotalPanCount != 0 {
t.Errorf("TotalPanCount: got %d, want 0", found.TotalPanCount)
}
if found.Locations != "" {
t.Errorf("Locations: got %q, want empty string", found.Locations)
}
}
// TestFindColorDetailNoPans is the same NULL guard for the color-detail query.
func TestFindColorDetailNoPans(t *testing.T) {
if testing.Short() {
t.Skip("skipping integration test in short mode")
}
m := newTestProductModel(t)
ctx := context.Background()
p := &InvProduct{
ProductId: uniqueID("pid_color"),
ProductName: uniqueID("color_nopan_product"),
Spec: "80cm",
Color: "blue",
SalesPrice: 50.0,
Status: 1,
TenantId: "t_default_001",
}
res, err := m.Insert(ctx, p)
if err != nil {
t.Fatalf("Insert: %v", err)
}
id, _ := res.LastInsertId()
t.Cleanup(func() { _ = m.Delete(ctx, id) })
list, err := m.FindColorDetail(ctx, "t_default_001", p.ProductName)
if err != nil {
t.Fatalf("FindColorDetail returned error for product with no pans: %v", err)
}
if len(list) != 1 {
t.Fatalf("expected 1 color detail row, got %d", len(list))
}
if list[0].Color != "blue" {
t.Errorf("Color: got %q, want %q", list[0].Color, "blue")
}
if list[0].Locations != "" {
t.Errorf("Locations: got %q, want empty string", list[0].Locations)
}
}