Process when list of policy empty - #1611
Conversation
📝 WalkthroughWalkthroughFleetMdmClient adds a generic parseList helper using Jackson CollectionType. Four list methods (listPolicies, listScheduledQueries, listPoliciesAsync, listScheduledQueriesAsync) now use this helper, returning an empty list instead of throwing when the response field is missing or null. ChangesFleet MDM list parsing
Estimated code review effort: 1 (Trivial) | ~5 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
60af9ad to
4f5b165
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
sdk/fleetmdm/src/main/java/com/openframe/sdk/fleetmdm/FleetMdmClient.java (1)
813-822: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd unit tests for
parseList.
parseListis new shared logic used by four public methods (listPolicies,listScheduledQueries,listPoliciesAsync,listScheduledQueriesAsync). Add tests covering the missing-field case, the null-field case, and the present-field case with items, since this method centralizes the behavior the PR title describes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdk/fleetmdm/src/main/java/com/openframe/sdk/fleetmdm/FleetMdmClient.java` around lines 813 - 822, Add unit tests targeting the private FleetMdmClient.parseList helper, covering a missing field and an explicit null field returning an empty list, plus a present field deserializing multiple items into the requested type. Reuse the existing FleetMdmClient test fixtures and JSON mapper setup, and ensure the tests exercise the shared behavior used by the synchronous and asynchronous list methods.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@sdk/fleetmdm/src/main/java/com/openframe/sdk/fleetmdm/FleetMdmClient.java`:
- Around line 813-822: The parseList method must not silently convert a missing
or null Fleet list field into an empty result. Require the named field to be a
non-null array before conversion, and throw or propagate an explicit API/parsing
error when it is absent, null, or invalid; preserve empty-list behavior only
when the response contains an actual empty array.
---
Nitpick comments:
In `@sdk/fleetmdm/src/main/java/com/openframe/sdk/fleetmdm/FleetMdmClient.java`:
- Around line 813-822: Add unit tests targeting the private
FleetMdmClient.parseList helper, covering a missing field and an explicit null
field returning an empty list, plus a present field deserializing multiple items
into the requested type. Reuse the existing FleetMdmClient test fixtures and
JSON mapper setup, and ensure the tests exercise the shared behavior used by the
synchronous and asynchronous list methods.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 31d8d09c-a084-493d-aeca-e6787056656c
📒 Files selected for processing (1)
sdk/fleetmdm/src/main/java/com/openframe/sdk/fleetmdm/FleetMdmClient.java
| private static <T> List<T> parseList(String responseBody, String fieldName, Class<T> itemType) throws Exception { | ||
| JsonNode root = MAPPER.readTree(responseBody); | ||
| JsonNode node = root.get(fieldName); | ||
| if (node == null || node.isNull()) { | ||
| return List.of(); | ||
| } | ||
| CollectionType listType = MAPPER.getTypeFactory().constructCollectionType(List.class, itemType); | ||
| return MAPPER.convertValue(node, listType); | ||
| } | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Fleet MDM API global policies list endpoint: does it omit or null the "policies" field for any reason other than an empty list?
💡 Result:
The Fleet MDM API endpoint for listing global policies (GET /api/v1/fleet/global/policies) typically returns a JSON object containing a policies field, which is an array of policy objects [1][2]. The policies field is generally present even when no policies are configured, in which case it is represented as an empty list ([]) rather than being omitted or null. While the API structure is consistent, there are specific circumstances in Fleet where policy-related fields are omitted or restricted based on licensing or endpoint context, though these differ from a simple null vs. empty list distinction: 1. Premium Licensing Restrictions: In some cases, Fleet limits access to policy information for non-premium users. For instance, in certain device-specific endpoints, policy information is omitted entirely from the response if the instance is not licensed for Fleet Premium, rather than being returned as empty or null [3]. 2. JSON Unmarshalling and Zero Values: Fleet's internal handling of JSON, particularly in PATCH requests, has historically dealt with nuances between null and omitted fields [4][5]. While this primarily affects how the server interprets client-provided payloads (e.g., whether a null value unsets an automation [4]), it highlights that the server's serialization logic for structs often defaults to specific patterns (like using custom unmarshalers for optional fields) that may influence how fields are emitted or expected [4][5]. In summary, for the global policies list endpoint, you should expect an empty array ([]) when no policies exist, rather than the field being omitted or null [1][2]. If you are encountering missing fields in other contexts, it is likely related to premium feature gating or specific structural definitions in the API's Go-based backend implementation [5][3].
Citations:
- 1: http://fleetdm.com/guides/sysadmin-diaries-exporting-policies
- 2: add premium, device authed endpoint to retrieve policies fleetdm/fleet#5967
- 3: only include policies in device endpoints for premium users fleetdm/fleet#6077
- 4: Policy automations don't unset on explicit null in PATCH fleetdm/fleet#23490
- 5: optjson.go better handling of zero value fleetdm/fleet#24695
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the client code around parseList/checkResponse/usages.
if [ -f sdk/fleetmdm/src/main/java/com/openframe/sdk/fleetmdm/FleetMdmClient.java ]; then
wc -l sdk/fleetmdm/src/main/java/com/openframe/sdk/fleetmdm/FleetMdmClient.java
sed -n '740,900p' sdk/fleetmdm/src/main/java/com/openframe/sdk/fleetmdm/FleetMdmClient.java | cat -n -v
echo '--- parseList usages ---'
rg -n "parseList|policies|queries|checkResponse" sdk/fleetmdm/src/main/java/com.openframe.sdk.fleetmdm.FleetMdmClient.java || true
else
fd -a 'FleetMdmClient.java' .
fiRepository: flamingo-stack/openframe-oss-lib
Length of output: 6382
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- package/files ---'
git ls-files | rg 'FleetMdmClient|fleetmdm|fleet' || true
echo '--- target file (if present) ---'
if [ -f sdk/fleetmdm/src/main/java/com/openframe/sdk/fleetmdm/FleetMdmClient.java ]; then
wc -l sdk/fleetmdm/src/main/java/com/openframe/sdk/fleetmdm/FleetMdmClient.java
echo '--- parseList and nearby API calls ---'
sed -n '70,180p;800,900p' sdk/fleetmdm/src/main/java/com/openframe/sdk/fleetmdm/FleetMdmClient.java | cat -n
echo '--- usages/fields in FleetMdmClient.java ---'
rg -n "parseList|policies|queries|GET|GETs|global/policies|/queries" sdk/fleetmdm/src/main/java/com/openframe/sdk/fleetmdm/FleetMdmClient.java || true
fiRepository: flamingo-stack/openframe-oss-lib
Length of output: 18689
🌐 Web query:
Fleet API list global policies JSON response policies field empty array
💡 Result:
If you receive a JSON response with an empty policies array (e.g., "policies": []) when querying the Fleet API, it indicates that the endpoint successfully returned a response, but no policies were found based on the parameters or the current state of your Fleet instance [1][2]. In the context of the Fleet Device Management API, this commonly occurs for the following reasons: 1. Absence of Data: The most straightforward reason is that there are no policies created in the scope you are querying (either global or within a specific team) [1]. 2. Filtering and Pagination: If you are using parameters such as query, page, or per_page, the filters may be excluding all available policies from the result set [2]. 3. Incorrect Endpoint: Ensure you are hitting the correct API route. Fleet has transitioned away from /global/ prefixes in its API paths [3]. For example, instead of /api/v1/fleet/global/policies, you should use /api/v1/fleet/policies [1][3]. If you are instead working with Elastic's Fleet (often used in the Elastic Stack), an empty array or missing fields may occur if the API request requires specific parameters—such as?full=true—to return populated data [4][5]. To troubleshoot: Verify your authentication and permissions to ensure your API token has sufficient access to view policies. Check the API documentation for your specific version of Fleet, as deprecated routes or parameter changes can sometimes cause unexpected empty responses [3][2]. If you are using the Fleet (FleetDM) CLI or API client, ensure you are not accidentally filtering the results [1].
Citations:
- 1: https://github.com/fleetdm/fleet/blob/main/server/service/client_policies.go
- 2: Paginate Policies API fleetdm/fleet#13434
- 3: Update API services to deprecate use of /global/ routes fleetdm/fleet#3395
- 4: https://www.elastic.co/docs/api/doc/kibana/operation/operation-get-fleet-agent-policies
- 5: GET /agent_policies doesn't return any package_policies elastic/fleet-server#1865
Do not treat a missing Fleet list field as an empty result.
parseList returns List.of() when policies or queries is absent or null. Fleet policy/query responses should emit an empty array when no items exist. If Fleet returns a broken or partial 2xx response, mapping it to an empty list can make the client report “no policies” instead of surfacing the API response failure. Require a non-null array field for these list endpoints, or make the missing-field case return an explicit API error.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@sdk/fleetmdm/src/main/java/com/openframe/sdk/fleetmdm/FleetMdmClient.java`
around lines 813 - 822, The parseList method must not silently convert a missing
or null Fleet list field into an empty result. Require the named field to be a
non-null array before conversion, and throw or propagate an explicit API/parsing
error when it is absent, null, or invalid; preserve empty-list behavior only
when the response contains an actual empty array.
1 participant
Summary by CodeRabbit