From 5253a88e78e265452709c3c784a8a2b3a4e8e0ac Mon Sep 17 00:00:00 2001 From: Imteyaz Ahmed Khan <30792765+khanimteyaz@users.noreply.github.com> Date: Wed, 2 Jan 2019 18:10:03 +0530 Subject: [PATCH] Replace hard coded dubbo-config-api values and small code optimization (#3108) * Refactoring to replace hard coded value with constant and some code optiomization * Fixed UT failure * Added review comment by Ian and code4wt --- .../org/apache/dubbo/common/Constants.java | 44 +++++++++++ .../dubbo/config/AbstractServiceConfig.java | 6 +- .../dubbo/config/ApplicationConfig.java | 25 +++++-- .../dubbo/config/ConfigCenterConfig.java | 2 +- .../org/apache/dubbo/config/MethodConfig.java | 2 +- .../org/apache/dubbo/config/ModuleConfig.java | 7 +- .../apache/dubbo/config/ProtocolConfig.java | 33 +++++---- .../apache/dubbo/config/ProviderConfig.java | 8 +- .../apache/dubbo/config/ReferenceConfig.java | 61 ++++++++++------ .../apache/dubbo/config/RegistryConfig.java | 21 ++---- .../apache/dubbo/config/ServiceConfig.java | 73 +++++++++++-------- 11 files changed, 175 insertions(+), 107 deletions(-) diff --git a/dubbo-common/src/main/java/org/apache/dubbo/common/Constants.java b/dubbo-common/src/main/java/org/apache/dubbo/common/Constants.java index 71cc04dba9f..f799358afe3 100644 --- a/dubbo-common/src/main/java/org/apache/dubbo/common/Constants.java +++ b/dubbo-common/src/main/java/org/apache/dubbo/common/Constants.java @@ -231,6 +231,8 @@ public class Constants { public static final String DOBBO_PROTOCOL = DUBBO; + public static final String ZOOKEEPER_PROTOCOL = "zookeeper"; + public static final String PROXY_KEY = "proxy"; public static final String WEIGHT_KEY = "weight"; @@ -324,6 +326,8 @@ public class Constants { public static final String EXCHANGER_KEY = "exchanger"; + public static final String DISPACTHER_KEY = "dispacther"; + public static final String TRANSPORTER_KEY = "transporter"; public static final String SERVER_KEY = "server"; @@ -769,6 +773,46 @@ public class Constants { public static final String[] DEFAULT_REGISTER_CONSUMER_KEYS = {APPLICATION_KEY, VERSION_KEY, GROUP_KEY, DUBBO_VERSION_KEY, SPECIFICATION_VERSION_KEY}; public static final String TELNET = "telnet"; + + /** + * Application name; + */ + public static final String NAME = "name"; + + /** + * Application owner name; + */ + public static final String OWNER = "owner"; + + /** + * Running application organization name. + */ + public static final String ORGANIZATION = "organization"; + + /** + * Application architecture name. + */ + public static final String ARCHITECTURE = "architecture"; + + /** + * Environment name + */ + public static final String ENVIRONMENT = "environment"; + + /** + * Test environment key. + */ + public static final String TEST_ENVIRONMENT = "test"; + + /** + * Development environment key. + */ + public static final String DEVELOPMENT_ENVIRONMENT = "develop"; + + /** + * Production environment key. + */ + public static final String PRODUCTION_ENVIRONMENT = "product"; /* * private Constants(){ } */ diff --git a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/AbstractServiceConfig.java b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/AbstractServiceConfig.java index 2f3311b546a..191fcf8c454 100644 --- a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/AbstractServiceConfig.java +++ b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/AbstractServiceConfig.java @@ -79,7 +79,7 @@ public String getVersion() { } public void setVersion(String version) { - checkKey("version", version); + checkKey(Constants.VERSION_KEY, version); this.version = version; } @@ -88,7 +88,7 @@ public String getGroup() { } public void setGroup(String group) { - checkKey("group", group); + checkKey(Constants.GROUP_KEY, group); this.group = group; } @@ -138,7 +138,7 @@ public void setToken(Boolean token) { } public void setToken(String token) { - checkName("token", token); + checkName(Constants.TOKEN_KEY, token); this.token = token; } diff --git a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ApplicationConfig.java b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ApplicationConfig.java index a5c29dbf30a..6b1f53181eb 100644 --- a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ApplicationConfig.java +++ b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ApplicationConfig.java @@ -102,9 +102,9 @@ public String getName() { } public void setName(String name) { - checkName("name", name); + checkName(Constants.NAME, name); this.name = name; - if (id == null || id.length() == 0) { + if (StringUtils.isEmpty(id)) { id = name; } } @@ -123,7 +123,7 @@ public String getOwner() { } public void setOwner(String owner) { - checkMultiName("owner", owner); + checkMultiName(Constants.OWNER, owner); this.owner = owner; } @@ -132,7 +132,7 @@ public String getOrganization() { } public void setOrganization(String organization) { - checkName("organization", organization); + checkName(Constants.ORGANIZATION, organization); this.organization = organization; } @@ -141,7 +141,7 @@ public String getArchitecture() { } public void setArchitecture(String architecture) { - checkName("architecture", architecture); + checkName(Constants.ARCHITECTURE, architecture); this.architecture = architecture; } @@ -150,10 +150,18 @@ public String getEnvironment() { } public void setEnvironment(String environment) { - checkName("environment", environment); + checkName(Constants.ENVIRONMENT, environment); if (environment != null) { - if (!("develop".equals(environment) || "test".equals(environment) || "product".equals(environment))) { - throw new IllegalStateException("Unsupported environment: " + environment + ", only support develop/test/product, default is product."); + if (!(Constants.DEVELOPMENT_ENVIRONMENT.equals(environment) + || Constants.TEST_ENVIRONMENT.equals(environment) + || Constants.PRODUCTION_ENVIRONMENT.equals(environment))) { + + throw new IllegalStateException(String.format("Unsupported environment: %s, only support %s/%s/%s, default is %s.", + environment, + Constants.DEVELOPMENT_ENVIRONMENT, + Constants.TEST_ENVIRONMENT, + Constants.PRODUCTION_ENVIRONMENT, + Constants.PRODUCTION_ENVIRONMENT)); } } this.environment = environment; @@ -284,4 +292,5 @@ public void setShutwait(String shutwait) { public boolean isValid() { return !StringUtils.isEmpty(name); } + } \ No newline at end of file diff --git a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ConfigCenterConfig.java b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ConfigCenterConfig.java index fe62fda18bf..84e7fddd302 100644 --- a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ConfigCenterConfig.java +++ b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ConfigCenterConfig.java @@ -115,7 +115,7 @@ private URL toConfigUrl() { map.put(Constants.PATH_KEY, ConfigCenterConfig.class.getSimpleName()); // use 'zookeeper' as the default configcenter. if (StringUtils.isEmpty(map.get(Constants.PROTOCOL_KEY))) { - map.put(Constants.PROTOCOL_KEY, "zookeeper"); + map.put(Constants.PROTOCOL_KEY, Constants.ZOOKEEPER_PROTOCOL); } return UrlUtils.parseURL(address, map); } diff --git a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/MethodConfig.java b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/MethodConfig.java index 48045d4261c..50ac47dd6f2 100644 --- a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/MethodConfig.java +++ b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/MethodConfig.java @@ -89,7 +89,7 @@ public String getName() { public void setName(String name) { checkMethodName("name", name); this.name = name; - if (id == null || id.length() == 0) { + if (StringUtils.isEmpty(id)) { id = name; } } diff --git a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ModuleConfig.java b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ModuleConfig.java index 0f1501870d2..f07638fe61b 100644 --- a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ModuleConfig.java +++ b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ModuleConfig.java @@ -16,6 +16,7 @@ */ package org.apache.dubbo.config; +import org.apache.dubbo.common.Constants; import org.apache.dubbo.config.support.Parameter; import java.util.ArrayList; @@ -64,7 +65,7 @@ public String getName() { } public void setName(String name) { - checkName("name", name); + checkName(Constants.NAME, name); this.name = name; if (id == null || id.length() == 0) { id = name; @@ -85,7 +86,7 @@ public String getOwner() { } public void setOwner(String owner) { - checkName("owner", owner); + checkName(Constants.OWNER, owner); this.owner = owner; } @@ -94,7 +95,7 @@ public String getOrganization() { } public void setOrganization(String organization) { - checkName("organization", organization); + checkName(Constants.ORGANIZATION, organization); this.organization = organization; } diff --git a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ProtocolConfig.java b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ProtocolConfig.java index 7313f59b699..1f6bc3c7b54 100644 --- a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ProtocolConfig.java +++ b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ProtocolConfig.java @@ -16,6 +16,7 @@ */ package org.apache.dubbo.config; +import org.apache.dubbo.common.Constants; import org.apache.dubbo.common.extension.ExtensionLoader; import org.apache.dubbo.common.serialize.Serialization; import org.apache.dubbo.common.status.StatusChecker; @@ -154,7 +155,7 @@ public String getName() { return name; } - public void setName(String name) { + public final void setName(String name) { checkName("name", name); this.name = name; this.updateIdIfAbsent(name); @@ -166,7 +167,7 @@ public String getHost() { } public void setHost(String host) { - checkName("host", host); + checkName(Constants.HOST_KEY, host); this.host = host; } @@ -175,7 +176,7 @@ public Integer getPort() { return port; } - public void setPort(Integer port) { + public final void setPort(Integer port) { this.port = port; } @@ -205,7 +206,7 @@ public String getThreadpool() { } public void setThreadpool(String threadpool) { - checkExtension(ThreadPool.class, "threadpool", threadpool); + checkExtension(ThreadPool.class, Constants.THREADPOOL_KEY, threadpool); this.threadpool = threadpool; } @@ -254,8 +255,8 @@ public String getCodec() { } public void setCodec(String codec) { - if ("dubbo".equals(name)) { - checkMultiExtension(Codec.class, "codec", codec); + if (Constants.DOBBO_PROTOCOL.equals(name)) { + checkMultiExtension(Codec.class, Constants.CODEC_KEY, codec); } this.codec = codec; } @@ -265,8 +266,8 @@ public String getSerialization() { } public void setSerialization(String serialization) { - if ("dubbo".equals(name)) { - checkMultiExtension(Serialization.class, "serialization", serialization); + if (Constants.DOBBO_PROTOCOL.equals(name)) { + checkMultiExtension(Serialization.class, Constants.SERIALIZATION_KEY, serialization); } this.serialization = serialization; } @@ -308,8 +309,8 @@ public String getServer() { } public void setServer(String server) { - if ("dubbo".equals(name)) { - checkMultiExtension(Transporter.class, "server", server); + if (Constants.DOBBO_PROTOCOL.equals(name)) { + checkMultiExtension(Transporter.class, Constants.SERVER_KEY, server); } this.server = server; } @@ -319,8 +320,8 @@ public String getClient() { } public void setClient(String client) { - if ("dubbo".equals(name)) { - checkMultiExtension(Transporter.class, "client", client); + if (Constants.DOBBO_PROTOCOL.equals(name)) { + checkMultiExtension(Transporter.class, Constants.CLIENT_KEY, client); } this.client = client; } @@ -338,7 +339,7 @@ public String getTelnet() { } public void setTelnet(String telnet) { - checkMultiExtension(TelnetHandler.class, "telnet", telnet); + checkMultiExtension(TelnetHandler.class, Constants.TELNET, telnet); this.telnet = telnet; } @@ -373,7 +374,7 @@ public String getTransporter() { } public void setTransporter(String transporter) { - checkExtension(Transporter.class, "transporter", transporter); + checkExtension(Transporter.class, Constants.TRANSPORTER_KEY, transporter); this.transporter = transporter; } @@ -382,7 +383,7 @@ public String getExchanger() { } public void setExchanger(String exchanger) { - checkExtension(Exchanger.class, "exchanger", exchanger); + checkExtension(Exchanger.class, Constants.EXCHANGER_KEY, exchanger); this.exchanger = exchanger; } @@ -412,7 +413,7 @@ public String getDispatcher() { } public void setDispatcher(String dispatcher) { - checkExtension(Dispatcher.class, "dispacther", dispatcher); + checkExtension(Dispatcher.class, Constants.DISPACTHER_KEY, dispatcher); this.dispatcher = dispatcher; } diff --git a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ProviderConfig.java b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ProviderConfig.java index 7d69395ad61..aeb4efa43a9 100644 --- a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ProviderConfig.java +++ b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ProviderConfig.java @@ -169,7 +169,7 @@ public String getThreadpool() { } public void setThreadpool(String threadpool) { - checkExtension(ThreadPool.class, "threadpool", threadpool); + checkExtension(ThreadPool.class, Constants.THREADPOOL_KEY, threadpool); this.threadpool = threadpool; } @@ -258,7 +258,7 @@ public String getTelnet() { } public void setTelnet(String telnet) { - checkMultiExtension(TelnetHandler.class, "telnet", telnet); + checkMultiExtension(TelnetHandler.class, Constants.TELNET, telnet); this.telnet = telnet; } @@ -320,7 +320,7 @@ public String getTransporter() { } public void setTransporter(String transporter) { - checkExtension(Transporter.class, "transporter", transporter); + checkExtension(Transporter.class, Constants.TRANSPORTER_KEY, transporter); this.transporter = transporter; } @@ -329,7 +329,7 @@ public String getExchanger() { } public void setExchanger(String exchanger) { - checkExtension(Exchanger.class, "exchanger", exchanger); + checkExtension(Exchanger.class, Constants.EXCHANGER_KEY, exchanger); this.exchanger = exchanger; } diff --git a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ReferenceConfig.java b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ReferenceConfig.java index 3c8eb635e78..198f7c3a41c 100644 --- a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ReferenceConfig.java +++ b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ReferenceConfig.java @@ -147,34 +147,13 @@ public void checkAndUpdateSubConfigs() { } resolveFile(); if (consumer != null) { - if (application == null) { - application = consumer.getApplication(); - } - if (module == null) { - module = consumer.getModule(); - } - if (registries == null) { - registries = consumer.getRegistries(); - } - if (monitor == null) { - monitor = consumer.getMonitor(); - } + inheritIfAbsentFromConsumer(); } if (module != null) { - if (registries == null) { - registries = module.getRegistries(); - } - if (monitor == null) { - monitor = module.getMonitor(); - } + inheritIfAbsentFromModule(); } if (application != null) { - if (registries == null) { - registries = application.getRegistries(); - } - if (monitor == null) { - monitor = application.getMonitor(); - } + inheritIfAbsentFromApplication(); } checkApplication(); checkMetadataReport(); @@ -380,6 +359,40 @@ private void checkDefault() { consumer.refresh(); } + private void inheritIfAbsentFromConsumer() { + if (application == null) { + application = consumer.getApplication(); + } + if (module == null) { + module = consumer.getModule(); + } + if (registries == null) { + registries = consumer.getRegistries(); + } + if (monitor == null) { + monitor = consumer.getMonitor(); + } + } + + private void inheritIfAbsentFromModule() { + if (registries == null) { + registries = module.getRegistries(); + } + if (monitor == null) { + monitor = module.getMonitor(); + } + } + + private void inheritIfAbsentFromApplication() { + if (registries == null) { + registries = application.getRegistries(); + } + if (monitor == null) { + monitor = application.getMonitor(); + } + } + + public Class getInterfaceClass() { if (interfaceClass != null) { return interfaceClass; diff --git a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/RegistryConfig.java b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/RegistryConfig.java index 7f15e4c90cf..512fbe8305a 100644 --- a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/RegistryConfig.java +++ b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/RegistryConfig.java @@ -120,7 +120,7 @@ public String getProtocol() { } public void setProtocol(String protocol) { - checkName("protocol", protocol); + checkName(Constants.PROTOCOL_KEY, protocol); this.protocol = protocol; this.updateIdIfAbsent(protocol); } @@ -232,7 +232,7 @@ public String getTransporter() { } public void setTransporter(String transporter) { - checkName("transporter", transporter); + checkName(Constants.TRANSPORTER_KEY, transporter); /*if(transporter != null && transporter.length() > 0 && ! ExtensionLoader.getExtensionLoader(Transporter.class).hasExtension(transporter)){ throw new IllegalStateException("No such transporter type : " + transporter); }*/ @@ -244,7 +244,7 @@ public String getServer() { } public void setServer(String server) { - checkName("server", server); + checkName(Constants.SERVER_KEY, server); /*if(server != null && server.length() > 0 && ! ExtensionLoader.getExtensionLoader(Transporter.class).hasExtension(server)){ throw new IllegalStateException("No such server type : " + server); }*/ @@ -349,18 +349,8 @@ public boolean isZookeeperProtocol() { if (!isValid()) { return false; } - boolean isZookeeper = StringUtils.isNotEmpty(this.getProtocol()) && this.getProtocol().equals("zookeeper"); - if (!isZookeeper) { - String address = this.getAddress(); - int index = address.indexOf("://"); - if (StringUtils.isNotEmpty(address) && index >= 0) { - address = address.substring(0, index); - } - if (address.equals("zookeeper")) { - isZookeeper = true; - } - } - return isZookeeper; + return Constants.ZOOKEEPER_PROTOCOL.equals(getProtocol()) + || getAddress().startsWith(Constants.ZOOKEEPER_PROTOCOL); } @Override @@ -369,4 +359,5 @@ public boolean isValid() { // empty protocol will default to 'dubbo' return !StringUtils.isEmpty(address); } + } diff --git a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ServiceConfig.java b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ServiceConfig.java index e0f419be789..84b8188cfa8 100644 --- a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ServiceConfig.java +++ b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ServiceConfig.java @@ -163,10 +163,7 @@ private static ProviderConfig convertProtocolToProvider(ProtocolConfig protocol) private static Integer getRandomPort(String protocol) { protocol = protocol.toLowerCase(); - if (RANDOM_PORT_MAP.containsKey(protocol)) { - return RANDOM_PORT_MAP.get(protocol); - } - return Integer.MIN_VALUE; + return RANDOM_PORT_MAP.getOrDefault(protocol, Integer.MIN_VALUE); } private static void putRandomPort(String protocol, Integer port) { @@ -198,37 +195,13 @@ public boolean isUnexported() { public void checkAndUpdateSubConfigs() { checkDefault(); if (provider != null) { - if (application == null) { - application = provider.getApplication(); - } - if (module == null) { - module = provider.getModule(); - } - if (registries == null) { - registries = provider.getRegistries(); - } - if (monitor == null) { - monitor = provider.getMonitor(); - } - if (protocols == null) { - protocols = provider.getProtocols(); - } + inheritIfAbsentFromProvider(); } if (module != null) { - if (registries == null) { - registries = module.getRegistries(); - } - if (monitor == null) { - monitor = module.getMonitor(); - } + inheritIfAbsentFromModule(); } if (application != null) { - if (registries == null) { - registries = application.getRegistries(); - } - if (monitor == null) { - monitor = application.getMonitor(); - } + inheritIfAbsentFromApplication(); } checkApplication(); @@ -372,7 +345,7 @@ private void doExportUrls() { private void doExportUrlsFor1Protocol(ProtocolConfig protocolConfig, List registryURLs) { String name = protocolConfig.getName(); if (name == null || name.length() == 0) { - name = "dubbo"; + name = Constants.DUBBO; } Map map = new HashMap(); @@ -553,6 +526,42 @@ private void exportLocal(URL url) { } } + private void inheritIfAbsentFromProvider() { + if (application == null) { + application = provider.getApplication(); + } + if (module == null) { + module = provider.getModule(); + } + if (registries == null) { + registries = provider.getRegistries(); + } + if (monitor == null) { + monitor = provider.getMonitor(); + } + if (protocols == null) { + protocols = provider.getProtocols(); + } + } + + private void inheritIfAbsentFromModule() { + if (registries == null) { + registries = module.getRegistries(); + } + if (monitor == null) { + monitor = module.getMonitor(); + } + } + + private void inheritIfAbsentFromApplication() { + if (registries == null) { + registries = application.getRegistries(); + } + if (monitor == null) { + monitor = application.getMonitor(); + } + } + protected Class getServiceClass(T ref) { return ref.getClass(); }