datastore: runQuery should return complete objects rather than objects and keys

[jasonpolites]

The callback for runQuery returns keys and objects as separate collections:

https://github.com/GoogleCloudPlatform/gcloud-node#querying

This is a little problematic if you want to return "whole" objects back to the client. Perhaps this is a Datatstore specific thing, but in almost all cases the client will want the key in the object. Do I have to iterate the collections and re-constitute the object by inserting a "key" (or "id") property?

E.g what I want on the client is something like this:

{
    "_id" : 5654313976201216,
    "foo" : "bar"
}

what I get is:

{
    "foo" : "bar"
}

with the ID/key in a separate (symmetrical) collection.

[jasonpolites]

This also applies to get.

[rakyll]

This is a little problematic if you want to return "whole" objects back to the client. Perhaps this is a Datatstore specific thing, but in almost all cases the client will want the key in the object. Do I have to iterate the collections and re-constitute the object by inserting a "key" (or "id") property?

Why do you need to reconstruct the object with its identifier? The client applies the separation of keys and objects everywhere.

Not embedding keys allows users to work with plain objects. Yes, you need two declarations, but it's much easier than having model definitions and constructing an instance from your model definitions.

[jasonpolites]

It's just an issue of simplicity. In 99% of cases the use case will require the id/key at the same time the object is required. Having separate collections just means the developer has to index into the key collection for every item in the object collection. It's just more code to write/test etc.

It also means that when the developer wants to update that object, they need to either internalize the key themselves, or keep a reference to it somewhere else. Again, more code and more chance of bugs.

The "plain objects" argument applies less to dynamic languages like JS IMO. If collision with existing named properties is a concern, then adding the ability to set the name of the "key" field would work but honestly most developers are familiar with the "_id" syntax (from mongodb).

[rakyll]

As long as we provide a setting to override the key of the identifier, +1 to _id. @proppy, any comments/concerns?

[proppy]

Yes, to me clashing with the namespace of the property names is a concern, you can't assume no existing App Engine app have an existing entities w/ _id properties, you never know.

IIRC a few property name are reserved (@pcostell could correct me if I'm wrong), for example: __key__ pseudo property could be a good fit here.

Another option is not have a compound entity object closer to the existing Cloud Datastore API entity payload

{
   "key": [ "Kind", "name", "Kind", 42],
   "properties": {
        "name": "value"
   }
}

[pcostell]

If you want to expose key as a property, you should do so via __key__. This is also consistent with how the user would provide it in a projection SELECT __key__ FROM ..... I don't think you want to expose just the id, as keys can have names or ids and the ancestor / Kind can be important.

[rakyll]

you can't assume no existing App Engine app have an existing entities w/ _id properties, you never know.

Actually, it's more likely that existing entities have an _id field due to its popularity.

If you want to expose key as a property, you should do so via key.

__key__ is an obvious option, but in terms of code readability, I think it's ugly and you shouldn't be supposed to touch a key with an underscore from an external module.

We don't support GQL yet, I'll make sure we're consistent if we do.

I don't think you want to expose just the id, as keys can have names or ids and the ancestor / Kind can be important.

Yes, key is a list of kinds and numeric or string identifiers with optional namespace. So a gcloud-node key is literally a Key [1].

[1] https://github.com/GoogleCloudPlatform/gcloud-golang/blob/master/internal/datastore/datastore_v1.proto#L50