Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"primary" conversion panics on custom string types #123

Open
hypnoglow opened this issue Nov 21, 2017 · 0 comments
Open

"primary" conversion panics on custom string types #123

hypnoglow opened this issue Nov 21, 2017 · 0 comments

Comments

@hypnoglow
Copy link
Contributor

hypnoglow commented Nov 21, 2017

Consider the following example:

type (
	IBSN string

	Book struct {
		ID IBSN `jsonapi:"primary,books"`
	}
)

book := &Book{ID: IBSN("978-3-16-148410-0")}

MarshalPayload(w, book)

The IBSN type has an underlying type string, so it should be easily convertible to string. But the current marshaler implementation for jsonapi:"primary,smth" panics:

panic: interface conversion: interface {} is jsonapi.IBSN, not string

goroutine 5 [running]:
testing.tRunner.func1(0xc4200b00f0)
	/usr/local/Cellar/go/1.9.2/libexec/src/testing/testing.go:711 +0x2d2
panic(0x11685e0, 0xc42000e700)
	/usr/local/Cellar/go/1.9.2/libexec/src/runtime/panic.go:491 +0x283
github.com/hypnoglow/jsonapi.visitModelNode(0x11530e0, 0xc420050760, 0xc42004cde8, 0x1, 0xc42007c3c0, 0xc420050760, 0x16)
	/Users/hypnoglow/go/src/github.com/hypnoglow/jsonapi/response.go:254 +0x2f28
github.com/hypnoglow/jsonapi.marshalOne(0x11530e0, 0xc420050760, 0x16, 0x116bf20, 0xc420050760)
	/Users/hypnoglow/go/src/github.com/hypnoglow/jsonapi/response.go:143 +0x7d
github.com/hypnoglow/jsonapi.Marshal(0x11530e0, 0xc420050760, 0x80, 0x1266200, 0x12be000, 0x0)
...

Important

This is kinda related to #114, but for types that are literally type T string this library should work without any custom type registration or other redundant efforts.

I think the same may be applied to other primitive types like type T int64, they should be also convertible without any problem; but I haven't implemented them in the PR below.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant