---------- Forwarded message ----------
From: Rowan Collins rowan.collins@gmail.com
To: internals@lists.php.net
Cc:
Date: Thu, 21 Jan 2016 19:18:51 +0000I think the main problem with a close method on something that represents
a resource like a PDO connection object is how the object should behave
after it has been closed. That is, what should the following code do?$pdo = new PDO($dsn);
$pdo->closeConnection();
$pdo->query('Select 1 as test');
I hear your point, but I think that the problem is mostly similar to the
following piece:
$fp = fopen('filename', 'w');
fclose($fp);
fwrite($fp, 'data');
In other words, the developer who explicitly closes a resource, knows why
he is doing it, supposedly.
The object needs to track the fact that it is in a closed state, and do
something - maybe throw a ConnectionAlreadyClosedException?
I think that the PHP developers are quite familiar with this when it comes
to files or sockets, and people like to use fclose when they think it's
needed.
Suppose you opened a file for writing, and then you're done with it (so
that other processes can have it), but your script must then enter a
time-consuming loop, or sleep, or wait blockingly for another resource. You
don't want to lock stalled the initial file when you know you're finished
with it, therefore you call fclose. And you already know, that, should you
need it again, you would call fopen again.
So that means the close() method needs to be accompanied by a reopen()
method, which means saving the DSN and credentials
This use case remains true, if there are other references to $fp elsewhere
in the code. The underlying resource is simply freed, and the object did
not keep track of the path or opening mode, etc.
One solution to this is to create a wrapper around PDO, which is the only
thing allowed to have a direct reference to the PDO object itself.
I think you are perfectly right when it comes to design patterns and good
practices. But alas not every PHP user will or can implement them in this
way.
Even when doing things properly, e.g. using a stable and popular ORM like
Doctrine, I could end up writing:
$pdo = $entityManager->getConnection()->getWrappedConnection();
and then even if Doctrine offered a way to nullify its internal PDO
reference, I ruined it.
Besides, expecting from PHP users that they write a Facade/Singleton
pattern to wrap a core class and proxy all its publics, seems high
standards :)
Of course a PDO closeConnection method is not the hottest emergency, but
all I'm saying is that I could use it... !
Thanks,
Regards,
Gabriel
Gabriel Zerbib wrote on 22/01/2016 13:35:
I hear your point, but I think that the problem is mostly similar to the
following piece:$fp = fopen('filename', 'w');
fclose($fp);
fwrite($fp, 'data');
Sure, but does that mean that PDO should gain a close function, or that
the file functions would be better if they didn't have it? That is
effectively the question we're discussing.
Suppose you opened a file for writing, and then you're done with it (so
that other processes can have it), but your script must then enter a
time-consuming loop, or sleep, or wait blockingly for another resource. You
don't want to lock stalled the initial file when you know you're finished
with it, therefore you call fclose. And you already know, that, should you
need it again, you would call fopen again.
If you have a single reference to the file pointer in your code, then
this is true, and is equivalent to the current unset($pdo) case. When
you need it again, you call new PDO() again.
If you, as you described in your original post, "end up never knowing
what other component of the application still retains a reference", then
running fclose($fh) seems very dangerous indeed - none of those places
know that next time they read from the file handle, they need to reopen
the file to avoid an error; indeed, they probably don't know how to
reopen the file. Most will just assume that, because they were given a
file handle, rather than creating their own, that file handle is and
will remain valid.
Besides, expecting from PHP users that they write a Facade/Singleton
pattern to wrap a core class and proxy all its publics, seems high
standards :)
It doesn't seem that complex to me. I just hacked together a version
(untested, and unoptimised) in 22 lines of code which implements the
"close and then error forever" pattern:
https://gist.github.com/IMSoP/bb0aecf214071a5b59cb This would be really
easy to add to core, but also not that useful - do you really want every
class which has a reference to that object to generate a stream of
exceptions when they try to query?
To actually make it useful, you need to have something you can do about
it, like an openConnection() method. Here's a more complex wrapper,
still only 30 lines of code, which implements a lazy connection (connect
on first use, reconnect on first use after closing):
https://gist.github.com/IMSoP/a8e329e81eacd22a66c1
But this is probably missing features, which different people might want
to do in different ways, e.g. if you call setAttribute(), should the
option stay set after a close() / open() or be reverted (I can imagine
use cases for both ways around)? Should the connection automatically
reopen itself like this, or should it throw an exception and expect the
caller to call open()? Should the connection be opened on construct, or
on first use? Should there be an accessor for isOpen(), or an
ensureOpen()? And if this class replaced PDO, would we be storing
credentials in memory which are normally used once at connection time
and then discarded?
As ever, the devil is in the detail. PDO in general operates at quite a
low-level, rather than being an opinionated implementation. The joy of
the PHP ecosystem right now is that there are tons of nice libraries out
there, and you don't have to wait for a new PHP version to get a bugfix
or feature, just "composer update".
Regards,
Rowan Collins
[IMSoP]