markap14 commented on a change in pull request #4852:
URL: https://github.com/apache/nifi/pull/4852#discussion_r585068088
##########
File path:
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-nar-utils/src/main/java/org/apache/nifi/nar/StandardExtensionDiscoveringManager.java
##########
@@ -145,74 +152,103 @@ public void discoverExtensions(final Set<Bundle>
narBundles) {
*
* @param bundle from which to load extensions
*/
- @SuppressWarnings("unchecked")
private void loadExtensions(final Bundle bundle) {
- for (final Map.Entry<Class, Set<Class>> entry :
definitionMap.entrySet()) {
- final boolean isControllerService =
ControllerService.class.equals(entry.getKey());
- final boolean isProcessor = Processor.class.equals(entry.getKey());
- final boolean isReportingTask =
ReportingTask.class.equals(entry.getKey());
-
- final ServiceLoader<?> serviceLoader =
ServiceLoader.load(entry.getKey(), bundle.getClassLoader());
- for (final Object o : serviceLoader) {
- try {
- loadExtension(o, entry.getKey(), bundle);
- } catch (Exception e) {
- logger.warn("Failed to register extension {} due to: {}" ,
new Object[]{o.getClass().getCanonicalName(), e.getMessage()});
- if (logger.isDebugEnabled()) {
- logger.debug("", e);
+ for (final Class extensionType : definitionMap.keySet()) {
+ final String serviceType = extensionType.getName();
+
+ try {
+ final Set<URL> serviceResourceUrls =
getServiceFileURLs(bundle, extensionType);
+ logger.debug("Bundle {} has the following Services File URLs
for {}: {}", bundle, serviceType, serviceResourceUrls);
+
+ for (final URL serviceResourceUrl : serviceResourceUrls) {
+ final Set<String> implementationClassNames =
getServiceFileImplementationClassNames(serviceResourceUrl);
+ logger.debug("Bundle {} defines {} implementations of
interface {}", bundle, implementationClassNames.size(), serviceType);
+
+ for (final String implementationClassName :
implementationClassNames) {
+ try {
+ loadExtension(implementationClassName,
extensionType, bundle);
+ logger.debug("Successfully loaded {} {} from {}",
extensionType.getSimpleName(), implementationClassName, bundle);
+ } catch (final Exception e) {
+ logger.error("Failed to register {} of type {} in
bundle {}" , extensionType.getSimpleName(), implementationClassName, bundle, e);
+ }
}
}
+ } catch (final IOException e) {
+ throw new RuntimeException("Failed to get resources of type "
+ serviceType + " from bundle " + bundle);
}
-
- classLoaderBundleLookup.put(bundle.getClassLoader(), bundle);
}
+
+ classLoaderBundleLookup.put(bundle.getClassLoader(), bundle);
}
- protected void loadExtension(final Object extension, final Class<?>
extensionType, final Bundle bundle) {
- final boolean isControllerService =
ControllerService.class.equals(extensionType);
- final boolean isProcessor = Processor.class.equals(extensionType);
- final boolean isReportingTask =
ReportingTask.class.equals(extensionType);
+ private Set<String> getServiceFileImplementationClassNames(final URL
serviceFileUrl) throws IOException {
+ final Set<String> implementationClassNames = new HashSet<>();
- // create a cache of temp ConfigurableComponent instances, the
initialize here has to happen before the checks below
- if ((isControllerService || isProcessor || isReportingTask) &&
extension instanceof ConfigurableComponent) {
- final ConfigurableComponent configurableComponent =
(ConfigurableComponent) extension;
- initializeTempComponent(configurableComponent);
+ try (final InputStream in = serviceFileUrl.openStream();
+ final Reader inputStreamReader = new InputStreamReader(in);
+ final BufferedReader reader = new
BufferedReader(inputStreamReader)) {
- final String cacheKey =
getClassBundleKey(extension.getClass().getCanonicalName(),
bundle.getBundleDetails().getCoordinate());
- tempComponentLookup.put(cacheKey, configurableComponent);
- }
+ String line;
+ while ((line = reader.readLine()) != null) {
+ // Remove anything after the #
+ final int index = line.indexOf("#");
+ if (index >= 0) {
+ line = line.substring(0, index);
+ }
- // only consider extensions discovered directly in this bundle
- boolean registerExtension =
bundle.getClassLoader().equals(extension.getClass().getClassLoader());
+ // Ignore empty line
+ line = line.trim();
+ if (line.isEmpty()) {
+ continue;
+ }
- if (registerExtension) {
- final Class<?> extensionClass = extension.getClass();
- if (isControllerService &&
!checkControllerServiceEligibility(extensionClass)) {
- registerExtension = false;
- logger.error(String.format(
- "Skipping Controller Service %s because it is bundled with
its supporting APIs and requires instance class loading.",
extensionClass.getName()));
+ implementationClassNames.add(line);
Review comment:
@exceptionfactory we could add a check like that, but I don't know that
it really buys us anything. It seems very arbitrary to me, to check one
character on each line. Unless you have some deeper understanding that I am
lacking, as to why that is an important check, I don't think it's worth
muddying the codebase to try to discover an anomaly with the first character of
each line, in particular, when any other character could be an issue as well
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
