fix cache issue in Count/Value functions for gdb.Model (#2300)

* add Tag* functions to retreive most commonly used tag value from struct field for package gstructs; use description tag as default value if brief is empty for gcmd.Argument

* fix cache issue in Count/Value functions for gdb.Model

* add more ut case for package gdb

* version updates

contrib/drivers/mysql/mysql_model_test.go

@@ -731,16 +731,157 @@ func Test_Model_Count(t *testing.T) {
 		t.AssertNil(err)
 		t.Assert(count, TableSize)
 	})
-	// gtest.C(t, func(t *gtest.T) {
-	//	count, err := db.Model(table).Fields("id myid").Where("id>8").Count()
-	//	t.AssertNil(err)
-	//	t.Assert(count, 2)
-	// })
-	// gtest.C(t, func(t *gtest.T) {
-	//	count, err := db.Model(table).As("u1").LeftJoin(table, "u2", "u2.id=u1.id").Fields("u2.id u2id").Where("u1.id>8").Count()
-	//	t.AssertNil(err)
-	//	t.Assert(count, 2)
-	// })
+}
+
+func Test_Model_Value_WithCache(t *testing.T) {
+	table := createTable()
+	defer dropTable(table)
+
+	gtest.C(t, func(t *gtest.T) {
+		value, err := db.Model(table).Where("id", 1).Cache(gdb.CacheOption{
+			Duration: time.Second * 10,
+			Force:    false,
+		}).Value()
+		t.AssertNil(err)
+		t.Assert(value.Int(), 0)
+	})
+	gtest.C(t, func(t *gtest.T) {
+		result, err := db.Model(table).Data(g.MapStrAny{
+			"id":       1,
+			"passport": fmt.Sprintf(`passport_%d`, 1),
+			"password": fmt.Sprintf(`password_%d`, 1),
+			"nickname": fmt.Sprintf(`nickname_%d`, 1),
+		}).Insert()
+		t.AssertNil(err)
+		n, _ := result.RowsAffected()
+		t.Assert(n, 1)
+	})
+	gtest.C(t, func(t *gtest.T) {
+		value, err := db.Model(table).Where("id", 1).Cache(gdb.CacheOption{
+			Duration: time.Second * 10,
+			Force:    false,
+		}).Value()
+		t.AssertNil(err)
+		t.Assert(value.Int(), 1)
+	})
+}
+
+func Test_Model_Count_WithCache(t *testing.T) {
+	table := createTable()
+	defer dropTable(table)
+
+	gtest.C(t, func(t *gtest.T) {
+		count, err := db.Model(table).Where("id", 1).Cache(gdb.CacheOption{
+			Duration: time.Second * 10,
+			Force:    false,
+		}).Count()
+		t.AssertNil(err)
+		t.Assert(count, 0)
+	})
+	gtest.C(t, func(t *gtest.T) {
+		result, err := db.Model(table).Data(g.MapStrAny{
+			"id":       1,
+			"passport": fmt.Sprintf(`passport_%d`, 1),
+			"password": fmt.Sprintf(`password_%d`, 1),
+			"nickname": fmt.Sprintf(`nickname_%d`, 1),
+		}).Insert()
+		t.AssertNil(err)
+		n, _ := result.RowsAffected()
+		t.Assert(n, 1)
+	})
+	gtest.C(t, func(t *gtest.T) {
+		count, err := db.Model(table).Where("id", 1).Cache(gdb.CacheOption{
+			Duration: time.Second * 10,
+			Force:    false,
+		}).Count()
+		t.AssertNil(err)
+		t.Assert(count, 1)
+	})
+}
+
+func Test_Model_Count_All_WithCache(t *testing.T) {
+	table := createTable()
+	defer dropTable(table)
+
+	gtest.C(t, func(t *gtest.T) {
+		count, err := db.Model(table).Cache(gdb.CacheOption{
+			Duration: time.Second * 10,
+			Force:    false,
+		}).Count()
+		t.AssertNil(err)
+		t.Assert(count, 0)
+	})
+	gtest.C(t, func(t *gtest.T) {
+		result, err := db.Model(table).Data(g.MapStrAny{
+			"id":       1,
+			"passport": fmt.Sprintf(`passport_%d`, 1),
+			"password": fmt.Sprintf(`password_%d`, 1),
+			"nickname": fmt.Sprintf(`nickname_%d`, 1),
+		}).Insert()
+		t.AssertNil(err)
+		n, _ := result.RowsAffected()
+		t.Assert(n, 1)
+	})
+	gtest.C(t, func(t *gtest.T) {
+		count, err := db.Model(table).Cache(gdb.CacheOption{
+			Duration: time.Second * 10,
+			Force:    false,
+		}).Count()
+		t.AssertNil(err)
+		t.Assert(count, 1)
+	})
+	gtest.C(t, func(t *gtest.T) {
+		result, err := db.Model(table).Data(g.MapStrAny{
+			"id":       2,
+			"passport": fmt.Sprintf(`passport_%d`, 2),
+			"password": fmt.Sprintf(`password_%d`, 2),
+			"nickname": fmt.Sprintf(`nickname_%d`, 2),
+		}).Insert()
+		t.AssertNil(err)
+		n, _ := result.RowsAffected()
+		t.Assert(n, 1)
+	})
+	gtest.C(t, func(t *gtest.T) {
+		count, err := db.Model(table).Cache(gdb.CacheOption{
+			Duration: time.Second * 10,
+			Force:    false,
+		}).Count()
+		t.AssertNil(err)
+		t.Assert(count, 1)
+	})
+}
+
+func Test_Model_CountColumn_WithCache(t *testing.T) {
+	table := createTable()
+	defer dropTable(table)
+
+	gtest.C(t, func(t *gtest.T) {
+		count, err := db.Model(table).Where("id", 1).Cache(gdb.CacheOption{
+			Duration: time.Second * 10,
+			Force:    false,
+		}).CountColumn("id")
+		t.AssertNil(err)
+		t.Assert(count, 0)
+	})
+	gtest.C(t, func(t *gtest.T) {
+		result, err := db.Model(table).Data(g.MapStrAny{
+			"id":       1,
+			"passport": fmt.Sprintf(`passport_%d`, 1),
+			"password": fmt.Sprintf(`password_%d`, 1),
+			"nickname": fmt.Sprintf(`nickname_%d`, 1),
+		}).Insert()
+		t.AssertNil(err)
+		n, _ := result.RowsAffected()
+		t.Assert(n, 1)
+	})
+	gtest.C(t, func(t *gtest.T) {
+		count, err := db.Model(table).Where("id", 1).Cache(gdb.CacheOption{
+			Duration: time.Second * 10,
+			Force:    false,
+		}).CountColumn("id")
+		t.AssertNil(err)
+		t.Assert(count, 1)
+	})
 }
 
 func Test_Model_Select(t *testing.T) {

database/gdb/gdb.go

@@ -289,20 +289,23 @@ type CatchSQLManager struct {
 	DoCommit bool
 }
 
+type queryType int
+
 const (
-	defaultModelSafe        = false
-	defaultCharset          = `utf8`
-	defaultProtocol         = `tcp`
-	queryTypeNormal         = 0
-	queryTypeCount          = 1
-	unionTypeNormal         = 0
-	unionTypeAll            = 1
-	defaultMaxIdleConnCount = 10               // Max idle connection count in pool.
-	defaultMaxOpenConnCount = 0                // Max open connection count in pool. Default is no limit.
-	defaultMaxConnLifeTime  = 30 * time.Second // Max lifetime for per connection in pool in seconds.
-	ctxTimeoutTypeExec      = iota
-	ctxTimeoutTypeQuery
-	ctxTimeoutTypePrepare
+	defaultModelSafe                      = false
+	defaultCharset                        = `utf8`
+	defaultProtocol                       = `tcp`
+	queryTypeNormal           queryType   = 0
+	queryTypeCount            queryType   = 1
+	queryTypeValue            queryType   = 2
+	unionTypeNormal                       = 0
+	unionTypeAll                          = 1
+	defaultMaxIdleConnCount               = 10               // Max idle connection count in pool.
+	defaultMaxOpenConnCount               = 0                // Max open connection count in pool. Default is no limit.
+	defaultMaxConnLifeTime                = 30 * time.Second // Max lifetime for per connection in pool in seconds.
+	ctxTimeoutTypeExec                    = 0
+	ctxTimeoutTypeQuery                   = 1
+	ctxTimeoutTypePrepare                 = 2
 	cachePrefixTableFields                = `TableFields:`
 	cachePrefixSelectCache                = `SelectCache:`
 	commandEnvKeyForDryRun                = "gf.gdb.dryrun"

database/gdb/gdb_core_underlying.go

@@ -397,7 +397,7 @@ func (c *Core) RowsToResult(ctx context.Context, rows *sql.Rows) (Result, error)
 		record := Record{}
 		for i, value := range values {
 			if value == nil {
-				// Do not use `gvar.New(nil)` here as it creates an initialized object
+				// DO NOT use `gvar.New(nil)` here as it creates an initialized object
 				// which will cause struct converting issue.
 				record[columnNames[i]] = nil
 			} else {

database/gdb/gdb_model_cache.go

@@ -85,18 +85,19 @@ func (m *Model) getSelectResultFromCache(ctx context.Context, sql string, args .
 		if cacheItem, ok = v.Val().(*selectCacheItem); ok {
 			// In-memory cache.
 			return cacheItem.Result, nil
-		} else {
-			// Other cache, it needs conversion.
-			if err = json.UnmarshalUseNumber(v.Bytes(), &cacheItem); err != nil {
-				return nil, err
-			}
-			return cacheItem.Result, nil
 		}
+		// Other cache, it needs conversion.
+		if err = json.UnmarshalUseNumber(v.Bytes(), &cacheItem); err != nil {
+			return nil, err
+		}
+		return cacheItem.Result, nil
 	}
 	return
 }
 
-func (m *Model) saveSelectResultToCache(ctx context.Context, result Result, sql string, args ...interface{}) (err error) {
+func (m *Model) saveSelectResultToCache(
+	ctx context.Context, queryType queryType, result Result, sql string, args ...interface{},
+) (err error) {
 	if !m.cacheEnabled || m.tx != nil {
 		return
 	}
@@ -108,22 +109,38 @@ func (m *Model) saveSelectResultToCache(ctx context.Context, result Result, sql
 		if _, errCache := cacheObj.Remove(ctx, cacheKey); errCache != nil {
 			intlog.Errorf(ctx, `%+v`, errCache)
 		}
-	} else {
-		// In case of Cache Penetration.
-		if result.IsEmpty() && m.cacheOption.Force {
-			result = Result{}
-		}
-		var cacheItem = &selectCacheItem{
-			Result: result,
-		}
-		if internalData := m.db.GetCore().GetInternalCtxDataFromCtx(ctx); internalData != nil {
-			cacheItem.FirstResultColumn = internalData.FirstResultColumn
-		}
-		if errCache := cacheObj.Set(ctx, cacheKey, cacheItem, m.cacheOption.Duration); errCache != nil {
-			intlog.Errorf(ctx, `%+v`, errCache)
+		return
+	}
+	// Special handler for Value/Count operations result.
+	if len(result) > 0 {
+		switch queryType {
+		case queryTypeValue, queryTypeCount:
+			if internalData := m.db.GetCore().GetInternalCtxDataFromCtx(ctx); internalData != nil {
+				if result[0][internalData.FirstResultColumn].IsEmpty() {
+					result = nil
+				}
+			}
 		}
 	}
-	return nil
+
+	// In case of Cache Penetration.
+	if result.IsEmpty() {
+		if m.cacheOption.Force {
+			result = Result{}
+		} else {
+			result = nil
+		}
+	}
+	var cacheItem = &selectCacheItem{
+		Result: result,
+	}
+	if internalData := m.db.GetCore().GetInternalCtxDataFromCtx(ctx); internalData != nil {
+		cacheItem.FirstResultColumn = internalData.FirstResultColumn
+	}
+	if errCache := cacheObj.Set(ctx, cacheKey, cacheItem, m.cacheOption.Duration); errCache != nil {
+		intlog.Errorf(ctx, `%+v`, errCache)
+	}
+	return
 }
 
 func (m *Model) makeSelectCacheKey(sql string, args ...interface{}) string {

database/gdb/gdb_model_select.go

@@ -12,7 +12,6 @@ import (
 	"reflect"
 
 	"github.com/gogf/gf/v2/container/gset"
-	"github.com/gogf/gf/v2/container/gvar"
 	"github.com/gogf/gf/v2/errors/gcode"
 	"github.com/gogf/gf/v2/errors/gerror"
 	"github.com/gogf/gf/v2/internal/reflection"
@@ -31,75 +30,6 @@ func (m *Model) All(where ...interface{}) (Result, error) {
 	return m.doGetAll(ctx, false, where...)
 }
 
-// doGetAll does "SELECT FROM ..." statement for the model.
-// It retrieves the records from table and returns the result as slice type.
-// It returns nil if there's no record retrieved with the given conditions from table.
-//
-// The parameter `limit1` specifies whether limits querying only one record if m.limit is not set.
-// The optional parameter `where` is the same as the parameter of Model.Where function,
-// see Model.Where.
-func (m *Model) doGetAll(ctx context.Context, limit1 bool, where ...interface{}) (Result, error) {
-	if len(where) > 0 {
-		return m.Where(where[0], where[1:]...).All()
-	}
-	sqlWithHolder, holderArgs := m.getFormattedSqlAndArgs(ctx, queryTypeNormal, limit1)
-	return m.doGetAllBySql(ctx, queryTypeNormal, sqlWithHolder, holderArgs...)
-}
-
-// getFieldsFiltered checks the fields and fieldsEx attributes, filters and returns the fields that will
-// really be committed to underlying database driver.
-func (m *Model) getFieldsFiltered() string {
-	if m.fieldsEx == "" {
-		// No filtering, containing special chars.
-		if gstr.ContainsAny(m.fields, "()") {
-			return m.fields
-		}
-		// No filtering.
-		if !gstr.ContainsAny(m.fields, ". ") {
-			return m.db.GetCore().QuoteString(m.fields)
-		}
-		return m.fields
-	}
-	var (
-		fieldsArray []string
-		fieldsExSet = gset.NewStrSetFrom(gstr.SplitAndTrim(m.fieldsEx, ","))
-	)
-	if m.fields != "*" {
-		// Filter custom fields with fieldEx.
-		fieldsArray = make([]string, 0, 8)
-		for _, v := range gstr.SplitAndTrim(m.fields, ",") {
-			fieldsArray = append(fieldsArray, v[gstr.PosR(v, "-")+1:])
-		}
-	} else {
-		if gstr.Contains(m.tables, " ") {
-			panic("function FieldsEx supports only single table operations")
-		}
-		// Filter table fields with fieldEx.
-		tableFields, err := m.TableFields(m.tablesInit)
-		if err != nil {
-			panic(err)
-		}
-		if len(tableFields) == 0 {
-			panic(fmt.Sprintf(`empty table fields for table "%s"`, m.tables))
-		}
-		fieldsArray = make([]string, len(tableFields))
-		for k, v := range tableFields {
-			fieldsArray[v.Index] = k
-		}
-	}
-	newFields := ""
-	for _, k := range fieldsArray {
-		if fieldsExSet.Contains(k) {
-			continue
-		}
-		if len(newFields) > 0 {
-			newFields += ","
-		}
-		newFields += m.db.GetCore().QuoteWord(k)
-	}
-	return newFields
-}
-
 // Chunk iterates the query result with given `size` and `handler` function.
 func (m *Model) Chunk(size int, handler ChunkHandler) {
 	page := m.start
@@ -147,45 +77,6 @@ func (m *Model) One(where ...interface{}) (Record, error) {
 	return nil, nil
 }
 
-// Value retrieves a specified record value from table and returns the result as interface type.
-// It returns nil if there's no record found with the given conditions from table.
-//
-// If the optional parameter `fieldsAndWhere` is given, the fieldsAndWhere[0] is the selected fields
-// and fieldsAndWhere[1:] is treated as where condition fields.
-// Also see Model.Fields and Model.Where functions.
-func (m *Model) Value(fieldsAndWhere ...interface{}) (Value, error) {
-	var ctx = m.GetCtx()
-	if len(fieldsAndWhere) > 0 {
-		if len(fieldsAndWhere) > 2 {
-			return m.Fields(gconv.String(fieldsAndWhere[0])).Where(fieldsAndWhere[1], fieldsAndWhere[2:]...).Value()
-		} else if len(fieldsAndWhere) == 2 {
-			return m.Fields(gconv.String(fieldsAndWhere[0])).Where(fieldsAndWhere[1]).Value()
-		} else {
-			return m.Fields(gconv.String(fieldsAndWhere[0])).Value()
-		}
-	}
-	var (
-		all Result
-		err error
-	)
-	if all, err = m.doGetAll(ctx, true); err != nil {
-		return nil, err
-	}
-	if len(all) == 0 {
-		return gvar.New(nil), nil
-	}
-	if internalData := m.db.GetCore().GetInternalCtxDataFromCtx(ctx); internalData != nil {
-		record := all[0]
-		if v, ok := record[internalData.FirstResultColumn]; ok {
-			return v, nil
-		}
-	}
-	return nil, gerror.NewCode(
-		gcode.CodeInternalError,
-		`query value error: the internal context data is missing. there's internal issue should be fixed`,
-	)
-}
-
 // Array queries and returns data values as slice from database.
 // Note that if there are multiple columns in the result, it returns just one column values randomly.
 //
@@ -375,6 +266,45 @@ func (m *Model) ScanList(structSlicePointer interface{}, bindToAttrName string,
 	})
 }
 
+// Value retrieves a specified record value from table and returns the result as interface type.
+// It returns nil if there's no record found with the given conditions from table.
+//
+// If the optional parameter `fieldsAndWhere` is given, the fieldsAndWhere[0] is the selected fields
+// and fieldsAndWhere[1:] is treated as where condition fields.
+// Also see Model.Fields and Model.Where functions.
+func (m *Model) Value(fieldsAndWhere ...interface{}) (Value, error) {
+	var ctx = m.GetCtx()
+	if len(fieldsAndWhere) > 0 {
+		if len(fieldsAndWhere) > 2 {
+			return m.Fields(gconv.String(fieldsAndWhere[0])).Where(fieldsAndWhere[1], fieldsAndWhere[2:]...).Value()
+		} else if len(fieldsAndWhere) == 2 {
+			return m.Fields(gconv.String(fieldsAndWhere[0])).Where(fieldsAndWhere[1]).Value()
+		} else {
+			return m.Fields(gconv.String(fieldsAndWhere[0])).Value()
+		}
+	}
+	var (
+		sqlWithHolder, holderArgs = m.getFormattedSqlAndArgs(ctx, queryTypeValue, true)
+		all, err                  = m.doGetAllBySql(ctx, queryTypeValue, sqlWithHolder, holderArgs...)
+	)
+	if err != nil {
+		return nil, err
+	}
+	if len(all) > 0 {
+		if internalData := m.db.GetCore().GetInternalCtxDataFromCtx(ctx); internalData != nil {
+			record := all[0]
+			if v, ok := record[internalData.FirstResultColumn]; ok {
+				return v, nil
+			}
+		}
+		return nil, gerror.NewCode(
+			gcode.CodeInternalError,
+			`query value error: the internal context data is missing. there's internal issue should be fixed`,
+		)
+	}
+	return nil, nil
+}
+
 // Count does "SELECT COUNT(x) FROM ..." statement for the model.
 // The optional parameter `where` is the same as the parameter of Model.Where function,
 // see Model.Where.
@@ -526,8 +456,23 @@ func (m *Model) Having(having interface{}, args ...interface{}) *Model {
 	return model
 }
 
+// doGetAll does "SELECT FROM ..." statement for the model.
+// It retrieves the records from table and returns the result as slice type.
+// It returns nil if there's no record retrieved with the given conditions from table.
+//
+// The parameter `limit1` specifies whether limits querying only one record if m.limit is not set.
+// The optional parameter `where` is the same as the parameter of Model.Where function,
+// see Model.Where.
+func (m *Model) doGetAll(ctx context.Context, limit1 bool, where ...interface{}) (Result, error) {
+	if len(where) > 0 {
+		return m.Where(where[0], where[1:]...).All()
+	}
+	sqlWithHolder, holderArgs := m.getFormattedSqlAndArgs(ctx, queryTypeNormal, limit1)
+	return m.doGetAllBySql(ctx, queryTypeNormal, sqlWithHolder, holderArgs...)
+}
+
 // doGetAllBySql does the select statement on the database.
-func (m *Model) doGetAllBySql(ctx context.Context, queryType int, sql string, args ...interface{}) (result Result, err error) {
+func (m *Model) doGetAllBySql(ctx context.Context, queryType queryType, sql string, args ...interface{}) (result Result, err error) {
 	if result, err = m.getSelectResultFromCache(ctx, sql, args...); err != nil || result != nil {
 		return
 	}
@@ -548,11 +493,11 @@ func (m *Model) doGetAllBySql(ctx context.Context, queryType int, sql string, ar
 		return
 	}
 
-	err = m.saveSelectResultToCache(ctx, result, sql, args...)
+	err = m.saveSelectResultToCache(ctx, queryType, result, sql, args...)
 	return
 }
 
-func (m *Model) getFormattedSqlAndArgs(ctx context.Context, queryType int, limit1 bool) (sqlWithHolder string, holderArgs []interface{}) {
+func (m *Model) getFormattedSqlAndArgs(ctx context.Context, queryType queryType, limit1 bool) (sqlWithHolder string, holderArgs []interface{}) {
 	switch queryType {
 	case queryTypeCount:
 		queryFields := "COUNT(1)"
@@ -604,6 +549,60 @@ func (m *Model) getAutoPrefix() string {
 	return autoPrefix
 }
 
+// getFieldsFiltered checks the fields and fieldsEx attributes, filters and returns the fields that will
+// really be committed to underlying database driver.
+func (m *Model) getFieldsFiltered() string {
+	if m.fieldsEx == "" {
+		// No filtering, containing special chars.
+		if gstr.ContainsAny(m.fields, "()") {
+			return m.fields
+		}
+		// No filtering.
+		if !gstr.ContainsAny(m.fields, ". ") {
+			return m.db.GetCore().QuoteString(m.fields)
+		}
+		return m.fields
+	}
+	var (
+		fieldsArray []string
+		fieldsExSet = gset.NewStrSetFrom(gstr.SplitAndTrim(m.fieldsEx, ","))
+	)
+	if m.fields != "*" {
+		// Filter custom fields with fieldEx.
+		fieldsArray = make([]string, 0, 8)
+		for _, v := range gstr.SplitAndTrim(m.fields, ",") {
+			fieldsArray = append(fieldsArray, v[gstr.PosR(v, "-")+1:])
+		}
+	} else {
+		if gstr.Contains(m.tables, " ") {
+			panic("function FieldsEx supports only single table operations")
+		}
+		// Filter table fields with fieldEx.
+		tableFields, err := m.TableFields(m.tablesInit)
+		if err != nil {
+			panic(err)
+		}
+		if len(tableFields) == 0 {
+			panic(fmt.Sprintf(`empty table fields for table "%s"`, m.tables))
+		}
+		fieldsArray = make([]string, len(tableFields))
+		for k, v := range tableFields {
+			fieldsArray[v.Index] = k
+		}
+	}
+	newFields := ""
+	for _, k := range fieldsArray {
+		if fieldsExSet.Contains(k) {
+			continue
+		}
+		if len(newFields) > 0 {
+			newFields += ","
+		}
+		newFields += m.db.GetCore().QuoteWord(k)
+	}
+	return newFields
+}
+
 // formatCondition formats where arguments of the model and returns a new condition sql and its arguments.
 // Note that this function does not change any attribute value of the `m`.
 //

database/gdb/gdb_type_result.go

@@ -16,7 +16,7 @@ import (
 
 // IsEmpty checks and returns whether `r` is empty.
 func (r Result) IsEmpty() bool {
-	return r.Len() == 0
+	return r == nil || r.Len() == 0
 }
 
 // Len returns the length of result list.

os/gcmd/gcmd_command_object.go

@@ -159,6 +159,10 @@ func newCommandFromObjectMeta(object interface{}, name string) (command *Command
 	if command.Description == "" {
 		command.Description = metaData[gtag.DescriptionShort]
 	}
+	if command.Brief == "" && command.Description != "" {
+		command.Brief = command.Description
+		command.Description = ""
+	}
 	if command.Examples == "" {
 		command.Examples = metaData[gtag.ExampleShort]
 	}

version.go

@@ -2,5 +2,5 @@ package gf
 
 const (
 	// VERSION  is the current GoFrame version.
-	VERSION = "v2.2.3"
+	VERSION = "v2.2.4"
 )