Skip to content

[SVCS-234][SVCS-21][SVCS-273] Fix resolve_to_ids to better disambiguate paths, extensions etc.#210

Open
Johnetordoff wants to merge 2 commits intoCenterForOpenScience:developfrom
Johnetordoff:disambiguate-slashes
Open

[SVCS-234][SVCS-21][SVCS-273] Fix resolve_to_ids to better disambiguate paths, extensions etc.#210
Johnetordoff wants to merge 2 commits intoCenterForOpenScience:developfrom
Johnetordoff:disambiguate-slashes

Conversation

@Johnetordoff
Copy link
Copy Markdown
Contributor

@Johnetordoff Johnetordoff commented Apr 11, 2017

Purpose

This arose out of the confluence of three tickets:

  1. Support slashes in google drive file names
  2. Google Docs (and spreadsheets, presentations) are accessible w/ and w/o extension
  3. Some read-only GDocs on GoogleDrive show up as 'Not Found'

After trying to resolve each individually I decided they had to be fixed together within the same function. Since Gdrive doesn't really use a file tree structure, we have to do some detective work to disambiguate the path we are given. There are many edge cases and tricky situations, but this should solve the solvable ones. Here are the two basic problems with passing only a path to GDrive.

  1. The extension problem:
    Files which are unique to Gdrive (gdocs, gsheets, ext) can have the same name so for example a gdoc and gsheet can both be called "Test" with no extension. So in order to tell the difference between these files we require the user to add supply an extension (Test.gdoc for gdocs, Test.gsheet for gsheets, etc.) We use the mimeType in the metadata to pick out the correct file. There are exceptions to this rule, if the user doesn't have permission to download the file they can't use the extension.

  2. The slash problem:
    When we get the path, we divide it into parts by splitting the path by the '/' characters. Example: 1/2/3 is split into parts "1", "2", "3", but this creates a problem if a file contains a '/', Example 1/2/3 is actually "1", "2/3". We resolve this problem by assuming there are no folders slashes in them, so any time a folder.

Changes

Added utility functions for sorting json response items, reworked resolve_to_ids function to better parse path and find files by extension fixed tests and added comments.

Side Effects

Everything works aside from '/' containing folder names.

Tickets / PRs

  1. Support slashes in google drive file names

  2. Google Docs (and spreadsheets, presentations) are accessible w/ and w/o extension
    PR: [SVCS-21] Add exception to prevent accessing gdoc files without file extension #195

  3. Some read-only GDocs on GoogleDrive show up as 'Not Found'

  4. Google Drive .g files of the same name do not display correctly
    Found this one after I put in this PR should be closable if this goes through.

@Johnetordoff Johnetordoff changed the title [SVCS-234][SVCS-21][SVCS-273] Fix resolve_to_ids to better disambiguate paths, extensions ect. [SVCS-234][SVCS-21][SVCS-273][SVCS-37] Fix resolve_to_ids to better disambiguate paths, extensions ect. Apr 13, 2017
@Johnetordoff Johnetordoff changed the title [SVCS-234][SVCS-21][SVCS-273][SVCS-37] Fix resolve_to_ids to better disambiguate paths, extensions ect. [SVCS-234][SVCS-21][SVCS-273] Fix resolve_to_ids to better disambiguate paths, extensions ect. May 16, 2017
@cslzchen cslzchen self-assigned this Jun 27, 2018
Copy link
Copy Markdown
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.

This is a review for Campfire ⛺️ 🔥 .

This PR involves four tickets/PRs.

  • SVCS-234 is the original ticket for this PR.
  • SVCS-256 solves the root problem which will deprecate this PR.
  • Both SVCS-21 and SVCS-273 have their own PR and has been merged.

I suggest we go with SVCS-256 the fix directly and deprecate this one.

  • SVCS-256 fix the root bug, and I think it is the right fix.
  • SVCS-256 fix is far simpler than this one.

@cslzchen cslzchen changed the title [SVCS-234][SVCS-21][SVCS-273] Fix resolve_to_ids to better disambiguate paths, extensions ect. [SVCS-234][SVCS-21][SVCS-273] Fix resolve_to_ids to better disambiguate paths, extensions etc. Jun 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants