[MGNLLDAP-71] Inconsistencies in loading ldap.properties (code vs comments vs doc) Created: 16/Aug/12 Updated: 12/Jul/13 Resolved: 11/Jul/13 |
|
| Status: | Closed |
| Project: | LDAP Connector |
| Component/s: | None |
| Affects Version/s: | 1.5.3 |
| Fix Version/s: | 1.6.2 |
| Type: | Bug | Priority: | Neutral |
| Reporter: | Magnolia International | Assignee: | Milan Divilek |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||
| Template: |
|
||||||||||||
| Acceptance criteria: |
Empty
|
||||||||||||
| Task DoD: |
[ ]*
Doc/release notes changes? Comment present?
[ ]*
Downstream builds green?
[ ]*
Solution information and context easily available?
[ ]*
Tests
[ ]*
FixVersion filled and not yet released
[ ] 
Architecture Decision Record (ADR)
|
||||||||||||
| Bug DoR: |
[ ]*
Steps to reproduce, expected, and actual results filled
[ ]*
Affected version filled
|
||||||||||||
| Description |
|
Docu for the ldap connector tells me to configure a "jndi.ldap.config" property in my magnolia.properties. I get the vague feeling this is vaguely new, as I thought it used to be configured in jaas.properties. But OK, fair enough, I'll change my config.
Both methods have a comment that says // for backword compatability, this use to be in magnolia.properties as a relative path. Typos apart, this tells me the opposite of the documentation, and that the current way of configuring would be to use jaas.config ? IF all calls would go to (2), then things would be consistent, and I'd believe that it's just the code comment that's poorly written. The issue is not so much where the calls go rather than the fact that the methods are just behaving inconsistently. So I would suggest the following:
If we don't restore backwards compatibility, then http://documentation.magnolia-cms.com/releases/4-5-1.html#IfyouareusingtheLDAPConnectormodule should not say ".. is now configurable in .." but "must now be configured in ...". While we're at it: docu says jndi.ldap.config should be relative to the web-app. In fact, it looks it can also be an absolute path, or use \${magnolia.home} like other path properties. This should be verified and documented. |