-
Notifications
You must be signed in to change notification settings - Fork 76
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
[SVCS-256] Adding "undecoded_path" to providers #248
base: develop
Are you sure you want to change the base?
[SVCS-256] Adding "undecoded_path" to providers #248
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs some styling fixes and better support for corner cases. Please reformat the commit message, too. General rule of thumb is 50 chars for the title, wrap the body at 72. I don't mind a little longer than 50 for a good title, but don't go overboard.
@@ -62,6 +62,9 @@ class ProviderHandler(core.BaseHandler, CreateMixin, MetadataMixin, MoveCopyMixi | |||
|
|||
self.auth = await auth_handler.get(self.resource, provider, self.request) | |||
self.provider = utils.make_provider(provider, self.auth['auth'], self.auth['credentials'], self.auth['settings']) | |||
# Find start of provider name, and start of ? marker and pull out file name from between them. | |||
self.provider.undecoded_path = self.request.uri[self.request.uri.rfind(self.path_kwargs['provider'] + '/') + (len(self.path_kwargs['provider'])): self.request.uri.rfind("?")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line length!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please break down the components of this indexing operation into individual variables with descriptive names. This is hard to skim.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indexing issue:
http://localhost:7777/v1/resources/p8a63/providers/googledrive/me%2fow/provider/googledrive/beef?meta=
self.provider.undecoded_path
is beef
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indexing issue:
http://localhost:7777/v1/resources/p8a63/providers/googledrive/me%2fow/provider/googledrive/beef
self.provider.undecoded_path
is bee
Currently, a URL can get encoded and decoded many times. This can cause the original name of a file to be near impossible to figure out if it uses special characters. This variable will help solve this problem.
a19fc66
to
a8717a5
Compare
Added requested Changes |
@AddisonSchiller Might want to wrap up this conflict. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For Campfire ⛺️ 🔥 , see #210 (review).
refs: https://openscience.atlassian.net/browse/SVCS-256
Purpose
We need the un-decoded file name to distinguish between files with special characters.
ie: "me/ow.txt and me%2Fow.txt" The decoding tornado does makes these files hard to distinguish.
Summary of changes
Added an "undecoded_path" variable to the base_provider. This gets set in the "prepare" function to be the undecoded_path received from the request. The path is pulled from 'self.request.uri'
QA
QA will come on individual tickets for providers