Hello Yair Zaslavsky,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/30487
to review the following change.
Change subject: aaa: more fixes to sync and usages
......................................................................
aaa: more fixes to sync and usages
More fixes were introcued to sync and usages- especially around
adding an already added group/principal.
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1120720
Change-Id: I0d6198409b7c3e66054716e1abdfcd06e8dd204d
Signed-off-by: Yair Zaslavsky <[email protected]>
---
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddGroupCommand.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddUserCommand.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SyncUsers.java
M packaging/dbscripts/user_sp.sql
4 files changed, 29 insertions(+), 23 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/87/30487/1
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddGroupCommand.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddGroupCommand.java
index ce75c35..bc82f07 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddGroupCommand.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddGroupCommand.java
@@ -47,6 +47,7 @@
}
else {
dao.update(dbGroup);
+ groupToAdd = dbGroup;
}
// Return the identifier of the created group:
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddUserCommand.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddUserCommand.java
index 3d5633b..a1a4434 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddUserCommand.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddUserCommand.java
@@ -1,6 +1,5 @@
package org.ovirt.engine.core.bll;
-import java.util.Arrays;
import java.util.Collections;
import java.util.List;
@@ -10,7 +9,6 @@
import org.ovirt.engine.core.common.VdcObjectType;
import org.ovirt.engine.core.common.action.AddUserParameters;
import org.ovirt.engine.core.common.businessentities.DbUser;
-import org.ovirt.engine.core.compat.Guid;
import org.ovirt.engine.core.dal.dbbroker.DbFacade;
public class AddUserCommand<T extends AddUserParameters> extends
CommandBase<T> {
@@ -38,20 +36,21 @@
@Override
protected void executeCommand() {
- DbUser userToAdd = getParameters().getUserToAdd();
- for (DbUser syncedUser : SyncUsers.sync(Arrays.asList(userToAdd))) {
- if (Guid.isNullOrEmpty(syncedUser.getId())) {
- if (syncedUser.isActive()) {
- DbFacade.getInstance().getDbUserDao().save(syncedUser);
- }
- } else {
- DbFacade.getInstance().getDbUserDao().update(syncedUser);
- }
- }
+ DbUser user = getParameters().getUserToAdd();
+ DbUser syncResult = SyncUsers.sync(user);
+ user = syncResult != null ? syncResult : user;
DbUser userFromDb =
-
DbFacade.getInstance().getDbUserDao().getByExternalId(userToAdd.getDomain(),
userToAdd.getExternalId());
- setActionReturnValue(userFromDb.getId());
- setSucceeded(userFromDb.isActive());
+
DbFacade.getInstance().getDbUserDao().getByExternalId(user.getDomain(),
user.getExternalId());
+ if (userFromDb == null) {
+ if (user.isActive()) {
+ DbFacade.getInstance().getDbUserDao().save(user);
+ }
+ } else {
+ user.setId(userFromDb.getId());
+ DbFacade.getInstance().getDbUserDao().update(user);
+ }
+ setActionReturnValue(user.getId());
+ setSucceeded(user.isActive());
}
@Override
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SyncUsers.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SyncUsers.java
index bce943e..1ab64fd 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SyncUsers.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SyncUsers.java
@@ -1,6 +1,7 @@
package org.ovirt.engine.core.bll;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@@ -20,6 +21,11 @@
public class SyncUsers {
private static final Log log = LogFactory.getLog(SyncUsers.class);
+
+ public static DbUser sync(DbUser dbUser) {
+ List<DbUser> synchedUsers = sync(Arrays.asList(dbUser));
+ return synchedUsers.isEmpty() ? null : synchedUsers.get(0);
+ }
public static List<DbUser> sync(List<DbUser> dbUsers) {
List<DbUser> usersToUpdate = new ArrayList<>();
@@ -61,22 +67,22 @@
if (activeUser != null) {
if (!activeUser.equals(dbUser)) {
activeUser.setId(dbUser.getId());
- log.info(String.format("The user %1$s from authz
extension %2$s got updated since last interval",
+ log.infoFormat("Principal {0}::{1} synchronized",
activeUser.getLoginName(),
- activeUser.getDomain()));
+ activeUser.getDomain());
usersToUpdate.add(activeUser);
}
} else {
- log.info(String.format("The user %1$s from authz
extension %2$s could not be found, and will be marked as inactive",
+ log.infoFormat("Deactivating non existing principal
{0}::{1}",
dbUser.getLoginName(),
- dbUser.getDomain()));
+ dbUser.getDomain());
dbUser.setActive(false);
usersToUpdate.add(dbUser);
}
}
} catch (Exception ex) {
- log.error(String.format("Error during user synchronization of
extension %1$s. Exception message is %2$s",
- authz, ex.getMessage()));
+ log.errorFormat("Error during user synchronization of
extension {0}. Exception message is {1}",
+ authz, ex.getMessage());
log.debug("", ex);
}
}
diff --git a/packaging/dbscripts/user_sp.sql b/packaging/dbscripts/user_sp.sql
index 5b49e4f..c4c8379 100644
--- a/packaging/dbscripts/user_sp.sql
+++ b/packaging/dbscripts/user_sp.sql
@@ -62,7 +62,7 @@
external_id = v_external_id,
namespace = v_namespace,
_update_date = CURRENT_TIMESTAMP
- WHERE user_id = v_user_id;
+ WHERE external_id = v_external_id AND domain = v_domain;
GET DIAGNOSTICS updated_rows = ROW_COUNT;
RETURN updated_rows;
@@ -95,7 +95,7 @@
PERFORM UpdateUserImpl(v_department, v_domain, v_email, v_groups,
v_name, v_note, v_role, v_active, v_surname, v_user_id, v_username,
v_group_ids, v_external_id, v_namespace);
UPDATE users SET
last_admin_check_status = v_last_admin_check_status
- WHERE user_id = v_user_id;
+ WHERE domain = v_domain AND external_id = v_external_id;
END; $procedure$
LANGUAGE plpgsql;
--
To view, visit http://gerrit.ovirt.org/30487
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0d6198409b7c3e66054716e1abdfcd06e8dd204d
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
