Fixed orphaned job issue in JobController

System/Jobs now shows items currently in queue.

NzbDrone.Core.Test/Framework/TestBase.cs

@@ -1,4 +1,5 @@
 using NUnit.Framework;
+using NzbDrone.Core.Providers.Jobs;
 
 namespace NzbDrone.Core.Test.Framework
 {
@@ -17,6 +18,7 @@ namespace NzbDrone.Core.Test.Framework
         [TearDown]
         public void TearDown()
         {
+            JobProvider.Queue.Clear();
             ExceptionVerification.AssertNoUnexcpectedLogs();
         }
 

NzbDrone.Core.Test/JobProviderTest.cs

@@ -1,9 +1,11 @@
 // ReSharper disable RedundantUsingDirective
 using System;
+using System.Collections;
 using System.Collections.Generic;
 using System.Threading;
 using AutoMoq;
 using FluentAssertions;
+using Moq;
 using NUnit.Framework;
 using NzbDrone.Core.Model.Notification;
 using NzbDrone.Core.Providers.Jobs;
@@ -31,9 +33,8 @@ namespace NzbDrone.Core.Test
 
             //Assert
             var settings = timerProvider.All();
-            Assert.IsNotEmpty(settings);
+
             Assert.AreNotEqual(DateTime.MinValue, settings[0].LastExecution);
-            settings[0].Success.Should().BeTrue();
         }
 
         [Test]
@@ -51,32 +52,33 @@ namespace NzbDrone.Core.Test
             timerProvider.Initialize();
             timerProvider.RunScheduled();
 
+            Thread.Sleep(1000);
+
             //Assert
             var settings = timerProvider.All();
-            Assert.IsNotEmpty(settings);
             Assert.AreNotEqual(DateTime.MinValue, settings[0].LastExecution);
-            Assert.IsFalse(settings[0].Success);
+            settings[0].Success.Should().BeFalse();
             ExceptionVerification.ExcpectedErrors(1);
         }
 
         [Test]
-        //This test will confirm that the concurrency checks are rest
+       public void scheduler_skips_jobs_that_arent_mature_yet()
-        //after execution so the job can successfully run.
-        public void can_run_job_again()
         {
-            IList<IJob> fakeJobs = new List<IJob> { new FakeJob() };
+            var fakeJob = new FakeJob();
             var mocker = new AutoMoqer();
 
+            IList<IJob> fakeJobs = new List<IJob> { fakeJob };
             mocker.SetConstant(MockLib.GetEmptyDatabase());
             mocker.SetConstant(fakeJobs);
 
             var timerProvider = mocker.Resolve<JobProvider>();
             timerProvider.Initialize();
-            var firstRun = timerProvider.RunScheduled();
+            timerProvider.RunScheduled();
-            var secondRun = timerProvider.RunScheduled();
+            Thread.Sleep(500);
+            timerProvider.RunScheduled();
+            Thread.Sleep(500);
 
-            firstRun.Should().BeTrue();
+            fakeJob.ExexutionCount.Should().Be(1);
-            secondRun.Should().BeTrue();
         }
 
         [Test]
@@ -84,21 +86,21 @@ namespace NzbDrone.Core.Test
         //after execution so the job can successfully run.
         public void can_run_async_job_again()
         {
-            IList<IJob> fakeJobs = new List<IJob> { new FakeJob() };
+            var fakeJob = new FakeJob();
             var mocker = new AutoMoqer();
-
+            
+            IList<IJob> fakeJobs = new List<IJob> {fakeJob};
             mocker.SetConstant(MockLib.GetEmptyDatabase());
             mocker.SetConstant(fakeJobs);
 
             var timerProvider = mocker.Resolve<JobProvider>();
             timerProvider.Initialize();
-            var firstRun = timerProvider.QueueJob(typeof(FakeJob));
+            timerProvider.QueueJob(typeof(FakeJob));
-            Thread.Sleep(2000);
+            Thread.Sleep(1000);
-            var secondRun = timerProvider.QueueJob(typeof(FakeJob));
+            timerProvider.QueueJob(typeof(FakeJob));
-
+            Thread.Sleep(1000);
-            firstRun.Should().BeTrue();
-            secondRun.Should().BeTrue();
             JobProvider.Queue.Should().BeEmpty();
+            fakeJob.ExexutionCount.Should().Be(2);
         }
 
         [Test]
@@ -130,7 +132,8 @@ namespace NzbDrone.Core.Test
         //after execution so the job can successfully run.
         public void can_run_broken_async_job_again()
         {
-            IList<IJob> fakeJobs = new List<IJob> { new BrokenJob() };
+            var brokenJob = new BrokenJob();
+            IList<IJob> fakeJobs = new List<IJob> { brokenJob };
             var mocker = new AutoMoqer();
 
             mocker.SetConstant(MockLib.GetEmptyDatabase());
@@ -138,14 +141,14 @@ namespace NzbDrone.Core.Test
 
             var timerProvider = mocker.Resolve<JobProvider>();
             timerProvider.Initialize();
-            var firstRun = timerProvider.QueueJob(typeof(BrokenJob));
+            timerProvider.QueueJob(typeof(BrokenJob));
             Thread.Sleep(2000);
-            var secondRun = timerProvider.QueueJob(typeof(BrokenJob));
+            timerProvider.QueueJob(typeof(BrokenJob));
+
 
-            firstRun.Should().BeTrue();
-            secondRun.Should().BeTrue();
             Thread.Sleep(2000);
             JobProvider.Queue.Should().BeEmpty();
+            brokenJob.ExexutionCount.Should().Be(2);
             ExceptionVerification.ExcpectedErrors(2);
         }
 
@@ -154,7 +157,8 @@ namespace NzbDrone.Core.Test
         //after execution so the job can successfully run.
         public void can_run_two_jobs_at_the_same_time()
         {
-            IList<IJob> fakeJobs = new List<IJob> { new SlowJob() };
+            var slowJob = new SlowJob();
+            IList<IJob> fakeJobs = new List<IJob> { slowJob };
             var mocker = new AutoMoqer();
 
             mocker.SetConstant(MockLib.GetEmptyDatabase());
@@ -163,20 +167,18 @@ namespace NzbDrone.Core.Test
             var timerProvider = mocker.Resolve<JobProvider>();
             timerProvider.Initialize();
 
-            bool firstRun = false;
-            bool secondRun = false;
 
-            var thread1 = new Thread(() => firstRun = timerProvider.RunScheduled());
+            var thread1 = new Thread(() => timerProvider.RunScheduled());
             thread1.Start();
             Thread.Sleep(1000);
-            var thread2 = new Thread(() => secondRun = timerProvider.RunScheduled());
+            var thread2 = new Thread(() => timerProvider.RunScheduled());
             thread2.Start();
 
             thread1.Join();
             thread2.Join();
 
-            firstRun.Should().BeTrue();
+
-            Assert.IsFalse(secondRun);
+            slowJob.ExexutionCount = 2;
 
         }
 
@@ -389,6 +391,61 @@ namespace NzbDrone.Core.Test
             Assert.IsNotEmpty(settings);
             Assert.IsFalse(settings[0].Success);
         }
+
+
+        [Test]
+        public void existing_queue_should_start_queue_if_not_running()
+        {
+            var mocker = new AutoMoqer();
+
+            var fakeJob = new FakeJob();
+            IList<IJob> fakeJobs = new List<IJob> { fakeJob };
+
+
+            mocker.SetConstant(MockLib.GetEmptyDatabase());
+            mocker.SetConstant(fakeJobs);
+
+            var fakeQueueItem = new Tuple<Type, int>(fakeJob.GetType(), 12);
+            //Act
+            var jobProvider = mocker.Resolve<JobProvider>();
+            jobProvider.Initialize();
+            JobProvider.Queue.Add(fakeQueueItem);
+            jobProvider.QueueJob(fakeJob.GetType(), 12);
+            Thread.Sleep(1000);
+
+            //Assert
+            fakeJob.ExexutionCount.Should().Be(1);
+        }
+
+
+        [Test]
+        public void Item_added_to_queue_while_scheduler_runs_is_executed()
+        {
+            var mocker = new AutoMoqer();
+
+            var slowJob = new SlowJob();
+            var disabledJob = new DisabledJob();
+            IList<IJob> fakeJobs = new List<IJob> { slowJob, disabledJob };
+
+            mocker.SetConstant(MockLib.GetEmptyDatabase());
+            mocker.SetConstant(fakeJobs);
+
+            mocker.Resolve<JobProvider>().Initialize();
+
+            var _jobThread = new Thread(() => mocker.Resolve<JobProvider>().RunScheduled());
+            _jobThread.Start();
+
+            Thread.Sleep(200);
+
+            mocker.Resolve<JobProvider>().QueueJob(typeof(DisabledJob), 12);
+
+            Thread.Sleep(3000);
+
+            //Assert
+            JobProvider.Queue.Should().BeEmpty();
+            slowJob.ExexutionCount.Should().Be(1);
+            disabledJob.ExexutionCount.Should().Be(1);
+        }
     }
 
     public class FakeJob : IJob
@@ -403,9 +460,11 @@ namespace NzbDrone.Core.Test
             get { return 15; }
         }
 
+        public int ExexutionCount { get; set; }
+
         public void Start(ProgressNotification notification, int targetId)
         {
-
+            ExexutionCount++;
         }
     }
 
@@ -441,8 +500,11 @@ namespace NzbDrone.Core.Test
             get { return 15; }
         }
 
+        public int ExexutionCount { get; set; }
+
         public void Start(ProgressNotification notification, int targetId)
         {
+            ExexutionCount++;
             throw new ApplicationException("Broken job is broken");
         }
     }

NzbDrone.Core/Providers/Jobs/JobProvider.cs

@@ -67,40 +67,31 @@ namespace NzbDrone.Core.Providers.Jobs
         /// <summary>
         /// Iterates through all registered jobs and executed any that are due for an execution.
         /// </summary>
-        /// <returns>True if ran, false if skipped</returns>
+        public virtual void RunScheduled()
-        public virtual bool RunScheduled()
         {
             lock (ExecutionLock)
             {
                 if (_isRunning)
                 {
                     Logger.Trace("Queue is already running. Ignoring scheduler's request.");
-                    return false;
+                    return;
-                }
-                _isRunning = true;
-            }
-
-            try
-            {
-                var pendingJobs = All().Where(
-                    t => t.Enable &&
-                         (DateTime.Now - t.LastExecution) > TimeSpan.FromMinutes(t.Interval)
-                    );
-
-                foreach (var pendingTimer in pendingJobs)
-                {
-                    var timer = pendingTimer;
-                    var timerClass = _jobs.Where(t => t.GetType().ToString() == timer.TypeName).FirstOrDefault();
-                    Execute(timerClass.GetType());
                 }
             }
-            finally
+
+            var counter = 0;
+
+            var pendingJobs = All().Where(
+                t => t.Enable &&
+                     (DateTime.Now - t.LastExecution) > TimeSpan.FromMinutes(t.Interval)
+                ).Select(c => _jobs.Where(t => t.GetType().ToString() == c.TypeName).Single());
+
+            foreach (var job in pendingJobs)
             {
-                _isRunning = false;
+                QueueJob(job.GetType());
+                counter++;
             }
 
-            Logger.Trace("Finished executing scheduled tasks.");
+            Logger.Trace("{0} Scheduled tasks have been added to the queue", counter);
-            return true;
         }
 
         /// <summary>
@@ -109,32 +100,33 @@ namespace NzbDrone.Core.Providers.Jobs
         /// <param name="jobType">Type of the job that should be executed.</param>
         /// <param name="targetId">The targetId could be any Id parameter eg. SeriesId. it will be passed to the job implementation
         /// to allow it to filter it's target of execution.</param>
-        /// <returns>True if queued, false if duplicate and was skipped</returns>
         /// <remarks>Job is only added to the queue if same job with the same targetId doesn't already exist in the queue.</remarks>
-        public virtual bool QueueJob(Type jobType, int targetId = 0)
+        public virtual void QueueJob(Type jobType, int targetId = 0)
         {
             Logger.Debug("Adding [{0}:{1}] to the queue", jobType.Name, targetId);
-            lock (Queue)
-            {
-                var queueTuple = new Tuple<Type, int>(jobType, targetId);
-
-                if (Queue.Contains(queueTuple))
-                {
-                    Logger.Info("[{0}:{1}] already exists in job queue. Skipping.", jobType.Name, targetId);
-                    return false;
-                }
-
-                Queue.Add(queueTuple);
-                Logger.Trace("Job [{0}:{1}] added to the queue", jobType.Name, targetId);
-
-            }
 
             lock (ExecutionLock)
             {
+                lock (Queue)
+                {
+                    var queueTuple = new Tuple<Type, int>(jobType, targetId);
+
+                    if (!Queue.Contains(queueTuple))
+                    {
+                        Queue.Add(queueTuple);
+                        Logger.Trace("Job [{0}:{1}] added to the queue", jobType.Name, targetId);
+
+                    }
+                    else
+                    {
+                        Logger.Info("[{0}:{1}] already exists in job queue. Skipping.", jobType.Name, targetId);
+                    }
+                }
+
                 if (_isRunning)
                 {
                     Logger.Trace("Queue is already running. No need to start it up.");
-                    return true;
+                    return;
                 }
                 _isRunning = true;
             }
@@ -166,10 +158,8 @@ namespace NzbDrone.Core.Providers.Jobs
             else
             {
                 Logger.Error("Execution lock has fucked up. Thread still active. Ignoring request.");
-                return true;
             }
 
-            return true;
         }
 
         /// <summary>

NzbDrone.Web/Controllers/SystemController.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Linq;
 using System.Collections.Generic;
 using System.IO;
 using System.Web.Mvc;
@@ -28,6 +29,7 @@ namespace NzbDrone.Web.Controllers
 
         public ActionResult Jobs()
         {
+            ViewData["Queue"] = JobProvider.Queue.Select(c => new Tuple<String, int>(c.Item1.Name, c.Item2));
             return View(_jobProvider.All());
         }
 

NzbDrone.Web/NzbDrone.Web.csproj

@@ -45,6 +45,7 @@
     <PlatformTarget>x86</PlatformTarget>
   </PropertyGroup>
   <ItemGroup>
+    <Reference Include="Microsoft.CSharp" />
     <Reference Include="Microsoft.Web.Infrastructure, Version=1.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
       <SpecificVersion>False</SpecificVersion>
       <HintPath>..\Libraries\MVC3\Microsoft.Web.Infrastructure.dll</HintPath>

NzbDrone.Web/Views/System/Jobs.cshtml

@@ -1,9 +1,15 @@
-@model IEnumerable<NzbDrone.Core.Repository.JobDefinition>
+@using System.Collections
+@model IEnumerable<NzbDrone.Core.Repository.JobDefinition>
 @section TitleContent{
 Jobs
 }
 @section MainContent{
     @{Html.Telerik().Grid(Model).Name("Grid")
-            .TableHtmlAttributes(new { @class = "Grid" })
+    .Render();}
-            .Render();}
+
+    Items currently in queue
+
+    @{Html.Telerik().Grid((IEnumerable<Tuple<String, int>>)ViewData["Queue"]).Name("QueueGrid")
+          .Columns(c => c.Bound(g => g.Item1).Title("Type").Width(100)).Columns(c => c.Bound(g => g.Item2).Title("Target"))
+        .Render();}
 }