swagger-api / swagger-core

Examples and server integrations for generating the Swagger API Specification, which enables easy access to your REST API
http://swagger.io
Apache License 2.0
7.38k stars 2.18k forks source link

Processing a self-referencing subResource leads to a series of recursive scans #1424

Closed elakito closed 9 years ago

elakito commented 9 years ago

The problem is described here. https://groups.google.com/forum/#!topic/swagger-swaggersocket/v_VTJgMm3PM

I'll attach a PR with a proposed solution which caches classes at the Reader instance.

Alternatively, a slightly more complex solution would be to create a private read method that looks like the current recursively invoked read method but with an extra argument of a map that can be used to detect the self-referencing loop during its recursive calls. The original read method can call this private read method with an empty map object.

Which one to take depends on how the reader is used and how its expected behavior should be. And I would like to ask for your feedback.

regards, aki

webron commented 9 years ago

Thanks for this, but setting aside the solution, what would be the expected output? Wouldn't you end up with something like /resource/{*subresource}?

elakito commented 9 years ago

Hi Ron, that is a good question. I noticed this scanner issue when using some test resources included in cxf's system tests.

In the case of its wadl generation, the wadl initially has an resource entry for this subresource unexpanded. And every time when I make a call a step deeper, this entry gets expanded. http://pastebin.com/5sCbj24y

I don't think this expanding of the path on demand in this way is nice.

So I don't know how swagger should show recursive sub-resources.

I'll revert the initial PR for now because I don't think it is right to bookkeep sub-resources per reader anyway.

regards, aki

webron commented 9 years ago

Yeah, that's a recursive definition, which is generally ok, but is not supported in Swagger (for now), so I don't think we can add support for in the reader. That said, perhaps we should find a nicer way to deal with it than go into an infinite loop.

elakito commented 9 years ago

Yes. I agree with you. We should separate the two things and get the infinite loop thing fixed for now.

So for fixing the infinite loop, we need to cache the resources seen during the recursive traversal and just stop going deeper. I think we should make a private recursive read method with a resource map. With this approach, we won't stop and come back too early, which could happen when we cache the resources at the reader instance.

I'll think about it. Let me know how you think. thanks. regards, aki p.s. for the displaying part, if swagger can expand the node interactively over its UI, that would be cool. ;-)

webron commented 9 years ago

well, we don't support the recrusiveness at all, so no need to expand anything ;)

elakito commented 9 years ago

Hi Ron, I attached the new PR for the infinite loop issue, as mentioned earlier. regards, aki

webron commented 9 years ago

Thanks for all the effort, @elakito, it's really great. I'll do my best to review it tomorrow.

elakito commented 9 years ago

Thanks Ron and Tony, I have verified the merged master.