From 820605a1c53a43053dc25f42fc57521efdff2cec Mon Sep 17 00:00:00 2001 From: Stoyan Tzenkov Date: Tue, 30 Oct 2018 12:45:19 +0200 Subject: [PATCH 1/6] NY_4769: Fixed - phone number length limited to 15 DTMFs Signed-off-by: Stoyan Tzenkov --- .../nynja/account/components/Validator.java | 105 +++++++++++++++++- .../account/services/AccountServiceImpl.java | 3 +- 2 files changed, 102 insertions(+), 6 deletions(-) diff --git a/src/main/java/biz/nynja/account/components/Validator.java b/src/main/java/biz/nynja/account/components/Validator.java index 9f7adf6..e460550 100644 --- a/src/main/java/biz/nynja/account/components/Validator.java +++ b/src/main/java/biz/nynja/account/components/Validator.java @@ -71,6 +71,100 @@ public class Validator { } + private HashMap countryInfoMap; + + @PostConstruct + public void loadPhonesBook() { + + CountryInfo countryInfo = null; + BufferedReader reader = null; + countryInfoMap = new HashMap<>(); + + logger.debug("Loading phones information from file."); + try { + Resource resource = new ClassPathResource("countries.txt"); + InputStream resourceInputStream = resource.getInputStream(); + logger.debug("Phones information loaded."); + reader = new BufferedReader(new InputStreamReader(resourceInputStream)); + String line; + while ((line = reader.readLine()) != null) { + String[] args = line.split(";"); + countryInfo = new CountryInfo(); + countryInfo.setCountryCode(args[1]); + countryInfo.setCountryPhoneCode(args[0]); + countryInfo.setCountryName(args[2]); + if (args.length > 3) { + countryInfo.setPhoneFormat(args[3]); + } + countryInfoMap.put(args[1], countryInfo); + } + } catch (IOException e) { + logger.error("Error during load phones information: {}", e.getMessage()); + } finally { + try { + reader.close(); + } catch (IOException e) { + logger.error("Close reader error: {}", e.getMessage()); + } + } + + } + + public boolean isPhoneNumberValid(String phoneNumber, String countryCode) { + + logger.debug("Checking phoneNumber: {} for country: {}", phoneNumber, countryCode); + CountryInfo countryInfo = countryInfoMap.get(countryCode); + if (countryInfo == null) { + logger.debug("Country: {} not found!", countryCode); + return false; + } + + char[] digits = countryInfo.getCountryPhoneCode().toCharArray(); + StringBuilder builder = new StringBuilder(); + for (char digit : digits) { + builder.append("[" + digit + "]"); + } + String codePattern = builder.toString(); + + String phoneLength = null; + String phoneFormat = "XXXXXXXXXXXXXXX"; + if (countryInfo.getPhoneFormat() != null) { + phoneFormat = countryInfo.getPhoneFormat(); + } + phoneLength = "{" + phoneFormat.replaceAll("\\s", "").length() + "}"; + + final String PHONE_PATTERN = "((?:\\+?([0][0])?" + codePattern + ")?||([0][0]" + codePattern + ")?)(\\d" + + phoneLength + ")$"; + logger.debug("Generated phone pattern for country: {}, {}", countryCode, PHONE_PATTERN); + + Pattern pattern = Pattern.compile(PHONE_PATTERN); + Matcher matcher = pattern.matcher(phoneNumber); + + boolean isValid = matcher.matches(); + logger.debug("PhoneNumber: {} for country: {} is valid: {}", phoneNumber, countryCode, isValid); + + return isValid; + } + + public String getNormalizedPhoneNumber(String authenticationProvider) { + String[] provider = authenticationProvider.split(":"); + String country = provider[0]; + String phoneNumber = provider[1]; + logger.info("libphone: New phone number normalization request received - phone number: {}, country code: {}", + phoneNumber, country); + + PhoneNumberUtil phoneUtil = PhoneNumberUtil.getInstance(); + String normalizedPhoneNumber = ""; + try { + PhoneNumber pn = phoneUtil.parse(phoneNumber, country); + 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; + } public boolean isUsernameValid(String username) { @@ -170,9 +264,10 @@ public class Validator { return null; } - public Optional> validateContactInfo(ContactType type, String contactInfoValue) { + public Optional> validateContactInfo(ContactType type, String contactInfoValue) { if (contactInfoValue == null || contactInfoValue.trim().isEmpty()) { - return Optional.of(new ImmutablePair<>(Cause.MISSING_CONTACT_INFO_IDENTIFIER, "Missing contact info identifier")); + return Optional + .of(new ImmutablePair<>(Cause.MISSING_CONTACT_INFO_IDENTIFIER, "Missing contact info identifier")); } switch (type) { case MISSING_CONTACT_TYPE: @@ -295,7 +390,8 @@ public class Validator { return validateContactInfoRequest(accountId, editedContactInfoDetails); } - public Optional> validateContactInfoRequest(String accountId, ContactDetails contactDetails) { + public Optional> validateContactInfoRequest(String accountId, + ContactDetails contactDetails) { if ((accountId == null) || (accountId.isEmpty())) { return Optional.of(new ImmutablePair<>(Cause.MISSING_ACCOUNT_ID, "Missing account id")); } @@ -304,7 +400,8 @@ public class Validator { } if (contactDetails.getValue() == null || contactDetails.getValue().isEmpty()) { - return Optional.of(new ImmutablePair<>(Cause.MISSING_CONTACT_INFO_IDENTIFIER, "Missing contact info identifier")); + return Optional + .of(new ImmutablePair<>(Cause.MISSING_CONTACT_INFO_IDENTIFIER, "Missing contact info identifier")); } if (!isValidUuid(accountId)) { return Optional.of(new ImmutablePair<>(Cause.INVALID_ACCOUNT_ID, "Invalid account id")); diff --git a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java index 9f6ea47..d72ca3b 100644 --- a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java +++ b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java @@ -13,7 +13,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import biz.nynja.account.components.Validator; - import biz.nynja.account.grpc.*; import biz.nynja.account.grpc.ErrorResponse.Cause; import biz.nynja.account.models.Account; @@ -607,7 +606,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas StreamObserver responseObserver) { logger.info("Adding authentication provider to profile requested."); logger.debug("Adding authentication provider to profile requested: {}", request); - if ((request.getProfileId() == null) || (request.getProfileId().isEmpty())) { + if ((request.getProfileId() == null) || (request.getProfileId().isEmpty())) { responseObserver.onNext(StatusResponse.newBuilder() .setError(ErrorResponse.newBuilder().setCause(Cause.MISSING_PROFILE_ID)).build()); responseObserver.onCompleted(); -- GitLab From ce1735fe35bb23afeceda652c0519e5aab0c51d7 Mon Sep 17 00:00:00 2001 From: Stoyan Tzenkov Date: Tue, 30 Oct 2018 15:37:54 +0200 Subject: [PATCH 2/6] NY-4662 fixed as well Signed-off-by: Stoyan Tzenkov --- .../account/services/AccountServiceImpl.java | 327 ++++++++---------- 1 file changed, 149 insertions(+), 178 deletions(-) diff --git a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java index d72ca3b..b815a52 100644 --- a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java +++ b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java @@ -93,19 +93,22 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas logger.info("Getting account by authentication provider: {}", request); if (request.getAuthenticationTypeValue() == 0) { - sendErrorMessageForAccountResponse(responseObserver, Cause.MISSING_AUTH_PROVIDER_TYPE); + logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), + "Missing authentication provider", "", Cause.MISSING_AUTH_PROVIDER_TYPE); return; } if (request.getAuthenticationIdentifier() == null || request.getAuthenticationIdentifier().isEmpty()) { - sendErrorMessageForAccountResponse(responseObserver, Cause.MISSING_AUTH_PROVIDER_IDENTIFIER); + logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), + "Missing authentication provider identifier", "", Cause.MISSING_AUTH_PROVIDER_IDENTIFIER); return; } Optional account = accountsProvider.getAccountResponseByAuthenticationProvider( request.getAuthenticationType(), request.getAuthenticationIdentifier()); if (!account.isPresent()) { - sendErrorMessageForAccountResponse(responseObserver, Cause.ACCOUNT_NOT_FOUND); - return; + logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), + "Account not found for identifier: {}", request.getAuthenticationIdentifier(), + Cause.ACCOUNT_NOT_FOUND); } else { responseObserver.onNext(account.get()); responseObserver.onCompleted(); @@ -118,11 +121,11 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas public void searchByEmail(GetByEmailRequest request, StreamObserver responseObserver) { logger.info("Search account by e-mail: {}", request.getEmail()); if ((request.getEmail() == null) || request.getEmail().isEmpty()) { - logAndBuildGrpcResponse(responseObserver, new ValidationError("Missing e-mail.", Cause.MISSING_EMAIL)); + logAndBuildGrpcSearchResponse(responseObserver, SearchResponse.newBuilder(), "Missing e-mail.", "", Cause.MISSING_EMAIL); return; } if (!validator.isEmailValid(request.getEmail())) { - logAndBuildGrpcResponse(responseObserver, new ValidationError("Invalid e-mail!. Value : " + request.getEmail(), Cause.EMAIL_INVALID)); + logAndBuildGrpcSearchResponse(responseObserver, SearchResponse.newBuilder(), "Invalid e-mail!. Value : ", request.getEmail(), Cause.EMAIL_INVALID); return; } @@ -130,8 +133,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas request.getEmail()); if (!account.isPresent()) { - logAndBuildGrpcResponse(responseObserver, new ValidationError("No matching accounts found for e-mail: " + request.getEmail(), - Cause.EMAIL_NOT_FOUND)); + logAndBuildGrpcSearchResponse(responseObserver, SearchResponse.newBuilder(), "No matching accounts found for e-mail: ", request.getEmail(), Cause.EMAIL_NOT_FOUND); return; } else { SearchResultDetails searchResultDetails = buildSearchResultDetails(account.get().getAccountId().toString(), @@ -150,12 +152,12 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas logger.info("Search account by phone: {}", request.getPhoneNumber()); if ((request.getPhoneNumber() == null) || request.getPhoneNumber().isEmpty()) { - logAndBuildGrpcResponse(responseObserver, new ValidationError("Missing phone number.", Cause.MISSING_PHONENUMBER)); + logAndBuildGrpcSearchResponse(responseObserver, SearchResponse.newBuilder(), "Missing phone number.", "", Cause.MISSING_PHONENUMBER); return; } if (!phoneNumberValidator.isPhoneNumberValid(request.getPhoneNumber())) { - logAndBuildGrpcResponse(responseObserver, new ValidationError("Invalid phone number.", Cause.INVALID_PHONENUMBER)); + logAndBuildGrpcSearchResponse(responseObserver, SearchResponse.newBuilder(), "Invalid phone number. Value : ", request.getPhoneNumber(), Cause.INVALID_PHONENUMBER); return; } @@ -163,8 +165,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas request.getPhoneNumber()); if (!account.isPresent()) { - logAndBuildGrpcResponse(responseObserver, new ValidationError( - "No matching accounts found for phone:" + request.getPhoneNumber(), Cause.PHONENUMBER_NOT_FOUND)); + logAndBuildGrpcSearchResponse(responseObserver, SearchResponse.newBuilder(), "No matching accounts found for phone: ", request.getPhoneNumber(), Cause.PHONENUMBER_NOT_FOUND); return; } else { SearchResultDetails searchResultDetails = buildSearchResultDetails(account.get().getAccountId().toString(), @@ -183,19 +184,14 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas logger.info("Getting account by username: {}", request.getUsername()); Validation validation = validateGetByUsernameRequest(request); if(validation.hasErrors()) { - logger.debug(validation.getErrorMessage()); - responseObserver.onNext(AccountResponse.newBuilder() - .setError(ErrorResponse.newBuilder().setMessage(validation.getErrorMessage()) - .setCause(validation.getCause().get())).build()); - responseObserver.onCompleted(); + logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), "Invalid user name : ", request.getUsername(), Cause.USERNAME_INVALID); return; } - Optional accountResonse = - accountsProvider.getAccountResponseByUsername(request.getUsername()); + Optional accountResonse = accountsProvider.getAccountResponseByUsername(request.getUsername()); if (!accountResonse.isPresent()) { - sendErrorMessageForAccountResponse(responseObserver, Cause.ACCOUNT_NOT_FOUND); + logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), "Account not found", "", Cause.ACCOUNT_NOT_FOUND); return; } else { responseObserver.onNext(accountResonse.get()); @@ -209,14 +205,13 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas logger.info("Searching account by username: {}", request.getUsername()); Validation validation = validateGetByUsernameRequest(request); if(validation.hasErrors()) { - logAndBuildGrpcResponse(responseObserver, validation); + logAndBuildGrpcSearchResponse(responseObserver, SearchResponse.newBuilder(), "Invalid user name.", "", Cause.USERNAME_INVALID); return; } AccountByUsername account = accountByUsernameRepository.findByUsername(request.getUsername()); if (account == null) { - logAndBuildGrpcResponse(responseObserver, new ValidationError("No matching accounts found for username: "+ - request.getUsername(), Cause.USERNAME_NOT_FOUND)); + logAndBuildGrpcSearchResponse(responseObserver, SearchResponse.newBuilder(), "No matching accounts found for username: ", request.getUsername(), Cause.USERNAME_NOT_FOUND); return; } @@ -249,19 +244,14 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas public void getAccountByQrCode(GetByQrCodeRequest request, StreamObserver responseObserver) { logger.info("Search account by QR code: {}", request.getQrCode()); if ((request.getQrCode() == null) || request.getQrCode().isEmpty()) { - logger.debug("Missing QR code."); - responseObserver.onNext(AccountResponse.newBuilder() - .setError(ErrorResponse.newBuilder().setMessage("Missing QR code.") - .setCause(Cause.MISSING_QR_CODE)).build()); - responseObserver.onCompleted(); + logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), "Missing QR code.", "", Cause.MISSING_QR_CODE); return; } - Optional accountResonse = - accountsProvider.getAccountResponseByQrCode(request.getQrCode()); + Optional accountResonse = accountsProvider.getAccountResponseByQrCode(request.getQrCode()); if (!accountResonse.isPresent()) { - sendErrorMessageForAccountResponse(responseObserver, Cause.ACCOUNT_NOT_FOUND); + logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), "Account not found", "", Cause.ACCOUNT_NOT_FOUND); return; } else { responseObserver.onNext(accountResonse.get()); @@ -275,14 +265,13 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas public void searchByQrCode(GetByQrCodeRequest request, StreamObserver responseObserver) { logger.info("Search account by QR code: {}", request.getQrCode()); if ((request.getQrCode() == null) || request.getQrCode().isEmpty()) { - logAndBuildGrpcResponse(responseObserver, new ValidationError("Missing QR code.", Cause.MISSING_QR_CODE)); + logAndBuildGrpcSearchResponse(responseObserver, SearchResponse.newBuilder(), "Missing QR code.", "", Cause.MISSING_QR_CODE); return; } AccountByQrCode account = accountByQrCodeRepository.findByQrCode(request.getQrCode()); if (account == null) { - logAndBuildGrpcResponse(responseObserver, new ValidationError("No matching accounts found for QR code! Value: " + request.getQrCode(), - Cause.QR_CODE_NOT_FOUND)); + logAndBuildGrpcSearchResponse(responseObserver, SearchResponse.newBuilder(), "No matching accounts found for QR code! Value: ", request.getQrCode(), Cause.QR_CODE_NOT_FOUND); return; } @@ -304,17 +293,20 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas logger.info("Getting accounts by profile id: {}", request.getProfileId()); if ((request.getProfileId() == null) || (request.getProfileId().isEmpty())) { - sendErrorMessageForAccountsResponse(responseObserver, Cause.MISSING_PROFILE_ID); + logAndBuildGrpcAccountsResponse(responseObserver, AccountsResponse.newBuilder(), "Missing profile id", "", + Cause.MISSING_PROFILE_ID); return; } if (!validator.isValidUuid(request.getProfileId())) { - sendErrorMessageForAccountsResponse(responseObserver, Cause.INVALID_PROFILE_ID); + logAndBuildGrpcAccountsResponse(responseObserver, AccountsResponse.newBuilder(), "Invalid profile id: {}", + request.getProfileId(), Cause.INVALID_PROFILE_ID); return; } Optional accounts = accountsProvider.getAllAccountsByProfileId(request); if (!accounts.isPresent()) { - sendErrorMessageForAccountsResponse(responseObserver, Cause.ACCOUNT_NOT_FOUND); + logAndBuildGrpcAccountsResponse(responseObserver, AccountsResponse.newBuilder(), + "Account not found for profile id: {}", request.getProfileId(), Cause.ACCOUNT_NOT_FOUND); } else { responseObserver.onNext(accounts.get()); responseObserver.onCompleted(); @@ -329,20 +321,22 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas logger.info("Getting accounts by account id: {}", request.getAccountId()); if ((request.getAccountId() == null) || (request.getAccountId().isEmpty())) { - sendErrorMessageForAccountResponse(responseObserver, Cause.MISSING_ACCOUNT_ID); + logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), "Missing account id", "", + Cause.MISSING_ACCOUNT_ID); return; } if (!validator.isValidUuid(request.getAccountId())) { - sendErrorMessageForAccountResponse(responseObserver, Cause.INVALID_ACCOUNT_ID); + logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), "Invalid account id: {}", + request.getAccountId(), Cause.INVALID_ACCOUNT_ID); return; } Optional account = accountsProvider.getAccountByAccountId(request); if (!account.isPresent()) { - sendErrorMessageForAccountResponse(responseObserver, Cause.ACCOUNT_NOT_FOUND); - return; + logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), "Account id not found: {}", + request.getAccountId(), Cause.ACCOUNT_NOT_FOUND); } else { responseObserver.onNext(account.get()); responseObserver.onCompleted(); @@ -359,9 +353,8 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas Cause cause = validator.validateCreatePendingAccountRequest(request); if (cause != null) { - responseObserver.onNext(CreatePendingAccountResponse.newBuilder() - .setError(ErrorResponse.newBuilder().setCause(cause)).build()); - responseObserver.onCompleted(); + logAndBuildGrpcPendingAccountResponse(responseObserver, CreatePendingAccountResponse.newBuilder(), + "Validation failed", "", cause); return; } @@ -392,6 +385,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas PendingAccount updatePendingAccount = pendingAccountRepository.save(updatedPendingAccount); CreatePendingAccountResponse response = CreatePendingAccountResponse.newBuilder() .setPendingAccountDetails(updatePendingAccount.toProto()).build(); + logger.info("Pending account created."); responseObserver.onNext(response); responseObserver.onCompleted(); return; @@ -400,9 +394,8 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas if (accountRepositoryAdditional.authenticationProviderAlreadyUsedInAccount( AuthenticationProvider.createAuthenticationProviderFromStrings(request.getAuthenticationType().name(), request.getAuthenticationProvider()))) { - responseObserver.onNext(CreatePendingAccountResponse.newBuilder() - .setError(ErrorResponse.newBuilder().setCause(Cause.ACCOUNT_ALREADY_CREATED)).build()); - responseObserver.onCompleted(); + logAndBuildGrpcPendingAccountResponse(responseObserver, CreatePendingAccountResponse.newBuilder(), + "Account already created", "", Cause.ACCOUNT_ALREADY_CREATED); return; } @@ -434,17 +427,15 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas Cause cause = validator.validateCompletePendingAccountCreationRequest(request); if (cause != null) { - responseObserver - .onNext(AccountResponse.newBuilder().setError(ErrorResponse.newBuilder().setCause(cause)).build()); - responseObserver.onCompleted(); + logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), "Validation failed", "", + cause); return; } if (request.getUsername() != null && !request.getUsername().trim().isEmpty() && accountRepositoryAdditional .foundExistingNotOwnUsername(UUID.fromString(request.getAccountId()), request.getUsername())) { - responseObserver.onNext(AccountResponse.newBuilder() - .setError(ErrorResponse.newBuilder().setCause(Cause.USERNAME_ALREADY_USED)).build()); - responseObserver.onCompleted(); + logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), + "User name already in use: {}", request.getUsername(), Cause.USERNAME_ALREADY_USED); return; } @@ -455,9 +446,8 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas Account createdAccount = accountRepositoryAdditional.completePendingAccountCreation(request); if (createdAccount == null) { - responseObserver.onNext(AccountResponse.newBuilder() - .setError(ErrorResponse.newBuilder().setCause(Cause.ERROR_CREATING_ACCOUNT)).build()); - responseObserver.onCompleted(); + logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), + "Error creating account with useraname: {}", request.getUsername(), Cause.ERROR_CREATING_ACCOUNT); return; } else { logger.debug("Account \"{}\" saved into the DB", createdAccount.toString()); @@ -477,26 +467,23 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas logger.debug("Updating profile...: {}", request); if ((request.getProfileId() == null) || (request.getProfileId().isEmpty())) { - responseObserver.onNext(ProfileResponse.newBuilder() - .setError(ErrorResponse.newBuilder().setCause(Cause.MISSING_PROFILE_ID)).build()); - responseObserver.onCompleted(); + logAndBuildGrpcProfileResponse(responseObserver, ProfileResponse.newBuilder(), "Missing profile id", "", + Cause.MISSING_PROFILE_ID); return; } Cause cause = validator.validateUpdateProfileRequest(request); if (cause != null) { - responseObserver.onNext(ProfileResponse.newBuilder() - .setError(ErrorResponse.newBuilder().setCause(cause)).build()); - responseObserver.onCompleted(); + logAndBuildGrpcProfileResponse(responseObserver, ProfileResponse.newBuilder(), "Validation failed", "", + cause); return; } Profile updatedProfile = accountRepositoryAdditional.updateProfile(request); if (updatedProfile == null) { - responseObserver.onNext(ProfileResponse.newBuilder() - .setError(ErrorResponse.newBuilder().setCause(Cause.ERROR_UPDATING_PROFILE)).build()); - responseObserver.onCompleted(); + logAndBuildGrpcProfileResponse(responseObserver, ProfileResponse.newBuilder(), "Error updating profile", "", + Cause.ERROR_UPDATING_PROFILE); return; } logger.debug("Profile \"{}\" updated in the DB", updatedProfile.toString()); @@ -514,33 +501,29 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas logger.debug("Updating account...: {}", request); if ((request.getAccountId() == null) || (request.getAccountId().isEmpty())) { - responseObserver.onNext(AccountResponse.newBuilder() - .setError(ErrorResponse.newBuilder().setCause(Cause.MISSING_ACCOUNT_ID)).build()); - responseObserver.onCompleted(); + logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), "Missing account id", "", + Cause.MISSING_ACCOUNT_ID); return; } Cause validationCause = validator.validateUpdateAccountRequest(request); if (validationCause != null) { - responseObserver.onNext(AccountResponse.newBuilder() - .setError(ErrorResponse.newBuilder().setCause(validationCause)).build()); - responseObserver.onCompleted(); + logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), "Validation failed", "", + validationCause); return; } if (request.getUsername() != null && !request.getUsername().trim().isEmpty() && accountRepositoryAdditional .foundExistingNotOwnUsername(UUID.fromString(request.getAccountId()), request.getUsername())) { - responseObserver.onNext(AccountResponse.newBuilder() - .setError(ErrorResponse.newBuilder().setCause(Cause.USERNAME_ALREADY_USED)).build()); - responseObserver.onCompleted(); + logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), + "User name already in use: {}", request.getUsername(), Cause.USERNAME_ALREADY_USED); return; } Account updatedAccount = accountRepositoryAdditional.updateAccount(request); if (updatedAccount == null) { - responseObserver.onNext(AccountResponse.newBuilder() - .setError(ErrorResponse.newBuilder().setCause(Cause.ERROR_UPDATING_ACCOUNT)).build()); - responseObserver.onCompleted(); + logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), "Error updating account.", + "", Cause.ERROR_UPDATING_ACCOUNT); return; } logger.debug("Account \"{}\" updated in the DB", updatedAccount.toString()); @@ -557,22 +540,21 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas logger.debug("Deleting account...: {}", request); if ((request.getAccountId() == null) || (request.getAccountId().isEmpty())) { - responseObserver.onNext(StatusResponse.newBuilder() - .setError(ErrorResponse.newBuilder().setCause(Cause.MISSING_ACCOUNT_ID)).build()); - responseObserver.onCompleted(); + logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Missing account id", "", + Cause.MISSING_ACCOUNT_ID); return; } boolean wasAccountDeleted = accountRepositoryAdditional.deleteAccount(UUID.fromString(request.getAccountId())); if (wasAccountDeleted) { + logger.info("Account successfully deleted: {}", request.getAccountId()); responseObserver.onNext(StatusResponse.newBuilder().setStatus("SUCCESS").build()); responseObserver.onCompleted(); return; } - responseObserver.onNext(StatusResponse.newBuilder() - .setError(ErrorResponse.newBuilder().setCause(Cause.ERROR_DELETING_ACCOUNT)).build()); - responseObserver.onCompleted(); + logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Error deleting account with id", + request.getAccountId(), Cause.ERROR_DELETING_ACCOUNT); return; } @@ -580,9 +562,8 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas public void deleteProfile(DeleteProfileRequest request, StreamObserver responseObserver) { logger.debug("Deleting profile: {}", request); if ((request.getProfileId() == null) || (request.getProfileId().isEmpty())) { - responseObserver.onNext(StatusResponse.newBuilder() - .setError(ErrorResponse.newBuilder().setCause(Cause.MISSING_PROFILE_ID)).build()); - responseObserver.onCompleted(); + logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Missing profile id", "", + Cause.MISSING_PROFILE_ID); return; } @@ -594,10 +575,8 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas return; } - logger.info("Error deleting profile."); - responseObserver.onNext(StatusResponse.newBuilder() - .setError(ErrorResponse.newBuilder().setCause(Cause.ERROR_DELETING_PROFILE)).build()); - responseObserver.onCompleted(); + logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Error deleting profile.", "", + Cause.ERROR_DELETING_PROFILE); return; } @@ -607,29 +586,25 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas logger.info("Adding authentication provider to profile requested."); logger.debug("Adding authentication provider to profile requested: {}", request); if ((request.getProfileId() == null) || (request.getProfileId().isEmpty())) { - responseObserver.onNext(StatusResponse.newBuilder() - .setError(ErrorResponse.newBuilder().setCause(Cause.MISSING_PROFILE_ID)).build()); - responseObserver.onCompleted(); + logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Missing profile id", "", + Cause.MISSING_PROFILE_ID); return; } if (request.getAuthenticationProvider().getAuthenticationTypeValue() == 0) { - responseObserver.onNext(StatusResponse.newBuilder() - .setError(ErrorResponse.newBuilder().setCause(Cause.MISSING_AUTH_PROVIDER_TYPE)).build()); - responseObserver.onCompleted(); + logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Missing auth provider type", + "", Cause.MISSING_AUTH_PROVIDER_TYPE); return; } if (request.getAuthenticationProvider().getAuthenticationProvider() == null || request.getAuthenticationProvider().getAuthenticationProvider().isEmpty()) { - responseObserver.onNext(StatusResponse.newBuilder() - .setError(ErrorResponse.newBuilder().setCause(Cause.MISSING_AUTH_PROVIDER_IDENTIFIER)).build()); - responseObserver.onCompleted(); + logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), + "Missing auth provider identifier", "", Cause.MISSING_AUTH_PROVIDER_IDENTIFIER); return; } Cause cause = validator.validateAddAuthenticationProviderRequest(request); if (cause != null) { - responseObserver - .onNext(StatusResponse.newBuilder().setError(ErrorResponse.newBuilder().setCause(cause)).build()); - responseObserver.onCompleted(); + logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Validation failed", "", + cause); return; } @@ -648,10 +623,8 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas // Make sure that the requested profile id for update exists in DB. Profile profile = profileRepository.findByProfileId(UUID.fromString(request.getProfileId())); if (profile == null) { - logger.error("Profile id {} missing in DB.", request.getProfileId()); - responseObserver.onNext(StatusResponse.newBuilder() - .setError(ErrorResponse.newBuilder().setCause(Cause.PROFILE_NOT_FOUND)).build()); - responseObserver.onCompleted(); + logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), + "Profile id {} misnot found in DB.", request.getProfileId(), Cause.PROFILE_NOT_FOUND); return; } // Make sure that the requested authentication provider is not already used in the system. @@ -660,12 +633,9 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas request.getAuthenticationProvider().getAuthenticationProvider(), request.getAuthenticationProvider().getAuthenticationType().name()); if (profileByAuthProvider != null) { - logger.error("Authentication provider {}:{} already used.", - request.getAuthenticationProvider().getAuthenticationType().name(), - request.getAuthenticationProvider().getAuthenticationProvider()); - responseObserver.onNext(StatusResponse.newBuilder() - .setError(ErrorResponse.newBuilder().setCause(Cause.AUTH_PROVIDER_ALREADY_USED)).build()); - responseObserver.onCompleted(); + logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), + "Authentication provider: {} already used.", request.getAuthenticationProvider().toString(), + Cause.AUTH_PROVIDER_ALREADY_USED); return; } boolean result = accountRepositoryAdditional.addAuthenticationProvider(UUID.fromString(request.getProfileId()), @@ -678,12 +648,9 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas responseObserver.onCompleted(); return; } - logger.error("Authentication provider {}:{} was not successfuly added for profile id {}.", - request.getAuthenticationProvider().getAuthenticationType().name(), - request.getAuthenticationProvider().getAuthenticationProvider(), request.getProfileId()); - responseObserver.onNext(StatusResponse.newBuilder() - .setError(ErrorResponse.newBuilder().setCause(Cause.INTERNAL_SERVER_ERROR)).build()); - responseObserver.onCompleted(); + logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), + "Authentication provider was not successfuly added for profile id {}.", request.getProfileId(), + Cause.INTERNAL_SERVER_ERROR); return; } @@ -695,8 +662,8 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas Optional> validationResult = validator .validateContactInfoRequest(request.getAccountId(), request.getContactInfo()); if (validationResult.isPresent()) { - prepareErrorStatusResponse(responseObserver, validationResult.get().getKey(), - validationResult.get().getValue()); + logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), + validationResult.get().getValue(), "", validationResult.get().getKey()); return; } if (request.getContactInfo().getType() == ContactType.PHONE_CONTACT) { @@ -711,9 +678,8 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas responseObserver.onCompleted(); return; } - logger.error("Contact info {}:{} was not added to account {}.", request.getContactInfo().getType().name(), - request.getContactInfo().getValue(), request.getAccountId()); - prepareErrorStatusResponse(responseObserver, Cause.ERROR_ADDING_CONTACT_INFO, "Error adding contact info"); + logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), + "Contact info was not added to account {}.", request.getAccountId(), Cause.ERROR_ADDING_CONTACT_INFO); return; } @@ -725,8 +691,9 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas Optional> validationResult = validator .validateContactInfoRequest(request.getAccountId(), request.getContactInfo()); if (validationResult.isPresent()) { - prepareErrorStatusResponse(responseObserver, validationResult.get().getKey(), - validationResult.get().getValue()); + logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), + "Validation failed for key {}.", validationResult.get().getKey().toString(), + Cause.ERROR_REMOVING_CONTACT_INFO); return; } if (request.getContactInfo().getType() == ContactType.PHONE_CONTACT) { @@ -741,9 +708,9 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas responseObserver.onCompleted(); return; } - logger.error("Contact info {}:{} was not removed from account {}.", request.getContactInfo().getType().name(), - request.getContactInfo().getValue(), request.getAccountId()); - prepareErrorStatusResponse(responseObserver, Cause.ERROR_REMOVING_CONTACT_INFO, "Error removing contact info"); + logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), + "Contact info was not removed from account {}.", request.getAccountId(), + Cause.ERROR_REMOVING_CONTACT_INFO); return; } @@ -753,40 +720,34 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas logger.info("Deleting Authentication Provider from profile: {}", request); if ((request.getProfileId() == null) || (request.getProfileId().isEmpty())) { - responseObserver.onNext(StatusResponse.newBuilder() - .setError(ErrorResponse.newBuilder().setCause(Cause.MISSING_PROFILE_ID)).build()); - responseObserver.onCompleted(); + logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Missing profile id.", "", + Cause.MISSING_PROFILE_ID); return; } if (request.getAuthenticationProvider().getAuthenticationTypeValue() == 0) { - responseObserver.onNext(StatusResponse.newBuilder() - .setError(ErrorResponse.newBuilder().setCause(Cause.MISSING_AUTH_PROVIDER_TYPE)).build()); - responseObserver.onCompleted(); + logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Missing auth provider type.", + "", Cause.MISSING_AUTH_PROVIDER_TYPE); return; } if (request.getAuthenticationProvider().getAuthenticationProvider() == null || request.getAuthenticationProvider().getAuthenticationProvider().isEmpty()) { - responseObserver.onNext(StatusResponse.newBuilder() - .setError(ErrorResponse.newBuilder().setCause(Cause.MISSING_AUTH_PROVIDER_IDENTIFIER)).build()); - responseObserver.onCompleted(); + logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Missing auth provider id.", + "", Cause.MISSING_AUTH_PROVIDER_IDENTIFIER); return; } Cause cause = validator.validateDeleteAuthenticationProviderRequest(request); if (cause != null) { - responseObserver - .onNext(StatusResponse.newBuilder().setError(ErrorResponse.newBuilder().setCause(cause)).build()); - responseObserver.onCompleted(); + logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Validation failed", "", + cause); return; } // Make sure that the requested profile id for update exists in DB. Profile profile = profileRepository.findByProfileId(UUID.fromString(request.getProfileId())); if (profile == null) { - logger.error("Profile id {} missing in DB.", request.getProfileId()); - responseObserver.onNext(StatusResponse.newBuilder() - .setError(ErrorResponse.newBuilder().setCause(Cause.PROFILE_NOT_FOUND)).build()); - responseObserver.onCompleted(); + logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), "Profile id {} missing in DB.", + request.getProfileId(), Cause.PROFILE_NOT_FOUND); return; } @@ -833,17 +794,16 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas .validateEditContactInfoRequest(request.getAccountId(), request.getOldContactInfo(), request.getEditedContactInfo()); if (validationResultEditContactInfo.isPresent()) { - prepareErrorStatusResponse(responseObserver, validationResultEditContactInfo.get().getKey(), - validationResultEditContactInfo.get().getValue()); + logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), + validationResultEditContactInfo.get().getValue(), "", + validationResultEditContactInfo.get().getKey()); return; } if (!request.getOldContactInfo().getType().equals(request.getEditedContactInfo().getType())) { - logger.error("Error editing Contact info for account {}. Different types: {} and {}.", - request.getAccountId(), request.getOldContactInfo().getType().name(), - request.getEditedContactInfo().getType().name()); - prepareErrorStatusResponse(responseObserver, Cause.ERROR_EDITING_CONTACT_INFO, - "Error editing contact info"); + logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), + "Error editing Contact info for account {}. Different types: {} and {}.", request.getAccountId(), + Cause.ERROR_EDITING_CONTACT_INFO); return; } @@ -862,26 +822,12 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas responseObserver.onCompleted(); return; } - logger.error("Contact info {}:{} was not edited for account {}.", request.getOldContactInfo().getType().name(), - request.getOldContactInfo().getValue(), request.getAccountId()); - prepareErrorStatusResponse(responseObserver, Cause.ERROR_EDITING_CONTACT_INFO, "Error editing contact info"); + logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), + "Contact info was not edited for account {}.", request.getAccountId(), + Cause.ERROR_EDITING_CONTACT_INFO); return; } - private static void logAndBuildGrpcResponse(StreamObserver responseObserver, Validation validation) { - logger.debug(validation.getErrorMessage()); - responseObserver - .onNext(SearchResponse.newBuilder().setError(ErrorResponse.newBuilder().setCause(validation.getCause().get())).build()); - responseObserver.onCompleted(); - } - - private static void logAndBuildGrpcResponse(StreamObserver responseObserver, ValidationError error) { - logger.debug(error.getMessage()); - responseObserver - .onNext(SearchResponse.newBuilder().setError(ErrorResponse.newBuilder().setCause(error.getCause())).build()); - responseObserver.onCompleted(); - } - private SearchResultDetails buildSearchResultDetails(String id, ByteBuffer avatar, String firstName, String lastName) { SearchResultDetails.Builder searchResultDetails = SearchResultDetails.newBuilder(); @@ -892,23 +838,48 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas return searchResultDetails.build(); } + + private static void logAndBuildGrpcSearchResponse(StreamObserver responseObserver, + SearchResponse.Builder newBuilder, String logMessage, String logValue, Cause cause) { - private void sendErrorMessageForAccountsResponse(StreamObserver responseObserver, Cause cause) { - responseObserver.onNext(AccountsResponse.newBuilder() - .setError(ErrorResponse.newBuilder().setCause(cause)).build()); + logger.debug(logMessage, logValue); + responseObserver.onNext(newBuilder.setError(ErrorResponse.newBuilder().setCause(cause)).build()); responseObserver.onCompleted(); } - private void sendErrorMessageForAccountResponse(StreamObserver responseObserver, Cause cause) { - responseObserver.onNext(AccountResponse.newBuilder() - .setError(ErrorResponse.newBuilder().setCause(cause)).build()); + private static void logAndBuildGrpcAccountResponse(StreamObserver responseObserver, + AccountResponse.Builder newBuilder, String logMessage, String logValue, Cause cause) { + + logger.debug(logMessage, logValue); + responseObserver.onNext(newBuilder.setError(ErrorResponse.newBuilder().setCause(cause)).build()); responseObserver.onCompleted(); } - public void prepareErrorStatusResponse(StreamObserver responseObserver, Cause error, - String message) { - responseObserver.onNext(StatusResponse.newBuilder() - .setError(ErrorResponse.newBuilder().setCause(error).setMessage(message)).build()); + private static void logAndBuildGrpcAccountsResponse(StreamObserver responseObserver, + AccountsResponse.Builder newBuilder, String logMessage, String logValue, Cause cause) { + logger.debug(logMessage, logValue); + responseObserver.onNext(newBuilder.setError(ErrorResponse.newBuilder().setCause(cause)).build()); + responseObserver.onCompleted(); + } + + private static void logAndBuildGrpcPendingAccountResponse(StreamObserver responseObserver, + CreatePendingAccountResponse.Builder newBuilder, String logMessage, String logValue, Cause cause) { + logger.debug(logMessage, logValue); + responseObserver.onNext(newBuilder.setError(ErrorResponse.newBuilder().setCause(cause)).build()); + responseObserver.onCompleted(); + } + + private static void logAndBuildGrpcProfileResponse(StreamObserver responseObserver, + ProfileResponse.Builder newBuilder, String logMessage, String logValue, Cause cause) { + logger.debug(logMessage, logValue); + responseObserver.onNext(newBuilder.setError(ErrorResponse.newBuilder().setCause(cause)).build()); + responseObserver.onCompleted(); + } + + private static void logAndBuildGrpcStatusResponse(StreamObserver responseObserver, + StatusResponse.Builder newBuilder, String logMessage, String logValue, Cause cause) { + logger.debug(logMessage, logValue); + responseObserver.onNext(newBuilder.setError(ErrorResponse.newBuilder().setCause(cause)).build()); responseObserver.onCompleted(); } -- GitLab From 6d843789a1fb5592aecc930bb239d58c5257f508 Mon Sep 17 00:00:00 2001 From: Stoyan Tzenkov Date: Wed, 31 Oct 2018 10:26:14 +0200 Subject: [PATCH 3/6] NY-4396 fixed --- on error log at ERROR level and not DEBUG Signed-off-by: Stoyan Tzenkov --- .../biz/nynja/account/services/AccountServiceImpl.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java index b815a52..c7b8206 100644 --- a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java +++ b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java @@ -850,35 +850,35 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas private static void logAndBuildGrpcAccountResponse(StreamObserver responseObserver, AccountResponse.Builder newBuilder, String logMessage, String logValue, Cause cause) { - logger.debug(logMessage, logValue); + logger.error(logMessage, logValue); responseObserver.onNext(newBuilder.setError(ErrorResponse.newBuilder().setCause(cause)).build()); responseObserver.onCompleted(); } private static void logAndBuildGrpcAccountsResponse(StreamObserver responseObserver, AccountsResponse.Builder newBuilder, String logMessage, String logValue, Cause cause) { - logger.debug(logMessage, logValue); + logger.error(logMessage, logValue); responseObserver.onNext(newBuilder.setError(ErrorResponse.newBuilder().setCause(cause)).build()); responseObserver.onCompleted(); } private static void logAndBuildGrpcPendingAccountResponse(StreamObserver responseObserver, CreatePendingAccountResponse.Builder newBuilder, String logMessage, String logValue, Cause cause) { - logger.debug(logMessage, logValue); + logger.error(logMessage, logValue); responseObserver.onNext(newBuilder.setError(ErrorResponse.newBuilder().setCause(cause)).build()); responseObserver.onCompleted(); } private static void logAndBuildGrpcProfileResponse(StreamObserver responseObserver, ProfileResponse.Builder newBuilder, String logMessage, String logValue, Cause cause) { - logger.debug(logMessage, logValue); + logger.error(logMessage, logValue); responseObserver.onNext(newBuilder.setError(ErrorResponse.newBuilder().setCause(cause)).build()); responseObserver.onCompleted(); } private static void logAndBuildGrpcStatusResponse(StreamObserver responseObserver, StatusResponse.Builder newBuilder, String logMessage, String logValue, Cause cause) { - logger.debug(logMessage, logValue); + logger.error(logMessage, logValue); responseObserver.onNext(newBuilder.setError(ErrorResponse.newBuilder().setCause(cause)).build()); responseObserver.onCompleted(); } -- GitLab From f785de6026d76d0c52478261e93caaef00b6b661 Mon Sep 17 00:00:00 2001 From: Stoyan Tzenkov Date: Thu, 8 Nov 2018 14:39:12 +0200 Subject: [PATCH 4/6] validation() accounted for in logAndBuild...(), a bug fixed Signed-off-by: Stoyan Tzenkov --- .../account/services/AccountServiceImpl.java | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java index c7b8206..f2edad9 100644 --- a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java +++ b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java @@ -184,7 +184,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas logger.info("Getting account by username: {}", request.getUsername()); Validation validation = validateGetByUsernameRequest(request); if(validation.hasErrors()) { - logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), "Invalid user name : ", request.getUsername(), Cause.USERNAME_INVALID); + logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), validation.getErrorMessage(), "", validation.getCause().get()); return; } @@ -205,7 +205,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas logger.info("Searching account by username: {}", request.getUsername()); Validation validation = validateGetByUsernameRequest(request); if(validation.hasErrors()) { - logAndBuildGrpcSearchResponse(responseObserver, SearchResponse.newBuilder(), "Invalid user name.", "", Cause.USERNAME_INVALID); + logAndBuildGrpcSearchResponse(responseObserver, SearchResponse.newBuilder(), validation.getErrorMessage(), "", validation.getCause().get()); return; } @@ -353,8 +353,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas Cause cause = validator.validateCreatePendingAccountRequest(request); if (cause != null) { - logAndBuildGrpcPendingAccountResponse(responseObserver, CreatePendingAccountResponse.newBuilder(), - "Validation failed", "", cause); + logAndBuildGrpcPendingAccountResponse(responseObserver, CreatePendingAccountResponse.newBuilder(), "Validation failed", "", cause); return; } @@ -505,10 +504,10 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas Cause.MISSING_ACCOUNT_ID); return; } - Cause validationCause = validator.validateUpdateAccountRequest(request); - if (validationCause != null) { + Cause cause = validator.validateUpdateAccountRequest(request); + if (cause != null) { logAndBuildGrpcAccountResponse(responseObserver, AccountResponse.newBuilder(), "Validation failed", "", - validationCause); + cause); return; } @@ -692,8 +691,8 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas .validateContactInfoRequest(request.getAccountId(), request.getContactInfo()); if (validationResult.isPresent()) { logAndBuildGrpcStatusResponse(responseObserver, StatusResponse.newBuilder(), - "Validation failed for key {}.", validationResult.get().getKey().toString(), - Cause.ERROR_REMOVING_CONTACT_INFO); + "Validation failed. {}.", validationResult.get().getRight(), + validationResult.get().getLeft()); return; } if (request.getContactInfo().getType() == ContactType.PHONE_CONTACT) { -- GitLab From 16221817d026b92df2ce1f64b964f455d85f1be6 Mon Sep 17 00:00:00 2001 From: Stoyan Tzenkov Date: Thu, 8 Nov 2018 15:11:51 +0200 Subject: [PATCH 5/6] All phone validation moved to PhoneNumberValidator Signed-off-by: Stoyan Tzenkov --- .../nynja/account/components/Validator.java | 95 ------------------- .../account/phone/PhoneNumberValidator.java | 6 +- 2 files changed, 3 insertions(+), 98 deletions(-) diff --git a/src/main/java/biz/nynja/account/components/Validator.java b/src/main/java/biz/nynja/account/components/Validator.java index e460550..4dedb94 100644 --- a/src/main/java/biz/nynja/account/components/Validator.java +++ b/src/main/java/biz/nynja/account/components/Validator.java @@ -71,101 +71,6 @@ public class Validator { } - private HashMap countryInfoMap; - - @PostConstruct - public void loadPhonesBook() { - - CountryInfo countryInfo = null; - BufferedReader reader = null; - countryInfoMap = new HashMap<>(); - - logger.debug("Loading phones information from file."); - try { - Resource resource = new ClassPathResource("countries.txt"); - InputStream resourceInputStream = resource.getInputStream(); - logger.debug("Phones information loaded."); - reader = new BufferedReader(new InputStreamReader(resourceInputStream)); - String line; - while ((line = reader.readLine()) != null) { - String[] args = line.split(";"); - countryInfo = new CountryInfo(); - countryInfo.setCountryCode(args[1]); - countryInfo.setCountryPhoneCode(args[0]); - countryInfo.setCountryName(args[2]); - if (args.length > 3) { - countryInfo.setPhoneFormat(args[3]); - } - countryInfoMap.put(args[1], countryInfo); - } - } catch (IOException e) { - logger.error("Error during load phones information: {}", e.getMessage()); - } finally { - try { - reader.close(); - } catch (IOException e) { - logger.error("Close reader error: {}", e.getMessage()); - } - } - - } - - public boolean isPhoneNumberValid(String phoneNumber, String countryCode) { - - logger.debug("Checking phoneNumber: {} for country: {}", phoneNumber, countryCode); - CountryInfo countryInfo = countryInfoMap.get(countryCode); - if (countryInfo == null) { - logger.debug("Country: {} not found!", countryCode); - return false; - } - - char[] digits = countryInfo.getCountryPhoneCode().toCharArray(); - StringBuilder builder = new StringBuilder(); - for (char digit : digits) { - builder.append("[" + digit + "]"); - } - String codePattern = builder.toString(); - - String phoneLength = null; - String phoneFormat = "XXXXXXXXXXXXXXX"; - if (countryInfo.getPhoneFormat() != null) { - phoneFormat = countryInfo.getPhoneFormat(); - } - phoneLength = "{" + phoneFormat.replaceAll("\\s", "").length() + "}"; - - final String PHONE_PATTERN = "((?:\\+?([0][0])?" + codePattern + ")?||([0][0]" + codePattern + ")?)(\\d" - + phoneLength + ")$"; - logger.debug("Generated phone pattern for country: {}, {}", countryCode, PHONE_PATTERN); - - Pattern pattern = Pattern.compile(PHONE_PATTERN); - Matcher matcher = pattern.matcher(phoneNumber); - - boolean isValid = matcher.matches(); - logger.debug("PhoneNumber: {} for country: {} is valid: {}", phoneNumber, countryCode, isValid); - - return isValid; - } - - public String getNormalizedPhoneNumber(String authenticationProvider) { - String[] provider = authenticationProvider.split(":"); - String country = provider[0]; - String phoneNumber = provider[1]; - logger.info("libphone: New phone number normalization request received - phone number: {}, country code: {}", - phoneNumber, country); - - PhoneNumberUtil phoneUtil = PhoneNumberUtil.getInstance(); - String normalizedPhoneNumber = ""; - try { - PhoneNumber pn = phoneUtil.parse(phoneNumber, country); - 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; - } - public boolean isUsernameValid(String username) { logger.debug("Checking username: {}", username); diff --git a/src/main/java/biz/nynja/account/phone/PhoneNumberValidator.java b/src/main/java/biz/nynja/account/phone/PhoneNumberValidator.java index 125928a..c2a0c40 100644 --- a/src/main/java/biz/nynja/account/phone/PhoneNumberValidator.java +++ b/src/main/java/biz/nynja/account/phone/PhoneNumberValidator.java @@ -91,11 +91,11 @@ public class PhoneNumberValidator { String codePattern = builder.toString(); String phoneLength = null; + String phoneFormat = "XXXXXXXXXXXXXXX"; if (countryInfo.getPhoneFormat() != null) { - phoneLength = "{" + countryInfo.getPhoneFormat().replaceAll("\\s", "").length() + "}"; - } else { - phoneLength = "+"; + phoneFormat = countryInfo.getPhoneFormat(); } + phoneLength = "{" + phoneFormat.replaceAll("\\s", "").length() + "}"; final String PHONE_PATTERN = "((?:\\+?([0][0])?" + codePattern + ")?||([0][0]" + codePattern + ")?)(\\d" + phoneLength + ")$"; -- GitLab From c037455702701d9eca58b1fd5cf589b09e6577b1 Mon Sep 17 00:00:00 2001 From: Stoyan Tzenkov Date: Thu, 8 Nov 2018 16:39:34 +0200 Subject: [PATCH 6/6] phone number lenght limited to 15 - fixed Signed-off-by: Stoyan Tzenkov --- .../biz/nynja/account/phone/PhoneNumberValidator.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/main/java/biz/nynja/account/phone/PhoneNumberValidator.java b/src/main/java/biz/nynja/account/phone/PhoneNumberValidator.java index c2a0c40..61b0776 100644 --- a/src/main/java/biz/nynja/account/phone/PhoneNumberValidator.java +++ b/src/main/java/biz/nynja/account/phone/PhoneNumberValidator.java @@ -76,6 +76,8 @@ public class PhoneNumberValidator { public boolean isPhoneNumberValid(String phoneNumber, String countryCode) { + final int NATIONAL_NUMBER_MAX_LENGTH = 15; + logger.debug("Checking phoneNumber: {} for country: {}", phoneNumber, countryCode); CountryInfo countryInfo = countryInfoMap.get(countryCode); if (countryInfo == null) { @@ -91,11 +93,11 @@ public class PhoneNumberValidator { String codePattern = builder.toString(); String phoneLength = null; - String phoneFormat = "XXXXXXXXXXXXXXX"; if (countryInfo.getPhoneFormat() != null) { - phoneFormat = countryInfo.getPhoneFormat(); + phoneLength = "{" + countryInfo.getPhoneFormat().replaceAll("\\s", "").length() + "}"; + } else { + phoneLength = "{0," + NATIONAL_NUMBER_MAX_LENGTH + "}"; } - phoneLength = "{" + phoneFormat.replaceAll("\\s", "").length() + "}"; final String PHONE_PATTERN = "((?:\\+?([0][0])?" + codePattern + ")?||([0][0]" + codePattern + ")?)(\\d" + phoneLength + ")$"; @@ -110,8 +112,6 @@ public class PhoneNumberValidator { return isValid; } - - /** * * @param rawPhoneNumber -- GitLab