Re: [RFC] session_start(), read_only, lazy_write; Take 2

On Sat, Mar 29, 2014 at 3:09 AM, Yasuo Ohgaki <yohgaki@ohgaki.net> wrote: > Hi Andrey, > > On Sat, Mar 29, 2014 at 8:41 AM, Andrey Andreev <narf@devilix.net> wrote: >> >> You yourself told me that there's no way for the SessionHandler class >> to know if the storage module supports PS_UPDATE_TIMESTAMP_FUNC() or >> not, and that because of that we can't have >> SessionHandler::updateTimestamp(). > > > It's only applicable for object based user save handler. > Since user is implemented their own handler, this limitation should not > matter. You're saying this like nobody uses the SessionHandler class. You don't know that, it's just an assumption of yours, and based on it you're willing to have a limited instead of a complete feature, while there are no real technical limitations. This is surprising, considering that you cite "lack of API" for other features that are IMO not as useful. >> > Writing data and updating time stamp for GC is distinct feature. >> >> For file-based sessions - it is, but for i.e. a database, there's not >> much difference. write() does update the timestamp, I see no reason >> why it shouldn't be able to update just the timestamp. > > > How to update time stamp is up to save handler and its storage. > Write and updating time stamp is different operation. > Memcache does not need it if data is read. RDBMS may have separate > table to keep track time stamp. For PostgreSQL, updating one field is > faster than updating whole record. I'm not saying it isn't faster to update just one field, I'm saying it's still an UPDATE clause in both cases. >> > I don't see good reason not to have API for it. >> >> - Not being able to call parent::updateTimestamp() is a good reason. >> - Having the completely unnecessary SessionUpdateTimestampInterface >> is a good reason. >> - Breaking the design of current userland implementations is a good >> reason. >> - Not being able to have lazy_write (a performance improvement, I >> remind you) by default is a good reason. > > > I think current design of object based save handler is better to be > redesigned. > > Current object based save handler registers "previous" save handler as its > base. > However, it's mostly useless without calling parent open() function. i.e. > Other > calls simply fails when it does not. > > When open() is called, some resource, e.g. file handle, db connection, etc, > is > opened. These resources cannot be accessed from user land, since it's not > "PHP resource", but raw resource. Thus it's only useful for file based > storage. > If user is using their own file based storage for some reason, they are > better > to implement their own handler fully. > > I would like to remove and clean up this in future release. You don't know how it's used. A user may just prepend the session_id before calling parent::, or inject logging into the logic, or whatever - people do all kinds of crazy stuff. >> >> > Besides modular design, if write() and updateTimestamp() are merged, >> >> > flag >> >> > parameter >> >> > for write() should be added. It breaks compatibility with current >> >> > save >> >> > handlers. I don't >> >> > want BC that could be avoided also. >> >> >> >> No it shouldn't, the decision whether to write or just update the >> >> timestamp is based on an internal flag, or on $session_data. No >> >> additional parameters are required. >> > >> > >> > I removed PS(id) dependency from s_read() with new patch as planned. Why >> > should I introduce new dependency to s_write(), i.e. sub module, that >> > breaks >> > design? It does not make sense. >> >> Convenience and consistency in the userland APIs is more important >> than "breaking" internal design. I understand you want to stick to >> best practices, but those practices are not a religion and sometimes >> you need to break them. If you ask me, a lot of things in ext/session >> are broken by design ... creating files from inside read() is one >> example, all methods exposed to userland to be in an interface is >> another. I agree, it's not perfect, but that's just what happens when >> we introduce new features into something that was never designed with >> possible changes in the future in mind. For PHP6, we'll have the >> freedom to redesign the whole thing completely, but for the time being >> - userland code is more important than not having some dependancy in >> write(). > > > It's better to stick to cleaner code. Clean code is worth to have. > As I mentioned in previous, current object based save handler design has > very > limited usage and base class part would be better to be removed in the > future. > (It's not documented also :) And as I previously mentioned - it may be limited or not useful to you, but that's just your opinion. Also, it is documented. :) Cheers, Andrey.