Skip to content

ensure redis returns bool for hasKey - #26551

Merged
rullzer merged 1 commit into
masterfrom
fix/noid/redis-exists-bool
Apr 14, 2021
Merged

ensure redis returns bool for hasKey#26551
rullzer merged 1 commit into
masterfrom
fix/noid/redis-exists-bool

Conversation

@blizzz

Copy link
Copy Markdown
Member

Fixes a Return value of OC\\Http\\Client\\NegativeDnsCache::isNegativeCached() must be of the type bool, int returned", with Redis. To quote the module's doc:

[..]
* @SInCE >= 4.0 Returned int, if < 4.0 returned bool
[..]
* @return int|bool The number of keys tested that do exist

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@blizzzblizzz added 3. to review Waiting for reviews bug labels Apr 13, 2021
@blizzzblizzz added this to the Nextcloud 22 milestone Apr 13, 2021

@LukasReschkeLukasReschke left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😢

@@ -69,7 +69,7 @@ public function set($key, $value, $ttl = 0) {
}

public function hasKey($key) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we type the signature as well?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be done in the interface, didn't want to change that. For 22 should be ok. And just backport that line.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but then let's refurbish the whole ICache in a follow up PR since it is mostly from 2013.

@rullzer
rullzer merged commit f031dd6 into masterApr 14, 2021
@rullzer
rullzer deleted the fix/noid/redis-exists-bool branch April 14, 2021 11:37
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to reviewWaiting for reviewsbug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@blizzz@rullzer@LukasReschke