mholt/binding

Best practice for operating on slices of data and keeping context

itsjamie opened this issue · 4 comments

We currently receive a large JSON object that defines a list of steps, we validate them differently based on the type of step that is received, so we do multi-step JSON decoding with json.RawMessage and validation logic is different based on the type of field. So we didn't want to have to inline all the validation for every type.

The problem is that without inlining, with the current interface, we don't necessarily have a established method of passing context down, or adding context to the error messages. The comment for Error.Message indicates the intention of being able to say there is an issue with the 41st object in a slice of 100.

I can think of a few methods, wondering if anyone else had worked through this. We could utilize the context stored on req.Context.

var items []binding.Validator
for i, item := range items { 
    errs = item.Validate(req, errs) 
} 

Thanks for any insight.

Can you provide a fuller example of your current approach and exactly what it is you want to be able to access from with the Validate method?

Important to note, the below does not currently make full use of the binding API to do the validation, but we're moving towards it.

type CreateEncodingProfile struct {
	Name          string        `json:"name"`
	TextTracks    []string      `json:"text_tracks"`
	Data          *EncodingData `json:"data" binding:"required"`
	DigitalRights Rights        `json:"digital_rights"`
	Type          string        `json:"type"` 
	Locked        bool          `json:"locked"`
	Default       bool          `json:"-"`
}

Current Valid method

func (e *CreateEncodingProfile) Valid() (bool, Errors) {
	errs := Errors{}

	switch e.Name {
	case "":
		errs.Add([]string{"name"}, "invalid", "Profiles must have a name.")
	default:
		if !SafeNamingRegex.MatchString(e.Name) {
			errs.Add([]string{"name"}, "invalid", "Profile names must only contain alphanumeric characters, whitespace, and dashes.")
		}
	}

	errs = e.Data.Validate(nil, errs)

	if errs.Len() == 0 {
		return true, nil
	}

	return false, errs
}

Inside EncodingData is a EncodingVariant

// EncodingVariant is an object that represents a collection of renditions
// that all are related, and should be delivered together for adaptive playlists
type EncodingVariant struct {
	Name           string `json:"name"`
	ByteRange      bool   `json:"byte_range"`
	IFramePlaylist bool   `json:"iframe_playlist"`
	Renditions     []int  `json:"renditions"`
	Version        int    `json:"version"`
}

// Validate checks if this variant is able to be used
func (ev EncodingVariant) Validate(req *http.Request, errs Errors) Errors {
	switch ev.Name {
	case "":
		errs.Add([]string{"variant.name"}, "invalid", "Variants must have a name.")
	default:
		if !SafeNamingRegex.MatchString(ev.Name) {
			errs.Add([]string{"variant.name"}, "invalid", fmt.Sprintf("Variant names (%s) must only contain alphanumeric characters, whitespace, and dashes.", ev.Name))
		}
	}

	return errs
}

Right now, each of these objects have something that can uniquely identify them and we could change the field name to be like variant[variant.Name].name, but preferably, we would reference the index in the array of variants, so the name would be variant[1].name.

This particular object goes about one more layer deep, in that there is EncodingRendition, which contains both Audio and Video objects and a large portion of validation occurs there. Similarly in this case, where say a video setting is incorrect, we would want to backreference up the stack, rendition[1].video.width.

One solution would be to pull all the validation logic up onto the top-level CreateEncodingProfile call, but that would end up being an extremely large function, and not very composable, for example we would need to rewrite the entire function when handling PATCH requests, etc.

Thank you for any assistance, or in helping solve this context problem.

Add an unexported field on your type to hold the contextual values you need, and set its value before validating.

Thank you, that makes sense.