From 21694d41caa29e317fa74eb8a10964c14d2c981d Mon Sep 17 00:00:00 2001 From: Eric Sanchez Date: Mon, 30 Mar 2020 15:37:01 -0700 Subject: [PATCH] feat: use https by default To summarize the following, this PR changes the default behavior of the `UrlBuilder` class. Before this change the default behavior used `http` by default. After this change, the default behavior uses `https` __by default__. When I began, I made a small change in the constructor definition and ran the test. All passed. I removed the change and began this PR by writing a _failing test_ first. I re-ran the test suite and this test, `testExamplePlainUsesHttpsByDefault`, failed as intended. I then made the appropriate change in the constructor definition, re-ran the tests, all passed. - first, I wrote a failing test; it tests that this SDK does use https by default, it failed - then the default constructor's default-parameter was changed to use `$useHttps = true` (instead of `$useHttps = false`) - this change is to ensure that this SDK uses https by default - with the new test `testExamplePlainUsesHttpsByDefault`, any subsequent changes to this behavior will be a "breaking change" - the naming of `testExamplePlainUsesHttpsByDefault` was used to emulate the naming-style present in the test suite - the structure of `testExamplePlainUsesHttpsByDefault` has been written to emulate the structure of similar tests (e.g. `testExamplePlain`) - **note**: with the given API, this change cannot be tested in isolation and requires a call to `createUrl` - **note**: the current API does not allow for incremental testing of individual builder-fields, e.g. there are no "getters" and the builder-fields are private: ```php private $domain; private $useHttps; private $signKey; ``` - the above note is looking forward to future tweaks to our API to ensure more complete test coverage. **Ensure at least one test case fails before change**: Note that the **expected** value is `https`. ``` Time: 95 ms, Memory: 10.00 MB There was 1 failure: 1) UrlBuilderTest::testExamplePlainUsesHttpsByDefault Failed asserting that two strings are equal. --- Expected +++ Actual @@ @@ -'https://demos.imgix.net/bridge.png?h=100&w=100' +'http://demos.imgix.net/bridge.png?h=100&w=100' /Users/Eric/GitHub/imgix-php/tests/Imgix/Tests/UrlBuilderTest.php:33 FAILURES! Tests: 50, Assertions: 354, Failures: 1. ``` **Ensure all tests pass after change**: Both the **expected** and **actual** values concur. ``` PHPUnit 9.0.1 by Sebastian Bergmann and contributors. .................................................. 50 / 50 (100%) Time: 93 ms, Memory: 10.00 MB OK (50 tests, 354 assertions) ``` This single test should be sufficient for our purposes, here and now. It is the only point of entry (i.e. no "getters"). `UrlHelper` does have a `getUrl` method, but that is not near this change (`getUrl` is not called until `UrlBuilder.createUrl` returns). --- src/Imgix/UrlBuilder.php | 2 +- tests/Imgix/Tests/UrlBuilderTest.php | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/Imgix/UrlBuilder.php b/src/Imgix/UrlBuilder.php index a766a09..aef9099 100644 --- a/src/Imgix/UrlBuilder.php +++ b/src/Imgix/UrlBuilder.php @@ -15,7 +15,7 @@ class UrlBuilder { // constants cannot be dynamically assigned; keeping as a class variable instead private $targetWidths; - public function __construct($domain, $useHttps = false, $signKey = "", $includeLibraryParam = true) { + public function __construct($domain, $useHttps = true, $signKey = "", $includeLibraryParam = true) { if (!is_string($domain)) { throw new \InvalidArgumentException("UrlBuilder must be passed a string domain"); diff --git a/tests/Imgix/Tests/UrlBuilderTest.php b/tests/Imgix/Tests/UrlBuilderTest.php index b288bc3..1ef2976 100644 --- a/tests/Imgix/Tests/UrlBuilderTest.php +++ b/tests/Imgix/Tests/UrlBuilderTest.php @@ -20,6 +20,19 @@ public function testExamplePlain() { $this->assertEquals("http://demos.imgix.net/bridge.png?h=100&w=100", $url); } + public function testExamplePlainUsesHttpsByDefault() { + // Test default `UrlBuilder` uses https by default. + // Construct the builder with a `$domain` __only__. + $builder = new UrlBuilder("demos.imgix.net"); + // Use `setIncludeLibraryParam`. + $builder->setIncludeLibraryParam(false); + // Construct a url in accordance with the other tests. + $params = array("w" => 100, "h" => 100); + // Create the url with the specified `$path` and `$params`. + $url = $builder->createURL("bridge.png", $params); + $this->assertEquals("https://demos.imgix.net/bridge.png?h=100&w=100", $url); + } + public function testExamplePlainHttps() { $builder = new UrlBuilder("demos.imgix.net", false, "", false);