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

request.query values as is not list data type #73

Closed
sugizo opened this issue Jan 7, 2021 · 4 comments
Closed

request.query values as is not list data type #73

sugizo opened this issue Jan 7, 2021 · 4 comments
Labels
enhancement New feature or request question Further information is requested

Comments

@sugizo
Copy link

sugizo commented Jan 7, 2021

Is your feature request related to a problem?

background
insert or update database from cli must convert first
e.g.

def dict_values(raw_ori):
    processed_raw = {}
    for i in raw_ori.items():
        if len(i[1] ) > 1:
            processed_raw[i[0] ] = i[1]
        else:
            for j in i[1]:
                processed_raw[i[0] ] = j
    return processed_raw

@app.route('/api/post/{tablename}', methods = ['POST'] )
def post(request, tablename):
    if request.query:
        values = dict_values(request.query)
    row = db[tablename].validate_and_insert(**values)
    db.commit()
    return response.json(row.as_dict() )

request.query
curl -X POST -i "http://localhost:8000/api/post/instrument?name=name3&strings=3"

result without calling dict_values() function, the value data type list, will return an error when insert or update database

request.query = {'name' : ['name3'], 'strings' : ['3'] }

expected result, solve by calling dict_values() function

request.query = {'name' : 'name3', 'strings' : '3'}

Describe the solution you'd like

insert or update database from cli as is data (values not list data type) from request.query
e.g.

@app.route('/api/post/{tablename}', methods = ['POST'] )
def post(request, tablename):
    if request.query:
        values = request.query
    row = db[tablename].validate_and_insert(**values)
    db.commit()
    return response.json(row.as_dict() )

Additional context

question
is it possible to have request.query values as is not list data type ?
so that can insert or update database without calling dict_values() function

thanks

@RobertoPrevato
Copy link
Member

RobertoPrevato commented Jan 8, 2021

Hi @sugizo,
I took some time to answer because I needed time to think. When I implemented the code api for request.query I paused to think about how to handle values.

I finally opted for the current behavior because I thought it would be the "lesser evil":

  • query string keys can be repeated, so the same key can be associated to many values
  • most of times, APIs are designed to use a single value per key - so I took into consideration to use a dictionary like Dict[str, str]
  • but I didn't want, and still don't want, to handle cases when there is more than one value as joined strings
  • I considered to convert lists to a single string when the client specifies a single value, but this might be even more confusing for programmers

The recommended way to read query parameters is to use the FromQuery binder, which supports automatic binding to str, List[str], and other values. But this wouldn't work with the level of dynamism you want to achieve.

Would you take into consideration to use a JSON request payload instead of query string? This way, you might achieve what you desire this way:

from blacksheep.server.bindings import FromJson

@app.route('/api/post/{tablename}', methods = ['POST'] )
def post(tablename, values: FromJson[dict]):
    # TODO: validate values
    row = db[tablename].validate_and_insert(**values)
    db.commit()
    return response.json(row.as_dict())

I could add an API to read the query as a flat Dict[str, str], as additional method on the Request class (e.g. request.query_uniq()), but I am not convinced this is a good idea.

@sugizo
Copy link
Author

sugizo commented Jan 8, 2021

from blacksheep.server.bindings import FromJson

@app.route('/api/post/{tablename}', methods = ['POST'] )
def post(tablename, values: FromJson[dict]):
    # TODO: validate values
    row = db[tablename].validate_and_insert(**values)
    db.commit()
    return response.json(row.as_dict() )

solution you provide consider it documented on here
so it must be same like :

@post("/something")
async def create_something(request: Request):
    data = await request.json()

taken on same link above

so, the code provided on first issue is cutted down to make it focus on request.query, here is the full code for insert new data

@app.route('/api/post/{tablename}', methods = ['POST'] )
async def post(request, tablename):
    if request.query:
        values = dict_values(request.query)
    elif await request.form():
        values = await request.form()
    elif await request.json():
        values = await request.json()
    row = db[tablename].validate_and_insert(**values)
    db.commit()
    return responses.json(row.as_dict() )

so can insert new data via cli either using any of these command

request.query use this
curl -X POST -i "http://localhost:8000/api/post/instrument?name=name3&strings=3"

request.form() use this
curl -X POST -d "name=name4&strings=4" -i http://localhost:8000/api/post/instrument

request.json() use this
curl -X POST -d '{"name": "name5", "strings": 5}' -i http://localhost:8000/api/post/instrument

question
so how to achieve it using blacksheep way without calling dict_values() ?

thanks

@RobertoPrevato
Copy link
Member

RobertoPrevato commented Jan 8, 2021

For such scenario, currently you need something like your dict_values function. The API in the web framework always return a dictionary like Dict[str, List[str]].

Notes:

  1. your function can be simplified to a one liner dictionary comprehension:
def dict_values(raw_ori):
    return {k: v[-1] for k, v in raw_ori.items()}
  1. if you are going to do synchronous database operations, you will block the event loop of the application, removing the benefits of using async/await.

  2. the built-in parse_qs function in Python standard library behaves like BlackSheep:

from urllib.parse import parse_qs

parse_qs("a=20&b=10")

# {"a": ["20"], "b": ["10"]}
  1. if you are going to do this operation often, you might implement a custom binder to achieve the same, to handle an optional query string

I hope this helps. I will take consider implementing something like Flask does on this matter, to improve UX when handling queries with single values.

@RobertoPrevato RobertoPrevato added enhancement New feature or request question Further information is requested labels Jan 8, 2021
@sugizo
Copy link
Author

sugizo commented Jan 8, 2021

>>> q = {'key1': ['val1', 'val3'], 'key2': ['val2']}
>>> {k: v[0] for k, v in q.items()}
{'key1': 'val1', 'key2': 'val2'}
>>> {k: v[-1] for k, v in q.items()}
{'key1': 'val3', 'key2': 'val2'}

yes aware of that, thx
but will leave it as it be for a moment because in pydal there are case to store list data type

or perhaps you have any idea to simplified dict_values() with if condition and for loop are welcome

thanks

@sugizo sugizo closed this as completed Jan 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants