Manager attaches itself to queries so they can run directly

[willembult]

It seems unnecessary that we have to feed a query that we got from a Manager back into that Manager to run it. With this change, a Query object retrieved from Manager.init_query() has a reference to the Manager and can be run with a call to perform(), like so:

manager = Manager(datastore, model=User)
manager.init_query().filter('name', '=', 'John').perform()

It can still also be run directly on the datastore, since it merely extends Query.

[jbenet]

Hm-- I don't like this solution. It adds state behind the scenes, on the query object, binding the query object to that manager. Queries right now are (a) datastore (/manager) agnostic, and (b) entirely serializable (dict, from_dict, repr). This allows queries to be used across datastores and even across processes.

If the intent is to reduce the interface:

manager = Manager(datastore, model=User)
query = manager.init_query().filter('name', '=', 'John')
results = manager.query(query)

perhaps another solution would yield:

manager = Manager(datastore, model=User)
query = Query().filter('name', '=', 'John')
results = manager.query(query)

This could be done by:

• making Query() default key=Key('/')
• making managers (selectively?) prepend their own key to queries. e.g.:

def query(self, query):
  mgr_query = query.copy()
  mgr_query.key = Key(self.model.key_type).child(query.key)
  return self.datastore.query(mgr_query)

this would also obviate the need for init_query, and ensure that ALL queries performed on this manager are actually queries for this manager (i.e. key prefix match)

thoughts, @willembult ?

[jbenet]

aside note: Manager.key should prob be:

@property
def key(self):
  return Key(self.model.key_type)

making the above:

def query(self, query):
  mgr_query = query.copy()
  mgr_query.key = self.key.child(query.key)
  return self.datastore.query(mgr_query)

[willembult]

Ah, much better, I agree.

[jbenet]

@willembult would this change code you've written already? (probably...)

Also, i'm wary of making Manager.query do a selective prefixing. It makes it a bit ambigious. Perhaps it should always prefix... problem is you end up with:

mgr = Manager(Person)               # Queries:
mgr.query(Query('/'))               #   /person
mgr.query(Query('/person'))         #   /person/person
mgr.query(Query('/person/person'))  #   /person/person/person
mgr.query(Query('/person:foobar'))  #   /person/person:foobar
mgr.query(Query('foobar'))          #   /person/foobar
mgr.query(Query(':foobar'))         #   /person:foobar

Not sure if this a problem at all... haven't run into a case where I want to query anyting but the toplevel (all objects of the type) from a manager... (e.g. so far, always mgr.query(Query('/')))

[willembult]

Ah yes, I hadn't thought of it, but I agree the ambiguity is a problem. With selective prefixing certain queries would become quite counter-intuitive:

mgr = Manager(Division)           # Queries:
mgr.query(Query('/'))             # /division
mgr.query(Query('/division'))     # /division
mgr.query(Query('/division/new')) # /division/new
mgr.query(Query('/staff/new'))    # /division/staff/new

As of now, the manager actually refuses to store any objects with keys more than one level deep, so I think always prefixing should be ok.