Skip to content

Commit

Permalink
feat: use https by default (#53)
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
ericdeansanchez authored Mar 31, 2020
1 parent 5ada0d8 commit ac02e69
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 1 deletion.
2 changes: 1 addition & 1 deletion src/Imgix/UrlBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
13 changes: 13 additions & 0 deletions tests/Imgix/Tests/UrlBuilderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down

0 comments on commit ac02e69

Please sign in to comment.