Automatyczne zwracanie z serwisu informacji o błędzie
Zadaniem, które wziąłem na warsztat w poprzednim tygodniu, była refaktoryzacja obsługi optimistic concurrency. Do tej pory obsługa konfliktu wersji przy aktualizacji rekordu była obsługiwana za pomocą wyjątku rzucanego z serwisu i łapanego w filtrze MVC. To oczywiście był błąd projektowy, bo przecież wystąpienie optimistic concurrency to spodziewany efekt edycji rekordu, a nie żaden wyjątek. Poprawiając to, stwierdziłem że dobrze będzie przy okazji w ogóle poprawić komunikację na linii serwis-kontroler, tak aby wyjątki z aplikacji nigdy nie docierały do GUI.
Po co?
Jest to potrzebne chociażby dlatego, że w przyszłości warstwa aplikacyjna może przecież pracować na innej fizycznej maszynie niż warstwa prezentacji, więc rzucenie wyjątku na taką odległość niekoniecznie się uda. Z tego powodu lepiej od razu mieć możliwość przesłania informacji o sukcesie/błędzie operacji w postaci obiektu, no i oczywiście najlepiej by było, jeśli będzie się to działo automatycznie. Poza tym, użytkownikowi warto wyświetlać jakieś lokalizowane komunikaty o błędach. Lokalizowane przez nas, a nie zlokalizowaną wersję .NET Frameworka z jego mądrościami ludowymi w stylu: “Odwołaniem do obiektu nie zostało ustawione na wystąpienie obiektu” albo “Do członka wystąpienia klasy nie można odwołać się z obrębu udostępnionej metody lub udostępnionego inicjatora członka bez jawnego wystąpienia klasy”. ;)
Dobra, dość drwin z Microsoftu, bo nie wygram XBoxa.
Typy do przesyłania odpowiedzi
Do tej pory metody ICrudServiceBase zwracały obiekt (GetJakiśtamModel), ID obiektu (Create) , DataPageResult<T> albo niczego nie zwracały (w przypadku operacji Update i Delete). To było złe, więc zacząłem od poprawienia tego.
Najpierw utworzyłem taki enum opisujący możliwe statusy odpowiedzi:
1 public enum CrudOperationState
2 {
3 Undefined,
4 Success,
5 DatabaseError,
6 AccessDeniedError,
7 OtherError,
8 OptimisticConcurrencyError
9 }Następnie stworzyłem taką prostą hierarchię typów:
1 public abstract class CrudOperationResultBase
2 {
3 public CrudOperationState State { private set; get; }
4 public string ErrorMessage { get; private set; }
5
6 public bool Succeed
7 {
8 get { return this.State == CrudOperationState.Success; }
9 }
10
11 protected CrudOperationResultBase(CrudOperationState state, string errorMessage)
12 {
13 this.State = state;
14 this.ErrorMessage = errorMessage;
15 }
16 }
17
18 public sealed class CrudOperationResult<TResultType> : CrudOperationResultBase
19 {
20 public TResultType Data { get; private set; }
21
22 public CrudOperationResult(CrudOperationState state, string errorMessage)
23 : base(state, errorMessage)
24 {
25 }
26
27 public CrudOperationResult(TResultType data)
28 : base(CrudOperationState.Success, null)
29 {
30 this.Data = data;
31 }
32 }
33
34 public sealed class CrudOperationResult : CrudOperationResultBase
35 {
36 public CrudOperationResult(CrudOperationState state, string errorMessage)
37 : base(state, errorMessage)
38 {
39 }
40
41 public static CrudOperationResult Success
42 {
43 get { return new CrudOperationResult(CrudOperationState.Success, null); }
44 }
45 }Jak widać - mamy abstrakcyjną wersję bazową, a do tego dwie konkretne - jedną dla metod bez danych wynikowych, drugą mogącą zwrócić wynik typu generycznego. Mogłem oczywiście zrobić tak, żeby generyczna wersja dziedziczyła z niegenerycznej, ale:
- Po pierwsze (o czym wielu programistów nie pamięta) nie dziedziczy się z klas nieabstrakcyjnych (o ile nie jest to konieczne, czyli np. nie hakujemy cudzego frameworka).
- Do faktycznych zastosowań stosuje się klasy z końca hierarchii, nigdy bazowe.
- Dziedziczenie z klasy abstrakcyjnej wymusza na klasie pochodnej zaimplementowanie konkretnych konstruktorów. Dzięki czemu mamy pewność, że każdy obiekt klasy pochodnej będzie mógł zostać stworzony w ten sam sposób. To bardzo ułatwia np. atuomatyzację tworzenia obiektów.
Ostatni punkt jest dość istotny, bo CrudOperationResultBase stało się klasą bazową dla DataPageResult<T>, czyli klasy zawierającej dane wypełniające grida. Klasa ta wygląda teraz tak:
1 public sealed class DataPageResult<T> : CrudOperationResultBase
2 {
3 public IList<T> Items { get; private set; }
4 public PagingInfo PagingInfo { get; private set; }
5
6 public DataPageResult(IList<T> items, int pageNumber, int pageSize, int totalItemsCount)
7 : base(CrudOperationState.Success, null)
8 {
9 this.Items = items;
10 this.PagingInfo = new PagingInfo(pageNumber, pageSize, totalItemsCount);
11 }
12
13 public DataPageResult(CrudOperationState state, string errorMessage)
14 : base(state, errorMessage)
15 {
16 }
17
18 public override string ToString()
19 {
20 var sb = new StringBuilder("DataPage");
21 sb.AppendLine("PagingInfo: " + this.PagingInfo);
22 sb.AppendLine("Items:");
23 foreach (var item in this.Items)
24 {
25 sb.AppendLine(item.ToString());
26 }
27
28 return sb.ToString();
29 }
30 }Czyli poza starym konstruktorem obsługującym “pozytywny” przypadek otrzymania danych z bazy, doszedł nowy konstruktor z klasy bazowej, który pozwala na zwrócenie informacji o błędzie.
Zastanawiałem się przez pewien czas, czy to dobry pomysł, czy nie lepiej byłoby tworzyć CrudOperationResult<DataPageResult<T>>, ale stwiedziłem, że wymusi to zmianę interfejsów serwisowych, a co za tym idzie - konieczność poprawienia połowy aplikacji i większości testów, nie dając w zamian nic.
Nowa sygnatura ICrudServiceBase
Po tych zmianach trzeba poprawić sygnaturę ICrudServiceBase oraz klasy go implementującej na taką:
1 public interface ICrudServiceBase<TGridModel, TDetailsModel, TEditModel, TCreateModel>
2 where TGridModel : IGridModelBase
3 where TDetailsModel : IDetailsModelBase
4 where TEditModel : IEditModelBase
5 where TCreateModel : ICreateModelBase
6 {
7 DataPageResult<TGridModel> GetDataPage(DataRequest<TGridModel> request);
8 CrudOperationResult<TCreateModel> GetCreateModel();
9 CrudOperationResult<TEditModel> GetEditModel(int id);
10 CrudOperationResult<TDetailsModel> GetDetailsModel(int id);
11 CrudOperationResult<int> Create(TCreateModel createModel);
12 CrudOperationResult Update(TEditModel editModel);
13 CrudOperationResult Delete(int id);
14 }Poprawka TransactionManagingInterceptora
Kontrakty za mną, teraz czas na implementacje. Ponieważ od zawsze uwazam, że w aplikacji powinno być jedno miejsce na komunikację z bazą oraz jedno miejsce na łapanie wyjątków, od początku w PizzaMVC stosuję takie podejście. Dzięki uprzejmości Autofaca i jego kompatybilności z Windsorowymi interceptorami powstała klasa TransactionManagingInterceptor. Autofac opakowuje wywołania wszystkich metod serwisów oznaczonych atrybutem Transactional właśnie tym interceptorem. (To jest przyczyna, dla której metody CrudServiceBase są wirtualne.) Interceptor zaś:
- Sprawdza, czy jest już otwarta transakcja w ramach obecnego UoW, jeśli nie, to ją otwiera.
- Wykonuje kod opakowanej metody.
- Zatwierdza transakcję (a przynajmniej próbuje).
Jeśli podczas powyższych operacji nastąpił wyjątek, to do tej pory interceptor go łapał, logował… i rzucał dalej. Przy okazji StaleObjectException z NHibernate opakowywał w mój własny OptimisticConcurrencyException. Szczerze mówiąc, nie wiem, co piłem, gdy to wymyśliłem, ale na wszelki wypadek od dzisiaj niczego mocniejszego od herbaty nie ruszam.
Teraz czas na dobrą zmianę - w przypadku wystąienia wyjątku należy:
- Jeśli metoda zwraca
CrudOperationResult,CrudOperationResult<T>alboDataPageResult<T>- utworzyć obiekt tego typu z odpowiednim statusem błędu oraz komunikatem. - W przypadku innych metod (a takie mogą się przecież znaleźć w serwisie), po prostu rzucić wyjątek dalej.
Wygląda to tak:
1 try
2 {
3 iAmTheFirst = this.BeginTransactionIfNeeded();
4 invocation.Proceed();
5 this.TryCommit(iAmTheFirst, invocation);
6 }
7 catch (Exception ex)
8 {
9 this.logger.Error(ex, "Exception in method: {0}", invocation.Method.GetFullMethodName());
10 this.RollbackTransaction(iAmTheFirst, invocation);
11
12 if (InvocationHelper.IsKnownCrudOperation(invocation))
13 {
14 invocation.ReturnValue = InvocationHelper.BuildReturnValueForError(invocation, ex);
15 }
16 else
17 {
18 throw;
19 }
20 }Bądźmy szczerzy - to nie jest piękny kod. Ale też trudno to zrobić dużo lepiej, bo throw niestety musi być w catch. :(
W każym razie - nieważne, czy czytamy dane, wstawiamy, kasujemy czy aktualizujemy, InvocationHelper ma utworzyć odpowiedni obiekt wynikowy. Robi to tak:
1 public static object BuildReturnValueForError(IInvocation invocation, Exception ex)
2 {
3 var returnType = invocation.Method.ReturnType;
4
5 var resultTypeToCreate = returnType;
6 if (returnType.IsGenericType)
7 {
8 var genericTypeDefinition = returnType.GetGenericTypeDefinition();
9 var genericArguments = returnType.GetGenericArguments();
10 resultTypeToCreate = genericTypeDefinition.MakeGenericType(genericArguments);
11 }
12
13 var state = GetCrudOperationStateForException(ex);
14
15 object crudOperationResult = Activator.CreateInstance(resultTypeToCreate, state, ex.Message);
16 return crudOperationResult;
17 }Piękno prostoty w całej okazałości - parę linijek banalnej refleksji i nie trzeba mieć try-catcha powielającego praktycznie ten sam kod (otwarcie transakcji, właściwa akcja, commit transakcji, obsługa błędów i ifologia) w każdej metodzie serwisu. Zaoszczędzone dziesiątki liniek nikomu niepotrzebnego kodu.
Warstwa prezentacji
Do tej pory warstwa prezentacji otrzymywała wyjątek, więc w przypadku zwykłych akcji łapał go UniversalExceptionFilter, a w przypadku żądań służących do pobrania danych do grida albo usunięcia rekordu, zajmowało się tym jQuery. Po zmianie interceptora, nie ma już wyjątków, więc trzeba ręcznie obsłużyć zwrócone statusy. Zmiany w warstwie prezentacji nie były duże. Po prostu dodałem kilka komunikatów o błędach do zasobów aplikacji. A w akcjach kontrolera CrudControllerBase sprawdzam, czy operacja się powiodła. Jeśli tak, to pokazuję dane/przekierowanie. Jeśli nie, to wyświetlam komunkat o błedzie. W przypadku JSONa, zwracam odpowiedni komunikat i ustawiam Response.StatusCode na HttpStatusCode.BadRequest, aby jQuery wiedziało, że ma wyświetlić błąd.