CrazyHZM commented on code in PR #13981: URL: https://github.com/apache/dubbo/pull/13981#discussion_r1544020847
########## dubbo-common/src/main/java/org/apache/dubbo/config/TripleConfig.java: ########## @@ -0,0 +1,105 @@ +/* + * 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.config; + +import java.io.Serializable; + +/** + * Configuration for triple protocol. + */ +public class TripleConfig implements Serializable { Review Comment: We should not add new config content in the 3.2 branches. ########## dubbo-spring-boot/dubbo-spring-boot-compatible/autoconfigure/README.md: ########## @@ -0,0 +1,25 @@ +# About spring-configuration-metadata + +## Why + +Configuration beans of dubbo are located in the `dubbo-common` module, and it's not suitable to add spring boot dependencies. +However, spring configuration metadata generation relies on read javadoc from the source code and cannot use the @NestedConfigurationProperty annotation. +This leads to missing comments and a lack of nested configuration options. Therefore, we use an independent module to copy the code and generate metadata. + +## Principles + +1. Copy classes under `org/apache/dubbo/config` from `dubbo-common` to the `generated-sources` directory. +2. Replace `@Nest` with `@NestedConfigurationProperty`. +3. Use an annotation-only option to compile and generate `spring-configuration-metadata.json`. + Review Comment: These should be moved to pr description. And add some reason for the plugin. ########## dubbo-spring-boot/dubbo-spring-boot-compatible/autoconfigure/src/main/java/org/apache/dubbo/spring/boot/autoconfigure/DubboConfigurationProperties.java: ########## @@ -26,110 +27,157 @@ import org.apache.dubbo.config.ProtocolConfig; import org.apache.dubbo.config.ProviderConfig; import org.apache.dubbo.config.RegistryConfig; +import org.apache.dubbo.config.SslConfig; import org.apache.dubbo.config.TracingConfig; -import org.apache.dubbo.config.context.ConfigMode; -import org.apache.dubbo.config.spring.ConfigCenterBean; -import org.apache.dubbo.config.spring.context.annotation.EnableDubbo; +import org.apache.dubbo.config.TripleConfig; import java.util.LinkedHashMap; -import java.util.LinkedHashSet; import java.util.Map; -import java.util.Set; import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.boot.context.properties.NestedConfigurationProperty; -import static org.apache.dubbo.spring.boot.util.DubboUtils.DEFAULT_MULTIPLE_CONFIG_PROPERTY_VALUE; -import static org.apache.dubbo.spring.boot.util.DubboUtils.DEFAULT_OVERRIDE_CONFIG_PROPERTY_VALUE; import static org.apache.dubbo.spring.boot.util.DubboUtils.DUBBO_PREFIX; /** - * Dubbo {@link ConfigurationProperties Config Properties} only used to generate JSON metadata(non-public class) + * Dubbo {@link ConfigurationProperties Config Properties} only used to generate JSON metadata (non-public class) * * @see ConfigKeys * @since 2.7.1 */ @ConfigurationProperties(DUBBO_PREFIX) public class DubboConfigurationProperties { - @NestedConfigurationProperty - private Config config = new Config(); - - @NestedConfigurationProperty - private Scan scan = new Scan(); - - // Single Config Bindings + /** + * Configuration properties for the application. + */ @NestedConfigurationProperty private ApplicationConfig application = new ApplicationConfig(); + /** + * Configuration properties for the module. + */ @NestedConfigurationProperty private ModuleConfig module = new ModuleConfig(); + /** + * Configuration properties for the registry. + */ @NestedConfigurationProperty private RegistryConfig registry = new RegistryConfig(); + /** + * Configuration properties for the protocol. + */ @NestedConfigurationProperty private ProtocolConfig protocol = new ProtocolConfig(); + /** + * Configuration properties for the monitor. + */ @NestedConfigurationProperty private MonitorConfig monitor = new MonitorConfig(); + /** + * Configuration properties for the provider. + */ @NestedConfigurationProperty private ProviderConfig provider = new ProviderConfig(); + /** + * Configuration properties for the consumer. + */ @NestedConfigurationProperty private ConsumerConfig consumer = new ConsumerConfig(); + /** + * Configuration properties for the config center. + */ @NestedConfigurationProperty - private ConfigCenterBean configCenter = new ConfigCenterBean(); + private ConfigCenterConfig configCenter = new ConfigCenterConfig(); + /** + * Configuration properties for the metadata report. + */ @NestedConfigurationProperty private MetadataReportConfig metadataReport = new MetadataReportConfig(); + /** + * Configuration properties for metrics. + */ @NestedConfigurationProperty private MetricsConfig metrics = new MetricsConfig(); + /** + * Configuration properties for tracing. + */ @NestedConfigurationProperty private TracingConfig tracing = new TracingConfig(); + /** + * Configuration properties for ssl. + */ + @NestedConfigurationProperty + private SslConfig ssl = new SslConfig(); + + /** + * Configuration properties for rpc. + */ + @NestedConfigurationProperty + private RpcConfig rpc = new RpcConfig(); + // Multiple Config Bindings + /** + * Multiple configurations for Module. + */ private Map<String, ModuleConfig> modules = new LinkedHashMap<>(); + /** + * Multiple configurations for Registry. + */ private Map<String, RegistryConfig> registries = new LinkedHashMap<>(); + /** + * Multiple configurations for Protocol. + */ private Map<String, ProtocolConfig> protocols = new LinkedHashMap<>(); + /** + * Multiple configurations for Monitor. + */ private Map<String, MonitorConfig> monitors = new LinkedHashMap<>(); + /** + * Multiple configurations for Provider. + */ private Map<String, ProviderConfig> providers = new LinkedHashMap<>(); + /** + * Multiple configurations for Consumer. + */ private Map<String, ConsumerConfig> consumers = new LinkedHashMap<>(); - private Map<String, ConfigCenterBean> configCenters = new LinkedHashMap<>(); + /** + * Multiple configurations for ConfigCenterBean. Review Comment: `ConfigCenterBean` change to `ConfigCenter` ########## dubbo-spring-boot/dubbo-spring-boot-compatible/autoconfigure/README.md: ########## @@ -0,0 +1,25 @@ +# About spring-configuration-metadata + +## Why + +Configuration beans of dubbo are located in the `dubbo-common` module, and it's not suitable to add spring boot dependencies. +However, spring configuration metadata generation relies on read javadoc from the source code and cannot use the @NestedConfigurationProperty annotation. +This leads to missing comments and a lack of nested configuration options. Therefore, we use an independent module to copy the code and generate metadata. + +## Principles + +1. Copy classes under `org/apache/dubbo/config` from `dubbo-common` to the `generated-sources` directory. +2. Replace `@Nest` with `@NestedConfigurationProperty`. +3. Use an annotation-only option to compile and generate `spring-configuration-metadata.json`. + +## How to add a new configuration option + +- For standard configuration options, add javadoc to the corresponding configuration classes in `dubbo-common`. +- For non-standard configuration options, there are unnecessary to add nested classes. add them directly to `additional-spring-configuration-metadata.json`. + +## Configuration Javadoc Guideline + +1. For noun-type configuration options, use "The xxx" format for comments. +2. For boolean-type configuration options, use "Whether to xxx" format and add ", default value is <code>true</code>" at the end. +3. For configuration options with longer comments, use multi-line comments, with a clear summary in the first line. +4. All comments should end with a period. Review Comment: I think these should be moved to Javadoc of DubboConfigurationProperties、DubboMetadataGenerateAutoConfiguration. ########## dubbo-spring-boot/dubbo-spring-boot-compatible/autoconfigure/src/main/resources/META-INF/additional-spring-configuration-metadata.json: ########## @@ -0,0 +1,44 @@ +{ + "properties": [ + { + "name": "dubbo.enabled", + "description": "Whether enable autoconfiguration of dubbo, default value is <code>true</code>.", + "type": "java.util.Set<java.lang.String>" Review Comment: The type should be `java.lang.Boolean`. ########## dubbo-spring-boot/dubbo-spring-boot-compatible/autoconfigure/src/main/java/org/apache/dubbo/spring/boot/autoconfigure/DubboConfigurationProperties.java: ########## @@ -298,64 +362,23 @@ public void setTracings(Map<String, TracingConfig> tracings) { this.tracings = tracings; } - static class Config { - - /** - * Config processing mode - * @see ConfigMode - */ - private ConfigMode mode = ConfigMode.STRICT; - - /** - * Indicates multiple properties binding from externalized configuration or not. - */ - private boolean multiple = DEFAULT_MULTIPLE_CONFIG_PROPERTY_VALUE; - - /** - * The property name of override Dubbo config - */ - private boolean override = DEFAULT_OVERRIDE_CONFIG_PROPERTY_VALUE; - - public boolean isOverride() { - return override; - } - - public void setOverride(boolean override) { - this.override = override; - } - - public boolean isMultiple() { - return multiple; - } - - public void setMultiple(boolean multiple) { - this.multiple = multiple; - } - - public ConfigMode getMode() { - return mode; - } - - public void setMode(ConfigMode mode) { - this.mode = mode; - } - } - - static class Scan { + /** + * Configuration for rpc. + */ + public static class RpcConfig { Review Comment: We should not add new config content in the 3.2 branches. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
