NPE correction when informing removed project in the `listProjectRoles` by Tonitzpp · Pull Request #13022 · apache/cloudstack

Tonitzpp · 2026-04-14T12:46:01Z

Description

Currently, when using the listProjectRoles API, passing the projectid parameter of a removed project results in an NPE. This PR changes this behavior to return the message Failed to find project by ID instead.

Types of changes

Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change which adds functionality)
Bug fix (non-breaking change which fixes an issue)
Enhancement (improves an existing feature and functionality)
Cleanup (Code refactoring and cleanup, that may add test cases)
Build/CI
Test (unit or integration test code)

Feature/Enhancement Scale

Major
Minor

Bug Severity

Screenshots (if appropriate):

Before the changes

After the changes

How Has This Been Tested?

To perform the tests, a test project was created along with a project role. The project was deleted and the listProjectRoles API was called passing the ID of the project role. This showed the new message.

codecov · 2026-04-14T16:49:32Z

Codecov Report

❌ Patch coverage is 0% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 18.08%. Comparing base (3bd5410) to head (63f0803).
⚠️ Report is 181 commits behind head on main.

Files with missing lines	Patch %	Lines
...command/admin/acl/project/ListProjectRolesCmd.java	0.00%	10 Missing ⚠️
.../apache/cloudstack/acl/ProjectRoleManagerImpl.java	0.00%	3 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #13022      +/-   ##
============================================
+ Coverage     18.02%   18.08%   +0.05%     
- Complexity    16450    16705     +255     
============================================
  Files          5968     6037      +69     
  Lines        537086   542443    +5357     
  Branches      65961    66423     +462     
============================================
+ Hits          96820    98081    +1261     
- Misses       429346   433347    +4001     
- Partials      10920    11015      +95

Flag	Coverage Δ
uitests	`3.52% <ø> (-0.01%)`	⬇️
unittests	`19.24% <0.00%> (+0.05%)`	⬆️

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.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot

Pull request overview

Fixes a NullPointerException triggered by calling the listProjectRoles API with a projectid that refers to a removed project, changing the behavior to return a parameter error instead of crashing.

Changes:

Update ProjectRoleManagerImpl.findProjectRoles to return an empty list when projectId is null (instead of returning null).
Add explicit project existence validation in ListProjectRolesCmd.execute() and throw InvalidParameterValueException when the project cannot be found.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
server/src/main/java/org/apache/cloudstack/acl/ProjectRoleManagerImpl.java	Prevents null list returns from `findProjectRoles` when `projectId` is null.
api/src/main/java/org/apache/cloudstack/api/command/admin/acl/project/ListProjectRolesCmd.java	Validates the project exists before listing roles and throws a parameter exception when not found.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

This PR aims to prevent a NullPointerException in the listProjectRoles API when the caller provides a projectid for a removed/non-existent project, changing the behavior to return a clear parameter error instead.

Changes:

Validate the provided projectId in ListProjectRolesCmd.execute() and throw an InvalidParameterValueException when the project can’t be found.
Avoid per-role project lookup when building ProjectRoleResponse by passing the project UUID through the response setup method.
Adjust ProjectRoleManagerImpl.findProjectRoles() to return an empty list when projectId is null (instead of returning null).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
`server/src/main/java/org/apache/cloudstack/acl/ProjectRoleManagerImpl.java`	Returns an empty list when `projectId` is null to reduce NPE risk from null list returns.
`api/src/main/java/org/apache/cloudstack/api/command/admin/acl/project/ListProjectRolesCmd.java`	Adds project existence validation and avoids NPE when a project has been removed by not dereferencing a null project during response building.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

DaanHoogland · 2026-04-28T09:46:39Z

@blueorangutan package

blueorangutan · 2026-04-28T09:48:05Z

@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress.

Copilot

Pull request overview

Fixes an NPE in the listProjectRoles API when a removed project is supplied as projectid, by validating project existence up-front and avoiding repeated project lookups while building responses.

Changes:

Validate the provided project ID in ListProjectRolesCmd.execute() and throw an InvalidParameterValueException when the project can’t be found.
Avoid calling _projectService.getProject(...).getUuid() per role by reusing the resolved project UUID.
Adjust ProjectRoleManagerImpl.findProjectRoles to return an empty list for null project IDs.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
server/src/main/java/org/apache/cloudstack/acl/ProjectRoleManagerImpl.java	Changes project-role listing behavior for null/invalid project IDs (now returns empty list for null).
api/src/main/java/org/apache/cloudstack/api/command/admin/acl/project/ListProjectRolesCmd.java	Adds explicit project existence validation and avoids the per-role project lookup that caused the NPE.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

blueorangutan · 2026-04-28T10:38:31Z

Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 17630

sureshanaparti · 2026-04-29T04:13:31Z

@Tonitzpp can you check the build failure.

DaanHoogland · 2026-05-07T07:36:22Z

@blueorangutan package

blueorangutan · 2026-05-07T07:38:04Z

@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2026-05-07T08:42:24Z

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 17749

weizhouapache · 2026-05-07T14:36:49Z

-        if (projectId == null || projectId < 1L || projectDao.findById(projectId) == null) {
-            logger.warn("Invalid project ID provided");
-            return null;
+        if (projectId == null) {


if projectid=-1, does it work before/after this change ?

Previously, the API returned an NPE with projectid = -1 because here it returns null, and in the calling method, it tries to iterate through this null object. Now it doesn't return the NPE because an empty list will be returned in projRoleDao.findAllRoles; since there's no need to query the database, I'll change it to keep the if statement the same and return an empty list directly.

weizhouapache · 2026-05-07T14:37:43Z

@Tonitzpp
does the issue exist in main branch only ?
if it exists in older branches, can you re-target to 4.20/4.22 ?

boring-cyborg Bot added component:api component:projects labels Apr 14, 2026

DaanHoogland requested a review from Copilot April 15, 2026 05:57

Copilot started reviewing on behalf of DaanHoogland April 15, 2026 05:57 View session

DaanHoogland reviewed Apr 15, 2026

View reviewed changes

Comment thread api/src/main/java/org/apache/cloudstack/api/command/admin/acl/project/ListProjectRolesCmd.java

Copilot AI reviewed Apr 15, 2026

View reviewed changes

Tonitzpp changed the title ~~NPE correction when informing removed project in the 'listProjectRoles'~~ NPE correction when informing removed project in the listProjectRoles Apr 17, 2026

sureshanaparti requested a review from Copilot April 21, 2026 05:57

Copilot started reviewing on behalf of sureshanaparti April 21, 2026 05:58 View session

Copilot AI reviewed Apr 21, 2026

View reviewed changes

Comment thread api/src/main/java/org/apache/cloudstack/api/command/admin/acl/project/ListProjectRolesCmd.java Outdated

Comment thread api/src/main/java/org/apache/cloudstack/api/command/admin/acl/project/ListProjectRolesCmd.java Outdated

DaanHoogland requested a review from Copilot April 28, 2026 09:46

Copilot started reviewing on behalf of DaanHoogland April 28, 2026 09:46 View session

Copilot AI reviewed Apr 28, 2026

View reviewed changes

Comment thread server/src/main/java/org/apache/cloudstack/acl/ProjectRoleManagerImpl.java

DaanHoogland reviewed Apr 29, 2026

View reviewed changes

Comment thread api/src/main/java/org/apache/cloudstack/api/command/admin/acl/project/ListProjectRolesCmd.java Outdated

weizhouapache reviewed May 7, 2026

View reviewed changes

NPE correction when informing removed project in the 'listProjectRoles'

f1e0864

Tonitzpp force-pushed the npe-in-listprojects branch from 63f0803 to f1e0864 Compare May 13, 2026 13:03

boring-cyborg Bot added component:packaging component:UI labels May 13, 2026

Tonitzpp changed the base branch from main to 4.22 May 13, 2026 13:03

Validate project ID in findProjectRoles method

db353f1

Conversation

Tonitzpp commented Apr 14, 2026

Description

Types of changes

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

Uh oh!

codecov Bot commented Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

DaanHoogland commented Apr 28, 2026

Uh oh!

blueorangutan commented Apr 28, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

blueorangutan commented Apr 28, 2026

Uh oh!

sureshanaparti commented Apr 29, 2026

Uh oh!

Uh oh!

DaanHoogland commented May 7, 2026

Uh oh!

blueorangutan commented May 7, 2026

Uh oh!

blueorangutan commented May 7, 2026

Uh oh!

weizhouapache May 7, 2026

Choose a reason for hiding this comment

Uh oh!

Tonitzpp May 15, 2026

Choose a reason for hiding this comment

Uh oh!

weizhouapache commented May 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

codecov Bot commented Apr 14, 2026 •

edited

Loading