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

jasonpolites · 2014-07-28T16:42:47Z

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 · 2014-07-28T16:56:56Z

This also applies to get.

rakyll · 2014-07-28T18:38:36Z

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 · 2014-07-28T19:40:00Z

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 · 2014-07-29T17:08:49Z

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

proppy · 2014-07-30T18:15:18Z

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 · 2014-07-30T19:37:13Z

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 · 2014-07-30T21:30:08Z

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

stephenplusplus · 2014-07-30T23:51:48Z

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

- @proppy

As a dev using this lib, I would prefer exactly this. Explicit and concise, without a chance of collisions.

To confirm:

datastore.runQuery({/*...*/}, function (err, entities, nextQuery) {
  /*
    entities = [
      {
        key: ['Kind', 'name', 'Kind', 42],
        properties: {
          name: 'value'
        }
      },
      // {},
      // {}
    ]
  */
});

@jasonpolites wdyt?

jasonpolites · 2014-07-31T14:51:52Z

I think that's fine, although the key structure is not going to be familiar to developers not familiar with datastore. Also is the second 'Kind' actually the namespace? Or the parent kind?

Assuming I can just throw the value for key into a separate call without needing to parse out any of its constituents then LGTM.

Can we copy/paste some language about key structure into the Entities & Keys section of the README? Perhaps from here?

rakyll · 2014-07-31T22:29:09Z

Also is the second 'Kind' actually the namespace? Or the parent kind?

Child kind.

jasonpolites · 2014-07-31T23:53:44Z

Oh that's unexpected. So an entity with 100 children would have 100 entries in the key?

rakyll · 2014-08-01T00:04:47Z

No, parent's key will be

['ParentKind', 'name']

and children keys will look like what's below.

['ParentKind', 'name', 'ChildKind', 42],
['ParentKind', 'name', 'ChildKind', 43],
['ParentKind', 'name', 'ChildKind', 44],
['ParentKind', 'name', 'ChildKind', 45]...

jasonpolites · 2014-08-01T00:06:23Z

Ah. I see. Thanks.
On Jul 31, 2014 5:04 PM, "Burcu Dogan" notifications@github.com wrote:

No, parent's key will be

['ParentKind', 'name']

and children keys will look like what's below.

['ParentKind', 'name', 'ChildKind', 42],['ParentKind', 'name', 'ChildKind', 43],['ParentKind', 'name', 'ChildKind', 44],['ParentKind', 'name', 'ChildKind', 45]...

—
Reply to this email directly or view it on GitHub
#38 (comment)
.

fixes #38 return key/prop combined object.

🤖 I have created a release \*beep\* \*boop\* --- ### [1.1.4](https://www.github.com/googleapis/nodejs-service-usage/compare/v1.1.3...v1.1.4) (2021-08-17) ### Bug Fixes * **deps:** google-gax v2.24.1 ([#38](https://www.github.com/googleapis/nodejs-service-usage/issues/38)) ([021b0eb](https://www.github.com/googleapis/nodejs-service-usage/commit/021b0eb4c69c1ed485aac388c2a5583547dbb3ad)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

🤖 I have created a release \*beep\* \*boop\* --- ### [1.0.5](https://www.github.com/googleapis/nodejs-dms/compare/v1.0.4...v1.0.5) (2021-08-17) ### Bug Fixes * **deps:** google-gax v2.24.1 ([#37](https://www.github.com/googleapis/nodejs-dms/issues/37)) ([ee8483a](https://www.github.com/googleapis/nodejs-dms/commit/ee8483a069e32a189579d44423fd726e9731b2e9)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>

* fix(build): migrate to using main branch * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>

🤖 I have created a release \*beep\* \*boop\* --- ### [1.0.6](https://www.github.com/googleapis/nodejs-essential-contacts/compare/v1.0.5...v1.0.6) (2021-08-19) ### Bug Fixes * **build:** migrate to using main branch ([#38](https://www.github.com/googleapis/nodejs-essential-contacts/issues/38)) ([600a78b](https://www.github.com/googleapis/nodejs-essential-contacts/commit/600a78b7cae96dbb46f450eb9438b7551b847460)) * **deps:** google-gax v2.24.1 ([#39](https://www.github.com/googleapis/nodejs-essential-contacts/issues/39)) ([8b55dd2](https://www.github.com/googleapis/nodejs-essential-contacts/commit/8b55dd2bc7ef8ecac7aa3be4441ee46b965b0867)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> Co-authored-by: Jeffrey Rennie <rennie@google.com>

* build!: Update library to use Node 12 Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>

🤖 I have created a release \*beep\* \*boop\* --- ## [1.2.0](https://www.github.com/googleapis/nodejs-bigquery-connection/compare/v1.1.0...v1.2.0) (2020-10-17) ### Features * add aws connection support ([#33](https://www.github.com/googleapis/nodejs-bigquery-connection/issues/33)) ([13fecb8](https://www.github.com/googleapis/nodejs-bigquery-connection/commit/13fecb87d0191a4654f14604164072570f640dc6)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please).

[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [webpack-cli](https://togithub.com/webpack/webpack-cli/tree/master/packages/webpack-cli) ([source](https://togithub.com/webpack/webpack-cli)) | [`^4.0.0` -> `^5.0.0`](https://renovatebot.com/diffs/npm/webpack-cli/4.10.0/5.0.1) | [![age](https://badges.renovateapi.com/packages/npm/webpack-cli/5.0.1/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/npm/webpack-cli/5.0.1/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/npm/webpack-cli/5.0.1/compatibility-slim/4.10.0)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/npm/webpack-cli/5.0.1/confidence-slim/4.10.0)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>webpack/webpack-cli</summary> ### [`v5.0.1`](https://togithub.com/webpack/webpack-cli/blob/HEAD/CHANGELOG.md#501-httpsgithubcomwebpackwebpack-clicomparewebpack-cli500webpack-cli501-2022-12-05) [Compare Source](https://togithub.com/webpack/webpack-cli/compare/webpack-cli@5.0.0...webpack-cli@5.0.1) ##### Bug Fixes - make `define-process-env-node-env` alias `node-env` ([#3514](https://togithub.com/webpack/webpack-cli/issues/3514)) ([346a518](https://togithub.com/webpack/webpack-cli/commit/346a518dd7423a726810ef1012031f92d318c9c5)) ### [`v5.0.0`](https://togithub.com/webpack/webpack-cli/blob/HEAD/CHANGELOG.md#500-httpsgithubcomwebpackwebpack-clicomparewebpack-cli4100webpack-cli500-2022-11-17) [Compare Source](https://togithub.com/webpack/webpack-cli/compare/webpack-cli@4.10.0...webpack-cli@5.0.0) ##### Bug Fixes - improve description of the `--disable-interpret` option ([#3364](https://togithub.com/webpack/webpack-cli/issues/3364)) ([bdb7e20](https://togithub.com/webpack/webpack-cli/commit/bdb7e20a3fc5a676bf5ba9912c091a2c9b3a1cfd)) - remove the redundant `utils` export ([#3343](https://togithub.com/webpack/webpack-cli/issues/3343)) ([a9ce5d0](https://togithub.com/webpack/webpack-cli/commit/a9ce5d077f90492558e2d5c14841b3b5b85f1186)) - respect `NODE_PATH` env variable ([#3411](https://togithub.com/webpack/webpack-cli/issues/3411)) ([83d1f58](https://togithub.com/webpack/webpack-cli/commit/83d1f58fb52d9dcfa3499efb342dfc47d0cca73a)) - show all CLI specific flags in the minimum help output ([#3354](https://togithub.com/webpack/webpack-cli/issues/3354)) ([35843e8](https://togithub.com/webpack/webpack-cli/commit/35843e87c61fd27be92afce11bd66ebf4f9519ae)) ##### Features - failOnWarnings option ([#3317](https://togithub.com/webpack/webpack-cli/issues/3317)) ([c48c848](https://togithub.com/webpack/webpack-cli/commit/c48c848c6c84eb73fbd829dc41bee301b0b7e2de)) - update commander to v9 ([#3460](https://togithub.com/webpack/webpack-cli/issues/3460)) ([6621c02](https://togithub.com/webpack/webpack-cli/commit/6621c023ab59cc510a5f76e262f2c81676d1920b)) - added the `--define-process-env-node-env` option - update `interpret` to v3 and `rechoir` to v0.8 - add an option for preventing interpret ([#3329](https://togithub.com/webpack/webpack-cli/issues/3329)) ([c737383](https://togithub.com/webpack/webpack-cli/commit/c7373832b96af499ad0813e07d114bdc927afdf4)) ##### BREAKING CHANGES - the minimum supported webpack version is v5.0.0 ([#3342](https://togithub.com/webpack/webpack-cli/issues/3342)) ([b1af0dc](https://togithub.com/webpack/webpack-cli/commit/b1af0dc7ebcdf746bc37889e4c1f978c65acc4a5)), closes [#3342](https://togithub.com/webpack/webpack-cli/issues/3342) - webpack-cli no longer supports webpack v4, the minimum supported version is webpack v5.0.0 - webpack-cli no longer supports webpack-dev-server v3, the minimum supported version is webpack-dev-server v4.0.0 - remove the `migrate` command ([#3291](https://togithub.com/webpack/webpack-cli/issues/3291)) ([56b43e4](https://togithub.com/webpack/webpack-cli/commit/56b43e4baf76c166ade3b282b40ad9d007cc52b6)), closes [#3291](https://togithub.com/webpack/webpack-cli/issues/3291) - remove the `--prefetch` option in favor the `PrefetchPlugin` plugin - remove the `--node-env` option in favor `--define-process-env-node-env` - remove the `--hot` option in favor of directly using the `HotModuleReplacement` plugin (only for `build` command, for `serve` it will work) - the behavior logic of the `--entry` option has been changed - previously it replaced your entries, now the option adds a specified entry, if you want to return the previous behavior please use ` webpack --entry-reset --entry './src/my-entry.js' ` </details> --- ### Configuration 📅 **Schedule**: Branch creation - "after 9am and before 3pm" (UTC), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/nodejs-run).

jgeewax added bug labels Jul 28, 2014

rakyll added 0.1.x and removed 0.1.x labels Jul 28, 2014

rakyll changed the title ~~runQuery should return complete objects rather than objects and keys~~ datastore: runQuery should return complete objects rather than objects and keys Jul 28, 2014

rakyll mentioned this issue Jul 28, 2014

datastore: Entity to proto and vice versa should support embedded key properties #39

Closed

rakyll added this to the M1: core, datastore & storage milestone Jul 29, 2014

rakyll modified the milestones: M1: core, datastore & storage, M2 Jul 31, 2014

rakyll added feature-request and removed 0.0.x labels Jul 31, 2014

stephenplusplus pushed a commit to stephenplusplus/gcloud-node that referenced this issue Jul 31, 2014

fixes googleapis#38 datastore.get returns key/prop combined object.

96ef76d

rakyll closed this as completed in 7197b6c Aug 1, 2014

rakyll pushed a commit that referenced this issue Aug 1, 2014

Merge pull request #75 from stephenplusplus/master

771d3cc

fixes #38 return key/prop combined object.

sofisl pushed a commit that referenced this issue Nov 11, 2022

fix(deps): google-gax v2.24.1 (#38)

9fb6256

This was referenced Nov 11, 2022

migrate code from googleapis/nodejs-service-usage #3610

Merged

migrate code from googleapis/nodejs-dms #3611

Merged

sofisl pushed a commit that referenced this issue Nov 11, 2022

chore: release 1.1.1 (#38)

e03944c

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>

sofisl pushed a commit that referenced this issue Nov 11, 2022

chore: release 1.0.3 (#38)

51f8fd0

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> Co-authored-by: Jeffrey Rennie <rennie@google.com>

This was referenced Nov 11, 2022

migrate code from googleapis/nodejs-org-policy #3614

Merged

migrate code from googleapis/nodejs-essential-contacts #3615

Merged

migrate code from googleapis/nodejs-private-catalog #3616

Merged

sofisl added a commit that referenced this issue Nov 12, 2022

build!: update library to use Node 12 (#38)

8ef133b

* build!: Update library to use Node 12 Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>

sofisl mentioned this issue Nov 12, 2022

migrate code from googleapis/nodejs-ids #3617

Merged

4 tasks

sofisl pushed a commit that referenced this issue Nov 12, 2022

test(nodejs): remove 15 add 16 (#1322) (#38)

409e1d7

sofisl mentioned this issue Nov 12, 2022

migrate code from googleapis/nodejs-filestore #3619

Merged

4 tasks

sofisl pushed a commit that referenced this issue Nov 16, 2022

chore(deps): drop dependency on through2 (#38)

938a325

This was referenced Nov 16, 2022

migrate code from googleapis/nodejs-recaptcha-enterprise #3645

Merged

migrate code from googleapis/nodejs-bigquery-connection #3646

Merged

sofisl mentioned this issue Jan 10, 2023

migrate code from googleapis/nodejs-dlp #3837

Merged

4 tasks

sofisl pushed a commit that referenced this issue Jan 10, 2023

feat: regenerated GAPIC v2 code (#38)

e041ba0

This was referenced Jan 10, 2023

migrate code from googleapis/nodejs-run #3838

Merged

migrate code from googleapis/nodejs-vision #3860

Merged

sofisl pushed a commit that referenced this issue Jan 17, 2023

v0.17.0 (#38)

ffcdc5d

sofisl pushed a commit that referenced this issue Jan 24, 2023

fix(package): update google-gax to version 0.16.0 (#38)

92b8ec8

sofisl pushed a commit that referenced this issue Jan 25, 2023

fix(package): update google-gax to version 0.16.0 (#38)

c259f15

sofisl pushed a commit that referenced this issue Sep 14, 2023

chore: timeout for system test (#38)

0cb5f9b

sofisl mentioned this issue Sep 14, 2023

migrate code from googleapis/nodejs-dns #4657

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

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

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

Comments