WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 6 years ago

#22746 closed defect (bug) (fixed)

The metadata_exists() function should return the correct value if the get_$meta_type_metadata filter returns a non null value

Reported by: xknown Owned by: nacin
Milestone: 3.9 Priority: normal
Severity: normal Version:
Component: Options, Meta APIs Keywords: has-patch
Focuses: Cc:

Description

The line 324 should return $check instead of true, otherwise the filter doesn't normally work when someone wants to always return false for a given meta key.

Here's a quick way to reproduce it

> add_filter('get_user_metadata', '__return_false'); return metadata_exists('user', 1, 1)
TRUE

Attachments (3)

meta.diff (421 bytes) - added by xknown 7 years ago.
Return $check instead of true.
meta.2.diff (470 bytes) - added by xknown 6 years ago.
Updated patch. Cast to bool to match the metadata_exists() documentation.
meta-unit-test.diff (898 bytes) - added by xknown 6 years ago.
Add two more assertions to the test_metadata_exists() test method.

Download all attachments as: .zip

Change History (10)

@xknown
7 years ago

Return $check instead of true.

#1 @nacin
6 years ago

  • Component changed from General to Options and Meta

#2 @nacin
6 years ago

  • Keywords has-patch commit added
  • Milestone changed from Awaiting Review to 3.9

This sounds OK to change. A simple unit test would be nice.

#3 follow-up: @nacin
6 years ago

  • Keywords needs-unit-tests added

@xknown
6 years ago

Updated patch. Cast to bool to match the metadata_exists() documentation.

@xknown
6 years ago

Add two more assertions to the test_metadata_exists() test method.

#4 in reply to: ↑ 3 ; follow-ups: @SergeyBiryukov
6 years ago

Cast to bool to match the metadata_exists() documentation.

I think the documentation should be changed to @return mixed instead, which would be consistent with term_exists(): tags/3.8.1/src/wp-includes/taxonomy.php#L1561.

#5 in reply to: ↑ 4 @DrewAPicture
6 years ago

  • Keywords needs-docs added; commit removed

Replying to SergeyBiryukov:

Cast to bool to match the metadata_exists() documentation.

I think the documentation should be changed to @return mixed instead, which would be consistent with term_exists(): tags/3.8.1/src/wp-includes/taxonomy.php#L1561.

+1 for using mixed, as long as you spell out the various return types and conditions.

#6 in reply to: ↑ 4 @nacin
6 years ago

  • Keywords needs-unit-tests needs-docs removed

Replying to SergeyBiryukov:

Cast to bool to match the metadata_exists() documentation.

I think the documentation should be changed to @return mixed instead

This is for metadata_exists(), not get_metadata(). It either exists or doesn't.

#7 @nacin
6 years ago

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

In 27562:

Return false from metadata_exists() if the get_$type_metadata filter returns a false value.

props xknown.
fixes #22746.

Note: See TracTickets for help on using tickets.