gharris1727 opened a new pull request, #14089: URL: https://github.com/apache/kafka/pull/14089
This method is misleading, as it makes the caller assume that a given plugin class has only one supertype, when it may actually have multiple. Rather than documenting this limitation, or having this return an EnumSet (which duplicates the instanceof keyword), it is best to eliminate this function and rewrite all of the call-sites. There were two main places that used this method: 1. `AbstractHerder#connectorPluginConfig`. Here, it turns out to have been safe, because the config() method is the same for all of the plugins. Also the SinkConnector and SourceConnector superclasses are exclusive, so there is no problem with multiple interfaces in this method. The switch-case is replaced with an if-elseif-else ladder with instanceof checks. 2. `PluginDesc#new` where the bug was noticed. Instead of calling PluginType.from on the instantiated class, we should instead use the PluginType based on which branch of execution we're on. If for example we're parsing Converters to populate the Set<PluginDesc<Converter>> then we should use PluginDesc.CONVERTER. This turns out to duplicate the existing Class<T> klass argument to the `PluginScanner#getServiceLoaderPluginDesc` and `ReflectiveScanner#getPluginDesc` argument, so refactor these and change the log messages to use the type argument instead. Explaining why the rest of the diff exists and is so large: There were also a lot of test call-sites for `PluginDesc#new`, some of which also depended on `ReflectiveScanner#versionFor(Class)` which i've been trying to eliminate. I took the opportunity to re-write all of those call-sites to provide explicit versions and types instead of inferring it from the classes. I also changed `ConnectorPluginsResourceTestConnector#version()` to emit the appVersion, rather than set up a new constant version string. I'll update the other UNDEFINED_VERSION cases with real versions in a follow-up. I found some test call-sites which were passing in invalid classes into the PluginDesc (and therefore, PluginType.from) triggering the `UNKNOWN` case. Since the `UNKNOWN` could only come from this function, I altered those call-sites to use SINK instead, and eliminated the now-unused `UNKNOWN` value. I also realized that we need to update the compareTo method, since with the new constructor implementation, the comparison may be inconsistent with the equals method. In particular, two PluginDesc with the same class, version, and loader but different types are already unequal, but should also have a nonzero comparison result. The implementation is a little bit flawed due to hashCode collisions, so let me know if there's a better way. ### Committer Checklist (excluded from commit message) - [ ] Verify design and implementation - [ ] Verify test coverage and CI build status - [ ] Verify documentation (including upgrade notes) -- 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: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
