OrchardCMS / Orchard

Orchard is a free, open source, community-focused Content Management System built on the ASP.NET MVC platform.
https://orchardproject.net
BSD 3-Clause "New" or "Revised" License
2.37k stars 1.12k forks source link

Back LocalizationPart's LazyFields with Infoset #8692

Closed MatteoPiovanelli-Laser closed 12 months ago

MatteoPiovanelli-Laser commented 1 year ago

The LazyFields in LocalizationPart are "backed" by information in the LocalizationPartRecord. In the Loader and Setter for those LazyFields we are currently referring directly to properties of the Record. https://github.com/OrchardCMS/Orchard/blob/610b3c4f53ce9a4bdbe26a5a095bd606bfabc811/src/Orchard.Web/Modules/Orchard.Localization/Handlers/LocalizationPartHandler.cs#L32 As a consequence, whenever there is a ContentItem that has a LocalizationPart and we need to access one of those properties, a query has to be performed. This happens often when displaying multi-language websites that rely on localized content and widgets: we easily end up having 10+ queries for that to visualize a simple page.

It seems trivial to use Store/Retrieve methods in the Loader and Setter for those LazyFields. I have a PR coming soon for this. I've tested it a little, and it seems to work for all cases I've put it through, but localization is a complex topic so I'm not yet 100% sure I've covered all corner cases.

MatteoPiovanelli-Laser commented 1 year ago

For example, I worry about import/export scenarios

sebastienros commented 1 year ago

I don't recall any reason it's not in the infoset.

sebastienros commented 1 year ago

Maybe run the specflow tests (based on Benedek's changes)

MatteoPiovanelli-Laser commented 1 year ago

Leaving this open for the results of the tests

MatteoPiovanelli-Laser commented 12 months ago

Specflows seem fine. To be fair, I'm not sure they involve localization. I'm closing this issue anyway.