zicht / messages-bundle

Library - ZichtMessagesBundle - Edit your translations in the database
MIT License
1 stars 0 forks source link

Rewrite translations loader #30

Closed erik-zicht closed 3 years ago

erik-zicht commented 3 years ago

Fix #25

Hiermee is er dus geen automatische cache-herschrijf actie meer bij een cache-clear en moeten we gaan wennen aan het gebruik van een separaat commando zicht:messages:cache

Ideeen @7ochem @boudewijn-zicht ?

boudewijn-zicht commented 3 years ago

Ik kan me herinneren dat er een check is of het cache bestand bestaat, en wanneer deze niet bestaat, dat deze gemaakt wordt. Bestaat dit mechanisme nog? Zo ja, is het dan mogelijk om het oude cache bestand te verwijderen wanneer de cache geleegds wordt (maar dit bestand niet opnieuw te genereren)?

Ik vraag dit omdat het voorheen erg handig werkte... wat raken we daar nu exact van kwijt?

*edit: het probleem van #25 is het opbouwen van de cache, dat mag niet plaatsvinden bij cache-clear, maar het automatisch opbouwen van de cache wanneer je een translation opvraagd, ik hoop dat dat nog wel blijft bestaan

7ochem commented 3 years ago

Ik verwacht dat dit veel verwarring gaat opleveren in bepaalde cases. Dat developers uren kunnen gaan zitten puzzelen terwijl ze dan gewoon het nieuw geïntroduceerde command zijn vergeten.

Ik zou graag ook zo'n zelfde werking willen als Boudewijn omschrijft. Dat tijdens de cache clear + warmup de database niet wordt aangesproken, maar dat bijv. bij een eerste page load wel de translations uit de database nog worden ingeladen. Daarbij kunnen we tijdens deploys wel via een command dit vooraf al laten doen, zodat je op productie wel alles goed klaar hebt staan.

erik-zicht commented 3 years ago

Ik kan me herinneren dat er een check is of het cache bestand bestaat

Daar kan ik niks van terugvinden, misschien werkte dat zo in een oude versie

Dat tijdens de cache clear + warmup de database niet wordt aangesproken, maar dat bijv. bij een eerste page load wel de translations uit de database nog worden ingeladen.

Dit lijkt me dan een RequestListener die alleen op debug-omgevingen aan staat. Ik weet niet of dit gaat werken als je al aan een request begonnen ben. Mogelijk werkt het dan pas bij het volgende request, maar dat is dan iets om uit te puzzelen

erik-zicht commented 3 years ago

Ik heb het idee van de RequestListener uitgewerkt in een nieuwe commit, dit lijkt ook functioneel goed te werken.

boudewijn-zicht commented 3 years ago

Heb even met @erik-zicht overlegd, en misschien kan het een stuk eenvoudiger door de https://github.com/zicht/messages-bundle/blob/release/5.x/src/Translation/Loader.php#L48 <-- hier middels de request_stack service te checken of het een http request is.

Erik gaat dat nog even checken, want dan is dat de enige aanpassing die nodig is.

erik-zicht commented 3 years ago

Ik hoopte dat er een kwartje zou vallen als ik het ging uittesten maar ik begrijp nog steeds niet waarom jouw voorstel zou kunnen werken.

Deze Loader https://github.com/zicht/messages-bundle/blob/release/5.x/src/Translation/Loader.php#L48 is onderdeel van een collectie Loaders die Symfony afspeelt om de cache op te bouwen in een cache:clear. De code van een Loader wordt nooit meer aangeroepen als de vertaling-cache eenmaal is gevuld, en die is altijd gevuld na een cache:clear.

Dit werkt alleen als je op de terminal een rm -rf var/cache/* doet en dan in je browser een refresh. Maar dat is natuurlijk geen sluitende oplossing.