Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Key.toUrlSafe() consistent with the URL-safe key displayed in Google Cloud Datastore Console #1357

Closed
sai-pullabhotla opened this issue Oct 31, 2016 · 27 comments
Assignees
Labels
api: datastore Issues related to the Datastore API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@sai-pullabhotla
Copy link

The Key.toUrlSafe() returns a URL encoded (similar to URLEncoder.encode) version of the key like the one below:

partition_id+%7B%0A++project_id%3A+%22my-project%22%0A++namespace_id%3A+%22MyNamespace%22%0A%7D%0Apath+%7B%0A++kind%3A+%22MyEntity%22%0A++name%3A+%22MyKey%22%0A%7D%0A

The Google Cloud Datastore Console displays a Base64 encoded version of the key, which looks something like -

agVoZWxsb3IPCxIHQWNjb3VudBiZiwIM

The Console allows filtering on key by URL-safe key, which works fine if I use the URL-safe value displayed by the GCD Console, but does not work when using the value returned from Key.toUrlSafe().

The Console Documentation states that -

URL-safe key

This is a base64-encoded, serialized version of your entity key. The encoding method is available in any Cloud Datastore client library

Example: agVoZWxsb3IPCxIHQWNjb3VudBiZiwIM

So, the client libraries are expected to match with the Base64 version? Nonetheless, it would be nice to have the client libraries and Console in sync to help with crosschecking/debugging purposes.

@mziccard
Copy link
Contributor

@sai-pullabhotla This is interesting. @aozarov any reason our toUrlSafe is different from the one used in the cloud console? Do you think it's possible this changed over time and we failed to keep up-to-date?

@aozarov
Copy link
Contributor

aozarov commented Oct 31, 2016

I think that at the time that we provided the toUrlSafe it only worked for this library (there was no "standard"). I am glad to see that now there is and we should change the code to adopt it.

@mziccard
Copy link
Contributor

Then @sai-pullabhotla thanks for bringing this up. I'll work on a fix soon.

@mziccard
Copy link
Contributor

mziccard commented Nov 1, 2016

@eddavisson @pcostell can you guys help me understand how the base64 URL-safe key shown in the Cloud Console is computed?

I expected it to match the following code:

BaseEncoding.base64Url().omitPadding().encode(toPb().toByteArray());

But that is not the case.

@pcostell
Copy link

pcostell commented Nov 2, 2016

The base64 URL-safe key shown in the console only works with the App Engine client libraries. If you think it would be useful for it to provide a URL-safe key usable in the Google Cloud client libraries, please send feedback in the Cloud Console so it can get triaged by the right team.

@mziccard
Copy link
Contributor

mziccard commented Nov 6, 2016

@pcostell Can't we generate this base64 URL-safe in Google Cloud? I think it would make sense for us to generate that value as it is used in AE libraries as well as in the Cloud console.

@garrettjonesgoogle garrettjonesgoogle added api: datastore Issues related to the Datastore API. triaged for beta labels Nov 23, 2016
@JoshuaFox
Copy link

JoshuaFox commented Jan 12, 2017

Parsing and generating URL-Safe keys in the format of the Google App Engine API is essential for us to be able to use the Google Cloud API.

@shinfan shinfan added triaged for GA type: question Request for information or clarification. Not an issue. breaking change labels Feb 23, 2017
@lesv
Copy link
Contributor

lesv commented Feb 23, 2017

@JoshuaFox Are you asking for compatibility within the library, or between GAE Standard API's / Cloud Console and this library?

Also, how would it affect you if we changed the format generated by this library to be compatible with GAE Standard / Cloud Console?

@JoshuaFox
Copy link

I'm not sure what "compatibility within the library" means, but of course I'd like all kinds of compatibility. Specifically, we have URL-Safe keys generated in the GAE API which need to be parsed into Key objects in Google Cloud API; and the Google Cloud API code has to be able to generate keys in the GAE URL-Safe string format.

The Cloud Console is less important.

In any case, we have since Jan 12 written code that digs into this format and does the above conversions, so for us it is less urgent; though still, this compatibility seems like an obvious requirement.

@lesv
Copy link
Contributor

lesv commented Feb 27, 2017

@JoshuaFox What I'm trying to ask is, if we fix (ie. change the current behavior of the google-cloud API) to match what the GAE API does, would it break your app?

@JoshuaFox
Copy link

I don't think it would break my app.

@lesv
Copy link
Contributor

lesv commented Feb 27, 2017

@garrettjonesgoogle I think we should fix it, and the sooner, the better. (ie. before GA)

@lesv
Copy link
Contributor

lesv commented Feb 27, 2017

@jabubake PTAL - you might wish to inquire / make sure that that thing your working on is compatible in this.

@lesv
Copy link
Contributor

lesv commented Feb 27, 2017

UrlSafe keys are allowed to be used in URL's (ie. bookmark-able). Developers who move / augment their GAE Standard apps with GAE Flex apps will want to easily move between them. Not having this will block the adoption of GAE Flex for many developers.

@lesv
Copy link
Contributor

lesv commented Feb 27, 2017

Note - we should rename the existing toSafeUrl() to toOldSafeUrl() and fromUrlSafe() to fromOldUrlSafe as well, so that we don't mess up too many users.

Alternatively, but equally bad, we should create toGAESafeUrl() and fromGAEUrlSafe().

@garrettjonesgoogle
Copy link
Member

Maybe: create toBase64SafeUrl() and fromBase64SafeUrl() so that the reasons to choose one vs the other is clearer, and not so tied to a specific technology that is incidental.

@garrettjonesgoogle
Copy link
Member

Since we will be adding methods instead of changing them, this isn't a breaking change any more.

@lesv
Copy link
Contributor

lesv commented Mar 10, 2017

There has been some offline discussion and it doesn't seem like this can be fixed for GA, and might not be able to be fixed at all.

@garrettjonesgoogle
Copy link
Member

garrettjonesgoogle commented Mar 10, 2017

It turns out that replicating the base 64 format of the cloud console in open source isn't possible right now; it requires a proto that isn't open source, and an app id that isn't available in the client library context.

The solution is probably for cloud console to produce an alternative base 64 format that the client library can reproduce.

@JoshuaFox
Copy link

My requirement was for comparability between Datastore Cloud API and Datastore GAE API; the Cloud Console (the GUI) is less relevant.

@garrettjonesgoogle
Copy link
Member

@JoshuaFox ok, but the issue is the same anyway, right?

@JoshuaFox
Copy link

I don't necessarily know the implementation details, but if the Cloud Console value is generated by the GAE API; in contrast to the Cloud API; then yes.

@koichirokamoto
Copy link

Is there any plan to add implementation comparable to GAE API?
FWIW, in cloud golang library, implementation is at https://github.com/GoogleCloudPlatform/google-cloud-go/blob/master/datastore/key.go#L198.
Here is GAE API at https://github.com/golang/appengine/blob/master/datastore/key.go#L211.
It seems they are comparable. I would hope that implementation is also added in Java library.

@tcoffee-google tcoffee-google added priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. and removed type: question Request for information or clarification. Not an issue. labels Jun 12, 2017
@garrettjonesgoogle garrettjonesgoogle added priority: p2 Moderately-important priority. Fix may not be included in next release. and removed priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. labels Jul 18, 2017
@danoscarmike
Copy link
Contributor

@garrettjonesgoogle have you had a chance to check if Go can achieve this compatibility why Java cannot?

@danoscarmike danoscarmike removed priority: p2 Moderately-important priority. Fix may not be included in next release. triaged for GA labels Aug 9, 2017
@garrettjonesgoogle
Copy link
Member

Theoretically Java could, it would just require duplicating code instead of reusing it.

@garrettjonesgoogle garrettjonesgoogle added the priority: p2 Moderately-important priority. Fix may not be included in next release. label Aug 11, 2017
@garrettjonesgoogle garrettjonesgoogle changed the title Return value from Key.toUrlSafe() is inconsistent with the URL-safe key displayed in Google Cloud Datastore Console Make Key.toUrlSafe() consistent with the URL-safe key displayed in Google Cloud Datastore Console Aug 11, 2017
@garrettjonesgoogle
Copy link
Member

This has been added to our feature backlog: https://github.com/GoogleCloudPlatform/google-cloud-java/wiki/Feature-backlog . This issue will be closed but is linked in the backlog and can continue to be used for comment and discussion.

@kwiesmueller
Copy link

How is the status on this?
This issue seems to persist for quite some time now and we are unable to use the url safe key spanning services built in various languages due to this.

github-actions bot pushed a commit to suztomo/google-cloud-java that referenced this issue Jun 29, 2022
…oogleapis#725)

Source-Link: googleapis/synthtool@e122cb0
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-java:latest@sha256:387835a1375a0049ec44e02542c844302854c732d8291bdf8e472c0ff70a8f67
github-actions bot pushed a commit that referenced this issue Jul 1, 2022
Source-Link: googleapis/synthtool@e122cb0
Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-java:latest@sha256:387835a1375a0049ec44e02542c844302854c732d8291bdf8e472c0ff70a8f67
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: datastore Issues related to the Datastore API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests