fix(types): split comma-separated labels when decoding LabelOptions

José M. Requena Plensrequested to merge
gitlab-community/gitlab-org/api/client-go:jmrp-label-options-unmarshal-split into main

What does this MR do?

LabelOptions.MarshalJSON writes the labels as one comma-separated string (LabelOptions{"bug", "ui"} becomes "bug,ui"), the form the API documents for labels, add_labels and remove_labels. UnmarshalJSON does not read that form back: it wraps anything that is not a JSON array in brackets, so "bug,ui" decodes to the single label "bug,ui" and "" to {""}. null gives {""} too, or cuts a value that already held labels down to its first one. The issue, merge request, epic, board and issue statistics options that carry a *LabelOptions do not survive a JSON round trip.

This MR splits a string on commas, decodes "" to an empty LabelOptions, and leaves the value unchanged on null, as ISOTime.UnmarshalJSON does. Arrays decode as before.

Is this a breaking change?

Not at compile time, but decode results change on purpose: a string with commas gives several labels, "" gives no labels, and null leaves the value as it was. A single label that contains a comma can no longer be decoded from a string. MarshalJSON cannot write one either, and GitLab splits these parameters on commas (lib/api/issues.rb#L126-128).

Requests do not change: MarshalJSON and EncodeValues are untouched, and re-encoding a decoded string sends the same string as before. The one response field of this type, BlockingMergeRequest.Labels, gets a JSON array from GitLab, which decodes as before.

How was this tested?

  • Table-driven tests in types_test.go for each input form, and a round trip through UpdateIssueOptions. On main at 0d4556f9, with only the tests added, the comma, empty string and null cases and two round trips fail; with the fix all 13 cases pass, on Go 1.26.10 and 1.27.3.
  • make generate and make fmt change no file, and make lint (golangci-lint 2.15.0, buf 1.55.1) and make test pass on Go 1.27.3. make test also passes on Go 1.26.10.
  • Changing each branch of the new code in turn (dropping the array, null or empty string check, not splitting, ignoring the error) makes at least one new test fail.
  • There is no integration test: GitLab never sends the string form, so no API call reaches the changed path.

Related: !1840 (merged) introduced this MarshalJSON and UnmarshalJSON pair.

I found this while maintaining gitlab-mcp-server, which is built on this library.

Merge request reports