Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

jsonapi_rpc response structure #37

Closed
wicol opened this issue May 17, 2019 · 14 comments
Closed

jsonapi_rpc response structure #37

wicol opened this issue May 17, 2019 · 14 comments

Comments

@wicol
Copy link
Contributor

wicol commented May 17, 2019

Hello again! Here's another request regarding jsonapi_rpc :)

So I've got a working endpoint now - beautifully documented - generating an object with relations and all. Great stuff!
The next step is of course returning this object to the client in a jsonapi compliant fashion. I was hoping to simply do return obj.to_dict(), but that gives me this response:

{
  "meta": {
    "result": {
      "name": "test",
      "description": "text etc"
    }
  }
}

I realise this structure probably originates from an idea of jsonapi_rpc-methods performing arbitrary tasks and returning some information related to that task. But in some cases (like mine :)) these endpoints could easily want to return an instance of the model they belong to.

What do you think? Would be possible to give more control over the type of response to the method declaration somehow? Or just leave it up to the method completely to form its response? If the latter is preferred - what relevant internals should I be looking at for building a jsonapi compliant resource object etc?

@wicol wicol changed the title jsonapi_rpc response type jsonapi_rpc response structure May 17, 2019
@thomaxxl
Copy link
Owner

Personally, I don't like that jsonapi doesn't provide a means to perform rpc as it's strictly focused on CRUD.

Anyway, would it have been possible for you to create your objects in init, for example, adding extra arguments in **kwargs and deleting them befor instantiating the sqla model?, eg.

    def __init__(self, *args, **kwargs):

        some_arg = kwargs.pop("some_arg", None)
        if not some_arg is None: self.do_stuff() # do the rpc stuff
        super().__init__(self, *args, **kwargs)

this way you can do a POST and the result will be a regular jsonapi response.

Another option may be to return a HTTP 201 and a Location header like in https://jsonapi.org/format/#crud-creating-responses-201

It is possible to return whatever you want by wrapping your request in a decorator, for example

import json

def test_decorator(f):
    @wraps(f)
    def fwrapper(*args, **kwargs):
        result = f(*args, **kwargs)
        result.status_code = 201
        result.headers['Location'] = 'https://blah/bleh'
        result.data = json.dumps({'hoho' : 'ddd' })
        return result
    return fwrapper

class Book(SAFRSBase, db.Model):
    """
        description: Book description
    """

    __tablename__ = "Books"
    id = db.Column(db.String, primary_key=True)
    title = db.Column(db.String, default="")
    reader_id = db.Column(db.String, db.ForeignKey("People.id"))
    author_id = db.Column(db.String, db.ForeignKey("People.id"))
    publisher_id = db.Column(db.String, db.ForeignKey("Publishers.id"))
    publisher = db.relationship("Publisher", back_populates="books")
    reviews = db.relationship(
        "Review", backref="book", cascade="save-update, merge, delete, delete-orphan"
    )

    custom_decorators = [test_decorator]

    @classmethod
    @jsonapi_rpc(http_methods=["GET"])
    def get_by_name(cls, *args, **kwargs):
        """
            description : Generate and return a Thing based on name
            args:
                - name: name
                  type: string
                  default: xx
            pageable: false
        """
        return { "result" : 1}

Can you provide some code so I can see what it is you're trying to achieve exactly, and maybe I can implement it generically

@thomaxxl
Copy link
Owner

well, the example wraps the SAFRSRestMethodAPI.get, not get_by_name.
The point remains that it's possible to do whatever you want by implementing the appropriate hooks.

@thomaxxl
Copy link
Owner

There's also SAFRSFormattedResponse, like in

@classmethod
@jsonapi_rpc(http_methods=['POST'])
def startswith(cls, **kwargs):
    """
        pageable: True
        description : lookup column names
        args:
           name: ""
    """
    # from .jsonapi import SAFRSFormattedResponse, paginate, jsonapi_format_response
    result = cls
    response = SAFRSFormattedResponse()
    try:
        instances = result.query
        links, instances, count = paginate(instances)
        data = [item for item in instances]
        meta = {}
        errors = None
        response.response = jsonapi_format_response(data, meta, links, errors, count)
    except Exception as exc:
        raise GenericError("Failed to execute query {}".format(exc))

    for key, value in kwargs.items():
        column = getattr(cls, key, None)
        if not column:
            raise ValidationError('Invalid Column "{}"'.format(key))
        try:
            instances = result.query.filter(column.like(value + "%"))
            links, instances, count = paginate(instances)
            data = [item for item in instances]
            meta = {}
            errors = None
            response.response = jsonapi_format_response(data, meta, links, errors, count)

        except Exception as exc:
            raise GenericError("Failed to execute query {}".format(exc))
    return response

def startswith(cls, **kwargs):

@wicol
Copy link
Contributor Author

wicol commented May 20, 2019

Thank you for the pointers, I've experimented a bit but didn't make it all the way..

Doing hacks in __init__ feels kinda dirty, and doesn't allow for correct swagger docs.

A 201 redirect could be a pretty good workaround, but it's still a workaround - needed because I can't form the response of my custom endpoint the way I want. Still, I'm not sure how I would pull it off?

I tried using a SAFRSFormattedResponse and jsonapi_format_response like in startswith, like so:

response = SAFRSFormattedResponse()
result = jsonapi_format_response(thing, meta={}, count=1)
response.response = result
return response

...and that gives me a proper jsonapi-structure for the instance, but: the structure is still wrapped in the "meta": {"result"-structure:

{
  "meta": {
    "result": {
      "data": {
        "type": "thing",
        "attributes": {
          "name": "test"
...

Do you agree that there should be a way to control this response structure, or do you need a more complete example use case?

@thomaxxl
Copy link
Owner

Hi,

The startswith is implemented here as a POST request
http://thomaxxl.pythonanywhere.com/api#!/People/InvokePersonstartswith_0 , it does return the data[] , I'll figure out why it doesn't work for GET.

thomaxxl added a commit that referenced this issue May 21, 2019
@thomaxxl
Copy link
Owner

thomaxxl commented May 21, 2019

I commited some updates, can you check this example and see if it makes sense to you?

def my_rpc(cls, *args, **kwargs):
"""
description : Generate and return a Thing based on name
args:
my_post_body_param:
default: 5
type: int
pageable: false
parameters:
- name : my_query_string_param
"""
print(kwargs)
print(args)
result = cls
response = SAFRSFormattedResponse()
try:
instances = result.query
links, instances, count = paginate(instances)
data = [item for item in instances]
meta = {}
errors = None
response.response = jsonapi_format_response(data, meta, links, errors, count)
except Exception as exc:
log.exception(exc)
return response

The example runs here:
http://thomaxxl.pythonanywhere.com/api#!/People/InvokePersonmyrpc_0

Apparently query string arguments were already implemented, using the "paramaters" docstring key. I changed it a bit however.
I'll add some documentation later.

@wicol
Copy link
Contributor Author

wicol commented May 21, 2019

Firstly, thanks for the fix. Returning a SAFRSFormattedResponse now works as expected 👍

Secondly, some thoughts on the doc structure (do with it what you will):
I don't think it's exactly self explanatory; args represent keys in post body that are mapped to **kwargs, and parameters represent query parameters that are mapped to *args, ..right?
Maybe they should just be called body_schema and queryargs or something?

Or maybe it'd make more sense to use raw swagger-syntax and just pass it through? Passing required parameters to *args and optionals to **kwargs? (This way you could just refer to the openapi/swagger docs for syntax). i.e:

@classmethod
@jsonapi_rpc(http_methods=["GET"])
def get_by_name(cls, some_query_parameter, some_body_key=None):
    """
        description : Generate and return a Thing based on name
        parameters:
            - name: some_query_parameter
              type : string
              in: query
              required: true
            - name: some_body_key
              type : integer
              in: body
              description: Some body key
    """

Just some ideas since you asked if it made sense :)

@thomaxxl
Copy link
Owner

thomaxxl commented Jun 5, 2019

Hi, can you check if the latest version still works for you? I'm going to release it like this for now.

@wicol
Copy link
Contributor Author

wicol commented Jun 6, 2019

Sure, tomorrow I'll be able to check it out. I'll get back to you then.

@wicol
Copy link
Contributor Author

wicol commented Jun 7, 2019

Tried the latest version/commit and it still works for my use cases at least :)

@thomaxxl
Copy link
Owner

thomaxxl commented Jun 7, 2019

v2.2.2

@thomaxxl thomaxxl closed this as completed Jun 7, 2019
@wicol
Copy link
Contributor Author

wicol commented Jun 14, 2019

Hello again! Sorry to say this didn't work all the way. Once I turned off some debugging it doesn't work anymore because of this: https://github.com/thomaxxl/safrs/blob/master/safrs/json_encoder.py#L77.

WARNING safrs: Unknown obj type "<class 'safrs.json_encoder.SAFRSFormattedResponse'>" for <safrs.json_encoder.SAFRSFormattedResponse object at 0x7f641bccd358>

(Also related lines 83 and 85 are duplicates?)

thomaxxl added a commit that referenced this issue Jun 14, 2019
@thomaxxl
Copy link
Owner

Hi,

can you try again with the latest commit please?

@wicol
Copy link
Contributor Author

wicol commented Jun 17, 2019

Yes, that'll work! Thanks :)

Edit: oh, right, I'm still running in debug mode.. should be ok though. Will try again..
Edit2: yep, works 👍

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

No branches or pull requests

2 participants