[ISSUE] reflect: NumField of non-struct type

[vzool]

Hi,

I have to use your package with the package hide.ID and I have an error with Update and Delete. Here is the full error report:
I use SQLite driver

2019/09/24 14:39:23 [Recovery] 2019/09/24 - 14:39:23 panic recovered:
PUT /api/v1/country/lk4KbkPpG HTTP/1.1
Host: localhost:8080
Accept: application/json, text/plain, */*
Accept-Encoding: gzip, deflate, br
Accept-Language: en-US,en;q=0.9,ar;q=0.8
Authorization: *
Connection: keep-alive
Content-Length: 29
Content-Type: application/json;charset=UTF-8
Origin: http://localhost:3000
Referer: http://localhost:3000/
Sec-Fetch-Mode: cors
Sec-Fetch-Site: same-site
User-Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/77.0.3865.90 Safari/537.36

reflect: NumField of non-struct type
/usr/lib64/go/1.11/src/runtime/panic.go:513 (0x4333d8)
	gopanic: reflectcall(nil, unsafe.Pointer(d.fn), deferArgs(d), uint32(d.siz), uint32(d.siz))
/home/vzool/go/pkg/mod/github.com/jinzhu/gorm@v1.9.10/scope.go:884 (0x989542)
	(*Scope).callCallbacks.func1: panic(err)
/usr/lib64/go/1.11/src/runtime/asm_amd64.s:522 (0x45ebda)
	call32: CALLFN(·call32, 32)
/usr/lib64/go/1.11/src/runtime/panic.go:513 (0x4333d8)
	gopanic: reflectcall(nil, unsafe.Pointer(d.fn), deferArgs(d), uint32(d.siz), uint32(d.siz))
/usr/lib64/go/1.11/src/reflect/type.go:984 (0x4b4b4d)
	(*rtype).NumField: panic("reflect: NumField of non-struct type")
/home/vzool/go/pkg/mod/github.com/sas1024/gorm-loggable@v0.0.0-20190307192712-3ae87a295625/util.go:99 (0x997b32)
	getLoggableFieldNames: for i := 0; i < t.NumField(); i++ {
/home/vzool/go/pkg/mod/github.com/sas1024/gorm-loggable@v0.0.0-20190307192712-3ae87a295625/callbacks.go:141 (0x9954cb)
	computeUpdateDiff: names := getLoggableFieldNames(old)
/home/vzool/go/pkg/mod/github.com/sas1024/gorm-loggable@v0.0.0-20190307192712-3ae87a295625/callbacks.go:88 (0x994d81)
	addUpdateRecord: diff := computeUpdateDiff(scope)
/home/vzool/go/pkg/mod/github.com/sas1024/gorm-loggable@v0.0.0-20190307192712-3ae87a295625/callbacks.go:70 (0x994a27)
	(*Plugin).addUpdated: addUpdateRecord(scope, p.opts)
/home/vzool/go/pkg/mod/github.com/sas1024/gorm-loggable@v0.0.0-20190307192712-3ae87a295625/plugin.go:25 (0x998543)
	(*Plugin).addUpdated-fm: callback.Update().After("gorm:after_update").Register("loggable:update", p.addUpdated)
/home/vzool/go/pkg/mod/github.com/jinzhu/gorm@v1.9.10/scope.go:888 (0x974b47)
	(*Scope).callCallbacks: (*f)(scope)
/home/vzool/go/pkg/mod/github.com/jinzhu/gorm@v1.9.10/main.go:467 (0x9648bf)
	(*DB).Save: newDB := scope.callCallbacks(s.parent.callbacks.updates).db
/home/vzool/Workspace/seren/webapp/app/http/controllers/country.go:210 (0xbd0b81)
	CountryUpdate: if err := db.Save(&data).Error; err != nil {
/home/vzool/go/pkg/mod/github.com/gin-gonic/gin@v1.4.0/context.go:124 (0x8d4a09)
	(*Context).Next: c.handlers[c.index](c)
/home/vzool/Workspace/seren/webapp/app/http/middleware/auth.go:55 (0xbf5492)
	Auth.func1: ctx.Next()
/home/vzool/go/pkg/mod/github.com/gin-gonic/gin@v1.4.0/context.go:124 (0x8d4a09)
	(*Context).Next: c.handlers[c.index](c)
/home/vzool/Workspace/seren/webapp/app/http/middleware/hideID.go:32 (0xbf5ef1)
	HideID.func1: ctx.Next()
/home/vzool/go/pkg/mod/github.com/gin-gonic/gin@v1.4.0/context.go:124 (0x8d4a09)
	(*Context).Next: c.handlers[c.index](c)
/home/vzool/Workspace/seren/webapp/app/http/middleware/cors.go:26 (0xbf5b72)
	CORS.func1: c.Next()
/home/vzool/go/pkg/mod/github.com/gin-gonic/gin@v1.4.0/context.go:124 (0x8d4a09)
	(*Context).Next: c.handlers[c.index](c)
/home/vzool/go/pkg/mod/github.com/gin-contrib/gzip@v0.0.1/gzip.go:47 (0xbf155e)
	Gzip.func2: c.Next()
/home/vzool/go/pkg/mod/github.com/gin-gonic/gin@v1.4.0/context.go:124 (0x8d4a09)
	(*Context).Next: c.handlers[c.index](c)
/home/vzool/Workspace/seren/webapp/app/http/middleware/secure.go:42 (0xbf61bf)
	Secure.func1: ctx.Next()
/home/vzool/go/pkg/mod/github.com/gin-gonic/gin@v1.4.0/context.go:124 (0x8d4a09)
	(*Context).Next: c.handlers[c.index](c)
/home/vzool/go/pkg/mod/github.com/gin-gonic/gin@v1.4.0/recovery.go:83 (0x8e7c19)
	RecoveryWithWriter.func1: c.Next()
/home/vzool/go/pkg/mod/github.com/gin-gonic/gin@v1.4.0/context.go:124 (0x8d4a09)
	(*Context).Next: c.handlers[c.index](c)
/home/vzool/go/pkg/mod/github.com/gin-gonic/gin@v1.4.0/logger.go:240 (0x8e6d80)
	LoggerWithConfig.func1: c.Next()
/home/vzool/go/pkg/mod/github.com/gin-gonic/gin@v1.4.0/context.go:124 (0x8d4a09)
	(*Context).Next: c.handlers[c.index](c)
/home/vzool/go/pkg/mod/github.com/gin-gonic/gin@v1.4.0/gin.go:389 (0x8de0c4)
	(*Engine).handleHTTPRequest: c.Next()
/home/vzool/go/pkg/mod/github.com/gin-gonic/gin@v1.4.0/gin.go:351 (0x8dd8f1)
	(*Engine).ServeHTTP: engine.handleHTTPRequest(c)
/usr/lib64/go/1.11/src/net/http/server.go:2741 (0x70760a)
	serverHandler.ServeHTTP: handler.ServeHTTP(rw, req)
/usr/lib64/go/1.11/src/net/http/server.go:1847 (0x703855)
	(*conn).serve: serverHandler{c.server}.ServeHTTP(w, w.req)
/usr/lib64/go/1.11/src/runtime/asm_amd64.s:1333 (0x4608f0)
	goexit: BYTE	$0x90	// NOP

Any advice could help me out? Thanks

[vzool]

After some investigation I found that when I enabled loggable.ComputeDiff() in loggable.Register() the issue occurred and when I removed the error will go. So the main issue should be within loggable.ComputeDiff() function.

[sas1024]

Hello @vzool.
Can you show me the structure of data variable somewhere in /home/vzool/Workspace/seren/webapp/app/http/controllers/country.go:210 ?

[vzool]

Hi @sas1024, data is an instance of Country struct so here it is:

// BaseModel model
type BaseModel struct {
	ID hide.ID `gorm:"primary_key" json:"id,omitempty"`
}

// TrackingModel model
type TrackingModel struct {
	CreatedAt *time.Time `json:"created_at,omitempty" gorm:"NOT NULL"`
	UpdatedAt *time.Time `json:"updated_at,omitempty" gorm:"DEFAULT NULL"`
	DeletedAt *time.Time `sql:"index" json:"deleted_at,omitempty" gorm:"DEFAULT NULL"`

	CreatedBy hide.ID `json:"created_by,omitempty" gorm:"NOT NULL"`
	UpdatedBy hide.ID `json:"updated_by,omitempty" gorm:"DEFAULT NULL"`
	DeletedBy hide.ID `json:"deleted_by,omitempty" gorm:"DEFAULT NULL"`

	loggable.LoggableModel
}

// Country model
type Country struct {
	BaseModel

	Name string `json:"name,omitempty" form:"name" valid:"required" gorm:"unique_index;NOT NULL" gorm-loggable:"true"`
	Code int    `json:"code,omitempty" form:"code" gorm:"unique_index;NOT NULL" gorm-loggable:"true"`

	TrackingModel
}

[vzool]

After extra investigation I think that this line below inside callbacks/computeUpdateDiff():

old := im.get(scope.Value, scope.PrimaryKeyValue())

Does not return the old record to make Diff operation with new one.

[sas1024]

I think you need to move loggable.LoggableModel from TrackingModel to Country.

[vzool]

This let me pass the error and fall into a new one:

reflect: call of reflect.Value.Field on ptr Value

And I can't share the complete logs because I changed a lot of things which become messy.

In fact, I did managed to pass first error with the following update:

func getLoggableFieldNames(value interface{}) []string {
	var names []string

	t := pointerType(reflect.TypeOf(value))

	for i := 0; i < t.NumField(); i++ {
		field := t.Field(i)
		value, ok := field.Tag.Lookup(loggableTag)
		if !ok || value != "true" {
			continue
		}

		names = append(names, field.Name)
	}

	fmt.Println("(Names): ", names)

	return names
}

// pointerType is made to chase for value through all the way following,
// leading pointers until reach the deep final value which is not a pointer
func pointerType(t reflect.Type) reflect.Type {
	for {
		if t.Kind() != reflect.Ptr {
			return t
		}
		t = t.Elem()
	}
}

So, I think it doesn't matter any more if I shift loggable.LoggableModel into upper level or leave it in the TrackingModel. Because the pointerType function will affects over all code and it behaves now more dynamic.

[vzool]

Okey I did reset the code and just add pointerType function with its call inside callbacks/getLoggableFieldNames() and produce the issue which gives me this:

2019/09/25 17:01:39 [Recovery] 2019/09/25 - 17:01:39 panic recovered:
PUT /api/v1/country/e0YPEGdBM HTTP/1.1
Host: localhost:8080
Accept: */*
Accept-Encoding: gzip, deflate, br
Accept-Language: en-US,en;q=0.9,ar;q=0.8
Authorization: *
Cache-Control: no-cache
Connection: keep-alive
Content-Length: 45
Content-Type: text/plain;charset=UTF-8
Origin: chrome-extension://fhbjgbiflinjbdggehcddcbncdddomop
Postman-Token: 8a460257-d870-b7f8-5ac6-ae1159a0c9ff
Sec-Fetch-Mode: cors
Sec-Fetch-Site: cross-site
User-Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/77.0.3865.90 Safari/537.36


reflect: call of reflect.Value.FieldByName on ptr Value
/usr/lib64/go/1.11/src/runtime/panic.go:513 (0x433358)
	gopanic: reflectcall(nil, unsafe.Pointer(d.fn), deferArgs(d), uint32(d.siz), uint32(d.siz))
/home/vzool/go/pkg/mod/github.com/jinzhu/gorm@v1.9.10/scope.go:884 (0x985422)
	(*Scope).callCallbacks.func1: panic(err)
/usr/lib64/go/1.11/src/runtime/asm_amd64.s:522 (0x45eb0a)
	call32: CALLFN(·call32, 32)
/usr/lib64/go/1.11/src/runtime/panic.go:513 (0x433358)
	gopanic: reflectcall(nil, unsafe.Pointer(d.fn), deferArgs(d), uint32(d.siz), uint32(d.siz))
/usr/lib64/go/1.11/src/reflect/value.go:207 (0x4bd2a3)
	flag.mustBe: panic(&ValueError{methodName(), f.kind()})
/usr/lib64/go/1.11/src/reflect/value.go:865 (0x4c017b)
	Value.FieldByName: v.mustBe(Struct)
/home/vzool/go/pkg/mod/github.com/sas1024/gorm-loggable@v0.0.0-20190307192712-3ae87a295625/callbacks.go:146 (0x99144f)
	computeUpdateDiff: ofv := ov.FieldByName(name).Interface()
/home/vzool/go/pkg/mod/github.com/sas1024/gorm-loggable@v0.0.0-20190307192712-3ae87a295625/callbacks.go:88 (0x990c61)
	addUpdateRecord: diff := computeUpdateDiff(scope)
/home/vzool/go/pkg/mod/github.com/sas1024/gorm-loggable@v0.0.0-20190307192712-3ae87a295625/callbacks.go:70 (0x990907)
	(*Plugin).addUpdated: addUpdateRecord(scope, p.opts)
/home/vzool/go/pkg/mod/github.com/sas1024/gorm-loggable@v0.0.0-20190307192712-3ae87a295625/plugin.go:25 (0x9944c3)
	(*Plugin).addUpdated-fm: callback.Update().After("gorm:after_update").Register("loggable:update", p.addUpdated)
/home/vzool/go/pkg/mod/github.com/jinzhu/gorm@v1.9.10/scope.go:888 (0x970a27)
	(*Scope).callCallbacks: (*f)(scope)
/home/vzool/go/pkg/mod/github.com/jinzhu/gorm@v1.9.10/main.go:467 (0x96079f)
	(*DB).Save: newDB := scope.callCallbacks(s.parent.callbacks.updates).db
/home/vzool/Workspace/seren/webapp/app/http/controllers/country.go:210 (0xb31f01)
	CountryUpdate: if err := db.Save(&data).Error; err != nil {
/home/vzool/go/pkg/mod/github.com/gin-gonic/gin@v1.4.0/context.go:124 (0x8d08e9)
	(*Context).Next: c.handlers[c.index](c)
/home/vzool/Workspace/seren/webapp/app/http/middleware/auth.go:55 (0xb562d2)
	Auth.func1: ctx.Next()
/home/vzool/go/pkg/mod/github.com/gin-gonic/gin@v1.4.0/context.go:124 (0x8d08e9)
	(*Context).Next: c.handlers[c.index](c)
/home/vzool/Workspace/seren/webapp/app/http/middleware/hideID.go:32 (0xb56d31)
	HideID.func1: ctx.Next()
/home/vzool/go/pkg/mod/github.com/gin-gonic/gin@v1.4.0/context.go:124 (0x8d08e9)
	(*Context).Next: c.handlers[c.index](c)
/home/vzool/Workspace/seren/webapp/app/http/middleware/cors.go:26 (0xb569b2)
	CORS.func1: c.Next()
/home/vzool/go/pkg/mod/github.com/gin-gonic/gin@v1.4.0/context.go:124 (0x8d08e9)
	(*Context).Next: c.handlers[c.index](c)
/home/vzool/go/pkg/mod/github.com/gin-contrib/gzip@v0.0.1/gzip.go:47 (0xb5239e)
	Gzip.func2: c.Next()
/home/vzool/go/pkg/mod/github.com/gin-gonic/gin@v1.4.0/context.go:124 (0x8d08e9)
	(*Context).Next: c.handlers[c.index](c)
/home/vzool/Workspace/seren/webapp/app/http/middleware/secure.go:42 (0xb56fff)
	Secure.func1: ctx.Next()
/home/vzool/go/pkg/mod/github.com/gin-gonic/gin@v1.4.0/context.go:124 (0x8d08e9)
	(*Context).Next: c.handlers[c.index](c)
/home/vzool/go/pkg/mod/github.com/gin-gonic/gin@v1.4.0/recovery.go:83 (0x8e3af9)
	RecoveryWithWriter.func1: c.Next()
/home/vzool/go/pkg/mod/github.com/gin-gonic/gin@v1.4.0/context.go:124 (0x8d08e9)
	(*Context).Next: c.handlers[c.index](c)
/home/vzool/go/pkg/mod/github.com/gin-gonic/gin@v1.4.0/logger.go:240 (0x8e2c60)
	LoggerWithConfig.func1: c.Next()
/home/vzool/go/pkg/mod/github.com/gin-gonic/gin@v1.4.0/context.go:124 (0x8d08e9)
	(*Context).Next: c.handlers[c.index](c)
/home/vzool/go/pkg/mod/github.com/gin-gonic/gin@v1.4.0/gin.go:389 (0x8d9fa4)
	(*Engine).handleHTTPRequest: c.Next()
/home/vzool/go/pkg/mod/github.com/gin-gonic/gin@v1.4.0/gin.go:351 (0x8d97d1)
	(*Engine).ServeHTTP: engine.handleHTTPRequest(c)
/usr/lib64/go/1.11/src/net/http/server.go:2741 (0x7034ea)
	serverHandler.ServeHTTP: handler.ServeHTTP(rw, req)
/usr/lib64/go/1.11/src/net/http/server.go:1847 (0x6ff735)
	(*conn).serve: serverHandler{c.server}.ServeHTTP(w, w.req)
/usr/lib64/go/1.11/src/runtime/asm_amd64.s:1333 (0x460820)
	goexit: BYTE	$0x90	// NOP

[sas1024]

For now i'm not able to resolve this issue, because i'm not using this addon anymore.

@vcraescu can you help with this issue?

[vetcher]

@vzool Hi. I think, the main reason, why it panics, because you use pointers in Update method. Could you post minimal example, that panics on https://play.golang.org ?
In current implementation, diff expects from model to be structure.

Also, as quick fix, I think, you may change code of identity_manager.go:
Before:

func (im *identityManager) save(value, pk interface{}) {
	t := reflect.TypeOf(value)
	newValue := reflect.New(t).Interface()
	err := copier.Copy(&newValue, value)
	if err != nil {
		panic(err)
	}

	im.m[genIdentityKey(t, pk)] = newValue
}

After:

func (im *identityManager) save(value, pk interface{}) {
	t := reflect.Indirect(reflect.ValueOf(value)).Type()
	newValue := reflect.New(t).Interface()
	err := copier.Copy(&newValue, value)
	if err != nil {
		panic(err)
	}

	im.m[genIdentityKey(t, pk)] = newValue
}

[vzool]

@vetcher I'm not sure now, because I forked it locally and I did changed a lot of things for a degree that I can get rid of identityManager itself.