SQL and Memcache store expire consistency

[GoogleCodeExporter]

To set that data newer expires in SQL store expire has to be set to NULL, and 
in Memcache store to 0 (Memcache store with e.g. Couchbase backend that has 
persistency, where this makes sense). Could this somehow be unified? One 
possible solution would be to not set expire value in SQL store when expire is 
0, e.g.

SimpleSAML_Store_SQL::set():

                if ($expire !== NULL && $expire !== 0) {
                        $expire = gmdate('Y-m-d H:i:s', $expire);
                }

What do you think?

Original issue reported on code.google.com by comel...@gmail.com on 21 Jun 2012 at 9:08

[GoogleCodeExporter]

I forgot about local changes that I made to local variant of 
SimpleSAML_Store_Memcache::set():

assert('is_null($expire) || is_int($expire)');

if ($expire !== NULL && $expire !== 0 && $expire < 2592000) {
        $expire += time();
}

to be able to set non-zero value memcache_store.expires in config, and still to 
be able to have data that do not expire (set($type, $key, $value, 0)), and also 
to have relative expire times.

So changes should be made also to SimpleSAML_Store_Memcache. I'll send the 
patch, just say in what way you want to handle this?

Original comment by comel...@gmail.com on 21 Jun 2012 at 12:43

[GoogleCodeExporter]

Hmmm... it looks like I made a mistake when writing the code. Both SQL and 
Memcache datastore lists NULL as the value for "do not expire":

    @param int|NULL $expire  The expiration time (unix timestamp), or NULL if it never expires.

But SimpleSAML_Memcache::set() treats NULL as "use default expire". It does not 
look like anything other than SimpleSAML_Store_Memcache calls 
SimpleSAML_Memcache::set(), so changing the behaviour to that function should 
be safe?

Original comment by olavmrk@gmail.com on 26 Jun 2012 at 5:52

[GoogleCodeExporter]

Hm, then memcache_store.expires would not be used anymore? IMO it would be good 
to have default expire time from config setting (e.g. store.expires) when 
expire = NULL, and never expires when explicitly expire = 0.

Original comment by comel...@gmail.com on 26 Jun 2012 at 7:43

[GoogleCodeExporter]

The expiration would then be up to the data. E.g. 'session.duration' for 
session data.

Original comment by olavmrk@gmail.com on 26 Jun 2012 at 8:28

[GoogleCodeExporter]

OK, committed r3125. Only SimpleSAML_Store_Memcache::set() is changed, 
everything else stays as is.

Original comment by comel...@gmail.com on 26 Jun 2012 at 2:44

• Changed state: Fixed