pavlov99 / json-rpc

🔁 JSON-RPC 1/2 transport implementation. Supports python 2/3 and pypy.
http://json-rpc.readthedocs.org
MIT License
464 stars 101 forks source link

misc updates #95

Open wonderbeyond opened 6 years ago

wonderbeyond commented 6 years ago

Some misc updates when I apply json-rpc to latest two projects. Just for your review.

$ tox -e py27,py35,py36 
  py27: commands succeeded
  py35: commands succeeded
  py36: commands succeeded
  congratulations :)

Some use cases from my real projects

from awesome.json import JSONEncoder
jsonrpc_api = JSONRPCAPI(json_encoder=JSONEncoder)
from awesome.auth.decorators import login_required
dispatcher.register_decorator(login_required)
@dispatcher.add_class
class CellAPI(APISet):
    rpc_exports = ['detail', 'create', 'update']
    rpc_method_prefix = 'Cell'
    ### ...
pavlov99 commented 5 years ago

Hi, @wonderbeyond

Thank you for the PR and sorry for coming back late.

After closer inspection of your code, I've noticed that there are quite many features there. Some of them I completely support, others - not.

Using custom json serializer is convenient, I'm on the same page.

Three other features I'd like to discuss:

  1. using decorators such as login_required is convenient, however, they could be added outside of dispatcher:
    @dispatcher.add_method
    @login_required
    def my_handler():
    pass
  2. rpc_method_prefix already exists in method registration:
    @dispatcher.add_method(prefix='Cell')
    def my_method():
    pass

    There is no support for prefix in case of class yet but this would be a proper place for prefix handler:

    @dispatcher.add_class(prefix='Cell')
    class CellAPI:
    pass
  3. Could you please elaborate on your use case with rpc_exports? At the moment all of the class methods (except methods starting with '_') are added to dispatcher.
pavlov99 commented 5 years ago

One more thing: dispatcher should not know about any hooks. They could be implemented in manager.

wonderbeyond commented 5 years ago

using decorators such as login_required is convenient, however, they could be added outside of dispatcher:

However I want a way for global decorator for all rpc handlers.

rpc_method_prefix already exists in method registration:

I see no prefix parameter of Dispatcher.add_method: https://github.com/pavlov99/json-rpc/blob/master/jsonrpc/dispatcher.py#L67

There is no support for prefix in case of class yet but this would be a proper place for prefix handler:

Yes, I think your style is better.

Could you please elaborate on your use case with rpcexports? At the moment all of the class methods (except methods starting with '') are added to dispatcher.

In my practice, I define a base class naming APISet:

image

This class acts as a template, provides common CURD methods, which a subclass has no need to override in general. Give the the right Model attr, and it just works.

image

For security considerations, I only export detail and list (i.e. readonly methods) as default. The subclass can make specific choices. It can also override or define new methods and make them exportable.

wonderbeyond commented 5 years ago

@pavlov99

One more thing: dispatcher should not know about any hooks. They could be implemented in manager.

I've tried, but I found JSONRPCResponseManager was intended to be a static class, which only has classmethods.

Code sample from README:

@Request.application
def application(request):
    # Dispatcher is dictionary {<method_name>: callable}
    dispatcher["echo"] = lambda s: s
    dispatcher["add"] = lambda a, b: a + b

    response = JSONRPCResponseManager.handle(
        request.data, dispatcher)
    return Response(response.json, mimetype='application/json')

I found no elegant way to bind extra data to the manager. Can we make some change?

S1983tt commented 1 year ago

4c13c487676043c27425abc52f87a502eb57a8a9

S1983tt commented 1 year ago

4c13c487676043c27425abc52f87a502eb57a8a9