Skip to content
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

🐛 Avoid multiple GuiceLocator instances, and handle potential multiple KapuaMessageListeners #4066

Merged

Conversation

dseurotech
Copy link
Contributor

Should guicelocator fail at broker startup, multiple instances of (Kapua)MessageListener are istantiated, failing device login due to split request/response maps.
In general, guice modules should avoid doing logic, and should avoid failing. Guice cannot guarantee that singleton objects previously instantiated are killed - which in some cases can be very distruptive.
This pr both avoids multiple guiceLocator instantiation attempts within a single process, but also puts in place safeguards that ensure that message correlation will continue to work even if there are multiple (Kapua)MessageListener (possibly due to changes in the wiring).

Copy link

codecov bot commented Jun 28, 2024

Codecov Report

Attention: Patch coverage is 10.52632% with 34 lines in your changes missing coverage. Please review.

Project coverage is 16.78%. Comparing base (296efcd) to head (fae4a63).
Report is 4 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #4066      +/-   ##
=============================================
- Coverage      16.79%   16.78%   -0.01%     
  Complexity        22       22              
=============================================
  Files           2008     2008              
  Lines          52210    52229      +19     
  Branches        4399     4401       +2     
=============================================
- Hits            8770     8768       -2     
- Misses         43044    43064      +20     
- Partials         396      397       +1     
Files Coverage Δ
...artemis/plugin/security/ArtemisSecurityModule.java 0.00% <ø> (ø)
.../kapua/client/security/bean/ResponseContainer.java 0.00% <ø> (ø)
...java/org/eclipse/kapua/commons/jpa/DataSource.java 93.75% <ø> (ø)
...e/kapua/commons/jpa/KapuaEntityManagerFactory.java 54.34% <ø> (ø)
.../eclipse/kapua/locator/KapuaLocatorErrorCodes.java 100.00% <100.00%> (ø)
...se/kapua/client/security/ClientSecurityModule.java 83.33% <0.00%> (-16.67%) ⬇️
...ua/client/security/ServiceClientMessagingImpl.java 0.00% <0.00%> (ø)
.../eclipse/kapua/locator/guice/GuiceLocatorImpl.java 84.25% <50.00%> (-5.01%) ⬇️
...n/java/org/eclipse/kapua/locator/KapuaLocator.java 46.00% <0.00%> (-5.12%) ⬇️
...se/kapua/client/security/KapuaMessageListener.java 0.00% <0.00%> (ø)

... and 1 file with indirect coverage changes

@dseurotech dseurotech force-pushed the fix-avoid_multiple_MessageListeners branch from 4f884aa to 54c7ae6 Compare July 2, 2024 07:07
…Kapua)MessageListenr are istantiated, failing device login due to split map

Signed-off-by: dseurotech <davide.salvador@eurotech.com>
Signed-off-by: dseurotech <davide.salvador@eurotech.com>
@dseurotech dseurotech force-pushed the fix-avoid_multiple_MessageListeners branch from 54c7ae6 to fae4a63 Compare July 3, 2024 07:50
@Coduz Coduz added the Bug This is a bug or an unexpected behaviour. Fix it! label Jul 8, 2024
Copy link
Contributor

@riccardomodanese riccardomodanese left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do not agree with this solution to solve the locator initialization issue but anyway, I'll aprove it

@Coduz Coduz merged commit 2d110c4 into eclipse-kapua:develop Jul 8, 2024
33 of 35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug This is a bug or an unexpected behaviour. Fix it!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants