SDKS-5184: Update sample app to showcase JSON config#212
Conversation
📝 WalkthroughWalkthroughThree new JSON asset configuration files (Journey, DaVinci, OpenID) are added to the sample app's assets directory. ChangesAsset Preset Loading and JSON Migration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
vibhorgoswami
temporarily deployed
to
github-pages
GitHub Actions
Inactive
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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
`@samples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/config/EnvViewModel.kt`:
- Around line 183-186: The userinfoEndpoint field is being parsed with an
incorrect JSON key name that has a capital I in "userInfoEndpoint", but the
actual key in the openIdJsonConfig.json file uses lowercase i as
"userinfoEndpoint". Change the JSON key parameter in the line assigning
userinfoEndpoint from "userInfoEndpoint" to "userinfoEndpoint" to match the
actual configuration file key and ensure the value is correctly populated
instead of defaulting to an empty string.
- Around line 135-137: The loadAssetConfigs() method opens file streams via
context.assets.open(fileName).bufferedReader() without closing them, causing
resource leaks in the loop. Wrap the bufferedReader().readText() call with the
Kotlin .use { } function to ensure the stream is automatically closed after
reading, preventing file descriptor leaks on each iteration of the loop.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4eb46e48-cd4a-4f03-9ff2-1f5dd43e1cba
📒 Files selected for processing (4)
samples/pingsampleapp/src/main/assets/DaVinciJsonConfig.jsonsamples/pingsampleapp/src/main/assets/journeyJsonConfig.jsonsamples/pingsampleapp/src/main/assets/openIdJsonConfig.jsonsamples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/config/EnvViewModel.kt
| val root = Json.parseToJsonElement( | ||
| context.assets.open(fileName).bufferedReader().readText() | ||
| ).jsonObject |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="samples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/config/EnvViewModel.kt"
# Verify current asset read pattern and whether open() calls are wrapped in use {}
rg -n -C2 'assets\.open\(' "$FILE"
rg -n -C2 'bufferedReader\(\)\.readText\(\)' "$FILE"
rg -n -C2 '\.use\s*\{' "$FILE"Repository: ForgeRock/ping-android-sdk
Length of output: 566
🏁 Script executed:
FILE="samples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/config/EnvViewModel.kt"
# Check full function context around the problematic code
sed -n '120,160p' "$FILE"Repository: ForgeRock/ping-android-sdk
Length of output: 1926
Close asset streams in loadAssetConfigs() loop.
The code at lines 135-137 opens a stream via context.assets.open(fileName).bufferedReader() but never closes it. In Kotlin, readText() does not auto-close the reader—each iteration of the loop leaks a file descriptor. Use .use { } to ensure closure.
Suggested fix
- val root = Json.parseToJsonElement(
- context.assets.open(fileName).bufferedReader().readText()
- ).jsonObject
+ val root = context.assets.open(fileName).bufferedReader().use { reader ->
+ Json.parseToJsonElement(reader.readText()).jsonObject
+ }🤖 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
`@samples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/config/EnvViewModel.kt`
around lines 135 - 137, The loadAssetConfigs() method opens file streams via
context.assets.open(fileName).bufferedReader() without closing them, causing
resource leaks in the loop. Wrap the bufferedReader().readText() call with the
Kotlin .use { } function to ensure the stream is automatically closed after
reading, preventing file descriptor leaks on each iteration of the loop.
| authorizationEndpoint = openIdObj?.str("authorizationEndpoint") ?: "", | ||
| tokenEndpoint = openIdObj?.str("tokenEndpoint") ?: "", | ||
| userinfoEndpoint = openIdObj?.str("userInfoEndpoint") ?: "", | ||
| endSessionEndpoint = openIdObj?.str("endSessionEndpoint") ?: "", |
There was a problem hiding this comment.
Device-auth userinfoEndpoint is parsed with the wrong JSON key.
openIdJsonConfig.json uses userinfoEndpoint, but this parser reads userInfoEndpoint (capital I), so the value is always empty and the loaded device-auth preset is incomplete.
Suggested fix
- userinfoEndpoint = openIdObj?.str("userInfoEndpoint") ?: "",
+ userinfoEndpoint = openIdObj?.str("userinfoEndpoint") ?: "",🤖 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
`@samples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/config/EnvViewModel.kt`
around lines 183 - 186, The userinfoEndpoint field is being parsed with an
incorrect JSON key name that has a capital I in "userInfoEndpoint", but the
actual key in the openIdJsonConfig.json file uses lowercase i as
"userinfoEndpoint". Change the JSON key parameter in the line assigning
userinfoEndpoint from "userInfoEndpoint" to "userinfoEndpoint" to match the
actual configuration file key and ensure the value is correctly populated
instead of defaulting to an empty string.
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## develop #212 +/- ##
==============================================
- Coverage 44.12% 26.05% -18.08%
+ Complexity 1393 1016 -377
==============================================
Files 316 309 -7
Lines 9756 9507 -249
Branches 1495 1429 -66
==============================================
- Hits 4305 2477 -1828
- Misses 4887 6749 +1862
+ Partials 564 281 -283
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
1 participant
JIRA Ticket
SDKS-5184
Description
This PR provides an option to load json files of configuration.
Summary by CodeRabbit
New Features
Refactor