I'm extending the SecurityService's user to add some more columns.
However, the DBUserManager's store code does:

275         Criteria criteria =
TurbineUserPeer.buildCriteria((TurbineUser)user);
276         try
277         {
278             TurbineUserPeer.doUpdate(criteria);
279         }

http://jakarta.apache.org/turbine/fulcrum/xref/org/apache/fulcrum/securi
ty/impl/db/DBUserManager.html#275

To me, this is really bad, because it's hard coded to the
TurbineUserPeer and not delegated to the configured peer the user sets
via services.SecurityService.userPeer.class.

Ideally some cool reflection could handle this (maybe), but since the
TurbineUser has a complex object model anyway, couldn't the
DBUserManager just do:

276         try
277         {
278             ((TurbineUser) user).save();
279         }
280         catch(Exception e)
281         {
282             throw new DataBackendException("Failed to save user
object", e);
283         }

Calling save will get down to the same call as the doUpdate, but it'll
first go through the user's object, allowing the user to more easily
extend TurbineUser.

Also, this has the added benefit of updating the user object's internal
state. E.g. if DBUserManager.createAccount(user) was called before, the
user object was in an invalid state because it thinks that it is new
when really it's already been saved. Calling save again results in a
duplicate entry error (assuming some kind of unique/primary key is
setup). This isn't a huge deal, but it's nice to have the user object in
a valid state.

I've attached a patch that makes the above changes in the store and
createAccount methods.

This has been working great in my app and, to me, it looks safe/general
enough to commit. And it shouldn't affect any existing extension
methods, correct?

- Stephen



--
To unsubscribe, e-mail:   <mailto:[EMAIL PROTECTED]>
For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>

Reply via email to