NPE correction when informing removed project in the listProjectRoles

[Tonitzpp]

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

• BLOCKER
• Critical
• Major
• Minor
• Trivial

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]

Codecov Report

❌ Patch coverage is 0% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 17.67%. Comparing base (a289bb0) to head (db353f1).
⚠️ Report is 82 commits behind head on 4.22.

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

Additional details and impacted files

@@ Coverage Diff @@## 4.22 #13022 +/- ##
============================================
- Coverage 17.67% 17.67% -0.01% + Complexity 15792 15786 -6 
============================================
Files 5922 5922 Lines 533123 533125 +2 Branches 65201 65201 ============================================
- Hits 94246 94240 -6 - Misses 428236 428240 +4 - Partials 10641 10645 +4 

Flag	Coverage Δ
uitests	3.69% <ø> (ø)
unittests	18.75% <0.00%> (-0.01%)	⬇️

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

☔ View full report in Codecov by Harness.
📢 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]

@blueorangutan package

[blueorangutan]

@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]

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

[sureshanaparti]

@Tonitzpp can you check the build failure.

[DaanHoogland]

@blueorangutan package

[blueorangutan]

@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]

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

[weizhouapache]

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

[DaanHoogland]

@blueorangutan package

[blueorangutan]

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

[blueorangutan]

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

[DaanHoogland]

@blueorangutan test

[blueorangutan]

@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

[blueorangutan]

[SF] Trillian Build Failed (tid-16145)

[blueorangutan]

[SF] Trillian test result (tid-16157)
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 60370 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr13022-t16157-kvm-ol8.zip
Smoke tests completed. 146 look OK, 3 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_02_1_create_iso_with_checksum_sha256_negative	Error	66.91	test_iso.py
test_02_create_iso_with_checksum_sha256	Error	66.98	test_iso.py
test_03_create_iso_with_checksum_md5	Error	66.75	test_iso.py
test_04_create_iso_with_no_checksum	Error	66.75	test_iso.py
test_04_extract_Iso	Error	124.45	test_iso.py
ContextSuite context=TestListIdsParams>:teardown	Error	1.19	test_list_ids_parameter.py
test_01_vpn_usage	Error	1.15	test_usage.py