Skip to content

Commit

Permalink
(Fast)ArrayDecorator - Emit expected exception when using WP and stri…
Browse files Browse the repository at this point in the history
…ct PSR-16

Suppose someone calls `ArrayDecorator::get()` or `FastArrayDecorator::get()`
with an invalid key (e.g.  passing `float` or an `array` instead of a
`string`).  This patch improves error-reporting for that scenario.

This is primarily about fixing multiple test failures in `E2E_Cache_ArrayDecoratorTest` reported on `wp-demo`. These
generally look like

```
1) E2E_Cache_ArrayDecoratorTest::testGetInvalidKeys with data set #1 (true)
array_key_exists(): The first argument should be either a string or an integer

/Users/totten/bknix/build/wpmaster/wp-content/plugins/civicrm/civicrm/CRM/Utils/Cache/ArrayDecorator.php:102
/Users/totten/bknix/build/wpmaster/wp-content/plugins/civicrm/civicrm/packages/Cache/IntegrationTests/LegacySimpleCacheTest.php:409
/Users/totten/bknix/civicrm-buildkit/extern/phpunit5/phpunit5.phar:598
```

Before
------

The ArrayDecorator first checks its front cache (`array_key_exists`) to see if the key is defined.  In the `wp-demo`
environment, this produces a warning and causes the test to fail.

The condition *is* erroneous, but PSR-16 specifies that the error should be reported as exception.

After
-----

The condition is reported as the expected exception. The test passes on `wp-demo`.

Comment
-------

This brings the code in `(Fast)ArrayDecorator.php` into alignment with most of the other `CRM/Utils/Cache/*.php`
drivers; in most drivers, it is typical to validate the key at the start of most functions.
  • Loading branch information
totten committed Mar 13, 2019
1 parent 097cb75 commit 8a8cbad
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 0 deletions.
3 changes: 3 additions & 0 deletions CRM/Utils/Cache/ArrayDecorator.php
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ public function set($key, $value, $ttl = NULL) {
}

public function get($key, $default = NULL) {
CRM_Utils_Cache::assertValidKey($key);
if (array_key_exists($key, $this->values) && $this->expires[$key] > CRM_Utils_Time::getTimeRaw()) {
return $this->reobjectify($this->values[$key]);
}
Expand All @@ -110,6 +111,7 @@ public function get($key, $default = NULL) {
}

public function delete($key) {
CRM_Utils_Cache::assertValidKey($key);
unset($this->values[$key]);
unset($this->expires[$key]);
return $this->delegate->delete($key);
Expand All @@ -126,6 +128,7 @@ public function clear() {
}

public function has($key) {
CRM_Utils_Cache::assertValidKey($key);
if (array_key_exists($key, $this->values) && $this->expires[$key] > CRM_Utils_Time::getTimeRaw()) {
return TRUE;
}
Expand Down
2 changes: 2 additions & 0 deletions CRM/Utils/Cache/FastArrayDecorator.php
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ public function set($key, $value, $ttl = NULL) {
}

public function get($key, $default = NULL) {
CRM_Utils_Cache::assertValidKey($key);
if (array_key_exists($key, $this->values)) {
return $this->values[$key];
}
Expand All @@ -114,6 +115,7 @@ public function get($key, $default = NULL) {
}

public function delete($key) {
CRM_Utils_Cache::assertValidKey($key);
unset($this->values[$key]);
return $this->delegate->delete($key);
}
Expand Down

0 comments on commit 8a8cbad

Please sign in to comment.