[go-experimental] Fix multiple go compilation errors and enable go integration test

[sebastien-rosset]

Fix issues listed in #5074

The generated go-experimental code has multiple issues that cause syntax errors:

Use but does not define the "NullableMap" type
Refer to the "NullableTime.Time" type, but the type is actually "NullableTime"
Import but does not use the "time" package
Refer to the Nullable[]map[string]interface{}, which is not a valid golang type.
Some "const" values are defined twice with the same identifier, once for the enum value, and once for the default value.
Other issues found:

Missing pom.xml files in the go and go-experimental directories.

PR checklist

• Read the contribution guidelines.
• If contributing template-only or documentation-only changes which will change sample output, build the project before.
• Run the shell script(s) under ./bin/ (or Windows batch scripts under.\bin\windows) to update Petstore samples related to your fix. This is important, as CI jobs will verify all generator outputs of your HEAD commit, and these must match the expectations made by your contribution. You only need to run ./bin/{LANG}-petstore.sh, ./bin/openapi3/{LANG}-petstore.sh if updating the code or mustache templates for a language ({LANG}) (e.g. php, ruby, python, etc).
• File the PR against the correct branch: master, 4.3.x, 5.0.x. Default: master.
• Copy the technical committee to review the pull request if your PR is targeting a particular programming language.

[sebastien-rosset]

@antihax @bvwells @grokify @kemokemo @bkabrda

[bkabrda]

So in general I'm 👍 for all of these changes (extra kudos for adding all of the explanatory comments!) except the one with Nullable maps/slices. The change I'd like to see there is actually fixing the feature, IOW define a nullable type specifically for that property, so it would actually be possible to serialize it as explicit null. Would that make sense? Thanks!

[sebastien-rosset]

So in general I'm 👍 for all of these changes (extra kudos for adding all of the explanatory comments!) except the one with Nullable maps/slices. The change I'd like to see there is actually fixing the feature, IOW define a nullable type specifically for that property, so it would actually be possible to serialize it as explicit null. Would that make sense? Thanks!

What are you trying to achieve with a NullableMap/Slice? See https://play.golang.org/p/Aw0ZWYkmWPS

Can we do this in a separate PR? I think this will require some discussion because we would probably have to generate NullableMaps for each type of map. Here I'm just trying to fix the breakage.

[sebastien-rosset]

How about using NullableTime for the OAS "time" type instead of plain "string" type? At least specifically for go-experimental, because this would be a breaking change for "go". Golang has the time.Time.Truncate() function, which could help to perform arithmetic with dates.

[sebastien-rosset]

Why not invoke the following code in go-experimental processOpts() instead of modifying the dataType here? What is the reason for changing the type in postProcessModels()?

typeMapping.put("DateTime", "NullableTime");

[wing328]

Agreed with using typeMapping to directly map the type instead of using the code above.

[sebastien-rosset]

So interestingly, adding typeMapping.put("DateTime", "NullableTime") does not produce the same output for the samples. I wonder if that was done on purpose. When the dataType is set in postProcessModels(), it does not have the same effect as when typeMapping is invoked in processOpts(). Is that done on purpose?

[bkabrda]

We only want to represent the DateTime object as NullableTime if it's marked as nullable in the spec. I don't think we should use it in all cases.

[sebastien-rosset]

For example, at https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/resources/go-experimental/api.mustache#L26

type api{{operationId}}Request struct {
	ctx _context.Context
	apiService *{{classname}}Service{{#allParams}}
	{{paramName}} {{^isPathParam}}*{{/isPathParam}}{{{dataType}}}{{/allParams}}
}

With "typeMapping", this will produce NullableTime.
With the dataType changed in processOpts(), this will produce *time.Time.

I propose that we merge this PR as is, and then work on enhancing/fixing the typeMapping in a separate PR.

[bkabrda]

Yeah, I agree this PR is ok as-is, let's keep more changes for followup PRs.

[sebastien-rosset]

We only want to represent the DateTime object as NullableTime if it's marked as nullable in the spec. I don't think we should use it in all cases.

Thanks for the explanation. I have added code comments to explain.

[sebastien-rosset]

Yeah, I agree this PR is ok as-is, let's keep more changes for followup PRs.

Thanks. Can you approve the PR then? Approving in the text does not help much. BTW, for some reason I am unable to add you as a reviewer.

[bkabrda]

Approved. Sorry, I keep forgetting doing the official review.

[sebastien-rosset]

Should I copy all of the existing unit test files from samples/client/petstore/go-experimental?

[wing328]

Yes please

[sebastien-rosset]

done, running the unit tests to verify.

[bkabrda]

What are you trying to achieve with a NullableMap/Slice? See https://play.golang.org/p/Aw0ZWYkmWPS

The aim with nullables is to be able to achieve these three different outcomes (this would apply to a property that is nullable and not required):

• The value is not present at all in the serialized JSON
• The value is present in the serialized JSON as null
• The value is present in the serialized JSON and has non-null value

Due to the fact that this doesn't even work now, I am ok with leaving that to a different PR.