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

feature: temporary url support #56

Conversation

WalrusSoup
Copy link

@WalrusSoup WalrusSoup commented Jan 18, 2023

Temporary URL Support

This PR adds support for temporary urls based on this documentation and validated using the first-party example here and solves this issue #50

Important Notes

  • /assets/ inside of the pullzone in tests were rewritten to include /assets/ in the call instead. I am not sure why this was done, because as far as I can see with bunny we are required to set the pullzone to point into a storage point in its entirety.
  • I wanted to add the pullzone auth token as a parameter on the adapter because it is different than the storage API key, but adding another key broke the arg-check (its limited to 2 args) - so I added it as a separate call setPullzoneToken

Tests Added

  • test_generating_a_temporary_url
  • test_generating_temporary_url_supports_image_optimization

Example Usage

Setup adapter:

$time = (new \DateTime('now'))->modify('+5 minutes');
$adapter = new \PlatformCommunity\Flysystem\BunnyCDN\BunnyCDNAdapter($client, new BunnyPullZone('https://[redacted].b-cdn.net', '');
$adapter->temporaryUrl('/images/saber2.png', $time);

//  https://[redacted]/images/saber2.png?token=[redacted]&expires=1674085360

saber2

With support for optimizer params:

$adapter->temporaryUrl('/images/saber2.png', $time, ['width' => 200]);

// https://[redacted]/images/saber2.png?token=[redacted]&width=200&expires=1674085307

saber2-smol

@tinect
Copy link
Contributor

tinect commented Jan 22, 2023

Great work! Thank you.

I'm confused by the taken name pullZoneKey. Could you please name it more meaningful? It doesn't sound like it belongs to the Token Authentification to me.

On bunnyCDN panel it's called Url Token Authentication Key, but in API it's called ZoneSecurityKey, damn :-(

@WalrusSoup
Copy link
Author

WalrusSoup commented Jan 23, 2023

On bunnyCDN panel it's called Url Token Authentication Key, but in API it's called ZoneSecurityKey, damn :-(

Totally agree with the confusing names. The fact that token authentication is per pull zone is why I chose the name pullzonekey. I didn't want to use token, because token makes it sound like its the primary user token for uploads\deletions\etc against storage when it's not.

I decided to spice things up and go with my intuition, which was to change the approach to using the object based approach where a pullzone can be passed in as a second constructer argument.

BunnyCDNAdapter(self::bunnyCDNClient(), new BunnyPullZone('https://example.org.local/', 'security-token-if-needed'));

I updated all the tests, what do you think about that approach? To me, it seems "correct" way to support this.

* fix: adjusting tests for new structure

* fix: error messages

* feat: pull zone object

* docs: adding phpdoc blocks
@WalrusSoup
Copy link
Author

Ima close this for now - i think a larger overhaul is in order and i believe bunny should be finish s3 compatible drivers soon.

@WalrusSoup WalrusSoup closed this Feb 21, 2023
@sifex
Copy link
Collaborator

sifex commented Feb 22, 2023

Hey @WalrusSoup , lmk if you want to reopen this I can have a look at merging tonight

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

Successfully merging this pull request may close these issues.

3 participants