UnityOfWorkRegistry antipattern?   zodpovězená otázka

C#

Zdravím,

reaguji především na přednášku DotNetCollege ohledně zajímavých návrhových vzorů zde:

https://www.wug.cz/brno/akce/743-Zajimav...

Kde si generický repozitář / query vytahuje DbContext nepřímo buďto z LocalThread storage, případně z HttpContext, rád bych se optal na vaše názory, jelikož já toto považuji velmi silně jako nebezpečné a jako antipattern.

Repozitář je totiž závislý velmi silně na externím kontextu, ve kterým je volán, což v hodně situacích nefunguje.

Nejdříve k prvnímu problému, pokud vytváříme byznys transakci ve fasádě, musíme na tomto místě říct, že se jedná o například HttpContextUnitOfWorkRegistry.. tedy na tvrdku "na blind" říkáme, že nad námi musí být HttpContext, což je špatně. Z principu fasády, ona neví co je nad ní a tedy můžu zavolat metodu této fasády z metody, kde není HttpContext k dispozici.

Ani ThreadLocalUnitOfWorkRegistry nám nepomůže, jelikož uvnitř metody této fasády může vézt k volání pomocí delegátů. Tedy jiný thread a náš UOW opět není k dispozici.

-------------------------------------

Pokud umístíme vytváření UOW o úroveň výše (controllery), už můžeme použít HttpContextUOWRegistry, ale pořád nám to vůbec neřeší problém, kdy je metoda volána z kontextu, kde není HttpContext k dispozici, nebo je uvnitř metody fasády nějaký delegát... tedy pořád se jedná o "doufání" v to, že je nad námi HttpContext, nebo že pod námi se nepouští delegát.

--------------------------------------------

Co nám také nepomůže, je udělat všechny naše metody "awaitovaný", jelikož u awaitu se od 4.5tky přenáší automaticky HttpContext.. tudíž na začátku naší metody ve fasádě by to fungovalo, ale někde dál - pokud se tam zase používá delegation už ne.

--------------------------------------

Typickým příkladem, kde je toto obrovský problém je IdentityFramework,.. pokud chci zachovat určitou konzistenci mojí N-tier architektury, tak si Identity framework rozdělím na Core(UserManager) a Dal (UserStore). Pokud se kouknete do implementace UserManageru zjistíte, že tam dělá prasárnu a sice, že dělá z async metod synchronní pomocí Unwrap a uvnitř unwrap nastavuje culture, aby to fungovalo.

Pokud to tedy takto máte tak to vypadá takto:

AuthorizationServerProvider

-> MyUserManager

-> MyUserStore (zde si store bere kontext pomocí registry)

Tedy standardní architektura, kterou máte v celé aplikaci. Problém je, že Store je ve skutečnosti mnohem více svázaný s kontextem "nad ním", než je zdrávo a tedy pokud někdo udělá nad ním nové vlákno a v něm použije ten store, tak jste nahraní... Proto ukládat kamkoliv UOW ve vrstvě N a pak si je getovat ve vrstve N+2 považuji za antipattern.

Samozřejmě by toto šlo alespoň částečně řešit tím, kdyby se UOW getovalo při vytváření storu a ne až getterem.. ale to řeší pouze část problému. Pořád je narušený koncept. I pokud kouknu jak jsou impelemntovány transakce atp., tak také jsou přímo posílány jako param.

Z tohoto důvodu jediné správné řešení považuji posílat UOW až do nejspodnější vrstvy, která si pak z něj "něco vezme". tedy mít:

public MujStore(UOW uow);

jedině takto je to bezpečné.

Co myslíte?

nahlásit spamnahlásit spam 2 / 2 odpovědětodpovědět

K této mojí domněnce mi přispívá nejvíce i to, že UOW je ve většině případů defakto jen abstrakce DB Contextu (který se také posílá do storů)

nahlásit spamnahlásit spam 0 odpovědětodpovědět

Nejdříve k prvnímu problému, pokud vytváříme byznys transakci ve fasádě, musíme na tomto místě říct, že se jedná o například HttpContextUnitOfWorkRegistry.. tedy na tvrdku "na blind" říkáme, že nad námi musí být HttpContext, což je špatně.

Tak to není. Fasáda o tom, jak si to IUnitOfWorkProvider nemá vůbec vědět - konkrétní registry se nastavuje v konfiguraci IoC/DI kontejneru v aplikaci, odkud fasádu používáte. HttpContextUnitOfWorkRegistry může navíc dostat ThreadLocalUnitOfWorkRegistry v konstruktoru jako fallback, takže pokud HttpContext není k dispozici, použije se ThreadLocal storage, což je na webu řešení, které zkousne i background vlákna, která můžete vytvořit (i když pro cokoliv reálného je stejně nedoporučuji používat).

...může vézt k volání pomocí delegátů. Tedy jiný thread a náš UOW opět není k dispozici.

Co má delegát společného s jiným vláknem, je mi záhadou.

Pokud umístíme vytváření UOW o úroveň výše (controllery), ...

Špatně. Jestli je něco antipattern, tak je to používání UOW v prezentační vrstvě, kterou controller nepochybně je. Controller má posbírat data z UI, zavolat funkci ve fasádě, která vrátí výsledky, a ty controller nějak aplikuje do UI.

UOW je vytvářena vždy ve fasádě.

...a tedy pokud někdo udělá nad ním nové vlákno a v něm použije ten store, tak jste nahraní...

Používat objekty, které nejsou thread safe (což je třeba příklad DbContextu, UserStore nebo dalších věcí, které na nich silně závisí, např. i těch repository), by se také nemělo. Do jiného vlákna byste entity ani kontext neměl předávat nikdy, a u async/await musíte hlídat, jak to konkrétní metoda interně dělá.

Anebo od .NET 4.6 je nová třída AsyncLocal, která by tohle měla řešit, ale to nic nemění na tom, že z více vláken bych to raději nepoužíval, ten DbContext to obecně nemá rád a může to mít i další nepříjemné dopady.

Ohledně toho UserStore - já to řeším tak, že jsem si jej podědil a DbContext mu předávám přímo v konstruktoru v době, kdy vzniká (volám tam unitOfWorkProvider.GetCurrent().Context), a UserManager do fasád injektuji vždy přes Func<UserManager>, takže mám pak kontrolu nad tím, kdy vzniká store a vím, že dostane správný kontext (vyrobím si instanci user manageru až uvnitř unit of work). A ještě je dobré tomu store říct, aby po disposnutí context nechalo být a nedisposovalo ho, je tam na to nějaká property.

Proto ukládat kamkoliv UOW ve vrstvě N a pak si je getovat ve vrstve N+2 považuji za antipattern.

Pokud vytváříte UOW vždy ve fasádách (což byste měl), a používáte ji jen z repository, queries, nebo toho UserStore (což by tak taky mělo být), pak je to skok jen o 1 vrstvu.

Z tohoto důvodu jediné správné řešení považuji posílat UOW až do nejspodnější vrstvy, která si pak z něj "něco vezme".

Mně se nelíbí zaprasit si polovinu funkcí v aplikaci tím, že jako první nebo poslední parametr budou přijímat unit of work. UOW v době, kdy vznikají instance, které ji potřebují, ještě neexistuje, tudíž se nedá nijak rozumně injektovat, můžete injektovat jen toho unit of work registry, který vám ji nějak musí dát.

On ten problém pramení hodně z Entity Frameworku samotného - věci jako change tracking a obecně samotný DbContext chtě nechtě budou prorůstat ven a ovlivňovat to, jak musíte aplikaci napsat. Zavádí to tam různá omezení, od kterých se můžete částečně odstínit pomocí architektury, ale nikdy ne úplně.

Jinak to, že UOW řeší víceméně jen abstrakci DbContextu, je dáno především tím, že většina aplikací je především grafický interface nad relační databází - celé jádro je v práci s tou databází a UOW tedy řeší hlavně to. Podle mě ale stále přínosy převažují nad nevýhodami, já jako velkou výhodu vidím právě to, že si UOW nemusím nikam předávat a zaplevelovat si tak svoje API, a dále to, že DbContext je skrytý, takže ho nikdo nemůže používat jen tak napřímo. Zároveň to řeší ten nesting.

nahlásit spamnahlásit spam 3 / 3 odpovědětodpovědět

Tak předně děkuji moc za odpověď, trošku jsem popravdě doufal v těch nejtajnějších představách, že mi odpovíte právě vy :-))

.... což je na webu řešení, které zkousne i background vlákna, která můžete vytvořit (i když pro cokoliv reálného je stejně nedoporučuji používat).

Jste si tímto jistý? Nebo,.. nevím ejstli to správně chápu, ale pokud vytvořím (někdo mi vytvoří nové vlákno) pomocí například TaskFactory.StartNew, tak se mu přeci implicitně nepřesype ThreadLocal storage? Je možné to snad nějak globálně ovlivnit?

Špatně. Jestli je něco antipattern, tak je to používání UOW v prezentační vrstvě, kterou controller nepochybně je. Controller má posbírat data z UI, zavolat funkci ve fasádě, která vrátí výsledky, a ty controller nějak aplikuje do UI.

Zde jsem se nad tímto hodně zamýšlel.. Jako příklad (možná špatný) si vzal WebAPI a rest, který by měl být plně bezstavový, tedy neměla by být "transakce" mezi více requesty.. jinými slovy 1 request = 1 transakce (byznys transakce). Pokud tedy takto konkrétní request definuje, že chci udělat konkrétní věc, proč nemůže reques celý definovat, že chci udělat nějakou věc zaráz? Vím moc dobře, že se transakce (je jedno jestli uow, nebo jiná) definuje na fasádě.. ale pokud si vezmu 3 ruzne requesty:

1) Otevřít krabičku

2) Zavřít krabičku

3) Otevřít krabičku, sníst bonbon a zavřít krabičku

Neměla by být správně definována byznys transakce v samotné akci controlleru? Tj.

1) uow { otevriKrabicku() }

2) uow { zavriKrabicku() }

3) uow { otevriKrabicku() snezBonbon() zavriKrabicku() } ? To jen taková myšlenka k debatě...

Používat objekty, které nejsou thread safe (což je třeba příklad DbContextu, UserStore nebo dalších věcí, které na nich silně závisí, např. i těch repository), by se také nemělo. Do jiného vlákna byste entity ani kontext neměl předávat nikdy, a u async/await musíte hlídat, jak to konkrétní metoda interně dělá...

Tomu rozumím.. jsem si vědom toho, že samotný DBContext není threadsafe a proto (mimo jiné) musí být u každé async operace ihned await. Pokud se kouknu na to, co dělá uvnitř identity framework, konkretne napr. v extension metode:

public static IdentityResult Update<TUser, TKey>(this UserManager<TUser, TKey> manager, TUser user) where TUser : class, IUser<TKey> where TKey : IEquatable<TKey>

Tak zjistím, že on z metody, která je původně async udělá synchronní poněkud zajímavým způsobem:

return AsyncHelper.RunSync<IdentityResult>((Func<Task<IdentityResult>>) (() => manager.UpdateAsync(user)));

A zde dojde k onomu nestastnemu ztraceni contextu.. at už HttpContextu, nebo LocalThread storage..

Ohledně toho UserStore - já to řeším tak, že jsem si jej podědil a DbContext mu předávám přímo v konstruktoru v době, kdy vzniká (volám tam unitOfWorkProvider.GetCurrent().Context), a UserManager do fasád injektuji vždy přes Func<UserManager>, takže mám pak kontrolu nad tím, kdy vzniká store a vím, že dostane správný kontext (vyrobím si instanci user manageru až uvnitř unit of work). A ještě je dobré tomu store říct, aby po disposnutí context nechalo být a nedisposovalo ho, je tam na to nějaká property.

Toto mě také napadlo, že tomuto storu (mmožná by stálo za to to udělat pro všechny) "dám" context hned v constructoru a nebudu ho getovat pomocí getteru, který si o to říká z UOW.

Ohledně toho vytváření ve fasádě toho usermanageru.. já používám na authorizaci AuthorizationServer provider a ten se vytvoří pouze jednou při startu serveru (definováno v AppStart), takže tam musím jednotlivé "manažery" dopravit také pomocí nějaké factorky.

Píšete, že si vytváříte instanci user manageru až uvnitř unity of work.. já vnímám user manager (mám ho poděděný) už jako konkrétní app servicu (tuším, že tomu říkáte UI facade), tedy bych měl správně asi UOW zakládat v každé metodě toho UserManageru, z čehož vyplývá že bych musel "opatchovat" každou metodu toho UserManagera, nebo použít nějaký method interception a to se mi zrovna asi moc nechce.. Takže to teď řeším tak, že AuthorizationServerProvider a jeho metody - tam vytvářím UOW.

Bohužel uvnitř se volá UserManager a ten tam dělá ty triky s vlákny, takže se tam vytratí ten http/thread context.. verim ale, ze to getnutí si DBContextu už v constructoru, by mohlo vyřešit moje problémy.

Mně se nelíbí zaprasit si polovinu funkcí v aplikaci tím, že jako první nebo poslední parametr budou přijímat unit of work. UOW v době, kdy vznikají instance, které ji potřebují, ještě neexistuje, tudíž se nedá nijak rozumně injektovat, můžete injektovat jen toho unit of work registry, který vám ji nějak musí dát.

Zcela a plně souhlasím, to je právě důvod, proč třeba mám věci jako IUserProvider a pak implementaci ve Webovém projektu, který vrátí nějaký obecný "UserDTO", ke kterému si musí vytáhnout Identity.User. Abych si nemusel do všech servis atp, posílat current usera.

------------------------------------------------------

------------------------------------------------------

Pokud se ještě můžu na něco zeptat k té prezentaci ... spíše si myslím, že by vás mohly zajímat moje otázky...

Viděl jsem, že používáte QueryObjecty, ale ty jsou přímo natvrdo refnuté ve fasádách,.. mám za to, že princip této abstrakce - ať už v podobě generic repozitáře, TAK PŘEDEVŠÍM query objectu (používám namísto query objectu poděděný generic repo IProjectStore -> ProjectStore, kde si nadefinuju další metody) je zakrýt konkrétní implementaci tak, abych ji v případě problémů mohl například patchnout (tedy například nahradit getování přes EF a LINQ za přímý SQL pomocí např. ExecuteCommandu, případně volání stored procedury atp..

Pokud použiji přímo ve fasádě NecoQuery, nejsem pak schopný tuto implementaci vyměnit za jinou, aniž bych musel upravit všechny fasády,.. jelikož tam nemám ten DI,.. pokud však například toto "query" umístím jako metodu do repa (které je zakryté za Interfacem), nic mi nebrání v případě problémů si třeba repository podědit a overridnout jen danou metodu.. pak změním registraci v DI a jedu dále..?

Jasně,. má to tu velkou nevýhodu, že můžu mít takto 1 dotaz X krát v různých repozitářích a je opravdu těžký správně určit kde by to asi tak mělo být (hledat správné agregátory v tabulkách), na druhou stranu, pokud budu chtít takto abstrahovat Query, tak to pro mě znamená vytvořit kromě 50ti tříd ještě 50 interfacu?

Mějte prima večer

nahlásit spamnahlásit spam 0 odpovědětodpovědět

JInak.. když jsme u těch threadu a IOC kontejneru,.. trošku mě tedy děsí některé případné problémy u "PerRequestLifetimeManager"u,.. co jsem koukal, tak vesměs všechny ukládají právě do HttpContextu a případně pak do local threadu, pokud není k dispozici

Tedy dělají to samé co v HttpContextUOWRegistry s tím fallbackem.

Hádám tedy, že pokud budu mít zaregistrovaný třeba DBContext s tímto lifetimemanagerem a budu mít dvě operace, které budou vytvořeny v novém threadu, tak přesto že ty thready budu synchronizovat ručně (třeba pomocí Thread.Join()), tak každý ten thread dostane vlastní instanci DBContextu,...

nahlásit spamnahlásit spam 0 odpovědětodpovědět

pokud vytvořím (někdo mi vytvoří nové vlákno) pomocí například TaskFactory.StartNew, tak se mu přeci implicitně nepřesype ThreadLocal storage

Ne, to jsme se špatně pochopili. Background vlákno samozřejmě tuto UOW neuvidí (což je správně, UOW by se mezi vlákny předávat neměla, DbContext stejně není thread safe atd.). Myslel jsem to tak, že background vláknu UnitOfWorkRegistry fallbackne na to ThreadLocal, takže i background vlákno bude moci vytvořit svou vlastní unit of work a nespadne to proto, že tam není HttpContext.

...WebAPI a rest, který by měl být plně bezstavový...

V tomto případě by asi fasádu šlo vypustit a prohlásit controller za fasádu k business vrstvě, pak by se tam dala UOW použít. Bohužel jsem ještě neviděl aplikaci, kde by to takhle bez výjimky fungovalo, kromě nějakého jednoduchého dema - většinou ani ten REST tu bezstavovost z praktických důvodů někde musel porušit, a ta fasáda se hodí i kdyby REST byl bezstavový, i když se může jevit jako zbytečná vrstva - časem přijde požadavek na to, že k tomu chcete ještě webový front-end, nebo jiné Web API (pro jiný druh klientů), nebo už jen to, že se API rozhodnete verzovat, a fasádu potřebujete. Takže UOW v controlleru bych viděl stejně spíš jako krátkodobé řešení.

Ad Identity Server - to nemají moc dobře udělané. Já ten produkt moc neznám, takže nevím, jestli to jde nějak rozumně vyřešit, aniž by člověk musel zasahovat do kódu.

Ad User Manager - já ho neberu jako fasádu, jen jako službu, kterou z fasád používám. Do fasády si injektuji Func<UserManager>, vytvořím unit of work a uvnitř si nechám vyrobit instanci UserManagera, kterou použiju.

Ad QueryObject - já to spíš beru tak, že pokud se jeden dotaz ukáže jako pomalý, tak ho přeimplementuju přímo v té třídě - nepotřebuju mít více implementací toho samého dotazu najednou. Proto tam ani nemám ty rozhraní - obecně je nedávám na místa, kde více implementací neočekávám. A pokud bych tam chtěl mít obě verze query, tak prostě vyrobím novou třídu a nareferencuji ji na místech, kde ji chci použít. Ale neříkám, že to je jediná správná cesta - dělám to tak, protože mi to přijde nejjednodušší.

PreRequestLifetimeManager je zrádný, já jsem jej pak v reálných projektech používal jen na fasády, kde se to hodilo (např. stránka použije dvě fasády, které mají jako dependency nějakou jednu společnou, tak aby neexistovaly pro jeden request dvě instance stejné fasády), vše ostatní bylo Transient nebo Singleton.

On je v .NETu problém, že nic lepšího než HttpContext s fallbackem na ThreadLocal asi nenajdete. Trochu se to dá vylepšit přes ten AsyncLocal, ale i tam budou určitě nějaká úskalí.

nahlásit spamnahlásit spam 3 / 3 odpovědětodpovědět

Výborně. Děkuji pěkně za odpovědi. Vážím si vás.

nahlásit spamnahlásit spam 0 odpovědětodpovědět
                       
Nadpis:
Antispam: Komu se občas házejí perly?
Příspěvek bude publikován pod identitou   anonym.
  • Administrátoři si vyhrazují právo komentáře upravovat či mazat bez udání důvodu.
    Mazány budou zejména komentáře obsahující vulgarity nebo porušující pravidla publikování.
  • Pokud nejste zaregistrováni, Vaše IP adresa bude zveřejněna. Pokud s tímto nesouhlasíte, příspěvek neodesílejte.

přihlásit pomocí externího účtu

přihlásit pomocí jména a hesla

Uživatel:
Heslo:

zapomenuté heslo

 

založit nový uživatelský účet

zaregistrujte se

 
zavřít

Nahlásit spam

Opravdu chcete tento příspěvek nahlásit pro porušování pravidel fóra?

Nahlásit Zrušit

Chyba

zavřít

feedback