Merge pull request #205 from bjester/a-sync-story

Don't close sync session unless also older than cutoff

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 List of the most important changes for each release.
 
+## 0.6.19
+- The `cleanupsyncs` management command now only cleans up sync sessions if also inactive for `expiration` amount of time
+- Fixes issue accessing index on queryset in `cleanupsyncs` management command
+
 ## 0.6.18
 - Prevent creation of Deleted and HardDeleted models during deserialization to allow propagation of syncable objects that are recreated after a previous deletion without causing a merge conflict.
 

morango/__init__.py

@@ -3,4 +3,4 @@
 from __future__ import unicode_literals
 
 default_app_config = "morango.apps.MorangoConfig"
-__version__ = "0.6.18"
+__version__ = "0.6.19"

morango/management/commands/cleanupsyncs.py

@@ -68,7 +68,8 @@ def handle(self, *args, **options):
 
         sync_sessions = SyncSession.objects.filter(active=True)
 
-        # if ids arg was passed, filter down sessions to only those IDs if included by expiration filter
+        # if ids arg was passed, filter down sessions to only those IDs
+        # if included by expiration filter
         if options["ids"]:
             sync_sessions = sync_sessions.filter(id__in=options["ids"])
 
@@ -97,8 +98,7 @@ def handle(self, *args, **options):
         transfer_count = transfer_sessions.count()
 
         # loop over the stale sessions one by one to close them out
-        for i in range(transfer_count):
-            transfer_session = transfer_sessions[0]
+        for i, transfer_session in enumerate(transfer_sessions):
             logger.info(
                 "TransferSession {} of {}: deleting {} Buffers and {} RMC Buffers...".format(
                     i + 1,
@@ -114,17 +114,21 @@ def handle(self, *args, **options):
                 transfer_session.active = False
                 transfer_session.save()
 
+        # in order to close a sync session, it must have no active transfer sessions
+        # and must have no activity since the cutoff
+        sync_sessions = sync_sessions.filter(
+            last_activity_timestamp__lt=cutoff,
+        ).exclude(
+            transfersession__active=True,
+        )
         sync_count = sync_sessions.count()
 
-        # finally loop over sync sessions and close out if there are no other active transfer sessions
-        for i in range(sync_count):
-            sync_session = sync_sessions[0]
-            if not sync_session.transfersession_set.filter(active=True).exists():
-                logger.info(
-                    "Closing SyncSession {} of {}".format(
-                        i + 1,
-                        sync_count,
-                    )
+        for i, sync_session in enumerate(sync_sessions):
+            logger.info(
+                "Closing SyncSession {} of {}".format(
+                    i + 1,
+                    sync_count,
                 )
-                sync_session.active = False
-                sync_session.save()
+            )
+            sync_session.active = False
+            sync_session.save()

tests/testapp/tests/test_management_commands.py

@@ -136,9 +136,11 @@ def test_filtering_sessions_by_pull_cleared(self):
         syncsession_new, transfersession_new = _create_sessions(push=False)
         call_command("cleanupsyncs", pull=not transfersession_old.push, expiration=0)
         self.assertTransferSessionIsCleared(transfersession_old)
-        self.assertSyncSessionIsActive(syncsession_old)
+        self.assertSyncSessionIsNotActive(syncsession_old)
         self.assertTransferSessionIsCleared(transfersession_new)
-        self.assertSyncSessionIsActive(syncsession_new)
+        self.assertSyncSessionIsNotActive(syncsession_new)
+        self.assertTransferSessionIsNotCleared(self.transfersession_old)
+        self.assertTransferSessionIsNotCleared(self.transfersession_new)
 
     def test_multiple_ids_as_list(self):
         ids = [self.syncsession_old.id, self.syncsession_new.id]
@@ -147,3 +149,29 @@ def test_multiple_ids_as_list(self):
         self.assertSyncSessionIsNotActive(self.syncsession_old)
         self.assertTransferSessionIsCleared(self.transfersession_new)
         self.assertSyncSessionIsNotActive(self.syncsession_new)
+
+    def test_sync_session_cutoff(self):
+        """
+        Test that sync sessions are not cleared even if they have no active transfer sessions,
+        if they are still within the cutoff window.
+        """
+        sync_session, transfer_session = _create_sessions(34)
+        # recent successful transfer session
+        transfer_session.active = False
+        transfer_session.save()
+        # create old incomplete transfer session for same session
+        _, old_transfer_session = _create_sessions(38, sync_session=sync_session)
+
+        call_command("cleanupsyncs", expiration=36)
+        self.assertSyncSessionIsActive(sync_session)
+
+    def test_sync_session_cleanup_with_active_xfer(self):
+        sync_session, transfer_session = _create_sessions(38)
+        # recent successful transfer session
+        transfer_session.active = False
+        transfer_session.save()
+        # create old incomplete transfer session for same session
+        _, new_transfer_session = _create_sessions(34, sync_session=sync_session)
+
+        call_command("cleanupsyncs", expiration=36)
+        self.assertSyncSessionIsActive(sync_session)