Bug 969114

Summary: Refactor unnecessary while loop in JndiAction class
Product: Red Hat Enterprise Virtualization Manager Reporter: Tomas Dosek <tdosek>
Component: ovirt-engineAssignee: Martin Perina <mperina>
Status: CLOSED UPSTREAM QA Contact: movciari
Severity: medium Docs Contact:
Priority: medium    
Version: 3.3.0CC: acathrow, bazulay, iheim, jkt, juan.hernandez, lpeer, mperina, pstehlik, Rhev-m-bugs, yeylon, yzaslavs
Target Milestone: ---Keywords: CodeChange, Triaged
Target Release: 3.3.0   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard: infra
Fixed In Version: is8 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: 968958 Environment:
Last Closed: 2013-08-12 07:00:44 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: Infra RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On: 968958    
Bug Blocks:    

Description Tomas Dosek 2013-05-30 16:50:22 UTC
+++ This bug was initially created as a clone of Bug #968958 +++

Description of problem:

Please refactor following unnecessary while loop:

"
                    while (answer.hasMoreElements()) {
                        // Print the objectGUID for the user as well as URI and query path
                        String guid = guidFromResults(answer.next());
                        if (guid == null) {
                            break;
                        }
                        userGuid.append(guid);
                        logQueryContext(userGuid.toString(), uri.toString(), currentLdapServer);
                        return AuthenticationResult.OK;
                    }

"

In any case this loop runs only once so the problem solved by it can be
replaced by "if" decission statement.

Version-Release number of selected component (if applicable):
Any

--- Additional comment from Tomas Dosek on 2013-05-30 07:31:13 EDT ---

Proposed patch:

if (answer.hasMoreElements()) {
    String guid = guidFromResults(answer.next());
    if (guid != null) {   
         userGuid.append(guid);
         logQueryContext(userGuid.toString(), uri.toString(), currentLdapServer);
         return AuthenticationResult.OK;
    }
}

Where logQueryContext procedure got added by http://gerrit.ovirt.org/#/c/15176/
to replace current weak logging options.

--- Additional comment from Tomas Dosek on 2013-05-30 12:47:50 EDT ---

Propsed patch in gerrit: http://gerrit.ovirt.org/15217/

Comment 4 Martin Perina 2013-08-01 13:53:13 UTC
JndiAction class is used only when adding, editing or validating domain from engine-manage-domains tool.

I verified the change by adding and validating domain using engine-manage-domains for ActiveDirectory, IPA and OpenLDAP providers:

$ ./bin/engine-manage-domains -action=add -domain=ad2.rhev.lab.eng.brq.redhat.com -user=vdcadmin -interactive -addPermissions -provider=ActiveDirectory
Enter password:

Successfully added domain ad2.rhev.lab.eng.brq.redhat.com. oVirt Engine restart is required in order for the changes to take place (service ovirt-engine restart).
Manage Domains completed successfully

$ ./bin/engine-manage-domains -action=validate
Domain ad2.rhev.lab.eng.brq.redhat.com is valid.
The configured user for domain ad2.rhev.lab.eng.brq.redhat.com is vdcadmin.LAB.ENG.BRQ.REDHAT.COM
Manage Domains completed successfully




$ ./bin/engine-manage-domains -action=add -domain=brq-ipa.rhev.lab.eng.brq.redhat.com -user=vdcadmin -interactive -addPermissions -provider=IPA
Enter password:

Successfully added domain brq-ipa.rhev.lab.eng.brq.redhat.com. oVirt Engine restart is required in order for the changes to take place (service ovirt-engine restart).
Manage Domains completed successfully

$ ./bin/engine-manage-domains -action=validate
Domain brq-ipa.rhev.lab.eng.brq.redhat.com is valid.
The configured user for domain brq-ipa.rhev.lab.eng.brq.redhat.com is vdcadmin.LAB.ENG.BRQ.REDHAT.COM
Manage Domains completed successfully




$./bin/engine-manage-domains -action=add -domain=brq-openldap.rhev.lab.eng.brq.redhat.com -user=vdcadmin -interactive -addPermissions -provider=OpenLDAP
Enter password:

Successfully added domain brq-openldap.rhev.lab.eng.brq.redhat.com. oVirt Engine restart is required in order for the changes to take place (service ovirt-engine restart).
Manage Domains completed successfully

$ ./bin/engine-manage-domains -action=validate
Domain brq-openldap.rhev.lab.eng.brq.redhat.com is valid.
The configured user for domain brq-openldap.rhev.lab.eng.brq.redhat.com is vdcadmin.LAB.ENG.BRQ.REDHAT.COM
Manage Domains completed successfully