[CMSP-459] bugfix: re-add missing else by jazzsequence · Pull Request #437 · pantheon-systems/wp-redis

jazzsequence · 2023-06-22T21:16:18Z

This re-adds the else statement that was inadvertently missed in a recent commit, leading to some of the Behat test failures.

This, as well as #430 account for all of the environment fixes that were mixed into #426 but separate from the actual change suggested in that PR.

Tests can be seen as passing (with #430) here: https://app.circleci.com/pipelines/github/pantheon-systems/wp-redis/1629/workflows/fd714abf-075f-45f8-9102-3ddc730f731e/jobs/4980

pwtyler · 2023-06-22T22:09:23Z

that was inadvertently missed in a recent commit

Can you link? Was this #428?

object-cache.php

jazzsequence · 2023-06-22T22:18:31Z

@pwtyler 69c194f

		} else { // tcp connection.
			$port = ! empty( $redis_server['port'] ) ? $redis_server['port'] : 6379;
		}

We can make improvements, but this is just to get us back to the baseline working. Any additional changes should (IMO) be in a separate PR.

And you can see in #430 that without this change, the tests are failing.

pwtyler · 2023-06-22T22:27:56Z

We can make improvements, but this is just to get us back to the baseline working.

+1 on the simpler fix first and ignore my second comment, ternary operator w/ fallback is still unnecessary if $port has a default value at the top of the func

jazzsequence · 2023-06-23T15:27:30Z

@pwtyler There is a chance that the $redis_server value passed into the function might be empty -- e.g.

$redis_server = [
    'host' => '127.0.0.1',
    'database' => 0,
];
$params = build_client_parameters( $redis_server );

In that case, $redis_server['port'] would never be set and the final return value would be missing a port because there would be no port in either $redis_server or $defaults. I think the ternary is still required.

* Reverts this incorrect change that was made due to the incorrect use of `array_replace_recursive()`.

timnolte · 2023-06-23T17:14:28Z

@jazzsequence & @pwtyler just noting that this change was actually incorrectly made as the real issue was the misused of array_replace_recursive(). I have fixed this again in my PR. #434

* Reverts this incorrect change that was made due to the incorrect use of `array_replace_recursive()`.

…er issues (#434) * fix: Fixes incorrect order of array_replace_recursive arguments & other issues * Fixes #433 * Fixes #432 * Fixes #431 * Further clean-up & standardization between object-cache.php & wp-redis.php. * Fixes incorrect order of array_replace_recursive arguments. * Addresses issue with port still not being null for socket connections due to defaults array_repalce_recursive use. * fix: Fixes sanitization methods and linting issues * Adjusts some items to use type-based sanitization. * Adds linting expection handling with comments for cases that require it. * fix: Removes invalid change made in #437 * Reverts this incorrect change that was made due to the incorrect use of `array_replace_recursive()`. * update changelog * Update wp-redis.php * update language in changelogs * fix missing closing ) --------- Co-authored-by: Chris Reynolds <[email protected]> Co-authored-by: Phil Tyler <[email protected]>

* re-add missing else * use the already-defined value of $port instead of hard-coding * remove ternary in favor of elseif

…er issues (#434) * fix: Fixes incorrect order of array_replace_recursive arguments & other issues * Fixes #433 * Fixes #432 * Fixes #431 * Further clean-up & standardization between object-cache.php & wp-redis.php. * Fixes incorrect order of array_replace_recursive arguments. * Addresses issue with port still not being null for socket connections due to defaults array_repalce_recursive use. * fix: Fixes sanitization methods and linting issues * Adjusts some items to use type-based sanitization. * Adds linting expection handling with comments for cases that require it. * fix: Removes invalid change made in #437 * Reverts this incorrect change that was made due to the incorrect use of `array_replace_recursive()`. * update changelog * Update wp-redis.php * update language in changelogs * fix missing closing ) --------- Co-authored-by: Chris Reynolds <[email protected]> Co-authored-by: Phil Tyler <[email protected]>

* re-add missing else * use the already-defined value of $port instead of hard-coding * remove ternary in favor of elseif

…er issues (pantheon-systems#434) * fix: Fixes incorrect order of array_replace_recursive arguments & other issues * Fixes pantheon-systems#433 * Fixes pantheon-systems#432 * Fixes pantheon-systems#431 * Further clean-up & standardization between object-cache.php & wp-redis.php. * Fixes incorrect order of array_replace_recursive arguments. * Addresses issue with port still not being null for socket connections due to defaults array_repalce_recursive use. * fix: Fixes sanitization methods and linting issues * Adjusts some items to use type-based sanitization. * Adds linting expection handling with comments for cases that require it. * fix: Removes invalid change made in pantheon-systems#437 * Reverts this incorrect change that was made due to the incorrect use of `array_replace_recursive()`. * update changelog * Update wp-redis.php * update language in changelogs * fix missing closing ) --------- Co-authored-by: Chris Reynolds <[email protected]> Co-authored-by: Phil Tyler <[email protected]>

jazzsequence requested review from a team as code owners June 22, 2023 21:16

jazzsequence changed the title ~~bugfix: re-add missing else~~ [CMSP-459] bugfix: re-add missing else Jun 22, 2023

jazzsequence mentioned this pull request Jun 22, 2023

[CMSP-459] Updates behat tests and behaviors. #430

Merged

pwtyler reviewed Jun 22, 2023

View reviewed changes

object-cache.php Outdated Show resolved Hide resolved

jazzsequence added 2 commits June 23, 2023 09:50

re-add missing else

8ae767c

use the already-defined value of $port instead of hard-coding

4b56952

jazzsequence force-pushed the cmsp-459-add-missing-else branch from e2a8ee2 to 4b56952 Compare June 23, 2023 15:50

remove ternary in favor of elseif

386457f

pwtyler approved these changes Jun 23, 2023

View reviewed changes

jazzsequence merged commit d1b6103 into default Jun 23, 2023

jazzsequence deleted the cmsp-459-add-missing-else branch June 23, 2023 16:59

pwtyler mentioned this pull request Jun 23, 2023

WIP -- DNM -- Rw update object cache #429

Closed

timnolte added a commit to timnolte/wp-redis that referenced this pull request Jun 23, 2023

fix: Removes invalid change made in pantheon-systems#437

37d25d1

* Reverts this incorrect change that was made due to the incorrect use of `array_replace_recursive()`.

jazzsequence pushed a commit to timnolte/wp-redis that referenced this pull request Jun 23, 2023

fix: Removes invalid change made in pantheon-systems#437

4d56e39

* Reverts this incorrect change that was made due to the incorrect use of `array_replace_recursive()`.

jazzsequence added a commit that referenced this pull request Jun 26, 2023

[CMSP-459] bugfix: re-add missing else (#437)

5301cb0

* re-add missing else * use the already-defined value of $port instead of hard-coding * remove ternary in favor of elseif

jazzsequence added a commit to earthlingdavey/wp-redis that referenced this pull request Feb 18, 2025

[CMSP-459] bugfix: re-add missing else (pantheon-systems#437)

e228c87

* re-add missing else * use the already-defined value of $port instead of hard-coding * remove ternary in favor of elseif

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CMSP-459] bugfix: re-add missing else#437

[CMSP-459] bugfix: re-add missing else#437
jazzsequence merged 3 commits intodefaultfrom
cmsp-459-add-missing-else

jazzsequence commented Jun 22, 2023 •

edited

Loading

Uh oh!

pwtyler commented Jun 22, 2023

Uh oh!

Uh oh!

jazzsequence commented Jun 22, 2023 •

edited

Loading

Uh oh!

pwtyler commented Jun 22, 2023 •

edited

Loading

Uh oh!

jazzsequence commented Jun 23, 2023 •

edited

Loading

Uh oh!

timnolte commented Jun 23, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

jazzsequence commented Jun 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pwtyler commented Jun 22, 2023

Uh oh!

Uh oh!

jazzsequence commented Jun 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pwtyler commented Jun 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jazzsequence commented Jun 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

timnolte commented Jun 23, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jazzsequence commented Jun 22, 2023 •

edited

Loading

jazzsequence commented Jun 22, 2023 •

edited

Loading

pwtyler commented Jun 22, 2023 •

edited

Loading

jazzsequence commented Jun 23, 2023 •

edited

Loading