Fix #280: Implement REST client for Digital Onboarding Adapter #287
Conversation
6f128ef to
a60d39b
Compare
11ce836 to
849a7a6
Compare
| @@ -29,14 +32,29 @@ | |||
| */ | |||
| @Builder | |||
| @Getter | |||
| @ToString(exclude = "otpCode") | |||
There was a problem hiding this comment.
Covered by a unit test.
| private static UserLookupRequestDto convert (final LookupUserRequest source) { | ||
| final UserLookupRequestDto target = new UserLookupRequestDto(); | ||
| target.setIdentification(source.getIdentification()); | ||
| target.setProcessId(null); // TODO (racansky, 2022-07-19, #299) process is not created yet, get process ID before user lookup |
There was a problem hiding this comment.
Will be done in a separate issue #299 and a dedicated PR.
| /** | ||
| * Client evaluation is in progress. | ||
| */ | ||
| CLIENT_EVALUATION, |
...-onboarding/src/main/java/com/wultra/app/onboardingserver/task/ClientEvaluationSyncTask.java
Show resolved
Hide resolved
...ding/src/main/java/com/wultra/app/onboardingserver/provider/rest/ClientErrorResponseDto.java
Outdated
Show resolved
Hide resolved
...rding/src/main/java/com/wultra/app/onboardingserver/provider/rest/ConsentTextRequestDto.java
Outdated
Show resolved
Hide resolved
...oarding/src/main/java/com/wultra/app/onboardingserver/provider/rest/LanguagePropertyDto.java
Outdated
Show resolved
Hide resolved
...ding/src/main/java/com/wultra/app/onboardingserver/provider/rest/ConsentTypePropertyDto.java
Outdated
Show resolved
Hide resolved
...oarding/src/main/java/com/wultra/app/onboardingserver/provider/rest/LanguagePropertyDto.java
Outdated
Show resolved
Hide resolved
...ding/src/main/java/com/wultra/app/onboardingserver/provider/rest/ConsentTypePropertyDto.java
Outdated
Show resolved
Hide resolved
...ding/src/main/java/com/wultra/app/onboardingserver/provider/rest/ClientErrorResponseDto.java
Outdated
Show resolved
Hide resolved
...rc/main/java/com/wultra/app/onboardingserver/database/entity/IdentityVerificationEntity.java
Outdated
Show resolved
Hide resolved
| @Test | ||
| void testToString() { | ||
| final String result = SendOtpCodeRequest.builder() | ||
| .otpCode("top secret") |
There was a problem hiding this comment.
I would not use any funny values as 666 or 42, the test values should be simple and natural.
I have seen a plethora of similar values in live deployments and e-mails with test failures containing all such bizarre values :-/.
...ding/src/main/java/com/wultra/app/onboardingserver/provider/rest/ClientErrorResponseDto.java
Outdated
Show resolved
Hide resolved
| idVerification.setPhase(IdentityVerificationPhase.CLIENT_EVALUATION); | ||
| idVerification.setStatus(IdentityVerificationStatus.IN_PROGRESS); | ||
| idVerification.setTimestampLastUpdated(ownerId.getTimestamp()); | ||
| logger.info("Switched to wait for the client evaluation, {}, processId={}", ownerId, idVerification.getProcessId()); |
There was a problem hiding this comment.
It is consistent with logger.info("Switched to wait for the presence check, {}", ownerId);
There was a problem hiding this comment.
Yeps, could be, but that's also not nice text there.
| final ParameterizedTypeReference<OtpSendResponseDto> responseType = ParameterizedTypeReference.forType(OtpSendResponseDto.class); | ||
| final OtpSendResponseDto response = restClient.post("/otp/send", requestDto, responseType).getBody(); | ||
| if (response == null) { | ||
| throw new OnboardingProviderException("Unable to send otp for " + request); |
There was a problem hiding this comment.
missing a reason of the failure?
There was a problem hiding this comment.
What would you like to specify? This NPE check is here because underlying Mono#block() is @Nullable.
There was a problem hiding this comment.
That the response was null, it failed to get one. Debug logging would be enough.
...ding/src/main/java/com/wultra/app/onboardingserver/provider/rest/RestOnboardingProvider.java
Show resolved
Hide resolved
...ding/src/main/java/com/wultra/app/onboardingserver/provider/rest/RestOnboardingProvider.java
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| private Consumer<EvaluateClientResponse> createSuccessConsumer(final IdentityVerificationEntity identityVerification) { | ||
| return response -> { |
There was a problem hiding this comment.
Maybe we should use a separate service, e.g. new methods in IdentityVerificationStatusService to cover the status changes and rely on a standard transaction aware service method.
There was a problem hiding this comment.
You mean to add a save or update method to IdentityVerificationService or IdentityVerificationStatusService to avoid usage of transactionTemplate?
There was a problem hiding this comment.
Yeps, maybe separate those status changes to own method living in the IdentityVerificationStatusService. Currently those are just small snippets which could be more hard to test or discover.
There was a problem hiding this comment.
Ok, let's keep it this way, not to add dependency on another services. May be refactored later, if needed for tests.
No description provided.