C0urante commented on code in PR #12805:
URL: https://github.com/apache/kafka/pull/12805#discussion_r1029585009
##########
connect/runtime/src/test/resources/test-plugins/read-version-from-resource-v1/test/plugins/ReadVersionFromResource.java:
##########
@@ -35,22 +38,37 @@ public void configure(final Map<String, ?> configs, final
boolean isKey) {
}
- @Override
- public byte[] fromConnectData(final String topic, final Schema schema,
final Object value) {
- try (InputStream stream =
this.getClass().getResourceAsStream("/version");
- BufferedReader reader = new BufferedReader(new
InputStreamReader(stream))) {
+ private String version(InputStream stream) throws IOException {
+ try (BufferedReader reader = new BufferedReader(new
InputStreamReader(stream))) {
return reader.lines()
.filter(s -> !s.isEmpty() && !s.startsWith("#"))
.collect(Collectors.toList())
- .get(0)
- .getBytes(StandardCharsets.UTF_8);
+ .get(0);
+ }
+ }
+
+ @Override
+ public byte[] fromConnectData(final String topic, final Schema schema,
final Object value) {
Review Comment:
We should add some Javadocs either to this class or to its `fromConnectData`
and `toConnectData` methods explaining that these aren't "real" converters and
the methods that they implement from that interface are just used during tests
to relay information about which class loader resources were available to them
at the time of invocation.
(Same goes for the "v2" variant as well)
##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/PluginClassLoader.java:
##########
@@ -81,6 +85,48 @@ public String toString() {
return "PluginClassLoader{pluginLocation=" + pluginLocation + "}";
}
+ @Override
+ public URL getResource(String name) {
+ Objects.requireNonNull(name);
+
+ URL url = findResource(name);
+ if (url == null) {
+ url = super.getResource(name);
+ }
+ return url;
+ }
+
+ @Override
+ public Enumeration<URL> getResources(String name) throws IOException {
+ Objects.requireNonNull(name);
+ @SuppressWarnings("unchecked")
+ Enumeration<URL>[] tmp = (Enumeration<URL>[]) new Enumeration<?>[2];
+ tmp[0] = findResources(name);
+ // Explicitly call the parent implementation instead of super to avoid
double-listing the local resources
+ tmp[1] = getParent().getResources(name);
+ return new Enumeration<URL>() {
+ private int index;
+ private boolean next() {
+ while (index < tmp.length) {
+ if (tmp[index] != null && tmp[index].hasMoreElements()) {
+ return true;
+ }
+ index++;
+ }
+ return false;
+ }
+ public boolean hasMoreElements() {
+ return next();
+ }
+ public URL nextElement() {
+ if (!next()) {
+ throw new NoSuchElementException();
+ }
+ return tmp[index].nextElement();
+ }
+ };
Review Comment:
We can simplify this by using the `Vector` class, which is a `List`
implementation capable of providing an `Enumeration` for its elements:
```suggestion
Vector<URL> resources = new Vector<>();
for (Enumeration<URL> foundLocally = findResources(name);
foundLocally.hasMoreElements();) {
URL url = foundLocally.nextElement();
if (url != null)
resources.add(url);
}
// Explicitly call the parent implementation instead of super to
avoid double-listing the local resources
for (Enumeration<URL> foundByParent =
getParent().getResources(name); foundByParent.hasMoreElements();) {
URL url = foundByParent.nextElement();
if (url != null)
resources.add(url);
}
return resources.elements();
```
--
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]