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

Caching remote gifs are saved based on their remote filename and not overwritten #105

Closed
KelvinTegelaar opened this issue Jul 17, 2020 · 6 comments
Assignees
Labels

Comments

@KelvinTegelaar
Copy link
Contributor

Steps to reproduce

$heroimage = New-BTImage -Source 'https://media.giphy.com/media/eiwIMNkeJ2cu5MI2XC/giphy.gif' -HeroImage
$heroimage2 = New-BTImage -Source 'https://otherhost.com/giphy.gif' -HeroImage

Expected behavior

$Heroimage and $Heroimage 2 are expected to be 2 different images, instead its both $heroimage

Actual behavior

It seems that the cached file retains its previous name. if the name is a duplicate it no longer downloads the file believing it's already been cached.

Environment data

Name                           Value
----                           -----
PSVersion                      5.1.18362.752
PSEdition                      Desktop
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
BuildVersion                   10.0.18362.752
CLRVersion                     4.0.30319.42000
WSManStackVersion              3.0
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
OsName         : Microsoft Windows 10 Pro
OsVersion      : 10.0.18363
OsArchitecture : 64-bit
ModuleType Version    Name                                ExportedCommands
---------- -------    ----                                ----------------
Script     0.7.1      BurntToast                          {Get-BTHistory, New-BTAction, New-BTAppId, New-BTAudio...}
@KelvinTegelaar
Copy link
Contributor Author

Created pull request #106 to resolve.

@KelvinTegelaar KelvinTegelaar changed the title Caching remote gifs are saved based on their username and not overwritten Caching remote gifs are saved based on their remote filename and not overwritten Jul 17, 2020
@Windos
Copy link
Owner

Windos commented Jul 17, 2020

Good catch @KelvinTegelaar!

This is actually expected behaviour... but I hadn't thought through enough the case where images were named the same but are actually different (prime culprits of this being giphy and generic "image.png").

The intention was that you could reference a large image and grab a local copy of it that you can reference repeatedly (using the online URI) without having to download it everytime.

I think what might work here is instead of just taking the file name, we take all (or part) of the URI and use that in the local filename.

e.g.

https://media.giphy.com/media/eiwIMNkeJ2cu5MI2XC/giphy.gif -> media.giphy.com-media-eiwIMNkeJ2cu5MI2XC-giphy.gif

https://otherhost.com/giphy.gif -> otherhost.com-giphy.gif

In addition, maybe a switch could be added to New-BTImage that ignores any cached images and re-downloads, this was a toast author that has replaced the upstream image can ensure the client is using the updated version.

@KelvinTegelaar
Copy link
Contributor Author

How about:

$Source -replace '/|:','-'

Which would make:
https://media.giphy.com/media/eiwIMNkeJ2cu5MI2XC/giphy.gif -> https---media.giphy.com-media-eiwIMNkeJ2cu5MI2XC-giphy.gif

https://otherhost.com/giphy.gif -> https---otherhost.com-giphy.gif

not as clean as your example, but it does make life easier. ;)

@Windos
Copy link
Owner

Windos commented Jul 17, 2020

It'll work. Humans don't need to interact with the resulting file 😂

I haven't tested anything, but assume that $RemoteFileName = $Source.Split('/\')[-1] -replace '[\[\]*?]','' could be changed to $RemoteFileName = $Source -replace '/|:', '-'

No more splitting, love it.

@KelvinTegelaar
Copy link
Contributor Author

updated PR #106 with fix. :)

Windos added a commit that referenced this issue Jul 17, 2020
@Windos
Copy link
Owner

Windos commented Aug 6, 2020

v0.7.2 has shipped with this fix in place, thanks again @KelvinTegelaar!

@Windos Windos closed this as completed Aug 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants