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

check Id equality while ignoring case. #725

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

AlzubaidiTurki
Copy link

@AlzubaidiTurki AlzubaidiTurki commented Feb 7, 2023

LDAP attribute names are case insensitive. however, they are case aware. figured this when I tried to implement an entry.

Take this case, let us say we have an attribute named "Mobile" with capital M in LDAP, if we type:

@Attribute(name="mobile") // small m
String mobile;

In fetching process, this is fine. attribute is mapped properly and mobile variable will store whatever value. However, during saving process, the condition !id.equals(that.id) will compare "mobile" to "Mobile", and it will assume these attributes are not the same. thus it will be treated as an attribute that should go to REPLACE LDAP operation.

Now, using Replace to non existing attribute will add that attribute to the LDAP entry. so "mobile" should be added. but actually what happens is the same as when we fetch attributes. LDAP will treat replace the value and won't create a new attribute.

What is the new value? it is the same as the original one, since value haven't been touched. This causes a meaningless LDAP REPLACE operations.

Hope that is clear and makes sense :)

LDAP attribute names are case insensitive. however, they are case aware. figured this when I tried to implement an entry.

Take this case, let us say we have an attribute named "Mobile" with capital M in LDAP, if we type:
@Attribute(name="mobile") // small m
String mobile;

In fetching process, this is fine. attribute is mapped properly and mobile variable will store whatever value.
However, during saving process, the condition !id.equals(that.id) will compare "mobile" to "Mobile", and it will assume these attributes are not the same. thus it will be treated as an attribute that should go to REPLACE LDAP operation.

Now, using Replace to non existing attribute will add that attribute to the LDAP entry. so "mobile" should be added. but actually what happens is the same as when we fetch attributes. LDAP will treat replace the value and won't create a new attribute.

What is the new value? it is the same as the original one, since value haven't been touched.
This causes a meaningless LDAP REPLACE operations.

Hope that is clear and makes sense :)

Acknowledgement: Wouldn't figure this out without the help of my dear mentor Mosaed Alsharyan, and my friend Omar Alhamdan
@jzheaux
Copy link
Contributor

jzheaux commented Feb 7, 2023

Thanks for the PR, @AlzubaidiTurki, and the explanation.

It seems the ideal solution would be for the application to do @Attribute(name='Mobile') since that is the value in LDAP (or for the entries to change their case so everything matches). In a way, it is the misalignment between the app and the data that is causing the extra REPLACE.

There's also the issue that changing the .equals in this way breaks the equals/hashCode contract since now two ids of different cases will be "equal" but their hashcodes will not.

That said, I can see that having a case-insensitive read paired with a case-sensitive write may be disorienting, and a change may be worth considering. Are you able to add tests to the PR that demonstrate that the new behavior works with the arrangement you have indicated?

@AlzubaidiTurki
Copy link
Author

AlzubaidiTurki commented Apr 2, 2023

Thank you Mr.@jzheaux.
I am embarrassed from my very late reply. I hope you accept my apology.
Could you please give me a reference that will teach me how to do a proper testing on this?

Thanks,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants