sandynz commented on code in PR #22677:
URL: https://github.com/apache/shardingsphere/pull/22677#discussion_r1039577568
##########
jdbc/core/src/main/java/org/apache/shardingsphere/driver/jdbc/core/driver/ShardingSphereDriverURL.java:
##########
@@ -72,4 +72,30 @@ public byte[] toConfigurationBytes() {
return builder.toString().getBytes(StandardCharsets.UTF_8);
}
}
+
+ private InputStream getResourceAsStream(final String resource) throws
IOException {
+ ClassLoader[] classLoader = new ClassLoader[]{
+ Thread.currentThread().getContextClassLoader(),
+ getClass().getClassLoader(),
+ ClassLoader.getSystemClassLoader(),
+ };
+
+ for (ClassLoader cl : classLoader) {
Review Comment:
Variable name `cl` should be `each`
##########
jdbc/core/src/main/java/org/apache/shardingsphere/driver/jdbc/core/driver/ShardingSphereDriverURL.java:
##########
@@ -72,4 +72,30 @@ public byte[] toConfigurationBytes() {
return builder.toString().getBytes(StandardCharsets.UTF_8);
}
}
+
+ private InputStream getResourceAsStream(final String resource) throws
IOException {
+ ClassLoader[] classLoader = new ClassLoader[]{
+ Thread.currentThread().getContextClassLoader(),
+ getClass().getClassLoader(),
+ ClassLoader.getSystemClassLoader(),
+ };
+
+ for (ClassLoader cl : classLoader) {
+ if (null != cl) {
+
+ // try to find the resource as passed
+ InputStream returnValue = cl.getResourceAsStream(resource);
+
+ // now, some class loaders want this leading "/", so we'll add
it and try again if we didn't find the resource
+ if (null == returnValue) {
+ returnValue = cl.getResourceAsStream("/" + resource);
+ }
Review Comment:
Did you test which class loader need '/'?
If the most frequent used one need '/', could we try to load with '/'
firstly?
##########
jdbc/core/src/main/java/org/apache/shardingsphere/driver/jdbc/core/driver/ShardingSphereDriverURL.java:
##########
@@ -72,4 +72,30 @@ public byte[] toConfigurationBytes() {
return builder.toString().getBytes(StandardCharsets.UTF_8);
}
}
+
+ private InputStream getResourceAsStream(final String resource) throws
IOException {
+ ClassLoader[] classLoader = new ClassLoader[]{
+ Thread.currentThread().getContextClassLoader(),
+ getClass().getClassLoader(),
+ ClassLoader.getSystemClassLoader(),
+ };
+
+ for (ClassLoader cl : classLoader) {
+ if (null != cl) {
+
+ // try to find the resource as passed
+ InputStream returnValue = cl.getResourceAsStream(resource);
+
+ // now, some class loaders want this leading "/", so we'll add
it and try again if we didn't find the resource
+ if (null == returnValue) {
+ returnValue = cl.getResourceAsStream("/" + resource);
+ }
+
Review Comment:
Empty lines in method could be removed
##########
jdbc/core/src/main/java/org/apache/shardingsphere/driver/jdbc/core/driver/ShardingSphereDriverURL.java:
##########
@@ -72,4 +72,30 @@ public byte[] toConfigurationBytes() {
return builder.toString().getBytes(StandardCharsets.UTF_8);
}
}
+
+ private InputStream getResourceAsStream(final String resource) throws
IOException {
+ ClassLoader[] classLoader = new ClassLoader[]{
+ Thread.currentThread().getContextClassLoader(),
+ getClass().getClassLoader(),
+ ClassLoader.getSystemClassLoader(),
+ };
Review Comment:
`classLoader` could be `classLoaders`
##########
jdbc/core/src/main/java/org/apache/shardingsphere/driver/jdbc/core/driver/ShardingSphereDriverURL.java:
##########
@@ -72,4 +72,30 @@ public byte[] toConfigurationBytes() {
return builder.toString().getBytes(StandardCharsets.UTF_8);
}
}
+
+ private InputStream getResourceAsStream(final String resource) throws
IOException {
+ ClassLoader[] classLoader = new ClassLoader[]{
+ Thread.currentThread().getContextClassLoader(),
+ getClass().getClassLoader(),
+ ClassLoader.getSystemClassLoader(),
+ };
Review Comment:
Did you test which class loader satisify most cases or the most common case?
Could we put `getClass().getClassLoader()` at first?
##########
jdbc/core/src/main/java/org/apache/shardingsphere/driver/jdbc/core/driver/ShardingSphereDriverURL.java:
##########
@@ -72,4 +72,30 @@ public byte[] toConfigurationBytes() {
return builder.toString().getBytes(StandardCharsets.UTF_8);
}
}
+
+ private InputStream getResourceAsStream(final String resource) throws
IOException {
+ ClassLoader[] classLoader = new ClassLoader[]{
+ Thread.currentThread().getContextClassLoader(),
+ getClass().getClassLoader(),
+ ClassLoader.getSystemClassLoader(),
+ };
+
+ for (ClassLoader cl : classLoader) {
+ if (null != cl) {
+
+ // try to find the resource as passed
+ InputStream returnValue = cl.getResourceAsStream(resource);
Review Comment:
`returnValue` should be `result`
##########
jdbc/core/src/main/java/org/apache/shardingsphere/driver/jdbc/core/driver/ShardingSphereDriverURL.java:
##########
@@ -72,4 +72,30 @@ public byte[] toConfigurationBytes() {
return builder.toString().getBytes(StandardCharsets.UTF_8);
}
}
+
+ private InputStream getResourceAsStream(final String resource) throws
IOException {
+ ClassLoader[] classLoader = new ClassLoader[]{
+ Thread.currentThread().getContextClassLoader(),
+ getClass().getClassLoader(),
+ ClassLoader.getSystemClassLoader(),
+ };
+
+ for (ClassLoader cl : classLoader) {
+ if (null != cl) {
+
+ // try to find the resource as passed
Review Comment:
Comments in method could be removed
##########
jdbc/core/src/main/java/org/apache/shardingsphere/driver/jdbc/core/driver/ShardingSphereDriverURL.java:
##########
@@ -72,4 +72,30 @@ public byte[] toConfigurationBytes() {
return builder.toString().getBytes(StandardCharsets.UTF_8);
}
}
+
+ private InputStream getResourceAsStream(final String resource) throws
IOException {
+ ClassLoader[] classLoader = new ClassLoader[]{
+ Thread.currentThread().getContextClassLoader(),
+ getClass().getClassLoader(),
+ ClassLoader.getSystemClassLoader(),
+ };
Review Comment:
It's better to put them in one line for shorter code
--
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]