WordPress.org

Make WordPress Core

Opened 21 months ago

Last modified 6 weeks ago

#21330 new enhancement

Allow filtering of expiration in set_transient()

Reported by: wpsmith Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Cache API Keywords: has-patch 2nd-opinion
Focuses: Cc:

Description

I recently needed to change an expiration of a transient and expected pre_set_site_transient_* filter to be able to do that. However, the filter only affects the value. I recommend adjusting this to affect both.

Attachments (6)

21330.patch (674 bytes) - added by wpsmith 21 months ago.
Site Transient Filter Adjustment, first pass
21330_opt3.patch (538 bytes) - added by wpsmith 21 months ago.
Option 3: New filter
21330_opt2.patch (534 bytes) - added by wpsmith 21 months ago.
Option 2: Same filter different 2nd value.
21330_opt1.patch (882 bytes) - added by wpsmith 21 months ago.
Option 1: Reworked, removed extract()
21330_opt4.patch (919 bytes) - added by wpsmith 21 months ago.
Option 4: Combo of Option 3 & modified Option 1
21330.diff (1.2 KB) - added by nacin 20 months ago.

Download all attachments as: .zip

Change History (22)

comment:1 follow-up: obenland21 months ago

AFAIK, filters are only supposed to filter the first supplied argument. So $expirationprobably should get its own filter.

comment:2 in reply to: ↑ 1 wpsmith21 months ago

Replying to obenland:

AFAIK, filters are only supposed to filter the first supplied argument. So $expirationprobably should get its own filter.

Yes that's right. I knew that! Adjusted patch for one filter.

wpsmith21 months ago

Site Transient Filter Adjustment, first pass

comment:3 follow-up: nacin21 months ago

extract( $options, EXTR_OVERWRITE ); is dangerous and not something we're likely going to do here, as it doesn't fit patterns found throughout. Please do not replace patches, we can't see what changed — I actually can't tell if this was the original patch, or a replacement.

comment:4 in reply to: ↑ 3 wpsmith21 months ago

Replying to nacin:

extract( $options, EXTR_OVERWRITE ); is dangerous and not something we're likely going to do here, as it doesn't fit patterns found throughout. Please do not replace patches, we can't see what changed — I actually can't tell if this was the original patch, or a replacement.

My apologies. The first one was VERY much similiar to the one that's there now using extract. I've added a couple more options.

wpsmith21 months ago

Option 3: New filter

wpsmith21 months ago

Option 2: Same filter different 2nd value.

wpsmith21 months ago

Option 1: Reworked, removed extract()

comment:5 follow-up: nacin21 months ago

I don't understand how 21330_opt2.patch would work in a plugin.

Could you go a bit more into your use case, perhaps some sample code?

comment:6 in reply to: ↑ 5 wpsmith21 months ago

Replying to nacin:

I don't understand how 21330_opt2.patch would work in a plugin.

Could you go a bit more into your use case, perhaps some sample code?

The expiration time is always set (right? if not set, it's set to 0 by the time of the filter) and is always an integer. For example, wp_check_browser_version() has a set expiration that is known. So...assuming I knew the $key (93ce43dc55bd29ecc99e83ca12b5d7f4) in this example.
`
add_filter( 'pre_set_site_transient_browser_93ce43dc55bd29ecc99e83ca12b5d7f4', 'wps_change_browser_transient' );
function wps_change_browser_transient( $val ) {

if ( is_int( $val ) && 604800 == $val )

$val = 1209600;

return $val;

}
`

However, I am not convinced that this is the best solution...

Version 0, edited 21 months ago by wpsmith (next)

wpsmith21 months ago

Option 4: Combo of Option 3 & modified Option 1

comment:7 obenland21 months ago

In 21330_opt4.patch, what do you need the option part for? Value and expiration are both filterable at this point.

nacin20 months ago

comment:8 nacin20 months ago

If we were to do this, it would probably look like 21330.diff.

comment:9 nacin20 months ago

  • Type changed from defect (bug) to enhancement

comment:10 nacin19 months ago

  • Summary changed from Change pre_set_site_transient_* to affect $expiration to Allow filtering of expiration in set_transient()

comment:11 DrewAPicture19 months ago

  • Cc xoodrew@… added

comment:12 mordauk2 months ago

I haven't personally had the need to do this, but it seems perfectly reasonable that a plugin would want to. For example, in Easy Digital Downloads we use transients for caching some earnings data. If our cache is too aggressive or too weak, a user could better tweak our transient expirations to better suit their site.

comment:13 wonderboymusic2 months ago

  • Keywords dev-feedback removed
  • Milestone changed from Awaiting Review to 3.9

Let's decide soon in 3.9

comment:14 ircbot6 weeks ago

This ticket was mentioned in IRC in #wordpress-dev by nacin. View the logs.

comment:15 nacin6 weeks ago

As evidenced by IRC, naming this filter is a pain.

But also:

<?php
add_action( 'set_transient_my_transient', function( $value, $expiration ) {
    $desired_expiration = 600;
    if ( $expiration != $desired_expiration ) {
        set_transient( 'my_transient', $value, $desired_expiration );
    }
});

comment:16 nacin6 weeks ago

  • Keywords 2nd-opinion added
  • Milestone changed from 3.9 to Future Release
Note: See TracTickets for help on using tickets.