Hook Overhaul

[luanpotter]

This changes a lot regarding hooks, namely:

• Hooks are now mandatory, enableHooks no longer exists. Hooks now take a more low-level fundamental step that is always activated, because they are no longer to be used for API security and other high level functions. For that, one must use Shields. Shields protect the API, Hooks bind into the Datastore.
  • More hooks options: hooks added for afterQuery.
  • Few other breaking changes: since we are breaking the API, I took the opportunity to refactor some bits that were messy (typos, generics, etc).

It's a huge commit that introduces lots of breaking changes. Please review it carefully.

[coveralls]

Coverage increased (+0.3%) to 64.381% when pulling 6660a4d72f7f5a5332f45e1bd7a8937f70ba0d04 on new-hooks into 26c18a007dc026508e0bd5f45aa4ab35940f3b27 on master.

[feroult]

@luanpotter thanks for the great PR!

Just a note, wouldn't it be possible to have only on beforeQuery and afterQuery and have different methods in QueryBuilder<T> ?

For instance:

@Override
public void beforeQuery(QueryBuilder<Object> q) {
    q.forceId(id);
    q.forceList(list);
    q.forceObject(object);
}

Maybe with Response suffix?

For afterQuery:

@Override
public void afterQuery(QueryBuilder<Object> q) {
    q.id();
    q.list();
    q.first();
    q.only();
}

Since it is afterQuery it would return the value it just got from database.

Possibly, if you don't like that one, we could have something like this:

@Override
public void afterQuery(QueryBuilder<Object> q) {
    q.getIdResponse();
    q.getListResponse();
    q.getObjectResponse();
}

[luanpotter]

I added the BeforeQueryObject because I need to know the QueryType in the hook. I could add that as a property of QueryBuilder and then access it via q.getType(), I actually considered doing that but I thought it might pollute the QueryBuilder scope.

Regarding the afterQuery, if I have the getType() method available, it can be a single afterQuery hook, but I'll need three methods to fetch the three possible responses.

It could be these:

    @Override
    public void afterQuery(QueryBuilder<Object> q) {
        q.id();
        q.list();
        q.first();
        q.only();
    }

But that would cause problems because these already exist as query finalizers. Think like so: you called .list() somewhere to finish a query, it starts running the list method, runs the before hook, fetches the data and run the after hook, before ending. While it runs the after hook it's still in the list() method call chain. I could add a control to the start of the method to see if it's the second time it's called in the same chain, but I think it would be a bit convoluted.

I think the solution of adding distinct methods for this might be better than reusing list, ids, etc.

However, I do think it might be confusing for the regular user (that does not do hooks) to have these extra names polluting the scope; for example, you are building your pretty query in your action and go:

    yawp(Foo.class).where("bar", "=", bar).

And after this dot the option getListResponse is presented, but it won't do anything, as the only correct option in this place would be .list(). The other, equally tempting option will simply return null.

On that sense, reusing the method and applying some sort of state control to know the second call to a finalizer method might be better, but still seems unnatural, odd, convoluted and confusing to me (trying to think like a regular user).

Thoughts?

[feroult]

Think like so: you called .list() somewhere to finish a query, it starts running the list method, runs the before hook, fetches the data and run the after hook, before ending.

I think afterQuery is supposed to run after the query has finished. It is safe to do that. And I also think that a state control inside QueryBuilder is a good option..

On that sense, reusing the method and applying some sort of state control to know the second call to a finalizer method might be better, but still seems unnatural, odd, convoluted and confusing to me (trying to think like a regular user).

For me, since it is in afterQuery, it is natural to suppose that the query has already ran.

if I have the getType() method available, it can be a single afterQuery hook

QueryBuilder already has a:

protected Class<T> getClazz() {
      return clazz;
}

We can make it public...

What do you think?

[luanpotter]

getType is not getClazz, it's to get the QueryType (it can be LIST, IDS, FETCH). On beforeQuery, I need to know which finalizer was called. Because otherwise that information is totally unavailable to the hook. I also need to know this on afterQuery if I'm supposed to know if I need to call ids(), list() or what to get the results.

Regarding the state control, how exactly would it work?

Some cases : if I call a finalizer on the beforeQuery, for instance, what happens? should throw an exception, invalid state? because I'm in the middle of running a finalizer and I'm calling it again. if I call it in the after, it returns immediately with the result gotten previously. Outside of hooks, the finalizers should work as is.

There are several finalizers: ids(), only(), onlyId(), list(), fetch(), executeQuery(), executeQueryIds() (that I can remember now from the top of my head). The basal ones that the other use are three: LIST, FETCH and IDS (that's the query type). If I call in a hook a finalizer with different basal than the one that was actually called, what do I do? Return null or throw?

[luanpotter]

The current version exposes the following methods in the Query Builder:

QueryType getQueryType()
Object getExecutedResponse()

void forceResult(QueryType type, Object obj)
void clearForcedResult(Type tpye)
void clearForcedResults()
bool hasForcedResult(QueryType type)
bool hasForcedResult()

Also, beforeQuery and afterQuery hooks have each a single signature taking in only the QueryBuilder as a parameter.

A very naïve possible cache implementation is shown in a test.