Add guard clauses

[dphaener]

This adds tests and a guard clause for the expand_path function in the terms module. This type checks the input and raises a type error if it is not a string.

[codecov-io]

Current coverage is 95.41%

Merging #2 into develop will increase coverage by +18.57% as of 04f332c

@@            develop      #2   diff @@
=======================================
  Files             6       7     +1
  Stmts           311     240    -71
  Branches          0       0       
  Methods           0       0       
=======================================
- Hit             239     229    -10
  Partial           0       0       
+ Missed           72      11    -61

Review entire Coverage Diff as of 04f332c

Powered by Codecov. Updated on successful CI builds.

[dphaener]

@dmpayton This isn't ready to merge yet, but I wanted to run this by you. One improvement I can see here is what I did with the expand_path function in the terms module. I think it's probably a valid case for us to be checking input and spitting out some sort of standard exception if it's not what we expect. It seems a bit clunky to add it to EVERY function, so if it's something you agree with, and that we should do, I can try to think of a better way to do some checking. Or maybe you have some ideas.

[dphaener]

@dmpayton Go ahead and take a look at this. I'm done with it for now.

[dphaener]

@dmpayton ping

[dmpayton]

Thanks @dphaener, I think this is a really good start.

Any custom exception class should inherit from a single ReqonError, e.g.,

class ReqonError(Exception):
    pass

InvalidFilter(ReqonError):
    pass

This will make it easier to differentiate between ReQON exceptions and other kinds of exceptions. Once this is in place, the query() function can be updated to look something like this:

def query(query):
    try:
        reql = r.db(query['$db']).table(query['$table'])
    except KeyError:
        try:
            reql = r.table(query['$table'])
        except KeyError:
            raise ReqonError('The query descriptor requires a $table key.')

    for sequence in query['$query']:
        term = sequence[0]
        try:
            reql = TERMS[term](reql, *sequence[1:])
        except ReqonError:
            raise  # Let any ReqonError bubble to the top.
        except r.ReqlError:
            message = 'Invalid values for {0} with args {1}'
            raise ReqonError(message.format(term, sequence[1:]))
        except Exception:
            message = 'Unknown exception, {0}: {1}'
            raise ReqonError(message.format(term, sequence[1:]))

    return reql

[dmpayton]

I generally prefer duck typing to manual type checking, but there are some cases where we should raise an appropriate ReqonError if the wrong type is passed in.

I'm wondering if it might be appropriate to put together a JSON-Schema definition for the query descriptor. It might be better to have a single validation step before we start building the ReQL, rather than validating as we go. That schema could be a beast though, and would have to be maintained.

I don't have a good solution yet, so the type checking can be left as-is for now.

[dphaener]

@dmpayton Does it seem like we'd be processing the query twice in the case of running a validation before hand?

[dphaener]

@dmpayton Cleaned up some stuff per your suggestions. I also moved most of the type checking out into a decorator function. This makes the code a bit easier to reason about, and dries up actual term functions. However, the type_checker decorator function is a bit magical, and slightly difficult to reason about. Let me know what you think about it. We can possibly open up an issue about how to better implement this via duck typing or something else. But as you said, for now this is probably the way to go.

[dmpayton]

Does it seem like we'd be processing the query twice in the case of running a validation before hand?

We wouldn't be processing it twice, per se. We'd first validate the query with, e.g., jsonschema. Once we know it's good to go, we iterate over it to build the actual ReQL.

[dmpayton]

Some day we'll have the tooling necessary to make use of the type hinting introduced in PEP 0484. Some day...

[dmpayton]

@dphaener Thanks for your work on this. I'll create a separate issue to look into json schema validation.