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

Fix NPE in Organization Audience Shared Role Deletion #432

Conversation

HasiniSama
Copy link
Contributor

Purpose

Fixes: wso2/product-is#22394

An NPE gets thrown when deleting Organization audience shared roles from an application. This is reproduced when switching the role audience to Application audience.

The NPE is thrown here, as the userRealm is null because the tenant flow hasn't started in the thread.

Approach

Started tenant flow before calling the roleManagementService.getAssociatedApplicationByRoleId() method.

Copy link

codecov bot commented Jan 24, 2025

Codecov Report

Attention: Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.

Project coverage is 48.72%. Comparing base (b1959c9) to head (c16f172).
Report is 41 commits behind head on main.

Files with missing lines Patch % Lines
...gement/handler/listener/SharedRoleMgtListener.java 0.00% 6 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main     #432      +/-   ##
============================================
- Coverage     48.77%   48.72%   -0.06%     
+ Complexity     1280     1258      -22     
============================================
  Files           143      144       +1     
  Lines          8024     7986      -38     
  Branches        979      990      +11     
============================================
- Hits           3914     3891      -23     
+ Misses         3785     3770      -15     
  Partials        325      325              
Flag Coverage Δ
unit 37.26% <0.00%> (-0.45%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@HasiniSama
Copy link
Contributor Author

This is tested for the following flows

  • Switching the role audience to Application audience from Organization audience. (application update post-listener)
  • Delete a shared application with roles set to the Organization audience. (application deletion post-listener)
  • Unshare an application with roles set to the Organization audience. (application deletion post-listener)

@HasiniSama HasiniSama force-pushed the npe-shared-org-audience-role-deletion branch from b1acca8 to b1959c9 Compare January 26, 2025 11:19
@HasiniSama HasiniSama force-pushed the npe-shared-org-audience-role-deletion branch from 0d33d3b to 2209f97 Compare January 26, 2025 16:07
@HasiniSama
Copy link
Contributor Author

However, the IDE shows the code coverage as 100% for lines added:

Screenshot 2025-01-26 at 22 30 31

sahandilshan
sahandilshan previously approved these changes Jan 26, 2025
@SujanSanjula96 SujanSanjula96 merged commit 30e5b6c into wso2-extensions:main Jan 26, 2025
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NPE on changing role audience of a shared app
4 participants