From 6fc82a4359b93d65d13675db68ffdc1448bd9c48 Mon Sep 17 00:00:00 2001 From: Stanimir Penkov Date: Mon, 15 Oct 2018 16:27:09 +0300 Subject: [PATCH 1/9] NY-3808: Endpoint for adding contact info to account - implemented endpoint for adding contact info to account; - changed access modifiers in AccountRepositoryAdditionalImpl.java Signed-off-by: Stanimir Penkov --- src/main/java/biz/nynja/account/components/Validator.java | 1 + .../java/biz/nynja/account/services/AccountServiceImpl.java | 3 --- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/main/java/biz/nynja/account/components/Validator.java b/src/main/java/biz/nynja/account/components/Validator.java index 2da72ad..93d801d 100644 --- a/src/main/java/biz/nynja/account/components/Validator.java +++ b/src/main/java/biz/nynja/account/components/Validator.java @@ -346,6 +346,7 @@ public class Validator { } if (request.getContactInfo().getTypeValue() == 0) { return Cause.MISSING_CONTACT_INFO_TYPE; + } if (request.getContactInfo().getValue() == null || request.getContactInfo().getValue().isEmpty()) { return Cause.MISSING_CONTACT_INFO_IDENTIFIER; diff --git a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java index fe0bea3..04b6872 100644 --- a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java +++ b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java @@ -560,7 +560,6 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas StreamObserver responseObserver) { logger.info("Adding contact info to account requested."); logger.debug("Adding contact info to account requested: {}", request); - Cause cause = validator.checkAddContactInfoRequest(request); if (cause != null) { responseObserver @@ -568,11 +567,9 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas responseObserver.onCompleted(); return; } - if (request.getContactInfo().getType() == ContactType.PHONE_CONTACT) { request = normalizePhoneNumber(request); } - boolean result = accountRepositoryAdditional.addContactInfo(UUID.fromString(request.getAccountId()), ContactInfo.createContactInfoFromProto(request.getContactInfo())); if (result) { -- GitLab From 03c0520d6d9c111b56b6a6a1bdb22b70712e0cd8 Mon Sep 17 00:00:00 2001 From: Stanimir Penkov Date: Tue, 16 Oct 2018 14:00:18 +0300 Subject: [PATCH 2/9] NY-3809: Endpoint for removing contact info from account Signed-off-by: Stanimir Penkov --- .../nynja/account/components/Validator.java | 17 ++++++++ .../AccountRepositoryAdditional.java | 2 + .../AccountRepositoryAdditionalImpl.java | 27 +++++++++++++ .../account/services/AccountServiceImpl.java | 39 +++++++++++++++++++ 4 files changed, 85 insertions(+) diff --git a/src/main/java/biz/nynja/account/components/Validator.java b/src/main/java/biz/nynja/account/components/Validator.java index 93d801d..b0737cb 100644 --- a/src/main/java/biz/nynja/account/components/Validator.java +++ b/src/main/java/biz/nynja/account/components/Validator.java @@ -29,6 +29,7 @@ import biz.nynja.account.grpc.CompletePendingAccountCreationRequest; import biz.nynja.account.grpc.ContactType; import biz.nynja.account.grpc.CreatePendingAccountRequest; import biz.nynja.account.grpc.DeleteAuthenticationProviderRequest; +import biz.nynja.account.grpc.DeleteContactInfoRequest; import biz.nynja.account.grpc.ErrorResponse.Cause; import biz.nynja.account.grpc.UpdateAccountRequest; import biz.nynja.account.grpc.UpdateProfileRequest; @@ -357,6 +358,22 @@ public class Validator { return validateContactInfo(request.getContactInfo().getType(), request.getContactInfo().getValue()); } + public Cause checkDeleteContactInfoRequest(DeleteContactInfoRequest request) { + if ((request.getAccountId() == null) || (request.getAccountId().isEmpty())) { + return Cause.MISSING_ACCOUNT_ID; + } + if (request.getContactInfo().getTypeValue() == 0) { + return Cause.MISSING_CONTACT_INFO_TYPE; + } + if (request.getContactInfo().getValue() == null || request.getContactInfo().getValue().isEmpty()) { + return Cause.MISSING_CONTACT_INFO_IDENTIFIER; + } + if (!isValidUuid(request.getAccountId())) { + return Cause.INVALID_ACCOUNT_ID; + } + return validateContactInfo(request.getContactInfo().getType(), request.getContactInfo().getValue()); + } + public Cause validateDeleteAuthenticationProviderRequest(DeleteAuthenticationProviderRequest request) { if (!isValidUuid(request.getProfileId())) { return Cause.INVALID_PROFILE_ID; diff --git a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditional.java b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditional.java index 382adca..876f037 100644 --- a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditional.java +++ b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditional.java @@ -40,4 +40,6 @@ public interface AccountRepositoryAdditional { boolean deleteAuthenticationProvider(Profile profile, AuthenticationProvider authProvider); boolean addContactInfo(UUID accountId, ContactInfo contactInfo); + + boolean deleteContactInfo(UUID accountId, ContactInfo contactInfo); } diff --git a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java index 8aa3bfc..1162356 100644 --- a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java +++ b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java @@ -544,6 +544,33 @@ public class AccountRepositoryAdditionalImpl implements AccountRepositoryAdditio return false; } + @Override + public boolean deleteContactInfo(UUID accountId, ContactInfo contactInfo) { + Account accountToUpdate = accountRepository.findByAccountId(accountId); + if (accountToUpdate == null) { + logger.error("Existing account with the provided id {} was not found.", accountId); + return false; + } + Set contactsInfoSet = accountToUpdate.getContactsInfo(); + if (contactsInfoSet == null) { + logger.error("No existing contact info found for account {}.", accountId); + return false; + } + if (!contactsInfoSet.remove(contactInfo)) { + logger.error("Error removing contact info from account {}. Existing contact info: {} was not found.", accountId, + contactInfo.toString()); + return false; + } + accountToUpdate.setContactsInfo(contactsInfoSet); + Long timeUpdated = new Date().getTime(); + accountToUpdate.setLastUpdateTimestamp(timeUpdated); + WriteResult wr = cassandraTemplate.update(accountToUpdate, UpdateOptions.builder().ifExists(true).build()); + if (wr != null && wr.wasApplied()) { + return true; + } + return false; + } + @Override public boolean deleteAuthenticationProvider(Profile profile, AuthenticationProvider authProvider) { diff --git a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java index 04b6872..614aca2 100644 --- a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java +++ b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java @@ -35,6 +35,7 @@ import biz.nynja.account.grpc.CreatePendingAccountRequest; import biz.nynja.account.grpc.CreatePendingAccountResponse; import biz.nynja.account.grpc.DeleteAccountRequest; import biz.nynja.account.grpc.DeleteAuthenticationProviderRequest; +import biz.nynja.account.grpc.DeleteContactInfoRequest; import biz.nynja.account.grpc.StatusResponse; import biz.nynja.account.grpc.DeleteProfileRequest; import biz.nynja.account.grpc.ErrorResponse; @@ -598,6 +599,44 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas return request; } + @Override + public void deleteContactInfoFromAccount(DeleteContactInfoRequest request, + StreamObserver responseObserver) { + logger.info("Removing contact info from account requested."); + logger.debug("Removing contact info from account requested: {}", request); + Cause cause = validator.checkDeleteContactInfoRequest(request); + if (cause != null) { + responseObserver + .onNext(StatusResponse.newBuilder().setError(ErrorResponse.newBuilder().setCause(cause)).build()); + responseObserver.onCompleted(); + return; + } + if (request.getContactInfo().getType() == ContactType.PHONE_CONTACT) { + // Get the normalized phone number from libphone + ContactDetails newContactDetails = ContactDetails.newBuilder().setType(request.getContactInfo().getType()) + .setValue(validator.getNormalizedPhoneNumber(request.getContactInfo().getValue())) + .setLabel(request.getContactInfo().getLabel()).build(); + DeleteContactInfoRequest newRequest = DeleteContactInfoRequest.newBuilder() + .setAccountId(request.getAccountId()).setContactInfo(newContactDetails).build(); + request = newRequest; + } + boolean result = accountRepositoryAdditional.deleteContactInfo(UUID.fromString(request.getAccountId()), + ContactInfo.createContactInfoFromProto(request.getContactInfo())); + if (result) { + logger.info("Contact info {}:{} was removed from account {}.", request.getContactInfo().getType().name(), + request.getContactInfo().getValue(), request.getAccountId()); + responseObserver.onNext(StatusResponse.newBuilder().setStatus("SUCCESS").build()); + responseObserver.onCompleted(); + return; + } + logger.error("Contact info {}:{} was not removed from account {}.", request.getContactInfo().getType().name(), + request.getContactInfo().getValue(), request.getAccountId()); + responseObserver.onNext(StatusResponse.newBuilder() + .setError(ErrorResponse.newBuilder().setCause(Cause.INTERNAL_SERVER_ERROR)).build()); + responseObserver.onCompleted(); + return; + } + @Override public void deleteAuthenticationProviderFromProfile(DeleteAuthenticationProviderRequest request, StreamObserver responseObserver) { -- GitLab From c8497aae633444c4c6e4cf15960c68422505fdf0 Mon Sep 17 00:00:00 2001 From: Stanimir Penkov Date: Tue, 16 Oct 2018 14:09:46 +0300 Subject: [PATCH 3/9] NY-3809: Apply code alignment Signed-off-by: Stanimir Penkov --- .../account/repositories/AccountRepositoryAdditionalImpl.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java index 1162356..f36e87f 100644 --- a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java +++ b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java @@ -557,8 +557,8 @@ public class AccountRepositoryAdditionalImpl implements AccountRepositoryAdditio return false; } if (!contactsInfoSet.remove(contactInfo)) { - logger.error("Error removing contact info from account {}. Existing contact info: {} was not found.", accountId, - contactInfo.toString()); + logger.error("Error removing contact info from account {}. Existing contact info: {} was not found.", + accountId, contactInfo.toString()); return false; } accountToUpdate.setContactsInfo(contactsInfoSet); -- GitLab From 3f187b62739f547a755828c2213d4190551a7226 Mon Sep 17 00:00:00 2001 From: Stanimir Penkov Date: Tue, 16 Oct 2018 16:03:20 +0300 Subject: [PATCH 4/9] NY-3809: Use the cause for error removing contact info Signed-off-by: Stanimir Penkov --- .../java/biz/nynja/account/services/AccountServiceImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java index 614aca2..40feb45 100644 --- a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java +++ b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java @@ -632,7 +632,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas logger.error("Contact info {}:{} was not removed from account {}.", request.getContactInfo().getType().name(), request.getContactInfo().getValue(), request.getAccountId()); responseObserver.onNext(StatusResponse.newBuilder() - .setError(ErrorResponse.newBuilder().setCause(Cause.INTERNAL_SERVER_ERROR)).build()); + .setError(ErrorResponse.newBuilder().setCause(Cause.ERROR_REMOVING_CONTACT_INFO)).build()); responseObserver.onCompleted(); return; } -- GitLab From e896e015d8e3e46a0fe33404cfffc415aca67f34 Mon Sep 17 00:00:00 2001 From: Stanimir Penkov Date: Fri, 19 Oct 2018 10:08:31 +0300 Subject: [PATCH 5/9] NY-3809: Code improvements - added a single method for validation of contact info request; - updated setting of time when removing contact info; - extracted additional method. Signed-off-by: Stanimir Penkov --- .../nynja/account/components/Validator.java | 29 +++++-------------- .../biz/nynja/account/models/ContactInfo.java | 1 - .../AccountRepositoryAdditionalImpl.java | 2 +- .../account/services/AccountServiceImpl.java | 23 +++++++++------ 4 files changed, 22 insertions(+), 33 deletions(-) diff --git a/src/main/java/biz/nynja/account/components/Validator.java b/src/main/java/biz/nynja/account/components/Validator.java index b0737cb..d30af4f 100644 --- a/src/main/java/biz/nynja/account/components/Validator.java +++ b/src/main/java/biz/nynja/account/components/Validator.java @@ -26,6 +26,7 @@ import com.google.i18n.phonenumbers.Phonenumber.PhoneNumber; import biz.nynja.account.grpc.AuthProviderDetails; import biz.nynja.account.grpc.AuthenticationType; import biz.nynja.account.grpc.CompletePendingAccountCreationRequest; +import biz.nynja.account.grpc.ContactDetails; import biz.nynja.account.grpc.ContactType; import biz.nynja.account.grpc.CreatePendingAccountRequest; import biz.nynja.account.grpc.DeleteAuthenticationProviderRequest; @@ -341,37 +342,21 @@ public class Validator { request.getAuthenticationProvider().getAuthenticationProvider()); } - public Cause checkAddContactInfoRequest(AddContactInfoRequest request) { - if ((request.getAccountId() == null) || (request.getAccountId().isEmpty())) { + public Cause validateContactInfoRequest(String accountId, ContactDetails contactDetails) { + if ((accountId == null) || (accountId.isEmpty())) { return Cause.MISSING_ACCOUNT_ID; } - if (request.getContactInfo().getTypeValue() == 0) { + if (contactDetails.getTypeValue() == 0) { return Cause.MISSING_CONTACT_INFO_TYPE; } - if (request.getContactInfo().getValue() == null || request.getContactInfo().getValue().isEmpty()) { + if (contactDetails.getValue() == null || contactDetails.getValue().isEmpty()) { return Cause.MISSING_CONTACT_INFO_IDENTIFIER; } - if (!isValidUuid(request.getAccountId())) { + if (!isValidUuid(accountId)) { return Cause.INVALID_ACCOUNT_ID; } - return validateContactInfo(request.getContactInfo().getType(), request.getContactInfo().getValue()); - } - - public Cause checkDeleteContactInfoRequest(DeleteContactInfoRequest request) { - if ((request.getAccountId() == null) || (request.getAccountId().isEmpty())) { - return Cause.MISSING_ACCOUNT_ID; - } - if (request.getContactInfo().getTypeValue() == 0) { - return Cause.MISSING_CONTACT_INFO_TYPE; - } - if (request.getContactInfo().getValue() == null || request.getContactInfo().getValue().isEmpty()) { - return Cause.MISSING_CONTACT_INFO_IDENTIFIER; - } - if (!isValidUuid(request.getAccountId())) { - return Cause.INVALID_ACCOUNT_ID; - } - return validateContactInfo(request.getContactInfo().getType(), request.getContactInfo().getValue()); + return validateContactInfo(contactDetails.getType(), contactDetails.getValue()); } public Cause validateDeleteAuthenticationProviderRequest(DeleteAuthenticationProviderRequest request) { diff --git a/src/main/java/biz/nynja/account/models/ContactInfo.java b/src/main/java/biz/nynja/account/models/ContactInfo.java index 982681d..9fffa48 100644 --- a/src/main/java/biz/nynja/account/models/ContactInfo.java +++ b/src/main/java/biz/nynja/account/models/ContactInfo.java @@ -99,7 +99,6 @@ public class ContactInfo { if (contactType != null) { builder.setType(contactType); } - if (getValue() != null) { builder.setValue(getValue()); } diff --git a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java index f36e87f..d105163 100644 --- a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java +++ b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java @@ -562,7 +562,7 @@ public class AccountRepositoryAdditionalImpl implements AccountRepositoryAdditio return false; } accountToUpdate.setContactsInfo(contactsInfoSet); - Long timeUpdated = new Date().getTime(); + Long timeUpdated = Instant.now().toEpochMilli(); accountToUpdate.setLastUpdateTimestamp(timeUpdated); WriteResult wr = cassandraTemplate.update(accountToUpdate, UpdateOptions.builder().ifExists(true).build()); if (wr != null && wr.wasApplied()) { diff --git a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java index 40feb45..8f67e83 100644 --- a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java +++ b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java @@ -561,7 +561,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas StreamObserver responseObserver) { logger.info("Adding contact info to account requested."); logger.debug("Adding contact info to account requested: {}", request); - Cause cause = validator.checkAddContactInfoRequest(request); + Cause cause = validator.validateContactInfoRequest(request.getAccountId(), request.getContactInfo()); if (cause != null) { responseObserver .onNext(StatusResponse.newBuilder().setError(ErrorResponse.newBuilder().setCause(cause)).build()); @@ -604,7 +604,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas StreamObserver responseObserver) { logger.info("Removing contact info from account requested."); logger.debug("Removing contact info from account requested: {}", request); - Cause cause = validator.checkDeleteContactInfoRequest(request); + Cause cause = validator.validateContactInfoRequest(request.getAccountId(), request.getContactInfo()); if (cause != null) { responseObserver .onNext(StatusResponse.newBuilder().setError(ErrorResponse.newBuilder().setCause(cause)).build()); @@ -612,13 +612,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas return; } if (request.getContactInfo().getType() == ContactType.PHONE_CONTACT) { - // Get the normalized phone number from libphone - ContactDetails newContactDetails = ContactDetails.newBuilder().setType(request.getContactInfo().getType()) - .setValue(validator.getNormalizedPhoneNumber(request.getContactInfo().getValue())) - .setLabel(request.getContactInfo().getLabel()).build(); - DeleteContactInfoRequest newRequest = DeleteContactInfoRequest.newBuilder() - .setAccountId(request.getAccountId()).setContactInfo(newContactDetails).build(); - request = newRequest; + request = normalizePhoneNumber(request); } boolean result = accountRepositoryAdditional.deleteContactInfo(UUID.fromString(request.getAccountId()), ContactInfo.createContactInfoFromProto(request.getContactInfo())); @@ -637,6 +631,17 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas return; } + private DeleteContactInfoRequest normalizePhoneNumber(DeleteContactInfoRequest request) { + // Get the normalized phone number from libphone + ContactDetails newContactDetails = ContactDetails.newBuilder().setType(request.getContactInfo().getType()) + .setValue(validator.getNormalizedPhoneNumber(request.getContactInfo().getValue())) + .setLabel(request.getContactInfo().getLabel()).build(); + DeleteContactInfoRequest newRequest = DeleteContactInfoRequest.newBuilder() + .setAccountId(request.getAccountId()).setContactInfo(newContactDetails).build(); + request = newRequest; + return request; + } + @Override public void deleteAuthenticationProviderFromProfile(DeleteAuthenticationProviderRequest request, StreamObserver responseObserver) { -- GitLab From 64085f2e094edf1e731d93dcd9a491a1d43f89b2 Mon Sep 17 00:00:00 2001 From: Stanimir Penkov Date: Tue, 23 Oct 2018 15:16:28 +0300 Subject: [PATCH 6/9] NY-3809: Code improvements - changed validation function for contact info; - extracted additional method. Signed-off-by: Stanimir Penkov --- pom.xml | 6 +++ .../nynja/account/components/Validator.java | 26 +++++++------ .../account/services/AccountServiceImpl.java | 38 ++++++++++--------- 3 files changed, 41 insertions(+), 29 deletions(-) diff --git a/pom.xml b/pom.xml index 7e8627c..78278a1 100644 --- a/pom.xml +++ b/pom.xml @@ -143,6 +143,12 @@ io.micrometer micrometer-registry-prometheus + + + org.apache.commons + commons-lang3 + 3.8.1 + diff --git a/src/main/java/biz/nynja/account/components/Validator.java b/src/main/java/biz/nynja/account/components/Validator.java index d30af4f..0703c1d 100644 --- a/src/main/java/biz/nynja/account/components/Validator.java +++ b/src/main/java/biz/nynja/account/components/Validator.java @@ -8,11 +8,13 @@ import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; import java.util.HashMap; +import java.util.Optional; import java.util.regex.Matcher; import java.util.regex.Pattern; import javax.annotation.PostConstruct; +import org.apache.commons.lang3.tuple.ImmutablePair; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.core.io.ClassPathResource; @@ -240,32 +242,32 @@ public class Validator { return null; } - public Cause validateContactInfo(ContactType type, String contactInfoValue) { + public Optional> validateContactInfo(ContactType type, String contactInfoValue) { if (contactInfoValue == null || contactInfoValue.trim().isEmpty()) { - return Cause.MISSING_CONTACT_INFO_IDENTIFIER; + return Optional.of(new ImmutablePair<>(Cause.MISSING_CONTACT_INFO_IDENTIFIER, "Missing contact info identifier")); } switch (type) { case MISSING_CONTACT_TYPE: - return Cause.MISSING_CONTACT_INFO_TYPE; + return Optional.of(new ImmutablePair<>(Cause.MISSING_CONTACT_INFO_TYPE, "Missing contact info type")); case PHONE_CONTACT: // We expect to receive phone number in the following format : ":" String[] provider = contactInfoValue.split(":"); if (provider == null || provider.length != 2) { - return Cause.PHONE_NUMBER_INVALID; + return Optional.of(new ImmutablePair<>(Cause.PHONE_NUMBER_INVALID, "Invalid phone number")); } if (!isPhoneNumberValid(provider[1], provider[0])) { - return Cause.PHONE_NUMBER_INVALID; + return Optional.of(new ImmutablePair<>(Cause.PHONE_NUMBER_INVALID, "Invalid phone number")); } break; case EMAIL_CONTACT: if (!isEmailValid(contactInfoValue)) { - return Cause.EMAIL_INVALID; + return Optional.of(new ImmutablePair<>(Cause.EMAIL_INVALID, "Invalid email")); } break; default: break; } - return null; + return Optional.empty(); } public Cause validateCreatePendingAccountRequest(CreatePendingAccountRequest request) { @@ -342,19 +344,19 @@ public class Validator { request.getAuthenticationProvider().getAuthenticationProvider()); } - public Cause validateContactInfoRequest(String accountId, ContactDetails contactDetails) { + public Optional> validateContactInfoRequest(String accountId, ContactDetails contactDetails) { if ((accountId == null) || (accountId.isEmpty())) { - return Cause.MISSING_ACCOUNT_ID; + return Optional.of(new ImmutablePair<>(Cause.MISSING_ACCOUNT_ID, "Missing account id")); } if (contactDetails.getTypeValue() == 0) { - return Cause.MISSING_CONTACT_INFO_TYPE; + return Optional.of(new ImmutablePair<>(Cause.MISSING_CONTACT_INFO_TYPE, "Missing contact info type")); } if (contactDetails.getValue() == null || contactDetails.getValue().isEmpty()) { - return Cause.MISSING_CONTACT_INFO_IDENTIFIER; + return Optional.of(new ImmutablePair<>(Cause.MISSING_CONTACT_INFO_IDENTIFIER, "Missing contact info identifier")); } if (!isValidUuid(accountId)) { - return Cause.INVALID_ACCOUNT_ID; + return Optional.of(new ImmutablePair<>(Cause.INVALID_ACCOUNT_ID, "Invalid account id")); } return validateContactInfo(contactDetails.getType(), contactDetails.getValue()); } diff --git a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java index 8f67e83..7b26f44 100644 --- a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java +++ b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java @@ -7,8 +7,10 @@ import java.time.Instant; import java.util.ArrayList; import java.util.Date; import java.util.List; +import java.util.Optional; import java.util.UUID; +import org.apache.commons.lang3.tuple.ImmutablePair; import org.lognet.springboot.grpc.GRpcService; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -561,11 +563,10 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas StreamObserver responseObserver) { logger.info("Adding contact info to account requested."); logger.debug("Adding contact info to account requested: {}", request); - Cause cause = validator.validateContactInfoRequest(request.getAccountId(), request.getContactInfo()); - if (cause != null) { - responseObserver - .onNext(StatusResponse.newBuilder().setError(ErrorResponse.newBuilder().setCause(cause)).build()); - responseObserver.onCompleted(); + Optional> validationResult = validator.validateContactInfoRequest(request.getAccountId(), request.getContactInfo()); + if (validationResult.isPresent()) { + prepareErrorStatusResponse(responseObserver, validationResult.get().getKey(), + validationResult.get().getValue()); return; } if (request.getContactInfo().getType() == ContactType.PHONE_CONTACT) { @@ -582,9 +583,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas } logger.error("Contact info {}:{} was not added to account {}.", request.getContactInfo().getType().name(), request.getContactInfo().getValue(), request.getAccountId()); - responseObserver.onNext(StatusResponse.newBuilder() - .setError(ErrorResponse.newBuilder().setCause(Cause.ERROR_ADDING_CONTACT_INFO)).build()); - responseObserver.onCompleted(); + prepareErrorStatusResponse(responseObserver, Cause.ERROR_ADDING_CONTACT_INFO, "Error adding contact info"); return; } @@ -604,11 +603,11 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas StreamObserver responseObserver) { logger.info("Removing contact info from account requested."); logger.debug("Removing contact info from account requested: {}", request); - Cause cause = validator.validateContactInfoRequest(request.getAccountId(), request.getContactInfo()); - if (cause != null) { - responseObserver - .onNext(StatusResponse.newBuilder().setError(ErrorResponse.newBuilder().setCause(cause)).build()); - responseObserver.onCompleted(); + Optional> validationResult = validator + .validateContactInfoRequest(request.getAccountId(), request.getContactInfo()); + if (validationResult.isPresent()) { + prepareErrorStatusResponse(responseObserver, validationResult.get().getKey(), + validationResult.get().getValue()); return; } if (request.getContactInfo().getType() == ContactType.PHONE_CONTACT) { @@ -625,9 +624,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas } logger.error("Contact info {}:{} was not removed from account {}.", request.getContactInfo().getType().name(), request.getContactInfo().getValue(), request.getAccountId()); - responseObserver.onNext(StatusResponse.newBuilder() - .setError(ErrorResponse.newBuilder().setCause(Cause.ERROR_REMOVING_CONTACT_INFO)).build()); - responseObserver.onCompleted(); + prepareErrorStatusResponse(responseObserver, Cause.ERROR_REMOVING_CONTACT_INFO, "Error removing contact info"); return; } @@ -718,4 +715,11 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas responseObserver.onCompleted(); return; } -} \ No newline at end of file + + public void prepareErrorStatusResponse(StreamObserver responseObserver, Cause error, + String message) { + responseObserver.onNext(StatusResponse.newBuilder() + .setError(ErrorResponse.newBuilder().setCause(error).setMessage(message)).build()); + responseObserver.onCompleted(); + } +} -- GitLab From 414bb6ae3e43b3fdc5b8b84b2953f81ee1215cbc Mon Sep 17 00:00:00 2001 From: Stanimir Penkov Date: Tue, 23 Oct 2018 15:20:22 +0300 Subject: [PATCH 7/9] NY-3809: Apply code alignment Signed-off-by: Stanimir Penkov --- .../java/biz/nynja/account/services/AccountServiceImpl.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java index 7b26f44..2115276 100644 --- a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java +++ b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java @@ -563,7 +563,8 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas StreamObserver responseObserver) { logger.info("Adding contact info to account requested."); logger.debug("Adding contact info to account requested: {}", request); - Optional> validationResult = validator.validateContactInfoRequest(request.getAccountId(), request.getContactInfo()); + Optional> validationResult = validator + .validateContactInfoRequest(request.getAccountId(), request.getContactInfo()); if (validationResult.isPresent()) { prepareErrorStatusResponse(responseObserver, validationResult.get().getKey(), validationResult.get().getValue()); -- GitLab From fd5db8831b6eea002ab523c5a71b0280cb5e6aae Mon Sep 17 00:00:00 2001 From: Stanimir Penkov Date: Tue, 23 Oct 2018 18:03:28 +0300 Subject: [PATCH 8/9] NY-3809: Code Review Feedback - moved methods to service layer. Signed-off-by: Stanimir Penkov --- .../components/PhoneNumberNormalization.java | 40 +++++++++++++++++++ .../account/services/AccountServiceImpl.java | 30 +++----------- 2 files changed, 46 insertions(+), 24 deletions(-) create mode 100644 src/main/java/biz/nynja/account/components/PhoneNumberNormalization.java diff --git a/src/main/java/biz/nynja/account/components/PhoneNumberNormalization.java b/src/main/java/biz/nynja/account/components/PhoneNumberNormalization.java new file mode 100644 index 0000000..ed6943e --- /dev/null +++ b/src/main/java/biz/nynja/account/components/PhoneNumberNormalization.java @@ -0,0 +1,40 @@ +/** + * Copyright (C) 2018 Nynja Inc. All rights reserved. + */ +package biz.nynja.account.components; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.stereotype.Service; + +import biz.nynja.account.grpc.AddContactInfoRequest; +import biz.nynja.account.grpc.ContactDetails; +import biz.nynja.account.grpc.DeleteContactInfoRequest; + +@Service +public class PhoneNumberNormalization { + + @Autowired + private Validator validator; + + public AddContactInfoRequest normalizePhoneNumber(AddContactInfoRequest request) { + // Get the normalized phone number from libphone + ContactDetails newContactDetails = ContactDetails.newBuilder().setType(request.getContactInfo().getType()) + .setValue(validator.getNormalizedPhoneNumber(request.getContactInfo().getValue())) + .setLabel(request.getContactInfo().getLabel()).build(); + AddContactInfoRequest newRequest = AddContactInfoRequest.newBuilder().setAccountId(request.getAccountId()) + .setContactInfo(newContactDetails).build(); + request = newRequest; + return request; + } + + public DeleteContactInfoRequest normalizePhoneNumber(DeleteContactInfoRequest request) { + // Get the normalized phone number from libphone + ContactDetails newContactDetails = ContactDetails.newBuilder().setType(request.getContactInfo().getType()) + .setValue(validator.getNormalizedPhoneNumber(request.getContactInfo().getValue())) + .setLabel(request.getContactInfo().getLabel()).build(); + DeleteContactInfoRequest newRequest = DeleteContactInfoRequest.newBuilder().setAccountId(request.getAccountId()) + .setContactInfo(newContactDetails).build(); + request = newRequest; + return request; + } +} diff --git a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java index 2115276..b5415a8 100644 --- a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java +++ b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java @@ -16,6 +16,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; import biz.nynja.account.components.AccountServiceHelper; +import biz.nynja.account.components.PhoneNumberNormalization; import biz.nynja.account.components.Validator; import biz.nynja.account.grpc.AccountByAccountIdRequest; import biz.nynja.account.grpc.AccountByAuthenticationProviderRequest; @@ -104,6 +105,9 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas @Autowired private AccountServiceHelper accountServiceHelper; + @Autowired + private PhoneNumberNormalization phoneNumberNormalization; + @Override public void createAccount(CreateAccountRequest request, StreamObserver responseObserver) { @@ -571,7 +575,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas return; } if (request.getContactInfo().getType() == ContactType.PHONE_CONTACT) { - request = normalizePhoneNumber(request); + request = phoneNumberNormalization.normalizePhoneNumber(request); } boolean result = accountRepositoryAdditional.addContactInfo(UUID.fromString(request.getAccountId()), ContactInfo.createContactInfoFromProto(request.getContactInfo())); @@ -588,17 +592,6 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas return; } - private AddContactInfoRequest normalizePhoneNumber(AddContactInfoRequest request) { - // Get the normalized phone number from libphone - ContactDetails newContactDetails = ContactDetails.newBuilder().setType(request.getContactInfo().getType()) - .setValue(validator.getNormalizedPhoneNumber(request.getContactInfo().getValue())) - .setLabel(request.getContactInfo().getLabel()).build(); - AddContactInfoRequest newRequest = AddContactInfoRequest.newBuilder().setAccountId(request.getAccountId()) - .setContactInfo(newContactDetails).build(); - request = newRequest; - return request; - } - @Override public void deleteContactInfoFromAccount(DeleteContactInfoRequest request, StreamObserver responseObserver) { @@ -612,7 +605,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas return; } if (request.getContactInfo().getType() == ContactType.PHONE_CONTACT) { - request = normalizePhoneNumber(request); + request = phoneNumberNormalization.normalizePhoneNumber(request); } boolean result = accountRepositoryAdditional.deleteContactInfo(UUID.fromString(request.getAccountId()), ContactInfo.createContactInfoFromProto(request.getContactInfo())); @@ -629,17 +622,6 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas return; } - private DeleteContactInfoRequest normalizePhoneNumber(DeleteContactInfoRequest request) { - // Get the normalized phone number from libphone - ContactDetails newContactDetails = ContactDetails.newBuilder().setType(request.getContactInfo().getType()) - .setValue(validator.getNormalizedPhoneNumber(request.getContactInfo().getValue())) - .setLabel(request.getContactInfo().getLabel()).build(); - DeleteContactInfoRequest newRequest = DeleteContactInfoRequest.newBuilder() - .setAccountId(request.getAccountId()).setContactInfo(newContactDetails).build(); - request = newRequest; - return request; - } - @Override public void deleteAuthenticationProviderFromProfile(DeleteAuthenticationProviderRequest request, StreamObserver responseObserver) { -- GitLab From cfea9c767a1b2e9b96d6ca449ad7f4dd8822a229 Mon Sep 17 00:00:00 2001 From: Stanimir Penkov Date: Tue, 23 Oct 2018 18:20:31 +0300 Subject: [PATCH 9/9] NY-3809: Code Review Feedback - renamed PhoneNumberNormalization to PhoneNumberNormalizer. Signed-off-by: Stanimir Penkov --- ...umberNormalization.java => PhoneNumberNormalizer.java} | 2 +- .../biz/nynja/account/services/AccountServiceImpl.java | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) rename src/main/java/biz/nynja/account/components/{PhoneNumberNormalization.java => PhoneNumberNormalizer.java} (97%) diff --git a/src/main/java/biz/nynja/account/components/PhoneNumberNormalization.java b/src/main/java/biz/nynja/account/components/PhoneNumberNormalizer.java similarity index 97% rename from src/main/java/biz/nynja/account/components/PhoneNumberNormalization.java rename to src/main/java/biz/nynja/account/components/PhoneNumberNormalizer.java index ed6943e..afac39d 100644 --- a/src/main/java/biz/nynja/account/components/PhoneNumberNormalization.java +++ b/src/main/java/biz/nynja/account/components/PhoneNumberNormalizer.java @@ -11,7 +11,7 @@ import biz.nynja.account.grpc.ContactDetails; import biz.nynja.account.grpc.DeleteContactInfoRequest; @Service -public class PhoneNumberNormalization { +public class PhoneNumberNormalizer { @Autowired private Validator validator; diff --git a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java index b5415a8..19fbadd 100644 --- a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java +++ b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java @@ -16,7 +16,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; import biz.nynja.account.components.AccountServiceHelper; -import biz.nynja.account.components.PhoneNumberNormalization; +import biz.nynja.account.components.PhoneNumberNormalizer; import biz.nynja.account.components.Validator; import biz.nynja.account.grpc.AccountByAccountIdRequest; import biz.nynja.account.grpc.AccountByAuthenticationProviderRequest; @@ -106,7 +106,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas private AccountServiceHelper accountServiceHelper; @Autowired - private PhoneNumberNormalization phoneNumberNormalization; + private PhoneNumberNormalizer phoneNumberNormalizer; @Override public void createAccount(CreateAccountRequest request, StreamObserver responseObserver) { @@ -575,7 +575,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas return; } if (request.getContactInfo().getType() == ContactType.PHONE_CONTACT) { - request = phoneNumberNormalization.normalizePhoneNumber(request); + request = phoneNumberNormalizer.normalizePhoneNumber(request); } boolean result = accountRepositoryAdditional.addContactInfo(UUID.fromString(request.getAccountId()), ContactInfo.createContactInfoFromProto(request.getContactInfo())); @@ -605,7 +605,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas return; } if (request.getContactInfo().getType() == ContactType.PHONE_CONTACT) { - request = phoneNumberNormalization.normalizePhoneNumber(request); + request = phoneNumberNormalizer.normalizePhoneNumber(request); } boolean result = accountRepositoryAdditional.deleteContactInfo(UUID.fromString(request.getAccountId()), ContactInfo.createContactInfoFromProto(request.getContactInfo())); -- GitLab