Juan Hernandez has uploaded a new change for review. Change subject: core: Don't duplicate directory name in the DB ......................................................................
core: Don't duplicate directory name in the DB Currently in the "users" table we store the directory name in the "domain" column and also, as a suffix added to the user name, in the in the "username" column. This duplication causes confusion, and makes the SQL schema a bit more non normalized. This patch removes that duplication and changes the places that need to display the directory name (GUI and RESTAPI) so that it is done explicitly. Bug-Url: https://bugzilla.redhat.com/1055881 Bug-Url: https://bugzilla.redhat.com/1059258 Change-Id: I53ebaf3254f829d73b2574230ae333f13cfcd546 Signed-off-by: Juan Hernandez <juan.hernan...@redhat.com> --- M backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/UserMapper.java M backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/BaseConditionFieldAutoCompleter.java M backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/VdcUserConditionFieldAutoCompleter.java A frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/utils/FormatUtils.java M frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/auth/CurrentUser.java M frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/MainTabUserView.java M packaging/dbscripts/common_sp.sql M packaging/dbscripts/create_functions.sql A packaging/dbscripts/upgrade/03_05_0010_remove_domain_from_user_name.sql 9 files changed, 96 insertions(+), 36 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/02/23902/1 diff --git a/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/UserMapper.java b/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/UserMapper.java index c36002e..608dffa 100644 --- a/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/UserMapper.java +++ b/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/UserMapper.java @@ -17,7 +17,7 @@ User model = template != null ? template : new User(); model.setExternalId(entity.getExternalId().toHex()); model.setName(entity.getFirstName()); - model.setUserName(entity.getLoginName()); + model.setUserName(entity.getLoginName() + "@" + entity.getDomain()); model.setId(entity.getId().toString()); model.setLastName(entity.getLastName()); model.setEmail(entity.getEmail()); @@ -42,7 +42,7 @@ public static User map(DirectoryUser entity, User template) { User model = template != null ? template : new User(); model.setName(entity.getFirstName()); - model.setUserName(entity.getName()); + model.setUserName(entity.getName() + "@" + entity.getDirectory().getName()); model.setId(entity.getId().toHex()); model.setLastName(entity.getLastName()); model.setEmail(entity.getEmail()); diff --git a/backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/BaseConditionFieldAutoCompleter.java b/backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/BaseConditionFieldAutoCompleter.java index 9182b3a..3375b9f 100644 --- a/backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/BaseConditionFieldAutoCompleter.java +++ b/backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/BaseConditionFieldAutoCompleter.java @@ -363,7 +363,7 @@ } @Override - public final String buildConditionSql(String fieldName, String customizedValue, String customizedRelation, + public String buildConditionSql(String fieldName, String customizedValue, String customizedRelation, String tableName, boolean caseSensitive) { Pair<String, String> pair = new Pair<String, String>(); pair.setFirst(customizedRelation); diff --git a/backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/VdcUserConditionFieldAutoCompleter.java b/backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/VdcUserConditionFieldAutoCompleter.java index 04abcc4..9bb7ac1 100644 --- a/backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/VdcUserConditionFieldAutoCompleter.java +++ b/backend/manager/modules/searchbackend/src/main/java/org/ovirt/engine/core/searchbackend/VdcUserConditionFieldAutoCompleter.java @@ -12,6 +12,8 @@ mVerbs.add("NAME"); mVerbs.add("LASTNAME"); mVerbs.add("USRNAME"); + mVerbs.add("LOGIN"); + mVerbs.add("DIRECTORY"); mVerbs.add("DEPARTMENT"); mVerbs.add("GROUP"); mVerbs.add("TITLE"); @@ -27,6 +29,8 @@ getTypeDictionary().put("NAME", String.class); getTypeDictionary().put("LASTNAME", String.class); getTypeDictionary().put("USRNAME", String.class); + getTypeDictionary().put("LOGIN", String.class); + getTypeDictionary().put("DIRECTORY", String.class); getTypeDictionary().put("DEPARTMENT", String.class); getTypeDictionary().put("TITLE", String.class); getTypeDictionary().put("GROUP", String.class); @@ -40,6 +44,8 @@ columnNameDict.put("NAME", "name"); columnNameDict.put("LASTNAME", "surname"); columnNameDict.put("USRNAME", "username"); + columnNameDict.put("LOGIN", "username"); + columnNameDict.put("DIRECTORY", "domain"); columnNameDict.put("DEPARTMENT", "department"); columnNameDict.put("TITLE", "role"); columnNameDict.put("GROUP", "groups"); @@ -60,4 +66,45 @@ return StringConditionRelationAutoCompleter.INSTANCE; } } + + @Override + public String buildConditionSql( + String fieldName, + String customizedValue, + String customizedRelation, + String tableName, + boolean caseSensitive) { + if ("USRNAME".equals(fieldName) && customizedValue.contains("@")) { + // When the given user name contains the at sign, we need to split it and compare it to two columns in the + // database: the column containing the login name of the user and the column containg the name of the + // directory. + int index = customizedValue.lastIndexOf("@"); + String loginValue = customizedValue.substring(0, index) + "'"; + String directoryValue = "'" + customizedValue.substring(index + 1); + String loginSql = buildConditionSql( + "LOGIN", + loginValue, + customizedRelation, + tableName, + caseSensitive + ); + String directorySql = buildConditionSql( + "DIRECTORY", + directoryValue, + customizedRelation, + tableName, + caseSensitive + ); + return "(" + loginSql + " AND " + directorySql + ")"; + } + else { + return super.buildConditionSql( + fieldName, + customizedValue, + customizedRelation, + tableName, + caseSensitive + ); + } + } } diff --git a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/utils/FormatUtils.java b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/utils/FormatUtils.java new file mode 100644 index 0000000..a5e5541 --- /dev/null +++ b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/utils/FormatUtils.java @@ -0,0 +1,34 @@ +package org.ovirt.engine.ui.frontend.utils; + +import org.ovirt.engine.core.common.businessentities.DbUser; + +/** + * This class contains static methods useful for formatting text for display in the GUI. + */ +public class FormatUtils { + /** + * Format the name of a user for display using the login name, followed by the at sign and the name of the + * directory. + * + * @param user the user object + */ + public static String getFullLoginName(DbUser user) { + return getFullLoginName(user.getLoginName(), user.getDomain()); + } + + /** + * Format the name of a user for display using the login name, followed by the at sign and the name of the + * directory. + * + * @param loginName the login name of the user + * @param directoryName the name of the directory + */ + public static String getFullLoginName(String loginName, String directoryName) { + if (directoryName != null && directoryName.length() > 0) { + return loginName + "@" + directoryName; //$NON-NLS-1$ + } + else { + return loginName; + } + } +} diff --git a/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/auth/CurrentUser.java b/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/auth/CurrentUser.java index da50959..2ec2c66 100644 --- a/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/auth/CurrentUser.java +++ b/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/auth/CurrentUser.java @@ -6,6 +6,7 @@ import com.google.gwt.event.shared.GwtEvent; import com.google.gwt.event.shared.HasHandlers; import com.google.inject.Inject; +import org.ovirt.engine.ui.frontend.utils.FormatUtils; /** * Holds data relevant for the current user. @@ -75,14 +76,7 @@ * Returns full user name ({@code user@domain}) if the user is currently logged in, {@code null} otherwise. */ public String getFullUserName() { - String userName = getUserName(); - String domain = getDomain(); - - if (userName != null && !userName.contains("@") && domain != null) { //$NON-NLS-1$ - return userName + "@" + domain; //$NON-NLS-1$ - } - - return userName; + return isLoggedIn()? FormatUtils.getFullLoginName(loggedUser): null; } public boolean isAutoLogin() { diff --git a/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/MainTabUserView.java b/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/MainTabUserView.java index 5aa8227..14da0df 100644 --- a/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/MainTabUserView.java +++ b/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/MainTabUserView.java @@ -4,6 +4,7 @@ import org.ovirt.engine.ui.common.idhandler.ElementIdHandler; import org.ovirt.engine.ui.common.uicommon.model.MainModelProvider; import org.ovirt.engine.ui.common.widget.table.column.TextColumnWithTooltip; +import org.ovirt.engine.ui.frontend.utils.FormatUtils; import org.ovirt.engine.ui.uicommonweb.UICommand; import org.ovirt.engine.ui.uicommonweb.models.users.UserListModel; import org.ovirt.engine.ui.webadmin.ApplicationConstants; @@ -51,7 +52,7 @@ getTable().addColumn(new TextColumnWithTooltip<DbUser>() { @Override public String getValue(DbUser object) { - return object.getLoginName(); + return FormatUtils.getFullLoginName(object); } }, constants.userNameUser(), "150px"); //$NON-NLS-1$ diff --git a/packaging/dbscripts/common_sp.sql b/packaging/dbscripts/common_sp.sql index ca00f20..426a8a7 100644 --- a/packaging/dbscripts/common_sp.sql +++ b/packaging/dbscripts/common_sp.sql @@ -206,7 +206,6 @@ v_user_id UUID; v_name VARCHAR(255); v_domain VARCHAR(255); - v_user_name VARCHAR(255); v_document VARCHAR(64); v_index INTEGER; @@ -227,17 +226,7 @@ v_name := substring( v_name from v_index + 1 ); end if; --- find if name already includes domain (@) - v_index := POSITION('@' IN v_name); - - if (v_index = 0) then - v_user_name := coalesce(v_name,'') || '@' || coalesce(v_domain,''); - else - v_user_name := v_name; - end if; - - -insert into users(user_id,name,domain,username,groups,active) select v_user_id, v_name, v_domain, v_user_name,'',true where not exists (select user_id,name,domain,username,groups,active from users where user_id = v_user_id and name = v_name and domain = v_domain and username = v_user_name and groups = '' and active); +insert into users(user_id,name,domain,username,groups,active) select v_user_id, v_name, v_domain, v_name,'',true where not exists (select user_id,name,domain,username,groups,active from users where user_id = v_user_id and name = v_name and domain = v_domain and username = v_name and groups = '' and active); insert into permissions(id,role_id,ad_element_id,object_id,object_type_id) select v_permission_id, '00000000-0000-0000-0000-000000000001', v_user_id, getGlobalIds('system'), 1 where not exists(select role_id,ad_element_id,object_id,object_type_id from permissions where role_id = '00000000-0000-0000-0000-000000000001' and ad_element_id = v_user_id and object_id= getGlobalIds('system') and object_type_id = 1); END; $procedure$ @@ -317,27 +306,17 @@ RETURNS void AS $BODY$ DECLARE - v_user_name VARCHAR(255); v_document VARCHAR(64); - v_index INTEGER; input_uuid uuid; v_external_id BYTEA; BEGIN input_uuid = CAST( v_user_id AS uuid ); --- find if name already includes domain (@) - v_index := POSITION('@' IN v_name); - - if (v_index = 0) then - v_user_name := coalesce(v_name,'') || '@' || coalesce(v_domain,''); - else - v_user_name := v_name; - end if; -- The external identifier is the user identifier converted to an array of -- bytes: v_external_id := decode(replace(v_user_id::text, '-', ''), 'hex'); -insert into users(user_id,external_id,name,domain,username,groups,active) select input_uuid, v_external_id, v_name, v_domain, v_user_name,'',true where not exists (select user_id,name,domain,username,groups,active from users where user_id = input_uuid); +insert into users(user_id,external_id,name,domain,username,groups,active) select input_uuid, v_external_id, v_name, v_domain, v_name,'',true where not exists (select user_id,name,domain,username,groups,active from users where user_id = input_uuid); insert into permissions(id,role_id,ad_element_id,object_id,object_type_id) select v_permission_id, '00000000-0000-0000-0000-000000000001', input_uuid, getGlobalIds('system'), 1 where not exists(select role_id,ad_element_id,object_id,object_type_id from permissions where role_id = '00000000-0000-0000-0000-000000000001' and ad_element_id = input_uuid and object_id= getGlobalIds('system') and object_type_id = 1); END; $BODY$ diff --git a/packaging/dbscripts/create_functions.sql b/packaging/dbscripts/create_functions.sql index a808589..392342b 100644 --- a/packaging/dbscripts/create_functions.sql +++ b/packaging/dbscripts/create_functions.sql @@ -463,7 +463,7 @@ if (v_ad_element_id = getGlobalIds('everyone')) then result := 'Everyone'; else - select(COALESCE(name,'') || ' ' || COALESCE(surname,'') || ' (' || COALESCE(username,'') || ')') INTO result from users where user_id = v_ad_element_id; + select(COALESCE(name,'') || ' ' || COALESCE(surname,'') || ' (' || COALESCE(username,'') || '@' || COALESCE(domain,'') || ')') INTO result from users where user_id = v_ad_element_id; if (result is null) then select name INTO result from ad_groups where ID = v_ad_element_id; end if; diff --git a/packaging/dbscripts/upgrade/03_05_0010_remove_domain_from_user_name.sql b/packaging/dbscripts/upgrade/03_05_0010_remove_domain_from_user_name.sql new file mode 100644 index 0000000..b770df1 --- /dev/null +++ b/packaging/dbscripts/upgrade/03_05_0010_remove_domain_from_user_name.sql @@ -0,0 +1,5 @@ +-- +-- Remove the @directory suffix from "username" column of the "users" table, as +-- this value is already stored in the "domain" column of the same table: +-- +update users set username = regexp_replace(username, '@.*', ''); -- To view, visit http://gerrit.ovirt.org/23902 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I53ebaf3254f829d73b2574230ae333f13cfcd546 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Juan Hernandez <juan.hernan...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
