leonelquinteros / gotext

Go (Golang) GNU gettext utilities package
Other
434 stars 58 forks source link

Refactoring of gettext/locale constructs #18

Closed Dexus closed 6 years ago

Dexus commented 6 years ago

Please describe your issue

Is this a bug, an improvement, a proposal or something else?

Unfortunately it is not so ideal for a multilingual web application if the files are reloaded for each request, but it should be possible to use a cache for each locale, and with an option (devel) and function to grant the possibility of rebuild.

Therefore I consider it necessary to revise the modules.

Since I notice that it is delivered with 50k simultaneous requests with some the wrong language. Rather, it should be possible to determine the locale for each request and access it in this request, instead of a global "memory".

Regards, Josef

leonelquinteros commented 6 years ago

@Dexus So you mean "something" like the package does for the globalConfig global status, but for Locale objects, right?

The globalConfig was a necessary evil to allow the implementation of the "package level" functions, so simple usage of the package is allowed and the preload of the translation files was handled by the package. Anybody looking to use the package for simple use cases can rely on the package level functions to access the translations and these will not be reloaded on every request.

Now to your point, I agree that having to parse the translation files on every request isn't the best approach, but I think the Locale object caching should be handled by each implementation. So, if somebody wants to encode to gob and save the Locale object on a Redis instance it should be OK. Then somebody else would like to (for some exotic reason) encode the Locale object to JSON and save that into MongoDB, and it should be OK too.

That said, my examples aren't possible as the objects are defined now, the private fields won't make it to any encoding method, and that's something I'd like to work on.

There are 3 main questions I'd like to discuss here:

  1. Do we need to handle caching of Locale/Po objects or just allow the correct serialization of them? My vote is to avoid caching mechanisms in the package and go for an easy way to enable caching for the package users.
  2. Should we implement encoder/decoder interfaces for common serialization methods?
  3. Would be easier to just make content properties exported (instead of private) so any serialization works out of the box and at the same time we'll allow users to fine tune their usage and handle content as they want?

2 and 3 aren't exclusive and can be both, but I'd like to think deeper before move forward.

Dexus commented 6 years ago

I already built something there, only I haven't put it into our repo yet...

A simple, if not nice variant for the problem is, you can write several methods for the object and call something like LoadLocale(locale) per request and pass the Locale through.

I'll push something into our repo the next few days and then you can take a look at it and give your opinion.

leonelquinteros commented 6 years ago

Sounds good! I'll look forward for it.

Have a nice weekend!

Dexus commented 6 years ago

I mean a cache for the locale and po/mo files, so that it not need to load every change of the locale(language) or domain.

Another thing is the module should have more like GNU Gettext Named Functions and not like now GetC... which are very uncool if you have worked long times with gettext.

Currently the gettext file dont cache locale when you change somthing that called the "force" on the loadData(force bool) function, so this is a lumps of scrap to use productively.

Where the translations come from should not matter to us and we should offer the possibility to extend them, for example there are also JSON, XML etc. which could be supported if the interface is the same.

It has nothing to do with directly caching and creating files or serialization.

leonelquinteros commented 6 years ago

I mean a cache for the locale and po/mo files, so that it not need to load every change of the locale(language) or domain.

Sorry, I'm not sure i'm following you, can you provide an example?

Another thing is the module should have more like GNU Gettext Named Functions and not like now GetC... which are very uncool if you have worked long times with gettext.

Well, the package names are shorter than, say, Gettext, NGettext, DCNGettect and I'm OK with the current approach. Also, if we're heading towards allowing different (non-gettext) translation storages, we should keep the breach between the function names.

Where the translations come from should not matter to us and we should offer the possibility to extend them, for example there are also JSON, XML etc. which could be supported if the interface is the same.

I'm OK with this and it should be addressed in the Translator interface proposed on the #19 reviews. As long as "non-gettext" implementation stay outside the scope of this package, I agree on this one.

Dexus commented 6 years ago

I mean a cache for the locale and po/mo files, so that it not need to load every change of the locale(language) or domain.

Sorry, I'm not sure i'm following you, can you provide an example?


// loadStorage creates a new Locale object at package level based on the Global variables settings.
// It's called automatically when trying to use Get or GetD methods.
func loadStorage(force bool) {
globalConfig.Lock()
if globalConfig.storage == nil || force {
    globalConfig.storage = NewLocale(globalConfig.library, globalConfig.language)
}

if _, ok := globalConfig.storage.domains[globalConfig.domain]; !ok || force {
    globalConfig.storage.AddDomain(globalConfig.domain)
}

globalConfig.Unlock()

}

leonelquinteros commented 6 years ago

Closing this one in favor of https://github.com/leonelquinteros/gotext/issues/23