feat: support custom marshaling and unmarshaling for attributes

[ctrombley]

WIP

This PR attempts support for custom marshaling and unmarshaling for resource attributes.

The AttributeMarshaler and AttributeUnmarshaler interfaces provide methods that allow custom logic at marshal/unmarshal time.

This may prove useful for a related PR: hashicorp/go-tfe#786

cc @nfagerlund @brandonc

Example use

The UnsetableTime type below implements the AttributeMarshaler and AttributeUnmarshaler interfaces. The methods in these interfaces allow complex attribute values to be transformed to simple values during marshaling:

type UnsetableTime struct {
	Value *time.Time
}

func (t *UnsetableTime) MarshalAttribute() (interface{}, error) {
	if t == nil {
		return nil, nil
	}

	if t.Value == nil {
		return json.RawMessage(nil), nil
	} else {
		return t.Value, nil
	}
}

func (t *UnsetableTime) UnmarshalAttribute(obj interface{}) error {
	var ts time.Time
	var err error

	if obj == nil {
		t.Value = nil
		return nil
	}

	if tsStr, ok := obj.(string); ok {
		ts, err = time.Parse(tsStr, time.RFC3339)
		if err == nil {
			t.Value = &ts
			return nil
		}
	} else if tsFloat, ok := obj.(float64); ok {
		ts = time.Unix(int64(tsFloat), 0)

		t.Value = &ts
		return nil
	}

	return errors.New("couldn't parse time")
}

Now you can use a pointer to the custom type in place of a primitive value like *time.Time in the payload struct. omitempty still works as expected:

type UserSetting struct {
  ID uint64 `jsonapi:"primary,user_settings"`
  Name string `jsonapi:"attr,name,omitempty"`
  AutoDestroyAt *UnsetableTime `jsonapi:"attr,auto_destroy_at,omitempty"`
}

nextWeek := time.Now().AddDate(0, 0, 7)

setTheValue  := {
  AutoDestroyAt: &UnsetableTime{&now},
}

// {
//   "data": {
//     "type": "user_settings",
//     "id": "1",
//     "attributes": { 
//       "auto_destroy_at": "2024-01-02T10:58:55-0800"
//     },
//     ...
//   }
// }

unsetTheValue  := {
  AutoDestroyAt: &UnsetableTime{ },
}

// {
//   "data": {
//     "type": "user_settings",
//     "id": "1",
//     "attributes": { 
//       "auto_destroy_at": null
//     },
//     ...
//   }
// }

keepTheExistingValue  := {
  AutoDestroyAt: nil,
}

// {
//   "data": {
//     "type": "user_settings",
//     "id": "1",
//     "attributes": { },
//     ...
//   }
// }

Background

Certain HTTP API designs can lead to friction when wrapped with a Go SDK. A common case is an API endpoint whose request payload contains one or more optional attributes that, when included, communicate the users' intent to update their values:

Example PATCH payload for enabling auto_destroy_at

{
  "name": "updated name",
  "auto_destroy_at": "2023-12-27T10:58:55-0800"
}

The payload above will update the name of the model and enable the auto_destroy_at toggle, while also providing a value for it to use. To unset auto_destroy_at, a null value can be passed for the attribute.

Example PATCH payload for disabling auto_destroy_at

{
  "auto_destroy_at": null
}

Omitting the attribute from the payload allows the model to keep whatever value has been previously set.

jsonapi allows use of the omitempty struct tag to create payloads that optionally omit attributes when marshaling if they have a zero value:

type UserSetting struct {
  ID uint64 `jsonapi:"primary,user_settings"`
  Name string `jsonapi:"attr,name,omitempty"`
  AutoDestroyAt *time.Time `jsonapi:"attr,auto_destroy_at,omitempty"`
}

Unfortunately, the underlying API uses a JSON null for disabling the toggle, so using a corresponding nil value will cause the attribute to be omitted.

Notes/Questions

• The interface methods take inspiration from encoding/json, but they are based on interface{} rather than byte[]. They effectively act as hooks that allow custom transformation logic during the marshaling and unmarshaling of attribute values. Is there a better verb than Marshal for the names of these interfaces?
• This PR attempts to support nested jsonapi nodes as attribute values, but currently for the marshaling case only. Control is yielded to the jsonapi library after a custom value is returned form MarshalAttribute. This also allows the existing struct-tag-configured date formatting logic to be applied to the transformed value without the need for re-implementation. This makes it easier to implement custom marshaling logic, but it may be harder to document and understand. A similar solution may be achievable for the unmarshaling case, but hasn't been included here.

[nfagerlund]

Well!! 👀

Ok, so I spent some time trying to get my example scrap to work with this patch. It didn't quite make it across the finish line!

jsonapi: Can't unmarshal It'S HAPPNEING (string) to struct field `Problem`, which is a pointer to `ProblematicString (struct)`

The issue here seems to be something like:

• The really interesting use case here is for struct-typed attributes where the type implements custom un/marshal.
• This draft only puts custom unmarshal logic in the top-level unmarshalNode func, but in order to handle attributes with custom serialization, we need an alternate dispatch path down in unmarshalAttribute.

At this point, I took a stab at implementing that, and immediately got lost in a spiky thicket of reflect chaos. Then eventually I started doubting everything I thought I knew (for example, how in the world does handleStruct work? It looks like it's treating a struct-typed attribute as... a whole-ass jsonapi node? That can't be right, surely I'm misreading something).

Anyway, hopefully that's useful despite me not having a complete answer to deliver. If it'd be helpful, I can try and clean up that gist to make a more legible test case; it's descended from my initial research into this space, so it's a bit of a mess.

[nfagerlund]

WAIT. Actually, I think I figured it out.

func handleCustom(attribute interface{}, fieldValue reflect.Value) (reflect.Value, error) {
	// Re-create the raw json representation of whatever we got over the wire --
	// could be string, bool, Object, etc. etc. etc., but we want json bytes
	// so we can take another crack at it.
	data, err := json.Marshal(attribute)
	if err != nil {
		return reflect.Value{}, err
	}
	// Get something to serialize into, based on what the field is intended to
	// be. (I think fieldValue is a wrapped instance of the field type's zero
	// value... which, if the field's a pointer, is pointing to nil instead of
	// allocated memory, so we're better off starting from scratch than trying
	// to actually use it.)
	var model reflect.Value
	if fieldValue.Kind() == reflect.Pointer {
		// We want the value type.
		model = reflect.New(fieldValue.Type().Elem())
	} else {
		model = reflect.New(fieldValue.Type())
	}

	ref := model.Interface() // a type-erased pointer to the inner value, IIUC
	if err := json.Unmarshal(data, ref); err != nil {
		return reflect.Value{}, err
	}
	// Interestingly, we don't have to worry about whether the destination field
	// is a pointer or a value; something downstream of here just handles that
	// for us.
	return model, nil
}

And then in unmarshalAttribute, you need to do this before all the rest of the dispatch options:

	// Handle field with arbitrary custom unmarshal logic
	if _, ok := fieldValue.Interface().(Unmarshaler); ok {
		value, err = handleCustom(attribute, fieldValue)
		return
	}

[notchairmk]

I'm pretty sure this isn't quite the direction we want to go, at least for the moment. A couple of thoughts:

• this implementation doesn't work for nested jsonapi structs (correct? I haven't actually tested, so this point is just from reading the code)
• this implementation forces the user to define their own marshal strategy for each struct that wants some degree of customizability

To the latter point: we don't need to grant users complete marshal customizability to support the intent of hashicorp/go-tfe#786 in an alternative way. Essentially all we want to do there is have some conditional mechanism for marshaling a struct field when nil.

[nfagerlund]

Right — I guess I didn't say so explicitly, but I don't think defining custom marshaling for an entire jsonapi resource makes sense for go-tfe's use case. For that, we just want optional custom logic at the attribute level. (But, I think the thing I pasted is pretty close to getting there! It works in my experimental code.)

[brandonc]

Can this be a package level var so it doesn't have to be evaluated on every attribute?

[ctrombley]

superceded by #21