Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cache remember-unless #54342

Closed
wants to merge 1 commit into from
Closed

Conversation

gpibarra
Copy link

Context

I have a cached data that takes 5 minutes to execute the closure, this process of recalculating the cache is executed on demand only in certain circumstances, validating the administrator's permissions and with a special parameter in the URL request. In addition this cache has a TTL of 24 hours

The first approach (which was incorrect), when I needed to recalculate the cache, first I would do a forget and then I would do a remember. However, if at the time the value was being calculated (after the forget but during the 5 minutes it takes to process) a request arrives, the get (or remember) that the user does would give null because the value had already been deleted from the cache and had not yet been saved again.

Obviously the admin should execute the put method to overwrite the value from the cache, if while the recalculation process is running, a request arrives, the value that is still in the cache and was not overwritten is used.

I've looked at the documentation for flexible but it's not exactly what I need, because the cache might still be valid when it needs to be recalculated.

I am aware that this can be solved by putting the closure in a separate method and differentiating the user calls vs. the administrator calls.

class MyData {

    protected $keyCacheStore = 'my_key';
    
    protected function getTtlCacheStore(): int
    {
        return \Carbon\CarbonInterval::createFromDateString('1 day')->total('seconds');
    }

    public function getData() {
        return Cache::remember($this->keyCacheStore, $this->getCacheTtl(), function () {
            return $this->calculateData();
        });
    }

    public function refreshData() {
        $value = $this->calculateData();
        Cache::put($this->keyCacheStore, $value, $this->getCacheTtl());

        return $value;
    }

    public function calculateData() {
        // a long process
        return [];
    }
}

But the proposal is to use a new method to make it clearer and save a few lines of code.

class MyData {

    public function getData($refresh = false) {
        $ttlCacheStore = \Carbon\CarbonInterval::createFromDateString('1 day')->total('seconds');
        return Cache::rememberUnless($refresh, 'my_key', $ttlCacheStore, function () {
            return $this->calculateData();
        });
    }

    public function calculateData() {
        // a long process
        return [];
    }
}

Another alternative would be to add the parameter $when to the remember function, but I thought a new method would be better.

@taylorotwell
Copy link
Member

Thanks for your pull request to Laravel!

Unfortunately, I'm going to delay merging this code for now. To preserve our ability to adequately maintain the framework, we need to be very careful regarding the amount of code we include.

If applicable, please consider releasing your code as a package so that the community can still take advantage of your contributions!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants