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

Bugfix - ExtensionLoader can not inject by type properly. #9187

Merged
merged 10 commits into from Feb 6, 2022
Expand Up @@ -19,7 +19,7 @@
import org.apache.dubbo.common.config.configcenter.DynamicConfiguration;
import org.apache.dubbo.common.context.FrameworkExt;
import org.apache.dubbo.common.context.LifecycleAdapter;
import org.apache.dubbo.common.extension.DisableInject;
import org.apache.dubbo.common.extension.Inject;
import org.apache.dubbo.common.logger.Logger;
import org.apache.dubbo.common.logger.LoggerFactory;
import org.apache.dubbo.config.AbstractConfig;
Expand Down Expand Up @@ -77,14 +77,14 @@ public void initialize() throws IllegalStateException {
this.appExternalConfiguration.setProperties(appExternalConfigurationMap);
}

@DisableInject
@Inject(enable = false)
public void setExternalConfigMap(Map<String, String> externalConfiguration) {
if (externalConfiguration != null) {
this.externalConfigurationMap = externalConfiguration;
}
}

@DisableInject
@Inject(enable = false)
public void setAppExternalConfigMap(Map<String, String> appExternalConfiguration) {
if (appExternalConfiguration != null) {
this.appExternalConfigurationMap = appExternalConfiguration;
Expand Down Expand Up @@ -179,7 +179,7 @@ public boolean isConfigCenterFirst() {
return configCenterFirst;
}

@DisableInject
@Inject(enable = false)
public void setConfigCenterFirst(boolean configCenterFirst) {
this.configCenterFirst = configCenterFirst;
}
Expand All @@ -188,7 +188,7 @@ public Optional<DynamicConfiguration> getDynamicConfiguration() {
return Optional.ofNullable(dynamicConfiguration);
}

@DisableInject
@Inject(enable = false)
public void setDynamicConfiguration(DynamicConfiguration dynamicConfiguration) {
this.dynamicConfiguration = dynamicConfiguration;
}
Expand Down
Expand Up @@ -25,5 +25,6 @@
@Documented
@Retention(RetentionPolicy.RUNTIME)
@Target({ElementType.TYPE, ElementType.METHOD})
@Deprecated
public @interface DisableInject {
}
}
Expand Up @@ -452,6 +452,7 @@ public T getExtension(String name, boolean wrap) {

/**
* get the original type.
*
* @param name
* @return
*/
Expand Down Expand Up @@ -707,35 +708,58 @@ private T injectExtension(T instance) {
if (!isSetter(method)) {
continue;
}
/**
* Check {@link DisableInject} to see if we need auto injection for this property

/*
* Check {@link DisableInject} to see if we need autowire injection for this property
*/
if (method.getAnnotation(DisableInject.class) != null) {
continue;
}

Class<?> pt = method.getParameterTypes()[0];
if (ReflectUtils.isPrimitives(pt)) {
continue;
}

try {
String property = getSetterProperty(method);
Object object = objectFactory.getExtension(pt, property);
if (object != null) {
method.invoke(instance, object);
/*
* Check {@link Inject} to see if we need auto-injection for this property
* {@link Inject#enable} == false will skip inject property phase
* {@link Inject#InjectType#ByName} default inject by name
*/
String property = getSetterProperty(method);
Inject inject = method.getAnnotation(Inject.class);
warmonipa marked this conversation as resolved.
Show resolved Hide resolved
if (inject == null) {
injectValue(instance, method, pt, property);
} else {
if (!inject.enable()) {
continue;
}
} catch (Exception e) {
logger.error("Failed to inject via method " + method.getName()
+ " of interface " + type.getName() + ": " + e.getMessage(), e);
}

if (inject.type() == Inject.InjectType.ByType) {
injectValue(instance, method, pt, null);
} else {
injectValue(instance, method, pt, property);
}
}
}
} catch (Exception e) {
logger.error(e.getMessage(), e);
}
return instance;
}

private void injectValue(T instance, Method method, Class<?> pt, String property) {
try {
Object object = objectFactory.getExtension(pt, property);
if (object != null) {
method.invoke(instance, object);
}
} catch (Exception e) {
logger.error("Failed to inject via method " + method.getName()
+ " of interface " + type.getName() + ": " + e.getMessage(), e);
}
}

private void initExtension(T instance) {
if (instance instanceof Lifecycle) {
Lifecycle lifecycle = (Lifecycle) instance;
Expand Down
@@ -0,0 +1,41 @@
/*
* 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.dubbo.common.extension;

import java.lang.annotation.Documented;
import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

import static org.apache.dubbo.common.extension.Inject.InjectType.ByName;

@Documented
@Retention(RetentionPolicy.RUNTIME)
@Target({ElementType.TYPE, ElementType.METHOD})
public @interface Inject {
// whether enable injection or not
boolean enable() default true;

// inject type default by name injection
InjectType type() default ByName;

enum InjectType{
ByName,
ByType
}
warmonipa marked this conversation as resolved.
Show resolved Hide resolved
}
Expand Up @@ -18,7 +18,7 @@

import org.apache.dubbo.common.context.FrameworkExt;
import org.apache.dubbo.common.context.LifecycleAdapter;
import org.apache.dubbo.common.extension.DisableInject;
import org.apache.dubbo.common.extension.Inject;
import org.apache.dubbo.common.logger.Logger;
import org.apache.dubbo.common.logger.LoggerFactory;
import org.apache.dubbo.common.utils.CollectionUtils;
Expand Down Expand Up @@ -77,7 +77,7 @@ public ConfigManager() {
}

// ApplicationConfig correlative methods
@DisableInject
@Inject(enable = false)
public void setApplication(ApplicationConfig application) {
addConfig(application, true);
}
Expand All @@ -92,7 +92,7 @@ public ApplicationConfig getApplicationOrElseThrow() {

// MonitorConfig correlative methods

@DisableInject
@Inject(enable = false)
public void setMonitor(MonitorConfig monitor) {
addConfig(monitor, true);
}
Expand All @@ -102,7 +102,7 @@ public Optional<MonitorConfig> getMonitor() {
}

// ModuleConfig correlative methods
@DisableInject
@Inject(enable = false)
public void setModule(ModuleConfig module) {
addConfig(module, true);
}
Expand All @@ -111,7 +111,7 @@ public Optional<ModuleConfig> getModule() {
return ofNullable(getConfig(getTagName(ModuleConfig.class)));
}

@DisableInject
@Inject(enable = false)
public void setMetrics(MetricsConfig metrics) {
addConfig(metrics, true);
}
Expand All @@ -120,7 +120,7 @@ public Optional<MetricsConfig> getMetrics() {
return ofNullable(getConfig(getTagName(MetricsConfig.class)));
}

@DisableInject
@Inject(enable = false)
public void setSsl(SslConfig sslConfig) {
addConfig(sslConfig, true);
}
Expand Down
Expand Up @@ -16,7 +16,7 @@
*/
package org.apache.dubbo.common.extension.injection.impl;

import org.apache.dubbo.common.extension.DisableInject;
import org.apache.dubbo.common.extension.Inject;
import org.apache.dubbo.common.extension.ext1.SimpleExt;
import org.apache.dubbo.common.extension.injection.InjectExt;

Expand All @@ -32,7 +32,7 @@ public void setSimpleExt(SimpleExt simpleExt) {
this.simpleExt = simpleExt;
}

@DisableInject
@Inject(enable = false)
public void setSimpleExt1(SimpleExt simpleExt1) {
this.simpleExt1 = simpleExt1;
}
Expand Down
Expand Up @@ -19,7 +19,7 @@
import org.apache.dubbo.common.URL;
import org.apache.dubbo.common.config.configcenter.AbstractDynamicConfigurationFactory;
import org.apache.dubbo.common.config.configcenter.DynamicConfiguration;
import org.apache.dubbo.common.extension.DisableInject;
import org.apache.dubbo.common.extension.Inject;
import org.apache.dubbo.remoting.zookeeper.ZookeeperTransporter;

/**
Expand All @@ -33,7 +33,7 @@ public ZookeeperDynamicConfigurationFactory() {
this.zookeeperTransporter = ZookeeperTransporter.getExtension();
}

@DisableInject
@Inject(enable = false)
public void setZookeeperTransporter(ZookeeperTransporter zookeeperTransporter) {
this.zookeeperTransporter = zookeeperTransporter;
}
Expand Down
Expand Up @@ -17,7 +17,7 @@
package org.apache.dubbo.metadata.store.zookeeper;

import org.apache.dubbo.common.URL;
import org.apache.dubbo.common.extension.DisableInject;
import org.apache.dubbo.common.extension.Inject;
import org.apache.dubbo.metadata.report.MetadataReport;
import org.apache.dubbo.metadata.report.support.AbstractMetadataReportFactory;
import org.apache.dubbo.remoting.zookeeper.ZookeeperTransporter;
Expand All @@ -33,7 +33,7 @@ public ZookeeperMetadataReportFactory() {
this.zookeeperTransporter = ZookeeperTransporter.getExtension();
}

@DisableInject
@Inject(enable = false)
public void setZookeeperTransporter(ZookeeperTransporter zookeeperTransporter) {
this.zookeeperTransporter = zookeeperTransporter;
}
Expand Down
Expand Up @@ -17,7 +17,7 @@
package org.apache.dubbo.registry.zookeeper;

import org.apache.dubbo.common.URL;
import org.apache.dubbo.common.extension.DisableInject;
import org.apache.dubbo.common.extension.Inject;
import org.apache.dubbo.registry.Registry;
import org.apache.dubbo.registry.support.AbstractRegistryFactory;
import org.apache.dubbo.remoting.zookeeper.ZookeeperTransporter;
Expand All @@ -38,7 +38,7 @@ public ZookeeperRegistryFactory() {
*
* @param zookeeperTransporter
*/
@DisableInject
@Inject(enable = false)
public void setZookeeperTransporter(ZookeeperTransporter zookeeperTransporter) {
this.zookeeperTransporter = zookeeperTransporter;
}
Expand Down