Nette\Context – skutečně je současná implementace to pravé ořechové?

Upozornění: Tohle vlákno je hodně staré a informace nemusí být platné pro současné Nette.
pekelnik
Člen | 462
+
0
-

Nette\Context – co dál?

Následující text je dlouhý, nedává příliš smysl ale snažil jsem se aby nebyl emotivní :P

První co mě mate je to klonování $contextu, byl bych moc rád kdyby se k němu vyjádřil David…

Myslel jsem že Nette\Context se zavádí kvůli tomu aby se zamezilo „ošklivému“ globálnímu volání Environment::getService() a spol.
Zdá se mi ale, že

Environment::getContext()->getService('Nette\Application\Application')->getContext()->getService('Nette\Application\IRouter')`

je ještě větší úlet než globální služby… resp. jaký je rozdíl mezi globálním ServiceLocatorem v Environment a o-nic-méně globálním Context v Application?


Myslel jsem si, že Nette\Context by měl fungovat tak, že se do něj zaregistrují služby a tento $context se potom poskytuje třídám které ho potřebují.
Čekal bych že se $context tedy dostane do $presenteru a potažmo do komponent. Tak to ale není.


Context je globální pro celou aplikaci. Slyšel jsem i jiná vyjádření: „Kontext je specifický pro každou instanci aplikace“. K tomu se dá dodat opravdu jenom: „A kolik těch instancí existuje?“
Kontext v současné podobě je něco mězi globálním service lokátorem a service kontejnerem.


Pokud chci tedy například v aplikaci spřístupnit Doctrine\ORM\EntityManager:

Dříve fungovalo:

// Create EntityManager
$em = Doctrine\ORM\EntityManager::create($connection, $config);
// Register EntityManager as a service
Environment::getContext()->addService('Doctrine\ORM\EntityManager', $em);
Environment::setServiceAlias('Doctrine\ORM\EntityManager', 'EntityManager');

a pro získání:

Environment::getEntityManager();

Nyní to bude takhle: (do globálního Environment::$context to přidat nemůžete protože taková věc jako je EntityManager není moc kompatibliní s clone)

$app = Environment::getApplication();
// $app->context vznikne v Configurator::createApplication() klonováním Environment::getContext()
$app->context->addService('Doctrine\ORM\EntityManager');

Service alias tak jako v Environment::setServiceAlias() nastavit nelze (?!) – takže pro získání asi:

Environment::getApplication()->getContext()->getService('Doctrine\ORM\EntityManager')

Vzhledem k tomu že 90+ procent kódu se nachází v presenterech a komponentách je takovýto kontext na úrovni aplikace celkem na draka…

Můžeme si udělat zkratku v Presenteru:

protected function getEntityManager()
{
	Environment::getApplication()->getContext()->getService('Doctrine\ORM\EntityManager');
}

Ale jak dál? Co v komponentě? Přijde vám to taky divné?

Navíc mi nepřijde že bychom se nějak zbavovali závislosti na statickém Environment :)

Co vlastně brání tomu aby Environment byla normální třída: $env = new Environment();?


Čekal bych že se kontext injektuje do presenteru při jeho vytváření, podobně u komponent…


Služby jsou prostě globální – od toho jsou to služby a ne lokální objekty.


  • ještě jsou tady už známé problémy, jako že nefungují custom továrny, nastavení se ignoruje
  • ten kdo používal vlastní Router, Application nebo podobné třídy se po upgradu mohl jí pověsit…
  • spolu s nakopnutým debugem

Hm… je to celkem dlouhý… a nedává to smysl… ale musel jsem to napsat… jsem zvědavý co se dozvím nového…

No a na závěr trocha poučného (přitom vtipného) čtení: http://googletesting.blogspot.com/…le-code.html

Editoval pekelnik (2. 10. 2010 17:26)

Ondřej Mirtes
Člen | 1536
+
0
-

Řekl bych, že aktuální implementace není ještě hotová a ve výsledku by to mělo fungovat opravdu tak, že kontext bude přístupný v jednotlivých třídách jako $this->getContext(), tedy že bude probublávat.

_Martin_
Generous Backer | 679
+
0
-

Já jsem z předchozí diskuze nabyl dojmu, že tato část Nette ještě není v konečné podobě. Možná je teda předčasné kritizovat současné pojetí kontextu jako by šlo o hotovou a neměnnou věc.

Honza Marek
Člen | 1664
+
0
-

Pravda, je to dlouhé a zmatené.

pekelnik napsal(a):

Navíc mi nepřijde že bychom se nějak zbavovali závislosti na statickém Environment :)

Ovšem toto je stěžejní bod. Podle mě, pokud má Nette\Context dávat smysl, třída Environment nesmí existovat.

pekelnik
Člen | 462
+
0
-

@martin: Chci se dovědět co nevím a vznáším své – doufám že podnětné – připomínky. pokud se jedná o kritiku, doufám že konstruktivní :)

@honza: Třída Environment může existovat, ale jako normální (mockovatelná) třída.

PS: že to není hotové doufám :) právě pro to o tom mluvím… kdybych jen na chvíli uvěřil, že je to finální verze asi přejdu na brainfuck :D

Editoval pekelnik (2. 10. 2010 15:05)

arron
Člen | 464
+
0
-

pekelnik napsal(a):

Tak precejenom nejsem sam, akorat jsem asi svoje myslenky v jinem vlakne nenapsal dostatecne srozumitelne. A je to presne duvod, proc jsem zamrznul na poslednim commitu pred touto zmenou a cekam, co bude dal:-)

Editoval arron (2. 10. 2010 15:57)

pekelnik
Člen | 462
+
0
-

Až to bude hotové dojde ke zrušení bootstrap.php!

Všechno se zkonfiguruje v konfiguračním souboru:

…no a loader.php spustí továrnu createEnvironment()

:D

Majkl578
Moderator | 1364
+
0
-

Já jsem teda ještě nepřišel na nějaké zásadní výhody, navede mě někdo správným směrem?

Ondřej Mirtes napsal(a):

Řekl bych, že aktuální implementace není ještě hotová a ve výsledku by to mělo fungovat opravdu tak, že kontext bude přístupný v jednotlivých třídách jako $this->getContext(), tedy že bude probublávat.

Pokud je to tak jak říkáš, vývoj téhle feature měl probíhat ve vlastním branchi, ne to dát spíš nefunkční než funkční přímo do master větve.

David Grudl
Nette Core | 8129
+
0
-

:))

To, že jsem ted pár dní nic nekomitnul (školení), neznamená, že verze dva je hotová, stabilní a kompletní. Ani master branch není místem, kam patří jen stabilní kompletní hotové verze.

Majkl578
Moderator | 1364
+
0
-

David Grudl napsal(a):

Ani master branch není místem, kam patří jen stabilní kompletní hotové verze.

To neříkám, ale na druhou stranu by tam nemusely být věci nefunkční, ne?

David Grudl
Nette Core | 8129
+
0
-

To si možná jen špatně vykládáš smysl master branche a night build verzí.

Patrik Votoček
Člen | 2221
+
0
-

Taky si myslím proto bych byl pro aby se v masteru objevilo všechno rozpracované co se „syslí“ a podílelo se na „odlaďování“ více lidí. (víc hlav víc ví) :-p (nicméně chápu i to proč to tam není)

paranoiq
Člen | 392
+
0
-

David Grudl napsal(a):

To si možná jen špatně vykládáš smysl master branche a night build verzí.

také si myslím, že smysl master větve by mohl být jiný, než jak je provozována v případě Nette. mě zaujal tenhle přístup: http://nvie.com/…ching-model/

pekelnik
Člen | 462
+
0
-

Ha dneska mi to docvaklo: Environment je ve skutečnosti Context :)

$env = new Environment();
//...
$app = new App($env);
paranoiq
Člen | 392
+
0
-

ano. vlastně by se celý Environment dal zredukovat na instanci Contextu v jediné globální proměnné (viz JavaScriptové frameworky). globální proměnná není o moc horší než ‚globální‘ statická třída. na testování naopak ještě lepší – statickou třídu nelze mockovat

Ped
Člen | 64
+
0
-

Rad bych upozornil ze po poslednich zmenach (pouzivani Closures) je problem se serializaci. Proc to spominam prave tady?

Protoze vzorovy skeleton obsahuje krome jineho:
$application = Environment::getApplication();

V teto chvili (netestoval jsem konkretne u skeletonu, ale ve vetsi aplikaci se mi to stalo) $application obsahuje jiz nejake Closures (pravdepodobne v contextu), co cini $GLOBALS neserializovatelnymi.

Co je problem s PHPUnit-em.

Aktualne jsem to vyresil tim ze na konci bootstrapu delam unset( $application ); aby nestrasil v $GLOBALS, na druhou stranu tim prichazim o backup/restore stavu Aplikace, t.j. ztracim zaruku ze kazdy unit test ma stejne vychozi podminky.

Narvani kontextu natvrdo do globalni promenne a nevyreseni jeho serializace by bylo primo smrtelne pro unit testing (nebo by se muselo vypnout backupovani GLOBALS, co mi prijde malo robustni reseni).

Jinak muj osobni pohled na vec je (jsem tak trochu konzervativni typ :) ):

  1. jedna rozumne navrzena globalni promenna je lepsi nez lecjaky singleton a podobna havet
  2. Closures jsou asi nekdy sikovna vec, ale co jsem videl aktualni commity do Nette treba ve tride Environment, tak mi puvodni kod prisel snad i prehlednejsi, nechapu jakou vyhodu ma davat do Closure funkci ktera ma 3+ radky, a proc ji nevypsat osobitne bokem.

(t.j. vice mene souhlasim s Paranoiqem, jenom upozornuji ze zrovna ted si to nerozumi s PHPUnitem)

Editoval Ped (7. 10. 2010 11:27)

Honza Marek
Člen | 1664
+
0
-

U phpunitu číhá problém se serializací na každém rohu, takže je asi lepší tu serializaci vypnout (ať celkově nebo pro problémové proměnné) a při psaní testů s tím počítat.

Patrik Votoček
Člen | 2221
+
0
-

V PHPUnitu to řeším úplným vypnutím serializace…

<phpunit backupGlobals="false"
         backupStaticAttributes="true">
</phpunit>

nebo --no-globals-backup

viz: http://www.phpunit.de/…uration.html#…

Ped
Člen | 64
+
0
-

vrtak-cz napsal(a):

V PHPUnitu to řeším úplným vypnutím serializace…

ani bych nerek, spis jen obchazis…
Hlavne nevidim duvod proc Nette trochu neprepsat aby tyto casti zustali serializovatelne. Az mi ukazete nejaky dobry duvod proc je to opravdu o dost lepsi kdyz se to napise neserializovatelne, tak zmlknu, ale zatim mi to unika.

@Honza: asi chodim kolem dlouhych sten a nepotkavam moc rohu, tyhle problemy mam jen vyjimecne.

při psaní testů s tím počítat

No tak ono je lepsi s tim pocitat jiz pri psani kodu a nedelat chyby, pak se nemusi psat ani testy, takze je to lepsi tuplem!

Honza Marek
Člen | 1664
+
0
-

Ped napsal(a):

No tak ono je lepsi s tim pocitat jiz pri psani kodu a nedelat chyby, pak se nemusi psat ani testy, takze je to lepsi tuplem!

Ne, znamená to jen to, že je lepší psát testy, které nemění globální stav.

David Grudl
Nette Core | 8129
+
0
-

Spíš je otázka, proč PHPUnit chce něco serializovat. Na podobný problém se už narazilo v souvislosti s dibi, kde není možné serializovat připojení k databázi. Což ale, podobně jako closures, skutečně technicky možné není. Takže mám-li volit mezi tím, zda ustoupím PHPUnitu a přestanu používat closures a upravím dibi tak, aby naoko connection serializovalo, nebo naopak doporučím uživatelům PHPUnit serializaci vypnout, jednoznačně volím cestu druhou.

Ped
Člen | 64
+
0
-

Honza Marek napsal(a):

Ped napsal(a):

No tak ono je lepsi s tim pocitat jiz pri psani kodu a nedelat chyby, pak se nemusi psat ani testy, takze je to lepsi tuplem!

Ne, znamená to jen to, že je lepší psát testy, které nemění globální stav.

V cem se tohle lisi od tvrzeni ze je lepsi psat kod ktery nema chyby?
Uznavam ze omylem napsat kod ktery meni globalni stav je podstatne slozitejsi a vyzaduje vetsi omyl nez napsani kodu s nejakou jinou beznou chybou, presto krome statistiky vyskytu dane situace rozdil nevidim.

Pri psani testu ma clovek pocitat hlavne s tim, co tam chce testovat a o zbytek by se meli postarat jeho vyvojarske nastroje, t.j. aby testovali opravdu to co napsal a aby vychozi podminky byly jednoznacne.
Backup $GLOBALS je proste jenom dalsi stupen robustnosti, jak se priblizit idealu, netvrdim ze je zrovna extremne dulezity, ale nechci se ho vzdat zbytecne.

Urcite by bylo jeste lepsi kdyby misto zalohy $GLOBALS delali rovnou kontrolu na neocekavane zmeny (t.j. ktere programator explicitne nepovolil v testu).

@David: delaji tim zalohu aktualniho stavu promennych. Nevim jestli existuje lepsi reseni, ale mne ta serializace funguje docela slusne, zatim sem ji vypnout nemusel. I ted s nette2 to zatim dokazu obejit, staci unset($application) aby to nestrasilo v GLOBALS, ale predtim slo $application serializovat. Tak mne zajima jestli ty closures jsou opravdu takovym prinosem, mne to osobne prijde jenom jako syntax sugar bez ktereho bych se obesel, ale treba to nekde pouzivas takovym stylem, ze to bez closures nijak rozumne nejde. Az tak do toho nevidim abych tohle soudil.

paranoiq
Člen | 392
+
0
-

David Grudl napsal(a):

Spíš je otázka, proč PHPUnit chce něco serializovat.

chce serializovat, protože je navržen s chybnou myšlenkou, že samostatný proces pro každý test je příliš drahá sranda

Honza Marek
Člen | 1664
+
0
-

Ped napsal(a):

Uznavam ze omylem napsat kod ktery meni globalni stav je podstatne slozitejsi a vyzaduje vetsi omyl nez napsani kodu s nejakou jinou beznou chybou, presto krome statistiky vyskytu dane situace rozdil nevidim.

Já myslim, že to nevyžaduje omyl, ale speciální úsilí. Tzn. nastavování něčeho v Environmentu či nastavování nějakého statického atributu. A toho si všimneš…

Tak mne zajima jestli ty closures jsou opravdu takovym prinosem, mne to osobne prijde jenom jako syntax sugar bez ktereho bych se obesel

Ano, máš pravdu, všechno se dá naprogramovat i bez closures. Nicméně jejich přínos je větší než možnost mít zapnutou serializaci při testování :-P

David Grudl
Nette Core | 8129
+
0
-

V tomto pripade by bylo lepsi usilovat o napravu „chyby“, tj. lobbovat za zmenu pristupu PHPUnitu (pouziti reflexe, vice procesu atd.) nez ji prenaset na okoli.

Patrik Votoček
Člen | 2221
+
0
-

V další verzi PHPUnitu (3.6) bude serializace defaultně vypnutá viz.: http://www.slideshare.net/…practices/35 a podpora běhu ve více vláknech je taky na programu https://github.com/…on-with-make

westrem
Člen | 398
+
0
-

<OT>

Clovek si chce nieco precitat o tom ako zatial funguje Context v Nette a ake maju ludia nan nazory, no namiesto sa docita o problemoch PHPUnitu, nezisli ste tak trochu z povodnej temy threadu?

</OT>

juzna.cz
Člen | 248
+
0
-

Můžu se ještě vrátit k otázce, proč se tedy context klonuje? Je vůbec potřeba nějaký vnější (environmentální) kontext? Nebylo by jednodušší mít pouze jeden?

juzna.cz
Člen | 248
+
0
-

A ještě dotaz: proč Context dějí od FreezableObject? Nikde jsem si nevšiml, že by se mrazil. Nebo je to v plánu jej někdy v budoucnu mrazit?

Filip Procházka
Moderator | 4668
+
0
-

Podle jakýchsi zásad DI, by se měl nejpozději před run v Application zmrazit. Což ovšem nedovoluje zpětná kompatibilita.

jtousek
Člen | 951
+
0
-

Zpětná kompatibilita s čím?

Filip Procházka
Moderator | 4668
+
0
-

Netvrdím, že je to ten konkrétní důvod, ale co když si někdo přidává služby v bootstrap nebo v presenteru? to jim pak nemůžeš suše utnout přístup do contextu a v půli cesty jim ho zmrazit.

kravčo
Člen | 721
+
0
-

Ale veď aplikácia má svoj vlastný (klonovaný) kontext, nie? Ten predsa zmraziť bez obáv môžem (ale čo mi to bude platné?):

$router = $application->getRouter();
$application->getContext()->freeze();
$router[] = new Route(...);
$application->run();
Ondřej Mirtes
Člen | 1536
+
0
-

Jde o to, že třída by si sama sobě neměla být schopna měnit kontext, ve kterém je spouštěna, protože na to nemá právo (proto je zmražený), ale jen těm objektům, které sama instanciuje (v objektovém grafu jsou pod ní).

Stejně je ale Service Locator dost haprující vzor (API tříd, které ho používají, nevyjadřuje svoje závislosti – pro zjištění, čím vlastně mám Context naplnit, musím zkoumat jejich zdroják), těším se, až se do Nette dostane DI kontejner – ten se vyznačuje tím, že třída si nechává předávat všechny svoje závislosti v konstruktoru nebo metodách a většina aplikace o existenci nějakého kontejneru vůbec nemusí vědět).

Zkoumali jste už Dundeeho Diphy?

Honza Marek
Člen | 1664
+
0
-

Zkoumali a moc se mi nezamlouvá ;)

Filip Procházka
Moderator | 4668
+
0
-

Zkoumal a subjektivně mi to přijde jako krok zpět. Sice to vypadá funkčně, ale něco mi říká, že je to až moc jednoduché.

BigCharlie
Člen | 283
+
0
-

Patrik Votoček napsal(a):

V PHPUnitu to řeším úplným vypnutím serializace…

<phpunit backupGlobals="false"
         backupStaticAttributes="true">
</phpunit>

nebo --no-globals-backup

Napadá někoho, proč výše uvedené nezabere? Zkoušel jsem jak konfiguraci přes xml, tak volbu přímo z příkazové řádky a nic. Pořád hláška Serialization of ‚Closure‘ is not allowed.

Nette Framework 2.0-beta
PHPUnit 3.5.5

Ondřej Mirtes
Člen | 1536
+
0
-

A co backupStaticAttributes="false"? Nahrává se tvůj phpunit.xml?

BigCharlie
Člen | 283
+
0
-

backupStaticAttributes="false" pomohlo, díky! Taky mě to mohlo trknout…

Ještě jednou dík.