server: ignore projectid=-1 for finding owner#12695
server: ignore projectid=-1 for finding owner#12695shwstppr wants to merge 1 commit intoapache:4.20from
Conversation
Fixes apache#12693 Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.20 #12695 +/- ##
============================================
- Coverage 16.25% 16.25% -0.01%
Complexity 13425 13425
============================================
Files 5662 5662
Lines 500155 500155
Branches 60738 60738
============================================
- Hits 81319 81310 -9
- Misses 409749 409759 +10
+ Partials 9087 9086 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@blueorangutan package |
There was a problem hiding this comment.
Pull request overview
This PR fixes an issue where the UI is unable to list service offerings when the projects toggle button is enabled. The fix treats projectId=-1 as a special sentinel value (meaning "all projects") and skips project validation for this value in the finalizeOwner method.
Changes:
- Modified
finalizeOwnermethod to ignoreprojectId=-1when determining entity owner - Reordered imports alphabetically (StringUtils and Ternary)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if ((accountName != null && domainId != null) && projectId != null) { | ||
| throw new InvalidParameterValueException("ProjectId and account/domainId can't be specified together"); |
There was a problem hiding this comment.
The validation at line 2562 should also exclude projectId=-1 to be consistent with the fix at line 2566. When projectId=-1 is passed (indicating "all projects"), it should not conflict with specifying accountName and domainId together, since -1 is a special sentinel value rather than an actual project identifier. The condition should be updated to:
if ((accountName != null && domainId != null) && projectId != null && projectId != -1L)
| } | ||
|
|
||
| if (projectId != null) { | ||
| if (projectId != null && projectId != -1L) { |
There was a problem hiding this comment.
The change to handle projectId=-1 lacks test coverage. Consider adding tests to AccountManagerImplTest.java that verify the behavior when projectId=-1 is passed to finalizeOwner. Tests should cover scenarios such as:
- When projectId=-1 is passed alone, it should not throw an "Unable to find project" exception
- When projectId=-1 is passed with accountName and domainId, it should not throw the "ProjectId and account/domainId can't be specified together" exception
- The method should return the caller account when projectId=-1 is passed without other parameters
Description
Fixes #12693
projectid = -1 is used by UI and APIs to return entities from all projects. Therefore, it may make sense to ignore finding project for it when deducing entity owner.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?