Bug 969114 - Refactor unnecessary while loop in JndiAction class
Summary: Refactor unnecessary while loop in JndiAction class
Keywords:
Status: CLOSED UPSTREAM
Alias: None
Product: Red Hat Enterprise Virtualization Manager
Classification: Red Hat
Component: ovirt-engine
Version: 3.3.0
Hardware: Unspecified
OS: Unspecified
medium
medium
Target Milestone: ---
: 3.3.0
Assignee: Martin Perina
QA Contact: movciari
URL:
Whiteboard: infra
Depends On: 968958
Blocks:
TreeView+ depends on / blocked
 
Reported: 2013-05-30 16:50 UTC by Tomas Dosek
Modified: 2016-02-10 19:34 UTC (History)
11 users (show)

Fixed In Version: is8
Doc Type: Bug Fix
Doc Text:
Clone Of: 968958
Environment:
Last Closed: 2013-08-12 07:00:44 UTC
oVirt Team: Infra
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
oVirt gerrit 15217 0 None None None Never

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


Note You need to log in before you can comment on or make changes to this bug.