From a3b060fe755a580a0f25b002a8509145b6579855 Mon Sep 17 00:00:00 2001 From: Daniel Svensson Date: Sun, 5 Apr 2026 23:34:57 +0200 Subject: [PATCH 01/11] Add Icollection for improved avalonia support --- .../Framework/EntityCollection.cs | 59 ++++++++++++++++++- .../Framework/EntitySet.cs | 39 +++++++++++- 2 files changed, 96 insertions(+), 2 deletions(-) diff --git a/src/OpenRiaServices.Client/Framework/EntityCollection.cs b/src/OpenRiaServices.Client/Framework/EntityCollection.cs index d73850244..51257db8e 100644 --- a/src/OpenRiaServices.Client/Framework/EntityCollection.cs +++ b/src/OpenRiaServices.Client/Framework/EntityCollection.cs @@ -20,7 +20,7 @@ namespace OpenRiaServices.Client /// Represents a collection of associated Entities. /// /// The type of in the collection - public sealed class EntityCollection : IEntityCollection, IEntityCollection + public sealed class EntityCollection : IEntityCollection, IEntityCollection, IList #if HAS_COLLECTIONVIEW , ICollectionViewFactory #endif @@ -1021,6 +1021,7 @@ bool ICollection.IsReadOnly return IsSourceExternal; } } + void ICollection.CopyTo(TEntity[] array, int arrayIndex) { this.Load(); @@ -1046,6 +1047,62 @@ void ICollection.Clear() foreach (var item in this.Entities.ToList()) Remove(item); } + + #endregion + + #region IList + bool IList.IsFixedSize => false; + + bool IList.IsReadOnly => this.IsSourceExternal; + + bool ICollection.IsSynchronized => ((ICollection)_sourceSet).IsSynchronized; + + object ICollection.SyncRoot => ((ICollection)_sourceSet).SyncRoot; + + // TODO Load entities for all these operations + object IList.this[int index] { get => ((IList)Entities)[index]; set => Entities[index] = (TEntity)value; } + + int IList.Add(object value) + { + Add((TEntity)value); + return Entities.Count - 1; + } + + void IList.Clear() + { + throw new NotImplementedException(); + } + + bool IList.Contains(object value) + { + return value is TEntity entity && ((ICollection)this).Contains(entity); + } + + int IList.IndexOf(object value) + { + throw new NotImplementedException(); + } + + void IList.Insert(int index, object value) + { + throw new NotImplementedException(); + } + + void IList.Remove(object value) + { + Remove((TEntity)value); + } + + void IList.RemoveAt(int index) + { + throw new NotImplementedException(); + } + + void ICollection.CopyTo(Array array, int index) + { + this.Load(); + ((ICollection)Entities).CopyTo(array, index); + } #endregion } } diff --git a/src/OpenRiaServices.Client/Framework/EntitySet.cs b/src/OpenRiaServices.Client/Framework/EntitySet.cs index 5e04b18e2..07819e779 100644 --- a/src/OpenRiaServices.Client/Framework/EntitySet.cs +++ b/src/OpenRiaServices.Client/Framework/EntitySet.cs @@ -18,7 +18,7 @@ namespace OpenRiaServices.Client /// /// Represents a collection of instances. /// - public abstract class EntitySet : IEnumerable, ICollection, INotifyCollectionChanged, IRevertibleChangeTracking, INotifyPropertyChanged + public abstract class EntitySet : IEnumerable, ICollection, IList, INotifyCollectionChanged, IRevertibleChangeTracking, INotifyPropertyChanged { private readonly Dictionary> _associationUpdateCallbackMap = new(); private readonly Type _entityType; @@ -940,6 +940,43 @@ void ICollection.CopyTo(Array array, int index) } #endregion + #region IList + bool IList.IsFixedSize => _list.IsFixedSize; + + object IList.this[int index] { get => _list[index]; set => _list[index] = value; } + + int IList.Add(object value) + { + return _list.Add(value); + } + + bool IList.Contains(object value) + { + return _list.Contains(value); + } + + int IList.IndexOf(object value) + { + return _list.IndexOf(value); + } + + void IList.Insert(int index, object value) + { + _list.Insert(index, value); + } + + void IList.Remove(object value) + { + _list.Remove(value); + } + + void IList.RemoveAt(int index) + { + _list.RemoveAt(index); + } + + #endregion + #region INotifyCollectionChanged Members /// From 018bb7caa736abda83f3780242e8a69fbb8ec976 Mon Sep 17 00:00:00 2001 From: Daniel Svensson Date: Mon, 6 Apr 2026 09:25:46 +0200 Subject: [PATCH 02/11] Add IReadOnlyList --- .../Framework/EntityCollection.cs | 33 ++++++++++++++----- .../Framework/EntitySet.cs | 14 +++++--- 2 files changed, 35 insertions(+), 12 deletions(-) diff --git a/src/OpenRiaServices.Client/Framework/EntityCollection.cs b/src/OpenRiaServices.Client/Framework/EntityCollection.cs index 51257db8e..32270e503 100644 --- a/src/OpenRiaServices.Client/Framework/EntityCollection.cs +++ b/src/OpenRiaServices.Client/Framework/EntityCollection.cs @@ -20,7 +20,7 @@ namespace OpenRiaServices.Client /// Represents a collection of associated Entities. /// /// The type of in the collection - public sealed class EntityCollection : IEntityCollection, IEntityCollection, IList + public sealed class EntityCollection : IEntityCollection, IEntityCollection, IList, IReadOnlyList #if HAS_COLLECTIONVIEW , ICollectionViewFactory #endif @@ -1012,7 +1012,9 @@ void ICollectionChangedListener.OnCollectionChanged(object sender, NotifyCollect #endif #endregion - #region ICollection Members + #region ICollection, IReadOnlyList Members + TEntity IReadOnlyList.this[int index] => Entities[index]; + bool ICollection.IsReadOnly { get @@ -1027,17 +1029,20 @@ void ICollection.CopyTo(TEntity[] array, int arrayIndex) this.Load(); this.Entities.CopyTo(array, arrayIndex); } + bool ICollection.Contains(TEntity item) { this.Load(); return this.EntitiesHashSet.Contains(item); } + bool ICollection.Remove(TEntity item) { bool removed = this.EntitiesHashSet.Contains(item); Remove(item); return removed; } + /// /// Removes all items. /// @@ -1050,7 +1055,7 @@ void ICollection.Clear() #endregion - #region IList + #region IList, ICollection bool IList.IsFixedSize => false; bool IList.IsReadOnly => this.IsSourceExternal; @@ -1059,8 +1064,15 @@ void ICollection.Clear() object ICollection.SyncRoot => ((ICollection)_sourceSet).SyncRoot; - // TODO Load entities for all these operations - object IList.this[int index] { get => ((IList)Entities)[index]; set => Entities[index] = (TEntity)value; } + object IList.this[int index] + { + get + { + Load(); + return Entities[index]; + } + set => throw new NotImplementedException(); + } int IList.Add(object value) { @@ -1070,7 +1082,7 @@ int IList.Add(object value) void IList.Clear() { - throw new NotImplementedException(); + ((ICollection)this).Clear(); } bool IList.Contains(object value) @@ -1080,7 +1092,11 @@ bool IList.Contains(object value) int IList.IndexOf(object value) { - throw new NotImplementedException(); + if (value is not TEntity entity) + return -1; + + Load(); + return Entities.IndexOf(entity); } void IList.Insert(int index, object value) @@ -1095,7 +1111,8 @@ void IList.Remove(object value) void IList.RemoveAt(int index) { - throw new NotImplementedException(); + TEntity entity = Entities[index]; + Remove(entity); } void ICollection.CopyTo(Array array, int index) diff --git a/src/OpenRiaServices.Client/Framework/EntitySet.cs b/src/OpenRiaServices.Client/Framework/EntitySet.cs index 07819e779..934856e90 100644 --- a/src/OpenRiaServices.Client/Framework/EntitySet.cs +++ b/src/OpenRiaServices.Client/Framework/EntitySet.cs @@ -1250,12 +1250,14 @@ protected override void VisitEntityRef(IEntityRef entityRef, Entity parent, Meta /// Represents a collection of instances, providing change tracking and other services. /// /// The type of this set will contain - public sealed class EntitySet : EntitySet, IEntityCollection + public sealed class EntitySet : EntitySet, IEntityCollection, IReadOnlyList #if HAS_COLLECTIONVIEW , ICollectionViewFactory #endif where TEntity : Entity { + private new List List => (List)List; + /// /// Initializes a new instance of the EntitySet class /// @@ -1283,7 +1285,7 @@ protected override Entity CreateEntity() { throw new InvalidOperationException(string.Format(CultureInfo.CurrentCulture, Resources.Cannot_Create_Abstract_Entity, typeof(TEntity))); } - TEntity entity = (TEntity)Activator.CreateInstance(typeof(TEntity)); + TEntity entity = Activator.CreateInstance(); return entity; } @@ -1303,7 +1305,7 @@ protected override Entity CreateEntity() /// The enumerator public new IEnumerator GetEnumerator() { - return ((IList)List).GetEnumerator(); + return List.GetEnumerator(); } /// @@ -1387,7 +1389,7 @@ IEnumerator IEnumerable.GetEnumerator() #region ICollection Members void ICollection.CopyTo(TEntity[] array, int arrayIndex) { - ((IList)List).CopyTo(array, arrayIndex); + List.CopyTo(array, arrayIndex); } bool ICollection.Contains(TEntity item) @@ -1414,6 +1416,10 @@ bool ICollection.Remove(TEntity item) } #endregion + #region IReadOnlyList Members + TEntity IReadOnlyList.this[int index] => List[index]; + #endregion + #region ICollectionViewFactory #if HAS_COLLECTIONVIEW /// From 173d35d35b80d0f554f42e9e6b778379b4ba7988 Mon Sep 17 00:00:00 2001 From: Daniel Svensson Date: Mon, 6 Apr 2026 09:43:35 +0200 Subject: [PATCH 03/11] fix IList implementation --- src/OpenRiaServices.Client/Framework/EntitySet.cs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/OpenRiaServices.Client/Framework/EntitySet.cs b/src/OpenRiaServices.Client/Framework/EntitySet.cs index 934856e90..a97985fd1 100644 --- a/src/OpenRiaServices.Client/Framework/EntitySet.cs +++ b/src/OpenRiaServices.Client/Framework/EntitySet.cs @@ -947,12 +947,15 @@ void ICollection.CopyTo(Array array, int index) int IList.Add(object value) { - return _list.Add(value); + int countBefore = Count; + Add((Entity)value); + + return (Count == countBefore + 1) ? countBefore : -1; } bool IList.Contains(object value) { - return _list.Contains(value); + return value is Entity e && Contains(e); } int IList.IndexOf(object value) @@ -962,17 +965,17 @@ int IList.IndexOf(object value) void IList.Insert(int index, object value) { - _list.Insert(index, value); + throw new NotImplementedException(); } void IList.Remove(object value) { - _list.Remove(value); + Remove((Entity)value); } void IList.RemoveAt(int index) { - _list.RemoveAt(index); + throw new NotImplementedException(); } #endregion From 6adf0b0ba44a048c966ed6c52c089a855632d20e Mon Sep 17 00:00:00 2001 From: Daniel Svensson Date: Mon, 6 Apr 2026 13:55:21 +0200 Subject: [PATCH 04/11] * Improve logic around entity removal from EntityCollection ( --- .../Framework/EntityCollection.cs | 93 ++++++++++--------- .../Framework/EntitySet.cs | 26 ++++-- 2 files changed, 68 insertions(+), 51 deletions(-) diff --git a/src/OpenRiaServices.Client/Framework/EntityCollection.cs b/src/OpenRiaServices.Client/Framework/EntityCollection.cs index 32270e503..62256c5e3 100644 --- a/src/OpenRiaServices.Client/Framework/EntityCollection.cs +++ b/src/OpenRiaServices.Client/Framework/EntityCollection.cs @@ -311,7 +311,7 @@ public void Remove(TEntity entity) if (idx != -1) { - if (this.RemoveEntity(entity)) + if (this.RemoveEntity(entity, idx)) { // If the entity was removed, raise a collection changed notification. Note that the Detach call above might // have caused a dynamic removal behind the scenes resulting in the entity no longer being in the collection, @@ -368,14 +368,32 @@ private bool TryAddEntity(TEntity entity) } } - private bool RemoveEntity(TEntity entity) + private bool RemoveEntity(TEntity entity, int index) { if (this.EntitiesHashSet.Remove(entity)) { - bool isRemoved = this.Entities.Remove(entity); - Debug.Assert(isRemoved, "The entity should be present in both Entities and EntitiesHashSet"); + Debug.Assert(object.ReferenceEquals(entity, Entities[index])); + this.Entities.RemoveAt(index); return true; } + Debug.Fail("Expected item to be part of Set") + return false; + } + + /// + /// Remove the entity if part of the collection and returns it's index through .(-1 if no removal) + /// + /// entity to remove + /// the index of the entity before removal, or -1 if not removed + private bool TryRemoveEntity(TEntity entity, out int index) + { + if (this.EntitiesHashSet.Remove(entity)) + { + index = this.Entities.IndexOf(entity); + this.Entities.RemoveAt(index); + return true; + } + index = -1; return false; } @@ -624,13 +642,11 @@ private void OnEntityAssociationUpdated(Entity entity) Debug.Assert(added); this.RaiseCollectionChangedNotification(NotifyCollectionChangedAction.Add, typedEntity, this.Entities.Count - 1); } - else if (containsEntity && !this._entityPredicate(typedEntity)) + // The entity is in our set but is no longer a match, so we need to remove it. + // Here we use the predicate directly, since even if the entity is New if it + // no longer matches it should be removed. + else if (!this._entityPredicate(typedEntity) && this.TryRemoveEntity(typedEntity, out int idx)) { - // The entity is in our set but is no longer a match, so we need to remove it. - // Here we use the predicate directly, since even if the entity is New if it - // no longer matches it should be removed. - int idx = this.Entities.IndexOf(typedEntity); - this.RemoveEntity(typedEntity); this.RaiseCollectionChangedNotification(NotifyCollectionChangedAction.Remove, typedEntity, idx); } } @@ -648,14 +664,13 @@ private void SourceSet_CollectionChanged(object sender, NotifyCollectionChangedE if (this._parent.EntityState != EntityState.New && args.Action == NotifyCollectionChangedAction.Add) { - TEntity[] newEntities = args.NewItems.OfType().Where(this.Filter).ToArray(); - if (newEntities.Length > 0) + List newEntities = args.NewItems.OfType().Where(this.Filter).ToList(); + if (newEntities.Count > 0) { - int newStartingIdx = -1; + int newStartingIdx = this.Entities.Count; List affectedEntities = new List(); foreach (TEntity newEntity in newEntities) { - newStartingIdx = this.Entities.Count; if (this.TryAddEntity(newEntity)) { affectedEntities.Add(newEntity); @@ -664,34 +679,21 @@ private void SourceSet_CollectionChanged(object sender, NotifyCollectionChangedE if (affectedEntities.Count > 0) { -#if SILVERLIGHT - // SL doesn't support the constructor taking a list of objects - this.RaiseCollectionChangedNotification(args.Action, (TEntity)affectedEntities.Single(), newStartingIdx); -#else this.RaiseCollectionChangedNotification(args.Action, affectedEntities, newStartingIdx); -#endif } } } else if (args.Action == NotifyCollectionChangedAction.Remove) { // if the entity is in our cached collection, remove it - TEntity[] entitiesToRemove = args.OldItems.OfType().Where(p => this.EntitiesHashSet.Contains(p)).ToArray(); - if (entitiesToRemove.Length > 0) + foreach (TEntity entityToRemove in args.OldItems.OfType()) { - int oldStartingIdx = this.Entities.IndexOf(entitiesToRemove[0]); - foreach (TEntity removedEntity in entitiesToRemove) + // If entity was part of the collection and removed, raise an event + if (this.TryRemoveEntity(entityToRemove, out int idx)) { - this.RemoveEntity(removedEntity); + // Should we do a single reset event if multiple entitites are removed ?? + this.RaiseCollectionChangedNotification(args.Action, entityToRemove, idx); } - -#if SILVERLIGHT - //// REVIEW: Should we instead send out a reset event? - // SL doesn't support the constructor taking a list of objects - this.RaiseCollectionChangedNotification(args.Action, entitiesToRemove.Single(), oldStartingIdx); -#else - this.RaiseCollectionChangedNotification(args.Action, entitiesToRemove, oldStartingIdx); -#endif } } else if (args.Action == NotifyCollectionChangedAction.Reset) @@ -1038,9 +1040,13 @@ bool ICollection.Contains(TEntity item) bool ICollection.Remove(TEntity item) { - bool removed = this.EntitiesHashSet.Contains(item); - Remove(item); - return removed; + this.Load(); + if (this.EntitiesHashSet.Contains(item)) + { + Remove(item); + return true; + } + return false; } /// @@ -1060,24 +1066,26 @@ void ICollection.Clear() bool IList.IsReadOnly => this.IsSourceExternal; - bool ICollection.IsSynchronized => ((ICollection)_sourceSet).IsSynchronized; + bool ICollection.IsSynchronized => false; - object ICollection.SyncRoot => ((ICollection)_sourceSet).SyncRoot; + object ICollection.SyncRoot => ((ICollection)Entities).SyncRoot; object IList.this[int index] { get { - Load(); + // We skip Load, since caller is expected to have called .Count which triggers load return Entities[index]; } - set => throw new NotImplementedException(); + set => throw new NotSupportedException(string.Format(CultureInfo.InvariantCulture, Resource.IsNotSupported, "Index setter")); } int IList.Add(object value) { + int countBefore = this.Count; Add((TEntity)value); - return Entities.Count - 1; + + return (this.Count == countBefore + 1) ? countBefore : -1; } void IList.Clear() @@ -1101,7 +1109,7 @@ int IList.IndexOf(object value) void IList.Insert(int index, object value) { - throw new NotImplementedException(); + throw new NotSupportedException(string.Format(CultureInfo.InvariantCulture, Resource.IsNotSupported, "Insert")); } void IList.Remove(object value) @@ -1111,8 +1119,7 @@ void IList.Remove(object value) void IList.RemoveAt(int index) { - TEntity entity = Entities[index]; - Remove(entity); + throw new NotSupportedException(string.Format(CultureInfo.InvariantCulture, Resource.IsNotSupported, "RemoveAt")); } void ICollection.CopyTo(Array array, int index) diff --git a/src/OpenRiaServices.Client/Framework/EntitySet.cs b/src/OpenRiaServices.Client/Framework/EntitySet.cs index a97985fd1..671a943ac 100644 --- a/src/OpenRiaServices.Client/Framework/EntitySet.cs +++ b/src/OpenRiaServices.Client/Framework/EntitySet.cs @@ -8,6 +8,8 @@ using System.Globalization; using System.Linq; using OpenRiaServices.Client.Internal; +using System.Diagnostics.Contracts; + #if HAS_COLLECTIONVIEW using System.Windows.Data; @@ -481,7 +483,10 @@ public void Remove(Entity entity) this.OnCollectionChanged(NotifyCollectionChangedAction.Remove, entity, idx); } - internal bool Contains(Entity entity) + /// Determines whether the contains the specified entity. + /// The element to locate in the object. + /// true if the object contains the specified element; otherwise, false. + public bool Contains(Entity entity) { return this._set.Contains(entity); } @@ -941,9 +946,13 @@ void ICollection.CopyTo(Array array, int index) #endregion #region IList - bool IList.IsFixedSize => _list.IsFixedSize; + bool IList.IsFixedSize => false; - object IList.this[int index] { get => _list[index]; set => _list[index] = value; } + object IList.this[int index] + { + get => _list[index]; + set => throw new NotSupportedException(string.Format(CultureInfo.InvariantCulture, Resource.IsNotSupported, "Index setter")); + } int IList.Add(object value) { @@ -965,7 +974,7 @@ int IList.IndexOf(object value) void IList.Insert(int index, object value) { - throw new NotImplementedException(); + throw new NotSupportedException(string.Format(CultureInfo.InvariantCulture, Resource.IsNotSupported, "Insert")); } void IList.Remove(object value) @@ -975,7 +984,7 @@ void IList.Remove(object value) void IList.RemoveAt(int index) { - throw new NotImplementedException(); + throw new NotSupportedException(string.Format(CultureInfo.InvariantCulture, Resource.IsNotSupported, "RemoteAt")); } #endregion @@ -1395,7 +1404,8 @@ void ICollection.CopyTo(TEntity[] array, int arrayIndex) List.CopyTo(array, arrayIndex); } - bool ICollection.Contains(TEntity item) + /// + public bool Contains(TEntity item) { return base.Contains(item); } @@ -1489,7 +1499,7 @@ public bool Contains(object value) public int IndexOf(object value) { - return this.Source.List.IndexOf(value); + return ((IList)this.Source.List).IndexOf(value); } public void Insert(int index, object value) @@ -1536,7 +1546,7 @@ public object this[int index] public void CopyTo(Array array, int index) { - this.Source.List.CopyTo(array, index); + ((IList)this.Source.List).CopyTo(array, index); } public int Count From a08aa4615858454cd81ff876763796c519268ab7 Mon Sep 17 00:00:00 2001 From: Daniel Svensson Date: Mon, 6 Apr 2026 14:00:44 +0200 Subject: [PATCH 05/11] remove redundant cast --- src/OpenRiaServices.Client/Framework/EntityCollection.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/OpenRiaServices.Client/Framework/EntityCollection.cs b/src/OpenRiaServices.Client/Framework/EntityCollection.cs index 62256c5e3..c2d65a5a8 100644 --- a/src/OpenRiaServices.Client/Framework/EntityCollection.cs +++ b/src/OpenRiaServices.Client/Framework/EntityCollection.cs @@ -376,7 +376,7 @@ private bool RemoveEntity(TEntity entity, int index) this.Entities.RemoveAt(index); return true; } - Debug.Fail("Expected item to be part of Set") + Debug.Fail("Expected item to be part of Set"); return false; } @@ -647,6 +647,7 @@ private void OnEntityAssociationUpdated(Entity entity) // no longer matches it should be removed. else if (!this._entityPredicate(typedEntity) && this.TryRemoveEntity(typedEntity, out int idx)) { + Debug.Assert(idx >= 0); this.RaiseCollectionChangedNotification(NotifyCollectionChangedAction.Remove, typedEntity, idx); } } @@ -792,7 +793,7 @@ IEnumerable IEntityCollection.Entities { get { - return this.Cast(); + return this; } } From c089bf723d6770492c649810fd6fb005af83307f Mon Sep 17 00:00:00 2001 From: Daniel Svensson Date: Mon, 6 Apr 2026 15:41:26 +0200 Subject: [PATCH 06/11] Apply suggestions from code review Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- src/OpenRiaServices.Client/Framework/EntitySet.cs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/OpenRiaServices.Client/Framework/EntitySet.cs b/src/OpenRiaServices.Client/Framework/EntitySet.cs index 671a943ac..6d51a2356 100644 --- a/src/OpenRiaServices.Client/Framework/EntitySet.cs +++ b/src/OpenRiaServices.Client/Framework/EntitySet.cs @@ -8,8 +8,6 @@ using System.Globalization; using System.Linq; using OpenRiaServices.Client.Internal; -using System.Diagnostics.Contracts; - #if HAS_COLLECTIONVIEW using System.Windows.Data; @@ -984,7 +982,7 @@ void IList.Remove(object value) void IList.RemoveAt(int index) { - throw new NotSupportedException(string.Format(CultureInfo.InvariantCulture, Resource.IsNotSupported, "RemoteAt")); + throw new NotSupportedException(string.Format(CultureInfo.InvariantCulture, Resource.IsNotSupported, "RemoveAt")); } #endregion @@ -1268,8 +1266,7 @@ public sealed class EntitySet : EntitySet, IEntityCollection, #endif where TEntity : Entity { - private new List List => (List)List; - + private new List List => (List)base.List; /// /// Initializes a new instance of the EntitySet class /// From c6602fd07c4cfc4bd31ae9c1da1f10eb6c01280b Mon Sep 17 00:00:00 2001 From: Daniel Svensson Date: Mon, 6 Apr 2026 16:39:26 +0200 Subject: [PATCH 07/11] IReadOnlyList for LoadResult --- src/OpenRiaServices.Client/Framework/LoadResult.cs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/OpenRiaServices.Client/Framework/LoadResult.cs b/src/OpenRiaServices.Client/Framework/LoadResult.cs index 6fa67e441..e789f2898 100644 --- a/src/OpenRiaServices.Client/Framework/LoadResult.cs +++ b/src/OpenRiaServices.Client/Framework/LoadResult.cs @@ -1,7 +1,6 @@ using System; using System.Collections; using System.Collections.Generic; -using System.Collections.ObjectModel; using System.Diagnostics.CodeAnalysis; using System.Linq; @@ -11,9 +10,9 @@ namespace OpenRiaServices.Client /// The result of a sucessfully completed load operation /// /// The type of the entity loaded. - public class LoadResult : IReadOnlyCollection, ICollection, ILoadResult where TEntity : Entity + public class LoadResult : IReadOnlyList, ICollection, ILoadResult where TEntity : Entity { - private readonly ReadOnlyCollection _loadedEntites; + private readonly Data.ReadOnlyObservableLoaderCollection _loadedEntites; /// /// Initializes a new instance of the class. @@ -79,6 +78,9 @@ public LoadResult(EntityQuery query, LoadBehavior loadBehavior, IEnumer /// The number top level Entities loaded. public int Count { get { return _loadedEntites.Count; } } + /// + public TEntity this[int index] => _loadedEntites[index]; + #region ICollection, IEnumerator implementations /// /// Copies Entities to an array (implements ICollection.CopyTo) From 1cae005a8f200fff74c8d68b000b20e2ac0ea359 Mon Sep 17 00:00:00 2001 From: Daniel Svensson Date: Mon, 6 Apr 2026 22:51:22 +0200 Subject: [PATCH 08/11] fix review comments do some cleanup on ListCollectionViewProxy --- .../Framework/EntitySet.cs | 52 +++++-------------- 1 file changed, 14 insertions(+), 38 deletions(-) diff --git a/src/OpenRiaServices.Client/Framework/EntitySet.cs b/src/OpenRiaServices.Client/Framework/EntitySet.cs index 6d51a2356..0634c8473 100644 --- a/src/OpenRiaServices.Client/Framework/EntitySet.cs +++ b/src/OpenRiaServices.Client/Framework/EntitySet.cs @@ -946,6 +946,8 @@ void ICollection.CopyTo(Array array, int index) #region IList bool IList.IsFixedSize => false; + bool IList.IsReadOnly => (_supportedOperations & (EntitySetOperations.Remove | EntitySetOperations.Add)) == 0; + object IList.this[int index] { get => _list[index]; @@ -957,7 +959,12 @@ int IList.Add(object value) int countBefore = Count; Add((Entity)value); - return (Count == countBefore + 1) ? countBefore : -1; + if (Count == countBefore + 1) + return countBefore; + else if (Count == countBefore) + return -1; + else + return List.IndexOf(value); } bool IList.Contains(object value) @@ -1388,13 +1395,6 @@ protected override void OnCollectionChanged(NotifyCollectionChangedAction action base.OnCollectionChanged(action, affectedObject, index); } - #region IEnumerable Members - IEnumerator IEnumerable.GetEnumerator() - { - return this.GetEnumerator(); - } - #endregion - #region ICollection Members void ICollection.CopyTo(TEntity[] array, int arrayIndex) { @@ -1402,7 +1402,7 @@ void ICollection.CopyTo(TEntity[] array, int arrayIndex) } /// - public bool Contains(TEntity item) + public bool /*ICollection.*/Contains(TEntity item) { return base.Contains(item); } @@ -1467,22 +1467,7 @@ internal ListCollectionViewProxy(EntitySet source) #region IList public int Add(object value) - { - T entity = value as T; - if (entity == null) - { - throw new ArgumentException( - string.Format(CultureInfo.CurrentCulture, Resource.MustBeAnEntity, "value"), - nameof(value)); - } - - int countBefore = this.Source.Count; - this.Source.Add(entity); - - return this.Source.Count == countBefore + 1 - ? countBefore - : ((List)this.Source.List).IndexOf(entity, countBefore); - } + => ((IList)Source).Add(value); public void Clear() { @@ -1500,20 +1485,11 @@ public int IndexOf(object value) } public void Insert(int index, object value) - { - throw new NotSupportedException( - string.Format(CultureInfo.CurrentCulture, Resource.IsNotSupported, "Insert")); - } + => ((IList)Source).Insert(index, value); - public bool IsFixedSize - { - get { return !(this.Source.CanAdd || this.Source.CanRemove); } - } + public bool IsFixedSize => ((IList)Source).IsFixedSize; - public bool IsReadOnly - { - get { return !(this.Source.CanAdd || this.Source.CanRemove); } - } + public bool IsReadOnly => ((IList)Source).IsReadOnly; public void Remove(object value) { @@ -1525,7 +1501,7 @@ public void Remove(object value) public void RemoveAt(int index) { - this.Remove(this[index]); + Source.Remove(Source.List[index]); } public object this[int index] From dcc1b20931dfae434c4bee5a28e63974eacd28fc Mon Sep 17 00:00:00 2001 From: Daniel Svensson Date: Mon, 6 Apr 2026 23:01:38 +0200 Subject: [PATCH 09/11] make indexer public --- src/OpenRiaServices.Client/Framework/EntityCollection.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenRiaServices.Client/Framework/EntityCollection.cs b/src/OpenRiaServices.Client/Framework/EntityCollection.cs index c2d65a5a8..93d1c01f9 100644 --- a/src/OpenRiaServices.Client/Framework/EntityCollection.cs +++ b/src/OpenRiaServices.Client/Framework/EntityCollection.cs @@ -1016,7 +1016,7 @@ void ICollectionChangedListener.OnCollectionChanged(object sender, NotifyCollect #endregion #region ICollection, IReadOnlyList Members - TEntity IReadOnlyList.this[int index] => Entities[index]; + public TEntity this[int index] => Entities[index]; bool ICollection.IsReadOnly { From 91aa92336cffd13b7f1b437b782d9da77a3d3f19 Mon Sep 17 00:00:00 2001 From: Daniel Svensson Date: Tue, 7 Apr 2026 13:19:05 +0200 Subject: [PATCH 10/11] Update IList behaviour and add comments --- .../Framework/EntityCollection.cs | 27 ++++++++++++------- .../Framework/EntitySet.cs | 10 ++++++- .../Framework/LoadResult.cs | 2 +- 3 files changed, 27 insertions(+), 12 deletions(-) diff --git a/src/OpenRiaServices.Client/Framework/EntityCollection.cs b/src/OpenRiaServices.Client/Framework/EntityCollection.cs index 93d1c01f9..9c7f47989 100644 --- a/src/OpenRiaServices.Client/Framework/EntityCollection.cs +++ b/src/OpenRiaServices.Client/Framework/EntityCollection.cs @@ -638,16 +638,14 @@ private void OnEntityAssociationUpdated(Entity entity) { // Add matching entity to our set. When adding, we use the stronger Filter to // filter out New entities - bool added = this.TryAddEntity(typedEntity); - Debug.Assert(added); - this.RaiseCollectionChangedNotification(NotifyCollectionChangedAction.Add, typedEntity, this.Entities.Count - 1); + if (this.TryAddEntity(typedEntity)) + this.RaiseCollectionChangedNotification(NotifyCollectionChangedAction.Add, typedEntity, this.Entities.Count - 1); } // The entity is in our set but is no longer a match, so we need to remove it. // Here we use the predicate directly, since even if the entity is New if it // no longer matches it should be removed. else if (!this._entityPredicate(typedEntity) && this.TryRemoveEntity(typedEntity, out int idx)) { - Debug.Assert(idx >= 0); this.RaiseCollectionChangedNotification(NotifyCollectionChangedAction.Remove, typedEntity, idx); } } @@ -1016,6 +1014,13 @@ void ICollectionChangedListener.OnCollectionChanged(object sender, NotifyCollect #endregion #region ICollection, IReadOnlyList Members + + /// + /// Gets the entity at the specified index in the collection. + /// + /// **Important**: Make sure to check first to ensure the collection is initialized + /// The zero-based index of the entity to retrieve. + /// The entity located at the specified index. public TEntity this[int index] => Entities[index]; bool ICollection.IsReadOnly @@ -1071,13 +1076,10 @@ void ICollection.Clear() object ICollection.SyncRoot => ((ICollection)Entities).SyncRoot; + /// object IList.this[int index] { - get - { - // We skip Load, since caller is expected to have called .Count which triggers load - return Entities[index]; - } + get => this[index]; set => throw new NotSupportedException(string.Format(CultureInfo.InvariantCulture, Resource.IsNotSupported, "Index setter")); } @@ -1086,7 +1088,12 @@ int IList.Add(object value) int countBefore = this.Count; Add((TEntity)value); - return (this.Count == countBefore + 1) ? countBefore : -1; + if (this.Count == countBefore + 1) + return countBefore; + else if (this.Count == countBefore) + return -1; + else + return Entities.IndexOf((TEntity)value, countBefore); } void IList.Clear() diff --git a/src/OpenRiaServices.Client/Framework/EntitySet.cs b/src/OpenRiaServices.Client/Framework/EntitySet.cs index 0634c8473..24ceb92a3 100644 --- a/src/OpenRiaServices.Client/Framework/EntitySet.cs +++ b/src/OpenRiaServices.Client/Framework/EntitySet.cs @@ -984,7 +984,15 @@ void IList.Insert(int index, object value) void IList.Remove(object value) { - Remove((Entity)value); + try + { + if (value is Entity entity) + Remove(entity); + } + catch (InvalidOperationException ioe) when (ioe.Message == Resource.EntitySet_EntityNotInSet) + { + // Don't throw if item was not in the collection + } } void IList.RemoveAt(int index) diff --git a/src/OpenRiaServices.Client/Framework/LoadResult.cs b/src/OpenRiaServices.Client/Framework/LoadResult.cs index e789f2898..da77e60a8 100644 --- a/src/OpenRiaServices.Client/Framework/LoadResult.cs +++ b/src/OpenRiaServices.Client/Framework/LoadResult.cs @@ -76,7 +76,7 @@ public LoadResult(EntityQuery query, LoadBehavior loadBehavior, IEnumer /// Gets the number of top level Entities loaded /// /// The number top level Entities loaded. - public int Count { get { return _loadedEntites.Count; } } + public int Count => _loadedEntites.Count; /// public TEntity this[int index] => _loadedEntites[index]; From 4e07d0683b0e1c097c164704e66e33d8008d1ec1 Mon Sep 17 00:00:00 2001 From: Daniel Svensson Date: Tue, 7 Apr 2026 18:23:10 +0200 Subject: [PATCH 11/11] Fix IsFixedSize implementation to mirror ReadOnly, it is used to check if items are allowed to be added or removed by that instance (not if somebody else can change the list) --- src/OpenRiaServices.Client/Framework/EntityCollection.cs | 2 +- src/OpenRiaServices.Client/Framework/EntitySet.cs | 2 +- src/OpenRiaServices.Client/Framework/LoadResult.cs | 6 +++++- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/OpenRiaServices.Client/Framework/EntityCollection.cs b/src/OpenRiaServices.Client/Framework/EntityCollection.cs index 9c7f47989..2ff61cb78 100644 --- a/src/OpenRiaServices.Client/Framework/EntityCollection.cs +++ b/src/OpenRiaServices.Client/Framework/EntityCollection.cs @@ -1068,7 +1068,7 @@ void ICollection.Clear() #endregion #region IList, ICollection - bool IList.IsFixedSize => false; + bool IList.IsFixedSize => this.IsSourceExternal; bool IList.IsReadOnly => this.IsSourceExternal; diff --git a/src/OpenRiaServices.Client/Framework/EntitySet.cs b/src/OpenRiaServices.Client/Framework/EntitySet.cs index 24ceb92a3..0390cc9df 100644 --- a/src/OpenRiaServices.Client/Framework/EntitySet.cs +++ b/src/OpenRiaServices.Client/Framework/EntitySet.cs @@ -944,7 +944,7 @@ void ICollection.CopyTo(Array array, int index) #endregion #region IList - bool IList.IsFixedSize => false; + bool IList.IsFixedSize => (_supportedOperations & (EntitySetOperations.Remove | EntitySetOperations.Add)) == 0; bool IList.IsReadOnly => (_supportedOperations & (EntitySetOperations.Remove | EntitySetOperations.Add)) == 0; diff --git a/src/OpenRiaServices.Client/Framework/LoadResult.cs b/src/OpenRiaServices.Client/Framework/LoadResult.cs index da77e60a8..5857454c5 100644 --- a/src/OpenRiaServices.Client/Framework/LoadResult.cs +++ b/src/OpenRiaServices.Client/Framework/LoadResult.cs @@ -78,7 +78,11 @@ public LoadResult(EntityQuery query, LoadBehavior loadBehavior, IEnumer /// The number top level Entities loaded. public int Count => _loadedEntites.Count; - /// + /// + /// Gets the entity at the specified zero-based index. + /// + /// The zero-based index of the entity to retrieve. + /// The entity located at the specified index. public TEntity this[int index] => _loadedEntites[index]; #region ICollection, IEnumerator implementations