Přidání resolveru na relativní cesty#8
Conversation
ghost
requested review from
jakubenglicky
left a comment
jakubenglicky
left a comment
There was a problem hiding this comment.
Za mě ok, jen ten jeden komentář, tam si popravdě nejsem jistý 😄 Ale asi bych ocenil, kdyby se na to podíval ještě například @MilanPala, který do toho vidí asi nejvíc.
|
|
||
| private function isAbsoluteUrl(\Nette\Http\Url $url): bool | ||
| { | ||
| return $url->getHost() && ( ! $this->request || $url->getHost() !== $this->request->getUrl()->getHost()); |
There was a problem hiding this comment.
Není tady možnost, že dojde na volání nad NULL ? Je tady nebo, takže zkusí obě strany a když bude request NULL nic mu nezabrání, aby zkusil to getUrl nebo se pletu?
There was a problem hiding this comment.
toto je utržek z původního kódu ... chtěl jsem to nějak refaktorovat, ale trošku mi z toho explodovala hlava ... ale asi v překladu to má být ... existuje host A (exituje request nebo host není stejný jaho host requestu) ... 🤔 asi:
if ( ! $url->getHost()) {
return FALSE;
}
if ($this->request && $url->getHost() === $this->request->getUrl()->getHost()) {
return TRUE;
}
return FALSE;
to je asi lepší co?
|
Vím o tom, projdu si to během dneška |
MilanPala
left a comment
MilanPala
left a comment
There was a problem hiding this comment.
Refaktoring v pořádku. Jemně bych upravil názvosloví, v tom jsem se dlouho ztrácel, myslel jsem nejdřív, že se to chová jinak.
|
|
||
| namespace Pd\Version\Resolvers; | ||
|
|
||
| class AbsolutePathResolver extends \Pd\Version\Resolvers\AbstractPathResolver |
There was a problem hiding this comment.
Přejmenoval bych na AbsoluteUrl, ať je konzistentní s pojmenováním v $url->getAbsoluteUrl(). Nejdříve jsem myslel, že jde o absolutní cestu na disku.
|
|
||
| namespace Pd\Version\Resolvers; | ||
|
|
||
| class RelativePathResolver extends \Pd\Version\Resolvers\AbstractPathResolver |
There was a problem hiding this comment.
Protože relativní cesta je od adresáře a absolutní je začínající lomítkem, toto bych přejmenoval na PathResolver, protože je jedno, jakou cestu hledáme. Z domény se bere pouze path a jde o to, jaký bude slepenec předaného adresáře a cesty z domény. Ve výsledku cesta z domény může být absolutní vzhledem k umístění na disku, adresář nemusí být žádný.
|
@petrgala Prosím tedy o úpravu pojmenování, pokud v tom nevidíš nějaký rozpor. Potom můžeš merge do masteru. Akorát master je pro Nette 3, existuje i větev |
2 participants
Dekompozice třídy
\Pd\Version\Filter:AbsoluteUrlResolveraPathResolverRelativePathGetter, který lze projektově přetížit a upravit mapování cestKaždý z Resolverů řeší vlastní cesotu zpracování cesty a cache. Tudíž balíček vyhovuje multiwebovému použití.
Výpočet verze nefungoval multiwebově, což je záležitost projektu. Do balíčku byla přidána možnost getteru na cestu, která lze překrýt na projektu. Getter vrací originální cestu, problém je v cache, takže musela být provedena dekompozice.
Na projektu vyzkoušeno: https://github.com/peckadesign/Gant2017/pull/3250