PropertyEditors: Singletons or no?

[mmakunas]

Am I correct in my assuming that PropertyEditors for my domain objects should be configured with "singleton" set to false? I am using the standard naming convention for the PropertyEditors so I don't need to register them, but I do need to wire in a Dao to each of them.


Or maybe I have a deeper misunderstanding. Are spring beans (which are singletons by default) naturally thread safe? Or do I need to worry about multiple requests accessing the same beans at the same time?


-Michael

[ksarantakos]

Your assumption was correct, you'll have to set singleton="false" for those beans.

-kyri

[mmakunas]

Thanks.

That got me thinking, though. Does that mean that any spring bean that is going to be accessed by multiple request should have singleton set to false? Or should I synchronize specific methods within the bean. For instance, if I have a PersonService bean that retrieves a particular person, do I need to worry about multiple requests calling getPersonById() at the same time?

[ksarantakos]

Spring itself doesn't nothing to ensure the thread safety of the beans it wires.

In your example getPersonById() should be thread-safe, so in a way, yes you do need to be worried about multiple threads.

Generally there are very few instances in which you should need to use singleton="false", subclasses of ThrowawayController is one such rare instance, for example.

Your entire service layer should be thread safe, spring or no spring. Synchonization shouldn't be needed for most implementations of getPersonById().

-kyri

[mmakunas]

Spring itself doesn't nothing to ensure the thread safety of the beans it wires.


That is good to know.

Synchonization shouldn't be needed for most implementations of getPersonById().

This is probably exposing my ignorance of thread safety (or lack of sleep), but shouldn't I need to synchronize any method that's has more than one thread calling it at the same time? For instance here are two of my methods in my AdviceService bean (using Advice and nor Person this time):

public List searchForAdvice(Service[] services, AdvisorRole[] roles)
{

Integer[] roleIds = new Integer[roles.length];

for (int x = 0; x < roles.length; x++)
{
roleIds[x] = roles[x].getId();
}

Integer[] serviceIds = new Integer[services.length];

for (int x = 0; x < services.length; x++)
{
serviceIds[x] = services[x].getId();
}

return adviceDao.findAdviceByExperienceAndRoleIds(service Ids, roleIds);
}

and

public List findAdviceOfferedByUserId(int id)
{
return adviceDao.findAdviceOfferedByUserId(id);
}

I need to synchronize both those methods (and anything similar) if they are going to be called by a controller, correct?

-Michael

[ksarantakos]

While I can't say for certain without looking at the dao implementation as well, but both those methods are thead safe.

I think you're fine. Unless you've got something crazy happening in the DAO. But assuming it's backed by a reasonably modern database and you've implemented a DAO layer with Hibernate, TopLink, iBatis, or even straight JDBC (Hopefully using Spring's JdbcTemplate) you're probably cool.

As a quick example of how to make those methods thread unsafe, move the two Integer arrays out of the method and into class instance variables. Then you'd have to synchronize searchForAdvice and any other methods that modified those arrays. But with them inside the method each caller will get their own copy and you're ok.

Thread-safety doesn't mean synchronization is always required. In general, you'll want to avoid synchronizing methods for performance reasons.

-kyri

[mmakunas]

As a quick example of how to make those methods thread unsafe, move the two Integer arrays out of the method and into class instance variables. Then you'd have to synchronize searchForAdvice and any other methods that modified those arrays. But with them inside the method each caller will get their own copy and you're ok.

Got it now. I knew moving them into class instance variables would make it thread UNsafe, but I wasn't sure if method calls get their own copies. Thanks for the clarification.

But back to my original question about PropertyEditors, if each method call gets it's own copies of things, why would a PropertyEditor such as the one below need to have sigleton set to false?

public class SubservicePropertyEditor extends PropertyEditorSupport
{
private final EnumerationService enumerationService;

public SubservicePropertyEditor (final EnumerationService enumerationService){
this.enumerationService = enumerationService;
}
@Override
public String getAsText()
{
Subservice subservice = (Subservice) getValue();
return subservice.getId().toString();
}

@Override
public void setAsText(String value) throws IllegalArgumentException
{
try {
Subservice subservice = enumerationService.getSubservice((Integer.parseInt (value)));
super.setValue(subservice);
}
catch (final Exception e) {
super.setValue(null);
}
}
}

[ksarantakos]

Because PropertyEditorSupport stores value as an instance variable. It's possible that another thread could clobber the value of getValue(). Hence the object is not thread safe. You could write a thread safe implementation of PropertyEditor with ThreadLocals to make it thread safe, but you'd be wasting your time.

Your bean definition of the PropertyEditor really only needs to be a singleton if you are reusing that same PropertyEditor in another controller. But each controller needs it's own instance of the PropertyEditor's it's using, so you need to get each one a new instance.

I've personally never configured PropertyEditors via XML bean config. I just set them in the Controllers initBinder method, new'ing each one and passing them into binder.registerCustomEditor().

Now that I think about it some more, how are you actually setting these? Are you passing them in via the bean config and then setting them in initBinder? If so don't bother with that, just construct them within the initBinder method and forget about making them beans that get injected in. That's how most of us do it. No need to needlessly clutter up your bean config. Not everything that can be injected needs to be. Sometimes using Java as a configuration language is ok.

And it's not like your going to inject this bean with Mock or Stub PropertyEditors. I mean you could, but it's probably enough to unit test the PropertyEditors themselves, and then just mock up a HttpServletRequest and verify that binding went as planned. A stub property editor would probably look just like a "real" property editor anyway.

That said, occasionally I have property editors that reley on a dao or service to perform a db lookup of say an id that's passed in the request and then construct an Object from the db. This dao should be injected into the controller and then passed into the constructor of the PropertyEditor, so that you can test the PropertyEditor and Controller with a Stub/Mock dao.

-kyri

[mmakunas]

Because PropertyEditorSupport stores value as an instance variable. It's possible that another thread could clobber the value of getValue(). Hence the object is not thread safe. You could write a thread safe implementation of PropertyEditor with ThreadLocals to make it thread safe, but you'd be wasting your time.


OK...that makes sense.


I've personally never configured PropertyEditors via XML bean config. I just set them in the Controllers initBinder method, new'ing each one and passing them into binder.registerCustomEditor().


I don't need to call binder.registerCustomEditor() because I use the standard naming convention for property editors. A property editor for class Foo will automatically be found if you name it FooPropertyEditor and it is in the classpath.


That said, occasionally I have property editors that reley on a dao or service to perform a db lookup of say an id that's passed in the request and then construct an Object from the db. This dao should be injected into the controller and then passed into the constructor of the PropertyEditor, so that you can test the PropertyEditor and Controller with a Stub/Mock dao.


Yes, that's my case here. I have a bunch of property editors that need to hit the db. Therefore I wire in a DAO. Also none of my controllers have DAOs directly wired into them. They only have services wired into them and the DAOs are wired into the services.

-Michael