Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix find LdapTemplate, specifying return attributes. #569

Closed
wants to merge 3 commits into from

Conversation

orlandosso
Copy link
Contributor

@orlandosso orlandosso commented Feb 4, 2021

LdapQuery allows the user to define ToReturn attributes, but does not respect the attributes defined.

String [] atributtes = new String [] {"cn", "name", "email"};

var queryLdap = LdapQueryBuilder.query ()
        .attributes (attributtes)
        .base ("ou = company, o = department")
        . where ("cn"). is ("123456");

var user = ldapTemplate.findOne(queryLdap, UserEntity.class);

The attributes are just ignored, and overwritten by following code.

String[] attributes = this.odm.manageClass(clazz);
searchControls.setReturningAttributes(attributes);

The correction involves verifying that the attributes for return have been informed.

if (isNotCustomReturningAttributes(searchControls)) {
            String[] attributes = this.odm.manageClass(clazz);
            searchControls.setReturningAttributes(attributes);
}

@rwinch
Copy link
Member

rwinch commented Feb 5, 2021

Thanks for the PR. Can you please add test(s) to the PR?

Copy link
Member

@rwinch rwinch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add tests

@jzheaux
Copy link
Contributor

jzheaux commented Jan 19, 2022

Hi, @orlandosso, thanks for your contribute.

Are you able to address Rob's feedback? I can help sort things out, if not. If I haven't heard from you in a week or so, I'll apply the described changes myself.

@pivotal-cla
Copy link

@orlandosso Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@jzheaux jzheaux self-assigned this Jan 19, 2022
@jzheaux jzheaux added in: core type: bug A general bug labels Jan 19, 2022
@jzheaux jzheaux added this to the 2.4.0-M2 milestone Jan 19, 2022
@jzheaux
Copy link
Contributor

jzheaux commented Feb 1, 2022

This is now merged into 2.4.x with df40b39 and a polish of 255bfaa.

Note that the polish changes the behavior so that returning attributes are only populated if the value is initially null instead of if it is null or empty.

This is also now merged into main.

@jzheaux jzheaux closed this Feb 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants