0

I am trying t write a web controller for an API with GET, POST, PUT functionalities. 1. In the method that handles the requests , especially POST and PUT, I want to validate if the required keys/fields are present in the request body. 2. I also want to check the authorization key present in the request header and throw unauthorized error etc. as response.

Is there an elegant way of doing this in python, write multiple if...else doesn't look elegant.

I have the following code which handles the request body:

from werkzeug.wrappers import BaseResponse as Response
.
.
.
    root = ET.fromstring(data)
            for child in root:
                order_completed_date = child.find('Order_Completed_Date')
                if order_completed_date is None:
                    #return json.loads({"status":"400", "message":"Order_Completed_Date is missing"})
                    return Response('Bad Request, Order_Complete_At missing', status=400)
                else:
                    order_completed_date = order_completed_date.text
                order_id = child.find('Order_Number')
                if order_id is None:
                    return Response('Bad Request, Order_Number missing', status=400) 
                else:
                    order_id = order_id.text
                product_id =child.find('SKU')
                if product_id is None:
                    return Response("Bad request, SKU is missing", status=400)
                else:
                    product_id = product_id.text
                .
                .
                .

So on, I'm writing if else for each field

nish
  • 6,952
  • 18
  • 74
  • 128
  • What's "not elegant" about having an `if` statement that checks for the keys and a second `if` statement that checks the auth? (Also, I'm not sure why you need any `else` if you're going to raise an exception on failure.) Maybe if you can show us some [minimal sample code](http://stackoverflow.com/help/mcve) and explain what you don't like about it, you can get a better answer. – abarnert Dec 13 '14 at 05:17
  • @abarnert: thanks for your response. I have multiple fileds in the request body - 17. So, I'll have to write an if statement for each of the fields and throw appropriate response. – nish Dec 13 '14 at 05:20
  • @abarnert: I have shared some code – nish Dec 13 '14 at 05:25

1 Answers1

1

We've got some code, so we can look at how to refactor it.

You essentially have minor variations on this code over and over:

order_completed_date = child.find('Order_Completed_Date')
if order_completed_date is None:
    #return json.loads({"status":"400", "message":"Order_Completed_Date is missing"})
    return Response('Bad Request, Order_Complete_At missing', status=400)
else:
    order_completed_date = order_completed_date.text

So, how could we turn that into a function?

First, just turn it into a function and see what's wrong with it:

def find_key():
    order_completed_date = child.find('Order_Completed_Date')
    if order_completed_date is None:
        return Response('Bad Request, Order_Complete_At missing', status=400)
    else:
        order_completed_date = order_completed_date.text

So, the first problem is that child value. It's not a constant; it's different each time through the loop, so we need to take that as a parameter.*

* Well, we could define find_key locally, and get child as a closure variable, but let's keep it simple for now.

Similarly, while 'Order_Completed_Date' isn't different each time through the loop, it is different for each different key, so we need to take that as well.*

* There's also an 'Order_Complete_At' string, but that seems to be a typo for 'Order_Completed_Date'. If it isn't, then you'll need to add another parameter, like key_error_name, that you can use for that.

The variable name order_completed_date doesn't have to change, because local variable names don't mean anything to Python—but it's obviously misleading to a human reader if we use it as a generic name for "the value for each key".

Finally, the big problem is what we return. We have two different kinds of things we could return—a node's text, or an error Response. We could return the former and raise an exception with the latter. Or return a pair of things, a text-or-error and a flag that tells us which is which. Or we could return a text and an error, one of which is None. The exception seems like the most complicated, but it automatically gives us a "non-local return", a way to break out of the rest of the function without having to check each return value one by one.

So:

def find_key(child, key):
    value = child.find(key)
    if value is None:
        raise KeyError(key)
    else:
        return value.text

And now:

try:
    for child in root:
        order_completed_date = find_key(child, 'Order_Completed_Date')
        order_id = find_key(child, 'Order_Number')
        product_id = find_key(child, 'SKU')
        # ...
except KeyError as e:
    return Response("Bad request, {} is missing".format(e.args[0]), status=400)
abarnert
  • 354,177
  • 51
  • 601
  • 671
  • better. But suppose if three keys are missing, it'll first throw an error for first key. Once it is rectified, then the error for the 2nd key would be thrown. Is there a way to find all tags in a given child using element tree? If so, in a single function we can return all the missing keys? – nish Dec 13 '14 at 06:26
  • Thanks or the help abarnert – nish Dec 13 '14 at 06:30