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

Fixed bug override globally configured pageSize variable in the middle of the code #299

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

Conversation

abpai94
Copy link
Collaborator

@abpai94 abpai94 commented Jul 26, 2024

@abpai94 abpai94 self-assigned this Jul 26, 2024
@abpai94 abpai94 requested review from coudot and davidcoutadeur July 26, 2024 09:39
@coudot coudot requested a review from soisik July 26, 2024 10:58
@coudot coudot removed their request for review August 27, 2024 08:55
@coudot coudot added this to the 2.2 milestone Aug 27, 2024
@coudot coudot added the bug label Aug 27, 2024
@coudot coudot changed the title #176: Fixed bug override globally configured pageSize variable in the middle of the code. Fixed bug override globally configured pageSize variable in the middle of the code Aug 27, 2024
Copy link
Contributor

@soisik soisik left a comment

Choose a reason for hiding this comment

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

Hello Abishek,
Check out doGetAttrsList method, we seem to have lost paginated load of pivots (only first paged loaded ?) and some control are not passed on anymore.
To reproduce issue, one should have a LDAP directory with many users ( + 2000), a pageSize at 500, and an olcLimits should apply for the account that connect to source.
There when searching for over olcLimits in a dataset (for members for example), you should see less entry returns than expected (as they are limited by olcLimits), while one should expect a paginated load of theses entries so that olcLimits is not reach and no entry is missed.

@abpai94 abpai94 force-pushed the 176-bug-dn-elements-max-pagesize branch from 847c512 to 64e3dac Compare September 4, 2024 13:57
@abpai94
Copy link
Collaborator Author

abpai94 commented Sep 4, 2024

@davidcoutadeur I have understood the problem better and proposed a new fix. The commits have been squashed as requested.

@abpai94 abpai94 requested a review from soisik September 4, 2024 14:38
Copy link
Contributor

@davidcoutadeur davidcoutadeur left a comment

Choose a reason for hiding this comment

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

Here is my complete review

src/main/java/org/lsc/jndi/JndiServices.java Outdated Show resolved Hide resolved
src/main/java/org/lsc/jndi/JndiServices.java Show resolved Hide resolved
src/main/java/org/lsc/jndi/JndiServices.java Outdated Show resolved Hide resolved
src/main/java/org/lsc/jndi/JndiServices.java Show resolved Hide resolved
src/main/java/org/lsc/jndi/JndiServices.java Show resolved Hide resolved
src/main/java/org/lsc/jndi/JndiServices.java Outdated Show resolved Hide resolved
src/main/java/org/lsc/jndi/JndiServices.java Show resolved Hide resolved
- Clearing requestControlContext in finally block
@abpai94 abpai94 force-pushed the 176-bug-dn-elements-max-pagesize branch from 3308b20 to 74c3ca6 Compare November 12, 2024 10:40
@abpai94
Copy link
Collaborator Author

abpai94 commented Nov 12, 2024

@davidcoutadeur I have applied fixes corresponding to your comments. This is ready to merge.

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.

list of DN elements is truncated when using search method on JndiService
4 participants