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

Specify filename (not just directory) when downloading GDrive file #188

Closed
cspotcode opened this issue May 20, 2019 · 13 comments
Closed

Specify filename (not just directory) when downloading GDrive file #188

cspotcode opened this issue May 20, 2019 · 13 comments
Assignees

Comments

@cspotcode
Copy link

Is your feature request related to a problem? Please describe.

I have Google Drive files that contain forward slashes in their filename (not their path, actually in the filename itself) I want to download the contents of such a file, but PSGSuite is trying to create a local file with slashes in the filename. This fails because it expects to traverse into a subdirectory that does not exist.

Describe the solution you'd like

I want to be able to specify the full, local filename to download to. This allows me to bypass any issues with the GDrive filename. For example:

Get-GSDriveFile -FileID $idOfFileWithWeirdFilename -OutFilenamePath /foo/bar/normal-filename.zip

It will download the contents of the remote file and save it as baz.zip, regardless of the filename on GDrive.

Describe alternatives you've considered

Additional context

Here's the error I get. The filename (name, not path) on Google Drive is backup__2019-05-19T04:24:43Z__Personal-dev/@creationix/nvm.zip The name containers forward slash characters, which GDrive allows.

$ Get-GSDriveFile -FileId $id -OutFilePath '/tmp/157ee048-e182-4e13-94b5-abf0b1d3cd87'
Get-GSDriveFile : Exception calling "Create" with "1" argument(s): "Could not find a part of the path '/tmp/157ee048-e182-4e13-94b5-abf0b1d3cd87/backup__2019-05-19T04:24:43Z__Personal-dev/@creationix/nvm.zip.zip'."
@WJurecki
Copy link
Contributor

The invalid filename part of this issue could be addresses as you did in #169.

While I could also find specifying a desired filename would be beneficial, I don't know how to suggest to reconcile this filename option with the ability to send an array of File IDs to download.

@cspotcode
Copy link
Author

#169 sounds like it will change the target filename to be safe, but will not tell me what that target filename is. So I have to do some manual string manipulation and hope it matches the behavior of PSGSuite. I'm writing an automated backup and restore script, so immediately after downloading a file, I need to pass it to unzip and then delete it. The filename's arbitrary, but I need to have easy access to it.

For sending an array of FileIDs, can I simply specify an array of local filenames to match, with a one-to-one relationship? Or simply disallow multiple FileIDs when I supply a target filename? So if I want to download multiple FileIDs to specific local filenames, I'll need to do it in a foreach() loop.

@WJurecki
Copy link
Contributor

@cspotcode, I see the issue you would have with not knowing the output filename. I would as well in my intended process.

How about a property added to the output object to provide the actual file name used to output the file?

In this case the output object would have a name property with the original/Google Drive name, in this case, a value of backup__2019-05-19T04:24:43Z__Personal-dev/@creationix/nvm.zip and the new OutputName property with a value of backup__2019-05-19T04_24_43Z__Personal-dev_@creationix_nvm.zip

@scrthq I hope this discussion helps. I'm not trying to dictate any design ideas, but rather come up with a potential solution that made fit more needs. Your choice!

@cspotcode
Copy link
Author

That would work for me.

Today I generate a temp filename using OS APIs, download to that filename, then delete the temp file when I'm done. (I've patched my local copy of PSGSuite)

With your proposed solution, I would instead generate a new temp directory name, create the new empty directory, and download the file into that new directory. Then I would delete my temp directory. This would be necessary to avoid any possible name conflicts with other files in the OS's temp directory.

On Linux, mktemp can generate either files or directories, so both approaches are easy. On Windows, I'm not sure there's a straightforward OS-provided way to safely create temp directories, other than creating it in the system's temp directory with a randomly-generated name.

@scrthq
Copy link
Member

scrthq commented May 24, 2019

@cspotcode - I appreciate the feedback! How does expanding the OutFilePath parameter to allow full paths to be specified, with it falling back to using a special-character-replaced version of the file name as is in Drive?

To satisfy the resulting FileName piece, I can also add in the final OutFilePath value to the returned object as well.


Sample code to test the special-character-replacement against a string of your own (feedback appreciated in case there are characters missed!):

$Name = 'backup__2019-05-19T04:24:43Z__Personal-dev/@creationix/nvm.zip'
$Name -replace "[$(([System.IO.Path]::GetInvalidFileNameChars() + [System.IO.Path]::GetInvalidPathChars()) -join '')]","_"

Resulting output on Windows 10: backup__2019-05-19T04_24_43Z__Personal-dev_@creationix_nvm.zip


Sample returned object when downloading something to a specified path:

image


@WJurecki - Thanks for jumping in here!!

@scrthq
Copy link
Member

scrthq commented May 24, 2019

@cspotcode - With the above adjustment, existing usage could continue to function as is and you also have the option of piping in filenames and capturing the OutFilePath value of the resulting object as well as specifying it per file via loop.

@scrthq scrthq self-assigned this May 24, 2019
scrthq added a commit that referenced this issue May 31, 2019
…, #197 (#198)

## 2.28.0

* [Issue #188](#188)
  * Added: `Get-GSDriveFile` now supports specifying a full file path.
  * Fixed: `Get-GSDriveFile` will now replace any special path characters in the filename with underscores
  * Added: The File object returned by `Get-GSDriveFile` will now include an additional `OutFilePath` property if the file is downloaded. This property will contain the full path to the downloaded file.
* [Issue #190](#190)
  * Fixed: `Fields` parameter on `Get-GSDriveFile` and `Update-GSDriveFile` were not being honored.
* [Issue #192](#192)
  * Added: Parameters to `Update-GSDriveFile`:
    * `CopyRequiresWriterPermission [switch]`
    * `Starred [switch]`
    * `Trashed [switch]`
    * `WritersCanShare [switch]`
* [Issue #194](#194)
  * Added: Parameters to `Update-GSChromeOSDevice`:
    * `AnnotatedAssetId [string]`
    * `AnnotatedLocation [string]`
    * `AnnotatedUser [string]`
    * `Notes [string]`
* [Issue #195](#195)
  * Added: `Limit` parameter with `First` alias to the following `List` functions:
    * `Get-GSActivityReport`
    * `Get-GSAdminRole`
    * `Get-GSAdminRoleAssignment`
    * `Get-GSCalendar`
    * `Get-GSCalendarAcl`
    * `Get-GSCalendarEvent`
    * `Get-GSChromeOSDevice`
    * `Get-GSDataTransferApplication`
    * `Get-GSDrive`
    * `Get-GSDriveFileList`
    * `Get-GSDrivePermission`
    * `Get-GSGmailMessageList`
    * `Get-GSGroup`
    * `Get-GSGroupMember`
    * `Get-GSMobileDevice`
    * `Get-GSResource`
    * `Get-GSTask`
    * `Get-GSTaskList`
    * `Get-GSUsageReport`
    * `Get-GSUser`
    * `Get-GSUserLicense`
* [Issue #196](#196)
  * Fixed: `Get-GSTeamDrive` was not paginating through the results.
* [Issue #197](#197)
  * Renamed: `Get-GSTeamDrive` has been changed to `Get-GSDrive`. `Get-GSTeamDrive` has been turned into an alias for `Get-GSDrive` to maintain backwards compatibility.
  * Replaced: `SupportsTeamDrives = $true` with `SupportsAllDrives = $true` on all functions that have it.
* Miscellaneous
  * Fixed: `Export-PSGSuiteConfig` is faster due to safely assuming that the P12Key and/or ClientSecrets values have already been pulled from the corresponding keys.
  * Fixed: Incomplete documentation for `Test-GSGroupMembership`.
  * Added: `UseDomainAdminAccess` switch parameter to `Get-GSTeamDrive`
  * Removed: `Get-GSUserLicenseListPrivate` by rolling the `List` code into `Get-GSUserLicense`
  * Removed: `Get-GSResourceListPrivate` by rolling the `List` code into `Get-GSResource`
scrthq added a commit that referenced this issue May 31, 2019
…, #197

## 2.28.0

* [Issue #188](#188)
  * Added: Get-GSDriveFile now supports specifying a full file path.
  * Fixed: Get-GSDriveFile will now replace any special path characters in the filename with underscores
  * Added: The File object returned by Get-GSDriveFile will now include an additional OutFilePath property if the file is downloaded. This property will contain the full path to the downloaded file.
* [Issue #190](#190)
  * Fixed: Fields parameter on Get-GSDriveFile and Update-GSDriveFile were not being honored.
* [Issue #192](#192)
  * Added: Parameters to Update-GSDriveFile:
    * CopyRequiresWriterPermission [switch]
    * Starred [switch]
    * Trashed [switch]
    * WritersCanShare [switch]
* [Issue #194](#194)
  * Added: Parameters to Update-GSChromeOSDevice:
    * AnnotatedAssetId [string]
    * AnnotatedLocation [string]
    * AnnotatedUser [string]
    * Notes [string]
* [Issue #195](#195)
  * Added: Limit parameter with First alias to the following List functions:
    * Get-GSActivityReport
    * Get-GSAdminRole
    * Get-GSAdminRoleAssignment
    * Get-GSCalendar
    * Get-GSCalendarAcl
    * Get-GSCalendarEvent
    * Get-GSChromeOSDevice
    * Get-GSDataTransferApplication
    * Get-GSDrive
    * Get-GSDriveFileList
    * Get-GSDrivePermission
    * Get-GSGmailMessageList
    * Get-GSGroup
    * Get-GSGroupMember
    * Get-GSMobileDevice
    * Get-GSResource
    * Get-GSTask
    * Get-GSTaskList
    * Get-GSUsageReport
    * Get-GSUser
    * Get-GSUserLicense
* [Issue #196](#196)
  * Fixed: Get-GSTeamDrive was not paginating through the results.
* [Issue #197](#197)
  * Renamed: Get-GSTeamDrive has been changed to Get-GSDrive. Get-GSTeamDrive has been turned into an alias for Get-GSDrive to maintain backwards compatibility.
  * Replaced: SupportsTeamDrives = True with SupportsAllDrives = True on all functions that have it.
* Miscellaneous
  * Fixed: Export-PSGSuiteConfig is faster due to safely assuming that the P12Key and/or ClientSecrets values have already been pulled from the corresponding keys.
  * Fixed: Incomplete documentation for Test-GSGroupMembership.
  * Added: UseDomainAdminAccess switch parameter to Get-GSTeamDrive
  * Removed: Get-GSUserLicenseListPrivate by rolling the List code into Get-GSUserLicense
  * Removed: Get-GSResourceListPrivate by rolling the List code into Get-GSResource
@scrthq
Copy link
Member

scrthq commented May 31, 2019

Hey @cspotcode - This has been deployed in v2.28.0! Let me know if all is well or if you are seeing any issues!

@WJurecki
Copy link
Contributor

@scrthq - This appears to mostly work.

There is a problem in your private Get-SafeFileName

Consider this:

image

the \ still made it into the 'clean name' because the \ in the replace command needs to be escaped and actually \\ to work properly.

image

Also, you should probably consider not only the name needs to be cleaned, but also the extension.

consider these perfectly valid Drive filenames
image

Sorry for the bad news. :-(

@scrthq
Copy link
Member

scrthq commented May 31, 2019

Hey @WJurecki - Fair! The escaping is done against the current OS's invalid file and path characters, leaving anything alphanumeric or [System.IO.Path]::DirectorySeparatorChar alone. On Windows, [System.IO.Path]::DirectorySeparatorChar -eq '\', so that's why that's left alone after being returned from Get-SafeFileName.

I definitely see your point though in that the files in Drive are just that, files, so it would make sense to also replace any valid path separators so the resulting file actually is a file and not something that got buried in an unexpected naming path. Testing it out actually returns an error since it's trying to access a path that doesn't exist due to the DirectorySeparatorChar being in the FileName:

image

@WJurecki
Copy link
Contributor

@scrthq

but [System.IO.Path]::GetInvalidFileNameChars() returns "<>|:*?\/ (along with some non-printable characters) and [System.IO.Path]::GetInvalidPathChars() returns "<>|

I really appears that, at least on Windows, -replace needs \ to be escaped as \\ to work correctly.

in the code "[$(([System.IO.Path]::GetInvalidFileNameChars() + [System.IO.Path]::GetInvalidPathChars()) -join '')]" results in ["<>|:*?\/"<>|] (non-printable characters removed for display) when it really needs to be ["<>|:*?\\/"<>|] to work correctly.

@scrthq
Copy link
Member

scrthq commented May 31, 2019

Aha! This should do the trick instead, just wrapped the resulting character string in [RegEx]::Escape() so that special RegEx chars like \ are treated as literal. I wanted to avoid explicitly calling out chars to replace in the event that specific chars are acceptable depending on the OS.

Test block:

[PS 6.2.0]> 'Test\File/|Name' -replace "[$([RegEx]::Escape("$(([System.IO.Path]::GetInvalidFileNameChars() + [System.IO.Path]::GetInvalidPathChars()) -join '')"))]","_"
Test_File__Name

Thoughts @WJurecki ?

@WJurecki
Copy link
Contributor

WJurecki commented Jun 1, 2019

@scrthq, that looks great!

scrthq added a commit that referenced this issue Jun 1, 2019
## 2.28.1

* [Issue #188](#188)
  * Fixed: Get-SafeFileName correctly replaces special RegEx chars with underscores as well.
@scrthq scrthq closed this as completed in db1967b Jun 1, 2019
@scrthq
Copy link
Member

scrthq commented Jun 1, 2019

@WJurecki fixed rolled out in 2.28.1!

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

No branches or pull requests

3 participants