P1RECMOB-3476: Add recognize module and journey integration - #229
Open
mariodigirolamo1 wants to merge 10 commits into
Open
P1RECMOB-3476: Add recognize module and journey integration#229mariodigirolamo1 wants to merge 10 commits into
mariodigirolamo1 wants to merge 10 commits into
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new Android ChangesRecognize integration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Journey
participant RecognizeCallback
participant Recognize
participant Keyless
Journey->>RecognizeCallback: provide operationType and callback fields
RecognizeCallback->>Recognize: configure and invoke enroll or authenticate
Recognize->>Keyless: execute callback-based SDK operation
Keyless->>Recognize: return success or failure
Recognize->>Journey: submit result fields
sequenceDiagram
participant DaVinci
participant RecognizeCollector
participant Recognize
participant Keyless
DaVinci->>RecognizeCollector: provide operationType and collector input
RecognizeCollector->>Recognize: setup and execute selected operation
Recognize->>Keyless: invoke enrollment or authentication
Keyless->>Recognize: return operation result
Recognize->>DaVinci: return flat JSON payload
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
mariodigirolamo1
marked this pull request as ready for review
July 23, 2026 13:24
george-bafaloukas-forgerock
requested review from
spetrov,
vibhorgoswami and
witrisna
July 23, 2026 13:33
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (5)
recognize/src/test/kotlin/com/pingidentity/recognize/davinci/CollectorInitializerTest.kt (1)
25-33: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the registered factory’s concrete collector.
assertNotNullwould pass even if"RECOGNIZE"were mapped to the wrong factory. Verify that invoking the registered entry producesRecognizeCollector, using theCollectorFactoryAPI for factory invocation.🤖 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 `@recognize/src/test/kotlin/com/pingidentity/recognize/davinci/CollectorInitializerTest.kt` around lines 25 - 33, Update the test create registers RECOGNIZE collector and returns factory to invoke the registered "RECOGNIZE" entry through the CollectorFactory API, then assert the produced collector is a RecognizeCollector. Replace the insufficient assertNotNull check while preserving the existing factory identity assertion.recognize/src/test/kotlin/com/pingidentity/recognize/journey/RecognizeCallbackTest.kt (1)
84-104: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftExtract a shared JSON-builder helper to reduce duplication.
Most tests in this file hand-roll a full multi-line JSON string just to vary a single field (
mobileSDKOptionsentry,operationType, etc.). The siblingRecognizeAuthenticateCollectorTest.ktalready solves this with a smallauthInput(...)helper taking named defaults plus an optionalmobileSDKOptionsbuilder lambda — the same pattern here would cut this file's size considerably and make each test's actual variable clearer at a glance.Also applies to: 217-242, 496-522, 527-551, 617-643, 655-681, 684-710, 713-739, 742-768, 771-797, 833-862
🤖 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 `@recognize/src/test/kotlin/com/pingidentity/recognize/journey/RecognizeCallbackTest.kt` around lines 84 - 104, Extract a shared JSON-builder helper for the repeated callback payloads in RecognizeCallbackTest, following the authInput pattern from RecognizeAuthenticateCollectorTest. Give it named defaults for common fields and an optional mobileSDKOptions builder, then update metadataCallbackJson and the referenced test payload builders to use it while keeping each test’s variable fields explicit.recognize/src/main/kotlin/com/pingidentity/recognize/davinci/AbstractRecognizeCollector.kt (1)
28-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider
internalvisibility for this base class.
AbstractRecognizeCollectorand its subclasses are only referenced within this module (viaCollectorFactory.registerinCollectorInitializer.kt), not part of a documented public API surface.As per coding guidelines: "Prefer
internalfor module-private APIs, and declarepublicexplicitly on the documented API surface."♻️ Suggested visibility change
-abstract class AbstractRecognizeCollector : Collector<JsonObject> { +internal abstract class AbstractRecognizeCollector : Collector<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 `@recognize/src/main/kotlin/com/pingidentity/recognize/davinci/AbstractRecognizeCollector.kt` at line 28, Change AbstractRecognizeCollector to internal visibility, and update any directly declared subclasses if needed to satisfy Kotlin visibility rules. Keep the CollectorFactory.register integration and documented public API unchanged.Source: Coding guidelines
recognize/src/main/kotlin/com/pingidentity/recognize/davinci/CollectorInitializer.kt (1)
22-26: 🗄️ Data Integrity & Integration | 🔵 TrivialUnresolved server-contract TODO.
The registration key
"RECOGNIZE"is marked as unconfirmed with the server team. If the actual DaVinci discriminator differs, the collector factory silently fails to dispatch this component.Do you want me to open a tracking issue for confirming this value before release?
🤖 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 `@recognize/src/main/kotlin/com/pingidentity/recognize/davinci/CollectorInitializer.kt` around lines 22 - 26, Resolve the server-contract TODO in CollectorInitializer.create by confirming the DaVinci component discriminator with the server team and updating the CollectorFactory.register key to the verified value. Remove the TODO once confirmed, while preserving registration of RecognizeCollector and returning CollectorFactory.recognize/src/main/kotlin/com/pingidentity/recognize/davinci/RecognizeAuthenticateCollector.kt (1)
75-159: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSignificant boilerplate overlap with
RecognizeEnrollCollector.collect().
SetupConfigconstruction,OperationInfoparsing (lines 87-96),ClientStateTypemapping (lines 99-102), andJwtSigningInfobuilding (lines 106-110) are near-identical to the equivalent blocks inRecognizeEnrollCollector.kt. Consider extracting these as protected helpers onAbstractRecognizeCollectorto avoid drift as more operation types are added.🤖 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 `@recognize/src/main/kotlin/com/pingidentity/recognize/davinci/RecognizeAuthenticateCollector.kt` around lines 75 - 159, Extract the duplicated SetupConfig construction, OperationInfo parsing, ClientStateType mapping, and JwtSigningInfo creation from RecognizeAuthenticateCollector.collect() and RecognizeEnrollCollector.collect() into protected helpers on AbstractRecognizeCollector. Update both collectors to reuse those helpers while preserving their current inputs, defaults, and behavior.
🤖 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 `@recognize/build.gradle.kts`:
- Around line 25-29: Replace the string-literal project dependencies in
recognize/build.gradle.kts lines 25-29 with the specified typesafe accessors:
projects.foundation.utils, projects.foundation.logger,
projects.foundation.android, projects.foundation.davinciPlugin, and
projects.foundation.journeyPlugin. Also update
samples/pingsampleapp/build.gradle.kts line 92 from project(":recognize") to
projects.recognize.
In
`@recognize/src/main/kotlin/com/pingidentity/recognize/journey/AbstractRecognizeCallback.kt`:
- Around line 176-191: Update setValueCallback in AbstractRecognizeCallback to
use an exact suffix check with callback.id.endsWith(idSuffix) instead of
substring matching via contains. Preserve the existing ValueCallback filtering
and value assignment so CLIENT_ERROR_SUFFIX and CLIENT_ERROR_CODE_SUFFIX work
independently of update order.
- Around line 101-118: Guard the first-element access for server-supplied
MetadataCallback output arrays so empty arrays become a no-op rather than
throwing. In AbstractRecognizeCallback.init, replace the direct output[0] access
with a safe first-element lookup; apply the same firstOrNull handling to
jsonObject["output"] in RecognizeCallback. Update both listed sites:
recognize/src/main/kotlin/com/pingidentity/recognize/journey/AbstractRecognizeCallback.kt
lines 101-118 and
recognize/src/main/kotlin/com/pingidentity/recognize/journey/RecognizeCallback.kt
lines 66-92.
In
`@recognize/src/test/kotlin/com/pingidentity/recognize/davinci/CollectorInitializerTest.kt`:
- Around line 20-23: Add an `@AfterTest` cleanup method in
CollectorInitializerTest that calls CollectorFactory.reset() after each test,
while preserving the existing `@BeforeTest` setup reset. Ensure the global
CollectorFactory registry is cleared even when tests run in different orders.
In
`@samples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/journey/callback/JourneyContinueNode.kt`:
- Around line 176-185: Update the LaunchedEffect handlers for
PingOneRecognizeEnrollCallback and PingOneRecognizeAuthenticateCallback to
advance via onNext only when enroll() or authenticate() returns a successful
Result; preserve showNext = false and allow failures to remain on the current
journey step.
In `@settings.gradle.kts`:
- Around line 18-29: The top-level Cloudsmith token delegates in
settings.gradle.kts fail configuration when contributors have not defined the
properties. Update the settings initialization around cloudsmithTokenRecognize
and cloudsmithTokenAesWrap to use graceful optional fallbacks, and ensure the
Cloudsmith repository configuration remains valid without tokens so Gradle sync
and CI can proceed for all contributors.
---
Nitpick comments:
In
`@recognize/src/main/kotlin/com/pingidentity/recognize/davinci/AbstractRecognizeCollector.kt`:
- Line 28: Change AbstractRecognizeCollector to internal visibility, and update
any directly declared subclasses if needed to satisfy Kotlin visibility rules.
Keep the CollectorFactory.register integration and documented public API
unchanged.
In
`@recognize/src/main/kotlin/com/pingidentity/recognize/davinci/CollectorInitializer.kt`:
- Around line 22-26: Resolve the server-contract TODO in
CollectorInitializer.create by confirming the DaVinci component discriminator
with the server team and updating the CollectorFactory.register key to the
verified value. Remove the TODO once confirmed, while preserving registration of
RecognizeCollector and returning CollectorFactory.
In
`@recognize/src/main/kotlin/com/pingidentity/recognize/davinci/RecognizeAuthenticateCollector.kt`:
- Around line 75-159: Extract the duplicated SetupConfig construction,
OperationInfo parsing, ClientStateType mapping, and JwtSigningInfo creation from
RecognizeAuthenticateCollector.collect() and RecognizeEnrollCollector.collect()
into protected helpers on AbstractRecognizeCollector. Update both collectors to
reuse those helpers while preserving their current inputs, defaults, and
behavior.
In
`@recognize/src/test/kotlin/com/pingidentity/recognize/davinci/CollectorInitializerTest.kt`:
- Around line 25-33: Update the test create registers RECOGNIZE collector and
returns factory to invoke the registered "RECOGNIZE" entry through the
CollectorFactory API, then assert the produced collector is a
RecognizeCollector. Replace the insufficient assertNotNull check while
preserving the existing factory identity assertion.
In
`@recognize/src/test/kotlin/com/pingidentity/recognize/journey/RecognizeCallbackTest.kt`:
- Around line 84-104: Extract a shared JSON-builder helper for the repeated
callback payloads in RecognizeCallbackTest, following the authInput pattern from
RecognizeAuthenticateCollectorTest. Give it named defaults for common fields and
an optional mobileSDKOptions builder, then update metadataCallbackJson and the
referenced test payload builders to use it while keeping each test’s variable
fields explicit.
🪄 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 Plus
Run ID: 73b74d2c-3a60-4b48-adcd-41425e3735d9
📒 Files selected for processing (30)
CHANGELOG.mdgradle/libs.versions.tomlrecognize/README.mdrecognize/build.gradle.ktsrecognize/src/main/AndroidManifest.xmlrecognize/src/main/kotlin/com/pingidentity/recognize/Recognize.ktrecognize/src/main/kotlin/com/pingidentity/recognize/RecognizeException.ktrecognize/src/main/kotlin/com/pingidentity/recognize/RecognizeSuccess.ktrecognize/src/main/kotlin/com/pingidentity/recognize/davinci/AbstractRecognizeCollector.ktrecognize/src/main/kotlin/com/pingidentity/recognize/davinci/CollectorInitializer.ktrecognize/src/main/kotlin/com/pingidentity/recognize/davinci/RecognizeAuthenticateCollector.ktrecognize/src/main/kotlin/com/pingidentity/recognize/davinci/RecognizeCollector.ktrecognize/src/main/kotlin/com/pingidentity/recognize/davinci/RecognizeEnrollCollector.ktrecognize/src/main/kotlin/com/pingidentity/recognize/journey/AbstractRecognizeCallback.ktrecognize/src/main/kotlin/com/pingidentity/recognize/journey/CallbackInitializer.ktrecognize/src/main/kotlin/com/pingidentity/recognize/journey/PingOneRecognizeAuthenticateCallback.ktrecognize/src/main/kotlin/com/pingidentity/recognize/journey/PingOneRecognizeEnrollCallback.ktrecognize/src/main/kotlin/com/pingidentity/recognize/journey/RecognizeCallback.ktrecognize/src/main/kotlin/com/pingidentity/recognize/journey/RecognizeKeylessInitializer.ktrecognize/src/test/kotlin/com/pingidentity/recognize/RecognizeExceptionTest.ktrecognize/src/test/kotlin/com/pingidentity/recognize/RecognizeTest.ktrecognize/src/test/kotlin/com/pingidentity/recognize/davinci/CollectorInitializerTest.ktrecognize/src/test/kotlin/com/pingidentity/recognize/davinci/RecognizeAuthenticateCollectorTest.ktrecognize/src/test/kotlin/com/pingidentity/recognize/davinci/RecognizeCollectorTest.ktrecognize/src/test/kotlin/com/pingidentity/recognize/davinci/RecognizeEnrollCollectorTest.ktrecognize/src/test/kotlin/com/pingidentity/recognize/journey/CallbackInitializerTest.ktrecognize/src/test/kotlin/com/pingidentity/recognize/journey/RecognizeCallbackTest.ktsamples/pingsampleapp/build.gradle.ktssamples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/journey/callback/JourneyContinueNode.ktsettings.gradle.kts
mariodigirolamo1
force-pushed
the
develop
branch
from
July 23, 2026 14:33
303db33 to
671f8a4
Compare
mariodigirolamo1
force-pushed
the
develop
branch
from
July 23, 2026 14:55
671f8a4 to
a0d3a77
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
recognize/src/main/kotlin/com/pingidentity/recognize/journey/RecognizeCallback.kt (1)
37-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDeclare the documented callback class explicitly as
public.Line 37 is a documented, registry-facing API. Add explicit visibility for consistency with the project API guidelines.
Proposed fix
-class RecognizeCallback : AbstractCallback() { +public class RecognizeCallback : AbstractCallback() {As per coding guidelines, declare
publicexplicitly on the documented API surface.🤖 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 `@recognize/src/main/kotlin/com/pingidentity/recognize/journey/RecognizeCallback.kt` at line 37, Update the RecognizeCallback class declaration to explicitly use public visibility, preserving its existing inheritance from AbstractCallback and registry-facing API behavior.Source: Coding guidelines
🤖 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 `@CHANGELOG.md`:
- Line 2: Update the “Added” heading in the Unreleased changelog section from h4
to h3, changing “#### Added” to “### Added” while preserving the surrounding
changelog content.
In
`@recognize/src/main/kotlin/com/pingidentity/recognize/journey/RecognizeCallback.kt`:
- Around line 58-60: Update the MetadataCallback documentation near
AbstractRecognizeCallback.init to describe operationType at
output[0].value.operationType, removing the incorrect .data segment while
preserving the delegation description.
- Around line 66-73: Update RecognizeCallback.init to safely read the first
MetadataCallback output entry without indexing an empty jsonArray, allowing the
documented IllegalArgumentException validation to handle missing output. Add a
regression test covering a MetadataCallback with an empty output array.
---
Nitpick comments:
In
`@recognize/src/main/kotlin/com/pingidentity/recognize/journey/RecognizeCallback.kt`:
- Line 37: Update the RecognizeCallback class declaration to explicitly use
public visibility, preserving its existing inheritance from AbstractCallback and
registry-facing API behavior.
🪄 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 Plus
Run ID: 144214b5-3a7a-4bd4-a863-793765f088e6
📒 Files selected for processing (30)
CHANGELOG.mdgradle/libs.versions.tomlrecognize/README.mdrecognize/build.gradle.ktsrecognize/src/main/AndroidManifest.xmlrecognize/src/main/kotlin/com/pingidentity/recognize/Recognize.ktrecognize/src/main/kotlin/com/pingidentity/recognize/RecognizeException.ktrecognize/src/main/kotlin/com/pingidentity/recognize/RecognizeSuccess.ktrecognize/src/main/kotlin/com/pingidentity/recognize/davinci/AbstractRecognizeCollector.ktrecognize/src/main/kotlin/com/pingidentity/recognize/davinci/CollectorInitializer.ktrecognize/src/main/kotlin/com/pingidentity/recognize/davinci/RecognizeAuthenticateCollector.ktrecognize/src/main/kotlin/com/pingidentity/recognize/davinci/RecognizeCollector.ktrecognize/src/main/kotlin/com/pingidentity/recognize/davinci/RecognizeEnrollCollector.ktrecognize/src/main/kotlin/com/pingidentity/recognize/journey/AbstractRecognizeCallback.ktrecognize/src/main/kotlin/com/pingidentity/recognize/journey/CallbackInitializer.ktrecognize/src/main/kotlin/com/pingidentity/recognize/journey/PingOneRecognizeAuthenticateCallback.ktrecognize/src/main/kotlin/com/pingidentity/recognize/journey/PingOneRecognizeEnrollCallback.ktrecognize/src/main/kotlin/com/pingidentity/recognize/journey/RecognizeCallback.ktrecognize/src/main/kotlin/com/pingidentity/recognize/journey/RecognizeKeylessInitializer.ktrecognize/src/test/kotlin/com/pingidentity/recognize/RecognizeExceptionTest.ktrecognize/src/test/kotlin/com/pingidentity/recognize/RecognizeTest.ktrecognize/src/test/kotlin/com/pingidentity/recognize/davinci/CollectorInitializerTest.ktrecognize/src/test/kotlin/com/pingidentity/recognize/davinci/RecognizeAuthenticateCollectorTest.ktrecognize/src/test/kotlin/com/pingidentity/recognize/davinci/RecognizeCollectorTest.ktrecognize/src/test/kotlin/com/pingidentity/recognize/davinci/RecognizeEnrollCollectorTest.ktrecognize/src/test/kotlin/com/pingidentity/recognize/journey/CallbackInitializerTest.ktrecognize/src/test/kotlin/com/pingidentity/recognize/journey/RecognizeCallbackTest.ktsamples/pingsampleapp/build.gradle.ktssamples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/journey/callback/JourneyContinueNode.ktsettings.gradle.kts
🚧 Files skipped from review as they are similar to previous changes (28)
- recognize/src/main/kotlin/com/pingidentity/recognize/davinci/CollectorInitializer.kt
- recognize/src/test/kotlin/com/pingidentity/recognize/davinci/CollectorInitializerTest.kt
- samples/pingsampleapp/build.gradle.kts
- recognize/src/main/AndroidManifest.xml
- recognize/src/main/kotlin/com/pingidentity/recognize/davinci/RecognizeCollector.kt
- recognize/build.gradle.kts
- recognize/src/main/kotlin/com/pingidentity/recognize/RecognizeException.kt
- recognize/src/main/kotlin/com/pingidentity/recognize/journey/CallbackInitializer.kt
- recognize/src/main/kotlin/com/pingidentity/recognize/RecognizeSuccess.kt
- recognize/src/test/kotlin/com/pingidentity/recognize/RecognizeExceptionTest.kt
- recognize/src/main/kotlin/com/pingidentity/recognize/journey/RecognizeKeylessInitializer.kt
- recognize/src/test/kotlin/com/pingidentity/recognize/davinci/RecognizeCollectorTest.kt
- gradle/libs.versions.toml
- recognize/src/main/kotlin/com/pingidentity/recognize/journey/PingOneRecognizeEnrollCallback.kt
- recognize/src/test/kotlin/com/pingidentity/recognize/RecognizeTest.kt
- settings.gradle.kts
- samples/pingsampleapp/src/main/java/com/pingidentity/samples/pingsampleapp/journey/callback/JourneyContinueNode.kt
- recognize/src/main/kotlin/com/pingidentity/recognize/davinci/AbstractRecognizeCollector.kt
- recognize/README.md
- recognize/src/test/kotlin/com/pingidentity/recognize/journey/CallbackInitializerTest.kt
- recognize/src/main/kotlin/com/pingidentity/recognize/davinci/RecognizeAuthenticateCollector.kt
- recognize/src/main/kotlin/com/pingidentity/recognize/journey/PingOneRecognizeAuthenticateCallback.kt
- recognize/src/main/kotlin/com/pingidentity/recognize/davinci/RecognizeEnrollCollector.kt
- recognize/src/test/kotlin/com/pingidentity/recognize/davinci/RecognizeEnrollCollectorTest.kt
- recognize/src/test/kotlin/com/pingidentity/recognize/davinci/RecognizeAuthenticateCollectorTest.kt
- recognize/src/main/kotlin/com/pingidentity/recognize/journey/AbstractRecognizeCallback.kt
- recognize/src/main/kotlin/com/pingidentity/recognize/Recognize.kt
- recognize/src/test/kotlin/com/pingidentity/recognize/journey/RecognizeCallbackTest.kt
witrisna
reviewed
Jul 28, 2026
| * | ||
| * @return [Result] containing [RecognizeSuccess] on success, or a [Throwable] on failure. | ||
| */ | ||
| suspend fun enroll(retrieveSelfie: Boolean = false): Result<RecognizeSuccess> { |
Contributor
There was a problem hiding this comment.
Instead of providing the retrieveSelfie as parameter, do you want to create an config object instead? (reference:
With object, allow more flexibility to add more customization in the future.
Author
There was a problem hiding this comment.
yeah, I can totally do it this way 👍
witrisna
reviewed
Jul 28, 2026
| keylessId = success.keylessId, | ||
| ) | ||
| } | ||
| .recoverCatching { throw it as RecognizeException } |
Contributor
There was a problem hiding this comment.
Is it possible that the exception is not a RecognizeException? that will result with ClassCastExcetpion?
Author
There was a problem hiding this comment.
So, the exception at that point should always be correct, tho, it is not compiler safe.
Also this recoverCatching is actually redundant, so I removed it and just adopted a more lenient converstion from the throwable in this commit so that no casting issue will occurr (they shoudn't have occurred already logically but due to an absent compile time guarantee, this will make us safer against unexpected code changes or bugs that might actually trigger an unexpected throwable.
Let me know if this fix is good enough for this, i tried avoiding a custom Result.Failure or similar since it becomes an heavier change and forces the integrator to then handle it with a when statement, while the kotlin Result is idiomatic and preferred.
…gnize callbacks Introduce RecognizeEnrollConfig and RecognizeAuthenticateConfig so enroll/authenticate accept a lambda-with-receiver instead of a flat Boolean, matching the DeviceBindingConfig pattern and allowing future options without API breaks.
Contributor
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)
recognize/src/main/kotlin/com/pingidentity/recognize/journey/PingOneRecognizeAuthenticateCallback.kt (1)
91-101: 🩺 Stability & Availability | 🟠 MajorPreserve non-
RecognizeExceptionfailures.
onFailurereceives a generalThrowable, but both callbacks cast every failure toRecognizeException. Any unexpected SDK or setup failure becomes aClassCastException, masking the original error and preventing Journey error submission. Normalize failures safely or preserve the originalThrowable, including inrecoverCatching.
recognize/src/main/kotlin/com/pingidentity/recognize/journey/PingOneRecognizeAuthenticateCallback.kt#L91-L101: remove the unchecked casts in failure mapping and recovery.recognize/src/main/kotlin/com/pingidentity/recognize/journey/PingOneRecognizeEnrollCallback.kt#L68-L86: apply the same safe failure handling.🤖 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 `@recognize/src/main/kotlin/com/pingidentity/recognize/journey/PingOneRecognizeAuthenticateCallback.kt` around lines 91 - 101, Remove the unchecked RecognizeException casts in the failure mapping and recovery of recognize/src/main/kotlin/com/pingidentity/recognize/journey/PingOneRecognizeAuthenticateCallback.kt lines 91-101, preserving non-RecognizeException Throwables while still submitting appropriate Journey error details. Apply the same safe failure handling in recognize/src/main/kotlin/com/pingidentity/recognize/journey/PingOneRecognizeEnrollCallback.kt lines 68-86, including its recoverCatching path.
🧹 Nitpick comments (1)
recognize/src/main/kotlin/com/pingidentity/recognize/journey/PingOneRecognizeAuthenticateCallback.kt (1)
62-62: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDeclare exported APIs explicitly as
public.These documented APIs currently rely on Kotlin’s implicit public visibility. Add explicit
publicmodifiers to the callback methods, configuration classes, andretrieveSelfieproperties.
recognize/src/main/kotlin/com/pingidentity/recognize/journey/PingOneRecognizeAuthenticateCallback.kt#L62-L62: declareauthenticatepublic.recognize/src/main/kotlin/com/pingidentity/recognize/journey/PingOneRecognizeEnrollCallback.kt#L52-L52: declareenrollpublic.recognize/src/main/kotlin/com/pingidentity/recognize/journey/RecognizeConfig.kt#L22-L25: declareRecognizeEnrollConfigandretrieveSelfiepublic.recognize/src/main/kotlin/com/pingidentity/recognize/journey/RecognizeConfig.kt#L38-L41: declareRecognizeAuthenticateConfigandretrieveSelfiepublic.As per coding guidelines, documented public APIs must declare
publicexplicitly.🤖 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 `@recognize/src/main/kotlin/com/pingidentity/recognize/journey/PingOneRecognizeAuthenticateCallback.kt` at line 62, Declare the documented APIs explicitly as public: make authenticate public in PingOneRecognizeAuthenticateCallback.kt (line 62), enroll public in PingOneRecognizeEnrollCallback.kt (line 52), and make RecognizeEnrollConfig, RecognizeAuthenticateConfig, and both retrieveSelfie properties public in RecognizeConfig.kt (lines 22-25 and 38-41).Source: Coding guidelines
🤖 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.
Outside diff comments:
In
`@recognize/src/main/kotlin/com/pingidentity/recognize/journey/PingOneRecognizeAuthenticateCallback.kt`:
- Around line 91-101: Remove the unchecked RecognizeException casts in the
failure mapping and recovery of
recognize/src/main/kotlin/com/pingidentity/recognize/journey/PingOneRecognizeAuthenticateCallback.kt
lines 91-101, preserving non-RecognizeException Throwables while still
submitting appropriate Journey error details. Apply the same safe failure
handling in
recognize/src/main/kotlin/com/pingidentity/recognize/journey/PingOneRecognizeEnrollCallback.kt
lines 68-86, including its recoverCatching path.
---
Nitpick comments:
In
`@recognize/src/main/kotlin/com/pingidentity/recognize/journey/PingOneRecognizeAuthenticateCallback.kt`:
- Line 62: Declare the documented APIs explicitly as public: make authenticate
public in PingOneRecognizeAuthenticateCallback.kt (line 62), enroll public in
PingOneRecognizeEnrollCallback.kt (line 52), and make RecognizeEnrollConfig,
RecognizeAuthenticateConfig, and both retrieveSelfie properties public in
RecognizeConfig.kt (lines 22-25 and 38-41).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ae1efb16-374f-4a90-b1aa-3e69d35ce8b6
📒 Files selected for processing (3)
recognize/src/main/kotlin/com/pingidentity/recognize/journey/PingOneRecognizeAuthenticateCallback.ktrecognize/src/main/kotlin/com/pingidentity/recognize/journey/PingOneRecognizeEnrollCallback.ktrecognize/src/main/kotlin/com/pingidentity/recognize/journey/RecognizeConfig.kt
…ognize callbacks Remove the dead `recoverCatching` lines and replace the unchecked `error as RecognizeException` casts in `onFailure` with `asRecognizeException()`, an internal extension that returns the exception as-is if it is already a `RecognizeException`, or wraps any unexpected `Throwable` with code=-1 and the original as cause rather than crashing.
Signed-off-by: Mario Di Girolamo <69394590+mariodigirolamo1@users.noreply.github.com>
…tegration Journey's MetadataCallback router only dispatches to Protect and FIDO; Recognize data hits the else branch and is silently dropped, making the ContinueNodeAware, continueNode, derivedCallback, setValueCallback, and companion object suffix constants unreachable dead code.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
samples/pingsampleapp/build.gradle.kts (1)
90-90: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the typesafe project accessor.
Replace
implementation(project(":recognize"))withimplementation(projects.recognize). As per coding guidelines, use typesafe project accessors instead of string literals inbuild.gradle.kts.🤖 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/build.gradle.kts` at line 90, Update the dependency declaration in the Gradle configuration to use the typesafe projects accessor projects.recognize instead of the string-based project(":recognize") reference, leaving the implementation dependency unchanged.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@samples/pingsampleapp/build.gradle.kts`:
- Line 90: Update the dependency declaration in the Gradle configuration to use
the typesafe projects accessor projects.recognize instead of the string-based
project(":recognize") reference, leaving the implementation dependency
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 8f7a5180-8a2d-4523-8b3c-37cac9c11af1
📒 Files selected for processing (8)
gradle/libs.versions.tomlrecognize/src/main/kotlin/com/pingidentity/recognize/RecognizeException.ktrecognize/src/main/kotlin/com/pingidentity/recognize/journey/AbstractRecognizeCallback.ktrecognize/src/main/kotlin/com/pingidentity/recognize/journey/PingOneRecognizeAuthenticateCallback.ktrecognize/src/main/kotlin/com/pingidentity/recognize/journey/PingOneRecognizeEnrollCallback.ktrecognize/src/main/kotlin/com/pingidentity/recognize/journey/RecognizeCallback.ktrecognize/src/test/kotlin/com/pingidentity/recognize/journey/RecognizeCallbackTest.ktsamples/pingsampleapp/build.gradle.kts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
JIRA Ticket
https://pingidentity.atlassian.net/browse/P1RECMOB-3476
Description
Summary
Introduces the recognize module — a proxy wrapper around the Keyless SDK (v6.0.0) that bridges PingAM Journey and PingOne DaVinci flows with biometric recognition (face enroll/authenticate).
What's in scope:
What's NOT in scope: DaVinci direct Keyless SDK calls (deferred, tracked in epic).
Review guidance
The Keyless SDK is fetched from two private Cloudsmith Maven repos. settings.gradle.kts reads the tokens as Gradle properties:
cloudsmithTokenRecognize → https://dl.cloudsmith.io//keyless/partners/maven/
cloudsmithTokenAesWrap → https://dl.cloudsmith.io//keyless/aeswrap/maven/
For local builds — add to ~/.gradle/gradle.properties (not committed):
cloudsmithTokenRecognize=
cloudsmithTokenAesWrap=
For CI — both tokens must be injected as environment variables or Gradle properties in the pipeline config, otherwise dependency resolution will fail at the settings.gradle.kts phase (before any module is compiled).
Tokens should be obtainable from the Keyless/partners team or your internal secrets store.
Summary by CodeRabbit
New Features
Documentation
Tests