You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@servicecomb.apache.org by GitBox <gi...@apache.org> on 2018/03/30 08:34:47 UTC
[GitHub] liubao68 closed pull request #626: [SCB-447] Optimize SPIServiceUtils
liubao68 closed pull request #626: [SCB-447] Optimize SPIServiceUtils
URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/626
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/TestAbstractRestInvocation.java b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/TestAbstractRestInvocation.java
index 363bfab9f..72d0b1344 100644
--- a/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/TestAbstractRestInvocation.java
+++ b/common/common-rest/src/test/java/org/apache/servicecomb/common/rest/TestAbstractRestInvocation.java
@@ -656,7 +656,7 @@ void beforeSendResponse(Invocation invocation, HttpServletResponseEx responseEx)
}.getMockInstance();
initRestInvocation();
- List<HttpServerFilter> httpServerFilters = SPIServiceUtils.getSortedService(HttpServerFilter.class);
+ List<HttpServerFilter> httpServerFilters = SPIServiceUtils.loadSortedService(HttpServerFilter.class);
httpServerFilters.add(filter);
restInvocation.setHttpServerFilters(httpServerFilters);
diff --git a/foundations/foundation-common/src/main/java/org/apache/servicecomb/foundation/common/utils/SPIServiceUtils.java b/foundations/foundation-common/src/main/java/org/apache/servicecomb/foundation/common/utils/SPIServiceUtils.java
index 5011abe2d..9cbfc86db 100644
--- a/foundations/foundation-common/src/main/java/org/apache/servicecomb/foundation/common/utils/SPIServiceUtils.java
+++ b/foundations/foundation-common/src/main/java/org/apache/servicecomb/foundation/common/utils/SPIServiceUtils.java
@@ -22,8 +22,10 @@
import java.util.ArrayList;
import java.util.Comparator;
import java.util.List;
+import java.util.Map;
import java.util.Map.Entry;
import java.util.ServiceLoader;
+import java.util.concurrent.ConcurrentHashMap;
import java.util.stream.Collectors;
import org.slf4j.Logger;
@@ -38,31 +40,21 @@
public final class SPIServiceUtils {
private static final Logger LOGGER = LoggerFactory.getLogger(SPIServiceUtils.class);
+ // load one service, maybe trigger load another service
+ // computeIfAbsent can not support this feature
+ // so use double check
+ private static final Object LOCK = new Object();
+
+ private static final Map<Class<?>, List<Object>> cache = new ConcurrentHashMap<>();
+
private SPIServiceUtils() {
}
/**
- * get target service.if target services are array,only random access to a service.
+ * no cache, return new instances every time.
*/
- public static <T> T getTargetService(Class<T> serviceType) {
- ServiceLoader<T> loader = ServiceLoader.load(serviceType);
- for (T service : loader) {
- LOGGER.info("get the SPI service success, the extend service is: {}", service.getClass());
- return service;
- }
- LOGGER.info("Can not get the SPI service, the interface type is: {}", serviceType.toString());
- return null;
- }
-
- public static <T> List<T> getAllService(Class<T> serviceType) {
- List<T> list = new ArrayList<>();
- ServiceLoader.load(serviceType).forEach(list::add);
-
- return list;
- }
-
- public static <T> List<T> getSortedService(Class<T> serviceType) {
+ public static <T> List<T> loadSortedService(Class<T> serviceType) {
List<Entry<Integer, T>> serviceEntries = new ArrayList<>();
ServiceLoader<T> serviceLoader = ServiceLoader.load(serviceType);
serviceLoader.forEach(service -> {
@@ -76,18 +68,74 @@ private SPIServiceUtils() {
serviceEntries.add(entry);
});
- return serviceEntries.stream()
+ List<T> services = serviceEntries.stream()
.sorted(Comparator.comparingInt(Entry::getKey))
.map(Entry::getValue)
.collect(Collectors.toList());
+
+ LOGGER.info("Found SPI service {}, count={}.", serviceType.getName(), services.size());
+ for (int idx = 0; idx < services.size(); idx++) {
+ T service = services.get(idx);
+ LOGGER.info(" {}. {}.", idx, service.getClass().getName());
+ }
+
+ return services;
+ }
+
+ @SuppressWarnings("unchecked")
+ public static <T> List<T> getOrLoadSortedService(Class<T> serviceType) {
+ List<Object> services = cache.get(serviceType);
+ if (services == null) {
+ synchronized (LOCK) {
+ services = cache.get(serviceType);
+ if (services == null) {
+ services = (List<Object>) loadSortedService(serviceType);
+ cache.put(serviceType, services);
+ }
+ }
+ }
+
+ return (List<T>) services;
+ }
+
+ /**
+ * get target service.if target services are array,only random access to a service.
+ */
+ public static <T> T getTargetService(Class<T> serviceType) {
+ List<T> services = getOrLoadSortedService(serviceType);
+ if (services.isEmpty()) {
+ LOGGER.info("Can not find SPI service for {}", serviceType.getName());
+ return null;
+ }
+
+ return services.get(0);
+ }
+
+ public static <T> List<T> getAllService(Class<T> serviceType) {
+ return getOrLoadSortedService(serviceType);
+ }
+
+ public static <T> List<T> getSortedService(Class<T> serviceType) {
+ return getOrLoadSortedService(serviceType);
}
public static <T> T getPriorityHighestService(Class<T> serviceType) {
- List<T> services = getSortedService(serviceType);
+ List<T> services = getOrLoadSortedService(serviceType);
if (services.isEmpty()) {
+ LOGGER.info("Can not find SPI service for {}", serviceType.getName());
return null;
}
return services.get(0);
}
+
+ @SuppressWarnings("unchecked")
+ public static <T, IMPL> IMPL getTargetService(Class<T> serviceType, Class<IMPL> implType) {
+ List<T> services = getOrLoadSortedService(serviceType);
+ return (IMPL) services
+ .stream()
+ .filter(service -> service.getClass().equals(implType))
+ .findFirst()
+ .orElse(null);
+ }
}
diff --git a/foundations/foundation-common/src/test/java/org/apache/servicecomb/foundation/common/utils/SPIServiceDef0Impl.java b/foundations/foundation-common/src/test/java/org/apache/servicecomb/foundation/common/utils/SPIServiceDef0Impl.java
new file mode 100644
index 000000000..08cd5a3d2
--- /dev/null
+++ b/foundations/foundation-common/src/test/java/org/apache/servicecomb/foundation/common/utils/SPIServiceDef0Impl.java
@@ -0,0 +1,22 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.servicecomb.foundation.common.utils;
+
+public class SPIServiceDef0Impl implements SPIServiceDef0 {
+
+}
diff --git a/foundations/foundation-common/src/test/java/org/apache/servicecomb/foundation/common/utils/TestSPIServiceUtils.java b/foundations/foundation-common/src/test/java/org/apache/servicecomb/foundation/common/utils/TestSPIServiceUtils.java
index 24e81bf6f..2bb136bd3 100644
--- a/foundations/foundation-common/src/test/java/org/apache/servicecomb/foundation/common/utils/TestSPIServiceUtils.java
+++ b/foundations/foundation-common/src/test/java/org/apache/servicecomb/foundation/common/utils/TestSPIServiceUtils.java
@@ -18,6 +18,7 @@
package org.apache.servicecomb.foundation.common.utils;
import java.util.LinkedHashMap;
+import java.util.List;
import java.util.Map;
import java.util.ServiceLoader;
@@ -46,6 +47,19 @@ public void testGetTargetServiceNull() {
public void testGetTargetServiceNotNull() {
SPIServiceDef service = SPIServiceUtils.getTargetService(SPIServiceDef.class);
Assert.assertTrue(SPIServiceDef.class.isInstance(service));
+
+ Assert.assertSame(service, SPIServiceUtils.getTargetService(SPIServiceDef.class));
+ }
+
+ @Test
+ public void testGetTargetService_special_null() {
+ Assert.assertNull(SPIServiceUtils.getTargetService(SPIServiceDef0.class, SPIServiceDef0Impl.class));
+ }
+
+ @Test
+ public void testGetTargetService_special_notNull() {
+ SPIServiceDef service = SPIServiceUtils.getTargetService(SPIServiceDef.class, SPIServiceDefImpl.class);
+ Assert.assertTrue(SPIServiceDefImpl.class.isInstance(service));
}
@Test
@@ -68,6 +82,15 @@ public void testSort(@Mocked Ordered o1, @Mocked Ordered o2) {
};
Assert.assertThat(SPIServiceUtils.getSortedService(Ordered.class), Matchers.contains(o1, o2));
+ Assert.assertThat(SPIServiceUtils.getAllService(Ordered.class), Matchers.contains(o1, o2));
Assert.assertThat(SPIServiceUtils.getPriorityHighestService(Ordered.class), Matchers.is(o1));
+
+ Map<Class<?>, List<Object>> cache = Deencapsulation.getField(SPIServiceUtils.class, "cache");
+ cache.clear();
+ }
+
+ @Test
+ public void getPriorityHighestService_null() {
+ Assert.assertNull(SPIServiceUtils.getPriorityHighestService(SPIServiceDef0.class));
}
}
diff --git a/foundations/foundation-metrics/src/main/java/org/apache/servicecomb/foundation/metrics/MetricsBootstrap.java b/foundations/foundation-metrics/src/main/java/org/apache/servicecomb/foundation/metrics/MetricsBootstrap.java
index 233526467..0c6a52367 100644
--- a/foundations/foundation-metrics/src/main/java/org/apache/servicecomb/foundation/metrics/MetricsBootstrap.java
+++ b/foundations/foundation-metrics/src/main/java/org/apache/servicecomb/foundation/metrics/MetricsBootstrap.java
@@ -22,8 +22,6 @@
import java.util.concurrent.TimeUnit;
import org.apache.servicecomb.foundation.common.utils.SPIServiceUtils;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
import com.google.common.collect.Lists;
import com.google.common.eventbus.EventBus;
@@ -32,8 +30,6 @@
import com.netflix.spectator.api.Meter;
public class MetricsBootstrap {
- private static final Logger LOGGER = LoggerFactory.getLogger(MetricsBootstrap.class);
-
private CompositeRegistry globalRegistry;
private EventBus eventBus;
@@ -60,8 +56,6 @@ public void shutdown() {
protected void loadMetricsInitializers() {
SPIServiceUtils.getSortedService(MetricsInitializer.class).forEach(initializer -> {
- LOGGER.info("Found MetricsInitializer: {}", initializer.getClass().getName());
-
initializer.init(globalRegistry, eventBus, config);
});
}
diff --git a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/consumer/TestMicroserviceManager.java b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/consumer/TestMicroserviceManager.java
index c87504c19..8a5f23667 100644
--- a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/consumer/TestMicroserviceManager.java
+++ b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/consumer/TestMicroserviceManager.java
@@ -89,32 +89,6 @@ public void getOrCreateMicroserviceVersionRule() {
Assert.assertEquals(1, cachedVersions.size());
}
- @Test
- public void testCreateRuleServiceTooMany() throws Exception {
- new Expectations(RegistryUtils.class) {
- {
- RegistryUtils.findServiceInstances(appId, anyString, DefinitionConst.VERSION_RULE_ALL, null);
- result = microserviceInstances;
- }
- };
-
- for (int i = 0; i < 1005; i++) {
- MicroserviceVersionRule microserviceVersionRule =
- microserviceManager.getOrCreateMicroserviceVersionRule(serviceName + i, versionRule);
- Assert.assertEquals("0.0.0+", microserviceVersionRule.getVersionRule().getVersionRule());
- Assert.assertNull(microserviceVersionRule.getLatestMicroserviceVersion());
- if (i == 499) {
- Thread.sleep(1);
- }
- }
-
- Assert.assertEquals(1005, cachedVersions.size());
- Assert.assertEquals("msName1004", cachedVersions.get("msName1004").getMicroserviceName());
- Assert.assertEquals("msName1000", cachedVersions.get("msName1000").getMicroserviceName());
- Assert.assertEquals("msName500", cachedVersions.get("msName500").getMicroserviceName());
- Assert.assertEquals("msName0", cachedVersions.get("msName0").getMicroserviceName());
- }
-
@Test
public void testCreateRuleServiceNotExists() {
new Expectations(RegistryUtils.class) {
diff --git a/swagger/swagger-generator/generator-core/src/main/java/org/apache/servicecomb/swagger/generator/core/CompositeSwaggerGeneratorContext.java b/swagger/swagger-generator/generator-core/src/main/java/org/apache/servicecomb/swagger/generator/core/CompositeSwaggerGeneratorContext.java
index b69ebba34..ef3979353 100644
--- a/swagger/swagger-generator/generator-core/src/main/java/org/apache/servicecomb/swagger/generator/core/CompositeSwaggerGeneratorContext.java
+++ b/swagger/swagger-generator/generator-core/src/main/java/org/apache/servicecomb/swagger/generator/core/CompositeSwaggerGeneratorContext.java
@@ -20,24 +20,16 @@
import java.util.List;
import org.apache.servicecomb.foundation.common.utils.SPIServiceUtils;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
import org.springframework.context.EmbeddedValueResolverAware;
import org.springframework.stereotype.Component;
import org.springframework.util.StringValueResolver;
@Component
public class CompositeSwaggerGeneratorContext implements EmbeddedValueResolverAware {
- private static final Logger LOGGER = LoggerFactory.getLogger(CompositeSwaggerGeneratorContext.class);
-
private List<SwaggerGeneratorContext> contextList;
public CompositeSwaggerGeneratorContext() {
contextList = SPIServiceUtils.getSortedService(SwaggerGeneratorContext.class);
-
- for (SwaggerGeneratorContext context : contextList) {
- LOGGER.info("Found swagger generator context: {}", context.getClass().getName());
- }
}
@Override
diff --git a/swagger/swagger-invocation/invocation-core/src/main/java/org/apache/servicecomb/swagger/invocation/response/ResponseMapperFactorys.java b/swagger/swagger-invocation/invocation-core/src/main/java/org/apache/servicecomb/swagger/invocation/response/ResponseMapperFactorys.java
index 5a9d2c2c5..92e95c6d9 100644
--- a/swagger/swagger-invocation/invocation-core/src/main/java/org/apache/servicecomb/swagger/invocation/response/ResponseMapperFactorys.java
+++ b/swagger/swagger-invocation/invocation-core/src/main/java/org/apache/servicecomb/swagger/invocation/response/ResponseMapperFactorys.java
@@ -21,12 +21,8 @@
import org.apache.servicecomb.foundation.common.utils.SPIServiceUtils;
import org.apache.servicecomb.swagger.invocation.converter.ConverterMgr;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
public class ResponseMapperFactorys<MAPPER> {
- private static final Logger LOGGER = LoggerFactory.getLogger(ResponseMapperFactorys.class);
-
private List<ResponseMapperFactory<MAPPER>> factorys;
public ResponseMapperFactorys(Class<? extends ResponseMapperFactory<MAPPER>> factoryCls, ConverterMgr converterMgr) {
@@ -37,9 +33,6 @@ public ResponseMapperFactorys(Class<? extends ResponseMapperFactory<MAPPER>> fac
@SuppressWarnings("unchecked")
public ResponseMapperFactorys(Class<? extends ResponseMapperFactory<MAPPER>> factoryCls) {
factorys = (List<ResponseMapperFactory<MAPPER>>) SPIServiceUtils.getSortedService(factoryCls);
- factorys.forEach(factory -> {
- LOGGER.info("found factory {} of {}:", factory.getClass().getName(), factoryCls.getName());
- });
}
public void setConverterMgr(ConverterMgr converterMgr) {
diff --git a/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/http/VertxHttpMethod.java b/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/http/VertxHttpMethod.java
index b97c39c62..d4f2aa109 100644
--- a/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/http/VertxHttpMethod.java
+++ b/transports/transport-rest/transport-rest-client/src/main/java/org/apache/servicecomb/transport/rest/client/http/VertxHttpMethod.java
@@ -56,12 +56,6 @@
static List<HttpClientFilter> httpClientFilters = SPIServiceUtils.getSortedService(HttpClientFilter.class);
- static {
- for (HttpClientFilter filter : httpClientFilters) {
- LOGGER.info("Found HttpClientFilter: {}.", filter.getClass().getName());
- }
- }
-
public void doMethod(HttpClientWithContext httpClientWithContext, Invocation invocation,
AsyncResponse asyncResp) throws Exception {
OperationMeta operationMeta = invocation.getOperationMeta();
diff --git a/transports/transport-rest/transport-rest-servlet/src/test/java/org/apache/servicecomb/transport/rest/servlet/TestRestServletProducerInvocation.java b/transports/transport-rest/transport-rest-servlet/src/test/java/org/apache/servicecomb/transport/rest/servlet/TestRestServletProducerInvocation.java
index b623fe581..a83480a2b 100644
--- a/transports/transport-rest/transport-rest-servlet/src/test/java/org/apache/servicecomb/transport/rest/servlet/TestRestServletProducerInvocation.java
+++ b/transports/transport-rest/transport-rest-servlet/src/test/java/org/apache/servicecomb/transport/rest/servlet/TestRestServletProducerInvocation.java
@@ -26,7 +26,6 @@
import org.apache.servicecomb.common.rest.definition.RestOperationMeta;
import org.apache.servicecomb.common.rest.filter.HttpServerFilter;
import org.apache.servicecomb.core.definition.OperationMeta;
-import org.apache.servicecomb.foundation.common.utils.SPIServiceUtils;
import org.apache.servicecomb.foundation.vertx.http.HttpServletRequestEx;
import org.apache.servicecomb.foundation.vertx.http.StandardHttpServletRequestEx;
import org.junit.Assert;
@@ -60,7 +59,7 @@ void findRestOperation() {
};
List<HttpServerFilter> httpServerFilters = Arrays.asList(f1);
- new Expectations(SPIServiceUtils.class) {
+ new Expectations() {
{
f1.needCacheRequest(operationMeta);
result = true;
@@ -76,7 +75,7 @@ void findRestOperation() {
@Test
public void collectCacheRequestCacheTrue(@Mocked HttpServerFilter f1) {
List<HttpServerFilter> httpServerFilters = Arrays.asList(f1);
- new Expectations(SPIServiceUtils.class) {
+ new Expectations() {
{
f1.needCacheRequest(operationMeta);
result = true;
@@ -90,7 +89,7 @@ public void collectCacheRequestCacheTrue(@Mocked HttpServerFilter f1) {
@Test
public void collectCacheRequestCacheFalse(@Mocked HttpServerFilter f1) {
List<HttpServerFilter> httpServerFilters = Arrays.asList(f1);
- new Expectations(SPIServiceUtils.class) {
+ new Expectations() {
{
f1.needCacheRequest(operationMeta);
result = false;
diff --git a/transports/transport-rest/transport-rest-vertx/src/main/java/org/apache/servicecomb/transport/rest/vertx/AbstractVertxHttpDispatcher.java b/transports/transport-rest/transport-rest-vertx/src/main/java/org/apache/servicecomb/transport/rest/vertx/AbstractVertxHttpDispatcher.java
index 0634cb6ac..7999161a8 100644
--- a/transports/transport-rest/transport-rest-vertx/src/main/java/org/apache/servicecomb/transport/rest/vertx/AbstractVertxHttpDispatcher.java
+++ b/transports/transport-rest/transport-rest-vertx/src/main/java/org/apache/servicecomb/transport/rest/vertx/AbstractVertxHttpDispatcher.java
@@ -35,12 +35,6 @@
protected List<HttpServerFilter> httpServerFilters = SPIServiceUtils.getSortedService(HttpServerFilter.class);
- public AbstractVertxHttpDispatcher() {
- for (HttpServerFilter filter : httpServerFilters) {
- LOGGER.info("Found HttpServerFilter: {}.", filter.getClass().getName());
- }
- }
-
protected BodyHandler createBodyHandler() {
RestBodyHandler bodyHandler = new RestBodyHandler();
diff --git a/transports/transport-rest/transport-rest-vertx/src/main/java/org/apache/servicecomb/transport/rest/vertx/RestServerVerticle.java b/transports/transport-rest/transport-rest-vertx/src/main/java/org/apache/servicecomb/transport/rest/vertx/RestServerVerticle.java
index 6578d725e..79739ca31 100644
--- a/transports/transport-rest/transport-rest-vertx/src/main/java/org/apache/servicecomb/transport/rest/vertx/RestServerVerticle.java
+++ b/transports/transport-rest/transport-rest-vertx/src/main/java/org/apache/servicecomb/transport/rest/vertx/RestServerVerticle.java
@@ -94,7 +94,6 @@ private void mountAccessLogHandler(Router mainRouter) {
private void initDispatcher(Router mainRouter) {
List<VertxHttpDispatcher> dispatchers = SPIServiceUtils.getSortedService(VertxHttpDispatcher.class);
for (VertxHttpDispatcher dispatcher : dispatchers) {
- LOGGER.info("init vertx http dispatcher {}", dispatcher.getClass().getName());
dispatcher.init(mainRouter);
}
}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services