Instantiate Guice bound instances on-demand

[spootsy]

Hi,

First, let me say this is a really nice library - very easy to pick up and use :)

One thing I think could be improved though, is to provide an option of instantiating C10N interfaces on-demand when retrieved through Guice. At the moment, they're all created during injector initialisation so you can't, at runtime, change the locale of the instances provided through Guice.

Here's a rough idea of the change I'm proposing to the existing C10NModule (don't have git handy so not submitted as a patch, sorry)

protected void configure() {
    Set<Class<?>> c10nTypes = ... <snip>
    for (Class<?> c10nType : c10nTypes) {
        bind(c10nType).toProvider(new C10NProvider(c10nType));
    }
}

class C10NProvider<T> implements Provider<T> {
    private final Class<T> clazz;

    C10NProvider(Class<T> clazz) {
        this.clazz = clazz;
    }

    @Override
    public T get() {
        return C10N.get(clazz);
    }
}

This way, we can change our locale at will, and it also means we can delay configuring C10N until right before we're ready to use it.

[rodionmoiseev]

Thanks for the idea. I see where you're heading, but your current solution does not let me specify the locale at instanciation time.

I guess, you could enhance it to use assisted-injection and create a generic factory, with get() and get(Locale) methods, though I am not sure it its possible. Will need to experiment a little.

[Spikhalskiy]

@spootsy I don't agree that such code should be included in c10n core. We use quite different code:

public final class I18nStarter {

    private I18nStarter() {}

    public static void start() {
        C10N.configure(new C10NConfigBase() {
            @Override
            public void configure() {
                install(new DefaultC10NAnnotations());
                bindAnnotation(Ru.class);

                setLocaleProvider(new LocaleProvider() {
                    @Override
                    public Locale getLocale() {
                        return LocaleHolder.getLocale();
                    }
                });
            }

            @Override
            protected String getConfigurationPackage() {
                return "";
            }
        });
    }
}

public final class LocaleHolder {
    public final static Locale DEFAULT_LOCALE = LocaleUtils.toLocale("ru");

    private final static Logger logger = Logger.getLogger(LocaleHolder.class);

    private static ThreadLocal<Locale> threadLocales = new ThreadLocal<Locale>();

    private LocaleHolder() {
    }

    public static void setLocale(String localeString) {
        Locale locale;
        try {
            locale = LocaleUtils.toLocale(localeString);
        } catch (IllegalArgumentException ex) {
            locale = null;
            logger.warn("Illegal Locale string format " + localeString);
        }

        if (locale == null) locale = DEFAULT_LOCALE;

        threadLocales.set(locale);
    }

    public static void setLocale(Locale locale) {
        locale = Objects.firstNonNull(locale, DEFAULT_LOCALE);
        threadLocales.set(locale);
    }

    public static void clearLocale() {
        threadLocales.remove();
    }

    public static Locale getLocale() {
        return Objects.firstNonNull(threadLocales.get(), DEFAULT_LOCALE);
    }
}

And init it on the context listener of our web app.

public class I18nServletContextListener implements ServletContextListener {

    @Override
    public void contextInitialized(ServletContextEvent sce) {
        I18nStarter.start();
    }

    @Override
    public void contextDestroyed(ServletContextEvent sce) {
    }
}

All of this could be sample in docs, but as I think - not in a core code.

[rodionmoiseev]

Just to clarify, what @Spikhalsky is pointing out. Whenever you create a proxy with C10N.get(Class), it is not bound to any specific locale (by design). So, in fact, you can do something like:

Locale.setDefault(Locale.ENGLISH);
Messages msg = C10N.get(Message.class);
System.out.println(msg.greeting());

Locale.setDefault(Locale.RUSSIAN);
System.out.println(msg.greeting());

and you will see greetings in two different languages.

However, this is not the case if you use C10N.get(Class, Locale) API (new in 1.1 release).

The way I see @spootsy suggestion useful is if it allows us to inject a factory like:

interface C10NFactory<T>{
  T get(Locale);
}

So if you want to dynamically change locale in your app (not necessarily using a Servlets API, of course), you would just inject the factory and get the proxy for the locale you want.

[spootsy]

@rodionmoiseev Ah, sorry, I didn't realise this was the case. The issue I was having with the original C10N Guice module (and the reason I didn't notice this "unbound proxy" functionality) is that I was initialising the injector prior to C10N. This meant the proxies were created with an unconfigured C10N, and so they always returned default values, no matter what I returned from the locale provider.

Just another case of not RTFM properly :)

You may well already have a good reason for not doing so, but perhaps C10N could throw an exception if you request a proxy instance before calling "C10N.configure()"?

[rodionmoiseev]

No probs. I'll see if I can make it more clear in the manual.

Actually, I have had several reports where calling configure in the wrong order was causing the problem. I will take your advice and make it throw an exception. Just created a new issue #18 for that.