This is an automated email from the ASF dual-hosted git repository. mosi pushed a commit to branch revert-660-FINERACT-803 in repository https://gitbox.apache.org/repos/asf/fineract.git
commit 9674e0a718352d7fb890226cbbbf8ce9cf253f74 Author: Mohit Sinha <[email protected]> AuthorDate: Sat Dec 7 23:24:00 2019 +0700 Revert "FINERACT-803: validate username uniqueness" --- .../integrationtests/UserAdministrationTest.java | 122 --------------------- .../useradministration/users/UserHelper.java | 29 +---- .../core/data/DataValidatorBuilder.java | 6 - ...pUserWritePlatformServiceJpaRepositoryImpl.java | 2 +- .../service/UserDataValidator.java | 22 +--- 5 files changed, 8 insertions(+), 173 deletions(-) diff --git a/fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/UserAdministrationTest.java b/fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/UserAdministrationTest.java deleted file mode 100644 index 0985ee1..0000000 --- a/fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/UserAdministrationTest.java +++ /dev/null @@ -1,122 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.apache.fineract.integrationtests; - -import com.jayway.restassured.builder.RequestSpecBuilder; -import com.jayway.restassured.builder.ResponseSpecBuilder; -import com.jayway.restassured.http.ContentType; -import com.jayway.restassured.specification.RequestSpecification; -import com.jayway.restassured.specification.ResponseSpecification; -import java.util.ArrayList; -import java.util.List; -import java.util.Map; -import org.apache.fineract.integrationtests.common.Utils; -import org.apache.fineract.integrationtests.common.organisation.StaffHelper; -import org.apache.fineract.integrationtests.useradministration.roles.RolesHelper; -import org.apache.fineract.integrationtests.useradministration.users.UserHelper; -import org.junit.After; -import org.junit.Assert; -import org.junit.Before; -import org.junit.Test; - -public class UserAdministrationTest { - - private ResponseSpecification responseSpec; - private RequestSpecification requestSpec; - private List<Integer> transientUsers = new ArrayList<>(); - - private ResponseSpecification expectStatusCode(int code) { - return new ResponseSpecBuilder().expectStatusCode(code).build(); - } - - @Before - public void setup() { - Utils.initializeRESTAssured(); - this.requestSpec = new RequestSpecBuilder().setContentType(ContentType.JSON).build(); - this.requestSpec.header("Authorization", "Basic " + Utils.loginIntoServerAndGetBase64EncodedAuthenticationKey()); - this.responseSpec = expectStatusCode(200); - } - - @After - public void tearDown() { - for(Integer userId : this.transientUsers) { - UserHelper.deleteUser(this.requestSpec, this.responseSpec, userId); - } - this.transientUsers.clear(); - } - - @Test - public void testCreateNewUserBlocksDuplicateUsername() { - final Integer roleId = RolesHelper.createRole(this.requestSpec, this.responseSpec); - Assert.assertNotNull(roleId); - - final Integer staffId = StaffHelper.createStaff(this.requestSpec, this.responseSpec); - Assert.assertNotNull(staffId); - - final Integer userId = (Integer) UserHelper.createUser(this.requestSpec, this.responseSpec, roleId, staffId, "alphabet", "resourceId"); - Assert.assertNotNull(userId); - this.transientUsers.add(userId); - - final List errors = (List) UserHelper.createUser(this.requestSpec, expectStatusCode(400), roleId, staffId, "alphabet", "errors"); - Map reason = (Map) errors.get(0); - Assert.assertEquals("User with username 'alphabet' already exists.", reason.get("defaultUserMessage")); - } - - @Test - public void testUpdateUserAcceptsNewOrSameUsername() { - final Integer roleId = RolesHelper.createRole(this.requestSpec, this.responseSpec); - Assert.assertNotNull(roleId); - - final Integer staffId = StaffHelper.createStaff(this.requestSpec, this.responseSpec); - Assert.assertNotNull(staffId); - - final Integer userId = (Integer) UserHelper.createUser(this.requestSpec, this.responseSpec, roleId, staffId, "alphabet", "resourceId"); - Assert.assertNotNull(userId); - this.transientUsers.add(userId); - - final Integer userId2 = (Integer) UserHelper.updateUser(this.requestSpec, this.responseSpec, userId, "renegade", "resourceId"); - Assert.assertNotNull(userId2); - - final Integer userId3 = (Integer) UserHelper.updateUser(this.requestSpec, this.responseSpec, userId, "renegade", "resourceId"); - Assert.assertNotNull(userId3); - } - - @Test - public void testUpdateUserBlockDuplicateUsername() { - final Integer roleId = RolesHelper.createRole(this.requestSpec, this.responseSpec); - Assert.assertNotNull(roleId); - - final Integer staffId = StaffHelper.createStaff(this.requestSpec, this.responseSpec); - Assert.assertNotNull(staffId); - - final Integer userId = (Integer) UserHelper.createUser(this.requestSpec, this.responseSpec, roleId, staffId, "alphabet", "resourceId"); - Assert.assertNotNull(userId); - this.transientUsers.add(userId); - - final Integer userId2 = (Integer) UserHelper.createUser(this.requestSpec, this.responseSpec, roleId, staffId, "bilingual", "resourceId"); - Assert.assertNotNull(userId2); - this.transientUsers.add(userId2); - - final List errors = (List) UserHelper.updateUser(this.requestSpec, expectStatusCode(400), userId2, "alphabet", "errors"); - Map reason = (Map) errors.get(0); - Assert.assertEquals("User with username 'alphabet' already exists.", reason.get("defaultUserMessage")); - } - -} diff --git a/fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/useradministration/users/UserHelper.java b/fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/useradministration/users/UserHelper.java index ea5b4c8..8acabda 100644 --- a/fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/useradministration/users/UserHelper.java +++ b/fineract-provider/src/integrationTest/java/org/apache/fineract/integrationtests/useradministration/users/UserHelper.java @@ -32,40 +32,21 @@ public class UserHelper { return Utils.performServerPost(requestSpec, responseSpec, CREATE_USER_URL, getTestCreateUserAsJSON(roleId, staffId), "resourceId"); } - public static Object createUser(final RequestSpecification requestSpec, final ResponseSpecification responseSpec, int roleId, int staffId, String username, String attribute) { - return Utils.performServerPost(requestSpec, responseSpec, CREATE_USER_URL, getTestCreateUserAsJSON(roleId, staffId, username), attribute); - } - public static String getTestCreateUserAsJSON(int roleId, int staffId) { - return "{ \"username\": \"" + Utils.randomNameGenerator("User_Name_", 3) + String json = "{ \"username\": \"" + Utils.randomNameGenerator("User_Name_", 3) + "\", \"firstname\": \"Test\", \"lastname\": \"User\", \"email\": \"[email protected]\"," + " \"officeId\": \"1\", \"staffId\": " + "\"" - + staffId +"\",\"roles\": [\"" - + roleId + "\"], \"sendPasswordToEmail\": false}"; - } + + Integer.toString(staffId)+"\",\"roles\": [\"" + + Integer.toString(roleId) + "\"], \"sendPasswordToEmail\": false}"; + System.out.println(json); + return json; - private static String getTestCreateUserAsJSON(int roleId, int staffId, String username) { - return "{ \"username\": \"" + username - + "\", \"firstname\": \"Test\", \"lastname\": \"User\", \"email\": \"[email protected]\"," - + " \"officeId\": \"1\", \"staffId\": " + "\"" - + staffId +"\",\"roles\": [\"" - + roleId + "\"], \"sendPasswordToEmail\": false}"; - } - - private static String getTestUpdateUserAsJSON(String username) { - return "{ \"username\": \"" + username - + "\", \"firstname\": \"Test\", \"lastname\": \"User\", \"email\": \"[email protected]\"," - + " \"officeId\": \"1\"}"; } public static Integer deleteUser(final RequestSpecification requestSpec, final ResponseSpecification responseSpec, final Integer userId) { return Utils.performServerDelete(requestSpec, responseSpec, createRoleOperationURL(userId), "resourceId"); } - public static Object updateUser(final RequestSpecification requestSpec, final ResponseSpecification responseSpec, int userId, String username, String attribute) { - return Utils.performServerPut(requestSpec, responseSpec, createRoleOperationURL(userId), getTestUpdateUserAsJSON(username), attribute); - } - private static String createRoleOperationURL(final Integer userId) { return USER_URL + "/" + userId + "?" + Utils.TENANT_IDENTIFIER; } diff --git a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/data/DataValidatorBuilder.java b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/data/DataValidatorBuilder.java index 8824e28..8191cbc 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/data/DataValidatorBuilder.java +++ b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/data/DataValidatorBuilder.java @@ -104,12 +104,6 @@ public class DataValidatorBuilder { return this; } - public void failWithMessage(final String errorCode, final String errorMessage, final Object... defaultUserMessageArgs) { - String validationErrorCode = "validation.msg." + this.resource + "." + this.parameter + "." + errorCode; - final ApiParameterError error = ApiParameterError.parameterError(validationErrorCode, errorMessage, this.parameter, this.value, defaultUserMessageArgs); - this.dataValidationErrors.add(error); - } - public void failWithCode(final String errorCode, final Object... defaultUserMessageArgs) { final StringBuilder validationErrorCode = new StringBuilder("validation.msg.").append(this.resource).append(".") .append(this.parameter).append(".").append(errorCode); diff --git a/fineract-provider/src/main/java/org/apache/fineract/useradministration/service/AppUserWritePlatformServiceJpaRepositoryImpl.java b/fineract-provider/src/main/java/org/apache/fineract/useradministration/service/AppUserWritePlatformServiceJpaRepositoryImpl.java index 0176c27..15fa769 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/useradministration/service/AppUserWritePlatformServiceJpaRepositoryImpl.java +++ b/fineract-provider/src/main/java/org/apache/fineract/useradministration/service/AppUserWritePlatformServiceJpaRepositoryImpl.java @@ -192,7 +192,7 @@ public class AppUserWritePlatformServiceJpaRepositoryImpl implements AppUserWrit this.context.authenticatedUser(new CommandWrapperBuilder().updateUser(null).build()); - this.fromApiJsonDeserializer.validateForUpdate(command.json(), userId); + this.fromApiJsonDeserializer.validateForUpdate(command.json()); final AppUser userToUpdate = this.appUserRepository.findById(userId) .orElseThrow(() -> new UserNotFoundException(userId)); diff --git a/fineract-provider/src/main/java/org/apache/fineract/useradministration/service/UserDataValidator.java b/fineract-provider/src/main/java/org/apache/fineract/useradministration/service/UserDataValidator.java index 58f5bb9..57a72c7 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/useradministration/service/UserDataValidator.java +++ b/fineract-provider/src/main/java/org/apache/fineract/useradministration/service/UserDataValidator.java @@ -32,8 +32,6 @@ import org.apache.fineract.infrastructure.core.data.DataValidatorBuilder; import org.apache.fineract.infrastructure.core.exception.InvalidJsonException; import org.apache.fineract.infrastructure.core.exception.PlatformApiDataValidationException; import org.apache.fineract.infrastructure.core.serialization.FromJsonHelper; -import org.apache.fineract.useradministration.domain.AppUser; -import org.apache.fineract.useradministration.domain.AppUserRepository; import org.apache.fineract.useradministration.domain.PasswordValidationPolicy; import org.apache.fineract.useradministration.domain.PasswordValidationPolicyRepository; import org.springframework.beans.factory.annotation.Autowired; @@ -57,13 +55,10 @@ public final class UserDataValidator { private final PasswordValidationPolicyRepository passwordValidationPolicy; - private final AppUserRepository appUserRepository; - @Autowired - public UserDataValidator(final FromJsonHelper fromApiJsonHelper, final PasswordValidationPolicyRepository passwordValidationPolicy, AppUserRepository appUserRepository) { + public UserDataValidator(final FromJsonHelper fromApiJsonHelper, final PasswordValidationPolicyRepository passwordValidationPolicy) { this.fromApiJsonHelper = fromApiJsonHelper; this.passwordValidationPolicy = passwordValidationPolicy; - this.appUserRepository = appUserRepository; } public void validateForCreate(final String json) { @@ -80,14 +75,6 @@ public final class UserDataValidator { final String username = this.fromApiJsonHelper.extractStringNamed("username", element); baseDataValidator.reset().parameter("username").value(username).notBlank().notExceedingLengthOf(100); - if (!StringUtils.isEmpty(username)) { - AppUser exists = appUserRepository.findAppUserByName(username); - if (exists != null) { - final String errorMessage = "User with username '" + username + "' already exists."; - baseDataValidator.reset().parameter("username").failWithMessage("constraint.violation.duplicate.username", errorMessage); - } - } - final String firstname = this.fromApiJsonHelper.extractStringNamed("firstname", element); baseDataValidator.reset().parameter("firstname").value(firstname).notBlank().notExceedingLengthOf(100); @@ -162,7 +149,7 @@ public final class UserDataValidator { if (!dataValidationErrors.isEmpty()) { throw new PlatformApiDataValidationException(dataValidationErrors); } } - public void validateForUpdate(final String json, final Long userId) { + public void validateForUpdate(final String json) { if (StringUtils.isBlank(json)) { throw new InvalidJsonException(); } final Type typeOfMap = new TypeToken<Map<String, Object>>() {}.getType(); @@ -186,11 +173,6 @@ public final class UserDataValidator { if (this.fromApiJsonHelper.parameterExists("username", element)) { final String username = this.fromApiJsonHelper.extractStringNamed("username", element); baseDataValidator.reset().parameter("username").value(username).notBlank().notExceedingLengthOf(100); - AppUser current = appUserRepository.findAppUserByName(username); - if (current != null && !current.hasIdOf(userId)) { - final String errorMessage = "User with username '" + username + "' already exists."; - baseDataValidator.reset().parameter("username").failWithMessage("constraint.violation.duplicate.username", errorMessage); - } } if (this.fromApiJsonHelper.parameterExists("firstname", element)) {
