[pulsar] branch master updated: [fix][broker] Fix unexpected behaviour by invoke `PulsarConfigurationLoader#convertFrom` (#18816)

[penghui]

This is an automated email from the ASF dual-hosted git repository.

penghui pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pulsar.git


The following commit(s) were added to refs/heads/master by this push:
     new ac7a34fe757 [fix][broker] Fix unexpected behaviour by invoke 
`PulsarConfigurationLoader#convertFrom` (#18816)
ac7a34fe757 is described below

commit ac7a34fe757fd8acaf7b87aac427b9e515b0aa7c
Author: Qiang Zhao <mattisonc...@apache.org>
AuthorDate: Fri Dec 9 08:30:08 2022 +0800

    [fix][broker] Fix unexpected behaviour by invoke 
`PulsarConfigurationLoader#convertFrom` (#18816)
---
 .../configuration/PulsarConfigurationLoader.java   | 14 +++++++++-----
 .../proxy/server/ProxyConfigurationTest.java       | 22 ++++++++++++++++++++++
 2 files changed, 31 insertions(+), 5 deletions(-)

diff --git 
a/pulsar-broker-common/src/main/java/org/apache/pulsar/common/configuration/PulsarConfigurationLoader.java
 
b/pulsar-broker-common/src/main/java/org/apache/pulsar/common/configuration/PulsarConfigurationLoader.java
index 66b8e042675..cbc6bd692c8 100644
--- 
a/pulsar-broker-common/src/main/java/org/apache/pulsar/common/configuration/PulsarConfigurationLoader.java
+++ 
b/pulsar-broker-common/src/main/java/org/apache/pulsar/common/configuration/PulsarConfigurationLoader.java
@@ -182,12 +182,14 @@ public class PulsarConfigurationLoader {
             final ServiceConfiguration convertedConf = 
ServiceConfiguration.class
                     .getDeclaredConstructor().newInstance();
             Field[] confFields = conf.getClass().getDeclaredFields();
-            Properties properties = new Properties();
+            Properties sourceProperties = conf.getProperties();
+            Properties targetProperties = convertedConf.getProperties();
             Arrays.stream(confFields).forEach(confField -> {
                 try {
                     confField.setAccessible(true);
                     Field convertedConfField = 
ServiceConfiguration.class.getDeclaredField(confField.getName());
-                    if (!Modifier.isStatic(convertedConfField.getModifiers())) 
{
+                    if (!Modifier.isStatic(convertedConfField.getModifiers())
+                            && 
convertedConfField.getDeclaredAnnotation(FieldContext.class) != null) {
                         convertedConfField.setAccessible(true);
                         convertedConfField.set(convertedConf, 
confField.get(conf));
                     }
@@ -198,8 +200,9 @@ public class PulsarConfigurationLoader {
                     }
                     // add unknown fields to properties
                     try {
-                        if (confField.get(conf) != null) {
-                            properties.put(confField.getName(), 
confField.get(conf));
+                        String propertyName = confField.getName();
+                        if (!sourceProperties.containsKey(propertyName) && 
confField.get(conf) != null) {
+                            targetProperties.put(propertyName, 
confField.get(conf));
                         }
                     } catch (Exception ignoreException) {
                         // should not happen
@@ -208,7 +211,8 @@ public class PulsarConfigurationLoader {
                     throw new RuntimeException("Exception caused while 
converting configuration: " + e.getMessage());
                 }
             });
-            convertedConf.getProperties().putAll(properties);
+            // Put the rest of properties to new config
+            targetProperties.putAll(sourceProperties);
             return convertedConf;
         } catch (InstantiationException | IllegalAccessException
                 | InvocationTargetException | NoSuchMethodException e) {
diff --git 
a/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyConfigurationTest.java
 
b/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyConfigurationTest.java
index 7a2e122c108..ac1078a1a45 100644
--- 
a/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyConfigurationTest.java
+++ 
b/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyConfigurationTest.java
@@ -110,4 +110,26 @@ public class ProxyConfigurationTest {
         assertEquals(serviceConfig.getMetadataStoreSessionTimeoutMillis(), 
100);
         assertEquals(serviceConfig.getMetadataStoreCacheExpirySeconds(), 300);
     }
+
+
+    @Test
+    public void testConvert() throws IOException {
+        File testConfigFile = new File("tmp." + System.currentTimeMillis() + 
".properties");
+        if (testConfigFile.exists()) {
+            testConfigFile.delete();
+        }
+        try (PrintWriter printWriter = new PrintWriter(new 
OutputStreamWriter(new FileOutputStream(testConfigFile)))) {
+            printWriter.println("proxyAdditionalServlets=a,b,c");
+        }
+        testConfigFile.deleteOnExit();
+        try(InputStream stream = new FileInputStream(testConfigFile)) {
+            ProxyConfiguration proxyConfig = 
PulsarConfigurationLoader.create(stream, ProxyConfiguration.class);
+            
assertEquals(proxyConfig.getProperties().getProperty("proxyAdditionalServlets"),
 "a,b,c");
+            assertEquals(proxyConfig.getProxyAdditionalServlets().size(), 3);
+            PulsarConfigurationLoader.convertFrom(proxyConfig);
+            
assertEquals(proxyConfig.getProperties().getProperty("proxyAdditionalServlets"),
 "a,b,c");
+            assertEquals(proxyConfig.getProxyAdditionalServlets().size(), 3);
+        }
+    }
+
 }
\ No newline at end of file