Add Smart CDN Signature Auth helper

[kvz]

Allows you to generate signed URLs for Transloadit's Smart CDN.

[Acconut]

I have the same comment as in the Ruby SDK. Can we also compare the generated URL against a static string here?

[kvz]

That's what we have the unit tests for:

php-sdk/test/simple/TransloaditTest.php

Lines 149 to 178 in fe2748d

	public function testSignedSmartCDNUrl() {
	$transloadit = new Transloadit([
	'key' => 'test-key',
	'secret' => 'test-secret'
	]);
	// Test basic URL generation
	$url = $transloadit->signedSmartCDNUrl('workspace', 'template', 'file.jpg');
	$this->assertStringStartsWith('https://workspace.tlcdn.com/template/file.jpg', $url);
	$this->assertStringContainsString('auth_key=test-key', $url);
	$this->assertStringContainsString('sig=sha256%3A', $url);
	// Test with input field
	$url = $transloadit->signedSmartCDNUrl('workspace', 'template', 'input.jpg');
	$this->assertStringStartsWith('https://workspace.tlcdn.com/template/input.jpg', $url);
	// Test with additional params
	$url = $transloadit->signedSmartCDNUrl('workspace', 'template', 'file.jpg', ['width' => 100]);
	$this->assertStringContainsString('width=100', $url);
	// Test with custom sign props
	$url = $transloadit->signedSmartCDNUrl(
	'workspace',
	'template',
	'file.jpg',
	[],
	['authKey' => 'custom-key', 'authSecret' => 'custom-secret', 'expiryMs' => 60000]
	);
	$this->assertStringContainsString('auth_key=custom-key', $url);
	}

The parity test is to check it against the reference implementation. When we make changes, it's nice to see if the reference implementation would disagree in CI. After we finalize the reference implementation in the Node SDK, I'll use that SDK to verify parity, instead of this copy pasted function.

[Acconut]

Thanks for pointing to these unit tests. I think it would be useful if they assert the entire URL and not just parts of it, making the test case easier to understand/debug and help find subtle bugs outside of the examined parts of the URL.
The method has a field to define the expiration time, so it should be possible to generate them deterministically.

[kvz]

I now merged parity tests with regular tests to make it a bit more obvious, as well as checking both against hardcoded URLs

[Acconut]

The test name suggests that this logic covers situations where input might be empty, but as far as I can tell $params['input'] is always non-empty.

[kvz]

Empty string input is now allowed

[Acconut]

Same as in the Ruby SDK PR. Can we allow empty string values?

[kvz]

Do you want the empty parameter to show up? I don't think the node implementation did that at the time of writing?

For instance:

    // Test with empty param string
    $url = $transloadit->signedSmartCDNUrl('workspace', 'template', 'file.jpg', ['width' => '', 'height' => '200']);
    $this->assertStringNotContainsString('width=', $url, 'Empty parameter should be excluded from URL');

or

   // Test with empty param string
   $url = $transloadit->signedSmartCDNUrl('workspace', 'template', 'file.jpg', ['width' => '', 'height' => '200']);
   $this->assertStringContainsString('width=', $url, 'Empty parameter should be excluded from URL');

[Acconut]

I would expect the parameter to show up.

[kvz]

Okay, let's also fix this on the node side then, currently these SDKs are complaining about that bit

[kvz]

Or maybe it was a bad port by me 🤔

[Acconut]

The Node SDK PR already supports empty values in my tests.

[kvz]

Empty string e.g. width is now supported and will be shown in the URL

[github-actions]

Coverage report for commit: 09c9e2d
File: ./build/logs/clover.xml

Summary - Lines: 91.02% | Methods: 69.57%

Files	Lines	Methods	Branches
lib/transloadit
CurlRequest.php	91.67%	66.67%	100.00%
CurlResponse.php	100.00%	100.00%	100.00%
Transloadit.php	86.55%	55.56%	100.00%
TransloaditRequest.php	95.92%	71.43%	100.00%
TransloaditResponse.php	100.00%	100.00%	100.00%

_{🤖 comment via lucassabreu/comment-coverage-clover}

[Acconut]

I am no PHP expert but the logic looks sounds

[kvz]

Released as 3.2.0