Skip to content

Conversation

lgaida
Copy link
Contributor

@lgaida lgaida commented Jan 20, 2020

See #122

@coveralls
Copy link

coveralls commented Jan 20, 2020

Coverage Status

Coverage increased (+0.05%) to 92.062% when pulling fc31b92 on lgaida:master into 519bcf6 on ePages-de:master.

@mduesterhoeft
Copy link
Contributor

@lgaida thanks for your contribution.
I have to admit that I shy away a little bit from adding xml support. We never were in need for this feature and I have not much knowledge how spring-restdocs and openapi support xml. So I would rather not add this feature. @ozscheyge what do you think?

@ozscheyge
Copy link
Contributor

Yep, indeed we didn't consider XML capabilities included in Spring restdocs/OpenAPI standards, but focused on our use-case which is HTTP+JSON.

Iirc, we have a couple of assumptions built on jsonpaths in the code.
Having had a brief look on the openapi3 part @lgaida linked, we would need way to populate the xml object with information.

Mapping jsonpath to xpath might seem like a quick win, but first it's error-prone (couldn't find a library ad-hoc) and second you'd still lose information.

@lgaida
Copy link
Contributor Author

lgaida commented Jan 23, 2020

Thanks for your replies :)
It is totally understandable that you created this tool with your own use-cases in mind.
 In fact i’m thankful that you are sharing this tool as an open-source project. Http+Json is the most common use-case, so you’re already helping a lot of people out with this.



I just wanted to start a discussion here not leaving possibilities out since I see how xml support could enhance this tool.
 But as previously pointed out, a clean solution to this problem requires a lot of work. 
I may dig a little bit deeper into the whole xpath/jsonpath topic to verify whether it is actually possible to do a clean conversion between the two without information loss.

For now I’m totally fine with closing this PR and the issue itself.
 How about adding a notice to the REAME and explicitly point out that xml is currently not and most likely will never be supported ?

lgaida pushed a commit to lgaida/restdocs-api-spec that referenced this pull request Jan 23, 2020
@lgaida lgaida closed this Jan 23, 2020
ozscheyge added a commit that referenced this pull request Jan 23, 2020
* Add note about non-json content-types to README.md, see #122 #123

* Adopt suggestions to README.md

Co-Authored-By: Oliver <ozscheyge@users.noreply.github.com>

Co-authored-by: Oliver <ozscheyge@users.noreply.github.com>
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

Successfully merging this pull request may close these issues.

4 participants