diff --git a/Source/Collections/AsyncVirtualObservableReadOnlyList.cs b/Source/Collections/AsyncVirtualObservableReadOnlyList.cs
index 7cb43bd..cc4781e 100644
--- a/Source/Collections/AsyncVirtualObservableReadOnlyList.cs
+++ b/Source/Collections/AsyncVirtualObservableReadOnlyList.cs
@@ -23,6 +23,8 @@ using System.Collections.Generic;
using System.Diagnostics;
using System.Threading.Tasks;
using System.Diagnostics.CodeAnalysis;
+using System.Threading;
+
#if !NO_SPECIALIZED_COLLECTIONS
using System.Collections.Specialized;
@@ -63,7 +65,7 @@ namespace Nuclex.Avalonia.Collections {
/// Immediately releases all resources owned by the instance
public void Dispose() {
- this.virtualList = null!; // Only to make life easier got the GC
+ this.virtualList = null!; // Only to decouple and make the GC's work easier
}
/// The item at the enumerator's current position
@@ -73,6 +75,8 @@ namespace Nuclex.Avalonia.Collections {
checkVersion();
#endif
+ // If the most recent call to MoveNext() returned false, it means that
+ // the enumerator has reached the end of the list, so in that
if(this.lastMoveNextResult == false) {
throw new InvalidOperationException("Enumerator is not on a valid position");
}
@@ -122,8 +126,8 @@ namespace Nuclex.Avalonia.Collections {
}
/// The item at the enumerator's current position
- object IEnumerator.Current {
- get { return Current!; } // No idea what the compiler's issue is here
+ object? IEnumerator.Current {
+ get { return Current; }
}
#if DEBUG
@@ -149,9 +153,9 @@ namespace Nuclex.Avalonia.Collections {
#endregion // class Enumerator
/// Raised when an item has been added to the collection
- public event EventHandler>? ItemAdded;
+ public event EventHandler>? ItemAdded { add {} remove{} }
/// Raised when an item is removed from the collection
- public event EventHandler>? ItemRemoved;
+ public event EventHandler>? ItemRemoved { add {} remove{} }
/// Raised when an item is replaced in the collection
public event EventHandler>? ItemReplaced;
/// Raised when the collection is about to be cleared
@@ -160,9 +164,9 @@ namespace Nuclex.Avalonia.Collections {
/// contained in the collection, but it is often simpler and more efficient
/// to process the clearing of the entire collection as a special operation.
///
- public event EventHandler? Clearing;
+ public event EventHandler? Clearing { add {} remove{} }
/// Raised when the collection has been cleared
- public event EventHandler? Cleared;
+ public event EventHandler? Cleared { add {} remove{} }
#if !NO_SPECIALIZED_COLLECTIONS
/// Called when the collection has changed
@@ -182,7 +186,7 @@ namespace Nuclex.Avalonia.Collections {
/// performance from requesting multiple items at once.
///
public AsyncVirtualObservableReadOnlyList(int pageSize = 32) {
- this.typedList = new TItem[0];
+ this.typedList = new List();
this.objectList = (IList)this.typedList;
this.pageSize = pageSize;
this.fetchedPages = new bool[0];
@@ -197,18 +201,51 @@ namespace Nuclex.Avalonia.Collections {
/// will make them available for garbage collection.
///
public void InvalidateAll(bool purgeItems = false) {
- if(this.assumedCount.HasValue) { // If not fetched before, no action needed
- int pageCount = this.fetchedPages.Length;
- for(int index = 0; index < pageCount; ++index) {
- this.fetchedPages[index] = false;
+ int itemCount;
+ lock(this) {
+ if(!this.assumedCount.HasValue) {
+ return; // If not fetched before, no action is needed
}
- if(purgeItems) {
- int itemCount = this.assumedCount.Value;
- for(int index = 0; index < itemCount; ++index) {
- this.typedList[index] = default(TItem)!; // not going to be exposed to users
+ itemCount = this.assumedCount.Value;
+ }
+
+ // Mark the pages as un-fetched
+ int pageCount = this.fetchedPages.Length;
+ if(purgeItems) {
+ var oldItemList = new List(capacity: this.pageSize);
+
+ for(int pageIndex = 0; pageIndex < pageCount; ++pageIndex) {
+ if(this.fetchedPages[pageIndex]) {
+ this.fetchedPages[pageIndex] = false;
+
+ // Figure out the page's item start index and how big the page is
+ int offset = pageIndex * this.pageSize;
+ int count = Math.Min(itemCount - offset, this.pageSize);
+
+ // Make a backup of the old item values so we can provide them
+ // in the ItemReplaced change notification we'll send out.
+ for(int index = 0; index < count; ++index) {
+ oldItemList[index] = this.typedList[offset + index];
+ }
+
+ // Replace the loaded items with placeholder items
+ CreatePlaceholderItems(this.typedList, pageIndex * this.pageSize, count);
+
+ // Send out change notifications for all items we just replace with placeholders
+ for(int index = 0; index < count; ++index) {
+ OnReplaced(oldItemList[index], this.typedList[offset + index], offset + index);
+ }
+
+ // Make sure the old items aren't lingering around
+ oldItemList.Clear();
}
}
+
+ } else {
+ for(int pageIndex = 0; pageIndex < pageCount; ++pageIndex) {
+ this.fetchedPages[pageIndex] = false;
+ }
}
}
@@ -229,39 +266,30 @@ namespace Nuclex.Avalonia.Collections {
/// when any of them are next accessed.
///
public void Invalidate(int itemIndex, bool purgeItems = false) {
- if(this.assumedCount.HasValue) { // If not fetched before, no action needed
- int pageIndex = itemIndex / this.pageSize;
- this.fetchedPages[pageIndex] = false;
-
- if(purgeItems) {
- int count = Math.Min(
- this.assumedCount.Value - (this.pageSize * pageIndex),
- this.pageSize
- );
- for(int index = itemIndex / this.pageSize; index < count; ++index) {
- this.typedList[index] = default(TItem)!; // not going to be exposed to users
- }
+ lock(this) {
+ if(!this.assumedCount.HasValue) {
+ return; // If not fetched before, no action is needed
}
}
+
+ // Mark the entie page as needing re-fetching (we've got no other choice)
+ int pageIndex = itemIndex / this.pageSize;
+ this.fetchedPages[pageIndex] = false;
+
+ // If we're asked to also purge the item, replace the item with
+ // a placeholder item and trigger the change notification
+ if(purgeItems) {
+ TItem oldItem = this.typedList[itemIndex];
+ CreatePlaceholderItems(this.typedList, itemIndex, 1);
+ OnReplaced(oldItem, this.typedList[itemIndex], itemIndex);
+ }
}
/// Determines the index of the specified item in the list
/// Item whose index will be determined
/// The index of the item in the list or -1 if not found
public int IndexOf(TItem item) {
- requireCount();
- requireAllPages();
-
- // TODO: this won't work, it will compare the placeholder items :-/
-
- IComparer itemComparer = Comparer.Default;
- for(int index = 0; index < this.assumedCount.Value; ++index) {
- if(itemComparer.Compare(this.typedList[index], item) == 0) {
- return index;
- }
- }
-
- return -1;
+ return this.typedList.IndexOf(item);
}
/// Inserts an item into the list at the specified index
@@ -316,7 +344,10 @@ namespace Nuclex.Avalonia.Collections {
/// Item the list will be checked for
/// True if the list contains the specified items
public bool Contains(TItem item) {
- return (IndexOf(item) != -1);
+ requireCount();
+ requireAllPages();
+
+ return this.typedList.Contains(item);
}
/// Copies the contents of the list into an array
@@ -334,14 +365,16 @@ namespace Nuclex.Avalonia.Collections {
/// Total number of items in the list
public int Count {
get {
- requireCount();
- return this.assumedCount.Value;
+ return requireCount();
}
}
/// Whether the list is a read-only list
public bool IsReadOnly {
- get { return this.typedList.IsReadOnly; }
+ get {
+ //return this.typedList.IsReadOnly;
+ return true;
+ }
}
/// Removes the specified item from the list
@@ -402,14 +435,14 @@ namespace Nuclex.Avalonia.Collections {
///
/// The position at which the item has been inserted or -1 if the item was not inserted
///
- int IList.Add(object value) {
+ int IList.Add(object? value) {
throw new NotSupportedException("Cannot add items into a read-only list");
}
/// Checks whether the list contains the specified item
/// Item the list will be checked for
/// True if the list contains the specified items
- bool IList.Contains(object item) {
+ bool IList.Contains(object? item) {
requireCount();
requireAllPages();
return this.objectList.Contains(item);
@@ -418,7 +451,7 @@ namespace Nuclex.Avalonia.Collections {
/// Determines the index of the specified item in the list
/// Item whose index will be determined
/// The index of the item in the list or -1 if not found
- int IList.IndexOf(object item) {
+ int IList.IndexOf(object? item) {
requireCount();
requireAllPages();
return this.objectList.IndexOf(item);
@@ -427,7 +460,7 @@ namespace Nuclex.Avalonia.Collections {
/// Inserts an item into the list at the specified index
/// Index the item will be inserted at
/// Item that will be inserted into the list
- void IList.Insert(int index, object item) {
+ void IList.Insert(int index, object? item) {
throw new NotSupportedException("Cannot insert items into a read-only list");
}
@@ -438,14 +471,14 @@ namespace Nuclex.Avalonia.Collections {
/// Removes the specified item from the list
/// Item that will be removed from the list
- void IList.Remove(object item) {
+ void IList.Remove(object? item) {
throw new NotSupportedException("Cannot remove items from a read-only list");
}
/// Accesses the item at the specified index in the list
/// Index of the item that will be accessed
/// The item at the specified index
- object IList.this[int index] {
+ object? IList.this[int index] {
get {
requireCount();
requirePage(index / this.pageSize);
@@ -453,56 +486,12 @@ namespace Nuclex.Avalonia.Collections {
return this.objectList[index];
}
set {
- // Make sure the page is fetched, otherwise, the item would only suddenly
- // revert to its state in the source when the pages around it is fetchd later.
- requireCount();
- requirePage(index / this.pageSize);
-#if DEBUG
- ++this.version;
-#endif
- TItem oldItem = this.typedList[index];
- this.objectList[index] = value;
- TItem newItem = this.typedList[index];
- OnReplaced(oldItem, newItem, index);
+ throw new NotSupportedException("Cannot assign items into a read-only list");
}
}
#endregion // IList implementation
- /// Fires the 'ItemAdded' event
- /// Item that has been added to the collection
- /// Index of the added item
- protected virtual void OnAdded(TItem item, int index) {
- if(ItemAdded != null) {
- ItemAdded(this, new ItemEventArgs(item));
- }
-#if !NO_SPECIALIZED_COLLECTIONS
- if(CollectionChanged != null) {
- CollectionChanged(
- this,
- new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, item, index)
- );
- }
-#endif
- }
-
- /// Fires the 'ItemRemoved' event
- /// Item that has been removed from the collection
- /// Index the item has been removed from
- protected virtual void OnRemoved(TItem item, int index) {
- if(ItemRemoved != null) {
- ItemRemoved(this, new ItemEventArgs(item));
- }
-#if !NO_SPECIALIZED_COLLECTIONS
- if(CollectionChanged != null) {
- CollectionChanged(
- this,
- new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Remove, item, index)
- );
- }
-#endif
- }
-
/// Fires the 'ItemReplaced' event
/// Item that has been replaced
/// New item the original item was replaced with
@@ -523,25 +512,6 @@ namespace Nuclex.Avalonia.Collections {
#endif
}
- /// Fires the 'Clearing' event
- protected virtual void OnClearing() {
- if(Clearing != null) {
- Clearing(this, EventArgs.Empty);
- }
- }
-
- /// Fires the 'Cleared' event
- protected virtual void OnCleared() {
- if(Cleared != null) {
- Cleared(this, EventArgs.Empty);
- }
-#if !NO_SPECIALIZED_COLLECTIONS
- if(CollectionChanged != null) {
- CollectionChanged(this, Constants.NotifyCollectionResetEventArgs);
- }
-#endif
- }
-
/// Counts the total number of items in the virtual collection
/// The total number of items
protected abstract int CountItems();
@@ -583,16 +553,58 @@ namespace Nuclex.Avalonia.Collections {
IList target, int startIndex, int count
);
+ /// Reports errors when fetching data
+ /// Exception that has occured
+ /// Describes the action during which the error happened
+ protected abstract void HandleFetchError(Exception error, string action);
+
/// Ensures that the total number of items is known
+ /// The item count
[MemberNotNull(nameof(assumedCount))]
- private void requireCount() {
- if(!this.assumedCount.HasValue) {
- int itemCount = CountItems();
+ private int requireCount() {
+ lock(this) {
+ if(this.assumedCount.HasValue) {
+ return this.assumedCount.Value;
+ }
+ }
+
+ int itemCount;
+ try {
+ itemCount = CountItems();
+ }
+ catch(Exception error) {
+ lock(this) {
+ this.assumedCount = 0; // Act as if the collection has zero items
+ }
+
+ // Do not let the error bubble up into what is likely an unsuspecting
+ // data grid control or virtualized list widget. Let the user decide what
+ // to do about the error - i.e. set an error flag in their view model.
+ HandleFetchError(
+ error, "Failed to determine the number of items in the collection"
+ );
+ return 0;
+ } // try CountItems() catch
+
+ // Items were counted successfully, so adopt the count and initialize our
+ // internal arrays to avoid having to resize the arrays later on (which
+ // saves us a few lock statements)
+ lock(this) {
this.assumedCount = itemCount;
int pageCount = (itemCount + this.pageSize - 1) / this.pageSize;
this.fetchedPages = new bool[pageCount];
- }
+
+ // Resize the list (so we don't have to mutex-lock the list itself),
+ // but put default items in there. We'll create placeholders when they
+ // are accessed, not before (ideally, I'd want uninitialized instances
+ // here, but that would need very thorough verification
+ while(this.typedList.Count < itemCount) {
+ this.typedList.Add(default(TItem)!);
+ }
+ } // lock this
+
+ return itemCount;
}
/// Ensures that all items have fetched
@@ -600,55 +612,96 @@ namespace Nuclex.Avalonia.Collections {
/// Avoid if possible.
///
private void requireAllPages() {
- Debug.Assert(
- this.assumedCount.HasValue,
- "This method should only be called when item count is already known"
- );
+ lock(this) {
+ Debug.Assert(
+ this.assumedCount.HasValue,
+ "This method should only be called when item count is already known"
+ );
+ }
- // We may find that the list is shorter than expected while requesting pages.
- // But the results will come in asynchronously, so we can't wait for it.
- int pageCount = (this.assumedCount!.Value + this.pageSize - 1) / this.pageSize;
+ int pageCount = this.fetchedPages.Length;
for(int index = 0; index < pageCount; ++index) {
requirePage(index);
+ // CHECK: Should we throttle this by constructing a clever chain of
+ // ContinueWith() tasks so we don't cause a flood of async requests?
}
}
/// Ensures that the specified page has been fetched
/// Index of the page that needs to be fetched
private async void requirePage(int pageIndex) {
- Debug.Assert(
- this.assumedCount.HasValue,
- "This method should only be called when item count is already known"
- );
- if(this.fetchedPages[pageIndex]) {
+ int itemCount;
+ lock(this) {
+ if(!this.assumedCount.HasValue) {
+ Debug.Assert(
+ this.assumedCount.HasValue,
+ "This method should only be called when item count is already known"
+ );
+ return;
+ }
+ itemCount = this.assumedCount.Value;
+ }
+
+ // If the page is already fetched (or in flight), do nothing
+ if((pageIndex >= this.fetchedPages.Length) || this.fetchedPages[pageIndex]) {
return;
}
- int startIndex = pageIndex * this.pageSize;
- int count = Math.Min(this.assumedCount!.Value - startIndex, this.pageSize);
- CreatePlaceholderItems(this.typedList, pageIndex * this.pageSize, count);
+ // The page was not fetched, so synchronously fill it with place holder items
+ // as a first step (their creation should be fast and immediate).
+ int offset = pageIndex * this.pageSize;
+ int count = Math.Min(itemCount - offset, this.pageSize);
+ CreatePlaceholderItems(this.typedList, offset, count);
+ // Ensure the items are present before marking the page as fetched
+ Interlocked.MemoryBarrier();
this.fetchedPages[pageIndex] = true; // Prevent double-fetch
- // Send out change notifications that the items have been replaced
- // (at this point, they're only placeholder items, of course)
- var placeholderItems = new List(count);
- for(int index = startIndex; index < count; ++index) {
- placeholderItems[index - startIndex] = this.typedList[index];
- //OnReplaced(default(TItem), this.typedList[index], index);
+ // Remember the placeholder items that are going to be replaced by
+ // the fetch operation below,allowing us to report these in our change notification
+ var placeholderItems = new List(capacity: count);
+ for(int index = offset; index < count; ++index) {
+ placeholderItems.Add(this.typedList[index + offset]);
+
+ // We act as if the whole list was filled with place holders from the start,
+ // but only realize these as needed (so no change notification for these).
+ // If we did it right, the user will not be able to ever see the uninitialized
+ // items, yet we only create placeholders when they are really needed.
+ //OnReplaced(default(TItem), this.typedList[index + offset], index);
}
- int fetchedItemCount = await FetchItemsAsync(this.typedList, startIndex, count);
+ // Now request the items on the page. This request will run asynchronously,
+ // but we'll use an await to get to deliver change notifications once
+ // the page has been fetched and the items are there.
+ int fetchedItemCount;
+ try {
+ fetchedItemCount = await FetchItemsAsync(this.typedList, offset, count);
+ }
+ catch(Exception error) {
+
+ // Do not let the error bubble up into what is likely an unsuspecting
+ // data grid control or virtualized list widget. Let the user decide what
+ // to do about the error - i.e. set an error flag in their view model.
+ HandleFetchError(
+ error, $"Failed to fetch list items {offset} to {offset + count}"
+ );
+ this.fetchedPages[pageIndex] = false; // We failed!
+ return; // Leave the placeholder items in
+
+ }
if(fetchedItemCount < this.pageSize) {
- this.assumedCount = startIndex + fetchedItemCount;
+ itemCount = offset + fetchedItemCount;
+ lock(this) {
+ this.assumedCount = itemCount;
+ }
}
// The count may have been adjusted if this truncated the list,
// so recalculate the actual number of items. Then send out change
// notifications for the items that have now been fetched.
- count = Math.Min(this.assumedCount!.Value - startIndex, this.pageSize);
- for(int index = startIndex; index < count; ++index) {
- OnReplaced(placeholderItems[index - startIndex], this.typedList[index], index);
+ count = Math.Min(itemCount - offset, this.pageSize);
+ for(int index = offset; index < count; ++index) {
+ OnReplaced(placeholderItems[index - offset], this.typedList[index], index);
}
}
@@ -659,7 +712,7 @@ namespace Nuclex.Avalonia.Collections {
/// Tracks which pages have been fetched so far
private bool[] fetchedPages;
/// The wrapped list under its type-safe interface
- private TItem[] typedList;
+ private IList typedList;
/// The wrapped list under its object interface
private IList objectList;
#if DEBUG
diff --git a/Source/Collections/VirtualObservableReadOnlyList.cs b/Source/Collections/VirtualObservableReadOnlyList.cs
index f934531..ffadf55 100644
--- a/Source/Collections/VirtualObservableReadOnlyList.cs
+++ b/Source/Collections/VirtualObservableReadOnlyList.cs
@@ -23,7 +23,6 @@ using System.Collections.Generic;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
-
#if !NO_SPECIALIZED_COLLECTIONS
using System.Collections.Specialized;
#endif
@@ -597,19 +596,23 @@ namespace Nuclex.Avalonia.Collections {
this.assumedCount.HasValue,
"This method should only be called when item count is already known"
);
- if(!this.fetchedPages[pageIndex]) {
- int count = Math.Min(
- this.assumedCount!.Value - (this.pageSize * pageIndex),
- this.pageSize
- );
- int fetchedItemCount = FetchItems(this.typedList, pageIndex * this.pageSize, count);
- if(fetchedItemCount < this.pageSize) {
- this.assumedCount = pageIndex * this.pageSize + fetchedItemCount;
- }
-
- this.fetchedPages[pageIndex] = true;
+ // If the page is already fetched (or in flight), do nothing
+ if((pageIndex >= this.fetchedPages.Length) || this.fetchedPages[pageIndex]) {
+ return;
}
+
+ int count = Math.Min(
+ this.assumedCount!.Value - (this.pageSize * pageIndex),
+ this.pageSize
+ );
+
+ int fetchedItemCount = FetchItems(this.typedList, pageIndex * this.pageSize, count);
+ if(fetchedItemCount < this.pageSize) {
+ this.assumedCount = pageIndex * this.pageSize + fetchedItemCount;
+ }
+
+ this.fetchedPages[pageIndex] = true;
}
/// Number of items the collection believes it has