Merge pull request #155 from lekoala/patch-5

Fix mock time in GarbageCollection test
silverstripe · Aug 3, 2023 · bc5e08c · bc5e08c
2 parents c1f65ae + 30389d1
commit bc5e08c
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 7 deletions.
diff --git a/src/Services/GarbageCollectionService.php b/src/Services/GarbageCollectionService.php
@@ -3,6 +3,7 @@
 namespace SilverStripe\SessionManager\Services;
 
 use SilverStripe\Core\Injector\Injectable;
+use SilverStripe\ORM\FieldType\DBDatetime;
 use SilverStripe\Security\RememberLoginHash;
 use SilverStripe\SessionManager\Models\LoginSession;
 
@@ -26,21 +27,31 @@ public function collect(): void
     private function collectExpiredSessions(): void
     {
         $lifetime = LoginSession::config()->get('default_session_lifetime');
-        $sessions = LoginSession::get()->filter([
-            'LastAccessed:LessThan' => date('Y-m-d H:i:s', time() - $lifetime),
+        $now = DBDatetime::now()->getTimestamp() - $lifetime;
+        LoginSession::get()->filter([
+            'LastAccessed:LessThan' => date('Y-m-d H:i:s', $now),
             'Persistent' => 0
-        ]);
-        $sessions->removeAll();
+        ])->removeAll();
     }
 
     /**
      * Collect all persistent LoginSession records where the associated RememberLoginHash has expired
      */
     private function collectImplicitlyExpiredSessions(): void
     {
+        $now = DBDatetime::now()->getTimestamp();
+        LoginSession::get()->filter([
+            'Persistent' => 1,
+            'LoginHash.ExpiryDate:LessThan' => date('Y-m-d H:i:s', $now),
+        ])->removeAll();
+
+        $lifetime = LoginSession::config()->get('default_session_lifetime');
+        $now = DBDatetime::now()->getTimestamp() - $lifetime;
+        // If a persistent session has no login hash, use LastAccessed
         LoginSession::get()->filter([
+            'LastAccessed:LessThan' => date('Y-m-d H:i:s', $now),
             'Persistent' => 1,
-            'LoginHash.ExpiryDate:LessThan' => date('Y-m-d H:i:s')
+            'LoginHash.ExpiryDate' => null,
         ])->removeAll();
     }
 
@@ -49,8 +60,9 @@ private function collectImplicitlyExpiredSessions(): void
      */
     private function collectExpiredLoginHashes(): void
     {
+        $now = DBDatetime::now()->getTimestamp();
         RememberLoginHash::get()->filter([
-            'ExpiryDate:LessThan' => date('Y-m-d H:i:s')
+            'ExpiryDate:LessThan' => date('Y-m-d H:i:s', $now),
         ])->removeAll();
     }
 }
diff --git a/tests/php/Services/GarbageCollectionServiceTest.php b/tests/php/Services/GarbageCollectionServiceTest.php
@@ -21,7 +21,7 @@ class GarbageCollectionServiceTest extends SapphireTest
 
     public function testGarbageCollection()
     {
-        DBDatetime::set_mock_now('2003-08-15 12:00:00');
+        DBDatetime::set_mock_now('2003-05-16 12:00:00');
 
         $id1 = $this->objFromFixture(LoginSession::class, 'x1')->ID;
         $id2 = $this->objFromFixture(LoginSession::class, 'x2')->ID;
@@ -34,13 +34,29 @@ public function testGarbageCollection()
             LoginSession::get()->byID($id1),
             "Expired login session is deleted"
         );
+        // ExpiryDate for the hash is set to '2003-05-15 10:00:00' => it should be deleted
         $this->assertNull(
             LoginSession::get()->byID($id2),
             "Expired persistent login hash session is deleted"
         );
+        // LastAccessed is set to '2004-02-15 10:00:00' and it has no hash => it should not be deleted
         $this->assertNotNull(
             LoginSession::get()->byID($id3),
             "Valid login session is not deleted"
         );
+        $this->assertEquals(
+            0,
+            LoginSession::get()->byID($id3)->LoginHash()->ID,
+            "There is no hash but session is still valid"
+        );
+
+        DBDatetime::set_mock_now('2005-08-15 12:00:00');
+
+        $garbageCollectionService->collect();
+
+        $this->assertNull(
+            LoginSession::get()->byID($id3),
+            "Persistent Login session is now deleted"
+        );
     }
 }