Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

apollo client shared object context #3573

Closed

Conversation

vdiskg
Copy link
Contributor

@vdiskg vdiskg commented Feb 26, 2021

What's the purpose of this PR

  1. add a shared object context as a common way to customize apollo client
  2. customize http client by the shared object context

@nobodyiam
Copy link
Member

This is a good try. However, I've been thinking whether we could provide a more general solution so that users could customize other instances as well.

Here is one of my thought, please take a look and see if this works.

  1. Define an interface ApolloInjectorCustomizer like below:
package com.ctrip.framework.apollo.spi;

import com.ctrip.framework.apollo.core.spi.Ordered;
import com.ctrip.framework.apollo.internals.Injector;

public interface ApolloInjectorCustomizer extends Injector, Ordered {

}
  1. Modify the implementation of DefaultInjector as below:
public class DefaultInjector implements Injector {
  private com.google.inject.Injector m_injector;
  private List<ApolloInjectorCustomizer> customizers;

  public DefaultInjector() {
    try {
      m_injector = Guice.createInjector(new ApolloModule());
      customizers = ServiceBootstrap.loadAllOrdered(ApolloInjectorCustomizer.class);
    } catch (Throwable ex) {
      ApolloConfigException exception = new ApolloConfigException("Unable to initialize Guice Injector!", ex);
      Tracer.logError(exception);
      throw exception;
    }
  }

  @Override
  public <T> T getInstance(Class<T> clazz) {
    try {
      for (ApolloInjectorCustomizer customizer : customizers) {
        T instance = customizer.getInstance(clazz);
        if (instance != null) {
          return instance;
        }
      }
      return m_injector.getInstance(clazz);
    } catch (Throwable ex) {
      Tracer.logError(ex);
      throw new ApolloConfigException(
          String.format("Unable to load instance for %s!", clazz.getName()), ex);
    }
  }

  @Override
  public <T> T getInstance(Class<T> clazz, String name) {
    for (ApolloInjectorCustomizer customizer : customizers) {
      T instance = customizer.getInstance(clazz, name);
      if (instance != null) {
        return instance;
      }
    }
    //Guice does not support get instance by type and name
    return null;
  }

  private static class ApolloModule extends AbstractModule {
    @Override
    protected void configure() {
      bind(ConfigManager.class).to(DefaultConfigManager.class).in(Singleton.class);
      bind(ConfigFactoryManager.class).to(DefaultConfigFactoryManager.class).in(Singleton.class);
      bind(ConfigRegistry.class).to(DefaultConfigRegistry.class).in(Singleton.class);
      bind(ConfigFactory.class).to(DefaultConfigFactory.class).in(Singleton.class);
      bind(ConfigUtil.class).in(Singleton.class);
      bind(HttpUtil.class).to(DefaultHttpClient.class).in(Singleton.class);
      bind(ConfigServiceLocator.class).in(Singleton.class);
      bind(RemoteConfigLongPollService.class).in(Singleton.class);
      bind(YamlParser.class).in(Singleton.class);
      bind(PropertiesFactory.class).to(DefaultPropertiesFactory.class).in(Singleton.class);
    }
  }
}

we need to change the com.ctrip.framework.foundation.internals.ServiceBootstrap#loadAllOrdered to allow empty list

  1. Users can now offer their customized solution as below
public class SomeCustomizer implements ApolloInjectorCustomizer {

  @Override
  public <T> T getInstance(Class<T> clazz) {
    // do some custom logic here
    return null;
  }

  @Override
  public <T> T getInstance(Class<T> clazz, String name) {
    return null;
  }

  @Override
  public int getOrder() {
    return 0;
  }
}

@vdiskg vdiskg closed this Mar 10, 2021
@nobodyiam
Copy link
Member

@vdisk-group what's your idea on this? I think it's quite useful to have this feature.

@vdiskg
Copy link
Contributor Author

vdiskg commented Mar 11, 2021

@vdisk-group what's your idea on this? I think it's quite useful to have this feature.

It looks great, I hope it will be available on the next version

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants