From f9aa325afb9e202b75064747b7b57b113754016a Mon Sep 17 00:00:00 2001 From: Angel Botev Date: Sat, 10 Aug 2019 17:58:45 +0300 Subject: [PATCH 1/8] fix_search_by_username - transfer username search and input in lowercase; Signed-off-by: Angel Botev --- .../account/grid/ag/AdminServiceImpl.java | 2 +- .../nynja/account/models/AccountBuilder.java | 5 +++-- .../account/models/AccountByUsername.java | 2 +- .../AccountRepositoryAdditionalImpl.java | 2 +- .../account/services/AccountServiceImpl.java | 22 +++++++++---------- .../decomposition/AccountCreator.java | 2 +- .../nynja/account/validation/Validators.java | 4 ++-- 7 files changed, 20 insertions(+), 19 deletions(-) diff --git a/src/main/java/biz/nynja/account/grid/ag/AdminServiceImpl.java b/src/main/java/biz/nynja/account/grid/ag/AdminServiceImpl.java index d86831a..76f9127 100644 --- a/src/main/java/biz/nynja/account/grid/ag/AdminServiceImpl.java +++ b/src/main/java/biz/nynja/account/grid/ag/AdminServiceImpl.java @@ -155,7 +155,7 @@ public class AdminServiceImpl extends AdminAccountServiceGrpc.AdminAccountServic .newBuilder().setAccountId(pendingAccountResponse.getPendingAccountDetails().getAccountId()) .setAvatar(request.getAvatar()).setAccountMark(request.getAccountMark()) .setAccountName(request.getAccountName()).setFirstName(request.getFirstName()) - .setLastName(request.getLastName()).setUsername(request.getUsername()) + .setLastName(request.getLastName()).setUsername(request.getUsername().toLowerCase()) .addAllRoles(request.getRolesList()).setAccessStatus(request.getAccessStatus()) .setQrCode(request.getQrCode()).build(); diff --git a/src/main/java/biz/nynja/account/models/AccountBuilder.java b/src/main/java/biz/nynja/account/models/AccountBuilder.java index 1718ac1..6d7e3d5 100644 --- a/src/main/java/biz/nynja/account/models/AccountBuilder.java +++ b/src/main/java/biz/nynja/account/models/AccountBuilder.java @@ -10,6 +10,7 @@ import biz.nynja.account.validation.Validators; import org.apache.commons.lang3.SerializationUtils; import java.time.LocalDate; +import java.util.Locale; import java.util.stream.Collectors; public class AccountBuilder { @@ -26,7 +27,7 @@ public class AccountBuilder { newAccount.setLastName(request.getLastName()); newAccount.setAvatar(request.getAvatar()); newAccount.setAccountName(request.getAccountName()); - newAccount.setUsername(request.getUsername()); + newAccount.setUsername(request.getUsername().toLowerCase()); newAccount.setCreationTimestamp(creationTimestamp); newAccount.setQrCode(request.getQrCode()); newAccount.setRoles(request.getRolesList().stream().map(Enum::toString).collect(Collectors.toSet())); @@ -41,7 +42,7 @@ public class AccountBuilder { updatedAccount.setAccountName(request.getAccountName()); updatedAccount.setFirstName(request.getFirstName()); updatedAccount.setLastName(request.getLastName()); - updatedAccount.setUsername(request.getUsername()); + updatedAccount.setUsername(request.getUsername().toLowerCase()); updatedAccount.setLastUpdateTimestamp(lastUpdateTimestamp); updatedAccount.setAccessStatus(request.getAccessStatus().toString()); if (Validators.util.validateBirthdayIsSet(request.getBirthday())) { diff --git a/src/main/java/biz/nynja/account/models/AccountByUsername.java b/src/main/java/biz/nynja/account/models/AccountByUsername.java index e3c829e..a59e9b4 100644 --- a/src/main/java/biz/nynja/account/models/AccountByUsername.java +++ b/src/main/java/biz/nynja/account/models/AccountByUsername.java @@ -334,7 +334,7 @@ public class AccountByUsername { builder.setLastName(getLastName()); } if (getUsername() != null) { - builder.setUsername(getUsername()); + builder.setUsername(getUsername().toLowerCase()); } if (getQrCode() != null) { builder.setQrCode(getQrCode()); diff --git a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java index 277f72f..924de52 100644 --- a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java +++ b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java @@ -511,7 +511,7 @@ public class AccountRepositoryAdditionalImpl implements AccountRepositoryAdditio } public boolean foundExistingNotOwnUsername(UUID accountId, String username) { - AccountByUsername foundAccountByUsername = accountByUsernameRepository.findByUsername(username); + AccountByUsername foundAccountByUsername = accountByUsernameRepository.findByUsername(username.toLowerCase()); if (foundAccountByUsername == null) { return false; } else if (!foundAccountByUsername.getAccountId().equals(accountId)) { diff --git a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java index 63e4580..e443b9e 100644 --- a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java +++ b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java @@ -257,7 +257,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas @Permitted(role = RoleConstants.ACCOUNT_ADMIN) @Permitted(role = RoleConstants.USER) public void getAccountByUsername(GetByUsernameRequest request, StreamObserver responseObserver) { - logger.info("Getting account by username: {}", request.getUsername()); + logger.info("Getting account by username: {}", request.getUsername().toLowerCase()); Validation validation = validateGetByUsernameRequest(request); if (validation.hasErrors()) { logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), validation.getErrorMessage(), @@ -265,7 +265,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas return; } - Optional accountResponse = accountProvider.getAccountResponseByUsername(request.getUsername()); + Optional accountResponse = accountProvider.getAccountResponseByUsername(request.getUsername().toLowerCase()); if (!accountResponse.isPresent()) { logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), "Account not found", "", @@ -281,7 +281,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas } AccountResponse response = accountResponse.get(); - logger.info("SUCCESS: Found result for account by username {}. Account Id = {}.", request.getUsername(), + logger.info("SUCCESS: Found result for account by username {}. Account Id = {}.", request.getUsername().toLowerCase(), response.getAccountDetails().getAccountId()); responseObserver.onNext(response); responseObserver.onCompleted(); @@ -290,7 +290,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas @Override @Permitted(role = RoleConstants.ANY) public void searchByUsername(GetByUsernameRequest request, StreamObserver responseObserver) { - logger.info("Searching account by username: {}", request.getUsername()); + logger.info("Searching account by username: {}", request.getUsername().toLowerCase()); Validation validation = validateGetByUsernameRequest(request); if (validation.hasErrors()) { logAndBuildGrpcSearchResponse(responseObserver, SearchResponse.newBuilder(), validation.getErrorMessage(), @@ -298,10 +298,10 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas return; } - AccountByUsername account = accountByUsernameRepository.findByUsername(request.getUsername()); + AccountByUsername account = accountByUsernameRepository.findByUsername(request.getUsername().toLowerCase()); if (account == null) { logAndBuildGrpcSearchResponse(responseObserver, SearchResponse.newBuilder(), - "No matching accounts found for username: ", request.getUsername(), Cause.ACCOUNT_NOT_FOUND, + "No matching accounts found for username: ", request.getUsername().toLowerCase(), Cause.ACCOUNT_NOT_FOUND, "No matching accounts found for give username."); return; } @@ -310,7 +310,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas account.getAvatar(), account.getFirstName(), account.getLastName()); SearchResponse response = SearchResponse.newBuilder().setSearchResultDetails(searchResultDetails).build(); - logger.info("SUCCESS: Found result for account by username {}: \"{}\"", request.getUsername(), response); + logger.info("SUCCESS: Found result for account by username {}: \"{}\"", request.getUsername().toLowerCase(), response); responseObserver.onNext(response); responseObserver.onCompleted(); } @@ -320,9 +320,9 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas if ((request.getUsername() == null) || request.getUsername().isEmpty()) { validation.addError(new ValidationError("Missing username.", Cause.MISSING_USERNAME)); - } else if (!account.isValidUsername(request.getUsername())) { + } else if (!account.isValidUsername(request.getUsername().toLowerCase())) { validation.addError( - new ValidationError("Invalid username. Value: " + request.getUsername(), Cause.INVALID_USERNAME)); + new ValidationError("Invalid username. Value: " + request.getUsername().toLowerCase(), Cause.INVALID_USERNAME)); } return validation; } @@ -498,7 +498,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas StreamObserver responseObserver) { logger.info("Complete pending account creation for account id {} and username {} ...", request.getAccountId(), - request.getUsername()); + request.getUsername().toLowerCase()); logger.debug("Complete pending account creation...: {} ...", request); AccountResponse response = accountCreator.retrieveCompletePendingAccountResponse(request); @@ -547,7 +547,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas if (request.getUsername() != null && !request.getUsername().trim().isEmpty() && accountRepositoryAdditional .foundExistingNotOwnUsername(UUID.fromString(request.getAccountId()), request.getUsername())) { logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), - "User name already in use: {}", request.getUsername(), Cause.USERNAME_ALREADY_USED, + "User name already in use: {}", request.getUsername().toLowerCase(), Cause.USERNAME_ALREADY_USED, "Username is already used"); return; } diff --git a/src/main/java/biz/nynja/account/services/decomposition/AccountCreator.java b/src/main/java/biz/nynja/account/services/decomposition/AccountCreator.java index cc374ba..cdcb8ea 100644 --- a/src/main/java/biz/nynja/account/services/decomposition/AccountCreator.java +++ b/src/main/java/biz/nynja/account/services/decomposition/AccountCreator.java @@ -183,7 +183,7 @@ public class AccountCreator { if (request.getUsername() != null && !request.getUsername().trim().isEmpty() && accountRepositoryAdditional .foundExistingNotOwnUsername(UUID.fromString(request.getAccountId()), request.getUsername())) { return logAndBuildGrpcAccountResponse(AccountResponse.newBuilder(), "User name already in use: {}", - request.getUsername(), Cause.USERNAME_ALREADY_USED, "Username is already used"); + request.getUsername().toLowerCase(), Cause.USERNAME_ALREADY_USED, "Username is already used"); } return null; } diff --git a/src/main/java/biz/nynja/account/validation/Validators.java b/src/main/java/biz/nynja/account/validation/Validators.java index 388e89e..3d589f3 100644 --- a/src/main/java/biz/nynja/account/validation/Validators.java +++ b/src/main/java/biz/nynja/account/validation/Validators.java @@ -162,7 +162,7 @@ public class Validators { } if (request.getUsername() != null && !request.getUsername().trim().isEmpty() - && !isUsernameValid(request.getUsername())) { + && !isUsernameValid(request.getUsername().toLowerCase())) { validation.addError(new ValidationError("Invalid username", Cause.INVALID_USERNAME)); } @@ -519,7 +519,7 @@ public class Validators { } if (request.getUsername() != null && !request.getUsername().trim().isEmpty() - && !account.isUsernameValid(request.getUsername())) { + && !account.isUsernameValid(request.getUsername().toLowerCase())) { validation.addError(new ValidationError("Invalid username", Cause.INVALID_USERNAME)); } -- GitLab From 1378a9525dcba5e0808b0063a8460bb2e1e974c2 Mon Sep 17 00:00:00 2001 From: Angel Botev Date: Sat, 10 Aug 2019 18:00:36 +0300 Subject: [PATCH 2/8] Bumping new version Signed-off-by: Angel Botev --- charts/account-service/Chart.yaml | 2 +- releases/dev/account-service.yaml | 2 +- releases/prod/account-service.yaml | 2 +- releases/staging/account-service.yaml | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/charts/account-service/Chart.yaml b/charts/account-service/Chart.yaml index d3c5b5e..c75df32 100644 --- a/charts/account-service/Chart.yaml +++ b/charts/account-service/Chart.yaml @@ -2,4 +2,4 @@ apiVersion: v1 appVersion: "1.0" description: Deployment of the nynja account service. name: account-service -version: 0.2.1 +version: 0.2.2 diff --git a/releases/dev/account-service.yaml b/releases/dev/account-service.yaml index 30956c0..645dd2d 100644 --- a/releases/dev/account-service.yaml +++ b/releases/dev/account-service.yaml @@ -8,7 +8,7 @@ spec: chart: repository: https://nynjagroup.jfrog.io/nynjagroup/helm/ name: account-service - version: 0.2.1 + version: 0.2.2 values: replicaCount: 1 diff --git a/releases/prod/account-service.yaml b/releases/prod/account-service.yaml index 93bed20..95104ba 100644 --- a/releases/prod/account-service.yaml +++ b/releases/prod/account-service.yaml @@ -8,7 +8,7 @@ spec: chart: repository: https://nynjagroup.jfrog.io/nynjagroup/helm/ name: account-service - version: 0.2.1 + version: 0.2.2 values: replicaCount: 2 diff --git a/releases/staging/account-service.yaml b/releases/staging/account-service.yaml index 60a1b67..17882bd 100644 --- a/releases/staging/account-service.yaml +++ b/releases/staging/account-service.yaml @@ -8,7 +8,7 @@ spec: chart: repository: https://nynjagroup.jfrog.io/nynjagroup/helm/ name: account-service - version: 0.2.1 + version: 0.2.2 values: replicaCount: 2 -- GitLab From 4755e70c14d2eb22fc3eb496f3e530ce07d75bf1 Mon Sep 17 00:00:00 2001 From: Angel Botev Date: Sat, 10 Aug 2019 16:36:31 +0300 Subject: [PATCH 3/8] NY-7952 Fix validation of phone number - add only parse, remove validation; Signed-off-by: Angel Botev --- .../account/kafka/KafkaProducerConfig.java | 2 ++ .../nynja/account/validation/Validators.java | 22 +++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/src/main/java/biz/nynja/account/kafka/KafkaProducerConfig.java b/src/main/java/biz/nynja/account/kafka/KafkaProducerConfig.java index b9f2ebc..16ed9c7 100644 --- a/src/main/java/biz/nynja/account/kafka/KafkaProducerConfig.java +++ b/src/main/java/biz/nynja/account/kafka/KafkaProducerConfig.java @@ -41,6 +41,8 @@ public class KafkaProducerConfig { configProps.put(ProducerConfig.BOOTSTRAP_SERVERS_CONFIG, KAFKA_HOST + ":" + KAFKA_PORT); configProps.put(ProducerConfig.KEY_SERIALIZER_CLASS_CONFIG, StringSerializer.class); configProps.put(ProducerConfig.VALUE_SERIALIZER_CLASS_CONFIG, JsonSerializer.class); + // Check config below if kakfa is down + //configProps.put(ProducerConfig.RETRIES_CONFIG, 10); return new DefaultKafkaProducerFactory(configProps); diff --git a/src/main/java/biz/nynja/account/validation/Validators.java b/src/main/java/biz/nynja/account/validation/Validators.java index 3d589f3..e52244d 100644 --- a/src/main/java/biz/nynja/account/validation/Validators.java +++ b/src/main/java/biz/nynja/account/validation/Validators.java @@ -18,6 +18,7 @@ import org.slf4j.LoggerFactory; import com.google.i18n.phonenumbers.NumberParseException; import com.google.i18n.phonenumbers.PhoneNumberUtil; import com.google.i18n.phonenumbers.Phonenumber; +import com.google.i18n.phonenumbers.Phonenumber.PhoneNumber; import biz.nynja.account.grpc.AddAuthenticationProviderRequest; import biz.nynja.account.grpc.AuthProviderDetails; @@ -465,6 +466,27 @@ public class Validators { return true; } + + public boolean isParsablePhoneNumber(String phoneNumber) { + + logger.debug("Checking validity of phone number {}", phoneNumber); + + phoneNumber = phoneNumber.replaceAll("^0*", "").replaceAll("[^\\d]", ""); + // add + to phoneNumber. It must be with plus. Read documentation of phoneUtil.parse + + PhoneNumberUtil phoneUtil = PhoneNumberUtil.getInstance(); + Phonenumber.PhoneNumber pn = new PhoneNumber(); + try { + pn = phoneUtil.parse(phoneNumber, ""); + } catch (NumberParseException e) { + logger.error("Phone number parse exception for number: {}", phoneNumber); + return false; + } + + logger.debug("PhoneNumber: {} can be parsed!", phoneNumber); + + return true; + } } public static class Util { -- GitLab From 9d351ff2b67e81494cdcfda63adde96005089b49 Mon Sep 17 00:00:00 2001 From: Angel Botev Date: Mon, 12 Aug 2019 20:20:25 +0300 Subject: [PATCH 4/8] NY-7952 Fix validation of phone number - add normalizer; - remove validator; - change searchByPhoneNumber endpoint; Signed-off-by: Angel Botev --- .../account/phone/PhoneNumberNormalizer.java | 24 +++++++++++++++++++ .../account/services/AccountServiceImpl.java | 2 +- .../decomposition/AccountProvider.java | 16 +++++++++++++ .../nynja/account/validation/Validators.java | 20 ---------------- 4 files changed, 41 insertions(+), 21 deletions(-) diff --git a/src/main/java/biz/nynja/account/phone/PhoneNumberNormalizer.java b/src/main/java/biz/nynja/account/phone/PhoneNumberNormalizer.java index 6c09984..c2d54e4 100644 --- a/src/main/java/biz/nynja/account/phone/PhoneNumberNormalizer.java +++ b/src/main/java/biz/nynja/account/phone/PhoneNumberNormalizer.java @@ -10,6 +10,7 @@ import org.springframework.stereotype.Service; import com.google.i18n.phonenumbers.NumberParseException; import com.google.i18n.phonenumbers.PhoneNumberUtil; import com.google.i18n.phonenumbers.Phonenumber; +import com.google.i18n.phonenumbers.Phonenumber.PhoneNumber; import biz.nynja.account.grpc.AddContactInfoRequest; import biz.nynja.account.grpc.AuthProviderDetails; @@ -147,4 +148,27 @@ public class PhoneNumberNormalizer { } return country + ":" + normalizedPhoneNumber; } + + public String getNormalizedPhoneNumberWithoutSelector(String rawPhoneNumber) throws InvalidPhoneNumberException { + logger.info("libphone: New phone number normalization request received - phone number: {}", + rawPhoneNumber); + + String phone = rawPhoneNumber.replaceAll("^0*", "").replaceAll("[^\\d]", ""); + phone = "+" + phone; + + PhoneNumberUtil phoneUtil = PhoneNumberUtil.getInstance(); + + String normalizedPhoneNumber = ""; + Phonenumber.PhoneNumber pn = new PhoneNumber(); + try { + pn = phoneUtil.parse(phone, ""); + normalizedPhoneNumber = Integer.toString(pn.getCountryCode()) + Long.toString(pn.getNationalNumber()); + logger.info("libphone: Normalized phone number: " + normalizedPhoneNumber); + } catch (NumberParseException e) { + logger.error("libphone: NumberParseException was thrown: {}", e.toString()); + logger.debug("libphone: NumberParseException was thrown: {}", e.getCause()); + } + return normalizedPhoneNumber; + } + } diff --git a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java index e443b9e..8af84ac 100644 --- a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java +++ b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java @@ -231,7 +231,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas Optional account = Optional.empty(); try { - account = accountProvider.searchAccountByLoginOption(AuthenticationType.PHONE, request.getPhoneNumber()); + account = accountProvider.searchAccountByLoginOptionWithoutSelector(AuthenticationType.PHONE, request.getPhoneNumber()); } catch (IncorrectAccountCountException e) { logAndBuildGrpcSearchResponse(responseObserver, SearchResponse.newBuilder(), "Error while searching for phone: ", request.getPhoneNumber(), Cause.INTERNAL_SERVER_ERROR, ""); diff --git a/src/main/java/biz/nynja/account/services/decomposition/AccountProvider.java b/src/main/java/biz/nynja/account/services/decomposition/AccountProvider.java index a062706..1e82f08 100644 --- a/src/main/java/biz/nynja/account/services/decomposition/AccountProvider.java +++ b/src/main/java/biz/nynja/account/services/decomposition/AccountProvider.java @@ -136,6 +136,22 @@ public class AccountProvider { return Optional.of(accountByProfileId.get().toAccount()); } + public Optional searchAccountByLoginOptionWithoutSelector(AuthenticationType type, String authenticationIdentifier) + throws IncorrectAccountCountException { + if (type == AuthenticationType.PHONE) { + authenticationIdentifier = phoneNumberNormalizer.getNormalizedPhoneNumberWithoutSelector(authenticationIdentifier); + } + + Optional accountByProfileId = accountRepositoryAdditional.searchAccountByLoginOption( + AuthenticationProvider.createAuthenticationProviderFromStringsWithDefaultSearchableOption( + type.toString(), authenticationIdentifier)); + if (!accountByProfileId.isPresent()) { + return Optional.empty(); + } + + return Optional.of(accountByProfileId.get().toAccount()); + } + public Optional getAccountResponseByAuthenticationProvider(AuthenticationType type, String authenticationIdentifier) { Optional account = getAccountByAuthenticationProvider(type, authenticationIdentifier); diff --git a/src/main/java/biz/nynja/account/validation/Validators.java b/src/main/java/biz/nynja/account/validation/Validators.java index e52244d..78d2080 100644 --- a/src/main/java/biz/nynja/account/validation/Validators.java +++ b/src/main/java/biz/nynja/account/validation/Validators.java @@ -467,26 +467,6 @@ public class Validators { return true; } - public boolean isParsablePhoneNumber(String phoneNumber) { - - logger.debug("Checking validity of phone number {}", phoneNumber); - - phoneNumber = phoneNumber.replaceAll("^0*", "").replaceAll("[^\\d]", ""); - // add + to phoneNumber. It must be with plus. Read documentation of phoneUtil.parse - - PhoneNumberUtil phoneUtil = PhoneNumberUtil.getInstance(); - Phonenumber.PhoneNumber pn = new PhoneNumber(); - try { - pn = phoneUtil.parse(phoneNumber, ""); - } catch (NumberParseException e) { - logger.error("Phone number parse exception for number: {}", phoneNumber); - return false; - } - - logger.debug("PhoneNumber: {} can be parsed!", phoneNumber); - - return true; - } } public static class Util { -- GitLab From c5c5d6b60647af587f99382c2017b3dee7cee9b6 Mon Sep 17 00:00:00 2001 From: Stoyan Tzenkov Date: Tue, 13 Aug 2019 11:53:46 +0300 Subject: [PATCH 5/8] NY-7952: Fix implemented to handle NumberParseException. Signed-off-by: Stoyan Tzenkov --- .../nynja/account/phone/PhoneNumberNormalizer.java | 3 ++- .../nynja/account/services/AccountServiceImpl.java | 13 +++++++------ .../services/decomposition/AccountProvider.java | 3 ++- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/main/java/biz/nynja/account/phone/PhoneNumberNormalizer.java b/src/main/java/biz/nynja/account/phone/PhoneNumberNormalizer.java index c2d54e4..0b98c4a 100644 --- a/src/main/java/biz/nynja/account/phone/PhoneNumberNormalizer.java +++ b/src/main/java/biz/nynja/account/phone/PhoneNumberNormalizer.java @@ -153,7 +153,7 @@ public class PhoneNumberNormalizer { logger.info("libphone: New phone number normalization request received - phone number: {}", rawPhoneNumber); - String phone = rawPhoneNumber.replaceAll("^0*", "").replaceAll("[^\\d]", ""); + String phone = rawPhoneNumber.replaceAll("[^\\d]", "").replaceAll("^0*", ""); phone = "+" + phone; PhoneNumberUtil phoneUtil = PhoneNumberUtil.getInstance(); @@ -167,6 +167,7 @@ public class PhoneNumberNormalizer { } catch (NumberParseException e) { logger.error("libphone: NumberParseException was thrown: {}", e.toString()); logger.debug("libphone: NumberParseException was thrown: {}", e.getCause()); + throw new InvalidPhoneNumberException("Phone number with wrong format: " + rawPhoneNumber); } return normalizedPhoneNumber; } diff --git a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java index 8af84ac..f5bcaee 100644 --- a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java +++ b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java @@ -67,6 +67,7 @@ import biz.nynja.account.permissions.PerformPermissionCheck; import biz.nynja.account.permissions.PermissionsValidator; import biz.nynja.account.permissions.Permitted; import biz.nynja.account.permissions.RoleConstants; +import biz.nynja.account.phone.InvalidPhoneNumberException; import biz.nynja.account.phone.PhoneNumberNormalizer; import biz.nynja.account.repositories.AccountByProfileIdRepository; import biz.nynja.account.repositories.AccountByQrCodeRepository; @@ -222,12 +223,6 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas Cause.MISSING_PHONENUMBER, ""); return; } - if (!phoneValidator.isPhoneNumberValid(request.getPhoneNumber())) { - logAndBuildGrpcSearchResponse(responseObserver, SearchResponse.newBuilder(), - "Invalid phone number. Value: {}", request.getPhoneNumber(), Cause.INVALID_PHONENUMBER, - "Phone number parameter has invalid format."); - return; - } Optional account = Optional.empty(); try { @@ -235,6 +230,12 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas } catch (IncorrectAccountCountException e) { logAndBuildGrpcSearchResponse(responseObserver, SearchResponse.newBuilder(), "Error while searching for phone: ", request.getPhoneNumber(), Cause.INTERNAL_SERVER_ERROR, ""); + return; + } catch (InvalidPhoneNumberException e) { + logAndBuildGrpcSearchResponse(responseObserver, SearchResponse.newBuilder(), + "Invalid phone number. Value: {}", request.getPhoneNumber(), Cause.INVALID_PHONENUMBER, + "Phone number parameter has invalid format."); + return; } if (!account.isPresent()) { diff --git a/src/main/java/biz/nynja/account/services/decomposition/AccountProvider.java b/src/main/java/biz/nynja/account/services/decomposition/AccountProvider.java index 1e82f08..3065030 100644 --- a/src/main/java/biz/nynja/account/services/decomposition/AccountProvider.java +++ b/src/main/java/biz/nynja/account/services/decomposition/AccountProvider.java @@ -26,6 +26,7 @@ import biz.nynja.account.models.AccountByProfileId; import biz.nynja.account.models.AccountByQrCode; import biz.nynja.account.models.AccountByUsername; import biz.nynja.account.models.AuthenticationProvider; +import biz.nynja.account.phone.InvalidPhoneNumberException; import biz.nynja.account.phone.PhoneNumberNormalizer; import biz.nynja.account.repositories.AccountByProfileIdRepository; import biz.nynja.account.repositories.AccountByQrCodeRepository; @@ -137,7 +138,7 @@ public class AccountProvider { } public Optional searchAccountByLoginOptionWithoutSelector(AuthenticationType type, String authenticationIdentifier) - throws IncorrectAccountCountException { + throws IncorrectAccountCountException, InvalidPhoneNumberException { if (type == AuthenticationType.PHONE) { authenticationIdentifier = phoneNumberNormalizer.getNormalizedPhoneNumberWithoutSelector(authenticationIdentifier); } -- GitLab From c1ca99023194ad18341072316034d21659a2728f Mon Sep 17 00:00:00 2001 From: Stoyan Tzenkov Date: Tue, 13 Aug 2019 15:16:48 +0300 Subject: [PATCH 6/8] NY-7952: Unit tests fixed. Signed-off-by: Stoyan Tzenkov --- .../nynja/account/services/AccountServiceTests.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/test/java/biz/nynja/account/services/AccountServiceTests.java b/src/test/java/biz/nynja/account/services/AccountServiceTests.java index 955277f..148df8d 100644 --- a/src/test/java/biz/nynja/account/services/AccountServiceTests.java +++ b/src/test/java/biz/nynja/account/services/AccountServiceTests.java @@ -8,7 +8,9 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import java.util.ArrayList; import java.util.LinkedList; @@ -104,6 +106,7 @@ import biz.nynja.account.services.decomposition.IncorrectAccountCountException; import biz.nynja.account.services.decomposition.ProfileProvider; import biz.nynja.account.utils.GrpcServerTestBase; import biz.nynja.account.utils.Util; +import biz.nynja.account.phone.InvalidPhoneNumberException; import io.grpc.Metadata; import io.grpc.stub.MetadataUtils; @@ -1546,7 +1549,7 @@ public class AccountServiceTests extends GrpcServerTestBase { Optional response = Optional.of(savedAccount); - given(accountProvider.searchAccountByLoginOption(AuthenticationType.PHONE, request.getPhoneNumber())) + given(accountProvider.searchAccountByLoginOptionWithoutSelector(AuthenticationType.PHONE, request.getPhoneNumber())) .willReturn(response); final SearchResponse reply = searchServiceBlockingStub.searchByPhoneNumber(request); @@ -1585,10 +1588,13 @@ public class AccountServiceTests extends GrpcServerTestBase { } @Test - public void testSearchByPhoneNumberInvalid() { + public void testSearchByPhoneNumberInvalid() throws InvalidPhoneNumberException, IncorrectAccountCountException { + Exception ex = new InvalidPhoneNumberException("Invalid phone number"); final GetByPhoneNumberRequest request = GetByPhoneNumberRequest.newBuilder() .setPhoneNumber(Util.S_INVALID_PHONENUMBER).build(); + given(accountProvider.searchAccountByLoginOptionWithoutSelector(AuthenticationType.PHONE, request.getPhoneNumber())).willThrow(ex); + final SearchResponse reply = searchServiceBlockingStub.searchByPhoneNumber(request); assertNotNull("Reply should not be null", reply); -- GitLab From 9f21e52752ef7f3f2260b7c59bdf348476102f60 Mon Sep 17 00:00:00 2001 From: Stoyan Tzenkov Date: Tue, 13 Aug 2019 16:24:16 +0300 Subject: [PATCH 7/8] NY-7952: Got rid of some commented lines. Signed-off-by: Stoyan Tzenkov --- src/main/java/biz/nynja/account/kafka/KafkaProducerConfig.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/main/java/biz/nynja/account/kafka/KafkaProducerConfig.java b/src/main/java/biz/nynja/account/kafka/KafkaProducerConfig.java index 16ed9c7..b9f2ebc 100644 --- a/src/main/java/biz/nynja/account/kafka/KafkaProducerConfig.java +++ b/src/main/java/biz/nynja/account/kafka/KafkaProducerConfig.java @@ -41,8 +41,6 @@ public class KafkaProducerConfig { configProps.put(ProducerConfig.BOOTSTRAP_SERVERS_CONFIG, KAFKA_HOST + ":" + KAFKA_PORT); configProps.put(ProducerConfig.KEY_SERIALIZER_CLASS_CONFIG, StringSerializer.class); configProps.put(ProducerConfig.VALUE_SERIALIZER_CLASS_CONFIG, JsonSerializer.class); - // Check config below if kakfa is down - //configProps.put(ProducerConfig.RETRIES_CONFIG, 10); return new DefaultKafkaProducerFactory(configProps); -- GitLab From 1a2bf163cf965d4e4415f846b2c463b717a45184 Mon Sep 17 00:00:00 2001 From: Dimitar Ivanov Date: Tue, 13 Aug 2019 16:47:18 +0300 Subject: [PATCH 8/8] Bumping chart version. --- charts/account-service/Chart.yaml | 2 +- releases/dev/account-service.yaml | 2 +- releases/prod/account-service.yaml | 2 +- releases/staging/account-service.yaml | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/charts/account-service/Chart.yaml b/charts/account-service/Chart.yaml index c75df32..941149a 100644 --- a/charts/account-service/Chart.yaml +++ b/charts/account-service/Chart.yaml @@ -2,4 +2,4 @@ apiVersion: v1 appVersion: "1.0" description: Deployment of the nynja account service. name: account-service -version: 0.2.2 +version: 0.2.3 diff --git a/releases/dev/account-service.yaml b/releases/dev/account-service.yaml index 645dd2d..3d7e097 100644 --- a/releases/dev/account-service.yaml +++ b/releases/dev/account-service.yaml @@ -8,7 +8,7 @@ spec: chart: repository: https://nynjagroup.jfrog.io/nynjagroup/helm/ name: account-service - version: 0.2.2 + version: 0.2.3 values: replicaCount: 1 diff --git a/releases/prod/account-service.yaml b/releases/prod/account-service.yaml index 95104ba..0975a47 100644 --- a/releases/prod/account-service.yaml +++ b/releases/prod/account-service.yaml @@ -8,7 +8,7 @@ spec: chart: repository: https://nynjagroup.jfrog.io/nynjagroup/helm/ name: account-service - version: 0.2.2 + version: 0.2.3 values: replicaCount: 2 diff --git a/releases/staging/account-service.yaml b/releases/staging/account-service.yaml index 17882bd..32c2d3f 100644 --- a/releases/staging/account-service.yaml +++ b/releases/staging/account-service.yaml @@ -8,7 +8,7 @@ spec: chart: repository: https://nynjagroup.jfrog.io/nynjagroup/helm/ name: account-service - version: 0.2.2 + version: 0.2.3 values: replicaCount: 2 -- GitLab