Remove bugged session-related methods from AccountInterface

Closing in favor of #2345611: Load user entity in Cookie AuthenticationProvider instead of using manual queries.

The AccountInterface still mentions sessions in some comments. Those should be reworded.

There is an existing change record mentioning the getSession* methods. I propose to just remove the methods from there.

It seems to me that AccountInerface::getHostname() is broken as well. The hostname column is only present in the session table, but not in the user table. The property on the user entity just returns the client ip (which is obviously wrong for users loaded from the database). Also the current ip is readily available from $request->getClientIp(). Already in D7 core the $user->hostname property is never used (git grep -- "->hostname"). Even the poll module uses ip_address() instead, comment does the same.

I think we should also handle the disparity btw User::getLastAccessedTime() and UserSession::getLastAccessedTime() in this patch.

What about that: In the UserSession class rename the timestamp property to access (also bring comments inline with the ones in User/UserInterface entity) and then just remove the workaround from the cookie provider.

This would entail renaming the column in the {session} table (or transfer the workaround to the SessionHandler)

I don't think so. The timestamp column is set to REQUEST_TIME in SessionHandler::write(), it is not accessed at all in SessionHandler::read() and only used as a secondary index in order to purge old entries in SessionHandler::gc(). The session.timestamp column became completely meaningless outside of this class.

Note that in Drupal 7 it played an important role. It was used to throttle writes on the session table if the session record was not modified during a request. This role has been delegated to the WriteCheckSessionHandler proxy class in Drupal 8.

As per #9ff, setting this to CNW, I will watch this from the side-line and stick to the reviewer role here.

This is likely a random test fail (#2474909: Allow Simpletest to use the same APC user cache prefix so that tests can share the classmap and other cache objects). Please reupload the patch.

I very much like the diffstat here. Thanks!

After commit, one existing change record needs to be updated.

The AccountInterface represents an anonymous or authenticated user of the site. I'd probably move it into \Drupal\Core\Authentication. However that would be a major BC break which not only affects core but also contrib in many ways.

Filed #2477213: Move AccountInterface out of Session namespace.

The only place where the session is still mentioned in the AccountInterface is in the interface description. I propose to just delete the description entirely and only keep the first line (i.e. the brief description).

+++ b/core/lib/Drupal/Core/Session/AccountInterface.php
@@ -10,8 +10,8 @@
+ * Defines an object that has a user id, roles. The interface is implemented

The first sentence is still weird. The interface cannot really define an object - or can it. Also the comma should be an and now that the last part of the enumeration is gone.

+++ b/core/lib/Drupal/Core/Session/AccountInterface.php
@@ -10,8 +10,8 @@
+ * both by the global user account and the user entity.

How about currently logged in user instead of global?

Thanks. #30 addresses the easy part of the concerns in #25, the follow-up #2477213: Move AccountInterface out of Session namespace will address the complicated ones.