Improve code (#1552)

Change filters

Fix test

add more tests on drone

add more tests on drone

Improve code

Reviewed-on: https://gitea.com/xorm/xorm/pulls/1552

.drone.yml

@@ -22,6 +22,8 @@ steps:
   commands:
   - make test-sqlite
   - TEST_CACHE_ENABLE=true make test-sqlite
+  - go test ./caches/... ./core/... ./dialects/... ./internal/... \
+    ./log/... ./migrate/... ./names/... ./schemas/...
   when:
     event:
     - push

core/db_test.go

@@ -11,14 +11,15 @@ import (
 	"testing"
 	"time"
 
+	"xorm.io/xorm/names"
+
 	_ "github.com/go-sql-driver/mysql"
 	_ "github.com/mattn/go-sqlite3"
-	"xorm.io/xorm/names"
 )
 
 var (
-	dbtype         = flag.String("dbtype", "mysql", "database type")
+	dbtype         = flag.String("dbtype", "sqlite3", "database type")
-	dbConn         = flag.String("dbConn", "root:@/core_test?charset=utf8", "database connect string")
+	dbConn         = flag.String("dbConn", "./db_test.db", "database connect string")
 	createTableSql string
 )
 

dialects/filter.go

@@ -13,20 +13,20 @@ import (
 
 // Filter is an interface to filter SQL
 type Filter interface {
-	Do(sql string, dialect Dialect, table *schemas.Table) string
+	Do(sql string) string
 }
 
 // QuoteFilter filter SQL replace ` to database's own quote character
 type QuoteFilter struct {
+	quoter schemas.Quoter
 }
 
-func (s *QuoteFilter) Do(sql string, dialect Dialect, table *schemas.Table) string {
+func (s *QuoteFilter) Do(sql string) string {
-	quoter := dialect.Quoter()
+	if s.quoter.IsEmpty() {
-	if quoter.IsEmpty() {
 		return sql
 	}
 
-	prefix, suffix := quoter[0][0], quoter[1][0]
+	prefix, suffix := s.quoter[0][0], s.quoter[1][0]
 	raw := []byte(sql)
 	for i, cnt := 0, 0; i < len(raw); i = i + 1 {
 		if raw[i] == '`' {
@@ -66,6 +66,6 @@ func convertQuestionMark(sql, prefix string, start int) string {
 	return buf.String()
 }
 
-func (s *SeqFilter) Do(sql string, dialect Dialect, table *schemas.Table) string {
+func (s *SeqFilter) Do(sql string) string {
 	return convertQuestionMark(sql, s.Prefix, s.Start)
 }

dialects/filter_test.go

@@ -3,21 +3,15 @@ package dialects
 import (
 	"testing"
 
+	"xorm.io/xorm/schemas"
+
 	"github.com/stretchr/testify/assert"
 )
 
-type quoterOnly struct {
-	Dialect
-}
-
-func (q *quoterOnly) Quote(item string) string {
-	return "[" + item + "]"
-}
-
 func TestQuoteFilter_Do(t *testing.T) {
-	f := QuoteFilter{}
+	f := QuoteFilter{schemas.Quoter{"[", "]"}}
 	sql := "SELECT `COLUMN_NAME` FROM `INFORMATION_SCHEMA`.`COLUMNS` WHERE `TABLE_SCHEMA` = ? AND `TABLE_NAME` = ? AND `COLUMN_NAME` = ?"
-	res := f.Do(sql, new(quoterOnly), nil)
+	res := f.Do(sql)
 	assert.EqualValues(t,
 		"SELECT [COLUMN_NAME] FROM [INFORMATION_SCHEMA].[COLUMNS] WHERE [TABLE_SCHEMA] = ? AND [TABLE_NAME] = ? AND [COLUMN_NAME] = ?",
 		res,

dialects/mssql.go

@@ -534,7 +534,7 @@ func (db *mssql) ForUpdateSQL(query string) string {
 }
 
 func (db *mssql) Filters() []Filter {
-	return []Filter{&QuoteFilter{}}
+	return []Filter{&QuoteFilter{db.Quoter()}}
 }
 
 type odbcDriver struct {

dialects/oracle.go

@@ -848,7 +848,10 @@ func (db *oracle) GetIndexes(tableName string) (map[string]*schemas.Index, error
 }
 
 func (db *oracle) Filters() []Filter {
-	return []Filter{&QuoteFilter{}, &SeqFilter{Prefix: ":", Start: 1}}
+	return []Filter{
+		&QuoteFilter{db.Quoter()},
+		&SeqFilter{Prefix: ":", Start: 1},
+	}
 }
 
 type goracleDriver struct {

dialects/postgres.go

@@ -1159,7 +1159,7 @@ func (db *postgres) GetIndexes(tableName string) (map[string]*schemas.Index, err
 }
 
 func (db *postgres) Filters() []Filter {
-	return []Filter{&QuoteFilter{}, &SeqFilter{Prefix: "$", Start: 1}}
+	return []Filter{&QuoteFilter{db.Quoter()}, &SeqFilter{Prefix: "$", Start: 1}}
 }
 
 type pqDriver struct {

engine.go

@@ -24,6 +24,7 @@ import (
 	"xorm.io/xorm/caches"
 	"xorm.io/xorm/core"
 	"xorm.io/xorm/dialects"
+	"xorm.io/xorm/internal/utils"
 	"xorm.io/xorm/log"
 	"xorm.io/xorm/names"
 	"xorm.io/xorm/schemas"
@@ -80,7 +81,7 @@ func (engine *Engine) CondDeleted(col *schemas.Column) builder.Cond {
 	} else {
 		// FIXME: mssql: The conversion of a nvarchar data type to a datetime data type resulted in an out-of-range value.
 		if engine.dialect.DBType() != schemas.MSSQL {
-			cond = builder.Eq{col.Name: zeroTime1}
+			cond = builder.Eq{col.Name: utils.ZeroTime1}
 		}
 	}
 

helpers.go

@@ -11,6 +11,7 @@ import (
 	"sort"
 	"strconv"
 	"strings"
+	"time"
 )
 
 // str2PK convert string value to primary key value according to tp
@@ -200,3 +201,7 @@ func sliceEq(left, right []string) bool {
 func indexName(tableName, idxName string) string {
 	return fmt.Sprintf("IDX_%v_%v", tableName, idxName)
 }
+
+func formatTime(t time.Time) string {
+	return t.Format("2006-01-02 15:04:05")
+}

helpler_time.go

@@ -1,21 +0,0 @@
-// Copyright 2017 The Xorm Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-package xorm
-
-import "time"
-
-const (
-	zeroTime0 = "0000-00-00 00:00:00"
-	zeroTime1 = "0001-01-01 00:00:00"
-)
-
-func formatTime(t time.Time) string {
-	return t.Format("2006-01-02 15:04:05")
-}
-
-func isTimeZero(t time.Time) bool {
-	return t.IsZero() || formatTime(t) == zeroTime0 ||
-		formatTime(t) == zeroTime1
-}

interface.go

@@ -7,6 +7,7 @@ package xorm
 import (
 	"context"
 	"database/sql"
+	"encoding/json"
 	"reflect"
 	"time"
 
@@ -122,3 +123,27 @@ var (
 	_ EngineInterface = &Engine{}
 	_ EngineInterface = &EngineGroup{}
 )
+
+// JSONInterface represents an interface to handle json data
+type JSONInterface interface {
+	Marshal(v interface{}) ([]byte, error)
+	Unmarshal(data []byte, v interface{}) error
+}
+
+var (
+	// DefaultJSONHandler default json handler
+	DefaultJSONHandler JSONInterface = StdJSON{}
+)
+
+// StdJSON implements JSONInterface via encoding/json
+type StdJSON struct{}
+
+// Marshal implements JSONInterface
+func (StdJSON) Marshal(v interface{}) ([]byte, error) {
+	return json.Marshal(v)
+}
+
+// Unmarshal implements JSONInterface
+func (StdJSON) Unmarshal(data []byte, v interface{}) error {
+	return json.Unmarshal(data, v)
+}

internal/utils/zero.go

@@ -6,6 +6,7 @@ package utils
 
 import (
 	"reflect"
+	"time"
 )
 
 type Zeroable interface {
@@ -96,3 +97,13 @@ func IsArrayZero(v reflect.Value) bool {
 
 	return true
 }
+
+const (
+	ZeroTime0 = "0000-00-00 00:00:00"
+	ZeroTime1 = "0001-01-01 00:00:00"
+)
+
+func IsTimeZero(t time.Time) bool {
+	return t.IsZero() || t.Format("2006-01-02 15:04:05") == ZeroTime0 ||
+		t.Format("2006-01-02 15:04:05") == ZeroTime1
+}

json.go

@@ -1,31 +0,0 @@
-// Copyright 2019 The Xorm Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-package xorm
-
-import "encoding/json"
-
-// JSONInterface represents an interface to handle json data
-type JSONInterface interface {
-	Marshal(v interface{}) ([]byte, error)
-	Unmarshal(data []byte, v interface{}) error
-}
-
-var (
-	// DefaultJSONHandler default json handler
-	DefaultJSONHandler JSONInterface = StdJSON{}
-)
-
-// StdJSON implements JSONInterface via encoding/json
-type StdJSON struct{}
-
-// Marshal implements JSONInterface
-func (StdJSON) Marshal(v interface{}) ([]byte, error) {
-	return json.Marshal(v)
-}
-
-// Unmarshal implements JSONInterface
-func (StdJSON) Unmarshal(data []byte, v interface{}) error {
-	return json.Unmarshal(data, v)
-}

session_convert.go

@@ -14,6 +14,7 @@ import (
 	"strings"
 	"time"
 
+	"xorm.io/xorm/internal/utils"
 	"xorm.io/xorm/schemas"
 )
 
@@ -27,7 +28,7 @@ func (session *Session) str2Time(col *schemas.Column, data string) (outTime time
 		parseLoc = col.TimeZone
 	}
 
-	if sdata == zeroTime0 || sdata == zeroTime1 {
+	if sdata == utils.ZeroTime0 || sdata == utils.ZeroTime1 {
 	} else if !strings.ContainsAny(sdata, "- :") { // !nashtsai! has only found that mymysql driver is using this for time type column
 		// time stamp
 		sd, err := strconv.ParseInt(sdata, 10, 64)

session_delete.go

@@ -20,7 +20,7 @@ func (session *Session) cacheDelete(table *schemas.Table, tableName, sqlStr stri
 	}
 
 	for _, filter := range session.engine.dialect.Filters() {
-		sqlStr = filter.Do(sqlStr, session.engine.dialect, table)
+		sqlStr = filter.Do(sqlStr)
 	}
 
 	newsql := session.statement.convertIDSQL(sqlStr)

session_find.go

@@ -335,7 +335,7 @@ func (session *Session) cacheFind(t reflect.Type, sqlStr string, rowsSlicePtr in
 	}
 
 	for _, filter := range session.engine.dialect.Filters() {
-		sqlStr = filter.Do(sqlStr, session.engine.dialect, session.statement.RefTable)
+		sqlStr = filter.Do(sqlStr)
 	}
 
 	newsql := session.statement.convertIDSQL(sqlStr)

session_get.go

@@ -272,7 +272,7 @@ func (session *Session) cacheGet(bean interface{}, sqlStr string, args ...interf
 	}
 
 	for _, filter := range session.engine.dialect.Filters() {
-		sqlStr = filter.Do(sqlStr, session.engine.dialect, session.statement.RefTable)
+		sqlStr = filter.Do(sqlStr)
 	}
 	newsql := session.statement.convertIDSQL(sqlStr)
 	if newsql == "" {

session_raw.go

@@ -15,7 +15,7 @@ import (
 
 func (session *Session) queryPreprocess(sqlStr *string, paramStr ...interface{}) {
 	for _, filter := range session.engine.dialect.Filters() {
-		*sqlStr = filter.Do(*sqlStr, session.engine.dialect, session.statement.RefTable)
+		*sqlStr = filter.Do(*sqlStr)
 	}
 
 	session.lastSQL = *sqlStr

session_update.go

@@ -28,7 +28,7 @@ func (session *Session) cacheUpdate(table *schemas.Table, tableName, sqlStr stri
 		return ErrCacheFailed
 	}
 	for _, filter := range session.engine.dialect.Filters() {
-		newsql = filter.Do(newsql, session.engine.dialect, table)
+		newsql = filter.Do(newsql)
 	}
 	session.engine.logger.Debug("[cacheUpdate] new sql", oldhead, newsql)
 

time_test.go

@@ -11,6 +11,7 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/assert"
+	"xorm.io/xorm/internal/utils"
 )
 
 func TestTimeUserTime(t *testing.T) {
@@ -282,7 +283,7 @@ func TestTimeUserDeleted(t *testing.T) {
 	assert.EqualValues(t, formatTime(user.CreatedAt), formatTime(user2.CreatedAt))
 	assert.EqualValues(t, user.UpdatedAt.Unix(), user2.UpdatedAt.Unix())
 	assert.EqualValues(t, formatTime(user.UpdatedAt), formatTime(user2.UpdatedAt))
-	assert.True(t, isTimeZero(user2.DeletedAt))
+	assert.True(t, utils.IsTimeZero(user2.DeletedAt))
 	fmt.Println("user2", user2.CreatedAt, user2.UpdatedAt, user2.DeletedAt)
 	fmt.Println("user2 str", user2.CreatedAtStr, user2.UpdatedAtStr)
 
@@ -290,7 +291,7 @@ func TestTimeUserDeleted(t *testing.T) {
 	cnt, err = testEngine.Where("id = ?", "lunny").Delete(&user3)
 	assert.NoError(t, err)
 	assert.EqualValues(t, 1, cnt)
-	assert.True(t, !isTimeZero(user3.DeletedAt))
+	assert.True(t, !utils.IsTimeZero(user3.DeletedAt))
 
 	var user4 UserDeleted
 	has, err = testEngine.Unscoped().Get(&user4)
@@ -336,14 +337,14 @@ func TestTimeUserDeletedDiffLoc(t *testing.T) {
 	assert.EqualValues(t, formatTime(user.CreatedAt), formatTime(user2.CreatedAt))
 	assert.EqualValues(t, user.UpdatedAt.Unix(), user2.UpdatedAt.Unix())
 	assert.EqualValues(t, formatTime(user.UpdatedAt), formatTime(user2.UpdatedAt))
-	assert.True(t, isTimeZero(user2.DeletedAt))
+	assert.True(t, utils.IsTimeZero(user2.DeletedAt))
 	fmt.Println("user2", user2.CreatedAt, user2.UpdatedAt, user2.DeletedAt)
 
 	var user3 UserDeleted2
 	cnt, err = testEngine.Where("id = ?", "lunny").Delete(&user3)
 	assert.NoError(t, err)
 	assert.EqualValues(t, 1, cnt)
-	assert.True(t, !isTimeZero(user3.DeletedAt))
+	assert.True(t, !utils.IsTimeZero(user3.DeletedAt))
 
 	var user4 UserDeleted2
 	has, err = testEngine.Unscoped().Get(&user4)
@@ -407,14 +408,14 @@ func TestCustomTimeUserDeleted(t *testing.T) {
 	assert.EqualValues(t, formatTime(time.Time(user.CreatedAt)), formatTime(time.Time(user2.CreatedAt)))
 	assert.EqualValues(t, user.UpdatedAt.Unix(), user2.UpdatedAt.Unix())
 	assert.EqualValues(t, formatTime(time.Time(user.UpdatedAt)), formatTime(time.Time(user2.UpdatedAt)))
-	assert.True(t, isTimeZero(time.Time(user2.DeletedAt)))
+	assert.True(t, utils.IsTimeZero(time.Time(user2.DeletedAt)))
 	fmt.Println("user2", user2.CreatedAt, user2.UpdatedAt, user2.DeletedAt)
 
 	var user3 UserDeleted3
 	cnt, err = testEngine.Where("id = ?", "lunny").Delete(&user3)
 	assert.NoError(t, err)
 	assert.EqualValues(t, 1, cnt)
-	assert.True(t, !isTimeZero(time.Time(user3.DeletedAt)))
+	assert.True(t, !utils.IsTimeZero(time.Time(user3.DeletedAt)))
 
 	var user4 UserDeleted3
 	has, err = testEngine.Unscoped().Get(&user4)
@@ -460,14 +461,14 @@ func TestCustomTimeUserDeletedDiffLoc(t *testing.T) {
 	assert.EqualValues(t, formatTime(time.Time(user.CreatedAt)), formatTime(time.Time(user2.CreatedAt)))
 	assert.EqualValues(t, user.UpdatedAt.Unix(), user2.UpdatedAt.Unix())
 	assert.EqualValues(t, formatTime(time.Time(user.UpdatedAt)), formatTime(time.Time(user2.UpdatedAt)))
-	assert.True(t, isTimeZero(time.Time(user2.DeletedAt)))
+	assert.True(t, utils.IsTimeZero(time.Time(user2.DeletedAt)))
 	fmt.Println("user2", user2.CreatedAt, user2.UpdatedAt, user2.DeletedAt)
 
 	var user3 UserDeleted4
 	cnt, err = testEngine.Where("id = ?", "lunny").Delete(&user3)
 	assert.NoError(t, err)
 	assert.EqualValues(t, 1, cnt)
-	assert.True(t, !isTimeZero(time.Time(user3.DeletedAt)))
+	assert.True(t, !utils.IsTimeZero(time.Time(user3.DeletedAt)))
 
 	var user4 UserDeleted4
 	has, err = testEngine.Unscoped().Get(&user4)