Hello Yair Zaslavsky,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/31551
to review the following change.
Change subject: aaa: Remove "active" from groups
......................................................................
aaa: Remove "active" from groups
New optimized and reduced user sync algorithm, is syncing users and
the groups users actually belongs to.
This means that if there is group persisted at the database which
does not exist at the authz provider, it will be never reached and
hence not synchronized.
As result, the active/inactive flag in this case is useless as it will
be never changed, nor has any effect.
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1120720
Change-Id: Id12a172273228609715c7d09c4d173a83478c82c
Topic: AAA
Signed-off-by: Yair Zaslavsky <[email protected]>
---
M
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/aaa/DbGroup.java
M
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/aaa/LdapGroup.java
M
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/DbGroupDAODbFacadeImpl.java
M
backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/DbGroupDAOTest.java
M backend/manager/modules/dal/src/test/resources/fixtures.xml
M
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/users/UserListModel.java
M
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/user/SubTabUserGeneralView.java
M packaging/dbscripts/ad_groups_sp.sql
M packaging/dbscripts/create_views.sql
A packaging/dbscripts/upgrade/03_05_0890_remove_active_column_from_groups.sql
10 files changed, 23 insertions(+), 66 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/51/31551/1
diff --git
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/aaa/DbGroup.java
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/aaa/DbGroup.java
index 05650ed..76d4425 100644
---
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/aaa/DbGroup.java
+++
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/aaa/DbGroup.java
@@ -26,12 +26,6 @@
private String distinguishedName;
private Set<String> memberOf;
- /**
- * This flag indicates if the user was available in the directory the last
time that it was checked, so {@code true}
- * means it was available and {@code false} means it wasn't.
- */
- private boolean active;
-
public DbGroup() {
memberOf = new HashSet<String>();
}
@@ -84,14 +78,6 @@
name = value;
}
- public boolean isActive() {
- return active;
- }
-
- public void setActive(boolean value) {
- active = value;
- }
-
public void setDistinguishedName(String distinguishedName) {
this.distinguishedName = distinguishedName;
}
@@ -124,7 +110,6 @@
result = prime * result + ((name == null) ? 0 : name.hashCode());
result = prime * result + ((distinguishedName == null) ? 0 :
distinguishedName.hashCode());
result = prime * result + ((memberOf == null) ? 0 :
memberOf.hashCode());
- result = prime * result + (active ? 1231 : 1237);
return result;
}
@@ -145,7 +130,6 @@
&& ObjectUtils.objectsEqual(domain, other.domain)
&& ObjectUtils.objectsEqual(name, other.name)
&& ObjectUtils.objectsEqual(distinguishedName,
other.distinguishedName)
- && ObjectUtils.objectsEqual(memberOf, other.memberOf)
- && active == other.active);
+ && ObjectUtils.objectsEqual(memberOf, other.memberOf));
}
}
diff --git
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/aaa/LdapGroup.java
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/aaa/LdapGroup.java
index 21d63fc..9d1538a 100644
---
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/aaa/LdapGroup.java
+++
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/aaa/LdapGroup.java
@@ -13,8 +13,6 @@
private String name;
- private boolean active;
-
private String domain;
private List<String> memberOf;
@@ -22,7 +20,6 @@
private String distinguishedName;
public LdapGroup() {
- active = true;
name = "";
distinguishedName = "";
}
@@ -31,7 +28,6 @@
id = dbGroup.getExternalId();
name = dbGroup.getName();
domain = dbGroup.getDomain();
- active = dbGroup.isActive();
distinguishedName = dbGroup.getDistinguishedName();
memberOf = dbGroup.getMemberOf() != null ? new
ArrayList<String>(dbGroup.getMemberOf()) : null;
}
@@ -52,14 +48,6 @@
this.name = value;
}
- public boolean isActive() {
- return active;
- }
-
- public void setActive(boolean value) {
- active = value;
- }
-
public LdapGroup(String id, String name, String domain) {
this (id);
this.name = name;
@@ -72,14 +60,8 @@
this.setMemberOf(memberOf);
}
- /**
- * This constructor used only for Inactive groups
- *
- * @param id
- */
public LdapGroup(String id) {
this.id = id;
- active = false;
}
public String getdomain() {
@@ -120,7 +102,6 @@
result = prime * result + ((domain == null) ? 0 : domain.hashCode());
result = prime * result + ((memberOf == null) ? 0 :
memberOf.hashCode());
result = prime * result + ((name == null) ? 0 : name.hashCode());
- result = prime * result + (active ? 1231 : 1237);
return result;
}
@@ -140,7 +121,6 @@
&& ObjectUtils.objectsEqual(distinguishedName,
other.distinguishedName)
&& ObjectUtils.objectsEqual(domain, other.domain)
&& ObjectUtils.objectsEqual(memberOf, other.memberOf)
- && ObjectUtils.objectsEqual(name, other.name)
- && active == other.active);
+ && ObjectUtils.objectsEqual(name, other.name));
}
}
diff --git
a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/DbGroupDAODbFacadeImpl.java
b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/DbGroupDAODbFacadeImpl.java
index 2aa6672..be30cf8 100644
---
a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/DbGroupDAODbFacadeImpl.java
+++
b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/DbGroupDAODbFacadeImpl.java
@@ -101,7 +101,6 @@
getCallsHandler().executeModification(storedProcName,
getCustomMapSqlParameterSource()
.addValue("id", group.getId())
.addValue("name", group.getName())
- .addValue("active", group.isActive())
.addValue("domain", group.getDomain())
.addValue("distinguishedname", group.getDistinguishedName())
.addValue("external_id", group.getExternalId())
@@ -124,7 +123,6 @@
DbGroup entity = new DbGroup();
entity.setId(getGuidDefaultEmpty(rs, "id"));
entity.setName(rs.getString("name"));
- entity.setActive(rs.getBoolean("active"));
entity.setDomain(rs.getString("domain"));
entity.setDistinguishedName(rs.getString("distinguishedname"));
entity.setExternalId(rs.getString("external_id"));
diff --git
a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/DbGroupDAOTest.java
b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/DbGroupDAOTest.java
index 4a9c2ec..31c3602 100644
---
a/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/DbGroupDAOTest.java
+++
b/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/DbGroupDAOTest.java
@@ -33,7 +33,6 @@
newGroup.setExternalId("0");
newGroup.setDomain("domain");
newGroup.setName("name");
- newGroup.setActive(true);
newGroup.setNamespace("*");
existingGroup = dao.get(new
Guid("b399944a-81ab-4ec5-8266-e19ba7c3c9d1"));
}
@@ -139,8 +138,6 @@
public void testUpdate() {
existingGroup.setName(existingGroup.getName().toUpperCase());
existingGroup.setDomain(existingGroup.getDomain().toUpperCase());
- existingGroup.setActive(false);
-
dao.update(existingGroup);
DbGroup result = dao.get(existingGroup.getId());
diff --git a/backend/manager/modules/dal/src/test/resources/fixtures.xml
b/backend/manager/modules/dal/src/test/resources/fixtures.xml
index 3b614d5..2f218e3 100644
--- a/backend/manager/modules/dal/src/test/resources/fixtures.xml
+++ b/backend/manager/modules/dal/src/test/resources/fixtures.xml
@@ -391,13 +391,11 @@
<column>id</column>
<column>external_id</column>
<column>name</column>
- <column>active</column>
<column>domain</column>
<row>
<value>b399944a-81ab-4ec5-8266-e19ba7c3c9d1</value>
<value>a</value>
<value>administrators</value>
- <value>true</value>
<value>rhel</value>
<value>*</value>
</row>
@@ -405,7 +403,6 @@
<value>b399944a-81ab-4ec5-8266-e19ba7c3c9d2</value>
<value>b</value>
<value>[email protected]/test-all-users</value>
- <value>true</value>
<value>rhel</value>
<value>*</value>
</row>
@@ -413,7 +410,6 @@
<value>b399944a-81ab-4ec5-8266-e19ba7c3c9d3</value>
<value>c</value>
<value>[email protected]/test-all-users</value>
- <value>true</value>
<value>rhel</value>
<value>*</value>
</row>
@@ -421,7 +417,6 @@
<value>b399944a-81ab-4ec5-8266-e19ba7c3c9d4</value>
<value>d</value>
<value>[email protected]/test-all-users</value>
- <value>true</value>
<value>rhel</value>
<value>*</value>
</row>
@@ -429,7 +424,6 @@
<value>b399944a-81ab-4ec5-8266-e19ba7c3c9d5</value>
<value>e</value>
<value>[email protected]/test-all-users</value>
- <value>true</value>
<value>rhel</value>
<value>*</value>
</row>
@@ -437,7 +431,6 @@
<value>b399944a-81ab-4ec5-8266-e19ba7c3c9d6</value>
<value>f</value>
<value>[email protected]/test-all-users</value>
- <value>true</value>
<value>rhel</value>
<value>*</value>
</row>
@@ -445,7 +438,6 @@
<value>b399944a-81ab-4ec5-8266-e19ba7c3c9d7</value>
<value>g</value>
<value>[email protected]/test-all-users</value>
- <value>true</value>
<value>rhel</value>
<value>*</value>
</row>
@@ -453,7 +445,6 @@
<value>b399944a-81ab-4ec5-8266-e19ba7c3c9d8</value>
<value>h</value>
<value>[email protected]/test-all-users</value>
- <value>true</value>
<value>rhel</value>
<value>*</value>
</row>
@@ -461,7 +452,6 @@
<value>b399944a-81ab-4ec5-8266-e19ba7c3c9d9</value>
<value>i</value>
<value>[email protected]/test-all-users</value>
- <value>true</value>
<value>rhel</value>
<value>*</value>
</row>
@@ -469,7 +459,6 @@
<value>9bf7c640-b620-456f-a550-0348f366544b</value>
<value>j</value>
<value>philosophers</value>
- <value>true</value>
<value>rhel</value>
<value>*</value>
</row>
diff --git
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/users/UserListModel.java
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/users/UserListModel.java
index 284ea54..47b2212 100644
---
a/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/users/UserListModel.java
+++
b/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/users/UserListModel.java
@@ -469,7 +469,6 @@
grp.setNamespace(item.getNamespace());
grp.setId(item.getId());
grp.setDomain(item.getDomain());
- grp.setActive(item.isActive());
parameters = new AddGroupParameters(grp);
}
else
diff --git
a/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/user/SubTabUserGeneralView.java
b/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/user/SubTabUserGeneralView.java
index 8501b4a..2cc3877 100644
---
a/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/user/SubTabUserGeneralView.java
+++
b/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/user/SubTabUserGeneralView.java
@@ -58,18 +58,28 @@
formBuilder = new FormBuilder(formPanel, 1, 3);
formBuilder.addFormItem(new FormItem(constants.domainUserGeneral(),
domain, 0, 0));
- formBuilder.addFormItem(new FormItem(constants.activeUserGeneral(),
active, 1, 0));
+ formBuilder.addFormItem(new FormItem(constants.activeUserGeneral(),
active, 1, 0) {
+ @Override
+ public boolean getIsAvailable() {
+ return isUserElement(getDetailModel());
+ }
+ });
formBuilder.addFormItem(new FormItem(constants.emailUserGeneral(),
email, 2, 0) {
@Override
public boolean getIsAvailable() {
- if (getDetailModel().getEntity() == null) {
- return false;
- }
- return !((DbUser) getDetailModel().getEntity()).isGroup();
+ return isUserElement(getDetailModel());
}
});
}
+ private boolean isUserElement(UserGeneralModel userGeneralModel) {
+ if (getDetailModel().getEntity() == null) {
+ return false;
+ }
+ return !((DbUser) getDetailModel().getEntity()).isGroup();
+
+ }
+
@Override
public void setMainTabSelectedItem(DbUser selectedItem) {
driver.edit(getDetailModel());
diff --git a/packaging/dbscripts/ad_groups_sp.sql
b/packaging/dbscripts/ad_groups_sp.sql
index a81188d..4ccca21 100644
--- a/packaging/dbscripts/ad_groups_sp.sql
+++ b/packaging/dbscripts/ad_groups_sp.sql
@@ -9,7 +9,6 @@
Create or replace FUNCTION InsertGroup(v_id UUID,
v_name VARCHAR(255),
- v_active BOOLEAN,
v_domain VARCHAR(100),
v_distinguishedname VARCHAR(4000),
v_external_id TEXT,
@@ -17,8 +16,8 @@
RETURNS VOID
AS $procedure$
BEGIN
-INSERT INTO ad_groups(id, name,active,domain,distinguishedname,external_id,
namespace)
- VALUES(v_id,
v_name,v_active,v_domain,v_distinguishedname,v_external_id, v_namespace);
+INSERT INTO ad_groups(id, name,domain,distinguishedname,external_id, namespace)
+ VALUES(v_id, v_name,v_domain,v_distinguishedname,v_external_id,
v_namespace);
END; $procedure$
LANGUAGE plpgsql;
@@ -28,7 +27,6 @@
Create or replace FUNCTION UpdateGroup(v_id UUID,
v_name VARCHAR(255),
- v_active BOOLEAN,
v_domain VARCHAR(100),
v_distinguishedname VARCHAR(4000),
v_external_id TEXT,
@@ -39,7 +37,7 @@
AS $procedure$
BEGIN
UPDATE ad_groups
- SET name = v_name,active = v_active,domain = v_domain,distinguishedname
= v_distinguishedname,external_id = v_external_id, namespace = v_namespace
+ SET name = v_name,domain = v_domain,distinguishedname =
v_distinguishedname,external_id = v_external_id, namespace = v_namespace
WHERE id = v_id;
END; $procedure$
LANGUAGE plpgsql;
diff --git a/packaging/dbscripts/create_views.sql
b/packaging/dbscripts/create_views.sql
index a64baa0..ab58601 100644
--- a/packaging/dbscripts/create_views.sql
+++ b/packaging/dbscripts/create_views.sql
@@ -846,7 +846,7 @@
FROM users AS users_1
UNION
SELECT 'group' as user_group, ad_groups.name as name, ad_groups.id as id,
'' as surname, ad_groups.domain as domain, '' as username, '' as groups, '' as
department, '' as role,
- '' as email, '' as note, ad_groups.active as active, 1
as vm_admin, null as last_admin_check_status, '' as group_ids,
+ '' as email, '' as note, true as active, 1 as vm_admin,
null as last_admin_check_status, '' as group_ids,
ad_groups.external_id as external_id,
ad_groups.namespace as namespace
FROM ad_groups;
diff --git
a/packaging/dbscripts/upgrade/03_05_0890_remove_active_column_from_groups.sql
b/packaging/dbscripts/upgrade/03_05_0890_remove_active_column_from_groups.sql
new file mode 100644
index 0000000..5022842
--- /dev/null
+++
b/packaging/dbscripts/upgrade/03_05_0890_remove_active_column_from_groups.sql
@@ -0,0 +1,2 @@
+select fn_db_drop_column ('ad_groups', 'active');
+
--
To view, visit http://gerrit.ovirt.org/31551
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id12a172273228609715c7d09c4d173a83478c82c
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: ovirt-engine-3.5
Gerrit-Owner: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
