- Notifications
You must be signed in to change notification settings - Fork 146
Rename resolveReference to resolve and add <T> List<T> getBeans(Class<T> clazz) #2492
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
WalkthroughA substantial refactoring restructures the bean registry and YAML parsing subsystem. Bean registry classes move to a new package; APIs are renamed and simplified; a new async event-handling layer is introduced via Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes Areas requiring extra attention:
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| Merge #2488 first |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (6)
annot/src/test/java/com/predic8/membrane/annot/YAMLBeanParsingTest.java (1)
70-84: LGTM!Test correctly uses the renamed
resolvemethod. The test logic for singleton verification is sound.Consider renaming the test method from
singletonResolveReferencetosingletonResolveto match the new API naming.annot/src/test/java/com/predic8/membrane/annot/util/YamlParser.java (1)
18-18: Consider explicit imports over wildcard.Wildcard imports can obscure dependencies. Consider explicit imports for clarity:
import com.predic8.membrane.annot.beanregistry.BeanCacheObserver; import com.predic8.membrane.annot.beanregistry.BeanDefinition; import com.predic8.membrane.annot.beanregistry.BeanDefinitionChanged; import com.predic8.membrane.annot.beanregistry.BeanRegistry; import com.predic8.membrane.annot.beanregistry.BeanRegistryImplementation;annot/src/main/java/com/predic8/membrane/annot/yaml/BeanCacheObserver.java (1)
17-19: Remove unused import.
BeanRegistryImplementationis imported but not used in this interface. Consider removing it to keep the imports clean.🔎 Proposed fix
import com.predic8.membrane.annot.beanregistry.BeanDefinition; import com.predic8.membrane.annot.beanregistry.BeanDefinitionChanged; -import com.predic8.membrane.annot.beanregistry.BeanRegistryImplementation;annot/src/main/java/com/predic8/membrane/annot/beanregistry/AsyncBeanCollector.java (1)
22-25: Consider documenting thatisLastis intentionally ignored.The
isLastparameter is ignored because the async wrapper recalculates it based on queue state. A brief comment would clarify this design decision.@Override public void handle(ChangeEvent changeEvent, boolean isLast) { + // isLast is intentionally ignored; recalculated based on queue state in worker thread changeEvents.add(changeEvent); }annot/src/main/java/com/predic8/membrane/annot/beanregistry/ChangeEvent.java (1)
17-19: Unused imports.
WatchActionand its static import are not used in this file. They may be leftover from refactoring or intended forBeanDefinitionChangedwhich is defined in a separate file.🔎 Proposed fix
package com.predic8.membrane.annot.beanregistry; -import com.predic8.membrane.annot.yaml.WatchAction; - -import static com.predic8.membrane.annot.yaml.WatchAction.*; - public sealed interface ChangeEvent permits BeanDefinitionChanged, StaticConfigurationLoaded {core/src/main/java/com/predic8/membrane/core/cli/RouterCLI.java (1)
17-19: Unused import:BeanRegistry.
BeanRegistryis imported but not used in this file. OnlyBeanCollectorandBeanRegistryImplementationare referenced.🔎 Proposed fix
import com.predic8.membrane.annot.beanregistry.BeanCollector; -import com.predic8.membrane.annot.beanregistry.BeanRegistry; import com.predic8.membrane.annot.beanregistry.BeanRegistryImplementation;
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (28)
annot/pom.xmlannot/src/main/java/com/predic8/membrane/annot/bean/BeanFactory.javaannot/src/main/java/com/predic8/membrane/annot/beanregistry/AsyncBeanCollector.javaannot/src/main/java/com/predic8/membrane/annot/beanregistry/BeanCollector.javaannot/src/main/java/com/predic8/membrane/annot/beanregistry/BeanContainer.javaannot/src/main/java/com/predic8/membrane/annot/beanregistry/BeanDefinition.javaannot/src/main/java/com/predic8/membrane/annot/beanregistry/BeanDefinitionChanged.javaannot/src/main/java/com/predic8/membrane/annot/beanregistry/BeanRegistry.javaannot/src/main/java/com/predic8/membrane/annot/beanregistry/BeanRegistryImplementation.javaannot/src/main/java/com/predic8/membrane/annot/beanregistry/ChangeEvent.javaannot/src/main/java/com/predic8/membrane/annot/yaml/BeanCacheObserver.javaannot/src/main/java/com/predic8/membrane/annot/yaml/BeanRegistryImplementation.javaannot/src/main/java/com/predic8/membrane/annot/yaml/GenericYamlParser.javaannot/src/main/java/com/predic8/membrane/annot/yaml/MethodSetter.javaannot/src/main/java/com/predic8/membrane/annot/yaml/ParsingContext.javaannot/src/main/java/com/predic8/membrane/annot/yaml/WatchAction.javaannot/src/test/java/com/predic8/membrane/annot/YAMLBeanParsingTest.javaannot/src/test/java/com/predic8/membrane/annot/YAMLComponentsParsingTest.javaannot/src/test/java/com/predic8/membrane/annot/util/CompilerHelper.javaannot/src/test/java/com/predic8/membrane/annot/util/InMemoryClassLoader.javaannot/src/test/java/com/predic8/membrane/annot/util/StructureAssertionUtil.javaannot/src/test/java/com/predic8/membrane/annot/util/YamlParser.javacore/src/main/java/com/predic8/membrane/core/Router.javacore/src/main/java/com/predic8/membrane/core/cli/RouterCLI.javacore/src/main/java/com/predic8/membrane/core/config/spring/k8s/Envelope.javacore/src/main/java/com/predic8/membrane/core/kubernetes/KubernetesWatcher.javacore/src/test/java/com/predic8/membrane/core/config/spring/k8s/EnvelopeTest.javacore/src/test/java/com/predic8/membrane/core/kubernetes/GenericYamlParserTest.java
💤 Files with no reviewable changes (1)
- annot/src/main/java/com/predic8/membrane/annot/yaml/BeanRegistryImplementation.java
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-12-19T09:01:40.311Z
Learnt from: rrayst Repo: membrane/api-gateway PR: 2441 File: annot/pom.xml:0-0 Timestamp: 2025-12-19T09:01:40.311Z Learning: In annot/pom.xml, ensure log4j dependencies log4j-slf4j2-impl and log4j-core have <scope>test</scope>. In the distribution module (separate pom.xml), these dependencies should use the normal compile-scope. This guidance applies to all pom.xml files under the annot and distribution module structure where these dependencies appear; verify both modules are configured accordingly to reflect testing vs. runtime usage. Applied to files:
annot/pom.xml
📚 Learning: 2025-09-22T19:49:29.473Z
Learnt from: predic8 Repo: membrane/api-gateway PR: 2156 File: pom.xml:0-0 Timestamp: 2025-09-22T19:49:29.473Z Learning: In the Membrane API Gateway project, jackson-annotations is pinned to version "2.20" instead of using ${jackson.version} (2.20.0) because version 2.20.0 is not available in Maven repository yet for jackson-annotations, while it is available for jackson-core and jackson-databind. This version inconsistency is intentional and necessary due to different release schedules between Jackson modules. Applied to files:
core/src/main/java/com/predic8/membrane/core/kubernetes/KubernetesWatcher.java
📚 Learning: 2025-10-23T13:04:11.388Z
Learnt from: predic8 Repo: membrane/api-gateway PR: 2240 File: annot/src/main/java/com/predic8/membrane/annot/generator/JsonSchemaGenerator.java:210-215 Timestamp: 2025-10-23T13:04:11.388Z Learning: In JsonSchemaGenerator.java, when allowForeign is true for non-list child elements, use a property named "ref" (not "$ref") with type "string" to provide IDE completion hints for Spring bean references while avoiding JSON Schema keyword collision. Applied to files:
annot/src/main/java/com/predic8/membrane/annot/bean/BeanFactory.java
🧬 Code graph analysis (5)
annot/src/main/java/com/predic8/membrane/annot/yaml/MethodSetter.java (1)
annot/src/main/java/com/predic8/membrane/annot/yaml/McYamlIntrospector.java (1)
McYamlIntrospector(28-185)
annot/src/main/java/com/predic8/membrane/annot/yaml/GenericYamlParser.java (1)
annot/src/main/java/com/predic8/membrane/annot/beanregistry/BeanDefinition.java (1)
BeanDefinition(21-103)
core/src/main/java/com/predic8/membrane/core/Router.java (1)
annot/src/main/java/com/predic8/membrane/annot/beanregistry/BeanDefinition.java (1)
BeanDefinition(21-103)
core/src/main/java/com/predic8/membrane/core/cli/RouterCLI.java (1)
annot/src/main/java/com/predic8/membrane/annot/beanregistry/BeanRegistryImplementation.java (1)
BeanRegistryImplementation(29-176)
annot/src/main/java/com/predic8/membrane/annot/yaml/BeanCacheObserver.java (2)
annot/src/main/java/com/predic8/membrane/annot/beanregistry/BeanDefinition.java (1)
BeanDefinition(21-103)annot/src/main/java/com/predic8/membrane/annot/beanregistry/BeanRegistryImplementation.java (1)
BeanRegistryImplementation(29-176)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (java)
🔇 Additional comments (45)
core/src/test/java/com/predic8/membrane/core/config/spring/k8s/EnvelopeTest.java (1)
27-27: LGTM! Import path updated to reflect package relocation.The BeanRegistry import has been correctly updated to use the new
beanregistrypackage.annot/src/test/java/com/predic8/membrane/annot/util/StructureAssertionUtil.java (1)
17-17: LGTM! Import path updated to reflect package relocation.The BeanRegistry import has been correctly updated to use the new
beanregistrypackage.annot/src/main/java/com/predic8/membrane/annot/yaml/ParsingContext.java (1)
18-18: LGTM! Import path updated to reflect package relocation.The BeanRegistry import has been correctly updated to use the new
beanregistrypackage.core/src/main/java/com/predic8/membrane/core/config/spring/k8s/Envelope.java (1)
19-19: LGTM! Import path updated to reflect package relocation.The BeanRegistry import has been correctly updated to use the new
beanregistrypackage.annot/src/test/java/com/predic8/membrane/annot/util/CompilerHelper.java (1)
16-16: LGTM! Import improved from wildcard to specific import.The change from wildcard import to specific import is better practice and correctly reflects the BeanRegistry package relocation.
annot/src/test/java/com/predic8/membrane/annot/YAMLComponentsParsingTest.java (1)
18-18: LGTM! Import path updated to reflect package relocation.The BeanRegistry import has been correctly updated to use the new
beanregistrypackage.annot/src/main/java/com/predic8/membrane/annot/yaml/MethodSetter.java (2)
107-107: LGTM! API method renamed fromresolveReferencetoresolve.The call has been correctly updated to use the renamed method. This change aligns with the PR objectives to simplify the BeanRegistry API.
130-146: LGTM! API method renamed fromresolveReferencetoresolve.The call at Line 134 has been correctly updated to use the renamed method. The error handling and type checking logic remain unchanged.
annot/pom.xml (1)
59-63: Approved. SpotBugs annotations added for static analysis support.Version 4.9.8 is current and officially released. The compile scope is correct for annotations used in code documentation.
annot/src/test/java/com/predic8/membrane/annot/util/InMemoryClassLoader.java (1)
111-115: LGTM!The delegation check correctly references the new package location for
BeanRegistry(com.predic8.membrane.annot.beanregistry.BeanRegistry), ensuring the test class loader delegates to the parent for this interface.annot/src/main/java/com/predic8/membrane/annot/bean/BeanFactory.java (2)
18-18: LGTM!Import correctly references the relocated
BeanRegistryinterface.
240-251: LGTM!The method rename from
resolveReferencetoresolveis applied correctly. The null-safety check and type-compatibility validation logic remain intact.core/src/test/java/com/predic8/membrane/core/kubernetes/GenericYamlParserTest.java (2)
18-19: LGTM!Imports correctly updated to reference
BeanDefinitionandBeanRegistryfrom the newbeanregistrypackage.
335-359: LGTM!
TestRegistrycorrectly implements the renamedresolvemethod. The stub implementations for otherBeanRegistrymethods are appropriate for these parameterized tests that only exercise reference resolution.annot/src/main/java/com/predic8/membrane/annot/beanregistry/BeanDefinitionChanged.java (1)
1-11: LGTM!Well-designed record for signaling bean definition changes. The pairing of
WatchActionandBeanDefinitionclearly captures the event semantics.Consider adding null validation if
actionorbdshould never be null at construction time:public record BeanDefinitionChanged( WatchAction action, BeanDefinition bd) implements ChangeEvent { public BeanDefinitionChanged { Objects.requireNonNull(action, "action must not be null"); Objects.requireNonNull(bd, "bd must not be null"); } }annot/src/main/java/com/predic8/membrane/annot/yaml/WatchAction.java (1)
16-30: LGTM!The predicate methods (
isDeleted(),isModified(),isAdded()) provide a more readable API at call sites compared to direct enum comparisons.annot/src/test/java/com/predic8/membrane/annot/YAMLBeanParsingTest.java (3)
17-17: LGTM!Import correctly updated to the new
beanregistrypackage.
86-100: LGTM!Test correctly uses the renamed
resolvemethod for prototype scope verification.
102-113: LGTM!Error handling test correctly uses the renamed
resolvemethod.annot/src/main/java/com/predic8/membrane/annot/yaml/GenericYamlParser.java (4)
23-24: LGTM!Imports correctly reference
BeanDefinitionandBeanRegistryfrom the newbeanregistrypackage.
92-103: LGTM!Javadoc accurately updated to reflect that the method returns "list of parsed bean definitions" rather than a fully populated registry.
262-268: LGTM!The
resolvemethod call correctly replacesresolveReference, with existing error handling preserved.
293-297: LGTM!The
$refhandling path correctly uses the renamedresolvemethod.annot/src/test/java/com/predic8/membrane/annot/util/YamlParser.java (2)
51-56: LGTM!The two-step initialization pattern is correctly implemented:
- Create
BeanRegistryImplementationwith observer and grammar- Parse YAMLs via
parseYamls- Assign to the
beanRegistryfield (typed asBeanRegistryinterface)This allows the implementation-specific
parseYamlsmethod to be called before treating the registry through its interface.
84-88: LGTM!The
handleBeanEventsignature correctly updated to acceptBeanDefinitionChanged bdcinstead ofBeanDefinition bd, aligning with the new event model that pairs action with definition.core/src/main/java/com/predic8/membrane/core/kubernetes/KubernetesWatcher.java (2)
107-117: Consider whetherisLast=trueis always appropriate for watch events.The
handlemethod is always called withisLast=true. This works correctly for individual Kubernetes watch events processed one at a time. If batch processing is ever introduced, this would need adjustment to properly signal the last event in a batch.
49-51: No action required. Router implements BeanCacheObserver, so the constructor call is type-safe and correct. The code at lines 49-51 passes router to BeanRegistryImplementation, which expects a BeanCacheObserver as its first parameter—this is valid.Likely an incorrect or invalid review comment.
annot/src/main/java/com/predic8/membrane/annot/beanregistry/BeanCollector.java (2)
22-29: LGTM!The
parseYamlsdefault method correctly orchestrates YAML ingestion: parsing, emittingBeanDefinitionChangedevents with properisLastflags, signalingStaticConfigurationLoaded, and finally callingstart(). The empty list case is handled correctly sinceStaticConfigurationLoadedis always emitted.
31-41: Clean interface design.The abstract methods
handleandstartare well-documented and provide a clear contract for implementations. The separation between event handling and lifecycle management is appropriate.annot/src/main/java/com/predic8/membrane/annot/yaml/BeanCacheObserver.java (1)
47-47: API change fromBeanDefinitiontoBeanDefinitionChangedis appropriate.Using
BeanDefinitionChangedprovides theWatchActioncontext (ADDED/MODIFIED/DELETED) alongside theBeanDefinition, which is more informative for event handling. This aligns with the new event-driven architecture.annot/src/main/java/com/predic8/membrane/annot/beanregistry/BeanContainer.java (1)
3-26: LGTM - simple and clean container class.The design is straightforward. The
volatilemodifier onsingletonensures visibility across threads. If singleton initialization is performed in a single-threaded context (e.g., during bean registry startup) with only concurrent reads afterward, this is appropriate.annot/src/main/java/com/predic8/membrane/annot/beanregistry/BeanRegistry.java (1)
20-28: Clean API refinement.The changes improve the interface:
resolveis more concise thanresolveReferencewhile maintaining clarity.- The generic
getBeans(Class<T> clazz)enables type-safe retrieval, complementing the existinggetBeans()for unfiltered access.- Separation of concerns is improved by moving lifecycle methods (
registerBeanDefinitions,start) toBeanCollector.annot/src/main/java/com/predic8/membrane/annot/beanregistry/ChangeEvent.java (1)
21-28: LGTM!The sealed interface design cleanly separates configuration events.
StaticConfigurationLoadedbeing package-private appropriately restricts its creation to internal components.core/src/main/java/com/predic8/membrane/core/Router.java (2)
18-19: LGTM!The imports correctly reflect the refactored types:
BeanDefinitionfor theisActivatablemethod andBeanDefinitionChangedfor the event-drivenhandleBeanEventmethod.
654-677: LGTM!The refactored method correctly uses
BeanDefinitionChangedto encapsulate both the action type and bean definition. The patternbdc.action().isAdded()cleanly separates concerns, and accessing the wrapped definition viabdc.bd()is straightforward.annot/src/main/java/com/predic8/membrane/annot/beanregistry/BeanRegistryImplementation.java (6)
50-63: LGTM!The
definemethod correctly dispatches betweenBeanFactoryfor explicit "bean" kinds andGenericYamlParserfor other types. Exception wrapping is appropriate for propagation.
69-87: LGTM!The
handlemethod cleanly uses pattern matching for deconstruction. The flow correctly processesStaticConfigurationLoadedandBeanDefinitionChangedevents, triggering activation when appropriate.
124-139: LGTM!The
resolvemethod correctly handles prototype vs singleton scope: prototypes are always instantiated fresh, while singletons are cached in the container. The exception message is clear when a reference is not found.
145-161: Inconsistent filtering betweengetBeans()andgetBeans(Class<T>).
getBeans()filters out components via!bd.getDefinition().isComponent(), butgetBeans(Class<T>)does not apply this filter. This may return component beans unexpectedly when filtering by class.Verify if this inconsistency is intentional. If not:
🔎 Proposed fix to add component filtering
@Override public <T> List<T> getBeans(Class<T> clazz) { return bcs.values().stream() + .filter(bd -> !bd.getDefinition().isComponent()) .map(BeanContainer::getSingleton) .filter(Objects::nonNull) .filter(clazz::isInstance) .map(clazz::cast) .toList(); }
168-175: LGTM!The prototype scope detection correctly differentiates between regular definitions (using
isPrototype()) and explicit "bean" kinds (reading from JSON). Defaulting toSINGLETONis a sensible convention.
37-41: Thread-safety ofuidsToActivaterequires either explicit documentation or synchronization.In production usage (KubernetesWatcher),
BeanRegistryImplementationis wrapped withAsyncBeanCollector, which enforces thread confinement by processing allhandle()calls from a single background thread. However, the code doesn't document this assumption, and direct usage withoutAsyncBeanCollector(as in RouterCLI and tests) could allow concurrent modification of the plainLinkedHashSetif called from multiple threads.Consider either: (1) documenting that
handle()must be called from a single thread, or (2) makinguidsToActivatethread-safe (e.g.,Collections.synchronizedSet()or adding synchronization) to prevent accidental misuse when the wrapper is omitted.annot/src/main/java/com/predic8/membrane/annot/beanregistry/BeanDefinition.java (3)
14-19: LGTM!Package relocation to
beanregistryand theWatchActionimport support the new factory method pattern.
34-50: LGTM!The refactoring cleanly separates concerns: the private constructor handles K8S-specific JSON parsing, while
create4Kubernetesreturns aBeanDefinitionChangedthat encapsulates both the action and definition. This aligns well with the event-driven architecture introduced in this PR.
52-102: LGTM!The public constructor and accessor methods remain intact for non-Kubernetes use cases. The utility methods (
isComponent,isBean,isPrototype) provide clean abstractions for scope and type determination.core/src/main/java/com/predic8/membrane/core/cli/RouterCLI.java (1)
172-173: Review parseYamls method signature for potential simplification.The
grammarparameter passed toparseYamlsduplicates the instance field stored during construction. Both current usages pass identical grammar instances to the constructor and method. Consider whetherparseYamlsshould use the stored instance grammar instead of requiring the parameter, or document if allowing different grammars per parse operation is intentional design.
| t = new Thread(() -> { | ||
| while (true) { | ||
| try { | ||
| ChangeEvent changeEvent = changeEvents.take(); | ||
| delegate.handle(changeEvent, changeEvents.isEmpty()); | ||
| } catch (InterruptedException e) { | ||
| break; | ||
| } | ||
| } | ||
| }); | ||
| t.start(); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thread lifecycle and configuration issues.
Several concerns with the background thread:
-
No shutdown mechanism: There's no
stop()method to gracefully terminate the thread. Consider adding one that interrupts the thread and awaits termination. -
Non-daemon thread: The thread will prevent JVM shutdown. Consider setting
t.setDaemon(true)or providing explicit shutdown. -
Thread naming: Add a descriptive name for debugging:
t.setName("AsyncBeanCollector-worker"). -
Exception handling: If
delegate.handle()throws an unchecked exception, the thread will terminate silently.
🔎 Proposed improvements
@Override public synchronized void start() { if (t != null) return; t = new Thread(() -> { while (true) { try { ChangeEvent changeEvent = changeEvents.take(); delegate.handle(changeEvent, changeEvents.isEmpty()); } catch (InterruptedException e) { break; + } catch (Exception e) { + // Log and continue processing + e.printStackTrace(); } } }); + t.setName("AsyncBeanCollector-worker"); + t.setDaemon(true); t.start(); } + + public synchronized void stop() { + if (t != null) { + t.interrupt(); + try { + t.join(5000); + } catch (InterruptedException ignored) { + Thread.currentThread().interrupt(); + } + t = null; + } + }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In annot/src/main/java/com/predic8/membrane/annot/beanregistry/AsyncBeanCollector.java around lines 31-42, the background thread lacks lifecycle controls and resilience: add a public stop() method that interrupts the worker thread and waits (join with a timeout) for it to terminate, set the thread to daemon (t.setDaemon(true)) and give it a descriptive name (t.setName("AsyncBeanCollector-worker")), and wrap delegate.handle(...) in a try/catch(Throwable) that logs unexpected runtime errors and continues the loop so the thread does not die silently; ensure InterruptedException handling breaks the loop and preserves interrupt status as appropriate. | for (String uid : uidsToRemove) | ||
| uidsToActivate.remove(uid); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Removal from uidsToActivate will never work.
uidsToActivate is a Set<UidAction>, but the removal loop attempts to remove String UIDs. Since UidAction is a record with (String uid, WatchAction action), calling uidsToActivate.remove(uid) where uid is a String will never match any UidAction element.
🔎 Proposed fix
- for (String uid : uidsToRemove) - uidsToActivate.remove(uid); + uidsToActivate.removeIf(ua -> uidsToRemove.contains(ua.uid()));Or alternatively, clear processed items inline:
private void activationRun() { - Set<String> uidsToRemove = new HashSet<>(); + Iterator<UidAction> it = uidsToActivate.iterator(); - for (UidAction uidAction : uidsToActivate) { + while (it.hasNext()) { + UidAction uidAction = it.next(); BeanContainer bc = bcs.get(uidAction.uid()); try { // ... existing code ... - uidsToRemove.add(bc.getDefinition().getUid()); + it.remove(); } catch (Exception e) { // ... existing code ... } } - for (String uid : uidsToRemove) - uidsToActivate.remove(uid); }🤖 Prompt for AI Agents
In annot/src/main/java/com/predic8/membrane/annot/beanregistry/BeanRegistryImplementation.java around lines 113-114, the loop calls uidsToActivate.remove(uid) using a String uid while uidsToActivate is a Set<UidAction>, so removals never match; replace the removal logic to remove the UidAction entries whose uid field equals the String (for example, use uidsToActivate.removeIf(ua -> uidsToRemove.contains(ua.uid())) or iterate with an Iterator and remove when ua.uid().equals(uid)), or simply clear processed items inline after mapping, ensuring type-correct removal of UidAction instances rather than attempting to remove Strings. | This pull request needs "/ok-to-test" from an authorized committer. |
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.