WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 2 years ago

#16137 closed defect (bug) (fixed)

wp_list_filter() is inexact when dealing with nested arrays

Reported by: scribu Owned by: ryan
Milestone: 3.3 Priority: normal
Severity: normal Version:
Component: General Keywords: has-patch
Focuses: Cc:

Description (last modified by scribu)

To reproduce:

add_action('init', 'test_admin_notices', 11);
function test_admin_notices() {
    global $wp_taxonomies;
    print_r( wp_filter_object_list( array_values( $wp_taxonomies ), array( 'object_type' => array( 'post' ) ), 'and', 'object_type' ) );
}

This is because array_intersect_assoc() does a strict comparison: (string) $elem1 === (string) $elem2

Using a manual loop fixes the problem.

Attachments (6)

16137.diff (639 bytes) - added by scribu 3 years ago.
16137.patch (668 bytes) - added by SergeyBiryukov 3 years ago.
16137.2.patch (679 bytes) - added by SergeyBiryukov 3 years ago.
16137_tests.diff (3.1 KB) - added by ampt 3 years ago.
add unit tests
wplf-test.php (2.6 KB) - added by scribu 3 years ago.
Speed test
16137_tests.2.diff (3.1 KB) - added by ampt 3 years ago.

Download all attachments as: .zip

Change History (23)

scribu3 years ago

comment:1 scribu3 years ago

Context: #14084

comment:2 scribu3 years ago

  • Keywords needs-refresh added

comment:3 mark8barnes3 years ago

  • Cc mark@… added

I just came across this problem. The bug breaks get_taxonomies( $args, $output, $operator ) when $args = array('object_type' => array($custom_post_type)), but the patch fixes it. Can we get this in 3.2?

SergeyBiryukov3 years ago

comment:4 SergeyBiryukov3 years ago

  • Keywords needs-refresh 3.2-early removed
  • Milestone changed from Future Release to 3.3

comment:5 follow-up: scribu3 years ago

While we're at it, we should use get_object_vars() instead of casting to array, in case the object has non-public properties, which we don't care about.

Last edited 3 years ago by scribu (previous) (diff)

comment:6 scribu3 years ago

  • Description modified (diff)

comment:7 in reply to: ↑ 5 SergeyBiryukov3 years ago

Replying to scribu:

While we're at it, we should use get_object_vars() instead of casting to array

Done.

SergeyBiryukov3 years ago

comment:8 scribu3 years ago

  • Keywords commit added

comment:10 scribu3 years ago

  • Keywords needs-unit-tests added; commit removed

comment:11 nacin3 years ago

Performance benchmark?

ampt3 years ago

add unit tests

comment:12 ampt3 years ago

add unit tests, i'm not exactly certain of the correct behaviour and cases for test_filter_object_list_nested_array_or and also test_filter_object_list_or_nested_array_or_field, any guidance would be appreciated. any other cases required?

comment:13 scribu3 years ago

It seems using a foreach is actually faster than array_intersect_assoc().

Using wplf-test.php I get:

wp_list_filter_old: 1.6865890026093
wp_list_filter_new: 1.3801169395447
wp_list_filter_new2: 1.871533870697

wp_list_filter_new is the one in 16137.patch.

wp_list_filter_new2 uses an if + get_object_vars(). Although I made this suggestion, I think it's better to just use casting to arrays.

scribu3 years ago

Speed test

ampt3 years ago

comment:14 ampt3 years ago

Fix typo in function names

comment:15 ryan3 years ago

  • Owner set to ryan
  • Resolution set to fixed
  • Status changed from new to closed

In [18606]:

Properly handle nested arrays in wp_list_filter(). Props scribu, SergeyBiryukov. fixes #16137

comment:17 SergeyBiryukov2 years ago

  • Keywords needs-unit-tests removed
Note: See TracTickets for help on using tickets.