bobpaulin commented on PR #10691: URL: https://github.com/apache/nifi/pull/10691#issuecomment-3719320304
This is a fair call out as the implementations have drifted making the comment suspect. That was my original motivation for removing it. The following prompt to Claude highlights the differences: "Analyze the method quietlyInvokeMethodsWithAnnotation in @nifi-framework-bundle/nifi-framework/nifi-framework-nar-utils/src/main/java/org/apache/nifi/init/ReflectionUtils.java . Next analyze the method quietlyInvokeMethodsWithAnnotation in @nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/util/ReflectionUtils.java . Describe the differences between these methods." <section id="markdown-section-84c9c32c-2164-4918-b7ec-9091f1e61609-11" class="markdown-section chat-fade-in" data-markdown-raw=" ## Key Differences" data-section-index="11" style="border-radius: 4px; line-height: 19.5px; margin: 6px 0px; position: relative; scroll-margin-bottom: 40px; scroll-margin-top: 40px; color: rgba(228, 228, 228, 0.92); font-family: -apple-system, "system-ui", sans-serif; font-size: 13px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; white-space: normal; background-color: rgb(24, 24, 24); text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial;"><h2 style="animation: 0.25s ease 0s 1 normal none running fade-in; font-weight: 600 !important; font-size: 1.3em; line-height: 1.25; margin-bottom: 10px; margin-top: 20px;"><span style="animation: 0.25s ease 0s 1 normal none running fade-in;">Key Differences</span></h2></section><div cla ss="monaco-scrollable-element mac" role="presentation" style="scrollbar-width: none; overflow: hidden; color: rgba(228, 228, 228, 0.92); font-family: -apple-system, "system-ui", sans-serif; font-size: 13px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; white-space: normal; background-color: rgb(24, 24, 24); text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial; position: relative; margin: 1em 0px; width: 800px;"><div style="overflow: hidden;"> Aspect | nifi-framework-nar-utils | nifi-framework-components -- | -- | -- Package | org.apache.nifi.init | org.apache.nifi.util Caching | ❌ None | ✅ Uses WeakHashMap cache for discovered methods Method Overloads | Single signature (requires ComponentLog) | Two overloads (with/without ComponentLog) Alternate Annotation | ❌ Single annotation only | ✅ Supports preferred + alternate annotation pattern Superclass Traversal | ❌ Only looks at direct class methods | ✅ Recursively discovers methods from superclasses Method Deduplication | ❌ No deduplication | ✅ Uses TreeSet with comparator to dedupe by name + parameter types InvocationTargetException Handling | Returns true (only logs, doesn't fail) | Returns false (sets isSuccess = false) Implementation Style | Self-contained, inline logic | Modular, delegates to shared infrastructure </div><div role="presentation" aria-hidden="true" class="invisible scrollbar horizontal fade" style="visibility: hidden; opacity: 0; pointer-events: none; transition: opacity 0.8s linear; position: absolute; width: 650px; height: 10px; left: 0px; bottom: 0px;"><div class="slider" style="background: none 0% 0% / auto repeat scroll padding-box border-box rgba(228, 228, 228, 0.07); position: absolute; top: 0px; left: 232px; height: 10px; transform: translate3d(0px, 0px, 0px); contain: strict; width: 418px;"></div></div><div role="presentation" aria-hidden="true" class="invisible scrollbar vertical" style="visibility: hidden; opacity: 0; pointer-events: none; position: absolute; width: 0px; height: 294px; right: 0px; top: 0px;"><div class="slider" style="background: none 0% 0% / auto repeat scroll padding-box border-box rgba(228, 228, 228, 0.07); position: absolute; top: 0px; left: 0px; width: 10px; transform: translate3d(0px, 0px, 0px); contain: strict; height: 294px;"></div></div></ div><section id="markdown-section-84c9c32c-2164-4918-b7ec-9091f1e61609-13" class="markdown-section chat-fade-in" data-markdown-raw=" | Aspect | `nifi-framework-nar-utils` | `nifi-framework-components` | |--------|---------------------------|----------------------------| | **Package** | `org.apache.nifi.init` | `org.apache.nifi.util` | | **Caching** | ❌ None | ✅ Uses `ConcurrentHashMap` cache for discovered methods | | **Method Overloads** | Single signature (requires `ComponentLog`) | Two overloads (with/without `ComponentLog`) | | **Alternate Annotation** | ❌ Single annotation only | ✅ Supports preferred + alternate annotation pattern | | **Superclass Traversal** | ❌ Only looks at direct class methods | ✅ Recursively discovers methods from superclasses | | **Method Deduplication** | ❌ No deduplication | ✅ Uses `TreeSet` with comparator to dedupe by name + parameter types | | **InvocationTargetException Handling** | Returns `true` (only logs, doesn't fail) | Returns `false` (sets `isSuccess = false`) | | **Implementation Style** | Self-contained, inline logic | Modular, delegates to shared infrastructure | The blast radius of org.apache.nifi.init.ReflectionUtils is strictly to the nifi-framekwork-nar-utils and each usage within that module are ONLY using it to call the @Shutdown annotation on the specific classes. This usage eliminates the concern with caching given it only occurs on shutdown. The method overloads I do not believe is a concern. The alternative annotation and method deduplication also does not appear to come into play here as the framework appears to only use @Shutdown on single methods within classes [1] This leaves Superclass traversal. AbstractCacheServer is extended in 2 places [2] [3] based on the org.apache.nifi.init.ReflectionUtils I believe for both of these classes the @Shutdown method would NOT be called given it exists in the super class. I don't believe that is the correct behavior but if you look further ALL of the above class implement ConfigurableComponentInitializer [4] and these annotation methods appear to be all used within the teardown method. I do not see teardown called anywhere in the framework [5] [6]. Is it possible this is in fact dead code? Finally even if we are still nervous and want to keep org.apache.nifi.init.ReflectionUtils. We still need to move org.apache.nifi.utils.ReflectionUtils to nifi-nar-utils. This is needed so that the InstanceClassLoader has access to it in nifi-framework-nar-utils. The current location in nifi-framework-components would require me to add it as a dependency to nifi-framework-nar-utils which I think would be overkill. nifi-nar-utils seems like a good location for this class to allow it's use in both nifi-framework-components and nifi-framework-nar-utils. [1] https://github.com/search?q=repo%3Aapache%2Fnifi%20%40OnShutdown&type=code . [2] https://github.com/apache/nifi/blob/main/nifi-extension-bundles/nifi-standard-services/nifi-distributed-cache-services-bundle/nifi-distributed-cache-server/src/main/java/org/apache/nifi/distributed/cache/server/map/MapCacheServer.java#L37 [3] https://github.com/apache/nifi/blob/main/nifi-extension-bundles/nifi-standard-services/nifi-distributed-cache-services-bundle/nifi-distributed-cache-server/src/main/java/org/apache/nifi/distributed/cache/server/SetCacheServer.java#L32 [4] https://github.com/apache/nifi/blob/main/nifi-framework-bundle/nifi-framework/nifi-framework-nar-utils/src/main/java/org/apache/nifi/init/ConfigurableComponentInitializer.java#L28 [5] https://github.com/search?q=repo%3Aapache%2Fnifi+teardown&type=code&p=4 [6] https://github.com/search?q=repo%3Aapache%2Fnifi-maven%20teardown&type=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]
