Yair Zaslavsky has posted comments on this change.
Change subject: engine: Integrate Atlassian Crowd Client as a new
Authentication Domain
......................................................................
Patch Set 1: (4 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/CrowdBrokerCommandBase.java
Line 10: super(parameters);
Line 11: }
Line 12: @Override
Line 13: protected String getPROTOCOL() {
Line 14: return "Crowd";
I would recommend you use a constant here.
I'm also kinda surprised that getPROTOCOL is not camel-case, but that's an
issue for another patch
Line 15: }
Line 16:
Line 17: @Override
Line 18: public LdapReturnValueBase execute() {
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/CrowdBrokerUtils.java
Line 57: m_CrowdConfig.load(new FileReader(f));
Line 58: } catch (Exception e) {
Line 59: log.warnFormat("Failed to load Crowd Configuration
from file {0}{1}: {2}",
Line 60: "/tmp/",
Line 61: "crowd.properties",
+1 on that comment - this is not so clear.
Line 62: e.getMessage());
Line 63: return false;
Line 64: }
Line 65: ClientPropertiesImpl crowdClientProperties =
ClientPropertiesImpl.newInstanceFromProperties(m_CrowdConfig);
Line 196: */
Line 197: public static AdUser getUserByUPN(String userName) {
Line 198: AdUser retVal = null;
Line 199: UserWithAttributes user;
Line 200: initCrowd();
I would consider not to call initCrowd in each method.
Maybe in static initializer, or some other way.
Line 201: if (userName.matches(".+@.+")) {
Line 202: String[] loginNameParts = userName.split("@");
Line 203: userName = loginNameParts[0];
Line 204: }
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/CrowdSearchGroupsByQueryCommand.java
Line 9: }
Line 10:
Line 11: @Override
Line 12: protected void ExecuteQuery() {
Line 13: java.util.List<ad_groups> groupList =
CrowdBrokerUtils.getAllGroups();
You might need to get into the search entities code, and see how to construct
search queries, based on the search entities code.
Line 14: setReturnValue(groupList);
Line 15: setSucceeded(true);
Line 16: }
Line 17:
--
To view, visit http://gerrit.ovirt.org/9324
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ide867f16d092eb329c0ce2fccf4ebd02f3aae0df
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Justin Hammond <[email protected]>
Gerrit-Reviewer: Doron Fediuck <[email protected]>
Gerrit-Reviewer: Ewoud Kohl van Wijngaarden <[email protected]>
Gerrit-Reviewer: Itamar Heim <[email protected]>
Gerrit-Reviewer: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Justin Hammond <[email protected]>
Gerrit-Reviewer: Laszlo Hornyak <[email protected]>
Gerrit-Reviewer: Oved Ourfali <[email protected]>
Gerrit-Reviewer: Roy Golan <[email protected]>
Gerrit-Reviewer: Sharad Mishra <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
