-
Notifications
You must be signed in to change notification settings - Fork 218
Support custom types (#114) #115
base: master
Are you sure you want to change the base?
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed, please reply here (e.g.
|
I just signed the CLA. |
CLAs look good, thanks! |
91ece5c
to
249a7ed
Compare
This applies to: - ID - Attributes - Relationships Custom types need to be registered before usage, using the new `jsonapi.RegisterType()` function which takes 3 arguments: - the type to support (`reflect.Type`) - the function to use when marshalling a response - the function to use when unmarshalling a response Example: ```` RegisterType(uuidType, func(value interface{}) (string, error) { result := value.(*UUID).String() return result, nil }, func(value string) (interface{}, error) { return UUIDFromString(value) }) ```` The custom type will be represented as a `string` in the JSON document in the requests and responses. Fixes google#114 Signed-off-by: Xavier Coulon <[email protected]>
249a7ed
to
7462def
Compare
+1 |
Please remove prints from tests so |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should not marshal/unmarshal to/from string but rather interface{}
. What if you want a number to go to a custom type? Input/output may not be a string.
Store state of return type by making type CustomType reflect.Type
Don't assume is custom type. Should be declared explicitly. IOW, add a comma separated value to struct tag, "custom", and add to constants.go.
var customTypeUnmarshallingFuncs map[reflect.Type]UnmarshallingFunc | ||
|
||
// init initializes the maps | ||
func init() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't use init(), don't use make, use var in block, e.g.
var (
customTypeMarshallingFuncs = map[reflect.Type]MarshallingFunc{}
cutomTypeUnmarshallingFuncs = map[reflect.Type]UnmarshallingFunc{}
)
|
||
import "reflect" | ||
|
||
type MarshallingFunc func(interface{}) (string, error) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Follow pattern of net/http
's http.Handle
and http.HandleFunc
, and don't rely on input/output being string e.g.
type TypeMarshaller interface {
Marshal(interface{}) (interface{}, error)
}
type TypeUnmarshaller interface {
UnMarshal(interface{}) (interface{}, error)
}
type TypeMarshalFunc func(interface{}) (interface{}, error)
func (tmf TypeMarshalFunc) func(thing interface{}) (interface{}, error) {
return tmf(thing)
}
type TypeUnmarshalFunc func(interface{}) (interface{}, error)
func (tuf TypeUnmarshalFunc) func(thing interface{}) (interface{}, error) {
return tuf(thing)
}
Then you can have RegisterCustomType(in, out reflect.Type, marshaller TypeMarshaller, unmarshaller TypeUnmarshaller) and
RegisterCustomTypeFunc(in, out reflect.Type, marshallerFunc TypeMarshallerFunc, unmarshallerFunc TypeUnmarshallerFunc)`
I'm working on changing this. See wip @ https://github.com/shwoodard/jsonapi/blob/custom_types/custom_types.go |
thanks for the review, @shwoodard ! I'm not sure I understood your last comment: do you mean I should abandon this PR since you're working on it, too ? |
I reviewed this pull request and also took a look at https://github.com/shwoodard/jsonapi/blob/custom_types/custom_types.go Package encoding/json has There are numerous packages out there, which auto generates code to create |
This applies to:
Custom types need to be registered before usage, using the new
jsonapi.RegisterType()
function which takes 3 arguments:reflect.Type
)Example:
The custom type will be represented as a
string
in the JSON documentin the requests and responses.
Fixes #114
Signed-off-by: Xavier Coulon [email protected]