Pārlūkot izejas kodu

Use a naive implementation of StyleActivator.

As previous one was buggy and leaked.
Steven Kirk 10 gadi atpakaļ
vecāks
revīzija
8172d8214f

+ 8 - 8
src/Perspex.Base/Collections/PerspexList.cs

@@ -171,7 +171,7 @@ namespace Perspex.Collections
         /// Adds an item to the collection.
         /// </summary>
         /// <param name="item">The item.</param>
-        public void Add(T item)
+        public virtual void Add(T item)
         {
             Validate?.Invoke(item);
             int index = _inner.Count;
@@ -183,7 +183,7 @@ namespace Perspex.Collections
         /// Adds multiple items to the collection.
         /// </summary>
         /// <param name="items">The items.</param>
-        public void AddRange(IEnumerable<T> items)
+        public virtual void AddRange(IEnumerable<T> items)
         {
             Contract.Requires<ArgumentNullException>(items != null);
 
@@ -264,7 +264,7 @@ namespace Perspex.Collections
         /// </summary>
         /// <param name="index">The index.</param>
         /// <param name="item">The item.</param>
-        public void Insert(int index, T item)
+        public virtual void Insert(int index, T item)
         {
             Validate?.Invoke(item);
             _inner.Insert(index, item);
@@ -276,7 +276,7 @@ namespace Perspex.Collections
         /// </summary>
         /// <param name="index">The index.</param>
         /// <param name="items">The items.</param>
-        public void InsertRange(int index, IEnumerable<T> items)
+        public virtual void InsertRange(int index, IEnumerable<T> items)
         {
             Contract.Requires<ArgumentNullException>(items != null);
 
@@ -302,7 +302,7 @@ namespace Perspex.Collections
         /// </summary>
         /// <param name="item">The item.</param>
         /// <returns>True if the item was found and removed, otherwise false.</returns>
-        public bool Remove(T item)
+        public virtual bool Remove(T item)
         {
             int index = _inner.IndexOf(item);
 
@@ -320,7 +320,7 @@ namespace Perspex.Collections
         /// Removes multiple items from the collection.
         /// </summary>
         /// <param name="items">The items.</param>
-        public void RemoveAll(IEnumerable<T> items)
+        public virtual void RemoveAll(IEnumerable<T> items)
         {
             Contract.Requires<ArgumentNullException>(items != null);
 
@@ -337,7 +337,7 @@ namespace Perspex.Collections
         /// Removes the item at the specified index.
         /// </summary>
         /// <param name="index">The index.</param>
-        public void RemoveAt(int index)
+        public virtual void RemoveAt(int index)
         {
             T item = _inner[index];
             _inner.RemoveAt(index);
@@ -349,7 +349,7 @@ namespace Perspex.Collections
         /// </summary>
         /// <param name="index">The first index to remove.</param>
         /// <param name="count">The number of items to remove.</param>
-        public void RemoveRange(int index, int count)
+        public virtual void RemoveRange(int index, int count)
         {
             if (count > 0)
             {

+ 14 - 0
src/Perspex.Controls/Classes.cs

@@ -2,6 +2,7 @@
 // Licensed under the MIT license. See licence.md file in the project root for full license information.
 
 using System.Collections.Generic;
+using System.Linq;
 using Perspex.Collections;
 
 namespace Perspex.Controls
@@ -21,5 +22,18 @@ namespace Perspex.Controls
             : base(items)
         {            
         }
+
+        public override void Add(string item)
+        {
+            if (!Contains(item))
+            {
+                base.Add(item);
+            }
+        }
+
+        public override void AddRange(IEnumerable<string> items)
+        {
+            base.AddRange(items.Where(x => !Contains(x)));
+        }
     }
 }

+ 4 - 1
src/Perspex.Controls/Primitives/SelectingItemsControl.cs

@@ -682,7 +682,10 @@ namespace Perspex.Controls.Primitives
                 case NotifyCollectionChangedAction.Reset:
                     foreach (var item in ItemContainerGenerator.Containers)
                     {
-                        MarkContainerSelected(item, false);
+                        if (item != null)
+                        {
+                            MarkContainerSelected(item, false);
+                        }
                     }
 
                     if (!_syncingSelectedItems)

+ 8 - 7
src/Perspex.Styling/Styling/Selectors.cs

@@ -5,6 +5,7 @@ using System;
 using System.Collections.Generic;
 using System.Collections.Specialized;
 using System.Linq;
+using System.Reactive;
 using System.Reactive.Linq;
 using System.Reflection;
 
@@ -178,16 +179,16 @@ namespace Perspex.Styling
 
         private static SelectorMatch MatchClass(IStyleable control, string name)
         {
-            var changed = Observable.FromEventPattern<
+            var observable = Observable.FromEventPattern<
                     NotifyCollectionChangedEventHandler,
-                    NotifyCollectionChangedEventHandler>(
+                    NotifyCollectionChangedEventArgs>(
                 x => control.Classes.CollectionChanged += x,
-                x => control.Classes.CollectionChanged -= x);
+                x => control.Classes.CollectionChanged -= x)
+                .Select(_ => Unit.Default)
+                .StartWith(Unit.Default)
+                .Select(_ => control.Classes.Contains(name));
 
-            return new SelectorMatch(
-                Observable
-                    .Return(control.Classes.Contains(name))
-                    .Concat(changed.Select(e => control.Classes.Contains(name))));
+            return new SelectorMatch(observable);
         }
 
         private static SelectorMatch MatchDescendent(IStyleable control, Selector previous)

+ 12 - 115
src/Perspex.Styling/Styling/StyleActivator.cs

@@ -4,7 +4,8 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
-using System.Reactive.Disposables;
+using System.Reactive;
+using System.Reactive.Linq;
 
 namespace Perspex.Styling
 {
@@ -14,134 +15,30 @@ namespace Perspex.Styling
         Or,
     }
 
-    public class StyleActivator : IObservable<bool>, IDisposable
+    public class StyleActivator : ObservableBase<bool>
     {
+        private readonly IObservable<bool>[] _inputs;
         private readonly ActivatorMode _mode;
 
-        private readonly bool[] _values;
-
-        private readonly List<IDisposable> _subscriptions = new List<IDisposable>();
-
-        private readonly List<IObserver<bool>> _observers = new List<IObserver<bool>>();
-
         public StyleActivator(
             IList<IObservable<bool>> inputs,
             ActivatorMode mode = ActivatorMode.And)
         {
-            int i = 0;
-
+            _inputs = inputs.ToArray();
             _mode = mode;
-            _values = new bool[inputs.Count];
-
-            foreach (IObservable<bool> input in inputs)
-            {
-                int capturedIndex = i;
-
-                IDisposable subscription = input.Subscribe(
-                    x => Update(capturedIndex, x),
-                    x => Finish(capturedIndex),
-                    () => Finish(capturedIndex));
-                _subscriptions.Add(subscription);
-                ++i;
-            }
         }
 
-        public bool CurrentValue
+        protected override IDisposable SubscribeCore(IObserver<bool> observer)
         {
-            get;
-            private set;
-        }
-
-        public bool HasCompleted
-        {
-            get;
-            private set;
-        }
-
-        public void Dispose()
-        {
-            foreach (IObserver<bool> observer in _observers)
-            {
-                observer.OnCompleted();
-            }
-
-            foreach (IDisposable subscription in _subscriptions)
-            {
-                subscription.Dispose();
-            }
-        }
-
-        public IDisposable Subscribe(IObserver<bool> observer)
-        {
-            Contract.Requires<ArgumentNullException>(observer != null);
-
-            observer.OnNext(CurrentValue);
-
-            if (HasCompleted)
-            {
-                observer.OnCompleted();
-                return Disposable.Empty;
-            }
-            else
-            {
-                _observers.Add(observer);
-                return Disposable.Create(() => _observers.Remove(observer));
-            }
-        }
-
-        private void Update(int index, bool value)
-        {
-            _values[index] = value;
-
-            bool current;
-
-            switch (_mode)
-            {
-                case ActivatorMode.And:
-                    current = _values.All(x => x);
-                    break;
-                case ActivatorMode.Or:
-                    current = _values.Any(x => x);
-                    break;
-                default:
-                    throw new InvalidOperationException("Invalid Activator mode.");
-            }
-
-            if (current != CurrentValue)
-            {
-                Push(current);
-                CurrentValue = current;
-            }
-        }
-
-        private void Finish(int i)
-        {
-            // We can unsubscribe from everything if the completed observable:
-            // - Is the only subscription.
-            // - Has finished on 'false' and we're in And mode
-            // - Has finished on 'true' and we're in Or mode
-            var value = _values[i];
-            var unsubscribe =
-                (_values.Length == 1) ||
-                (_mode == ActivatorMode.And ? !value : value);
-
-            if (unsubscribe)
-            {
-                foreach (IDisposable subscription in _subscriptions)
-                {
-                    subscription.Dispose();
-                }
-
-                HasCompleted = true;
-            }
+            return _inputs.CombineLatest()
+                .Select(Calculate)
+                .DistinctUntilChanged()
+                .Subscribe(observer);
         }
 
-        private void Push(bool value)
+        private bool Calculate(IList<bool> values)
         {
-            foreach (IObserver<bool> observer in _observers)
-            {
-                observer.OnNext(value);
-            }
+            return _mode == ActivatorMode.And ? values.All(x => x) : values.Any(x => x);
         }
     }
 }

+ 0 - 5
tests/Perspex.LeakTests/StyleTests.cs

@@ -2,14 +2,10 @@
 // Licensed under the MIT license. See licence.md file in the project root for full license information.
 
 using System;
-using System.Collections.Generic;
 using System.Linq;
 using JetBrains.dotMemoryUnit;
 using Perspex.Controls;
-using Perspex.Controls.Primitives;
-using Perspex.Controls.Templates;
 using Perspex.Styling;
-using Perspex.VisualTree;
 using Xunit;
 using Xunit.Abstractions;
 
@@ -123,7 +119,6 @@ namespace Perspex.LeakTests
 
             dotMemory.Check(memory =>
                 Assert.Equal(1, memory.GetObjects(where => where.Type.Is<StyleActivator>()).ObjectsCount));
-            Assert.False(true);
         }
     }
 }

+ 5 - 0
tests/Perspex.Styling.UnitTests/Perspex.Styling.UnitTests.csproj

@@ -40,6 +40,11 @@
     <WarningLevel>4</WarningLevel>
   </PropertyGroup>
   <ItemGroup>
+    <Reference Include="Microsoft.Reactive.Testing, Version=2.2.5.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
+      <HintPath>..\..\packages\Rx-Testing.2.2.5\lib\net45\Microsoft.Reactive.Testing.dll</HintPath>
+      <Private>True</Private>
+    </Reference>
+    <Reference Include="Microsoft.VisualStudio.QualityTools.UnitTestFramework, Version=10.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a, processorArchitecture=MSIL" />
     <Reference Include="System" />
     <Reference Include="xunit.assert">
       <HintPath>..\..\packages\xunit.assert.2.0.0\lib\portable-net45+win+wpa81+wp80+monotouch+monoandroid+Xamarin.iOS\xunit.assert.dll</HintPath>

+ 47 - 54
tests/Perspex.Styling.UnitTests/StyleActivatorTests.cs

@@ -2,13 +2,44 @@
 // Licensed under the MIT license. See licence.md file in the project root for full license information.
 
 using System;
+using System.Collections.Generic;
+using System.Reactive;
 using System.Reactive.Linq;
+using Microsoft.Reactive.Testing;
 using Xunit;
 
 namespace Perspex.Styling.UnitTests
 {
-    public class StyleActivatorTests
+    public class StyleActivatorTests : ReactiveTest
     {
+        [Fact]
+        public void Activator_Should_Subscribe_To_Inputs_On_First_Subscription()
+        {
+            var scheduler = new TestScheduler();
+            var source = scheduler.CreateColdObservable<bool>();
+            var target = new StyleActivator(new[] { source }, ActivatorMode.And);
+
+            Assert.Equal(0, source.Subscriptions.Count);
+            target.Subscribe(_ => { });
+            Assert.Equal(1, source.Subscriptions.Count);
+        }
+
+        [Fact]
+        public void Activator_Should_Unsubscribe_From_Inputs_After_Last_Subscriber_Completes()
+        {
+            var scheduler = new TestScheduler();
+            var source = scheduler.CreateColdObservable<bool>();
+            var target = new StyleActivator(new[] { source }, ActivatorMode.And);
+
+            var dispose = target.Subscribe(_ => { });
+            Assert.Equal(1, source.Subscriptions.Count);
+            Assert.Equal(Subscription.Infinite, source.Subscriptions[0].Unsubscribe);
+
+            dispose.Dispose();
+            Assert.Equal(1, source.Subscriptions.Count);
+            Assert.Equal(0, source.Subscriptions[0].Unsubscribe);
+        }
+
         [Fact]
         public void Activator_And_Should_Follow_Single_Input()
         {
@@ -53,32 +84,6 @@ namespace Perspex.Styling.UnitTests
             Assert.Equal(1, inputs[2].SubscriberCount);
         }
 
-        [Fact]
-        public void Activator_And_Should_Unsubscribe_All_When_Input_Completes_On_False()
-        {
-            var inputs = new[]
-            {
-                new TestSubject<bool>(false),
-                new TestSubject<bool>(false),
-                new TestSubject<bool>(true),
-            };
-            var target = new StyleActivator(inputs, ActivatorMode.And);
-            var result = new TestObserver<bool>();
-
-            target.Subscribe(result);
-            Assert.False(result.GetValue());
-            inputs[0].OnNext(true);
-            inputs[1].OnNext(true);
-            Assert.True(result.GetValue());
-            inputs[0].OnNext(false);
-            Assert.False(result.GetValue());
-            inputs[0].OnCompleted();
-
-            Assert.Equal(0, inputs[0].SubscriberCount);
-            Assert.Equal(0, inputs[1].SubscriberCount);
-            Assert.Equal(0, inputs[2].SubscriberCount);
-        }
-
         [Fact]
         public void Activator_And_Should_Not_Unsubscribe_All_When_Input_Completes_On_True()
         {
@@ -96,7 +101,7 @@ namespace Perspex.Styling.UnitTests
             inputs[0].OnNext(true);
             inputs[0].OnCompleted();
 
-            Assert.Equal(1, inputs[0].SubscriberCount);
+            Assert.Equal(0, inputs[0].SubscriberCount);
             Assert.Equal(1, inputs[1].SubscriberCount);
             Assert.Equal(1, inputs[2].SubscriberCount);
         }
@@ -144,31 +149,6 @@ namespace Perspex.Styling.UnitTests
             Assert.Equal(1, inputs[2].SubscriberCount);
         }
 
-        [Fact]
-        public void Activator_Or_Should_Unsubscribe_All_When_Input_Completes_On_True()
-        {
-            var inputs = new[]
-            {
-                new TestSubject<bool>(false),
-                new TestSubject<bool>(false),
-                new TestSubject<bool>(true),
-            };
-            var target = new StyleActivator(inputs, ActivatorMode.Or);
-            var result = new TestObserver<bool>();
-
-            target.Subscribe(result);
-            Assert.True(result.GetValue());
-            inputs[2].OnNext(false);
-            Assert.False(result.GetValue());
-            inputs[0].OnNext(true);
-            Assert.True(result.GetValue());
-            inputs[0].OnCompleted();
-
-            Assert.Equal(0, inputs[0].SubscriberCount);
-            Assert.Equal(0, inputs[1].SubscriberCount);
-            Assert.Equal(0, inputs[2].SubscriberCount);
-        }
-
         [Fact]
         public void Activator_Or_Should_Not_Unsubscribe_All_When_Input_Completes_On_False()
         {
@@ -189,7 +169,7 @@ namespace Perspex.Styling.UnitTests
 
             Assert.Equal(1, inputs[0].SubscriberCount);
             Assert.Equal(1, inputs[1].SubscriberCount);
-            Assert.Equal(1, inputs[2].SubscriberCount);
+            Assert.Equal(0, inputs[2].SubscriberCount);
         }
 
         [Fact]
@@ -207,5 +187,18 @@ namespace Perspex.Styling.UnitTests
 
             Assert.True(completed);
         }
+
+        private Recorded<Notification<bool>>[] OnNextValues(params bool[] values)
+        {
+            var result = new List<Recorded<Notification<bool>>>();
+            var time = 1;
+
+            foreach (var value in values)
+            {
+                result.Add(new Recorded<Notification<bool>>(time, Notification.CreateOnNext<bool>(value)));
+            }
+
+            return result.ToArray();
+        }
     }
 }

+ 1 - 0
tests/Perspex.Styling.UnitTests/packages.config

@@ -6,6 +6,7 @@
   <package id="Rx-Linq" version="2.2.5" targetFramework="net45" />
   <package id="Rx-Main" version="2.2.5" targetFramework="net45" />
   <package id="Rx-PlatformServices" version="2.2.5" targetFramework="net45" />
+  <package id="Rx-Testing" version="2.2.5" targetFramework="net45" />
   <package id="xunit" version="2.0.0" targetFramework="net45" />
   <package id="xunit.abstractions" version="2.0.0" targetFramework="net45" />
   <package id="xunit.assert" version="2.0.0" targetFramework="net45" />