WordPress.org

Make WordPress Core

Opened 14 months ago

Last modified 10 months ago

#40736 new defect (bug)

Ensure that `get_blog_count()` and `get_user_count()` return an integer

Reported by: flixos90 Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Networks and Sites Keywords: 2nd-opinion
Focuses: multisite Cc:

Description

The documentation for the functions get_blog_count() and get_user_count() states that the return type is an integer, however the functions only call get_network_option() without any typecast.

The functions should be adjusted to actually reflect their documentation. While this might theoretically be problematic in terms of backward-compatibility, in #40724 some initial thoughts of this being a viable change were expressed, so let's think about it on this ticket.

Attachments (11)

40736.patch (736 bytes) - added by desrosj 14 months ago.
Type cast return values as integers.
ms-functions.php.patch (750 bytes) - added by pmbaldha 13 months ago.
make sure return values are interger by intval function
40736.2.patch (750 bytes) - added by pmbaldha 13 months ago.
make sure return values are interger by intval function
php33xaUA (1.6 KB) - added by pmbaldha 13 months ago.
Deprecated get_blog_count() function and developed get_site_count function
phpomrpum (1.6 KB) - added by pmbaldha 13 months ago.
Deprecated get_blog_count() function and developed get_site_count function
phpGrxrzT (1.6 KB) - added by pmbaldha 13 months ago.
Deprecated get_blog_count() function and developed get_site_count function
40736.3.patch (1.6 KB) - added by pmbaldha 13 months ago.
Deprecated get_blog_count() function and developed get_site_count function. This is final.
40736.4.patch (2.0 KB) - added by pmbaldha 13 months ago.
int typecating for get_user_count(). Deprecated get_blog_count() function.Developed get_site_count() function.
40736.5.patch (2.0 KB) - added by pmbaldha 13 months ago.
int typecating for get_user_count(). Deprecated get_blog_count() function.Developed get_site_count() function. All changes done.
40736.6.patch (2.0 KB) - added by pmbaldha 13 months ago.
int typecating for get_user_count(). Deprecated get_blog_count() function.Developed get_site_count() function.Final patch
40736.7.patch (2.0 KB) - added by desrosj 7 months ago.
Refreshed patch to apply cleanly

Download all attachments as: .zip

Change History (21)

@desrosj
14 months ago

Type cast return values as integers.

@pmbaldha
13 months ago

make sure return values are interger by intval function

#1 @pmbaldha
13 months ago

Sorry my mistake. patch nameshouldbe ticket number.

@pmbaldha
13 months ago

make sure return values are interger by intval function

#2 follow-up: @flixos90
13 months ago

Since we're striving towards an API with site/network terminology in favor of blog/site long-term, we should consider deprecating get_blog_count() in favor of a get_site_count(). The change of the return type, whether it was specified in the docs or not, is a breaking change anyway, so I suggest to deprecate the function at the same time and replace their usage in core with calls to get_site_count().

@pmbaldha
13 months ago

Deprecated get_blog_count() function and developed get_site_count function

@pmbaldha
13 months ago

Deprecated get_blog_count() function and developed get_site_count function

@pmbaldha
13 months ago

Deprecated get_blog_count() function and developed get_site_count function

#3 in reply to: ↑ 2 @pmbaldha
13 months ago

I have done all chanmges. I am not able to login with my current internetc connection. It gives me erroe like: Your IP (103.217.85.2) has been flagged for potential security violations. Find out more...

I am trying to upload patch by proxy, but it changesname. I will try to give patch again tommorow morning again Replying to flixos90:

Since we're striving towards an API with site/network terminology in favor of blog/site long-term, we should consider deprecating get_blog_count() in favor of a get_site_count(). The change of the return type, whether it was specified in the docs or not, is a breaking change anyway, so I suggest to deprecate the function at the same time and replace their usage in core with calls to get_site_count().

@pmbaldha
13 months ago

Deprecated get_blog_count() function and developed get_site_count function. This is final.

This ticket was mentioned in Slack in #core by pmbaldha. View the logs.


13 months ago

#5 @flixos90
13 months ago

Thanks for the patch @pmbaldha!

Looks good except for a few minor issues:

  • Let's cast the values to integers with (int) instead of using the old intval().
  • For version numbers, the 4.8 and 4.8.0 should be replaced with 4.9.0, as this change won't make it into 4.8.
  • MU should not be used in the @since annotation for new functions.
  • The function get_blog_count() should be moved to the ms-deprecated.php file.

@pmbaldha
13 months ago

int typecating for get_user_count(). Deprecated get_blog_count() function.Developed get_site_count() function.

@pmbaldha
13 months ago

int typecating for get_user_count(). Deprecated get_blog_count() function.Developed get_site_count() function. All changes done.

@pmbaldha
13 months ago

int typecating for get_user_count(). Deprecated get_blog_count() function.Developed get_site_count() function.Final patch

#6 @pmbaldha
12 months ago

May i know is thee any progress on this ticket?

#7 @flixos90
12 months ago

Highlighting https://core.trac.wordpress.org/ticket/40736#comment:2 again: Is it reasonable to change the name from get_blog_count() to the more appropriate get_site_count()? In my opinion it's fine since we're practically breaking back-compatibility anyway, so we might as well deprecate the function in favor of the new one (and of course also adjust its old calls in core).

Alternatively, we may even aim for get_network_site_count() (would also need a get_network_user_count()) in order to have parity with the other functions that deal with these settings (such as wp_update_network_site_counts()). This is only an idea though.

Last edited 12 months ago by flixos90 (previous) (diff)

#8 @pmbaldha
12 months ago

wp_update_network_site_counts() function is used or called by internal wordpress code. Generally, Normal Wordpress developer doesn't know about wp_update_network_site_counts() function. He/she are are familiar wth get_blog_count() function. There is no worth to change get_site_count() to get_network_site_count(). This is my personal opinion.

If community like get_network_site_count() (would also need a get_network_user_count()) function name, Please tell me. I will create new patch.

May i know what is opinion of other contributors?

This ticket was mentioned in Slack in #core-multisite by pmbaldha. View the logs.


10 months ago

#10 @flixos90
10 months ago

  • Milestone changed from Awaiting Review to Future Release

@desrosj
7 months ago

Refreshed patch to apply cleanly

Note: See TracTickets for help on using tickets.