Repository: go-swagger/go-swagger
Pull request #2241 (2020-03-10 21:31:59)

Basic support for XML model generation
This is related to #183 and #353. It enables the addition of xml struct tags to generated fields in order to be able to unmarshal and (theoretically) marshal XML.

Now, XML struct tags are added everywhere (not only for fields where xml.name is explicitly set, like before). I don't think there is a straightforward way to ensure that a given schema definition will never be used to transmit XML? Consequently, this change breaks lots of tests, which I made green
replacing `` `json:\\"([^,"]+)([^"]*)\\"` `` with `` `json:\\"$1$2\\" xml:\\"$1\\"` ``
in the right (TM) places.

Searching through the codebase with the above regex yields more results that I haven't replaced, particularly `json:\"-\"`. This is because I left the insertion of the XMLName as is, i.e. it is only used in structfield.gotmpl and not in schemabody.gotmpl and allofserializer.gotmpl. As far as I can tell, this means that currently no inline nested schema definitions can be used with XML.

Before changing this, one should IMHO think about refactoring the templates to a) remove redundancy of json struct tag insertion and b) possibly make json and xml struct tag insertion work the same way, i.e. either both concatenating the tags beforehand or both constructing them in the templates. For the same reason, omitempty is currently not set for XML.

I added a simple test for the model generation with XML attributes and custom element names. Retrieving an XML response works for me now. I did not try sending an XML request body.

I'm not at all familiar with the codebase, so please excuse major bloopers and review thoroughly, if it's even worth the effort for such a hacky feature.

**TL;DR:**
added/improved support for:
* xml struct tag generation without setting explicit swagger xml.name
* setting custom swagger xml.name
* setting swagger xml.attribute
* receiving XML
* \+ sending XML
* \+ omitempty

missing support:
* inline nested schema definitions
* ~sending XML (?)~
* namespaces
* wrapping arrays
* ~omitempty~
* additional properties
* ...

See also: https://swagger.io/docs/specification/data-models/representing-xml/

nubenum: For future reference and since it is not (yet) documented anywhere, my setup to consume an XML API:
```
import (
	"github.com/go-openapi/runtime"
	httptransport "github.com/go-openapi/runtime/client"
        "github.com/go-openapi/strfmt"
	apiclient "[...]/client"
	"os"
)
r := httptransport.New(apiclient.DefaultHost, apiclient.DefaultBasePath, apiclient.DefaultSchemes)
r.DefaultAuthentication = httptransport.BearerToken(os.Getenv("ACCESS_TOKEN"))
r.DefaultMediaType = runtime.XMLMime
r.Consumers = map[string]runtime.Consumer{
	runtime.XMLMime: runtime.XMLConsumer(),
}
r.Producers = map[string]runtime.Producer{
	"application/xhtml+xml": runtime.XMLProducer(),
}
client := apiclient.New(r, strfmt.Default)
res, err := client.Operations.MyGreatEndpoint()
```
casualjim: the integration test fails: 
```
--- FAIL: TestCodegen/fixtures/codegen/xml-model.yml (0.00s)
        --- FAIL: TestCodegen/fixtures/codegen/xml-model.yml/expand_spec_run (0.48s)
        --- FAIL: TestCodegen/fixtures/codegen/xml-model.yml/minimal_flatten_spec_run (0.54s)
```
you can run those tests locally with:
```sh
go test -v -timeout 20m -parallel 3 hack/codegen_nonreg_test.go -args -fixture-file codegen-fixtures.yaml -skip-models -skip-full-flatten
```
codecov[bot]: # [Codecov](https://codecov.io/gh/go-swagger/go-swagger/pull/2241?src=pr&el=h1) Report
> Merging [#2241](https://codecov.io/gh/go-swagger/go-swagger/pull/2241?src=pr&el=desc) into [master](https://codecov.io/gh/go-swagger/go-swagger/commit/4aad3a5f78b8f99a7752299d3c51f28d5f1e40a7?src=pr&el=desc) will **increase** coverage by `0.01%`.
> The diff coverage is `92.85%`.

[![Impacted file tree graph](https://codecov.io/gh/go-swagger/go-swagger/pull/2241/graphs/tree.svg?width=650&token=aQS1DepR8C&height=150&src=pr)](https://codecov.io/gh/go-swagger/go-swagger/pull/2241?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master    #2241      +/-   ##
==========================================
+ Coverage   81.21%   81.23%   +0.01%     
==========================================
  Files          50       50              
  Lines        8413     8418       +5     
==========================================
+ Hits         6833     6838       +5     
  Misses       1082     1082              
  Partials      498      498
```


| [Impacted Files](https://codecov.io/gh/go-swagger/go-swagger/pull/2241?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [generator/shared.go](https://codecov.io/gh/go-swagger/go-swagger/pull/2241/diff?src=pr&el=tree#diff-Z2VuZXJhdG9yL3NoYXJlZC5nbw==) | `88.52% <ø> (ø)` | :arrow_up: |
| [cmd/swagger/commands/generate/shared.go](https://codecov.io/gh/go-swagger/go-swagger/pull/2241/diff?src=pr&el=tree#diff-Y21kL3N3YWdnZXIvY29tbWFuZHMvZ2VuZXJhdGUvc2hhcmVkLmdv) | `79.64% <100%> (+0.36%)` | :arrow_up: |
| [generator/model.go](https://codecov.io/gh/go-swagger/go-swagger/pull/2241/diff?src=pr&el=tree#diff-Z2VuZXJhdG9yL21vZGVsLmdv) | `84.86% <91.66%> (+0.22%)` | :arrow_up: |
| [generator/structs.go](https://codecov.io/gh/go-swagger/go-swagger/pull/2241/diff?src=pr&el=tree#diff-Z2VuZXJhdG9yL3N0cnVjdHMuZ28=) | `75.89% <0%> (-1.79%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/go-swagger/go-swagger/pull/2241?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/go-swagger/go-swagger/pull/2241?src=pr&el=footer). Last update [4aad3a5...2f151df](https://codecov.io/gh/go-swagger/go-swagger/pull/2241?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

nubenum: @casualjim Yes, unfortunately I missed this. When running like this locally, quite a few other tests fail for me. Hope it'll work now. Is there any getting started documentation on how to setup the local environment so that all tests run?

@fredbi Added omitempty and enabling xml struct tags generation via --default-(consumes|produces). We'll need to add documentation for that, no matter how we're doing it.


Patch:
diff --git a/cmd/swagger/commands/generate/shared.go b/cmd/swagger/commands/generate/shared.go
index 28121548..f90fc33c 100644
--- a/cmd/swagger/commands/generate/shared.go
+++ b/cmd/swagger/commands/generate/shared.go
@@ -6,6 +6,7 @@ import (
 	"log"
 	"os"
 	"path/filepath"
+	"strings"
 
 	"github.com/go-openapi/analysis"
 	"github.com/go-openapi/swag"
@@ -94,6 +95,9 @@ type mediaOptions struct {
 func (m mediaOptions) apply(opts *generator.GenOpts) {
 	opts.DefaultProduces = m.DefaultProduces
 	opts.DefaultConsumes = m.DefaultConsumes
+
+	const xmlIdentifier = "xml"
+	opts.WithXML = strings.Contains(opts.DefaultProduces, xmlIdentifier) || strings.Contains(opts.DefaultConsumes, xmlIdentifier)
 }
 
 // WithShared adds the shared options group
diff --git a/fixtures/codegen/xml-model.yml b/fixtures/codegen/xml-model.yml
new file mode 100644
index 00000000..0a2f0cef
--- /dev/null
+++ b/fixtures/codegen/xml-model.yml
@@ -0,0 +1,56 @@
+swagger: "2.0"
+
+info:
+  version: "1.0"
+  title: "XML model with xml attributes and custom xml tag name"
+  description: ""
+
+produces:
+  - "application/xml"
+
+consumes:
+  - "application/xml"
+
+paths:
+  '/test':
+    get:
+      responses:
+        '200':
+          description: test endpoint
+          schema:
+            $ref: '#/definitions/XmlWithAttribute'
+definitions:
+  XmlWithAttribute:
+    type: object
+    required:
+      - author
+      - isPublished
+    properties:
+      id:
+        type: integer
+        xml:
+          attribute: true
+      title:
+        type: string
+        xml:
+          name: 'xml-title'
+      isPublished:
+        type: boolean
+        xml:
+          attribute: true
+          name: published
+      singleChild:
+        type: object
+        $ref: "#/definitions/XmlChild"
+      author:
+        type: string
+      children:
+        type: array
+        items:
+          $ref: "#/definitions/XmlChild"
+  XmlChild:
+    type: object
+    properties:
+      name:
+        type: string
+
diff --git a/generator/model.go b/generator/model.go
index 0acd2adf..c6f6c302 100644
--- a/generator/model.go
+++ b/generator/model.go
@@ -227,6 +227,7 @@ func makeGenDefinitionHierarchy(name, pkg, container string, schema spec.Schema,
 		IncludeValidator:           opts.IncludeValidator,
 		IncludeModel:               opts.IncludeModel,
 		StrictAdditionalProperties: opts.StrictAdditionalProperties,
+		WithXML:                    opts.WithXML,
 	}
 	if err := pg.makeGenSchema(); err != nil {
 		return nil, fmt.Errorf("could not generate schema for %s: %v", name, err)
@@ -404,6 +405,7 @@ type schemaGenContext struct {
 	IncludeValidator           bool
 	IncludeModel               bool
 	StrictAdditionalProperties bool
+	WithXML                    bool
 	Index                      int
 
 	Path         string
@@ -1575,16 +1577,21 @@ func (sg *schemaGenContext) buildAdditionalItems() error {
 	return nil
 }
 
-func (sg *schemaGenContext) buildXMLName() error {
-	if sg.Schema.XML == nil {
-		return nil
-	}
-	sg.GenSchema.XMLName = sg.Name
+func (sg *schemaGenContext) buildXMLNameWithTags() error {
+	if sg.WithXML || sg.Schema.XML != nil {
+		sg.GenSchema.XMLName = sg.Name
+
+		if sg.Schema.XML != nil {
+			if sg.Schema.XML.Name != "" {
+				sg.GenSchema.XMLName = sg.Schema.XML.Name
+			}
+			if sg.Schema.XML.Attribute {
+				sg.GenSchema.XMLName += ",attr"
+			}
+		}
 
-	if sg.Schema.XML.Name != "" {
-		sg.GenSchema.XMLName = sg.Schema.XML.Name
-		if sg.Schema.XML.Attribute {
-			sg.GenSchema.XMLName += ",attr"
+		if !sg.GenSchema.Required && sg.GenSchema.IsEmptyOmitted {
+			sg.GenSchema.XMLName += ",omitempty"
 		}
 	}
 	return nil
@@ -1934,7 +1941,7 @@ func (sg *schemaGenContext) makeGenSchema() error {
 		return err
 	}
 
-	if err := sg.buildXMLName(); err != nil {
+	if err := sg.buildXMLNameWithTags(); err != nil {
 		return err
 	}
 
diff --git a/generator/shared.go b/generator/shared.go
index 4da2cc14..e51298d2 100644
--- a/generator/shared.go
+++ b/generator/shared.go
@@ -249,6 +249,7 @@ type GenOpts struct {
 	DefaultScheme          string
 	DefaultProduces        string
 	DefaultConsumes        string
+	WithXML                bool
 	TemplateDir            string
 	Template               string
 	RegenerateConfigureAPI bool


Test patch:
diff --git a/generator/model_test.go b/generator/model_test.go
index e77d44ba..f27432fa 100644
--- a/generator/model_test.go
+++ b/generator/model_test.go
@@ -2595,3 +2595,62 @@ func TestGenModel_StrictAdditionalProperties(t *testing.T) {
 		}
 	}
 }
+
+func TestGenModel_XMLStructTags_WithXML(t *testing.T) {
+	specDoc, err := loads.Spec("../fixtures/codegen/xml-model.yml")
+	if assert.NoError(t, err) {
+		definitions := specDoc.Spec().Definitions
+		k := "XmlWithAttribute"
+		opts := opts()
+		opts.WithXML = true
+
+		genModel, err := makeGenDefinition(k, "models", definitions[k], specDoc, opts)
+		if assert.NoError(t, err) {
+			buf := bytes.NewBuffer(nil)
+			err := templates.MustGet("model").Execute(buf, genModel)
+			if assert.NoError(t, err) {
+				ct, err := opts.LanguageOpts.FormatContent("xml_with_attribute.go", buf.Bytes())
+				if assert.NoError(t, err) {
+					res := string(ct)
+					assertInCode(t, "Author *string `json:\"author\" xml:\"author\"`", res)
+					assertInCode(t, "Children []*XMLChild `json:\"children\" xml:\"children\"`", res)
+					assertInCode(t, "ID int64 `json:\"id,omitempty\" xml:\"id,attr,omitempty\"`", res)
+					assertInCode(t, "IsPublished *bool `json:\"isPublished\" xml:\"published,attr\"`", res)
+					assertInCode(t, "SingleChild *XMLChild `json:\"singleChild,omitempty\" xml:\"singleChild,omitempty\"`", res)
+					assertInCode(t, "Title string `json:\"title,omitempty\" xml:\"xml-title,omitempty\"`", res)
+				} else {
+					fmt.Println(buf.String())
+				}
+			}
+		}
+	}
+}
+
+func TestGenModel_XMLStructTags_Explicit(t *testing.T) {
+	specDoc, err := loads.Spec("../fixtures/codegen/xml-model.yml")
+	if assert.NoError(t, err) {
+		definitions := specDoc.Spec().Definitions
+		k := "XmlWithAttribute"
+		opts := opts()
+
+		genModel, err := makeGenDefinition(k, "models", definitions[k], specDoc, opts)
+		if assert.NoError(t, err) {
+			buf := bytes.NewBuffer(nil)
+			err := templates.MustGet("model").Execute(buf, genModel)
+			if assert.NoError(t, err) {
+				ct, err := opts.LanguageOpts.FormatContent("xml_with_attribute.go", buf.Bytes())
+				if assert.NoError(t, err) {
+					res := string(ct)
+					assertInCode(t, "Author *string `json:\"author\"`", res)
+					assertInCode(t, "Children []*XMLChild `json:\"children\"`", res)
+					assertInCode(t, "ID int64 `json:\"id,omitempty\" xml:\"id,attr,omitempty\"`", res)
+					assertInCode(t, "IsPublished *bool `json:\"isPublished\" xml:\"published,attr\"`", res)
+					assertInCode(t, "SingleChild *XMLChild `json:\"singleChild,omitempty\"`", res)
+					assertInCode(t, "Title string `json:\"title,omitempty\" xml:\"xml-title,omitempty\"`", res)
+				} else {
+					fmt.Println(buf.String())
+				}
+			}
+		}
+	}
+}