mrpmorris / Fluxor

Fluxor is a zero boilerplate Flux/Redux library for Microsoft .NET and Blazor.
MIT License
1.22k stars 139 forks source link

Req: Entity Adapter #403

Closed Metalvast closed 8 months ago

Metalvast commented 1 year ago

Hello. Is there a library that extends Fluxor by adding EntityAdapter functionality, as in Angular? Are there any plans for this?

mrpmorris commented 1 year ago

It keeps every entity you've ever loaded in memory doesn't it? Isn't that a memory leak?

Metalvast commented 1 year ago

Yes, it is, but the NgRx/Entity Api allows you to clean the collection of these objects, if necessary. I'm asking this because I want to try using FSD (Feature Sliced Design) + Fluxor for my project.

mrpmorris commented 1 year ago

How does it know which items can be removed?

Metalvast commented 1 year ago

If I understood correctly, the developer decides when he needs to call the cleanup. You can create a reducer for some action and call methods for deleting objects there.

I started trying to implement similar functionality and I think it would look something like this:

        // Abstract adapter class

        public void Remove(Predicate<TEntity> predicate, EntityState<TEntity> state)
            => state.Entities.RemoveAll(predicate);

        public void RemoveAll(EntityState<TEntity> state)
            => state.Entities.Clear();

        // Reducers

        [ReducerMethod]
        public static ProjectState ReduceClearProjectsByIdsAction(ProjectState state, ClearProjectsByIdsAction action)
        {
            Adapters.Get<Project>().Remove(x => action.Ids.Contains(x.Id), state);  
        }

        [ReducerMethod]
        public static ProjectState ReduceClearProjectAction(ProjectState state, ClearProjectAction action)
        {
            Adapters.Get<Project>().RemoveAll(state);
        }

This is a draft version of the code.

mrpmorris commented 1 year ago

How can the developer know which entities can be removed?

Metalvast commented 1 year ago

I didn't quite understand your question. Do you mean, how a developer should determine a moment in the application lifecycle to clean up resources? If so, it depends more on the business logic.

mrpmorris commented 1 year ago

Scenario Dashboard screen: Widget 1: Top 10 customers of all time Widget 2: Top 20 customers over past 30 days

Each of those widgets have their own state.

The server notifies the browser that one or both of those has changed due to a large order coming in from a customer.

If each widget owns 100% of its own state (including custom names) then it can clear out its state completely and replace it.

If each widget just stores Customer Ids and looks up the Customer.Name in a shared piece of state (entity collection) then how can we know which entities are still needed throughout the app?

Let's say that All Time top 10 are the same as the top 10 for the past 30 days. Id's A,B,C,D,E,F,G,H,I,J. There are two scenarios. Scenario 1: The new large order brings the customer into the top 10 for the past 30 days. All time = A,B,C,D,E,F,G,H,I,J 30 days = A,B,C,D,E,F,G,H,I,Z

Scenario 2: The new large order also brings the customer into the All time top 10. All time = A,B,C,D,E,F,G,H,I,Z 30 days = A,B,C,D,E,F,G,H,I,Z

In the first scenario, the customer info for Customer "J" is still needed because it's still in the All time top 10. In the second scenario customer "J" is no longer needed so could be discarded.

If you have a global dictionary of Customers, how do you know which ones you need to keep and which can be removed?

Metalvast commented 1 year ago

I understand your question, but I don't have a clear answer yet, so I'm going to look for information and try to collect some examples when I have spare time.

mrpmorris commented 1 year ago

https://mobile.twitter.com/yannick_baron/status/1626192603317403648

Arsync commented 1 year ago

If you have a global dictionary of Customers, how do you know which ones you need to keep and which can be removed?

Widget initially knows, how to query for data to sort it. So when server notifies for change, widget must do initial query again.

In other case, if widget is subscribed for changes within specific range (all time, 30 days), server can push not just one item, but a new list of top 10 ids. We can lookup trough existing items and decide to load missing data / remove unrelated.

I did similar thing in https://github.com/mrpmorris/Fluxor/discussions/421 with entity collection as tab items (which user can close). Active entity id is a parameter of route and other entities is just background tabs. When we click 'close' cross on tab, entitiy will be removed from collection. When we click another tab, active item is switching in Fluxor state.

mrpmorris commented 1 year ago

What I mean is, when you have X (unknown) number of widgets consuming a piece of state, how can you ever be sure it is okay to remove that state from memory?

Arsync commented 1 year ago

I've created the sample project (looks crazy a little), but in concept: why not to enumerate all widgets by their state models? And when any of them changed, just send action like "I have unused items, anyone use it? No?"

Sample project: https://github.com/Arsync/FluxorWidgetsMonospace All bad things happens in Pages/Index.razor.cs file.

VS 17.6.0, NET 7

image

Something like

public class WidgetPopulateEffect : Effect<WidgetPopulateAction>
{
    private readonly IState<WidgetState> _widgetState;

    public WidgetPopulateEffect(IState<WidgetState> widgetState)
    {
        _widgetState = widgetState;
    }

    public override Task HandleAsync(WidgetPopulateAction action, IDispatcher dispatcher)
    {
        var targetWidget = _widgetState.Value.Widgets.FirstOrDefault(x => x.Id == action.WidgetId);

        if (targetWidget == null)
            return Task.CompletedTask;

        var itemsToRemove = targetWidget.EntityIds.Except(action.EntityIds).ToList();

        dispatcher.Dispatch(new WidgetPopulatingCompleteAction(action.WidgetId, action.EntityIds));

        var unusedItems = GetUnusedEntities(itemsToRemove).ToArray();

        dispatcher.Dispatch(new DataSpaceRemoveUnusedAction(unusedItems));

        return Task.CompletedTask;
    }

    private IEnumerable<int> GetUnusedEntities(IEnumerable<int> itemsToCheck)
    {
        return itemsToCheck.Where(item =>
            _widgetState.Value.Widgets.All(x => !x.EntityIds.Contains(item))
        );
    }
}
Arsync commented 1 year ago

We can create widget component with registration in widgets service in OnInitialized method and unregistration in Dispose, when component is destroyed.

Somewhere in a widget component:

protected override void OnInitialized()
{
    WidgetsService.Register(this);
}

public void Dispose()
{
     WidgetsService.Unregister(this);
}

So widgets enumerator can decide to resize active widgets list to query, where 'dataspace' entities can be used. It can happens while user travels from page to page and different components going to active state.

mrpmorris commented 1 year ago

This doesn't look like something I'd want to do.

Arsync commented 1 year ago

As @ForeverFast says, it depends more on the business logic.

We can split entire app to working areas / modules. So when user goes to another area, all entities related to previous area, must be cleaned.

I see the goal of holding entities in Fluxor as sharing some entity with unsaved changes across different components. In enterprise app most of data can be presented as 'list of entities' and 'current opened entity' (single state) / 'several opened entities' (collection in the state).

Another case: we have documents of person. Working scope is 'person', document is 'entity'. All entities loaded to the state belongs to person with specific personId. User can open as many documents as needed (by clicking in the list which is not a part of the state). Requested document loads to the entities collection of the state. But when user is navigating to another person, it will cause 'cleanup' action to the collection (single for all persons).

Editable state of document: user writes "body temperature is 98.2 °F" to the document field. It will send entered value to the state. Another field is listening for temperature changes to show some tips, what to do next. If user populate 'height' and 'weight' fields, another component listens for that changes to calculate body mass index and also put it in the Fluxor state of unsaved document.

mrpmorris commented 1 year ago

If you clear down the state of shared entities then you can mess up the state of components that still need it.

If you don't clear it down, you have a memory leak.

If each component has its own copy of the state it needs, then there's no need for shared entities.

Am I missing something?

Arsync commented 1 year ago

As I saw in Entity Adapter manuals, its not about 'globally shared' entities. We can have one state per entity type, so the state can handle more than one entity just of the same type.

If you don't clear it down, you have a memory leak.

In such complex projects that requires sharing of entities, all components that uses shared entity, usually strictly belongs to the 'master component', that requested our shared entity to load. So we always knows, when its time to clear it.

Closest analogy is how to GC works - there always exist "root" which can be safely removed with its children.

I will try to explain... a piece of theory, that respects NDA. My English is not good enough, so sorry if something looks strange.

Inside master component there can many links between field components (to do dependent calculations). Fields count is not infinity (but dynamic). We always can say, how many field components is required by specific document (differs, NDA).

Document can have many fields and all components that uses shared values, located strictly in one document. They just communicates each other.

All field entities located in shared state will be cleared when user closes the exact document which fields belongs to. Main goal of sharing entities - components must react to changes in dataset.

Another one possible argument to use shared entities is to reduce state copying when its time to change field value such like

DocumentState ->
    with Collection of DocumentEntity ->
        with DocumentEntity + Collection of DocumentFieldEntity ->
           with DocumentFieldEntity.Value

We can have

DocumentState -> Collection of DocumentEntity + EntityAdapter,
DocumentFieldState -> Collection of DocumentFieldEntity + EntityAdapter 

And there is time when we come to the mentioned IStateSelection<TState, TElement> as candidate to EntityAdapter's role. Can't find good examples about usage of IStateSelection. Seems we need to call Select in OnInitialized method.

Am I missing something?

In regular cases which is 'get data from server to show it' we can use single state per component and it is right. But all will change when we need to do user edits and show visual gauges, tips and more for dynamic data, related to edits in dynamic count of fields (NDA).

Also, I don't look at Fluxor as client-side database :) As it does by Vuex ORM for Vuex (Vue state management). If we need a database, we can write IndexedDB adapter as middleware for Fluxor.

Arsync commented 1 year ago

I've tried to implement own 'similar functionality' (based on ImmutableDictionary):

public abstract record EntityState<TEntity, TKey>
    where TKey : notnull
{
    public ImmutableDictionary<TKey, TEntity> Entities { get; init; } = ImmutableDictionary<TKey, TEntity>.Empty;
}

public class EntityAdapter<TEntity, TKey>
    where TKey : notnull
{
    private readonly Func<TEntity, TKey> _selectKeyCallback;

    public EntityAdapter(Func<TEntity, TKey> selectKeyCallback)
    {
        _selectKeyCallback = selectKeyCallback;
    }

    public TState AddOne<TState>(TEntity entity, TState state)
        where TState : EntityState<TEntity, TKey>
    {
        var key = _selectKeyCallback(entity);

        return state.Entities.ContainsKey(key) ? state : state with
        {
            Entities = state.Entities.Add(key, entity)
        };
    }

    public TState AddRange<TState>(ICollection<TEntity> entities, TState state)
        where TState : EntityState<TEntity, TKey>
    {
        var items = entities.ToDictionary(x => _selectKeyCallback(x));

        return state with
        {
            Entities = state.Entities.AddRange(items)
        };
    }

    public TState UpdateOne<TState>(TEntity entity, TState state)
        where TState : EntityState<TEntity, TKey>
    {
        var key = _selectKeyCallback(entity);

        return state with
        {
            Entities = state.Entities.SetItem(key, entity)
        };
    }

    public TState RemoveOne<TState>(TKey key, TState state)
        where TState : EntityState<TEntity, TKey>
    {
        return state with
        {
            Entities = state.Entities.Remove(key)
        };
    }

    //public EntitySelectors<TEntity> GetSelectors()
    //{
    //    // TODO: return selectors like selectById, selectEntities etc.
    //    // IStateSelection logic?
    //}
}

State:

[FeatureState(Name = nameof(DocumentState)]
public record DocumentState : EntityState<DocumentEntity, string>
{

}

Usage in reducers:

public static class DocumentStateReducers
{
    private static readonly EntityAdapter<DocumentEntity, string> Adapter = new(
        selectKeyCallback: (entity) => entity.Key
    );

    [ReducerMethod]
    public static DocumentState OnDocumentAdd(DocumentState state, DocumentAddAction action)
    {
        return Adapter.AddOne(action.Document, state);
    }

    [ReducerMethod]
    public static DocumentState OnDocumentUpdate(DocumentState state, DocumentUpdateAction action)
    {
        return Adapter.UpdateOne(action.Document, state);
    }

    [ReducerMethod]
    public static DocumentState OnDocumentRemove(DocumentState state, DocumentRemoveAction action)
    {
        return Adapter.RemoveOne(action.Key, state);
    }
}
Metalvast commented 1 year ago

I've tried to implement own 'similar functionality' (based on ImmutableDictionary):

I have created a package that contains selectors from this repository and my version of the entity adapter. Perhaps this is not the best implementation and I think it can be improved. Here is a small usage example:

State and entity:

    internal sealed record ProductState : EntityState<int, Product>
    {
    }

    public record Product : AdapterEntity
    {
        public required int Id { get; init; }
        public required string Title { get; init; }
        public string? Description { get; set; }
        public required double Grams { get; init; }
        public required double Protein { get; init; }
        public required double Fat { get; set; }

        public required double Carbohydrate { get; init; }

        public int Order { get; init; }
    }

Reducers

    internal static class ProductStateReducers
    {
        private static ProductStateEntityAdapter s_adapter => (ProductStateEntityAdapter)ProductState.GetAdapter();

        [ReducerMethod]
        public static ProductState ReduceLoadProductsAction(ProductState state, LoadProductsSuccessAction _)
            => s_adapter.RemoveAll<ProductState>(state);

        [ReducerMethod]
        public static ProductState ReduceLoadProductsSuccessAction(ProductState state, LoadProductsSuccessAction action)
            => s_adapter.SetAll<ProductState>(action.Products, state);

        [ReducerMethod]
        public static ProductState ReduceCreateProductSuccessAction(ProductState state, CreateProductSuccessAction action)
            => s_adapter.Add<ProductState>(action.Product, state);

        [ReducerMethod]
        public static ProductState ReduceUpdateProductSuccessAction(ProductState state, UpdateProductSuccessAction action)
            => s_adapter.Update<ProductState>(action.Product, state);

        [ReducerMethod]
        public static ProductState ReduceDeleteProductSuccessAction(ProductState state, DeleteProductSuccessAction action)
            => s_adapter.Remove<ProductState>(action.Id, state);
    }

EntityAdapter

    internal sealed class ProductStateEntityAdapter : EntityAdapter<int, Product>
    {
        protected override Func<Product, int> SelectId
            => product => product.Id;

        public override EntityState<int, Product> GetInitialState()
            => new ProductState { };
    }
Metalvast commented 1 year ago

I understand your question, but I don't have a clear answer yet, so I'm going to look for information and try to collect some examples when I have spare time.

I still cannot fully devote myself to this issue, but at least as part of the renewed discussion I will share the code.

Arsync commented 1 year ago
private static ProductStateEntityAdapter s_adapter => (ProductStateEntityAdapter)ProductState.GetAdapter();

I'm not sure that we needs to add complexity of subclasses to EntityAdapter. As it looks in NgRx v8, there is enough of 'EntityAdapter<TEntity,TKey>`.

Because of Fluxor is not supports 'BlazorWebAssemblyLazyLoad', all states must be placed in single ClassLibrary (for example, MyCompany.MyApp.Data.Store, so lookups for cached adapters can be avoided.

// Program.cs
builder.Services.AddFluxor(opts =>
{
    opts.ScanAssemblies(typeof(Program).Assembly, typeof(DocumentState).Assembly);

    if (builder.HostEnvironment.IsDevelopment())
        opts.UseReduxDevTools(x => x.EnableStackTrace());
});

What is ProductStateEntityAdapter? Just bunch of well-known methods of managing entities in standartized way, am I right?

About selectors... Have you seen the StateSelection implementation? IStateSelection looks closer to your ISelector https://github.com/mrpmorris/Fluxor/blob/a63de165043bbd7da39cf926ac83e09049dc78bd/Source/Lib/Fluxor/State.cs#L3-L19

Metalvast commented 1 year ago

I'm not sure that we needs to add complexity of subclasses to EntityAdapter. As it looks in NgRx v8, there is enough of 'EntityAdapter<TEntity,TKey>`.

Initially, I was looking for how to create state correctly and the NgRx functionality seemed attractive to me and I tried to implement it for Fluxor.

What is ProductStateEntityAdapter? Just bunch of well-known methods of managing entities in standartized way, am I right?

Yes

About selectors... Have you seen the StateSelection implementation? IStateSelection looks closer to your ISelector

I added Selectors for tests and this is not my implementation. I added this functionality only for my university project and if I find a better implementation, I will delete this part of the code, leaving only the EntityAdapter

Arsync commented 1 year ago

I added this functionality only for my university project and if I find a better implementation, I will delete this part of the code

It all looks very interesting, my notes is not critic - just possible improvements (or not). Another question: what reason to 'AdapterEntity' subclassing? When I've tried to add entities 'just as it is', it works fine for the big app. Am I missing something that can bite me later?

Metalvast commented 1 year ago

Day is a similar entity with the same as Product State/Reducers/Actions/EntityAdapterClass

Another question: what reason to 'AdapterEntity' subclassing?

The reason for this is to use Map and MapRange:

        [ReducerMethod]
        public static DayState ReduceAttachDayToDateSuccessAction(DayState state, AttachDayToDateSuccessAction action)
            => s_adapter.Map<DayState>(action.DayDateBind.DayId, day => day with
            {
                DayDateBinds = day.DayDateBinds.Append(action.DayDateBind).ToList(),
            }, state);

        [ReducerMethod]
        public static DayState ReduceDetachDayFromDateSuccessAction(DayState state, DetachDayFromDateSuccessAction action)
            => s_adapter.Map<DayState>(action.DayId, day => day with
            {
                DayDateBinds = day.DayDateBinds.Where(x => x.Id != action.DayDateBindId).ToList(),
            }, state);

This is not the best use case, but in this method you can change the fields of a target entity

Arsync commented 1 year ago

The reason for this is to use Map and MapRange

I think, it can be simplified a little: Product is standalone record, without base class. EntityAdapter is just generic and non-abstract, without subclassing and cache. Cache is needed where derived classes has different implementation, which is not covered by default EntityAdapter's implementation. We can even make EntityAdapter sealed to ensure that it does what we expects.

I don't see how exactly AdapterEntity class is used, so we can remove it to grant more flexibility for complex entities (sometimes we need another base class, but there is AdapterEntity which is required by EntityAdapter system).

You may like onion architecture / clean architecture. Product or day looks like domain level entities and EntityAdapter is data level. So data level classes like AdapterEntity can't be applied to entities.

public record Product(int Id, string Title);

[FeatureState(Name = nameof(ProductState)]
public record ProductState : EntityState<Product, int>
{
}

public record ProductCreateSuccessAction(Product Entity);
public record ProductUpdateSuccessAction(Product Entity);

public static class ProductStateReducers
{
    private static readonly EntityAdapter<Product, int> Adapter = new(
        (entity) => entity.Id
    );

    [ReducerMethod]
    public static ProductState OnCreateProduct(ProductState state, ProductCreateSuccessAction action)
        => Adapter.AddOne(action.Entity, state);

    [ReducerMethod]
    public static ProductState OnUpdateProduct(ProductState state, ProductUpdateSuccessAction action)
        => Adapter.Map(action.Entity.Id, state, entity => entity with
        {
            Title = action.Entity.Title
        });
}