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

[SVCS-256] Adding "undecoded_path" to providers #248

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

AddisonSchiller
Copy link
Contributor

@AddisonSchiller AddisonSchiller commented Aug 23, 2017

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

@coveralls
Copy link

Coverage Status

Coverage increased (+0.008%) to 76.688% when pulling 7ea5079 on AddisonSchiller:feature/url_decode into b787a92 on CenterForOpenScience:develop.

Copy link
Member

@felliott felliott left a 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("?")]
Copy link
Member

Choose a reason for hiding this comment

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

Line length!

Copy link
Member

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.

Copy link
Member

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.

Copy link
Member

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

@coveralls
Copy link

coveralls commented Sep 11, 2017

Coverage Status

Coverage increased (+0.02%) to 79.342% when pulling a19fc66 on AddisonSchiller:feature/url_decode into 879aedd on CenterForOpenScience:develop.

@coveralls
Copy link

coveralls commented Sep 11, 2017

Coverage Status

Coverage increased (+0.02%) to 79.342% when pulling a19fc66 on AddisonSchiller:feature/url_decode into 879aedd on CenterForOpenScience:develop.

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.
@AddisonSchiller
Copy link
Contributor Author

Added requested Changes

@coveralls
Copy link

coveralls commented Sep 11, 2017

Coverage Status

Coverage increased (+0.02%) to 79.342% when pulling a8717a5 on AddisonSchiller:feature/url_decode into 879aedd on CenterForOpenScience:develop.

@AddisonSchiller AddisonSchiller changed the title Adding "undecoded_path" to providers. This should be the first step f… [SVCS-256] Adding "undecoded_path" to providers Oct 5, 2017
@Johnetordoff
Copy link
Contributor

Johnetordoff commented Nov 28, 2017

@AddisonSchiller Might want to wrap up this conflict.

Copy link
Contributor

@cslzchen cslzchen left a 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).

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.

5 participants