From f499f007c887d6dc31faf8ad9c30ff1fad322040 Mon Sep 17 00:00:00 2001 From: Stoyan Tzenkov Date: Fri, 18 Jan 2019 16:45:07 +0200 Subject: [PATCH 01/10] NY-5672: Label validation corrected. Signed-off-by: Stoyan Tzenkov --- src/main/java/biz/nynja/account/validation/Validators.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/main/java/biz/nynja/account/validation/Validators.java b/src/main/java/biz/nynja/account/validation/Validators.java index 9152ac6..b8946c6 100644 --- a/src/main/java/biz/nynja/account/validation/Validators.java +++ b/src/main/java/biz/nynja/account/validation/Validators.java @@ -143,13 +143,11 @@ public class Validators { if (!isContactInfoPhoneLabelValid(oldPhoneNumberLabel)) { validation.addError( - new ValidationError("Invalid phone label: " + oldPhoneNumberLabel, Cause.INVALID_PHONE_LABEL)); - return validation; + new ValidationError("Invalid old phone label: " + oldPhoneNumberLabel, Cause.INVALID_PHONE_LABEL)); } if (!isContactInfoPhoneLabelValid(editedPhoneNumberLabel)) { - validation.addError(new ValidationError("Invalid phone label: " + editedPhoneNumberLabel, + validation.addError(new ValidationError("Invalid new phone label: " + editedPhoneNumberLabel, Cause.INVALID_PHONE_LABEL)); - return validation; } return validation; -- GitLab From eef3c20754950851c7a45b1195910618a473d4a5 Mon Sep 17 00:00:00 2001 From: Stoyan Tzenkov Date: Mon, 21 Jan 2019 11:03:11 +0200 Subject: [PATCH 02/10] NY-6559: Logging for createPendingAccount corrected. Signed-off-by: Stoyan Tzenkov --- .../nynja/account/services/AccountServiceImpl.java | 12 +++++++++--- .../services/decomposition/AccountCreator.java | 9 ++------- .../biz/nynja/account/validation/Validators.java | 10 +++++++--- 3 files changed, 18 insertions(+), 13 deletions(-) diff --git a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java index 058737b..d33e624 100644 --- a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java +++ b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java @@ -5,6 +5,7 @@ package biz.nynja.account.services; import static biz.nynja.account.validation.Validators.account; import static biz.nynja.account.validation.Validators.authenticationProvider; +import static biz.nynja.account.validation.Validators.pendingAccount; import static biz.nynja.account.validation.Validators.phoneValidator; import static biz.nynja.account.validation.Validators.util; @@ -456,14 +457,19 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas @Permitted(role = RoleConstants.USER) public void createPendingAccount(CreatePendingAccountRequest request, StreamObserver responseObserver) { + CreatePendingAccountResponse response; logger.info("Creating pending account..."); logger.debug("Creating pending account: {} ...", request); - CreatePendingAccountResponse response = accountCreator.retrieveCreatePendingAccountResponse(request); + Optional cause = pendingAccount.validateCreatePendingAccountRequest(request); + if (cause.isPresent()) { + response = CreatePendingAccountResponse.newBuilder().setError(ErrorResponse.newBuilder().setCause(cause.get())) + .build(); + } else { + response = accountCreator.retrieveCreatePendingAccountResponse(request); + } - logger.info("SUCCESS: Created pending account for provider {}: \"{}\"", request.getAuthenticationProvider(), - response); responseObserver.onNext(response); responseObserver.onCompleted(); } diff --git a/src/main/java/biz/nynja/account/services/decomposition/AccountCreator.java b/src/main/java/biz/nynja/account/services/decomposition/AccountCreator.java index 9123af3..d8893c4 100644 --- a/src/main/java/biz/nynja/account/services/decomposition/AccountCreator.java +++ b/src/main/java/biz/nynja/account/services/decomposition/AccountCreator.java @@ -55,11 +55,6 @@ public class AccountCreator { * @return */ public CreatePendingAccountResponse retrieveCreatePendingAccountResponse(CreatePendingAccountRequest request) { - Optional cause = pendingAccount.validateCreatePendingAccountRequest(request); - if (cause.isPresent()) { - return CreatePendingAccountResponse.newBuilder().setError(ErrorResponse.newBuilder().setCause(cause.get())) - .build(); - } if (request.getAuthenticationType() == AuthenticationType.PHONE) { // Get the normalized phone number from libphone @@ -98,8 +93,8 @@ public class AccountCreator { logger.debug("Pending account \"{}\" saved into the DB", savedPendingAccount); response = CreatePendingAccountResponse.newBuilder().setPendingAccountDetails(savedPendingAccount.toProto()) .build(); - logger.info("Pending account created successfully."); - logger.debug("Pending account: \"{}\" created successfully.", response); + logger.info("SUCCESS: Pending account created successfully."); + logger.debug("SUCCESS: Pending account: \"{}\" created successfully.", response); return response; } catch (Exception e) { logger.error(e.getMessage()); diff --git a/src/main/java/biz/nynja/account/validation/Validators.java b/src/main/java/biz/nynja/account/validation/Validators.java index 21dc73c..ee89711 100644 --- a/src/main/java/biz/nynja/account/validation/Validators.java +++ b/src/main/java/biz/nynja/account/validation/Validators.java @@ -453,9 +453,13 @@ public class Validators { logger.info("Checking email: {}", email); EmailValidator eValidator = EmailValidator.getInstance(); - boolean result = eValidator.isValid(email); - logger.info("Email: {} is " + (result ? "" : "not ") + "valid.", email); - return result; + boolean validated = eValidator.isValid(email); + if (!validated) { + logger.error("Email: {} is not valid.", email); + } else { + logger.info("Email: {} is valid.", email); + } + return validated; } public boolean validateBirthdayIsSet(Date birthday) { -- GitLab From 45f46bc045350ccef8324234cea3ea5020cc75fa Mon Sep 17 00:00:00 2001 From: Dragomir Todorov Date: Mon, 21 Jan 2019 14:23:54 +0200 Subject: [PATCH 03/10] NY-6173: Removed check and clear roles directly --- .../services/decomposition/AccountCreator.java | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/main/java/biz/nynja/account/services/decomposition/AccountCreator.java b/src/main/java/biz/nynja/account/services/decomposition/AccountCreator.java index 9123af3..58f9e96 100644 --- a/src/main/java/biz/nynja/account/services/decomposition/AccountCreator.java +++ b/src/main/java/biz/nynja/account/services/decomposition/AccountCreator.java @@ -3,13 +3,12 @@ */ package biz.nynja.account.services.decomposition; +import static biz.nynja.account.validation.Validators.pendingAccount; + import java.time.Instant; import java.util.Optional; import java.util.UUID; -import biz.nynja.account.phone.PhoneNumberNormalizer; -import biz.nynja.account.validation.Validation; -import biz.nynja.account.validation.Validators; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.stereotype.Service; @@ -27,11 +26,10 @@ import biz.nynja.account.models.Account; import biz.nynja.account.models.AuthenticationProvider; import biz.nynja.account.models.PendingAccount; import biz.nynja.account.models.PendingAccountByAuthenticationProvider; +import biz.nynja.account.phone.PhoneNumberNormalizer; import biz.nynja.account.repositories.AccountRepositoryAdditional; import biz.nynja.account.repositories.PendingAccountRepository; -import static biz.nynja.account.validation.Validators.pendingAccount; - @Service public class AccountCreator { @@ -82,12 +80,14 @@ public class AccountCreator { pendingAccount.setProfileId(UUID.randomUUID()); if (pendingAccountRepository.findByAccountId(pendingAccount.getAccountId()) != null || accountRepositoryAdditional.accountIdAlreadyUsed(pendingAccount.getAccountId())) { - logger.error("Can not create pending account. The account id {} is already used.", pendingAccount.getAccountId()); + logger.error("Can not create pending account. The account id {} is already used.", + pendingAccount.getAccountId()); return CreatePendingAccountResponse.newBuilder() .setError(ErrorResponse.newBuilder().setCause(Cause.INTERNAL_SERVER_ERROR)).build(); } if (accountRepositoryAdditional.profileIdAlreadyUsed(pendingAccount.getProfileId())) { - logger.error("Can not create pending account. The profile id {} is already used.", pendingAccount.getProfileId()); + logger.error("Can not create pending account. The profile id {} is already used.", + pendingAccount.getProfileId()); return CreatePendingAccountResponse.newBuilder() .setError(ErrorResponse.newBuilder().setCause(Cause.INTERNAL_SERVER_ERROR)).build(); } @@ -151,9 +151,7 @@ public class AccountCreator { return response; } - if (request.getRolesList() == null || request.getRolesList().isEmpty()) { - request = CompletePendingAccountCreationRequest.newBuilder(request).addRoles(Role.USER).build(); - } + request = CompletePendingAccountCreationRequest.newBuilder(request).clearRoles().addRoles(Role.USER).build(); Account createdAccount = accountRepositoryAdditional.completePendingAccountCreation(request); -- GitLab From d8c62c1ad6396b936a9050dfade6020e4a351fce Mon Sep 17 00:00:00 2001 From: sergeyPensov Date: Mon, 14 Jan 2019 16:17:50 +0200 Subject: [PATCH 04/10] Implement flow for deleting profile --- .../AccountRepositoryAdditionalImpl.java | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java index a30e94f..02d4dd4 100644 --- a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java +++ b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java @@ -465,7 +465,7 @@ public class AccountRepositoryAdditionalImpl implements AccountRepositoryAdditio } public Optional deleteProfile(UUID profileId) { - CassandraBatchOperations batchOperations = cassandraTemplate.batchOps(); + Transaction sagaTransaction = new SagaTransaction(cassandraTemplate); Profile existingProfile = profileRepository.findByProfileId(profileId); if (existingProfile == null) { logger.error("Error deleting profile. Existing profile with the provided id {} was not found.", profileId); @@ -479,22 +479,28 @@ public class AccountRepositoryAdditionalImpl implements AccountRepositoryAdditio return Optional.of(Cause.ERROR_PERMISSION_DENIED); } - WriteResult wr = null; + WriteResult wr; try { - deleteProfileAccountsWhenDeletingProfile(batchOperations, existingAccountsForProfile); + deleteProfileAccountsWhenDeletingProfile(sagaTransaction, existingAccountsForProfile); if (existingProfile.getAuthenticationProviders() != null && existingProfile.getAuthenticationProviders().size() > 0) { - deleteAuthenticationProvidersFromProfile(batchOperations, existingProfile.getProfileId(), + deleteAuthenticationProvidersFromProfile(sagaTransaction, existingProfile.getProfileId(), existingProfile.getAuthenticationProviders()); } - deleteProfileData(batchOperations, existingProfile); - wr = batchOperations.execute(); + deleteProfileData(sagaTransaction, existingProfile); + wr = sagaTransaction.execute(); + if (!erlangAccountBridge.deleteProfile(existingProfile.getProfileId(), existingAccountsForProfile.stream() + .map(AccountByProfileId::getAccountId).collect(Collectors.toList()))) { + logger.error("Internal error with erlang"); + sagaTransaction.rollBack(); + return Optional.of(Cause.ERROR_DELETING_PROFILE); + } } catch (IllegalArgumentException | IllegalStateException e) { logger.info("Exception while deleting account."); logger.debug("Exception while deleting account: {} ...", e.getMessage()); return Optional.of(Cause.ERROR_DELETING_PROFILE); } - if (wr != null && wr.wasApplied()) { + if (wr.wasApplied()) { return Optional.empty(); } return Optional.of(Cause.ERROR_DELETING_PROFILE); -- GitLab From 327ac3c8774c5ccf3262b300a4ecd14b9e80ca37 Mon Sep 17 00:00:00 2001 From: sergeyPensov Date: Tue, 15 Jan 2019 13:06:04 +0200 Subject: [PATCH 05/10] add additional timeout checking --- .../account/repositories/AccountRepositoryAdditionalImpl.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java index 02d4dd4..beaa74c 100644 --- a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java +++ b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java @@ -495,6 +495,10 @@ public class AccountRepositoryAdditionalImpl implements AccountRepositoryAdditio sagaTransaction.rollBack(); return Optional.of(Cause.ERROR_DELETING_PROFILE); } + } catch (StatusRuntimeException e) { + logger.info("Error while lose connection with bridge"); + sagaTransaction.rollBack(); + return null; } catch (IllegalArgumentException | IllegalStateException e) { logger.info("Exception while deleting account."); logger.debug("Exception while deleting account: {} ...", e.getMessage()); -- GitLab From 69f4607d11b42296e2781e3febda5f4bd979ec75 Mon Sep 17 00:00:00 2001 From: sergeyPensov Date: Wed, 16 Jan 2019 12:19:09 +0200 Subject: [PATCH 06/10] Return optional --- .../account/repositories/AccountRepositoryAdditionalImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java index beaa74c..418a7c7 100644 --- a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java +++ b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java @@ -498,7 +498,7 @@ public class AccountRepositoryAdditionalImpl implements AccountRepositoryAdditio } catch (StatusRuntimeException e) { logger.info("Error while lose connection with bridge"); sagaTransaction.rollBack(); - return null; + return Optional.empty(); } catch (IllegalArgumentException | IllegalStateException e) { logger.info("Exception while deleting account."); logger.debug("Exception while deleting account: {} ...", e.getMessage()); -- GitLab From 42a80cc0d85e71a5952b7564265dd711ee1962e9 Mon Sep 17 00:00:00 2001 From: sergeyPensov Date: Wed, 16 Jan 2019 13:10:10 +0200 Subject: [PATCH 07/10] Return error when erlang connection failed --- .../account/repositories/AccountRepositoryAdditionalImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java index 418a7c7..fbaa1ea 100644 --- a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java +++ b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java @@ -498,7 +498,7 @@ public class AccountRepositoryAdditionalImpl implements AccountRepositoryAdditio } catch (StatusRuntimeException e) { logger.info("Error while lose connection with bridge"); sagaTransaction.rollBack(); - return Optional.empty(); + return Optional.of(Cause.ERROR_DELETING_PROFILE); } catch (IllegalArgumentException | IllegalStateException e) { logger.info("Exception while deleting account."); logger.debug("Exception while deleting account: {} ...", e.getMessage()); -- GitLab From c254f3e35ef1010052b86e86ba3a63935b543856 Mon Sep 17 00:00:00 2001 From: Dragomir Todorov Date: Mon, 21 Jan 2019 15:26:00 +0200 Subject: [PATCH 08/10] NY-6771: Fixed issues with update account --- .../biz/nynja/account/models/ContactInfo.java | 16 +++++++++++++++- .../AccountRepositoryAdditionalImpl.java | 12 ++++++------ 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/src/main/java/biz/nynja/account/models/ContactInfo.java b/src/main/java/biz/nynja/account/models/ContactInfo.java index 6acc182..05ffe13 100644 --- a/src/main/java/biz/nynja/account/models/ContactInfo.java +++ b/src/main/java/biz/nynja/account/models/ContactInfo.java @@ -3,6 +3,8 @@ */ package biz.nynja.account.models; +import java.io.Serializable; + import org.springframework.data.cassandra.core.mapping.UserDefinedType; import biz.nynja.account.grpc.ContactDetails; @@ -11,11 +13,14 @@ import biz.nynja.account.grpc.ContactType; import biz.nynja.account.phone.PhoneNumberUtils; @UserDefinedType -public class ContactInfo { +public class ContactInfo implements Serializable { + private static final long serialVersionUID = -5910187338057785293L; + private String type; private String value; private String label; + private String countrySelector; public String getType() { return type; @@ -41,6 +46,14 @@ public class ContactInfo { this.label = label; } + public String getCountrySelector() { + return countrySelector; + } + + public void setCountrySelector(String countrySelector) { + this.countrySelector = countrySelector; + } + @Override public int hashCode() { final int prime = 31; @@ -89,6 +102,7 @@ public class ContactInfo { contactInfo.setType(contactDetails.getType().toString()); contactInfo.setValue(contactDetails.getValue()); contactInfo.setLabel(contactDetails.getLabel()); + contactInfo.setCountrySelector(contactDetails.getCountrySelector()); return contactInfo; } diff --git a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java index a30e94f..c419fce 100644 --- a/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java +++ b/src/main/java/biz/nynja/account/repositories/AccountRepositoryAdditionalImpl.java @@ -200,18 +200,18 @@ public class AccountRepositoryAdditionalImpl implements AccountRepositoryAdditio Transaction sagaTransaction = new SagaTransaction(cassandraTemplate); Account existingAccount = accountRepository.findByAccountId(UUID.fromString(request.getAccountId())); + if (existingAccount == null) { + logger.error("Existing account with the provided id {} was not found.", request.getAccountId()); + logger.debug("Existing account with the provided id {} was not found.", request.getAccountId()); + return null; + } + if (!permissionsValidator.isAdminToken()) { // No permission to update roles, load old ones Set roles = existingAccount.getRoles().stream().map(Role::valueOf).collect(Collectors.toSet()); request = UpdateAccountRequest.newBuilder(request).clearRoles().addAllRoles(roles).build(); } - - if (existingAccount == null) { - logger.error("Existing account with the provided id {} was not found.", request.getAccountId()); - logger.debug("Existing account with the provided id {} was not found.", request.getAccountId()); - return null; - } Long timeUpdated = Instant.now().toEpochMilli(); WriteResult wr = null; try { -- GitLab From e0653b430590db4268a6b6704fb1af659e095916 Mon Sep 17 00:00:00 2001 From: Dragomir Todorov Date: Mon, 21 Jan 2019 15:36:39 +0200 Subject: [PATCH 09/10] NY-6771: Removed serialVersion --- src/main/java/biz/nynja/account/models/ContactInfo.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/main/java/biz/nynja/account/models/ContactInfo.java b/src/main/java/biz/nynja/account/models/ContactInfo.java index 05ffe13..a6c96b4 100644 --- a/src/main/java/biz/nynja/account/models/ContactInfo.java +++ b/src/main/java/biz/nynja/account/models/ContactInfo.java @@ -15,8 +15,6 @@ import biz.nynja.account.phone.PhoneNumberUtils; @UserDefinedType public class ContactInfo implements Serializable { - private static final long serialVersionUID = -5910187338057785293L; - private String type; private String value; private String label; -- GitLab From 16fa25393f52616d74ed5fd36c6b085aec83b95f Mon Sep 17 00:00:00 2001 From: Stoyan Hristov Date: Mon, 21 Jan 2019 16:24:49 +0200 Subject: [PATCH 10/10] fix unit tests --- .../biz/nynja/account/services/AccountServiceTests.java | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/test/java/biz/nynja/account/services/AccountServiceTests.java b/src/test/java/biz/nynja/account/services/AccountServiceTests.java index 0f0386d..919803c 100644 --- a/src/test/java/biz/nynja/account/services/AccountServiceTests.java +++ b/src/test/java/biz/nynja/account/services/AccountServiceTests.java @@ -97,12 +97,7 @@ import biz.nynja.account.services.decomposition.IncorrectAccountCountException; import biz.nynja.account.services.decomposition.ProfileProvider; import biz.nynja.account.utils.GrpcServerTestBase; import biz.nynja.account.utils.Util; -import io.grpc.Context; -import io.grpc.Contexts; import io.grpc.Metadata; -import io.grpc.ServerCall; -import io.grpc.ServerCallHandler; -import io.grpc.ServerInterceptor; import io.grpc.stub.MetadataUtils; /** @@ -1510,8 +1505,8 @@ public class AccountServiceTests extends GrpcServerTestBase { final SearchResponse reply = searchServiceBlockingStub.searchByUsername(request); assertNotNull("Reply should not be null", reply); - assertTrue(String.format("Reply should contain cause '%s'", Cause.MULTIPLE_INVALID_PARAMETERS), - reply.getError().getCause().equals(Cause.MULTIPLE_INVALID_PARAMETERS)); + assertTrue(String.format("Reply should contain cause '%s'", Cause.MISSING_USERNAME), + reply.getError().getCause().equals(Cause.MISSING_USERNAME)); } @Test -- GitLab