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

Uses SavedObjectsClient for Short URL Lookup #12787

Merged
merged 1 commit into from
Jul 12, 2017

Conversation

tylersmalley
Copy link
Contributor

No description provided.

@tylersmalley tylersmalley requested review from nreese and kobelb July 12, 2017 02:53
@tylersmalley tylersmalley force-pushed the short-url-saved-objects branch 2 times, most recently from 5893062 to e5dac08 Compare July 12, 2017 04:42
@tylersmalley
Copy link
Contributor Author

@nreese @kobelb, mind taking a look at this one? It's another blocker on the 5.6/6.0 release as we need to use the SavedObjectsClient as opposed to hitting the index directly. While at it, I changed to simply using the autogenerated ID's from elasticsearch instead of creating them ourselves.

@kobelb
Copy link
Contributor

kobelb commented Jul 12, 2017

@tylersmalley from what I can determine, we were originally creating the ID based on a md5 hash of the long-url, so that we don't end up creating a bunch of redundant documents in the .kibana index. Now that we're using an auto-generated ID, this is no longer the case.

With the way that the short-url is implemented via the UI, this could be problematic. Every time that you click the "Short URL" button in Kibana, it's calling the generateUrlId function, and now that the short ids aren't consistent this can create duplicate documents.

This is the way that it used to work if I continually toggle between short urls and long urls, you'll notice that the ID stays the same, and only one document is created:

short-url-toggle-old

This is how it works with these changes, keep in mind every time you see the short-id change it's created a new document:

short-url-toggle-new

@tylersmalley
Copy link
Contributor Author

@kobelb, updated.

> curl -i -X POST -H "Content-Type: application/json" -H "kbn-xsrf: 6.0.0-beta1" -d '{ "url": "/app/kibana#/visualize/edit" }' http://localhost:5601/shorten
HTTP/1.1 200 OK
kbn-name: kibana
kbn-version: 6.0.0-beta1
vary: origin
content-type: text/html; charset=utf-8
cache-control: no-cache
content-length: 32
Date: Wed, 12 Jul 2017 13:14:05 GMT
Connection: keep-alive

fbbd9c36d182b59b0d61e9db7796292c
curl -i http://localhost:5601/goto/fbbd9c36d182b59b0d61e9db7796292c
HTTP/1.1 302 Found
location: /app/kibana#/visualize/edit
kbn-name: kibana
kbn-version: 6.0.0-beta1
vary: origin
cache-control: no-cache
content-length: 0
Date: Wed, 12 Jul 2017 13:14:25 GMT
Connection: keep-alive

@tylersmalley tylersmalley force-pushed the short-url-saved-objects branch from ebb93fb to 00973dc Compare July 12, 2017 14:12
Copy link
Contributor

@kobelb kobelb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tylersmalley tylersmalley merged commit ef492a7 into elastic:master Jul 12, 2017
tylersmalley added a commit that referenced this pull request Jul 12, 2017
@tylersmalley
Copy link
Contributor Author

5.x/5.6: 8d1bbd9

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants