Skip to content

Commit 3190112

Browse files
committed
ZOOKEEPER-4966: Decouple ZKConfig from QuorumPeerConfig.ConfigException
`ZKConfig` is client accessible, it should avoid accessing server side `QuorumPeerConfig`. Changes: 1. Change method and constructor signatures of `ZKConfig` and `ZKClientConfig` to throw `o.a.zookeeper.common.ConfigException`. 3. Throw `QuorumPeerConfig.ConfigException` if it is in classpath. Given above changes, this pr maintain abi compatibility with old releases. **Breaking change**: methods and constructors of `ZKConfig` and `ZKClientConfig` throw `org.apache.zookeeper.common.ConfigException` now but not `QuorumPeerConfig.ConfigException`, so developers have to fix it to compile. This is a small step towards ZOOKEEPER-233. Refs: ZOOKEEPER-233, ZOOKEEPER-835, ZOOKEEPER-842, ZOOKEEPER-4970, ZOOKEEPER-4966.
1 parent 0dadfb8 commit 3190112

File tree

2 files changed

+52
-46
lines changed

2 files changed

+52
-46
lines changed

zookeeper-server/src/main/java/org/apache/zookeeper/client/ZKClientConfig.java

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import org.apache.yetus.audience.InterfaceAudience;
2424
import org.apache.zookeeper.common.ConfigException;
2525
import org.apache.zookeeper.common.ZKConfig;
26-
import org.apache.zookeeper.server.quorum.QuorumPeerConfig;
2726

2827
/**
2928
* Handles client specific properties
@@ -69,22 +68,22 @@ public ZKClientConfig() {
6968
/**
7069
* <p><b>Use {@link ZKClientConfig#ZKClientConfig(Path configPath)} instead.</b>
7170
*
72-
* <p><b>The signature of this method will be changed to throw {@link ConfigException}
73-
* instead of {@link QuorumPeerConfig.ConfigException} in future release.</b>
71+
* <p><b>The signature of this method has been changed to throw {@link ConfigException}
72+
* instead of {@code QuorumPeerConfig.ConfigException}.</b>
7473
*/
7574
@Deprecated
76-
public ZKClientConfig(File configFile) throws QuorumPeerConfig.ConfigException {
75+
public ZKClientConfig(File configFile) throws ConfigException {
7776
super(configFile);
7877
}
7978

8079
/**
8180
* <p><b>Use {@link ZKClientConfig#ZKClientConfig(Path configPath)} instead.</b>
8281
*
83-
* <p><b>The signature of this method will be changed to throw {@link ConfigException}
84-
* instead of {@link QuorumPeerConfig.ConfigException} in future release.</b>
82+
* <p><b>The signature of this method has been changed to throw {@link ConfigException}
83+
* instead of {@code QuorumPeerConfig.ConfigException}.</b>
8584
*/
8685
@Deprecated
87-
public ZKClientConfig(String configPath) throws QuorumPeerConfig.ConfigException {
86+
public ZKClientConfig(String configPath) throws ConfigException {
8887
super(configPath);
8988
}
9089

zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKConfig.java

Lines changed: 46 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,14 @@
2121
import java.io.File;
2222
import java.io.FileInputStream;
2323
import java.io.IOException;
24+
import java.lang.reflect.Constructor;
2425
import java.nio.file.Path;
26+
import java.nio.file.Paths;
2527
import java.util.HashMap;
2628
import java.util.Map;
2729
import java.util.Map.Entry;
2830
import java.util.Properties;
2931
import org.apache.zookeeper.Environment;
30-
import org.apache.zookeeper.server.quorum.QuorumPeerConfig;
3132
import org.apache.zookeeper.server.util.VerifyingFileFactory;
3233
import org.slf4j.Logger;
3334
import org.slf4j.LoggerFactory;
@@ -65,35 +66,33 @@ public ZKConfig() {
6566
/**
6667
* <p><b>Use {@link ZKConfig#ZKConfig(Path configPath)} instead.</b>
6768
*
68-
* <p><b>The signature of this method will be changed to throw {@link ConfigException}
69-
* instead of {@link QuorumPeerConfig.ConfigException} in future release.</b>
69+
* <p><b>The signature of this method has been changed to throw {@link ConfigException}
70+
* instead of {@code QuorumPeerConfig.ConfigException}.</b>
7071
*
7172
* @param configPath
7273
* Configuration file path
7374
* @throws ConfigException
7475
* if failed to load configuration properties
7576
*/
7677
@Deprecated
77-
public ZKConfig(String configPath) throws QuorumPeerConfig.ConfigException {
78-
this(new File(configPath));
78+
public ZKConfig(String configPath) throws ConfigException {
79+
this(Paths.get(configPath));
7980
}
8081

8182
/**
8283
* <p><b>Use {@link ZKConfig#ZKConfig(Path configPath)} instead.</b>
8384
*
84-
* <p><b>The signature of this method will be changed to throw {@link ConfigException}
85-
* instead of {@link QuorumPeerConfig.ConfigException} in future release.</b>
85+
* <p><b>The signature of this method has been changed to throw {@link ConfigException}
86+
* instead of {@code QuorumPeerConfig.ConfigException}.</b>
8687
*
8788
* @param configFile
8889
* Configuration file
8990
* @throws ConfigException
9091
* if failed to load configuration properties
9192
*/
9293
@Deprecated
93-
public ZKConfig(File configFile) throws QuorumPeerConfig.ConfigException {
94-
this();
95-
addConfiguration(configFile);
96-
LOG.info("ZK Config {}", this.properties);
94+
public ZKConfig(File configFile) throws ConfigException {
95+
this(configFile.toPath());
9796
}
9897

9998
/**
@@ -102,9 +101,10 @@ public ZKConfig(File configFile) throws QuorumPeerConfig.ConfigException {
102101
* @param configPath path to configuration file
103102
* @throws ConfigException
104103
*/
105-
@SuppressWarnings("deprecation")
106104
public ZKConfig(Path configPath) throws ConfigException {
107-
this(configPath.toFile());
105+
this();
106+
addConfiguration(configPath);
107+
LOG.info("ZK Config {}", this.properties);
108108
}
109109

110110
private void init() {
@@ -215,16 +215,39 @@ public void setProperty(String key, String value) {
215215
*
216216
* @param configPath path to Configuration file.
217217
*/
218-
@SuppressWarnings("deprecation")
219218
public void addConfiguration(Path configPath) throws ConfigException {
220-
addConfiguration(configPath.toFile());
219+
Path absoluteConfigPath = configPath.toAbsolutePath();
220+
LOG.info("Reading configuration from: {}", absoluteConfigPath);
221+
try {
222+
File configFile = (new VerifyingFileFactory.Builder(LOG).warnForRelativePath()
223+
.failForNonExistingPath()
224+
.build()).validate(configPath.toFile());
225+
Properties cfg = new Properties();
226+
try (FileInputStream in = new FileInputStream(configFile)) {
227+
cfg.load(in);
228+
}
229+
parseProperties(cfg);
230+
} catch (IOException | IllegalArgumentException e) {
231+
LOG.error("Error while configuration from: {}", absoluteConfigPath, e);
232+
String msg = "Error while processing " + absoluteConfigPath;
233+
try {
234+
Class<?> clazz = Class.forName("org.apache.zookeeper.server.quorum.QuorumPeerConfig.ConfigException");
235+
Class<? extends ConfigException> exceptionClass = clazz.asSubclass(ConfigException.class);
236+
Constructor<? extends ConfigException> constructor = exceptionClass.getDeclaredConstructor(String.class, Exception.class);
237+
throw constructor.newInstance(msg, e);
238+
} catch (ClassNotFoundException ignored) {
239+
} catch (Exception ignored) {
240+
LOG.warn("Fail to construct QuorumPeerConfig.ConfigException", e);
241+
}
242+
throw new ConfigException(msg, e);
243+
}
221244
}
222245

223246
/**
224247
* <p><b>Use {@link #addConfiguration(Path)} instead.</b></p>
225248
*
226-
* <p><b>The signature of this method will be changed to throw {@link ConfigException}
227-
* instead of {@link QuorumPeerConfig.ConfigException} in future release.</b>
249+
* <p><b>The signature of this method has been changed to throw {@link ConfigException}
250+
* instead of {@code QuorumPeerConfig.ConfigException}.</b>
228251
*
229252
* <p>Add a configuration resource. The properties form this configuration will
230253
* overwrite corresponding already loaded property and system property
@@ -233,31 +256,15 @@ public void addConfiguration(Path configPath) throws ConfigException {
233256
* Configuration file.
234257
*/
235258
@Deprecated
236-
public void addConfiguration(File configFile) throws QuorumPeerConfig.ConfigException {
237-
LOG.info("Reading configuration from: {}", configFile.getAbsolutePath());
238-
try {
239-
configFile = (new VerifyingFileFactory.Builder(LOG).warnForRelativePath()
240-
.failForNonExistingPath()
241-
.build()).validate(configFile);
242-
Properties cfg = new Properties();
243-
FileInputStream in = new FileInputStream(configFile);
244-
try {
245-
cfg.load(in);
246-
} finally {
247-
in.close();
248-
}
249-
parseProperties(cfg);
250-
} catch (IOException | IllegalArgumentException e) {
251-
LOG.error("Error while configuration from: {}", configFile.getAbsolutePath(), e);
252-
throw new QuorumPeerConfig.ConfigException("Error while processing " + configFile.getAbsolutePath(), e);
253-
}
259+
public void addConfiguration(File configFile) throws ConfigException {
260+
addConfiguration(configFile.toPath());
254261
}
255262

256263
/**
257264
* <p><b>Use {@link #addConfiguration(Path)} instead.</b></p>
258265
*
259-
* <p><b>The signature of this method will be changed to throw {@link ConfigException}
260-
* instead of {@link QuorumPeerConfig.ConfigException} in future release.</b>
266+
* <p><b>The signature of this method has been changed to throw {@link ConfigException}
267+
* instead of {@code QuorumPeerConfig.ConfigException}.</b>
261268
*
262269
* <p>Add a configuration resource. The properties form this configuration will
263270
* overwrite corresponding already loaded property and system property
@@ -266,8 +273,8 @@ public void addConfiguration(File configFile) throws QuorumPeerConfig.ConfigExce
266273
* Configuration file path.
267274
*/
268275
@Deprecated
269-
public void addConfiguration(String configPath) throws QuorumPeerConfig.ConfigException {
270-
addConfiguration(new File(configPath));
276+
public void addConfiguration(String configPath) throws ConfigException {
277+
addConfiguration(Paths.get(configPath));
271278
}
272279

273280
private void parseProperties(Properties cfg) {

0 commit comments

Comments
 (0)