From 713a97e37b4570a5245de449a94c10e7819bc8bd Mon Sep 17 00:00:00 2001 From: Stoyan Hristov Date: Wed, 12 Dec 2018 13:52:10 +0200 Subject: [PATCH 1/2] NY-5834 : Return only ACCOUNT_NOT_FOUND Return only ACCOUNT_NOT_FOUND in all search/get methods to simplify usage of the account methods. --- .../account/services/AccountServiceImpl.java | 44 +++++++++++-------- .../account/services/AccountServiceTests.java | 16 +++---- 2 files changed, 34 insertions(+), 26 deletions(-) diff --git a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java index 4057f14..23f4a21 100644 --- a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java +++ b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java @@ -160,12 +160,12 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas logger.info("Search account by e-mail: {}", request.getEmail()); if ((request.getEmail() == null) || request.getEmail().isEmpty()) { logAndBuildGrpcSearchResponse(responseObserver, SearchResponse.newBuilder(), "Missing e-mail.", "", - Cause.MISSING_EMAIL); + Cause.MISSING_EMAIL, ""); return; } if (!util.isEmailValid(request.getEmail())) { logAndBuildGrpcSearchResponse(responseObserver, SearchResponse.newBuilder(), "Invalid e-mail!. Value : ", - request.getEmail(), Cause.INVALID_EMAIL); + request.getEmail(), Cause.INVALID_EMAIL, ""); return; } @@ -174,12 +174,13 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas account = accountProvider.getAccountByLoginOption(AuthenticationType.EMAIL, request.getEmail()); } catch (IncorrectAccountCountException e) { logAndBuildGrpcSearchResponse(responseObserver, SearchResponse.newBuilder(), - "Error while searching for e-mail: ", request.getEmail(), Cause.INTERNAL_SERVER_ERROR); + "Error while searching for e-mail: ", request.getEmail(), Cause.INTERNAL_SERVER_ERROR, ""); } if (!account.isPresent()) { logAndBuildGrpcSearchResponse(responseObserver, SearchResponse.newBuilder(), - "No matching accounts found for e-mail: ", request.getEmail(), Cause.EMAIL_NOT_FOUND); + "No matching accounts found for e-mail: ", request.getEmail(), Cause.ACCOUNT_NOT_FOUND, + "No matching accounts found for given e-mail."); return; } else { SearchResultDetails searchResultDetails = buildSearchResultDetails(account.get().getAccountId().toString(), @@ -200,12 +201,13 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas logger.info("Search account by phone: {}", request.getPhoneNumber()); if ((request.getPhoneNumber() == null) || request.getPhoneNumber().isEmpty()) { logAndBuildGrpcSearchResponse(responseObserver, SearchResponse.newBuilder(), "Missing phone number.", "", - Cause.MISSING_PHONENUMBER); + Cause.MISSING_PHONENUMBER, ""); return; } if (!phoneValidator.isPhoneNumberValid(request.getPhoneNumber())) { logAndBuildGrpcSearchResponse(responseObserver, SearchResponse.newBuilder(), - "Invalid phone number. Value : ", request.getPhoneNumber(), Cause.INVALID_PHONENUMBER); + "Invalid phone number. Value : ", request.getPhoneNumber(), Cause.INVALID_PHONENUMBER, + "Phone number parameter has invalid format."); return; } @@ -214,12 +216,13 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas account = accountProvider.getAccountByLoginOption(AuthenticationType.PHONE, request.getPhoneNumber()); } catch (IncorrectAccountCountException e) { logAndBuildGrpcSearchResponse(responseObserver, SearchResponse.newBuilder(), - "Error while searching for phone: ", request.getPhoneNumber(), Cause.INTERNAL_SERVER_ERROR); + "Error while searching for phone: ", request.getPhoneNumber(), Cause.INTERNAL_SERVER_ERROR, ""); } if (!account.isPresent()) { logAndBuildGrpcSearchResponse(responseObserver, SearchResponse.newBuilder(), - "No matching accounts found for phone: ", request.getPhoneNumber(), Cause.PHONENUMBER_NOT_FOUND); + "No matching accounts found for phone: ", request.getPhoneNumber(), Cause.ACCOUNT_NOT_FOUND, + "No matching accounts found for given phone number."); return; } else { SearchResultDetails searchResultDetails = buildSearchResultDetails(account.get().getAccountId().toString(), @@ -270,14 +273,15 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas Validation validation = validateGetByUsernameRequest(request); if (validation.hasErrors()) { logAndBuildGrpcSearchResponse(responseObserver, SearchResponse.newBuilder(), validation.getErrorMessage(), - "", validation.getCause().get()); + "", validation.getCause().get(), validation.getErrorMessage()); return; } AccountByUsername account = accountByUsernameRepository.findByUsername(request.getUsername()); if (account == null) { logAndBuildGrpcSearchResponse(responseObserver, SearchResponse.newBuilder(), - "No matching accounts found for username: ", request.getUsername(), Cause.USERNAME_NOT_FOUND); + "No matching accounts found for username: ", request.getUsername(), Cause.ACCOUNT_NOT_FOUND, + "No matching accounts found for give username."); return; } @@ -353,14 +357,15 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas logger.info("Search account by QR code: {}", request.getQrCode()); if ((request.getQrCode() == null) || request.getQrCode().isEmpty()) { logAndBuildGrpcSearchResponse(responseObserver, SearchResponse.newBuilder(), "Missing QR code.", "", - Cause.MISSING_QR_CODE); + Cause.MISSING_QR_CODE, ""); return; } AccountByQrCode account = accountByQrCodeRepository.findByQrCode(request.getQrCode()); if (account == null) { logAndBuildGrpcSearchResponse(responseObserver, SearchResponse.newBuilder(), - "No matching accounts found for QR code! Value: ", request.getQrCode(), Cause.QR_CODE_NOT_FOUND); + "No matching accounts found for QR code! Value: ", request.getQrCode(), Cause.ACCOUNT_NOT_FOUND, + "No matching accounts found for given QR code."); return; } @@ -970,10 +975,11 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas } private static void logAndBuildGrpcSearchResponse(StreamObserver responseObserver, - SearchResponse.Builder newBuilder, String logMessage, String logValue, Cause cause) { + SearchResponse.Builder newBuilder, String logMessage, String logValue, Cause cause, String causeMessage) { logger.debug(logMessage, logValue); - responseObserver.onNext(newBuilder.setError(ErrorResponse.newBuilder().setCause(cause)).build()); + responseObserver.onNext( + newBuilder.setError(ErrorResponse.newBuilder().setCause(cause).setMessage(causeMessage)).build()); responseObserver.onCompleted(); } @@ -1125,12 +1131,14 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas if ((request.getAuthenticationType() != AuthenticationType.FACEBOOK) && (request.getAuthenticationType() != AuthenticationType.GOOGLEPLUS)) { logAndBuildGrpcSearchResponse(responseObserver, SearchResponse.newBuilder(), - "Only FACEBOOK and GOOGLEPLUS authentication types allowed.", "", Cause.FACEBOOK_GOOGLEPLUS_ONLY_EXPECTED); + "Only FACEBOOK and GOOGLEPLUS authentication types allowed.", "", + Cause.FACEBOOK_GOOGLEPLUS_ONLY_EXPECTED, + "Only FACEBOOK and GOOGLEPLUS authentication types allowed."); return; } if ((request.getAuthenticationIdentifier() == null) || request.getAuthenticationIdentifier().isEmpty()) { logAndBuildGrpcSearchResponse(responseObserver, SearchResponse.newBuilder(), - "Missing authentication provider.", "", Cause.MISSING_AUTH_PROVIDER_ID); + "Missing authentication provider.", "", Cause.MISSING_AUTH_PROVIDER_ID, ""); return; } @@ -1141,13 +1149,13 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas } catch (IncorrectAccountCountException e) { logAndBuildGrpcSearchResponse(responseObserver, SearchResponse.newBuilder(), "Error while searching for social provider: ", request.getAuthenticationIdentifier(), - Cause.INTERNAL_SERVER_ERROR); + Cause.INTERNAL_SERVER_ERROR, ""); } if (!account.isPresent()) { logAndBuildGrpcSearchResponse(responseObserver, SearchResponse.newBuilder(), "No matching accounts found for social provider: ", request.getAuthenticationIdentifier(), - Cause.AUTH_PROVIDER_NOT_FOUND); + Cause.AUTH_PROVIDER_NOT_FOUND, "No matching accounts found for given social provider."); return; } SearchResultDetails searchResultDetails = buildSearchResultDetails(account.get().getAccountId().toString(), diff --git a/src/test/java/biz/nynja/account/services/AccountServiceTests.java b/src/test/java/biz/nynja/account/services/AccountServiceTests.java index 75dcb45..6644db0 100644 --- a/src/test/java/biz/nynja/account/services/AccountServiceTests.java +++ b/src/test/java/biz/nynja/account/services/AccountServiceTests.java @@ -1433,8 +1433,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.USERNAME_NOT_FOUND), - reply.getError().getCause().equals(Cause.USERNAME_NOT_FOUND)); + assertTrue(String.format("Reply should contain cause '%s'", Cause.ACCOUNT_NOT_FOUND), + reply.getError().getCause().equals(Cause.ACCOUNT_NOT_FOUND)); } @Test @@ -1491,8 +1491,8 @@ public class AccountServiceTests extends GrpcServerTestBase { final SearchResponse reply = searchServiceBlockingStub.searchByPhoneNumber(request); assertNotNull("Reply should not be null", reply); - assertTrue(String.format("Reply should contain cause '%s'", Cause.PHONENUMBER_NOT_FOUND), - reply.getError().getCause().equals(Cause.PHONENUMBER_NOT_FOUND)); + assertTrue(String.format("Reply should contain cause '%s'", Cause.ACCOUNT_NOT_FOUND), + reply.getError().getCause().equals(Cause.ACCOUNT_NOT_FOUND)); } @Test @@ -1546,8 +1546,8 @@ public class AccountServiceTests extends GrpcServerTestBase { final SearchResponse reply = searchServiceBlockingStub.searchByEmail(request); assertNotNull("Reply should not be null", reply); - assertTrue(String.format("Reply should contain cause '%s'", Cause.EMAIL_NOT_FOUND), - reply.getError().getCause().equals(Cause.EMAIL_NOT_FOUND)); + assertTrue(String.format("Reply should contain cause '%s'", Cause.ACCOUNT_NOT_FOUND), + reply.getError().getCause().equals(Cause.ACCOUNT_NOT_FOUND)); } @Test @@ -1598,8 +1598,8 @@ public class AccountServiceTests extends GrpcServerTestBase { final SearchResponse reply = searchServiceBlockingStub.searchByQrCode(request); assertNotNull("Reply should not be null", reply); - assertTrue(String.format("Reply should contain cause '%s'", Cause.QR_CODE_NOT_FOUND), - reply.getError().getCause().equals(Cause.QR_CODE_NOT_FOUND)); + assertTrue(String.format("Reply should contain cause '%s'", Cause.ACCOUNT_NOT_FOUND), + reply.getError().getCause().equals(Cause.ACCOUNT_NOT_FOUND)); } @Test -- GitLab From e81249fce8dcf14801af990d1caf7d9d321ddfe5 Mon Sep 17 00:00:00 2001 From: Stoyan Hristov Date: Wed, 12 Dec 2018 16:26:41 +0200 Subject: [PATCH 2/2] NY-5790: return account id value --- src/main/java/biz/nynja/account/services/AccountServiceImpl.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java index 23f4a21..0788290 100644 --- a/src/main/java/biz/nynja/account/services/AccountServiceImpl.java +++ b/src/main/java/biz/nynja/account/services/AccountServiceImpl.java @@ -970,6 +970,7 @@ public class AccountServiceImpl extends AccountServiceGrpc.AccountServiceImplBas if (avatar != null) { searchResultDetails.setAvatar(avatar); } + searchResultDetails.setAccountId(id); return searchResultDetails.build(); } -- GitLab