Hi Stas,
On Sat, Jan 24, 2015 at 8:49 AM, Stanislav Malyshev <smalys...@gmail.com>
wrote:
> > This is the only reasonable use I know. I would to write user
> > serializer(read/writer)
> > handler for it.
>
> So we went from no reasonable use to one reasonable use, documented at
> the manual. I think it is also reasonable to suppose there are more uses
> for it.
>
This is because session module lacks user defined serializer. Save handler
handles session data storage. Serialize handler handles how data is
converted/represented. IMHO.
Basic save handler design is good while it is bad not to provide user
serialize
handler.
> > My point is SessionHandler class is (almost) useless as a CLASS. It
>
> I think we established it is not useless. It may not be useful for you,
> but may be useful for other people. Creating a large BC break for a
> reason that something is not useable for someone is not a good idea, I
> think. I do not see any benefit such action would produce.
>
> > Goal is removing abused class usage.(cleanup) e.g. Refer to
>
> The action should not be its own goal, and I don't see how it's
> "abused". So far we have seen an example of use which you recognized as
> reasonable, and which is endorsed by our own manual,, and no examples of
> abuse.
>
I've mentioned the code, PHP_FUNCTION(session_set_save_handler).
/* For compatibility reason, implemeted interface is not checked */
/* Find implemented methods - SessionHandlerInterface */
i = 0;
ZEND_HASH_FOREACH_STR_KEY(&php_session_iface_entry->function_table,
func_name) {
if ((current_mptr = zend_hash_find_ptr(&Z_OBJCE_P(obj)->function_table,
func_name))) {
if (!Z_ISUNDEF(PS(mod_user_names).names[i])) {
zval_ptr_dtor(&PS(mod_user_names).names[i]);
}
array_init_size(&PS(mod_user_names).names[i], 2);
Z_ADDREF_P(obj);
add_next_index_zval(&PS(mod_user_names).names[i], obj);
add_next_index_str(&PS(mod_user_names).names[i],
zend_string_copy(func_name));
} else {
php_error_docref(NULL, E_ERROR, "Session handler's function table is
corrupt");
RETURN_FALSE;
}
++i;
} ZEND_HASH_FOREACH_END();
/* Find implemented methods - SessionIdInterface (optional) */
ZEND_HASH_FOREACH_STR_KEY(&php_session_id_iface_entry->function_table,
func_name) {
if ((current_mptr = zend_hash_find_ptr(&Z_OBJCE_P(obj)->function_table,
func_name))) {
if (!Z_ISUNDEF(PS(mod_user_names).names[i])) {
zval_ptr_dtor(&PS(mod_user_names).names[i]);
}
array_init_size(&PS(mod_user_names).names[i], 2);
Z_ADDREF_P(obj);
add_next_index_zval(&PS(mod_user_names).names[i], obj);
add_next_index_str(&PS(mod_user_names).names[i],
zend_string_copy(func_name));
} else {
if (!Z_ISUNDEF(PS(mod_user_names).names[i])) {
zval_ptr_dtor(&PS(mod_user_names).names[i]);
ZVAL_UNDEF(&PS(mod_user_names).names[i]);
}
}
++i;
} ZEND_HASH_FOREACH_END();
/* Find implemented methods - SessionUpdateTimestampInterface (optional) */
ZEND_HASH_FOREACH_STR_KEY(&php_session_update_timestamp_iface_entry->function_table,
func_name) {
if ((current_mptr = zend_hash_find_ptr(&Z_OBJCE_P(obj)->function_table,
func_name))) {
if (!Z_ISUNDEF(PS(mod_user_names).names[i])) {
zval_ptr_dtor(&PS(mod_user_names).names[i]);
}
array_init_size(&PS(mod_user_names).names[i], 2);
Z_ADDREF_P(obj);
add_next_index_zval(&PS(mod_user_names).names[i], obj);
add_next_index_str(&PS(mod_user_names).names[i],
zend_string_copy(func_name));
} else {
if (!Z_ISUNDEF(PS(mod_user_names).names[i])) {
zval_ptr_dtor(&PS(mod_user_names).names[i]);
ZVAL_UNDEF(&PS(mod_user_names).names[i]);
}
}
++i;
} ZEND_HASH_FOREACH_END();
As you can see, it does not check if user class implements/extends
interface/class.
> PHP_FUNCTION(session_set_save_handler).
> > Advocate OOP best practice. i.e. Use INTERFACE when user is required to
> > implement all required methods.
>
> There's no OOP best practice that says you can not extend classes and
> override some of its methods in order to modify parent's behavior. On
> the contrary, doing this is one of common OOP practices.
>
The PHP_FUNCTION(session_set_save_handler) code could be best practice
hardly.
>
> > And provide good API (both internal/user). e.g. Missing functions like
> > user serializer, gc, create_id.
>
> If you want to provide additional API, by all means please make an RFC.
> But I do not see how removing this class is required for providing any API.
>
Fair enough. I'll implement user serialize handler. Removal of
SessionHandler
class may be discussed when use of user serialize handler became dominant.
I would like to make SessionHandler deprecated, encourage user serialize
handler for new PHP because it's more efficient and clean. Do you agree?
> If user need native handler, they should use it directly. IMO.
>
> I do not see any reason for that. You can clearly use native handler and
> add modifications for it. In fact, that is exactly why that class exists.
>
> > I would like to discuss one by one. Shall we discuss user serializer?
> > What do you think?
>
> Again, if you want to propose new feature, by all means please do. I
> still don't see how it requires removing existing classes. This is a
> very big BC break and requires very big gain to justify, and so far the
> gain was not shown I think.
OK. I'm willing to remove SessionHandler class if nobody objects.
It appears there are two at least.
Let's keep SessionHandler class. However,
PHP_FUNCTION(session_set_save_handler)
should be cleaned up to verify implemented/extended interface/class. It's
BC.
Do you have opinion for this?
Regards,
--
Yasuo Ohgaki
yohg...@ohgaki.net
