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

[AD-75] Extend the LDAP bundle #18

Merged
merged 11 commits into from
Feb 13, 2024
Merged

[AD-75] Extend the LDAP bundle #18

merged 11 commits into from
Feb 13, 2024

Conversation

Sylinsic
Copy link
Contributor

@Sylinsic Sylinsic commented Jan 30, 2024

Currently, the AD bundle repeats quite a bit of code, and only in some places inherits from the LDAP bundle.

As AD is fundamentally communicated with via LDAP, the AD bundle should inherit most of it’s functionality from the LDAP bundle, therefore inheriting LDAP capabilities, and just extending classes/overriding methods where necessary.

This issue is for part 2 of sorting this - the extension of the AD bundle

N.B. This pull request is reliant on version 1.5.9 of the LDAP bundle being released; this code won't compile until the counterpart code is available for usage.
Merging of Tirasa/ConnIdLDAPBundle#26 is required.

Modify configuration to only contain the specific configurations for this bundle
Modify configuration bean info to contain configurations from both this bundle, and the LDAP bundle
Extend most classes with their LDAP bundle counterparts - removing duplicate code
Extend functionality for AnyObjects
Add some configuration messages
@ilgrosso
Copy link
Member

@Sylinsic now that Tirasa/ConnIdLDAPBundle#26 is merged, please include the change to update ldap connector version to 1.5.9-SNAPSHOT in this project's pom.xml, so that build will have a chance to pass

@Sylinsic
Copy link
Contributor Author

@Sylinsic now that Tirasa/ConnIdLDAPBundle#26 is merged, please include the change to update ldap connector version to 1.5.9-SNAPSHOT in this project's pom.xml, so that build will have a chance to pass

@ilgrosso updated. I didn't commit it originally as I didn't want to push reliance on a snapshot version to the main repo

@ilgrosso
Copy link
Member

@Sylinsic now that Tirasa/ConnIdLDAPBundle#26 is merged, please include the change to update ldap connector version to 1.5.9-SNAPSHOT in this project's pom.xml, so that build will have a chance to pass

@ilgrosso updated. I didn't commit it originally as I didn't want to push reliance on a snapshot version to the main repo

I see there are several test failures now that workflows could run, not possible until LDAP version was updated.

@Sylinsic
Copy link
Contributor Author

Sylinsic commented Feb 3, 2024

@Sylinsic now that Tirasa/ConnIdLDAPBundle#26 is merged, please include the change to update ldap connector version to 1.5.9-SNAPSHOT in this project's pom.xml, so that build will have a chance to pass

@ilgrosso updated. I didn't commit it originally as I didn't want to push reliance on a snapshot version to the main repo

I see there are several test failures now that workflows could run, not possible until LDAP version was updated.

I believe these should all run successfully now

@ilgrosso
Copy link
Member

ilgrosso commented Feb 3, 2024

@Sylinsic the build is green again, good.
Let's wait for @fmartelli's checks before merge

@ilgrosso
Copy link
Member

ilgrosso commented Feb 7, 2024

@Sylinsic I see you keep committing on this PR: does this mean that it's not ready to be reviewed and merged?
If so, please make it draft.

AD doesn't utilise the ';binary' on binary attributes, which prevents them from being pulled in.
@Sylinsic
Copy link
Contributor Author

Sylinsic commented Feb 8, 2024

@Sylinsic I see you keep committing on this PR: does this mean that it's not ready to be reviewed and merged? If so, please make it draft.

I believe it is, however I just stumbled upon two issues when actually using the snapshot bundle, and as this hadn't been merged yet I figured I may as well whack them into this PR instead of open another

@ilgrosso
Copy link
Member

@Sylinsic after careful evaluation by @fmartelli we decided that this PR contains some breaking changes that cannot be merged for maintenance release 1.3.10.
We will go ahead and merge it anyway for the upcoming version 1.4.0.

@ilgrosso ilgrosso merged commit 479948e into Tirasa:master Feb 13, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants