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

replace http client implementation with interface #3594

Merged
merged 4 commits into from
Mar 16, 2021
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -22,7 +22,7 @@
import com.ctrip.framework.apollo.util.ExceptionUtil;
import com.ctrip.framework.apollo.util.http.HttpRequest;
import com.ctrip.framework.apollo.util.http.HttpResponse;
import com.ctrip.framework.apollo.util.http.HttpUtil;
import com.ctrip.framework.apollo.util.http.HttpClient;
import com.google.common.base.Joiner;
import com.google.common.base.Strings;
import com.google.common.collect.Lists;
@@ -33,7 +33,7 @@

public class ConfigServiceLocator {
private static final Logger logger = LoggerFactory.getLogger(ConfigServiceLocator.class);
private HttpUtil m_httpUtil;
private HttpClient m_httpClient;
private ConfigUtil m_configUtil;
private AtomicReference<List<ServiceDTO>> m_configServices;
private Type m_responseType;
@@ -49,7 +49,7 @@ public ConfigServiceLocator() {
m_configServices = new AtomicReference<>(initial);
m_responseType = new TypeToken<List<ServiceDTO>>() {
}.getType();
m_httpUtil = ApolloInjector.getInstance(HttpUtil.class);
m_httpClient = ApolloInjector.getInstance(HttpClient.class);
m_configUtil = ApolloInjector.getInstance(ConfigUtil.class);
this.m_executorService = Executors.newScheduledThreadPool(1,
ApolloThreadFactory.create("ConfigServiceLocator", true));
@@ -151,7 +151,7 @@ private synchronized void updateConfigServices() {
Transaction transaction = Tracer.newTransaction("Apollo.MetaService", "getConfigService");
transaction.addData("Url", url);
try {
HttpResponse<List<ServiceDTO>> response = m_httpUtil.doGet(request, m_responseType);
HttpResponse<List<ServiceDTO>> response = m_httpClient.doGet(request, m_responseType);
transaction.setStatus(Transaction.SUCCESS);
List<ServiceDTO> services = response.getBody();
if (services == null || services.isEmpty()) {
Original file line number Diff line number Diff line change
@@ -11,7 +11,8 @@
import com.ctrip.framework.apollo.util.ConfigUtil;
import com.ctrip.framework.apollo.util.factory.DefaultPropertiesFactory;
import com.ctrip.framework.apollo.util.factory.PropertiesFactory;
import com.ctrip.framework.apollo.util.http.HttpUtil;
import com.ctrip.framework.apollo.util.http.DefaultHttpClient;
import com.ctrip.framework.apollo.util.http.HttpClient;

import com.ctrip.framework.apollo.util.yaml.YamlParser;
import com.google.inject.AbstractModule;
@@ -60,7 +61,7 @@ protected void configure() {
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).in(Singleton.class);
bind(HttpClient.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);
Original file line number Diff line number Diff line change
@@ -18,7 +18,7 @@
import com.ctrip.framework.apollo.util.ExceptionUtil;
import com.ctrip.framework.apollo.util.http.HttpRequest;
import com.ctrip.framework.apollo.util.http.HttpResponse;
import com.ctrip.framework.apollo.util.http.HttpUtil;
import com.ctrip.framework.apollo.util.http.HttpClient;
import com.google.common.base.Joiner;
import com.google.common.base.Strings;
import com.google.common.collect.HashMultimap;
@@ -65,7 +65,7 @@ public class RemoteConfigLongPollService {
private Type m_responseType;
private static final Gson GSON = new Gson();
private ConfigUtil m_configUtil;
private HttpUtil m_httpUtil;
private HttpClient m_httpClient;
private ConfigServiceLocator m_serviceLocator;

/**
@@ -84,7 +84,7 @@ public RemoteConfigLongPollService() {
m_responseType = new TypeToken<List<ApolloConfigNotification>>() {
}.getType();
m_configUtil = ApolloInjector.getInstance(ConfigUtil.class);
m_httpUtil = ApolloInjector.getInstance(HttpUtil.class);
m_httpClient = ApolloInjector.getInstance(HttpClient.class);
m_serviceLocator = ApolloInjector.getInstance(ConfigServiceLocator.class);
m_longPollRateLimiter = RateLimiter.create(m_configUtil.getLongPollQPS());
}
@@ -171,7 +171,7 @@ private void doLongPollingRefresh(String appId, String cluster, String dataCente
transaction.addData("Url", url);

final HttpResponse<List<ApolloConfigNotification>> response =
m_httpUtil.doGet(request, m_responseType);
m_httpClient.doGet(request, m_responseType);

logger.debug("Long polling response: {}, url: {}", response.getStatusCode(), url);
if (response.getStatusCode() == 200 && response.getBody() != null) {
Original file line number Diff line number Diff line change
@@ -20,7 +20,7 @@
import com.ctrip.framework.apollo.util.ExceptionUtil;
import com.ctrip.framework.apollo.util.http.HttpRequest;
import com.ctrip.framework.apollo.util.http.HttpResponse;
import com.ctrip.framework.apollo.util.http.HttpUtil;
import com.ctrip.framework.apollo.util.http.HttpClient;
import com.google.common.base.Joiner;
import com.google.common.base.Strings;
import com.google.common.collect.Lists;
@@ -52,7 +52,7 @@ public class RemoteConfigRepository extends AbstractConfigRepository {
private static final Escaper queryParamEscaper = UrlEscapers.urlFormParameterEscaper();

private final ConfigServiceLocator m_serviceLocator;
private final HttpUtil m_httpUtil;
private final HttpClient m_httpClient;
private final ConfigUtil m_configUtil;
private final RemoteConfigLongPollService remoteConfigLongPollService;
private volatile AtomicReference<ApolloConfig> m_configCache;
@@ -79,7 +79,7 @@ public RemoteConfigRepository(String namespace) {
m_namespace = namespace;
m_configCache = new AtomicReference<>();
m_configUtil = ApolloInjector.getInstance(ConfigUtil.class);
m_httpUtil = ApolloInjector.getInstance(HttpUtil.class);
m_httpClient = ApolloInjector.getInstance(HttpClient.class);
m_serviceLocator = ApolloInjector.getInstance(ConfigServiceLocator.class);
remoteConfigLongPollService = ApolloInjector.getInstance(RemoteConfigLongPollService.class);
m_longPollServiceDto = new AtomicReference<>();
@@ -218,7 +218,7 @@ private ApolloConfig loadApolloConfig() {
transaction.addData("Url", url);
try {

HttpResponse<ApolloConfig> response = m_httpUtil.doGet(request, ApolloConfig.class);
HttpResponse<ApolloConfig> response = m_httpClient.doGet(request, ApolloConfig.class);
m_configNeedForceRefresh.set(false);
m_loadConfigFailSchedulePolicy.success();

Original file line number Diff line number Diff line change
@@ -19,14 +19,14 @@
/**
* @author Jason Song(song_s@ctrip.com)
*/
public class HttpUtil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we may still keep the class HttpUtil and delegate its implementation to the HttpClient because some application might already reference it. I did a quick search in github, and found the possibility is true.

To avoid future misuse, we could mark it as @Deprecated.

public class DefaultHttpClient implements HttpClient {
private ConfigUtil m_configUtil;
private static final Gson GSON = new Gson();

/**
* Constructor.
*/
public HttpUtil() {
public DefaultHttpClient() {
m_configUtil = ApolloInjector.getInstance(ConfigUtil.class);
}

@@ -38,6 +38,7 @@ public HttpUtil() {
* @return the response
* @throws ApolloConfigException if any error happened or response code is neither 200 nor 304
*/
@Override
public <T> HttpResponse<T> doGet(HttpRequest httpRequest, final Class<T> responseType) {
Function<String, T> convertResponse = new Function<String, T>() {
@Override
@@ -57,6 +58,7 @@ public T apply(String input) {
* @return the response
* @throws ApolloConfigException if any error happened or response code is neither 200 nor 304
*/
@Override
public <T> HttpResponse<T> doGet(HttpRequest httpRequest, final Type responseType) {
Function<String, T> convertResponse = new Function<String, T>() {
@Override
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package com.ctrip.framework.apollo.util.http;

import com.ctrip.framework.apollo.exceptions.ApolloConfigException;
import java.lang.reflect.Type;

/**
* @author Jason Song(song_s@ctrip.com)
*/
public interface HttpClient {

/**
* Do get operation for the http request.
*
* @param httpRequest the request
* @param responseType the response type
* @return the response
* @throws ApolloConfigException if any error happened or response code is neither 200 nor 304
*/
<T> HttpResponse<T> doGet(HttpRequest httpRequest, final Class<T> responseType)
throws ApolloConfigException;

/**
* Do get operation for the http request.
*
* @param httpRequest the request
* @param responseType the response type
* @return the response
* @throws ApolloConfigException if any error happened or response code is neither 200 nor 304
*/
<T> HttpResponse<T> doGet(HttpRequest httpRequest, final Type responseType)
throws ApolloConfigException;
}
Original file line number Diff line number Diff line change
@@ -20,7 +20,7 @@
import com.ctrip.framework.apollo.util.ConfigUtil;
import com.ctrip.framework.apollo.util.http.HttpRequest;
import com.ctrip.framework.apollo.util.http.HttpResponse;
import com.ctrip.framework.apollo.util.http.HttpUtil;
import com.ctrip.framework.apollo.util.http.HttpClient;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Lists;
import com.google.common.net.HttpHeaders;
@@ -51,7 +51,7 @@ public class RemoteConfigLongPollServiceTest {
@Mock
private HttpResponse<List<ApolloConfigNotification>> pollResponse;
@Mock
private HttpUtil httpUtil;
private HttpClient httpClient;
@Mock
private ConfigServiceLocator configServiceLocator;
private Type responseType;
@@ -63,7 +63,7 @@ public class RemoteConfigLongPollServiceTest {

@Before
public void setUp() throws Exception {
MockInjector.setInstance(HttpUtil.class, httpUtil);
MockInjector.setInstance(HttpClient.class, httpClient);

someServerUrl = "http://someServer";
ServiceDTO serviceDTO = mock(ServiceDTO.class);
@@ -114,7 +114,7 @@ public HttpResponse<List<ApolloConfigNotification>> answer(InvocationOnMock invo
longPollFinished.set(true);
return pollResponse;
}
}).when(httpUtil).doGet(any(HttpRequest.class), eq(responseType));
}).when(httpClient).doGet(any(HttpRequest.class), eq(responseType));

remoteConfigLongPollService.submit(someNamespace, someRepository);

@@ -156,7 +156,7 @@ public HttpResponse<List<ApolloConfigNotification>> answer(InvocationOnMock invo

return pollResponse;
}
}).when(httpUtil).doGet(any(HttpRequest.class), eq(responseType));
}).when(httpClient).doGet(any(HttpRequest.class), eq(responseType));

final SettableFuture<Boolean> onNotified = SettableFuture.create();
doAnswer(new Answer<Void>() {
@@ -218,7 +218,7 @@ public HttpResponse<List<ApolloConfigNotification>> answer(InvocationOnMock invo

return pollResponse;
}
}).when(httpUtil).doGet(any(HttpRequest.class), eq(responseType));
}).when(httpClient).doGet(any(HttpRequest.class), eq(responseType));

final SettableFuture<Boolean> onNotified = SettableFuture.create();
doAnswer(new Answer<Void>() {
@@ -289,7 +289,7 @@ public HttpResponse<List<ApolloConfigNotification>> answer(InvocationOnMock invo

return pollResponse;
}
}).when(httpUtil).doGet(any(HttpRequest.class), eq(responseType));
}).when(httpClient).doGet(any(HttpRequest.class), eq(responseType));

final SettableFuture<Boolean> onAnotherRepositoryNotified = SettableFuture.create();
doAnswer(new Answer<Void>() {
@@ -352,7 +352,7 @@ public HttpResponse<List<ApolloConfigNotification>> answer(InvocationOnMock invo

return pollResponse;
}
}).when(httpUtil).doGet(any(HttpRequest.class), eq(responseType));
}).when(httpClient).doGet(any(HttpRequest.class), eq(responseType));

final SettableFuture<Boolean> someRepositoryNotified = SettableFuture.create();
doAnswer(new Answer<Void>() {
@@ -421,7 +421,7 @@ public HttpResponse<List<ApolloConfigNotification>> answer(InvocationOnMock invo

return pollResponse;
}
}).when(httpUtil).doGet(any(HttpRequest.class), eq(responseType));
}).when(httpClient).doGet(any(HttpRequest.class), eq(responseType));

final SettableFuture<Boolean> onNotified = SettableFuture.create();
doAnswer(new Answer<Void>() {
Original file line number Diff line number Diff line change
@@ -28,7 +28,7 @@
import com.ctrip.framework.apollo.util.factory.PropertiesFactory;
import com.ctrip.framework.apollo.util.http.HttpRequest;
import com.ctrip.framework.apollo.util.http.HttpResponse;
import com.ctrip.framework.apollo.util.http.HttpUtil;
import com.ctrip.framework.apollo.util.http.HttpClient;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
@@ -63,7 +63,7 @@ public class RemoteConfigRepositoryTest {
private String someNamespace;
private String someServerUrl;
private ConfigUtil configUtil;
private HttpUtil httpUtil;
private HttpClient httpClient;
@Mock
private static HttpResponse<ApolloConfig> someResponse;
@Mock
@@ -93,8 +93,8 @@ public void setUp() throws Exception {
when(configServiceLocator.getConfigServices()).thenReturn(Lists.newArrayList(serviceDTO));
MockInjector.setInstance(ConfigServiceLocator.class, configServiceLocator);

httpUtil = spy(new MockHttpUtil());
MockInjector.setInstance(HttpUtil.class, httpUtil);
httpClient = spy(new MockHttpClient());
MockInjector.setInstance(HttpClient.class, httpClient);

remoteConfigLongPollService = new RemoteConfigLongPollService();

@@ -191,7 +191,7 @@ public HttpResponse<ApolloConfig> answer(InvocationOnMock invocation) throws Thr

return someResponse;
}
}).when(httpUtil).doGet(any(HttpRequest.class), any(Class.class));
}).when(httpClient).doGet(any(HttpRequest.class), any(Class.class));

RemoteConfigRepository remoteConfigRepository = new RemoteConfigRepository(someNamespace);

@@ -304,7 +304,7 @@ public Void answer(InvocationOnMock invocation) throws Throwable {

final ArgumentCaptor<HttpRequest> httpRequestArgumentCaptor = ArgumentCaptor
.forClass(HttpRequest.class);
verify(httpUtil, atLeast(2)).doGet(httpRequestArgumentCaptor.capture(), eq(ApolloConfig.class));
verify(httpClient, atLeast(2)).doGet(httpRequestArgumentCaptor.capture(), eq(ApolloConfig.class));

HttpRequest request = httpRequestArgumentCaptor.getValue();

@@ -407,7 +407,7 @@ public long getLongPollingInitialDelayInMills() {
}
}

public static class MockHttpUtil extends HttpUtil {
public static class MockHttpClient implements HttpClient {

@Override
public <T> HttpResponse<T> doGet(HttpRequest httpRequest, Class<T> responseType) {