Ability to round-trip worker bindings

[sodabrew]

Current cloudflare-go version

0.45

Description

I am writing a utility to download a script, modify it, and upload it again to a new name.

It would be useful if the input and output formats for the Bindings structure matched up. I am reading the code to see if there's a simple enough way to transform the structures, but I figured I'd file this before I got too deep.

Use cases

This is similar to the Workers Environments use case, so future work on that product feature might subsume this eventually, but download-modify-upload is still a valuable capability.

Potential cloudflare-go usage

cfapi, err := cloudflare.New(config.APIKey, config.APIEmail, cloudflare.UsingAccount(config.AccountID))
script, err := cfapi.DownloadWorker(ctx, &cloudflare.WorkerRequestParams{ ScriptName: "template" })
bindings, err := cfapi.ListWorkerBindings(ctx, &cloudflare.WorkerRequestParams{ ScriptName: "template" })
response, err := cfapi.UploadWorkerWithBindings(ctx,
  &cloudflare.WorkerRequestParams{
    ScriptName: "output",
  },
  &cloudflare.WorkerScriptParams{
    Script: script.Script,
    Bindings: bindings.BindingList,
  },
)

Unfortunately, this won't compile:

cannot use bindings.BindingList (variable of type []cloudflare.WorkerBindingListItem)
  as type map[string]cloudflare.WorkerBinding in struct literal

References

Aha, this might do it. I feel like this comment is a clue that maybe the data structure mismatch ought to have been resolved in the code review process. How would you feel if I put up a PR to make UploadWorkerWithBindings accept an array instead of a map? Was the idea to use a map to avoid the case of two bindings with the same name? In that case, should the ListWorkerBindings return an array? Maybe a convenience method to munge between them?

cloudflare-go/workers_test.go

Lines 233 to 240 in 9a9f293

	// Since bindings are specified in the Go API as a map but are uploaded as a
	// JSON array, the ordering of uploaded bindings is non-deterministic. To make
	// it easier to compare for equality without running into ordering issues, we
	// convert it back to a map
	bindingMeta := make(map[string]workerBindingMeta)
	for _, binding := range metadata.Bindings {
	bindingMeta[binding["name"].(string)] = binding
	}

[sodabrew]

Got it. Right, so this feels totally unnecessary impedance mismatch. Guidance on a PR to not need this code would be greatly appreciated!

bindingMap:= make(map[string]cloudflare.WorkerBinding)
for _, binding := range bindings.BindingList {
        bindingMap[binding.Name] = binding.Binding
}

End-to-end working example:

cfapi, err := cloudflare.New(config.APIKey, config.APIEmail, cloudflare.UsingAccount(config.AccountID))
script, err := cfapi.DownloadWorker(ctx, &cloudflare.WorkerRequestParams{ ScriptName: "template" })
bindings, err := cfapi.ListWorkerBindings(ctx, &cloudflare.WorkerRequestParams{ ScriptName: "template" })

bindingMap:= make(map[string]cloudflare.WorkerBinding)
for _, binding := range bindings.BindingList {
        bindingMap[binding.Name] = binding.Binding
}

response, err := cfapi.UploadWorkerWithBindings(ctx,
  &cloudflare.WorkerRequestParams{
    ScriptName: "output",
  },
  &cloudflare.WorkerScriptParams{
    Script: script.Script,
    Bindings: bindingMap,
  },
)