Bug 969114 - Refactor unnecessary while loop in JndiAction class
Refactor unnecessary while loop in JndiAction class
Status: CLOSED UPSTREAM
Product: Red Hat Enterprise Virtualization Manager
Classification: Red Hat
Component: ovirt-engine (Show other bugs)
3.3.0
Unspecified Unspecified
medium Severity medium
: ---
: 3.3.0
Assigned To: Martin Perina
movciari
infra
: CodeChange, Triaged
Depends On: 968958
Blocks:
  Show dependency treegraph
 
Reported: 2013-05-30 12:50 EDT by Tomas Dosek
Modified: 2016-02-10 14:34 EST (History)
11 users (show)

See Also:
Fixed In Version: is8
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: 968958
Environment:
Last Closed: 2013-08-12 03:00:44 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: Infra
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)


External Trackers
Tracker ID Priority Status Summary Last Updated
oVirt gerrit 15217 None None None Never

  None (edit)
Description Tomas Dosek 2013-05-30 12:50:22 EDT
+++ 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 09:53:13 EDT
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@AD2.RHEV.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@BRQ-IPA.RHEV.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@BRQ-OPENLDAP.RHEV.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.