Fix: Return empty list instead of error when registry resource has no properties#4796
Fix: Return empty list instead of error when registry resource has no properties#4796SasinduDilshara wants to merge 1 commit intowso2:masterfrom
Conversation
… properties
When a registry resource exists but has no associated .properties file,
getResourceProperties() returns null. The code was incorrectly treating
this null as an error and returning {"list":"Error while fetching properties"}.
Fix: When propertiesList is null, return Utils.createJSONList(0) to produce
{"count":0,"list":[]} — an empty list — instead of an error string.
Fixes wso2#4767
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Issue AnalysisIssue Analysis — [Issue #4767]: Error message returned when there are no properties for a registry resourceClassification
Reproducibility
Root Cause AnalysisIn Properties propertiesList = microIntegratorRegistry.getResourceProperties(registryPath);
if (propertiesList != null) {
// build list JSON...
} else {
jsonBody = new JSONObject();
jsonBody.put(LIST, "Error while fetching properties"); // BUG: treats null as error
}
Fix: In File to fix: Test Coverage Assessment
Verified steps of the issueFix Verification ReportIssue: #4767 Reproduction Steps Executed
ResultBefore fix (expected): After fix (observed): The non-existent resource path still returns a proper error: EvidenceCase 1: Registry resource with no properties (the reported issue)Case 2: Non-existent resource path (error case unchanged)Patch application log |
| } else { | ||
| jsonBody = new JSONObject(); | ||
| jsonBody.put(LIST, "Error while fetching properties"); | ||
| jsonBody = Utils.createJSONList(0); | ||
| } |
There was a problem hiding this comment.
Log Improvement Suggestion No: 1
| } else { | |
| jsonBody = new JSONObject(); | |
| jsonBody.put(LIST, "Error while fetching properties"); | |
| jsonBody = Utils.createJSONList(0); | |
| } | |
| } else { | |
| log.warn("Failed to fetch registry properties for path: " + path); | |
| jsonBody = Utils.createJSONList(0); |
There was a problem hiding this comment.
AI Agent Log Improvement Checklist
- The log-related comments and suggestions in this review were generated by an AI tool to assist with identifying potential improvements. Purpose of reviewing the code for log improvements is to improve the troubleshooting capabilities of our products.
- Please make sure to manually review and validate all suggestions before applying any changes. Not every code suggestion would make sense or add value to our purpose. Therefore, you have the freedom to decide which of the suggestions are helpful.
✅ Before merging this pull request:
- Review all AI-generated comments for accuracy and relevance.
- Complete and verify the table below. We need your feedback to measure the accuracy of these suggestions and the value they add. If you are rejecting a certain code suggestion, please mention the reason briefly in the suggestion for us to capture it.
| Comment | Accepted (Y/N) | Reason |
|---|---|---|
| #### Log Improvement Suggestion No: 1 |
Fix PlanClick to expand$(cat /Users/admin/Desktop/SE-AI_Agent/MI_TEST_PATCH_GENERAL_SKILLS/test-1/product-micro-integrator/.ai/fix-plan-4767.md) |
WalkthroughThe Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Verification ReportClick to expand$(cat /Users/admin/Desktop/SE-AI_Agent/MI_TEST_PATCH_GENERAL_SKILLS/test-1/product-micro-integrator/.ai/verify-4767.md) |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/org.wso2.micro.integrator.extensions/org.wso2.micro.integrator.management.apis/src/main/java/org/wso2/micro/integrator/management/apis/RegistryPropertiesResource.java (1)
232-235:⚠️ Potential issue | 🟡 MinorInconsistent handling of
nullproperties between methods.
populateRegistryProperties(line 202) now returns an empty list whengetResourceProperties()returnsnull, butpopulateRegistryPropertystill returns anINTERNAL_SERVER_ERROR. This creates inconsistent API behavior:
GET /registry-resources/properties?path=...→{"count":0,"list":[]}(200 OK)GET /registry-resources/properties?path=...&name=foo→{"Error":"Error while fetching properties"}(500)Consider aligning the behavior. For a resource with no properties file, returning
BAD_REQUESTwith a message like "Property named X does not exist" or "No properties file exists for this resource" would be more consistent and informative than a 500 error.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/org.wso2.micro.integrator.extensions/org.wso2.micro.integrator.management.apis/src/main/java/org/wso2/micro/integrator/management/apis/RegistryPropertiesResource.java` around lines 232 - 235, populateRegistryProperty currently returns an INTERNAL_SERVER_ERROR when getResourceProperties() is null, while populateRegistryProperties returns an empty list; make them consistent by checking for null in populateRegistryProperty (the method in RegistryPropertiesResource that builds jsonBody) and return a BAD_REQUEST JSON error message (e.g., "Property named X does not exist" or "No properties file exists for this resource") instead of INTERNAL_SERVER_ERROR; update the jsonBody assignment in populateRegistryProperty to set BAD_REQUEST with a clear message when properties == null so both populateRegistryProperties and populateRegistryProperty handle missing properties uniformly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@components/org.wso2.micro.integrator.extensions/org.wso2.micro.integrator.management.apis/src/main/java/org/wso2/micro/integrator/management/apis/RegistryPropertiesResource.java`:
- Around line 232-235: populateRegistryProperty currently returns an
INTERNAL_SERVER_ERROR when getResourceProperties() is null, while
populateRegistryProperties returns an empty list; make them consistent by
checking for null in populateRegistryProperty (the method in
RegistryPropertiesResource that builds jsonBody) and return a BAD_REQUEST JSON
error message (e.g., "Property named X does not exist" or "No properties file
exists for this resource") instead of INTERNAL_SERVER_ERROR; update the jsonBody
assignment in populateRegistryProperty to set BAD_REQUEST with a clear message
when properties == null so both populateRegistryProperties and
populateRegistryProperty handle missing properties uniformly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3267a783-9d53-4efb-bb02-dc1eedcf6320
📒 Files selected for processing (1)
components/org.wso2.micro.integrator.extensions/org.wso2.micro.integrator.management.apis/src/main/java/org/wso2/micro/integrator/management/apis/RegistryPropertiesResource.java
Summary
.propertiesfile,getResourceProperties()returnsnull.nullas an error, returning{"list":"Error while fetching properties"}instead of an empty list.propertiesListisnull, returnUtils.createJSONList(0)to produce{"count":0,"list":[]}.Root Cause
In
RegistryPropertiesResource.populateRegistryProperties(), theelsebranch (triggered whengetResourceProperties()returnsnull) was constructing an error JSON object instead of an empty list. Anullreturn fromgetResourceProperties()simply means no.propertiesmetadata file exists for the resource — it is not an error condition.Change
File:
components/org.wso2.micro.integrator.extensions/org.wso2.micro.integrator.management.apis/src/main/java/org/wso2/micro/integrator/management/apis/RegistryPropertiesResource.javaTest plan
testfolder/sampletext.txt) with no.propertiesfile.GET /management/registry-resources/properties?path=registry/governance/mi-resources/testfolder/sampletext.txt.{"count":0,"list":[]}(empty list, not error string).{"Error":"Can not find the registry: ..."}.🤖 Generated with Claude Code
Summary by CodeRabbit