Hi Andrey,
On Sat, Mar 29, 2014 at 10:47 AM, Andrey Andreev <[email protected]> wrote:
> On Sat, Mar 29, 2014 at 3:09 AM, Yasuo Ohgaki <[email protected]> wrote:
> > Hi Andrey,
> >
> > On Sat, Mar 29, 2014 at 8:41 AM, Andrey Andreev <[email protected]>
> 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.
It has very limited usage as I wrote.
Unless base save handler opens it, it's unusable by its spec. i.e.
It simply raises errors for not opening base class. Even when base save
handler open is called, there is no way to handle opened resource in user
land.
Due to these 2 limitations, only file based save handler could be extended
AFAIK.
In other words, it's useless for other save handlers currently. i.e. 2
connections
are required for server based storage. If user must open another
connection, it's
better to open by itself.
When user needs to specialized file save handler, it would better to write
complete handler. User would not be affected by save handler changes
with this way, too.
It's possible to return resources used by C written save handlers, but it's
not
good solution. It requires dependency for modules. I don't think such
dependency
worth it while user may write complete and more efficient handler in user
land.
In addition, there are many resources for a resource. PostgreSQL connection
can be PDO, pgsql, pq, or even more, for instance.
>> > 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.
>
SID extension is not documented.
User may simply call session_id() before session_start() for this.
It's simpler and more efficient.
> >> >> > 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. :)
http://jp1.php.net/manual/en/class.sessionhandler.php
I overlooked this. If you find doc, it would be great if you could provide
link.
There would be a note if base class support is removed.
Just providing interface is good enough as base class is unusable mostly
and less efficient. Making base class usable does not seem to worth the
effort. I think you'll understand what I mean if you try to write it by
yourself.
BTW, you're arguing unlocked session is dangerous. To make base class
usable, user save handler must ignore or disable locking. I think unlocked
session is useful, but you are considering unlocked session is harmful,
aren't you?
Regards,
--
Yasuo Ohgaki
[email protected]