wp_upload_dir caches calls with default arguments separately from their explicit equivalent

[stephenharris]

This a fairly minor bug. Prior to [36565] the following were equivalent

$upload = wp_upload_dir();
$upload2 = wp_upload_dir( gmstrftime('%Y/%m') );

(In deed, even $upload3 = wp_upload_dir( gmstrftime('%Y/%m yabadabadoo') ); is equivalent, but that behaviour is undocumented.)

Since caching has been introduced the first call to wp_upload_dir() is stored separately in the cache to the second call, despite the fact that they are otherwise interpreted the same.

This causes a disparity in behaviour when one of the caches is cleared after a setting has been changed:

$upload  = wp_upload_dir();
$upload2 = wp_upload_dir( gmstrftime('%Y/%m') );

// $upload == $upload2

//Disable year/month folders and clear the cache
add_filter( 'pre_option_uploads_use_yearmonth_folders', '__return_null' );
wp_upload_dir( null, false, true );

$upload  = wp_upload_dir( null );
$upload2 = wp_upload_dir( gmstrftime('%Y/%m') );

//$upload != $upload2

remove_filter( 'pre_option_uploads_use_yearmonth_folders', '__return_null' );

[joemcgill]

@stephenharris Thanks for the report.

I'd like to see @azaozz's feedback on this since he did most of the work on [36565].

Your explanation and patch both make sense, though I'm a bit uneasy about essentially forcing an undocumented default value for the $time parameter. Also, there is a slight difference in the way _wp_upload_dir() calculates the time string if it's set to null, using current_time( 'mysql' ). It would probably be best to remain consistent between these two functions.

[stephenharris]

@joemcgill Yes, it uses current_time( 'mysql' ) but then just extracts the year and month part. So you can pass it 2016/09 something else and it will extract 2016 and 09 as the year / month to form the subdirectory. So I don't think there should be an issue with forcing that as a default value.