silverstripe / silverstripe-session-manager

Allow users to manage and revoke access to multiple login sessions across devices.
BSD 3-Clause "New" or "Revised" License
9 stars 6 forks source link

Garbage collector act as a black box #160

Open lekoala opened 1 year ago

lekoala commented 1 year ago

This is something I've mentioned here https://github.com/silverstripe/silverstripe-session-manager/issues/153 already, but I feel that it's worth to discuss it as a distinct issue.

At the moment, the GarbageCollector acts as a black box. You can collect, and you don't know in advance if it will do something or if it did anything.

This leads to two issues:

Ideally, I think the various types of objects to be collected should be accessible from outside the service (maybe static getters on the login session? or as static methods on the garbage collector itself). This way, you can check how many sessions will be collected before calling ->collect(). This can be useful also to get some idea of the current state of the system (eg: do I have any session waiting to be collected?)

Then, I think that the collect method, instead of returning void, could return some kind of result. In the other issue, I've shown an example where it returns an array with a counter for each method, but it could be a result object. Or, if the signature of the collect method cannot be changed, maybe add a "getResults" method with the results of the latest collect method call.

lekoala commented 5 months ago

currently it's also kind of hard to get a full overview of all session about to be collected (since the logic is split accross three functions that you need to merge together in order to get a full list)

here is some sample code to get that in one go if anyone is interested

        $maxAge = LoginSession::getMaxAge();
        $nowTs = DBDatetime::now()->getTimestamp();
        $now = date('Y-m-d H:i:s', $nowTs);

        $case1 = "(LastAccessed < '$maxAge' AND Persistent = 0)";
        $case2 = "(Persistent = 1 AND RememberLoginHash.ExpiryDate < '$now')";
        $case3 = "(LastAccessed < '$maxAge' AND Persistent = 1 AND RememberLoginHash.ExpiryDate IS NULL)";

        $list = LoginSession::get()->leftJoin('RememberLoginHash', 'RememberLoginHash.LoginSessionID = LoginSession.ID');
        $list = $list->where("$case1 OR $case2 OR $case3");