Skip to content

Commit

Permalink
Merge branch 'return-type-hint'
Browse files Browse the repository at this point in the history
  • Loading branch information
tangrufus committed Feb 19, 2019
2 parents af38b7b + 58adb61 commit 883715a
Show file tree
Hide file tree
Showing 16 changed files with 83 additions and 67 deletions.
36 changes: 28 additions & 8 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,13 @@ orbs:
composer: itinerisltd/composer@0

itinerisltd:
orbs:
composer: itinerisltd/composer@0

executors:
default:
parameters:
image:
type: string
default: 'cibuilds/github:latest'
docker:
- image: << parameters.image >>
- image: 'cibuilds/github:latest'

jobs:
publishing-to-github-releases:
Expand Down Expand Up @@ -62,18 +61,26 @@ workflows:
- composer/exec:
command: test

- composer/install:
name: test-7.0
executor: composer/seven_zero
post-steps:
- composer/exec:
command: test

- composer/install:
name: style-check
post-steps:
- composer/exec:
command: style:check

build_release:
# Build with lowest support PHP version
# wp.org svn pre-commit hook rejects PHP 7.1+ syntax
build_lint_release:
jobs:
- composer/install:
name: build
# Build with lowest support PHP version
executor: composer/seven_one
executor: composer/seven_zero
post-steps:
- composer/exec:
command: release:build
Expand All @@ -87,10 +94,23 @@ workflows:
tags:
only: /.*/

- composer/install:
name: lint
executor: composer/seven_zero
post-steps:
- composer/exec:
command: release:build
- run: unzip release/disallow-pwned-passwords.zip -d /tmp/plugin-source-code
- run: find /tmp/plugin-source-code -name "*.php" -print0 | xargs -n 1 -0 php -l
filters:
tags:
only: /.*/

- itinerisltd/publishing-to-github-releases:
name: publishing-to-github-releases
requires:
- build
- lint
artifact_dir: release
pre-steps:
- attach_workspace:
Expand Down
8 changes: 4 additions & 4 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,12 @@
}
],
"require": {
"php": "^7.1",
"php": "^7.0",
"league/container": "^3.2",
"typisttech/wp-contained-hook": "^0.3.0"
"typisttech/wp-contained-hook": "dev-seven-zero"
},
"require-dev": {
"10up/wp_mock": "^0.3.0",
"10up/wp_mock": "^0.3.0 || ^0.4.0",
"codeception/base": "^2.4",
"itinerisltd/itineris-wp-coding-standards": "^0.2.1",
"mockery/mockery": "^1.1",
Expand All @@ -48,7 +48,7 @@
},
"extra": {
"branch-alias": {
"dev-master": "0.2.x-dev"
"dev-master": "0.3.x-dev"
}
},
"autoload": {
Expand Down
12 changes: 6 additions & 6 deletions src/Handlers/FormSubmission.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@ class FormSubmission extends AbstractFormSubmission
*
* @return void
*/
public function handle(WP_Error $error): void
public function handle(WP_Error $error)
{
$errorCode = $error->get_error_code();
$cleartext = $this->getPasswordCleartextFromSuperglobals();

if (! empty($errorCode) || null === $cleartext) {
if (! empty($errorCode) || empty($cleartext)) {
return;
}

Expand All @@ -30,14 +30,14 @@ public function handle(WP_Error $error): void
/**
* Make password instance from superglobals.
*
* @return string|null returns null if password not found.
* @return string returns empty string if password not found.
*/
protected function getPasswordCleartextFromSuperglobals(): ?string
protected function getPasswordCleartextFromSuperglobals(): string
{
if (empty($_POST['pass1']) && empty($_POST['password_1'])) { // WPCS: input var, CSRF ok.
return null;
return '';
}

return wp_unslash($_POST['pass1'] ?? $_POST['password_1']); // WPCS: input var, CSRF ok.
return (string) wp_unslash($_POST['pass1'] ?? $_POST['password_1']); // WPCS: input var, CSRF ok.
}
}
2 changes: 1 addition & 1 deletion src/Handlers/WCCheckoutFormSubmission.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class WCCheckoutFormSubmission extends AbstractFormSubmission
*
* @return void
*/
public function handle(array $data, WP_Error $error): void
public function handle(array $data, WP_Error $error)
{
$errorCode = $error->get_error_code();
$cleartext = $data['account_password'] ?? '';
Expand Down
34 changes: 15 additions & 19 deletions src/HaveIBeenPwned/Client.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,14 @@ class Client implements ClientInterface
*
* @param Password $password The password to be checked.
*
* @return int|null Return null if Have I Been Pwned API endpoint is unreachable.
* @return int Return -1 if Have I Been Pwned API endpoint is unreachable.
*/
public function getPwnedTimes(Password $password): ?int
public function getPwnedTimes(Password $password): int
{
$pwned = $this->fetchAndDecode($password);

if (null === $pwned) {
return null;
if (empty($pwned)) {
return -1;
}

return $pwned[$password->getHashSuffix()] ?? 0;
Expand All @@ -37,13 +37,13 @@ public function getPwnedTimes(Password $password): ?int
*
* @param Password $password The password to be checked.
*
* @return array|null
* @return array
*/
public function fetchAndDecode(Password $password): ?array
public function fetchAndDecode(Password $password): array
{
$responseBody = $this->fetch($password);
if (null === $responseBody) {
return null;
if (empty($responseBody)) {
return [];
}

return $this->decode($responseBody);
Expand All @@ -54,16 +54,16 @@ public function fetchAndDecode(Password $password): ?array
*
* @param Password $password The password to be checked.
*
* @return string|null Body of the response if successful. Otherwise, null.
* @return string Body of the response if successful. Otherwise, empty string.
*/
protected function fetch(Password $password): ?string
protected function fetch(Password $password): string
{
$url = static::ENDPOINT . $password->getHashPrefix();
$response = wp_remote_get($url);

$responseCode = wp_remote_retrieve_response_code($response);
if (200 !== $responseCode) {
return null;
return '';
}

return (string) $response['body'];
Expand All @@ -89,15 +89,11 @@ protected function decode(string $responseBody): array

$pwned = [];
foreach ($suffixesWithPwnedTimes as $suffixWithPwnedTimes) {
[
// phpcs:ignore WordPressVIPMinimum.Variables.VariableAnalysis.UndefinedVariable -- Because of phpcs bug.
0 => $suffix,
// phpcs:ignore WordPressVIPMinimum.Variables.VariableAnalysis.UndefinedVariable -- Because of phpcs bug.
1 => $pwnedTimes,
] = explode(':', trim($suffixWithPwnedTimes));
$exploded = explode(':', trim($suffixWithPwnedTimes));
$suffix = (string) $exploded[0];
$pwnedTimes = (int) $exploded[1];

// phpcs:ignore WordPressVIPMinimum.Variables.VariableAnalysis.UndefinedVariable -- Because of phpcs bug.
$pwned[(string) $suffix] = (int) $pwnedTimes;
$pwned[$suffix] = $pwnedTimes;
}

return $pwned;
Expand Down
4 changes: 2 additions & 2 deletions src/HaveIBeenPwned/ClientInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ interface ClientInterface
*
* @param Password $password The password to be checked.
*
* @return int|null Return null if Have I Been Pwned API endpoint is unreachable.
* @return int Return -1 if Have I Been Pwned API endpoint is unreachable.
*/
public function getPwnedTimes(Password $password): ?int;
public function getPwnedTimes(Password $password): int;
}
14 changes: 7 additions & 7 deletions src/HaveIBeenPwned/ObjectCachedClient.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,14 @@ public function __construct(Client $client)
*
* @param Password $password The password to be checked.
*
* @return int|null Return null if Have I Been Pwned API endpoint is unreachable.
* @return int Return -1 if Have I Been Pwned API endpoint is unreachable.
*/
public function getPwnedTimes(Password $password): ?int
public function getPwnedTimes(Password $password): int
{
$pwned = $this->fetchAndDecode($password);

if (null === $pwned) {
return null;
if (empty($pwned)) {
return -1;
}

return $pwned[$password->getHashSuffix()] ?? 0;
Expand All @@ -59,9 +59,9 @@ public function getPwnedTimes(Password $password): ?int
*
* @param Password $password The password to be checked.
*
* @return array|null
* @return array
*/
protected function fetchAndDecode(Password $password): ?array
protected function fetchAndDecode(Password $password): array
{
$result = wp_cache_get(
$password->getHashPrefix(),
Expand All @@ -73,7 +73,7 @@ protected function fetchAndDecode(Password $password): ?array
}

$result = $this->client->fetchAndDecode($password);
if (null !== $result) {
if (! empty($result)) {
// phpcs:ignore WordPressVIPMinimum.Cache.LowExpiryCacheTime.LowCacheTime -- Because of phpcs bug.
wp_cache_set(
$password->getHashPrefix(),
Expand Down
10 changes: 5 additions & 5 deletions src/Plugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,17 +37,17 @@ public function __construct()
*
* @return void
*/
public function run(): void
public function run()
{
$this->setUpContainer();
$this->setUpLoader();

add_action('plugins_loaded', function (): void {
add_action('plugins_loaded', function () {
// phpcs:ignore WordPressVIPMinimum.Variables.VariableAnalysis.UndefinedVariable -- Because of phpcs bug.
do_action(static::PREFIX . '_register', $this->container);
}, PHP_INT_MAX - 1000);

add_action('plugins_loaded', function (): void {
add_action('plugins_loaded', function () {
// phpcs:ignore WordPressVIPMinimum.Variables.VariableAnalysis.UndefinedVariable -- Because of phpcs bug.
do_action(static::PREFIX . '_boot', $this->container);
}, PHP_INT_MIN + 1000);
Expand All @@ -58,7 +58,7 @@ public function run(): void
*
* @return void
*/
protected function setUpContainer(): void
protected function setUpContainer()
{
// Register the reflection container as a delegate to enable auto wiring.
$this->container->delegate(
Expand All @@ -76,7 +76,7 @@ protected function setUpContainer(): void
*
* @return void
*/
protected function setUpLoader(): void
protected function setUpLoader()
{
$this->loader->add(
new Action('user_profile_update_errors', FormSubmission::class, 'handle'),
Expand Down
6 changes: 3 additions & 3 deletions src/Predicate.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@ class Predicate implements PredicateInterface
/**
* Whether the password should be allowed according to the number of times it has been pwned.
*
* @param int|null $pwnedTimes Number of times the password has been pwned.
* @param int $pwnedTimes Number of times the password has been pwned.
*
* @return bool
*/
public function shouldDisallow(?int $pwnedTimes): bool
public function shouldDisallow(int $pwnedTimes): bool
{
return null !== $pwnedTimes && $pwnedTimes > 0;
return $pwnedTimes > 0;
}
}
4 changes: 2 additions & 2 deletions src/PredicateInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ interface PredicateInterface
/**
* Whether the password should be allowed according to the number of times it has been pwned.
*
* @param int|null $pwnedTimes Number of times the password has been pwned.
* @param int $pwnedTimes Number of times the password has been pwned.
*
* @return bool
*/
public function shouldDisallow(?int $pwnedTimes): bool;
public function shouldDisallow(int $pwnedTimes): bool;
}
2 changes: 1 addition & 1 deletion tests/unit/Handlers/FormSubmissionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ public function testHandleUnreachable()
return $password->getHashPrefix() === '486B5' &&
$password->getHashSuffix() === '6622A23D08DAFACC8A11115A3CFC148E51D';
}))
->andReturnNull()
->andReturn(-1)
->once();

$formSubmission = new FormSubmission(
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/Handlers/WCCheckoutFormSubmissionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ public function testHandleUnreachable()
return $password->getHashPrefix() === '486B5' &&
$password->getHashSuffix() === '6622A23D08DAFACC8A11115A3CFC148E51D';
}))
->andReturnNull()
->andReturn(-1)
->once();

$formSubmission = new WCCheckoutFormSubmission(
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/Handlers/WCRegistrationFormSubmissionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ public function testHandleUnreachable()
return $password->getHashPrefix() === '486B5' &&
$password->getHashSuffix() === '6622A23D08DAFACC8A11115A3CFC148E51D';
}))
->andReturnNull()
->andReturn(-1)
->once();

$formSubmission = new WCRegistrationFormSubmission(
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/HaveIBeenPwned/ClientTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public function testGetPwnedTimesUnreachable()

$actual = $client->getPwnedTimes($password);

$this->assertNull($actual);
$this->assertSame(-1, $actual);
}

public function testFetchAndDecode()
Expand Down Expand Up @@ -120,6 +120,6 @@ public function testFetchAndDecodeUnreachable()

$actual = $client->fetchAndDecode($password);

$this->assertNull($actual);
$this->assertEmpty($actual);
}
}
4 changes: 2 additions & 2 deletions tests/unit/HaveIBeenPwned/ObjectCachedClientTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -95,13 +95,13 @@ public function testGetPwnedTimesCacheMissAndApiUnreachable()
$originalClient = Mockery::mock(Client::class);
$originalClient->expects('fetchAndDecode')
->with($password)
->andReturnNull()
->andReturn([])
->once();

$client = new ObjectCachedClient($originalClient);

$actual = $client->getPwnedTimes($password);

$this->assertNull($actual);
$this->assertSame(-1, $actual);
}
}
Loading

0 comments on commit 883715a

Please sign in to comment.