RFC - příznak m:hasMany(#inversed)

[janpecha]

Pokud v současné době potřebujeme vyjádřit obrácenou M:N vazbu (např. tag => knihy pro tabulku book_tag z quick startu), musíme uvést v příznaku m:hasMany název tabulky, jinak bude Lean Mapper dohledávat vazbu ve špatné tabulce (pro quick start v tabulce tag_book).

Napadlo mě zavést pro příznak m:hasMany nový modifikátor #inversed (podobně jako existuje modifikátor #union), který by způsobil, že v reflexi se prohodí zdrojová a cílová tabulka. Teoreticky by to nemělo způsobit žádný BC break.

Co říkáte?

[castamir]

není to jen otázkou mapperu?

[janpecha]

To sice částečně je, ale mapper počítá s tím, že do něj budou předány správné údaje. V současné chvíli to probíhá takto:

1) z názvu zdrojové entity se pomocí IMapper::getTable() získá název zdrojové tabulky (viz) 2) z názvu typu u property se pomocí IMapper::getTable() získá název cílové tabulky (viz) 3) název zdrojové a cílové tabulky se předá do IMapper::getRelationshipTable() a ta vrátí název spojovací tabulky. Výchozí implementace pracuje tak, že tyto názvy jen spojí pomocí podtržítka (viz) 4) pro volání $book->tags to správně vrátí tabulku book_tag, ale pro $tag->books to vrátí špatný název tag_book 5) v teoretické rovině je problém v metodě IMapper::getRelationshipTable() - měla by vrátit správnou tabulku, ale nevrací, prakticky je to však složitější:

• mapper nemůže korekci udělat automaticky, protože nemá informaci o tom, která tabulka je primární a která sekundární, spoléhá se na to, že mu budou předány správné parametry
• uživatel by si tedy musel upravovat mapper a pro každou takovou spojovací tabulku dopisovat podmínku if ($sourceTable === 'tag' && $targetTable === 'book') { return 'book_tag'; }
• to je trochu nepraktické (hlavně při větším množství takových vazeb) a osobně v takové chvíli raději dopíšu název té tabulky přímo do příznaku v entitě Tag ($books m:hasMany(:book_tag))
• a tuhle situaci by právě řešil ten modifikátor #inversed, který by naznačil, že se jedná o obrácenou hasMany vazbu

[castamir]

omlouvam se, jak uz 3. rok nedelam v PHP a tim padem asi s LM, tak mam problem se do toho dostat a dat ti relevantni zpetnou vazbu

[mibk]

Na jednu stranu proti tomu asi nic nemám, ale na druhou stranu Lean Mapper tady už nějakou chvíli je a zatím jsme se bez toho obešli, tak necítím nějaký silný důvod, proč to do knihovny přidávat. Počet uživatelů klesá, takže to reálně nebude snad ani nikdo používat. Jen to bude další API, které se bude muset udržovat, ideálně k němu psát dokumentace. Prostě celkově je to dost práce na to, že tahle situace nastane v jednom projektu tak jednou až dvakrát. Já jsem zastánce spíše nechat věci v co nejjednodušší podobě. Pokud přidávat nějaký zlepšovák, mělo by to řešit nějaký závažnější problém. To platí obzvláště u umírajících knihoven.

[janpecha]

Lean Mapper tady už nějakou chvíli je a zatím jsme se bez toho obešli

To je sice pravda, ale taky to neznamená, že je to tak správně. Obejít se dá totiž bez spousty věcí. Cílem tohoto drobného vylepšení je primárně to, aby aplikace obsahovaly co nejméně hardcodovaných názvů a hlavní díl práce vždy odvedl mapper. Osobně bych to hodně ocenil a hodilo by se mi to skoro u každé aplikace. Mimochodem z hodně podobného soudku je i změna navrhovaná v #77.

Pokud přidávat nějaký zlepšovák, mělo by to řešit nějaký závažnější problém. To platí obzvláště u umírajících knihoven.

Tady se asi neshodneme v pohledu, jestli se pokoušet takové "umírající" knihovny posouvat dál, nebo zcela rezignovat na jejich vývoj. I takové knihovny totiž mají uživatele, kteří nová vylepšení uvítají. Potíž je také s definicí "závažnějšího problému". Podle mě tato úprava řeší závažnější problém - za prvé čistotu kódu a za druhé ochranu uživatelů před "chybami" (viz příklad níže).

---

Napadl mě k tomu jeden příklad - částečně vychází ze situace, kterou jsem nedávno řešil. Řekněme, že potřebujeme prefixovat názvy všech tabulek. Prefix se může měnit - pro každou konkrétní instalaci může být jiný. Upravíme tedy mapper tak, aby k názvům tabulek doplňoval prefix.

V aplikaci pak máme knihy a tagy a potřebujeme volat jak $book->tags, tak i $tag->books. Pro property $tags m:hasMany v entitě Book to není problém - mapper správně vrátí prefix_book_tag. Horší je to pro entitu Tag a její property $books m:hasMany(:book_tag) - v takovém případě se mapper vůbec nedostane ke slovu a nemůže tedy do názvu tabulky doplnit prefix a uživatel v podstatě nemá možnost, jak z toho ven. Řešením by bylo právě $books m:hasMany(#inversed).

---

Jinak díky oběma za názory, dost mi pomohli.

[mibk]

Chápu, že i uživatelé umírajících knihoven uvítají nová vylepšení, Tebe ale beru jako spoluautora a nikdo jiný si na nic podobného nestěžoval. Beru to z pragmatického pohledu, že jakákoli nová vlastnost, která se do jakékoli knihovny zakomponuje, může zvýšit počet potenciálních bugů a nechtěných BC breaků. V situaci, kdy už tato knihovna přišla o 2 hlavní vývojáře, bych prostě automaticky volil spíše strategii starání se o knihovnu ve formě opravy případných nově objevených bugů.

Ale jak jsem řekl na začátku, vlastně proti tomu nic nemám. Jen je škoda, že jsme tohle neřešili na úplném začátku vývoje. Asi bychom tuhle issue mohli rozdělit do dvou bodů: jestli chceme mít možnost "prohození zdrojové a cílové tabulky", a pokud ano, jakým způsobem to chceme implementovat. Je třeba divné, že můžu v jednom zápisu vazby použít #inversed a zároveň i tabulku pojmenovat: m:hasMany(:tag_books#inversed), čímž je příznak ignorován. Bohužel mě nenapadlo nic lepšího (leda tedy pojmenovat tu vazební tabulku něco jako ~: m:hasMany(:~), jelikož ~ se v mnoha jazycích používá jako bitwise complement a mohl by tedy evokovat "obrácení". S tím ale také nejsem spokojen a navíc by to vlastně i byl BC break pro někoho, kdo tu tabulku takto opravdu pojmenoval :-D)

---

Co se týče #77, jestli tomu správně rozumím, tak toto jsem navrhoval v poznámce pod čarou již 4 a půl roku zpátky a nikdo mi na to nic moc neřekl: https://forum.dibiphp.com/cs/14592-lean-mapper-tenke-orm-nad-dibi?p=18#p115003.

[janpecha]

To je pravda, použití m:hasMany(:tag_books#inversed) by se muselo ošetřit, aby vyhazovalo nějakou LogicException.

---

77 - že se něco takového objevilo už na fóru mi úplně uniklo - to je právě nevýhoda jednoho dlouhého vlákna, kde se řeší naráz příliš věcí :-/ Osobně bych to už tehdy docela uvítal. Prozatím jsem přidal do #77 odkaz na tu diskusi na fóru.

[janpecha]

Připravil jsem pro tuhle změnu PR #125. Pokud je v anotaci uveden zároveň název tabulky (m:hasMany(:tag_books#inversed)) nebo je #inversed uvedeno u jiného typu vazby, vyhazujeme výjimku.

[janpecha]

Merged #125