org.jboss.portal.identity.ldap: Is a missing attribute really an error?
Good Morning,
Just for background, I'm not a Java or JBoss programmer, at all. What I am is the sysadmin and support guy who will have to answer the customer when he want to know why he's getting messages like this in his log:
2012-03-15 17:48:54,571 ERROR [org.jboss.portal.identity.ldap.LDAPUserProfileModuleImpl] (http-xxx.xxx.xxx.xxx-8080-9) No such attribute ('title') in entry: uid=Joe,ou=People,dc=foo,dc=bar
2012-03-15 17:48:54,571 ERROR [org.jboss.portal.identity.ldap.LDAPUserProfileModuleImpl] (http-xxx.xxx.xxx.xxx-8080-9) No such attribute ('mail') in entry: uid=Joe,ou=People,dc=foo,dc=bar
So, I go Googling for the error message and find the package...
22 package org.jboss.portal.identity.ldap;
...
52 public class LDAPUserProfileModuleImpl extends LDAPUserProfileModule
53 {
54 private static final org.jboss.logging.Logger log = org.jboss.logging.Logger.getLogger(LDAPUserProfileModuleImpl.class);
55
56 private UserModule userModule;
57
58 public Object getProperty(User user, String propertyName) throws IdentityException, IllegalArgumentException
59 {
60 if (user == null)
61 {
62 throw new IllegalArgumentException("User cannot be null");
63 }
64 if (propertyName == null)
65 {
66 throw new IllegalArgumentException("Property name need to have value");
67 }
68
69 if (user instanceof CachedUserImpl)
70 {
71 try
72 {
73 user = getUserModule().findUserById(user.getId());
74 }
75 catch(NoSuchUserException e)
76 {
77 throw new IdentityException("Illegal state - cached user doesn't exist in identity store: ", e);
78 }
79 }
80
81 LDAPUserImpl ldapUser = null;
82
83 if (user instanceof LDAPUserImpl)
84 {
85 ldapUser = (LDAPUserImpl)user;
86 }
87 else
88 {
89 throw new IllegalArgumentException("This UserProfileModule implementation supports only LDAPUserImpl objects");
90 }
91
92 String attributeName = resolveAttributeName(propertyName);
93 Object propertyValue = null;
94
95 if (attributeName == null)
96 {
97 log.error("Proper LDAP attribute mapping not found for such property name: " + propertyName);
98 return null;
99 }
100
101 LdapContext ldapContext = getConnectionContext().createInitialContext();
102
103
try
104
{
105
Attributes attrs = ldapContext.getAttributes(ldapUser.getDn());
106
107
Attribute attr = attrs.get(attributeName);
108
109
if (attr != null)
110
{
111
propertyValue = attr.get();
112
}
113
else
114
{
115
log.error("No such attribute ('" + attributeName + "') in entry: " + ldapUser.getDn());
116
}
117
}
118
catch (NamingException e)
119
{
120
throw new IdentityException("Cannot get user property value.", e);
121
}
122
finally
123
{
124
try
125
{
126
ldapContext.close();
127
}
128
catch (NamingException e)
129
{
130
throw new IdentityException("Failed to close LDAP connection", e);
131
}
132
}
133
134
PropertyInfo pi = getProfileInfo().getPropertyInfo(propertyName);
135
136
137
if (propertyValue != null && !pi.getType().equals(propertyValue.getClass().getName()))
138
{
139
log.error("Error on processing property:" + propertyName);
140
log.error("Wrong property type retreived from LDAP. Should be: " + pi.getType() + "; and found: " + propertyValue.getClass().getName());
141
}
142
143
return propertyValue;
144
145
}
I can see why LDAP connection failures would be ERRORs. Why are missing properties or property type mismatches considered errors? Wouldn't that be for the method caller to decide? In my case, okay, the application can't send an email, and it can't call the CEO 'CEO' in that email it can't send anyway. Those would be WARNings, at worst. But, in other cases, I can see a missing or invalid property being a significant problem.
Now, if I'm going to grumble about something, the least I can do is suggest an alternative. May I suggest that the log.error() messages be turned into log.warn()? I'm not as clear on the exceptions, but I think the log.error() on line 115 could be accompanied by its own IdentityException(). For the log.error()s on lines 139-40, as long as values got returned, wouldn't log.warn() be sufficient? If Java already has an exception for a bad variable type, maybe that would be appropriate, there? That method would still be sending its own messages, and the exceptions would allow the caller to recognize an error and perform whatever other actions it needed.
Thanks!
David