[go] fix default value for array type

[titaneric]

PR checklist

• Read the contribution guidelines.
• Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
• Run the following to build the project and update samples:

  ./mvnw clean package || exit
  ./bin/generate-samples.sh ./bin/configs/*.yaml || exit
  ./bin/utils/export_docs_generators.sh || exit
  

  (For Windows users, please run the script in WSL)
  Commit all changed files.
  This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
  These must match the expectations made by your contribution.
  You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
  IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
• File the PR against the correct branch: master (upcoming 7.x.0 minor release - breaking changes with fallbacks), 8.0.x (breaking changes without fallbacks)
• If your PR solves a reported issue, reference it using GitHub's linking syntax (e.g., having "fixes #123" present in the PR description)
• If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

Hi, I encounter the issue when I want to generate the go sdk for my company's API. It looks like the generated default value for array type is not handled correctly, and I intend to resolve it with this PR

[Mattias-Sehlstedt]

Hi, I interpret the changes as wanting to add so that the generated client sends the default values that the server specifies.

What is the background for wanting the generated client to submit the default value that is present in a specification?

The purpose of the default is for the server to specify a fallback that is used if the client does not submit a value. So the correct behavior is to send nothing if you as a client do not specifically want to state a certain value, and then that the server interprets the absence as their specified fallback.

If the client themselves sends the default value, then it will not be able to react to a change of default that the server might make. This may result in incorrect behavior, since the actual indented behavior of your application is most likely to follow the servers default value, rather than "hard coding" it to the default seen at the time of client code generation (because if you did not want to follow the default, you would always explicitly state a value).

Comment taken from previous answer on the subject #22455 (comment).

[titaneric]

Hi @Mattias-Sehlstedt ,

Here is our company's OpenAPI schema

components:
  parameters:
      ListConfigMapsParams.sort:
        name: sort
        in: query
        required: false
        schema:
          type: array
          items:
            $ref: '#/components/schemas/ConfigMapsSortKeys'
          default:
            - updatedAt:desc

Here is generated model

// All allowed values of ConfigMapsSortKeys enum
var AllowedConfigMapsSortKeysEnumValues = []ConfigMapsSortKeys{
	"configMapName:asc",
	"configMapName:desc",
	"updatedAt:asc",
	"updatedAt:desc",
}

Here is generated API

	if r.sort != nil {
		parameterAddToHeaderOrQuery(localVarQueryParams, "sort", r.sort, "form", "csv")
	} else {
		var defaultValue []ConfigMapsSortKeys = [updatedAt:desc]
		r.sort = &defaultValue
	}

It's clear to see that the default value for ConfigMapsSortKeys is not correct, which needs be quoted.

[Mattias-Sehlstedt]

Thanks for the additional details, so we are adjusting an already incorrect behavior so that the code behaves. I also saw that the previous comments that I had placed on this default-subject also was on the exact commit that introduced so that arrays can have default values 🙃.

Rather than introducing a new specification and generating a sample for that, I would suggest expanding the one that already exist for the issue, and make sure it also contains an array with only a single item. I would also extend the unit test to make it verify that a lone item is correctly generated.

Note: I am not affiliated with the project, and these are my personal thoughts and opinions as a contributor. They do not affect whether this gets merged.

[wing328]

thanks for the pr

please add the new folder to the go github action wrokflow so that it will be tested moving forward: https://github.com/OpenAPITools/openapi-generator/blob/master/.github/workflows/samples-go-client.yaml

[cubic-dev-ai]

2 issues found across 21 files

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="samples/client/others/go/array-default-value/client.go">

<violation number="1" location="samples/client/others/go/array-default-value/client.go:490">
P0: Critical bug: File is closed before being read. The `io.Copy(part, file)` will fail because the file was already closed. The file should be closed after copying, using `defer file.Close()` right after opening.</violation>
</file>

<file name="samples/client/others/go/array-default-value/api_default.go">

<violation number="1" location="samples/client/others/go/array-default-value/api_default.go:88">
P2: Inconsistent indentation in the else block - uses spaces instead of tabs. Go convention (enforced by `gofmt`) requires tabs for indentation. The template generating this code (`modules/openapi-generator/src/main/resources/go/api.mustache`) should use tabs (`\t`) instead of spaces for the default value block.</violation>
</file>

---

Since this is your first cubic review, here's how it works:

• cubic automatically reviews your code and comments on bugs and improvements
• Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
• Ask questions if you need clarification on any suggestion

_{Reply to cubic to teach it or ask questions. Tag @cubic-dev-ai to re-run a review.}

[cubic-dev-ai]

P0: Critical bug: File is closed before being read. The io.Copy(part, file) will fail because the file was already closed. The file should be closed after copying, using defer file.Close() right after opening.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/others/go/array-default-value/client.go, line 490:

<comment>Critical bug: File is closed before being read. The `io.Copy(part, file)` will fail because the file was already closed. The file should be closed after copying, using `defer file.Close()` right after opening.</comment>

<file context>
@@ -0,0 +1,656 @@
+	if err != nil {
+		return err
+	}
+	err = file.Close()
+	if err != nil {
+		return err
</file context>

[cubic-dev-ai]

P2: Inconsistent indentation in the else block - uses spaces instead of tabs. Go convention (enforced by gofmt) requires tabs for indentation. The template generating this code (modules/openapi-generator/src/main/resources/go/api.mustache) should use tabs (\t) instead of spaces for the default value block.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/others/go/array-default-value/api_default.go, line 88:

<comment>Inconsistent indentation in the else block - uses spaces instead of tabs. Go convention (enforced by `gofmt`) requires tabs for indentation. The template generating this code (`modules/openapi-generator/src/main/resources/go/api.mustache`) should use tabs (`\t`) instead of spaces for the default value block.</comment>

<file context>
@@ -0,0 +1,144 @@
+			parameterAddToHeaderOrQuery(localVarQueryParams, &quot;sort&quot;, t, &quot;form&quot;, &quot;multi&quot;)
+		}
+	} else {
+        var defaultValue []string = []string{&quot;updatedAt&quot;}
+        parameterAddToHeaderOrQuery(localVarQueryParams, &quot;sort&quot;, defaultValue, &quot;form&quot;, &quot;multi&quot;)
+        r.sort = &amp;defaultValue
</file context>

[titaneric]

Rather than introducing a new schema and generating a sample for that, I would suggest expanding the one that already exist for the issue, and make sure it also contains an array with only a single item. I would also extend the unit test to make it verify that a lone item is correctly generated.

@Mattias-Sehlstedt , I would take a look at the issue you mentioned. However, I am not quite sure why we need to make sure it also contains an array . In the generated go sdk, why don't we just generate the default values as the schema implies.

[Mattias-Sehlstedt]

@titaneric, I have adjusted the wording so that schema says specification. What I mean is that the existing sample specification that I referenced (which was introduced when default values for arrays were added) should be extended to also have a query parameter with an array that has a single value in its default definition. This so that that specification also includes a showcase for the scenario that you are fixing.

Currently it only has:

- name: "arrayparam"
  in: query
  schema:
    type: array
    default: ["test1", "test2", 1]

while it should preferably also have something like:

- name: "arrayparam2"
  in: query
  schema:
    type: array
    items:
      type: string
    default: ["test1"]

so that it also showcases the scenario that you have fixed.

Currently spec-with-array-default-value.yaml and issue_21077.yaml try to show the same thing (default values for arrays), and I believe it is unnecessary to have two samples that show the exact same thing, just with an extremely small difference. In my opinion it would be easier if they were joined.

[titaneric]

@Mattias-Sehlstedt , okay I got it. I would try to update the existing testcase instead of adding a new one.

Updated:

Update existing test case in 27fcbb2 and remove the new introduced spec.