Fully implemented the ProgressionTracker and created some NMock-assisted unit tests for validating its proper functioning

git-svn-id: file:///srv/devel/repo-conversion/nusu@35 d2e56fa2-650e-0410-a79f-9358c0239efd

Nuclex.Support (PC).csproj

@@ -174,10 +174,6 @@
       <XNAUseContentPipeline>false</XNAUseContentPipeline>
       <Name>ThreadOperation</Name>
     </Compile>
-    <Compile Include="Source\Scheduling\WeightedRequirableOperation.cs">
-      <XNAUseContentPipeline>false</XNAUseContentPipeline>
-      <Name>WeightedRequirableOperation</Name>
-    </Compile>
     <Compile Include="Source\Serialization\BinarySerializer.Test.cs">
       <XNAUseContentPipeline>false</XNAUseContentPipeline>
       <Name>BinarySerializer.Test</Name>
@@ -227,6 +223,11 @@
       <XNAUseContentPipeline>false</XNAUseContentPipeline>
       <Name>ProgressionTracker</Name>
     </Compile>
+    <Compile Include="Source\Tracking\ProgressionTracker.Test.cs">
+      <XNAUseContentPipeline>false</XNAUseContentPipeline>
+      <Name>ProgressionTracker.Test</Name>
+      <DependentUpon>ProgressionTracker.cs</DependentUpon>
+    </Compile>
     <Compile Include="Source\Tracking\SetProgression.cs">
       <XNAUseContentPipeline>false</XNAUseContentPipeline>
       <Name>SetProgression</Name>

Source/Scheduling/IAbortable.cs

@@ -30,8 +30,8 @@ namespace Nuclex.Support.Scheduling {
     ///   The receive should honor the abort request and stop whatever it is
     ///   doing as soon as possible. The method does not impose any requirement
     ///   on the timeliness of the reaction of the running process, but implementers
-    ///   are advised to not ignore the abort request and try to design their code
+    ///   are advised to not ignore the abort request and urged to try and design
-    ///   in such a way that it can be stopped in a reasonable time
+    ///   their code in such a way that it can be stopped in a reasonable time
     ///   (eg. within 1 second of the abort request).
     /// </remarks>
     void AsyncAbort();

Source/Scheduling/Operation.cs

@@ -69,7 +69,8 @@ namespace Nuclex.Support.Scheduling {
     /// <summary>Exception that occured while the operation was executing</summary>
     /// <remarks>
     ///   If this field is null, it is assumed that no exception has occured
-    ///   in the background process. When it is set, the End() method will
+    ///   in the background process. If it is set, however, the End() method will
+    ///   re-raise the exception to the calling thread when it is called.
     /// </remarks>
     public Exception OccuredException {
       get { return this.occuredException; }
@@ -82,8 +83,7 @@ namespace Nuclex.Support.Scheduling {
       // We allow the caller to set the exception multiple times. While I certainly
       // can't think of a scenario where this would happen, throwing an exception
       // in that case seems worse. The caller might just be executing an exception
-      // handling block and locking the operation instance could cause all even
+      // handling block and locking + throwing here could cause even more problems.
-      // more problems.
       this.occuredException = exception;
 
     }

Source/Tracking/Internal/ObservedWeightedProgression.cs

@@ -82,14 +82,13 @@ namespace Nuclex.Support.Tracking {
       asyncDisconnectEvents(); // We don't need those anymore!
 
       // If the progress hasn't reached 1.0 yet, make a fake report so that even
-      // when a progression doesn't report any progress at all, the set of queue
+      // when a progression doesn't report any progress at all, the set or queue
       // owning us will have a percentage of progressions completed.
       //
       // There is the possibility of a race condition here, as a final progress
-      // report could have been generated by some other thread that was preempted
+      // report could have been generated by a thread running the progression
-      // by this thread reporting the end of the progression. This is not a problem,
+      // that was preempted by this thread. This would cause the progress to
-      // however, since the progress is inteded only for informal purposes and
+      // jump to 1.0 and then back to whatever the waiting thread will report.
-      // not to be used for controlling program flow.
       if(this.progress != 1.0f) {
         this.progress = 1.0f;
         progressUpdateCallback();

Source/Tracking/ProgressionTracker.Test.cs

@@ -0,0 +1,258 @@
+#region CPL License
+/*
+Nuclex Framework
+Copyright (C) 2002-2007 Nuclex Development Labs
+
+This library is free software; you can redistribute it and/or
+modify it under the terms of the IBM Common Public License as
+published by the IBM Corporation; either version 1.0 of the
+License, or (at your option) any later version.
+
+This library is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+IBM Common Public License for more details.
+
+You should have received a copy of the IBM Common Public
+License along with this library
+*/
+#endregion
+using System;
+using System.Collections.Generic;
+using System.IO;
+
+#if UNITTEST
+
+using NUnit.Framework;
+using NMock2;
+
+namespace Nuclex.Support.Tracking {
+
+  /// <summary>Unit Test for the progression tracker class</summary>
+  [TestFixture]
+  public class ProgressionTrackerTest {
+
+    #region interface IProgressionTrackerSubscriber
+
+    /// <summary>Interface used to test the progression tracker.</summary>
+    public interface IProgressionTrackerSubscriber {
+
+      /// <summary>Called when the progression tracker's progress changes</summary>
+      /// <param name="sender">Progression tracker whose progress has changed</param>
+      /// <param name="e">Contains the new progress achieved</param>
+      void ProgressUpdated(object sender, ProgressUpdateEventArgs e);
+
+      /// <summary>Called when the progression tracker's idle state changes</summary>
+      /// <param name="sender">Progression tracker whose idle state has changed</param>
+      /// <param name="e">Contains the new idle state of the tracker</param>
+      void IdleStateChanged(object sender, IdleStateEventArgs e);
+
+    }
+
+    #endregion // interface IProgressionTrackerSubscriber
+
+    #region class ProgressUpdateEventArgsMatcher
+
+    /// <summary>Compares two ProgressUpdateEventArgsInstances for NMock validation</summary>
+    private class ProgressUpdateEventArgsMatcher : Matcher {
+
+      /// <summary>Initializes a new ProgressUpdateEventArgsMatcher </summary>
+      /// <param name="expected">Expected progress update event arguments</param>
+      public ProgressUpdateEventArgsMatcher(ProgressUpdateEventArgs expected) {
+        this.expected = expected;
+      }
+
+      /// <summary>
+      ///   Called by NMock to verfiy the ProgressUpdateEventArgs match the expected value
+      /// </summary>
+      /// <param name="actualAsObject">Actual value to compare to the expected value</param>
+      /// <returns>
+      ///   True if the actual value matches the expected value; otherwise false
+      /// </returns>
+      public override bool Matches(object actualAsObject) {
+        ProgressUpdateEventArgs actual = (actualAsObject as ProgressUpdateEventArgs);
+        if(actual == null)
+          return false;
+
+        return (actual.Progress == this.expected.Progress);
+      }
+
+      /// <summary>Creates a string representation of the expected value</summary>
+      /// <param name="writer">Writer to write the string representation into</param>
+      public override void DescribeTo(TextWriter writer) {
+        writer.Write(this.expected.Progress.ToString());
+      }
+
+      /// <summary>Expected progress update event args value</summary>
+      private ProgressUpdateEventArgs expected;
+
+    }
+
+    #endregion // class ProgressUpdateEventArgsMatcher
+
+    #region class TestProgression
+
+    /// <summary>Progression used for testing in this unit test</summary>
+    private class TestProgression : Progression {
+
+      /// <summary>Changes the testing progression's indicated progress</summary>
+      /// <param name="progress">
+      ///   New progress to be reported by the testing progression
+      /// </param>
+      public void ChangeProgress(float progress) {
+        OnAsyncProgressUpdated(progress);
+      }
+
+      /// <summary>Transitions the progression into the ended state</summary>
+      public void End() {
+        OnAsyncEnded();
+      }
+
+    }
+
+    #endregion // class TestProgression
+
+    /// <summary>Initialization routine executed before each test is run</summary>
+    [SetUp]
+    public void Setup() {
+      this.mockery = new Mockery();
+    }
+
+    /// <summary>Validates that the tracker properly sums the progress</summary>
+    [Test]
+    public void TestSummedProgress() {
+      ProgressionTracker tracker = new ProgressionTracker();
+
+      IProgressionTrackerSubscriber mockedSubscriber = mockSubscriber(tracker);
+
+      Expect.Once.On(mockedSubscriber).
+        Method("IdleStateChanged").
+        WithAnyArguments();
+
+      Expect.Exactly(2).On(mockedSubscriber).
+        Method("ProgressUpdated").
+        With(
+          new Matcher[] {
+            new NMock2.Matchers.TypeMatcher(typeof(ProgressionTracker)),
+            new ProgressUpdateEventArgsMatcher(new ProgressUpdateEventArgs(0.0f))
+          }
+        );
+
+      TestProgression test1 = new TestProgression();
+      tracker.Track(test1);
+      TestProgression test2 = new TestProgression();
+      tracker.Track(test2);
+
+      Expect.Once.On(mockedSubscriber).
+        Method("ProgressUpdated").
+        With(
+          new Matcher[] {
+            new NMock2.Matchers.TypeMatcher(typeof(ProgressionTracker)),
+            new ProgressUpdateEventArgsMatcher(new ProgressUpdateEventArgs(0.25f))
+          }
+        );
+
+      test1.ChangeProgress(0.5f);
+
+      this.mockery.VerifyAllExpectationsHaveBeenMet();
+    }
+
+    /// <summary>
+    ///   Validates that the tracker only removes progressions when the whole
+    ///   tracking list has reached the 'ended' state.
+    /// </summary>
+    /// <remarks>
+    ///   If the tracker would remove ended progressions right when they finished,
+    ///   the total progress would jump back each time. This is unwanted, of course.
+    /// </remarks>
+    [Test]
+    public void TestDelayedRemoval() {
+      ProgressionTracker tracker = new ProgressionTracker();
+
+      IProgressionTrackerSubscriber mockedSubscriber = mockSubscriber(tracker);
+
+      Expect.Once.On(mockedSubscriber).
+        Method("IdleStateChanged").
+        WithAnyArguments();
+
+      Expect.Exactly(2).On(mockedSubscriber).
+        Method("ProgressUpdated").
+        With(
+          new Matcher[] {
+            new NMock2.Matchers.TypeMatcher(typeof(ProgressionTracker)),
+            new ProgressUpdateEventArgsMatcher(new ProgressUpdateEventArgs(0.0f))
+          }
+        );
+
+      TestProgression test1 = new TestProgression();
+      tracker.Track(test1);
+      TestProgression test2 = new TestProgression();
+      tracker.Track(test2);
+
+      Expect.Once.On(mockedSubscriber).
+        Method("ProgressUpdated").
+        With(
+          new Matcher[] {
+            new NMock2.Matchers.TypeMatcher(typeof(ProgressionTracker)),
+            new ProgressUpdateEventArgsMatcher(new ProgressUpdateEventArgs(0.25f))
+          }
+        );
+
+      test1.ChangeProgress(0.5f);
+
+      Expect.Once.On(mockedSubscriber).
+        Method("ProgressUpdated").
+        With(
+          new Matcher[] {
+            new NMock2.Matchers.TypeMatcher(typeof(ProgressionTracker)),
+            new ProgressUpdateEventArgsMatcher(new ProgressUpdateEventArgs(0.75f))
+          }
+        );
+
+      // Total progress should be 0.75 after this call (one progression at 1.0,
+      // the other one at 0.5). If the second progression would be removed,
+      // the progress would jump to 0.5 instead.
+      test2.End();
+
+      Expect.Once.On(mockedSubscriber).
+        Method("ProgressUpdated").
+        With(
+          new Matcher[] {
+            new NMock2.Matchers.TypeMatcher(typeof(ProgressionTracker)),
+            new ProgressUpdateEventArgsMatcher(new ProgressUpdateEventArgs(1.0f))
+          }
+        );
+
+      Expect.Once.On(mockedSubscriber).
+        Method("IdleStateChanged").
+        WithAnyArguments();
+
+      test1.End();
+
+      this.mockery.VerifyAllExpectationsHaveBeenMet();
+    }
+
+    /// <summary>Mocks a subscriber for the events of a tracker</summary>
+    /// <param name="tracker">Tracker to mock an event subscriber for</param>
+    /// <returns>The mocked event subscriber</returns>
+    private IProgressionTrackerSubscriber mockSubscriber(ProgressionTracker tracker) {
+      IProgressionTrackerSubscriber mockedSubscriber =
+        this.mockery.NewMock<IProgressionTrackerSubscriber>();
+
+      tracker.AsyncIdleStateChanged +=
+        new EventHandler<IdleStateEventArgs>(mockedSubscriber.IdleStateChanged);
+
+      tracker.AsyncProgressUpdated +=
+        new EventHandler<ProgressUpdateEventArgs>(mockedSubscriber.ProgressUpdated);
+
+      return mockedSubscriber;
+    }
+
+    /// <summary>Mock object factory</summary>
+    private Mockery mockery;
+
+  }
+
+} // namespace Nuclex.Support.Tracking
+
+#endif // UNITTEST

Source/Tracking/ProgressionTracker.cs

@@ -19,7 +19,7 @@ License along with this library
 #endregion
 using System;
 using System.Collections.Generic;
-using System.Text;
+using System.Threading;
 
 namespace Nuclex.Support.Tracking {
 
@@ -39,6 +39,36 @@ namespace Nuclex.Support.Tracking {
   /// </remarks>
   public class ProgressionTracker : IDisposable {
 
+    #region class ProgressionMatcher
+
+    /// <summary>Matches a progression to a fully wrapped one</summary>
+    private class ProgressionMatcher {
+
+      /// <summary>
+      ///   Initializes a new progression matcher that matches against
+      ///   the specified progression
+      /// </summary>
+      /// <param name="toMatch">Progression to match against</param>
+      public ProgressionMatcher(Progression toMatch) {
+        this.toMatch = toMatch;
+      }
+
+      /// <summary>
+      ///   Checks whether the provided progression matches the comparison
+      ///   progression of the instance
+      /// </summary>
+      /// <param name="other">Progression to match to the comparison progression</param>
+      public bool Matches(ObservedWeightedProgression<Progression> other) {
+        return ReferenceEquals(other.WeightedProgression.Progression, this.toMatch);
+      }
+
+      /// <summary>Progression this instance compares against</summary>
+      private Progression toMatch;
+
+    }
+
+    #endregion // class ProgressionMatcher
+
     /// <summary>Triggered when the idle state of the tracker changes</summary>
     /// <remarks>
     ///   The tracker is idle when no progressions are being tracked in it. If you're
@@ -51,9 +81,30 @@ namespace Nuclex.Support.Tracking {
     /// <summary>Triggered when the total progress has changed</summary>
     public event EventHandler<ProgressUpdateEventArgs> AsyncProgressUpdated;
 
+    /// <summary>Initializes a new progression tracker</summary>
+    public ProgressionTracker() {
+
+      this.trackedProgressions = new List<ObservedWeightedProgression<Progression>>();
+      this.idle = true;
+
+      this.asyncEndedDelegate =
+        new ObservedWeightedProgression<Progression>.ReportDelegate(asyncEnded);
+      this.asyncProgressUpdatedDelegate =
+        new ObservedWeightedProgression<Progression>.ReportDelegate(asyncProgressUpdated);
+
+    }
+
     /// <summary>Immediately releases all resources owned by the instance</summary>
     public void Dispose() {
-      // TODO: Untrack all
+      lock(this.trackedProgressions) {
+
+        for(int index = 0; index < this.trackedProgressions.Count; ++index)
+          this.trackedProgressions[index].Dispose();
+
+        this.trackedProgressions.Clear();
+        this.trackedProgressions = null;
+
+      } // lock
     }
 
     /// <summary>Begins tracking the specified progression</summary>
@@ -66,21 +117,98 @@ namespace Nuclex.Support.Tracking {
     /// <param name="progression">Progression to be tracked</param>
     /// <param name="weight">Weight to assign to this progression</param>
     public void Track(Progression progression, float weight) {
+
+      // Add the new progression into the tracking list. This has to be done
+      // inside a lock to prevent issues with the progressUpdate callback, which could
+      // access the totalWeight field before it has been updated to reflect the
+      // new progression added to the collection.
+      lock(this.trackedProgressions) {
+
         this.trackedProgressions.Add(
-        new WeightedProgression<Progression>(progression, weight)
+          new ObservedWeightedProgression<Progression>(
+            new WeightedProgression<Progression>(progression, weight),
+            this.asyncProgressUpdatedDelegate,
+            this.asyncEndedDelegate
+          )
         );
+
+        // This can be done after we registered the wrapper to our delegates because
+        // any incoming progress updates will be stopped from the danger of a
+        // division-by-zero from the potentially still zeroed totalWeight by the lock.
         this.totalWeight += weight;
 
+        // If this is the first progression to be added to the list, tell our
+        // owner that we're idle no longer!
+        if(this.trackedProgressions.Count == 1)
+          OnAsyncIdleStateChanged(false);
+
+      } // lock
+
+      // All done, the total progress is different now, so force a recalculation and
+      // send out the AsyncProgressUpdated event.
       recalculateProgress();
+
     }
 
     /// <summary>Stops tracking the specified progression</summary>
     /// <param name="progression">Progression to stop tracking of</param>
-    public void Untrack(Progression progression) { }
+    public void Untrack(Progression progression) {
+      lock(this.trackedProgressions) {
+
+        // Locate the object to be untracked in our collection
+        int removeIndex = this.trackedProgressions.FindIndex(
+          new Predicate<ObservedWeightedProgression<Progression>>(
+            new ProgressionMatcher(progression).Matches
+          )
+        );
+        if(removeIndex == -1)
+          throw new InvalidOperationException("Item is not being tracked");
+
+        // Remove and dispose the progression the user wants to untrack
+        {
+          ObservedWeightedProgression<Progression> wrappedProgression =
+            this.trackedProgressions[removeIndex];
+          this.trackedProgressions.RemoveAt(removeIndex);
+          wrappedProgression.Dispose();
+        }
+
+        // If the list is empty, then we're back in the idle state
+        if(this.trackedProgressions.Count == 0) {
+
+          this.totalWeight = 0.0f;
+
+          // Report that we're idle now!
+          OnAsyncIdleStateChanged(true);
+
+        } else {
+
+          // Rebuild the total weight from scratch. Subtracting the removed progression's
+          // weight would work, too, but might accumulate rounding errors making the sum
+          // drift slowly away from the actual value.
+          this.totalWeight = 0.0f;
+          for(int index = 0; index < this.trackedProgressions.Count; ++index)
+            this.totalWeight += this.trackedProgressions[index].WeightedProgression.Weight;
+
+        }
+
+      } // lock
+    }
+
+    /// <summary>Whether the tracker is currently idle</summary>
+    public bool Idle {
+      get { return this.idle; }
+    }
+
+    /// <summary>Current summed progress of the tracked progressions</summary>
+    public float Progress {
+      get { return this.progress; }
+    }
 
     /// <summary>Fires the AsyncIdleStateChanged event</summary>
     /// <param name="idle">New idle state to report</param>
     protected virtual void OnAsyncIdleStateChanged(bool idle) {
+      this.idle = idle;
+
       EventHandler<IdleStateEventArgs> copy = AsyncIdleStateChanged;
       if(copy != null)
         copy(this, new IdleStateEventArgs(idle));
@@ -89,6 +217,8 @@ namespace Nuclex.Support.Tracking {
     /// <summary>Fires the AsyncProgressUpdated event</summary>
     /// <param name="progress">New progress to report</param>
     protected virtual void OnAsyncProgressUpdated(float progress) {
+      this.progress = progress;
+
       EventHandler<ProgressUpdateEventArgs> copy = AsyncProgressUpdated;
       if(copy != null)
         copy(this, new ProgressUpdateEventArgs(progress));
@@ -96,22 +226,77 @@ namespace Nuclex.Support.Tracking {
 
     /// <summary>Recalculates the total progress of the tracker</summary>
     private void recalculateProgress() {
-      float totalProgress;
+      float totalProgress = 0.0f;
 
-      for(int index = 0; index < trackedProgressions.Count; ++index) {
+      // Lock the progression to avoid trouble when someone tries to remove one
-        float weight = this.trackedProgressions[index].WeightedProgression;
+      // of our tracked progressions while we're just processing a progress update
-        totalProgress = this.trackedProgressions[index].Progress * weight;
+      lock(this.trackedProgressions) {
+
+        // This is a safety measure. In theory, even after all progressions have
+        // ended and collection of tracked progressions is cleared, a waiting
+        // thread might deliver another progress update causing this method to
+        // be entered. In this case, the right thing is to do nothing at all.
+        if(this.totalWeight == 0.0f)
+          return;
+
+        // Sum up the total progress
+        for(int index = 0; index < this.trackedProgressions.Count; ++index) {
+          float weight = this.trackedProgressions[index].WeightedProgression.Weight;
+          totalProgress += this.trackedProgressions[index].Progress * weight;
         }
 
+        // This also needs to be in the lock to guarantee that the totalWeight is
+        // the one for the number of progressions we just summed -- by design,
+        // the total weight always has to be updated at the same time as the collection.
         totalProgress /= this.totalWeight;
+      }
 
-      //OnAsyncProgressUpdated(
+      // Finally, trigger the event
+      OnAsyncProgressUpdated(totalProgress);
+    }
+
+    /// <summary>Called when one of the tracked progressions has ended</summary>
+    private void asyncEnded() {
+      lock(this.trackedProgressions) {
+
+        // If any progressions in the list are still going, keep the entire list.
+        // This behavior is intentional in order to prevent the tracker's progress from
+        // jumping back repeatedly when multiple tracked progressions come to an end.
+        for(int index = 0; index < this.trackedProgressions.Count; ++index)
+          if(!this.trackedProgressions[index].WeightedProgression.Progression.Ended)
+            return;
+
+        // All progressions have finished, get rid of the wrappers and disconnect
+        // their events.
+        for(int index = 0; index < this.trackedProgressions.Count; ++index)
+          this.trackedProgressions[index].Dispose();
+
+        this.trackedProgressions.Clear();
+        this.totalWeight = 0.0f;
+
+        // Notify our owner that we're idle now.
+        OnAsyncIdleStateChanged(true);
+
+      }
+    }
+
+    /// <summary>Called when one of the tracked progression has achieved progress</summary>
+    private void asyncProgressUpdated() {
+      recalculateProgress();
     }
 
     /// <summary>Total weight of all progressions being tracked</summary>
-    private float totalWeight;
+    private volatile float totalWeight;
     /// <summary>Progressions being tracked by this tracker</summary>
-    private List<ObservedWeightedProgression<ProgressionType>> trackedProgressions;
+    private List<ObservedWeightedProgression<Progression>> trackedProgressions;
+    /// <summary>Delegate for the asyncEnded() method</summary>
+    private ObservedWeightedProgression<Progression>.ReportDelegate asyncEndedDelegate;
+    /// <summary>Delegate for the asyncProgressUpdated() method</summary>
+    private ObservedWeightedProgression<Progression>.ReportDelegate asyncProgressUpdatedDelegate;
+    /// <summary>Whether the tracker is currently idle</summary>
+    private bool idle;
+    /// <summary>Current summed progress of the tracked progressions</summary>
+    private float progress;
 
   }