Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Examples can no longer be formatted without formatting all JSON output #181

Open
Blackbaud-ChristiSchneider opened this issue Apr 13, 2021 · 6 comments

Comments

@Blackbaud-ChristiSchneider

Previously, examples were able to be formatted separately from formatting the JSON output.

This change removed the JsonSerializerSettings which would allow you to set JsonSerializerSettings.Formatting to whatever you like.

I was able to get examples to format in the swagger document by including the following in my Startup.ConfigureServices:

                .AddNewtonsoftJson(options =>
                {
                    options.SerializerSettings.Formatting = Formatting.Indented;
                });

However, this means that all responses from my endpoints are now formatted, which increases the size of the response as well as breaking any tests that were looking for specific spacing.

@mattfrear
Copy link
Owner

Lots of folks were asking me to remove the Newtonsoft dependency from this library. This was a breaking change which is why I bumped the version number from 5.x to 6.0.

I'm confused though - why do you need to specify Formatting.Indented? I don't have that specified, and my examples look fine.

Here's what I have:

            services
                .AddNewtonsoftJson(options =>
                {
                    options.SerializerSettings.DateTimeZoneHandling = DateTimeZoneHandling.Utc;
                })

Results in nicely indented examples:
image

I suggest you remove options.SerializerSettings.Formatting = Formatting.Indented; and remove all the \t and \n from your tests. That shouldn't be a big job.

If a package does a major version upgrade you can expect there to be some breakages.

@Blackbaud-ChristiSchneider
Copy link
Author

The examples are formatted in the Swagger UI but not in the swagger.json, while they were formatted previously.

Before:

{
  ...
        "responses": {
          "200": {
            "description": "Success",
            "content": {
              "application/json": {
                "schema": {
                  "$ref": "#/components/schemas/Example"
                },
                "example": {
  "names": [
    "John",
    "Paul",
    "George",
    "Ringo"
  ],
  "primary_contact": false
}
              }
            }
          },
  ...
}

After:

{
  ...
        "responses": {
          "200": {
            "description": "Success",
            "content": {
              "application/json": {
                "schema": {
                  "$ref": "#/components/schemas/Example"
                },
                "example": {"names":["John","Paul","George","Ringo"],"primary_contact":false}
              }
            }
          },
  ...
}

The reason why this matters is that, if the examples are long (and many of them are much longer than the sample I have here), it is very difficult to see what has changed when comparing two documents.

There's a workaround, to indent all JSON serialization, but this also affects the JSON responses from operations. This means that the size of the response body is larger than it needs to be, which is not an acceptable trade off when the reasoning is readability in developer documentation.

My issue isn't really that this changed without any announcement, I was pointing out that it appears to have been broken unintentionally and I would like it if this option were restored. I'm happy to contribute a change for this if you'll accept it.

@Blackbaud-ChristiSchneider
Copy link
Author

I suggest you remove options.SerializerSettings.Formatting = Formatting.Indented; and remove all the \t and \n from your tests. That shouldn't be a big job.

I was suggesting that the workaround is the opposite, but it actually is a big job - we use this library in all of our .NET Core services so changing this for all of them will potentially result in thousands of unit test failures, and expecting every team to turn this on individually in each of the hundreds of services is not realistic. I think that's a fine argument if the consumers are individuals or small teams but there are large organizations using this library.

@mattfrear mattfrear reopened this Apr 15, 2021
@mattfrear
Copy link
Owner

mattfrear commented Apr 15, 2021

Background: originally one of the use cases of being able to set the JsonSerializer settings was so that the examples looked nice in SwaggerUI. But SwaggerUI indents them nicely now (it didn't use to). I don't think I paid much attention to what they looked like in the swagger.json, as that's hidden for most users.

I can think of a whole bunch of workarounds for your scenario (e.g. your tests shouldn't care about whitespace IMO) but that is beside the point.

The PR you cited made the examples use the same serializer as responses emitted from the API, which at the time made sense to me.

But now that I think about it, I think the examples should instead have the same formatting as the rest of the swagger.json document.

This differs from the example you have given above, e.g.
Not

{
  ...
        "responses": {
          "200": {
            "description": "Success",
            "content": {
              "application/json": {
                "schema": {
                  "$ref": "#/components/schemas/Example"
                },
                "example": {"names":["John","Paul","George","Ringo"],"primary_contact":false}
              }
            }
          },
  ...
}

Or

{
  ...
        "responses": {
          "200": {
            "description": "Success",
            "content": {
              "application/json": {
                "schema": {
                  "$ref": "#/components/schemas/Example"
                },
                "example": {
  "names": [
    "John",
    "Paul",
    "George",
    "Ringo"
  ],
  "primary_contact": false
}
              }
            }
          },
  ...
}

But

{
    "responses": {
        "200": {
            "description": "Success",
            "content": {
                "application/json": {
                    "schema": {
                        "$ref": "#/components/schemas/Example"
                    },
                    "example": {
                        "names": [
                            "John",
                            "Paul",
                            "George",
                            "Ringo"
                        ],
                        "primary_contact": false
                    }
                }
            }
        }
    }
}

However, I don't think this is possible with the current implementation.

@mattfrear
Copy link
Owner

mattfrear commented Apr 15, 2021

I was pointing out that it appears to have been broken unintentionally and I would like it if this option were restored.

It was intentional.

I'm happy to contribute a change for this if you'll accept it.

If you can implement it without depending on Newtonsoft I will accept it.

@Blackbaud-ChristiSchneider
Copy link
Author

Agreed that it would be nice to format it with the proper spacing matching the full document, but I think that may be complicated. I believe the reason the spacing was the way it was is that the library just plops the serialized example string in where it belongs as-is rather than injecting it into the document prior to the document bring serialized. I am actually not sure at which point that serialization happens in the Swashbuckle library and what it is that makes it format in the first place, so I'll be digging into that to see if this can be tied in there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants