Skip to content

Commit

Permalink
Issues with cached methods found during redis integration fixed (mosi…
Browse files Browse the repository at this point in the history 
…p#68)

Signed-off-by: ase-101 <[email protected]>
Signed-off-by: Mengleang <[email protected]>
  • Loading branch information
@ase-101 @mengleang-ngoun
ase-101 authored and mengleang-ngoun committed Mar 15, 2024
1 parent 721e3ef commit f3c240e
Show file tree
Hide file tree
Showing 8 changed files with 57 additions and 17 deletions.
4 changes: 4 additions & 0 deletions signup-service/src/main/java/io/mosip/signup/controllers/RegistrationController.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package io.mosip.signup.controllers;

import io.micrometer.core.annotation.Counted;
import io.micrometer.core.annotation.Timed;
import io.mosip.esignet.core.dto.RequestWrapper;
import io.mosip.esignet.core.dto.ResponseWrapper;
import io.mosip.esignet.core.util.IdentityProviderUtil;
Expand Down Expand Up @@ -70,6 +72,8 @@ public ResponseWrapper<VerifyChallengeResponse> verifyChallenge(@Valid @RequestB
return responseWrapper;
}


@Timed(value = "register.timer", percentiles = {0.95, 0.99})
@PostMapping("/register")
public ResponseWrapper<RegisterResponse> register(@Valid @RequestBody RequestWrapper<RegisterRequest> requestWrapper,
@Valid @NotBlank(message = ErrorConstants.INVALID_TRANSACTION)
Expand Down
2 changes: 2 additions & 0 deletions signup-service/src/main/java/io/mosip/signup/controllers/SignUpController.java
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package io.mosip.signup.controllers;


import io.micrometer.core.annotation.Timed;
import io.mosip.esignet.core.dto.RequestWrapper;
import io.mosip.esignet.core.dto.ResponseWrapper;
import io.mosip.esignet.core.util.IdentityProviderUtil;
Expand Down Expand Up @@ -43,6 +44,7 @@ public ResponseWrapper<SettingsResponse> getSignUpDetails() {
return responseWrapper;
}

@Timed(value = "resetpwd.timer", percentiles = {0.95, 0.99})
@PostMapping("/reset-password")
public ResponseWrapper<RegistrationStatusResponse> resetPassword(@Valid @RequestBody RequestWrapper<ResetPasswordRequest> requestWrapper,
@Valid @NotBlank(message = ErrorConstants.INVALID_TRANSACTION)
Expand Down
27 changes: 19 additions & 8 deletions signup-service/src/main/java/io/mosip/signup/services/CacheUtilService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,6 @@ public class CacheUtilService {
CacheManager cacheManager;

//---Setter---
@Cacheable(value = SignUpConstants.CHALLENGE_GENERATED, key = "#transactionId")
public RegistrationTransaction setChallengeGeneratedTransaction(String transactionId,
RegistrationTransaction registrationTransaction) {
return registrationTransaction;
}

@CacheEvict(value = SignUpConstants.CHALLENGE_GENERATED, key = "#transactionId")
@Cacheable(value = SignUpConstants.CHALLENGE_VERIFIED, key = "#verifiedTransactionId")
Expand All @@ -36,14 +31,15 @@ public RegistrationTransaction setChallengeVerifiedTransaction(String transactio
}

@CacheEvict(value = SignUpConstants.CHALLENGE_VERIFIED, key = "#transactionId")
@CachePut(value = SignUpConstants.STATUS_CHECK, key = "#transactionId")
@Cacheable(value = SignUpConstants.STATUS_CHECK, key = "#transactionId")
public RegistrationTransaction setStatusCheckTransaction(String transactionId,
RegistrationTransaction registrationTransaction) {
return registrationTransaction;
}

@CacheEvict(value = SignUpConstants.CHALLENGE_GENERATED, key = "#transactionId")
@Cacheable(value = SignUpConstants.BLOCKED_IDENTIFIER, key = "#key")
public String blockIdentifier(String key, String value) {
public String blockIdentifier(String transactionId, String key, String value) {
return value;
}

Expand All @@ -52,11 +48,26 @@ public String setSecretKey(String key, String secretKey) {
return secretKey;
}

@Cacheable(value = SignUpConstants.KEY_ALIAS, key = "#key")
@CachePut(value = SignUpConstants.KEY_ALIAS, key = "#key")
public String setActiveKeyAlias(String key, String alias) {
return alias;
}


//----- cache update is separated
//----- we are not using @cacheput as @cacheput extends the TTL on cache entry

public RegistrationTransaction createUpdateChallengeGeneratedTransaction(String transactionId,
RegistrationTransaction registrationTransaction) {
cacheManager.getCache(SignUpConstants.CHALLENGE_GENERATED).put(transactionId, registrationTransaction);
return registrationTransaction;
}

public void updateStatusCheckTransaction(String transactionId,
RegistrationTransaction registrationTransaction) {
cacheManager.getCache(SignUpConstants.STATUS_CHECK).put(transactionId, registrationTransaction);
}

//---Getter---
public RegistrationTransaction getChallengeGeneratedTransaction(String transactionId) {
return cacheManager.getCache(SignUpConstants.CHALLENGE_GENERATED).get(transactionId, RegistrationTransaction.class);
Expand Down
2 changes: 2 additions & 0 deletions signup-service/src/main/java/io/mosip/signup/services/ChallengeManagerService.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package io.mosip.signup.services;

import io.micrometer.core.annotation.Timed;
import io.mosip.esignet.core.util.IdentityProviderUtil;
import io.mosip.signup.dto.*;
import io.mosip.signup.exception.SignUpException;
Expand Down Expand Up @@ -39,6 +40,7 @@ public String generateChallenge(RegistrationTransaction transaction) throws Sign
throw new SignUpException(ErrorConstants.UNSUPPORTED_CHALLENGE_TYPE);
}

@Timed(value = "generateotp.api.timer", percentiles = {0.95, 0.99})
private String generateOTPChallenge(String challengeTransactionId) {
OtpRequest otpRequest = new OtpRequest();
otpRequest.setKey(challengeTransactionId);
Expand Down
2 changes: 2 additions & 0 deletions signup-service/src/main/java/io/mosip/signup/services/GoogleRecaptchaValidatorService.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package io.mosip.signup.services;

import io.micrometer.core.annotation.Timed;
import io.mosip.esignet.api.spi.CaptchaValidator;
import io.mosip.signup.dto.ReCaptchaResponse;
import lombok.extern.slf4j.Slf4j;
Expand Down Expand Up @@ -30,6 +31,7 @@ public class GoogleRecaptchaValidatorService implements CaptchaValidator {
@Autowired
private RestTemplate restTemplate;

@Timed(value = "validatecaptcha.api.timer", percentiles = {0.95, 0.99})
@Override
public boolean validateCaptcha(String captchaToken) {

Expand Down
28 changes: 22 additions & 6 deletions signup-service/src/main/java/io/mosip/signup/services/RegistrationService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import io.micrometer.core.annotation.Timed;
import io.mosip.esignet.core.util.IdentityProviderUtil;
import io.mosip.kernel.core.util.HMACUtils2;
import io.mosip.signup.dto.*;
Expand Down Expand Up @@ -146,7 +147,7 @@ public GenerateChallengeResponse generateChallenge(GenerateChallengeRequest gene
}
else {
transaction = cacheUtilService.getChallengeGeneratedTransaction(transactionId);
validateTransaction(transaction, identifier);
validateTransaction(transaction, identifier, generateChallengeRequest);
}

// generate Challenge
Expand All @@ -155,11 +156,11 @@ public GenerateChallengeResponse generateChallenge(GenerateChallengeRequest gene
transaction.setChallengeHash(challengeHash);
transaction.increaseAttempt();
transaction.setLocale(generateChallengeRequest.getLocale());
cacheUtilService.setChallengeGeneratedTransaction(transactionId, transaction);
cacheUtilService.createUpdateChallengeGeneratedTransaction(transactionId, transaction);

//Resend attempts exhausted, block the identifier for configured time.
if(transaction.getChallengeRetryAttempts() > resendAttempts)
cacheUtilService.blockIdentifier(transaction.getIdentifier(), "blocked");
cacheUtilService.blockIdentifier(transactionId, transaction.getIdentifier(), "blocked");

notificationHelper.sendSMSNotificationAsync(generateChallengeRequest.getIdentifier(), transaction.getLocale(),
SEND_OTP_SMS_NOTIFICATION_TEMPLATE_KEY, new HashMap<>(){{put("{challenge}", challenge);}})
Expand Down Expand Up @@ -265,10 +266,15 @@ public RegistrationStatusResponse updatePassword(ResetPasswordRequest resetPassw
}

if(!transaction.isValidIdentifier(resetPasswordRequest.getIdentifier().toLowerCase(Locale.ROOT))) {
log.error("generate-challenge failed: invalid identifier");
log.error("reset password failed: invalid identifier");
throw new SignUpException(ErrorConstants.IDENTIFIER_MISMATCH);
}

if(!transaction.getPurpose().equals(Purpose.RESET_PASSWORD)) {
log.error("reset password failed: purpose mismatch in transaction");
throw new SignUpException(ErrorConstants.UNSUPPORTED_PURPOSE);
}

Identity identity = new Identity();
identity.setUIN(cryptoHelper.symmetricDecrypt(transaction.getUin()));
identity.setIDSchemaVersion(idSchemaVersion);
Expand Down Expand Up @@ -339,7 +345,7 @@ public RegistrationStatusResponse getRegistrationStatus(String transactionId)
registrationTransaction.getHandlesStatus().put(handleRequestId, registrationStatus);
//TODO This is temporary fix, we need to remove this field later from registrationTransaction DTO.
registrationTransaction.setRegistrationStatus(registrationStatus);
cacheUtilService.setStatusCheckTransaction(transactionId, registrationTransaction);
cacheUtilService.updateStatusCheckTransaction(transactionId, registrationTransaction);
}
}
registrationTransaction = cacheUtilService.getStatusCheckTransaction(transactionId);
Expand Down Expand Up @@ -450,6 +456,7 @@ private void saveIdentityData(RegisterRequest registerRequest, String transactio
addIdentity(identityRequest, transactionId);
}

@Timed(value = "addidentity.api.timer", percentiles = {0.95, 0.99})
private void addIdentity(IdentityRequest identityRequest, String transactionId) throws SignUpException{

RestRequestWrapper<IdentityRequest> restRequest = new RestRequestWrapper<>();
Expand All @@ -475,6 +482,7 @@ private void addIdentity(IdentityRequest identityRequest, String transactionId)
restResponseWrapper.getErrors().get(0).getErrorCode() : ErrorConstants.ADD_IDENTITY_FAILED);
}

@Timed(value = "generatehash.api.timer", percentiles = {0.95, 0.99})
private Password generateSaltedHash(String password, String transactionId) throws SignUpException{

RestRequestWrapper<Password.PasswordPlaintext> restRequestWrapper = new RestRequestWrapper<>();
Expand All @@ -497,6 +505,7 @@ private Password generateSaltedHash(String password, String transactionId) throw
restResponseWrapper.getErrors().get(0).getErrorCode() : ErrorConstants.HASH_GENERATE_FAILED);
}

@Timed(value = "getuin.api.timer", percentiles = {0.95, 0.99})
private String getUniqueIdentifier(String transactionId) throws SignUpException {

RestResponseWrapper<UINResponse> restResponseWrapper = selfTokenRestTemplate.exchange(getUinEndpoint,
Expand All @@ -513,7 +522,8 @@ private String getUniqueIdentifier(String transactionId) throws SignUpException
restResponseWrapper.getErrors().get(0).getErrorCode() : ErrorConstants.GET_UIN_FAILED);
}

private void validateTransaction(RegistrationTransaction transaction, String identifier) {
private void validateTransaction(RegistrationTransaction transaction, String identifier,
GenerateChallengeRequest generateChallengeRequest) {
if(transaction == null) {
log.error("generate-challenge failed: validate transaction null");
throw new InvalidTransactionException();
Expand All @@ -533,8 +543,14 @@ private void validateTransaction(RegistrationTransaction transaction, String ide
log.error("generate-challenge failed: too early attempts");
throw new GenerateChallengeException(ErrorConstants.TOO_EARLY_ATTEMPT);
}

if(!transaction.getPurpose().equals(generateChallengeRequest.getPurpose())) {
log.error("generate-challenge failed: purpose mismatch");
throw new GenerateChallengeException(ErrorConstants.INVALID_PURPOSE);
}
}

@Timed(value = "getstatus.api.timer", percentiles = {0.95, 0.99})
private RegistrationStatus getRegistrationStatusFromServer(String applicationId) {
RestResponseWrapper<Map<String,String>> restResponseWrapper = selfTokenRestTemplate.exchange(getRegistrationStatusEndpoint,
HttpMethod.GET, null,
Expand Down
7 changes: 4 additions & 3 deletions signup-service/src/test/java/io/mosip/signup/services/CacheUtilServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public void test_RegistrationTransaction_cache() {
Mockito.when(cache.get("mock", RegistrationTransaction.class)).thenReturn(registrationTransaction);
Mockito.when(cacheManager.getCache(Mockito.anyString())).thenReturn(cache);

Assert.assertEquals(cacheUtilService.setChallengeGeneratedTransaction("mock",
Assert.assertEquals(cacheUtilService.createUpdateChallengeGeneratedTransaction("mock",
registrationTransaction), registrationTransaction);
Assert.assertEquals(cacheUtilService.setChallengeVerifiedTransaction("mock", "vmock",
registrationTransaction), registrationTransaction);
Expand All @@ -45,8 +45,9 @@ public void test_RegistrationTransaction_cache() {

@Test
public void setChallengeTransaction_thenPass() {
Mockito.when(cacheManager.getCache(Mockito.anyString())).thenReturn(cache);
RegistrationTransaction registrationTransaction = new RegistrationTransaction("+85512123123", Purpose.REGISTRATION);
Assert.assertEquals(cacheUtilService.setChallengeGeneratedTransaction("mock-transaction", registrationTransaction), registrationTransaction);
Assert.assertNotNull(cacheUtilService.setChallengeGeneratedTransaction("mock-transaction", registrationTransaction));
Assert.assertEquals(cacheUtilService.createUpdateChallengeGeneratedTransaction("mock-transaction", registrationTransaction), registrationTransaction);
Assert.assertNotNull(cacheUtilService.createUpdateChallengeGeneratedTransaction("mock-transaction", registrationTransaction));
}
}
2 changes: 2 additions & 0 deletions signup-service/src/test/java/io/mosip/signup/services/RegistrationServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1639,6 +1639,7 @@ public void doGenerateChallenge_withRetryAttemptsOver3time_thenFail() throws Sig
generateChallengeRequest.setIdentifier(identifier);
generateChallengeRequest.setCaptchaToken("mock-captcha");
generateChallengeRequest.setRegenerate(true);
generateChallengeRequest.setPurpose(Purpose.REGISTRATION);
String transactionId = "TRAN-1234";
RegistrationTransaction transaction = new RegistrationTransaction(identifier, Purpose.REGISTRATION);
transaction.setLastRetryAt(LocalDateTime.now(ZoneOffset.UTC).minusSeconds(40));
Expand All @@ -1665,6 +1666,7 @@ public void doGenerateChallenge_withTransactionId_thenPass() throws SignUpExcept
generateChallengeRequest.setIdentifier(identifier);
generateChallengeRequest.setCaptchaToken("mock-captcha");
generateChallengeRequest.setRegenerate(true);
generateChallengeRequest.setPurpose(Purpose.REGISTRATION);
String transactionId = "TRAN-1234";
RegistrationTransaction transaction = new RegistrationTransaction(identifier, Purpose.REGISTRATION);
transaction.setLastRetryAt(LocalDateTime.now(ZoneOffset.UTC).minusSeconds(40));
Expand Down

0 comments on commit f3c240e

Please sign in to comment.