Connexion: Handle nested paths in swagger definitions

[adelosa]

Connexion does not seem to handle nested paths.
Tried loading the example Uber swagger spec provided at editor.swagger.io.
Using RestyResolver to perform view resolution with 'api' as module.
Added placeholder views for all path actions.
Python 3.5.1 on MacOS
The spec has the following paths defined:

• /estimates/time
• /estimates/price
  Fails with Attribute error: module 'api.estimates' has no attribute 'get'
  There is no get action defined at the /estimates path although this seems to be what the resolver is looking for.
  I would expect it to use /api/estimates/time.py with get method and /api/estimates/price.py with get method instead.

[hjacobs]

@adelosa I think this is not a problem with Connexion in general, but with RestyResolver (which is part of Connexion :smirk:)..

[dfeinzeig]

@adelosa , /estimates/ and /estimates/1/ would both live in /api/estimates.py as search() and get(). is it weird for nested paths to live in directories under /api when the root doesn't? what if the methods for /time and /price lived in /api/estimates.py, since that is the module that handles the estimates resource? they could possibly be called get_price() and get_time(). thoughts?

[adelosa]

@hjacobs agree, its the RestyResolver thats the issue.
@dfeinzeig I'm fairly new to the whole swagger thing and not sure how often this actually happens or how deeply nested a resource could or should be but I do like the idea of auto generated method locations..
Is it possible in a swagger spec to have paths that could be 2, 3, 4 levels and if so, would a get_level1_level2_level3 type function be better than nested modules.. I personally think the nested modules works better from a structure perspective. The path nesting aligns to the directory nesting in your source. If you go the method path you could end up with long method names and harder to locate the methods in your modules.. Its probably bad design to have such nesting, but if Swagger supports it....

I think at a minimum it needs to be supported or at least an exception thrown if an operationId is not provided in such cases and the hint to add an operationId to such paths ..
I think both options are workable and would be happy with either.

If this is not a high priority for the project, I would be happy to try and put a pull request together if you tell me which way you want to go...

[rafaelcaricio]

@adelosa

I think a more clean way would be to have it with nested modules. And if someone does not agree and wants to add "nested method naming" (get_level1_level2_level3) they can send a new PR with a option to specify that they want nested method naming.

Please send your PR and I will be happy to review and get it merged.

What do you think?

[dfeinzeig]

@rafaelcaricio , sorry, I didn't mean to sound argumentative. I've been thinking about this problem too and was curious to hear other people's thoughts on the best approach. When I have time I'll put together a PR with my thoughts so we can discuss it.

[rafaelcaricio]

@dfeinzeig No problem! Thank you!

In my opinion both approaches are valid. It is hard so set a general rule that will work for everyone, so I suggest we could be as flexible as possible.

[LappleApple]

Hi @dfeinzeig, wondering if you've given more thought to the PR you suggested above (April 7). :)

[kler]

Perhaps something like this would solve it?
I've just created it, and it works. But please note I haven't tested it more than 5 minutes...

https://gist.github.com/kler/56710fdea361afa79cc2314bd6eb0e7e

[robkooper]

I created a resolver that give then name will strip out all the parameters (encapsulated in braces) and take the first argument as the module name, and concatenate the rest together. So /estimates/time becomes time_search() in estimates.py. Code is located at https://github.com/robkooper/openapi-flask/blob/master/resolver.py

If interested I will try and create a pull request to the current base.

[logi]

Interesting. I have an even more general resolver which allows either nesting or for the sub-paths to be included in the function name. This is a passing test for a failed lookup, but the exception shows the paths that are tried:

def test_CalmResolver_i_tried():
    resolver = CalmResolver('root')
    operation = Namespace(
        method='GET',
        path='/deep/{depth}/path/to/{target}/thing',
    )
    ex_message = (
        'Could not find a function for GET /deep/{depth}/path/to/{target}/thing. I tried '
        'root.deep.path.to.thing.get, '
        'root.deep.path.to.get_thing, '
        'root.deep.path.get_to_thing, '
        'root.deep.get_path_to_thing, '
        'root.get_deep_path_to_thing'
    )
    with pytest.raises(ResolverError, match=re.escape(ex_message)):
        resolver.resolve(operation)

I've been using this for ~3 years now and was about to start making a PR to finally upstream it, but https://github.com/zalando/connexion/pull/1265 has now been merged which implements a part of it.

Is there any interest in this alternative more general resolver?

[adnaanbheda]

@logi Hey, can you share your implementation of the Resolver? I'm facing this issue currently !

[logi]

@adnaanbheda Just pasting in the code without context. I removed the Handler super-class since we didn't use explicit operation_ids and it just complicated the code, but you may want to add that back in.

class CalmResolver(object):

    def __init__(self, root_module_name: str, collection_endpoint_name: str = 'search'):

        """
        Generalisation of RestyResolver which allows end-points to be grouped into modules and sub-modules.
        This function returns a configured function for passing to `connection.Resolver()` or `connexion.App.add_api()'
        """
        self.root_module_name = root_module_name
        self.collection_endpoint_name = collection_endpoint_name

    def resolve(self, operation):
        """
        Resolves the operationId using REST semantics

        :type operation: connexion.operation.Operation
        """
        without_vars, variable_count = re.subn(
            '{[^{\\}]*\\}',
            '',
            operation.path
        )
        operation_name = operation.method.lower()
        if operation_name == 'get' and variable_count == 0:
            operation_name = self.collection_endpoint_name

        parts = re.split(
            '[^\\wd]+',
            without_vars
        )
        while parts and not parts[0]:
            parts = parts[1:]
        while parts and not parts[-1]:
            parts = parts[:-1]

        parts.insert(0, self.root_module_name)

        tried = []
        for n in range(len(parts), 0, -1):
            module_name = '.'.join(parts[:n])
            function_part = '_'.join(parts[n:])
            function_name = operation_name + '_' + function_part if function_part else operation_name
            operation_id = module_name + '.' + function_name
            tried.append(operation_id)
            try:
                module = import_module(module_name)
                function = getattr(module, function_name)
                logging.debug('%s %s → %s', operation.method.upper(), operation.path, operation_id)
                return Resolution(function, operation_id)
            except:
                pass  # Module does not exist. Try one higher

        # none of the modules were found. Return the deepest one for the error message
        raise ResolverError('Could not find a function for {} {}. I tried {} '.format(
            operation.method, operation.path,
            ', '.join(tried),
        ))