Reviewing [and Changing] Effectus

I started to write a review about Ayende Rahien’s sample application Effectus, but I soon realized that what  I do better is to write code. So, this is an unconventional review, because I will focus on some parts of the code, and then show you the alternative.

You can read more about Effectus in Ayende Rahien’s blog.

Straight to the point

In this post I will focus only in the presenters of Effectus. Because this is the first thing that I don’t like.

private void LoadPage(int page)
{
    using (var tx = StatelessSession.BeginTransaction())
    {
        var actions = StatelessSession.CreateCriteria<ToDoAction>()
            .SetFirstResult(page * PageSize)
            .SetMaxResults(PageSize)
            .List<ToDoAction>();

        var total = StatelessSession.CreateCriteria<ToDoAction>()
            .SetProjection(Projections.RowCount())
            .UniqueResult<int>();

        this.NumberOfPages.Value = total / PageSize + (total % PageSize == 0 ? 0 : 1);
        this.Model = new Model
        {
            Actions = new ObservableCollection<ToDoAction>(actions),
            NumberOfPages = NumberOfPages,
            CurrentPage = CurrentPage + 1
        };
        this.CurrentPage.Value = page;

        tx.Commit();
    }
}

Ah? StatelessSession?, BeginTransaction?, CreateCriteria?. This presenter is GLUED to NHibernate. And this means for instance that you can’t test it without NHibernate, and this means that you can’t change your persistence layer.

So, pay attention to Conversation per Business Transaction. Fabio Maulo did a great job with this and I really like it.

MainPresenter

Ayende’s code

namespace Effectus.Features.Main
{
    using System.Collections.ObjectModel;
    using System.ComponentModel;
    using Effectus.Model;
    using Events;
    using Infrastructure;
    using NHibernate.Criterion;
 
    public class Presenter : AbstractPresenter<Model, View>
    {
        const int PageSize = 3;
 
        public Observable<int> NumberOfPages { get; set; }
 
        public Observable<int> CurrentPage { get; set; }
 
        public Presenter()
        {
            NumberOfPages = new Observable<int>();
            CurrentPage = new Observable<int>();
 
            EventPublisher.Register<ActionUpdated>(RefreshCurrentPage);
        }
 
        private void RefreshCurrentPage(ActionUpdated _)
        {
            LoadPage(CurrentPage);
        }
 
        public Fact CanMovePrev
        {
            get { return new Fact(CurrentPage, 
                        () => CurrentPage > 0); }
        }
 
        public Fact CanMoveNext
        {
            get { return new Fact(CurrentPage, 
                        () => CurrentPage + 1 < NumberOfPages); }
        }
 
        public void OnCreateNew()
        {
            Presenters.Show("CreateNew");
        }
 
        public void OnActionsChoosen(ToDoAction action)
        {
            Presenters.Show("Edit", action.Id);
        }
 
        public void OnLoaded()
        {
            LoadPage(0);
        }
 
        public void OnMoveNext()
        {
            LoadPage(CurrentPage + 1);
        }
 
        public void OnMovePrev()
        {
            LoadPage(CurrentPage - 1);
        }
 
        private void LoadPage(int page)
        {
            using (var tx = StatelessSession.BeginTransaction())
            {
                var actions = StatelessSession
                    .CreateCriteria<ToDoAction>()
                    .SetFirstResult(page * PageSize)
                    .SetMaxResults(PageSize)
                    .List<ToDoAction>();
 
                var total = StatelessSession
                       .CreateCriteria<ToDoAction>()
                    .SetProjection(Projections.RowCount())
                    .UniqueResult<int>();
 
                this.NumberOfPages.Value = 
                        total / PageSize + (total % PageSize == 0 ? 0 : 1);
                this.Model = new Model
                {
                    Actions = new ObservableCollection<ToDoAction>(actions),
                    NumberOfPages = NumberOfPages,
                    CurrentPage = CurrentPage + 1
                };
                this.CurrentPage.Value = page;
 
                tx.Commit();
            }
        }
    }
}

My code

using System.Linq;
using Effectus.DataAccess;
using uNhAddIns.Adapters;

namespace Effectus.Features.Main
{
    using System.Collections.ObjectModel;
    using Effectus.Model;
    using Events;
    using Infrastructure;

    [PersistenceConversational(
        DefaultEndMode = EndMode.End, 
        MethodsIncludeMode = MethodsIncludeMode.Explicit)]
    public class Presenter : AbstractPresenter<Model, View>
    {
        private readonly IReadOnlyDao<ToDoAction> _toDoActionsDao;
        const int PageSize = 3;

        public Observable<int> NumberOfPages { get; set; }

        public Observable<int> CurrentPage { get; set; }

        public Presenter(IReadOnlyDao<ToDoAction> toDoActionsDao)
        {
            _toDoActionsDao = toDoActionsDao;
            NumberOfPages = new Observable<int>();
            CurrentPage = new Observable<int>();

            EventPublisher.Register<ActionUpdated>(RefreshCurrentPage);
        }

        [PersistenceConversation]
        public virtual void RefreshCurrentPage(ActionUpdated _)
        {
            LoadPage(CurrentPage);
        }

        public Fact CanMovePrev
        {
            get { return new Fact(CurrentPage, 
                            () => CurrentPage > 0); }
        }

        public Fact CanMoveNext
        {
            get { return new Fact(CurrentPage, 
                            () => CurrentPage + 1 < NumberOfPages); }
        }

        public void OnCreateNew()
        {
            Presenters.Show("CreateNew");
        }

        public void OnActionsChoosen(ToDoAction action)
        {
            Presenters.Show("Edit", action.Id);
        }

        [PersistenceConversation]
        public virtual void OnLoaded()
        {
            LoadPage(0);
        }
        
        [PersistenceConversation]
        public virtual void OnMoveNext()
        {
            LoadPage(CurrentPage + 1);
        }

        [PersistenceConversation]
        public virtual void OnMovePrev()
        {
            LoadPage(CurrentPage - 1);
        }

        private void LoadPage(int page)
        {
            var actions = _toDoActionsDao.RetrieveAll()
                                         .Skip(page * PageSize)
                                         .Take(PageSize).ToList();

            var total = _toDoActionsDao.RetrieveAll().Count();
            
            NumberOfPages.Value = total / PageSize 
                                + (total % PageSize == 0 ? 0 : 1);
            
            Model = new Model
            {
                Actions = new ObservableCollection<ToDoAction>(actions),
                NumberOfPages = NumberOfPages,
                CurrentPage = CurrentPage + 1
            };
            
            CurrentPage.Value = page;
        }
    }
}

There are two things that you will notice:

  • A dependency with IReadonlyDao<ToDoAction>. I will show you later the code for daos.
  • Conversation per business transactions attributes. These attributes are from a separated assembly unrelated to nhibernate. This means that we could implement this for EntityFramework or other ORM and the presenter is not coupled to the persistence framework. This will pass the “acid” test, thanks Angel “Java” Lopez.

Ah, there is a last thing about that code. There is a bug with the current page number.

AbstractPresenter

Ayende’s Code

namespace Effectus.Infrastructure
{
    using System;
    using System.Windows;
    using NHibernate;

    public abstract class AbstractPresenter<TModel, TView> : IDisposable, IPresenter
        where TView : Window, new()
    {
        private TModel model;
        private ISessionFactory sessionFactory;
        private ISession session;
        private IStatelessSession statelessSession;

        protected AbstractPresenter()
        {
            View = new TView();
            View.Closed += (sender, args) => Dispose();
        }

        DependencyObject IPresenter.View { get { return View; } }

        public object Result { get; protected set; }

        protected TView View { get; set; }

        protected ISessionFactory SessionFactory
        {
            get { return sessionFactory; }
        }

        protected ISession Session
        {
            get
            {
                if (session == null)
                    session = sessionFactory.OpenSession();
                return session;
            }
        }

        protected IStatelessSession StatelessSession
        {
            get
            {
                if (statelessSession == null)
                    statelessSession = sessionFactory.OpenStatelessSession();
                return statelessSession;
            }
        }

        protected TModel Model
        {
            get { return model; }
            set
            {
                model = value;
                View.DataContext = model;
            }
        }

        protected void ReplaceSessionAfterError()
        {
            if(session!=null)
            {
                session.Dispose();
                session = sessionFactory.OpenSession();
                ReplaceEntitiesLoadedByFaultedSession();
            }
            if(statelessSession!=null)
            {
                statelessSession.Dispose();
                statelessSession = sessionFactory.OpenStatelessSession();
            }
        }

        protected virtual void ReplaceEntitiesLoadedByFaultedSession()
        {
            throw new InvalidOperationException(
                @"ReplaceSessionAfterError was called, but the presenter does not override ReplaceEntitiesLoadedByFaultedSession!
You must override ReplaceEntitiesLoadedByFaultedSession to call ReplaceSessionAfterError.");
        }


        public void SetSessionFactory(ISessionFactory theSessionFactory)
        {
            sessionFactory = theSessionFactory;
        }

        public void Show()
        {
            View.Show();
        }

        public void ShowDialog()
        {
            View.ShowDialog();
        }

        public event Action Disposed = delegate { };

        public virtual void Dispose()
        {
            if(session!=null)
                session.Dispose();
            if (statelessSession != null)
                statelessSession.Dispose();
            View.Close();
            Disposed();
        }
    }
}

My code

namespace Effectus.Infrastructure
{
    using System;
    using System.Windows;
    using NHibernate;

    public abstract class AbstractPresenter<TModel, TView> : IDisposable, IPresenter
        where TView : Window, new()
    {
        private TModel model;

        protected AbstractPresenter()
        {
            View = new TView();
            View.Closed += (sender, args) => Dispose();
        }

        DependencyObject IPresenter.View { get { return View; } }

        public object Result { get; protected set; }

        protected TView View { get; set; }

        protected TModel Model
        {
            get { return model; }
            set
            {
                model = value;
                View.DataContext = model;
            }
        }

        public void Show()
        {
            View.Show();
        }

        public void ShowDialog()
        {
            View.ShowDialog();
        }

        public event Action Disposed = delegate { };

        public virtual void Dispose()
        {
            View.Close();
            Disposed();
        }

        public virtual void OnException(Exception exception)
        {
            //TODO You should improve this.
            MessageBox.Show(exception.Message);
        }
    }
}

Again, I’ve deleted all NHibernate code.

EditPresenter

Ayende’s code

namespace Effectus.Features.Edit
{
    using System;
    using System.Windows;
    using Effectus.Model;
    using Events;
    using Infrastructure;
    using Merge;
    using NHibernate;

    public class Presenter : AbstractPresenter<Model, View>
    {
        public Presenter()
        {
            EventPublisher.Register<ActionUpdated>(RefreshAction);
        }

        private void RefreshAction(ActionUpdated actionUpdated)
        {
            if (actionUpdated.Id != Model.Action.Id)
                return;
            Session.Refresh(Model.Action);
        }

        public void Initialize(long id)
        {
            ToDoAction action;
            using (var tx = Session.BeginTransaction())
            {
                action = Session.Get<ToDoAction>(id);
                tx.Commit();
            }

            if (action == null)
                throw new InvalidOperationException("Action " + id + " does not exists");

            this.Model = new Model
            {
                Action = action
            };
        }
        [PersistenceConversation(ConversationEndMode = EndMode.Abort)]
        public void OnCancel()
        {
            View.Close();
        }

        public void OnCreateConcurrencyConflict()
        {
            using (var session = SessionFactory.OpenSession())
            using (var tx = session.BeginTransaction())
            {
                var anotherActionInstance = session.Get<ToDoAction>(Model.Action.Id);
                anotherActionInstance.Title = anotherActionInstance.Title + " - ";
                tx.Commit();
            }
            MessageBox.Show("Concurrency conflict created");
        }

        public void OnSave()
        {
            bool successfulSave;
            try
            {
                using (var tx = Session.BeginTransaction())
                {
                    // this isn't strictly necessary, NHibernate will 
                    // automatically do it for us, but it make things
                    // more explicit
                    Session.Update(Model.Action);

                    tx.Commit();
                }
                successfulSave = true;
            }
            catch (StaleObjectStateException)
            {
                var mergeResult = Presenters.ShowDialog<MergeResult?>("Merge", Model.Action);
                successfulSave = mergeResult != null;

                ReplaceSessionAfterError();
            }

            // we call ActionUpdated anyway, either we updated the value ourselves
            // or we encountered a concurrency conflict, in which case we _still_
            // want other parts of the application to update themselves with the values
            // from the db
            EventPublisher.Publish(new ActionUpdated
            {
                Id = Model.Action.Id
            }, this);

            if (successfulSave)
                View.Close();
        }

        protected override void ReplaceEntitiesLoadedByFaultedSession()
        {
            Initialize(Model.Action.Id);
        }
    }
}

There are several things with the OnSave method that I don’t like:

  • ReplaceSessionAfterError, to much responsibility for a presenter.
  • Session/Transaction again.
  • There is a BUG, the publish mechanism work in sync with the rest of the code. This means… that this windows is not going to close until others have finished handling the event. For the given example, the Edit windows is not going to close until the main window finish to refresh the current page.
  • Too much logic for this method = hard to test.

My code

using Effectus.DataAccess;
using Effectus.Features.Merge;
using Effectus.Infrastructure.Bootstrapping;
using Effectus.Infrastructure.ConversationsArtifacts;
using uNhAddIns.Adapters;
using uNhAddIns.SessionEasier.Conversations;

namespace Effectus.Features.Edit
{
    using System;
    using System.Windows;
    using Effectus.Model;
    using Events;
    using Infrastructure;
    using NHibernate;

    [PersistenceConversational(
            DefaultEndMode = EndMode.Continue, 
            UseConversationCreationInterceptorConvention = true)]
    public class Presenter : AbstractPresenter<Model, View>
    {
        private readonly IDao<ToDoAction> _toDoActionsDao;

        public Presenter(IDao<ToDoAction> toDoActionsDao)
        {
            _toDoActionsDao = toDoActionsDao;
            EventPublisher.Register<ActionUpdated>(RefreshAction);
        }
        
        [PersistenceConversation]
        public virtual void RefreshAction(ActionUpdated actionUpdated)
        {
            if (actionUpdated.Id != Model.Action.Id)
                return;

            _toDoActionsDao.Refresh(Model.Action);
        }

        [PersistenceConversation]
        public virtual void Initialize(long id)
        {
            var action = _toDoActionsDao.GetById(id);

            if (action == null)
                throw new InvalidOperationException("Action " + id + " does not exists");

            Model = new Model
            {
                Action = action
            };
        }

        public void OnCancel()
        {
            View.Close();
        }

        public void OnCreateConcurrencyConflict()
        {
            //NOTE: this portion of code is for generating a conflict.
            //don't use sessionfactory in presenters.

            var sessionFactory = BootStrapper.Container.Resolve<ISessionFactory>();

            using (var session = sessionFactory.OpenSession())
            using (var tx = session.BeginTransaction())
            {
                var anotherActionInstance = session.Get<ToDoAction>(Model.Action.Id);
                anotherActionInstance.Title = anotherActionInstance.Title + " - ";
                tx.Commit();
            }
            MessageBox.Show("Concurrency conflict created");
        }

        [PersistenceConversation(ConversationEndMode = EndMode.End)]
        public virtual void OnSave()
        {
            _toDoActionsDao.Update(Model.Action);

            EventPublisher.Enlist(new ActionUpdated
            {
                Id = Model.Action.Id
            }, this);

            View.Close();
        }

        public override void OnException(Exception exception)
        {
            if(exception is StaleEntityException)
            {
                Presenters.ShowDialog<MergeResult?>("Merge", Model.Action);

                EventPublisher.Enlist(new ActionUpdated
                {
                    Id = Model.Action.Id
                }, this);

            }
        }
    }

}

Pay atention:

  • I’ve defined a new convention, if a public method in the presenter throw an exception, the OnException method will be called. This is done by a castle interceptor, and this is the last line of defense for unhandled exceptions.
  • I’m using “StaleEntityException” rather than “StaleObjectStateException”, this is MY exception. This is easily done by a CpBT artifact.
  • I’m not calling “EventPublisher.Publish” anymore, this code use EventPublisher.Enlist. Here, I’ve split the “Publish” code in two different methods one for Enlist and other for Raise. The enlisted events will be raised right after the OnSave method is called and thus after the windows is closed.
  • Also, notice that here is the conversation per business transaction pattern with all its splendor. The two former methods are conversation participants, with EndMode equals to continue. This means that the NH Session will remain opened. The OnSave method has EndMode equals to End, this means that right after the method finished, CpBT internally will flush the Unit of Work and close it.

 

The code needed for exception interception:

public class RecoverableExceptionIntercepter : IInterceptor
{
    public void Intercept(IInvocation invocation)
    {
        if(!typeof(ISupportExceptionRecovery).IsAssignableFrom(invocation.TargetType))
        {
            throw new NotSupportedException(string.Format("{0} should implement {1}", 
                invocation.TargetType.Name, 
                typeof(ISupportExceptionRecovery).Name));
        }

        try
        {
            invocation.Proceed();
        }catch(Exception ex)
        {
            var recoverablePresenter = (ISupportExceptionRecovery) invocation.InvocationTarget;
            recoverablePresenter.OnException(ex);
        }

    }
}

The code needed for auto-flushing published events:

public class AutoFlushEventPublisherEvents : IInterceptor
{
    public void Intercept(IInvocation invocation)
    {
        invocation.Proceed();
        EventPublisher.FlushEvents((IPresenter) invocation.InvocationTarget);
    }
}

The code for daos

Interfaces:

public interface IReadOnlyDao<T>
    where T : EntityBase 
{
    T GetById(object id);
    T GetProxy(object id);
    IEnumerable<T> Retrieve(Expression<Func<T, bool>> predicate);

    int Count(Expression<Func<T, bool>> predicate);

    /// <summary>
    /// Retrieve all entities.
    /// </summary>
    /// <returns></returns>
    IEnumerable<T> RetrieveAll();
}

public interface IDao<T> : IReadOnlyDao<T>
    where T : EntityBase 
{
    void Save(T entity);
    void Update(T entity);
    void Delete(T entity);
    
    /// <summary>
    /// Re-read the state of the given instance from the underlying database.
    /// </summary>
    /// <param name="entity"></param>
    void Refresh(T entity);

    /// <summary>
    /// Merge the instance in to the persisted instance of the current unit of work.
    /// </summary>
    /// <param name="entity"></param>
    void Merge(T entity);

}

These interfaces were taken from Fabio Maulo’s post “Repository or DAO”?: Dao.

Implementation:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Linq.Expressions;
using Effectus.Model;
using NHibernate;
using NHibernate.Criterion;
using NHibernate.Linq;

namespace Effectus.DataAccess
{
    public class Dao<T> : IDao<T>
        where T : EntityBase
    {
        private readonly ISessionFactory _sessionFactory;

        public Dao(ISessionFactory sessionFactory)
        {
            _sessionFactory = sessionFactory;
        }

        private IQueryable<T> CurrentLinq
        {
            get { return ContextSession.Linq<T>(); }
        }

        private ISession ContextSession
        {
            get { return _sessionFactory.GetCurrentSession(); }
        }

        #region IDao<T> Members

        public void Save(T entity)
        {
            ContextSession.Save(entity);
        }

        public void Update(T entity)
        {
            ContextSession.Update(entity);
        }

        public void Delete(T entity)
        {
            ContextSession.Delete(entity);
        }

        public void Refresh(T entity)
        {
            ContextSession.Refresh(entity);
        }

        public void Merge(T entity)
        {
            ContextSession.Merge(entity);
        }

        public T GetById(object id)
        {
            return ContextSession.Get<T>(id);
        }

        public T GetProxy(object id)
        {
            return ContextSession.Load<T>(id);
        }

        public IEnumerable<T> Retrieve(
                    Expression<Func<T, bool>> predicate)
        {
            return CurrentLinq.Where(predicate);
        }

        public IEnumerable<T> RetrieveAll()
        {
            return CurrentLinq;
        }

        public int Count(Expression<Func<T, bool>> predicate)
        {
            return CurrentLinq.Count(predicate);
        }

        #endregion
    }

    
}

Download

Here is the full code. Maybe someone help me to “fork” github version, this will be very nice to make code diff.

The end

There is nothing more, I like very much the result of this review and I will be waiting your comments (please let me a comment). I hope you like it!


blog comments powered by Disqus
  • Categories

  • Archives