Skip to content

[ASP.NET Core] Added support for Operation EXAMPLE value - #7091

Merged
wing328 merged 2 commits into
swagger-api:masterfrom
knom:master
Dec 12, 2017
Merged

[ASP.NET Core] Added support for Operation EXAMPLE value#7091
wing328 merged 2 commits into
swagger-api:masterfrom
knom:master

Conversation

@knom

@knom knom commented Dec 1, 2017

Copy link
Copy Markdown
Contributor

RE: Issue #7092, #5423
@mandrean @jimschubert
Added support for Operation EXAMPLE value in ASP.NET Core Generator.

Now exampleJson is currectly generated.

{
// Replace " with \", \r, \n with \\r, \\n
String val = entry.getValue().replace("\"", "\\\"")
.replace("\r","\\r")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a few issues with this approach that I see…

  1. This will replace unexpected results. For instance, what if my example was:
     { 
         "example_commands": [
             "\run", "\nip", "\yap", "\rollover"
         ],
         "fast_commands": [
             "\\run", "\\rollover"
          ],
         "omg_slow_down_puppy": [
              "\\\run", "\\\rollover"
          ],
         "search_command" : "\\"
     }
    
  2. This will lead to failed compilation if we don't escape other things like double-quotes and format placeholders like {0}, {0,15}, {0:t}, etc.
  3. afiak this will result in multiline strings, regardless of how many times you escape these characters. I could be wrong on that, though.

The amount of effort to resolve this is, I don't think, worth the benefit.

Some of this could be resolved by replacing only on boundaries, for example a regex like \b\\r\b which would catch probably most cases outside of text occurrences:

this\r"matches
this\rdoesn't

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great suggestions @jimschubert!
On the double quotes.. I do replace these as well..

I'm not that fluent in Java..
How would I rewrite the code to do regex replace instead?

Thanks!

@wing328

wing328 commented Dec 2, 2017

Copy link
Copy Markdown
Contributor

Thanks for the PR but your commit (as shown in the Commits tab) is not linked to your Github account, which means this PR won't count as your contribution in https://github.com/swagger-api/swagger-codegen/graphs/contributors.

Let me know if you need help fixing it.

Ref: https://github.com/swagger-api/swagger-codegen/wiki/FAQ#how-can-i-update-commits-that-are-not-linked-to-my-github-account

@wing328 wing328 added this to the v2.3.0 milestone Dec 2, 2017
@knom
knom force-pushed the master branch 2 times, most recently from f5e2b65 to 3a99c43 Compare December 2, 2017 16:38
@wing328

wing328 commented Dec 11, 2017

Copy link
Copy Markdown
Contributor

@knom seems like the rebase failed as the PR now contains commits not authored by you.

I'll create another PR by cherry-picking the commits owned by you.

@knom
knom force-pushed the master branch 2 times, most recently from fe09b46 to b9048d3 Compare December 12, 2017 10:27
// return StatusCode(400);

string exampleJson = null;
exampleJson = "<Pet>\n <id>123456789</id>\n <name>doggie</name>\n <photoUrls>\n <photoUrls>aeiou</photoUrls>\n </photoUrls>\n <tags>\n </tags>\n <status>aeiou</status>\n</Pet>";

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any way to ONLY pick JSON examples here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably a hack, but iterate responses in the generator code and remove them if they don't start with a "{". If you go that route, is suggest logging a warning any time an example is dropped by the generator in this way.

Mustache doesn't really have logic. It does allow custom filters, so maybe we could write a filter that will only write a value of it is valid json. An issue with that approach is that you can't pick "the first json example* like you could in The generator processing.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was a discussion to add an isJson tag before but no one seems to have time to implement that yet.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why a tag rather than Mustache filter? just curious.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh. I get what you're saying. It would be weird to use a tag to get a single json object from ab list of mixed possible types, I think.

@knom

knom commented Dec 12, 2017

Copy link
Copy Markdown
Contributor Author

@wing328 I've just redone the REBASE.. now it should work!

@wing328
wing328 merged commit 9ca6395 into swagger-api:master Dec 12, 2017
@wing328 wing328 changed the title Added support for Operation EXAMPLE value in ASP.NET Core Generator. [ASP.NET Core] Added support for Operation EXAMPLE value Dec 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants