zendframework / zend-cache

BSD 3-Clause "New" or "Revised" License
69 stars 53 forks source link

Fixes cache entry deletion directly after creation #184

Closed alexz707 closed 5 years ago

alexz707 commented 5 years ago

See https://github.com/zendframework/zend-cache/issues/183

Only set TTL if there is a value which is not null!

michalbundyra commented 5 years ago

@weierophinney I've added tests to cover changes and prove the issue. Instead of mocking storage and options, I've implemented storage and logged TTL when setting the item in cache. Then in assertion I am checking what value of TTL was set for the item.

michalbundyra commented 5 years ago

Actually using mocking it will be also quite simply:

    public function testUseTtlFromOptionsOnSetMocking()
    {
        $this->options->getTtl()->willReturn(40);
        $this->options->setTtl(40)->will([$this->options, 'reveal']);

        $this->options->setTtl(null)->shouldNotBeCalled();

        $this->storage->getOptions()->will([$this->options, 'reveal']);
        $this->storage->setItem('foo', 'bar')->willReturn(true);

        self::assertTrue($this->cache->set('foo', 'bar'));
    }

    public function testUseTtlFromOptionsOnSetMultipleMocking()
    {
        $this->options->getTtl()->willReturn(40);
        $this->options->setTtl(40)->will([$this->options, 'reveal']);

        $this->options->setTtl(null)->shouldNotBeCalled();

        $this->storage->getOptions()->will([$this->options, 'reveal']);
        $this->storage->setItems(['foo' => 'bar', 'boo' => 'baz'])->willReturn([]);

        self::assertTrue($this->cache->setMultiple(['foo' => 'bar', 'boo' => 'baz']));
    }

But not sure if it is clear enough?

Line

$this->options->setTtl(null)->shouldNotBeCalled();

in both tests is not really needed. It says explicitly that we do not allow that cal, but as we are mocking options anyway, we are saying what calls are allowed (and all others are not).

michalbundyra commented 5 years ago

Thanks, @alexz707!