allow for multiple instances of typeconvertermanager

[GoogleCodeExporter]

Right now there is only one static TypeConverterManager per classloader. This 
causes problems when you want a different conversion behavior for the same type 
in different parts of the app. For example, when performing type conversion via 
RequestParamBeanLoader.loadBean, you may prefer an IntegerConverter that 
converts bad integers (e.g. string data) to null, whereas when calling the 
standard Convert.toObject you may want the default behavior of throwing an 
exception. In a multi-threaded app both of these conversions may happen at the 
same time, so we can't rely on a static manager.

It would be nice if you could inject a TypeConverterManager into each class 
that needs it. That would either require adding a LOT of parallel method calls, 
or a complete restructuring of the JODD API (changing all static/library 
classes to be object instances), so it is probably not practical right now. I 
do have a solution, though it is very ugly:

TypeConverterManager {
  static Map<Class, TypeConverter> defaultConverters;
  static Map<String, Map<Class, TypeConverter>> instanceConverters;
  static ThreadLocal<String> converterInstance;
  static void setCurrentConverterInstance(String instanceName) {
    converterInstance.set(instanceName);
    if (!instanceConverters.containsKey(instanceName)) {
      instanceConverters.add(new HashMap(defaultConverters));
    }
  static void unsetCurrentConverterInstance() {
      converterInstance.remove();
  }
  static void register(Class type, TypeConverter typeConverter) {
    if (converterInstance.get() == null) {
      defaultConverters.put(type, typeConverter);
    } else {
      instanceConverters.get(converterInstance.get()).put(type, typeConverter);
    }
  }
  ..etc
}

So then you would just have to setCurrentConverterInstance() and 
removeCurrentConverterInstance() in a try/finally around each of the places 
where you wanted to use a custom set of converters. (Obviously the real code 
would need better null/error checking, nicer naming convention etc).

Original issue reported on code.google.com by alisonat...@gmail.com on 24 Jan 2012 at 4:04

[GoogleCodeExporter]

Hello and thanks for stopping by ;)

You are right, many utilities and managers in Jodd are static. That is fine in 
most cases (utility methods for example), but you spotted one place where this 
approach is not so developer-friendly: the TypeConverterManager. Kudos for 
spotting that, you have a sharp eye;)

As you've said, the problem is that TypeConverterManager is one the tools that 
is used all over the Jodd; and making this configurable as requested would 
"either require adding a LOT of parallel method calls, or a ... changing all 
static/library classes to be object instances". During reading your post, I 
also immediately thought of using threadlocal for different set of converters. 
So I do not think that your solution is ugly; we just need to polish it;)

But I must say that I started to question the idea of having a non-static 
duplicate of some tools, like BeanUtil (used in RequestParamBeanLoader) where 
static methods just delegates to default instance... At the end BeanUtil (for 
example) does not have too many public methods, and it can be maintainable 
(even can be generated!).

Anyhow, I did't say anything new with this response:) After I finish some 
current workload for Jodd (some minor improvements that are already under 
development) I will lookup this ideas and let you know here.

Thank you!

Original comment by igor.spasic on 24 Jan 2012 at 8:47

[GoogleCodeExporter]

Original comment by igor.spasic on 24 Jan 2012 at 8:48

• Added labels: Type-Enhancement
• Removed labels: Type-Defect

[GoogleCodeExporter]

Btw, just wondering, how to name bean variants of static tools/managers? By 
adding the 'Bean' suffix? For example: BeanUtilBean, 
TypeConverterManagerBean...?

Original comment by igor.spasic on 25 Jan 2012 at 9:00

[GoogleCodeExporter]

It would certainly be very nice to have a "bean" version of many of the static 
classes and just forward to the default instance for the utility methods. It 
would allow a lot more flexibility in tweaking the behavior of different 
BeanUtil consumers, and it would make the Spring/Guice fans happy too.

With regard to naming convention, I'm not sure. You could call it *Bean, or 
perhaps *Object or *Instance, but either way once you have a lot of them it's 
going to feel a bit verbose. Apache hit this same problem with their 
commons-beanutils and ended up creating BeanUtilsBean, so there is a precedent 
for that.

Original comment by alisonat...@gmail.com on 27 Jan 2012 at 2:36

[GoogleCodeExporter]

(I think I prefer *Instance, though. Figuring out good names is always tricky.)

Original comment by alisonat...@gmail.com on 27 Jan 2012 at 2:37

[GoogleCodeExporter]

I think I will go for the *Bean (although it sounds funny in BeanUtilBean:). If 
we name the class as *Instance, it might sounds funny as it is a class, not an 
instance... It's tricky in deed. Anyhow, I've started to refactor Jodd, so 
users may use custom type manager.

Since TypeManagerBean is used from BeanUtil, users would actually set custom 
BeanUtil class.

Original comment by igor.spasic on 29 Jan 2012 at 10:00

• Changed state: Started

[GoogleCodeExporter]

Would it make sense to set the custom TypeManager and BeanUtil like this 
(sude-code):

TypeConverterManagerBean tcm = new TypeConverterManagerBean()
tcm.register... // set custom converters
BeanUtilBean bub = new BeanUtilBean();
bub.setTypeConverterManager(tcm);

RequestParamBeanLoader.setBeanUtil(bub);

Original comment by igor.spasic on 29 Jan 2012 at 10:34

[GoogleCodeExporter]

I need to think more - I have a strange feeling that beanutil should be skipped 
and instead to provide a way to set *just* TypeConverterManager on thread level 
or on classloader level, similar what you said in the first place. BeanUtil is 
used on many places and may be hard to track and configure all its usages.

Original comment by igor.spasic on 30 Jan 2012 at 12:10

[GoogleCodeExporter]

I do think the right way to do it is the way you decribed in comment 7. It can 
feel a bit verbose creating a big network of beans just to do one thing, but 
that's the only way you can have a truely configurable API. Not to mention you 
will gain the ability to create and inject mock objects, so your unit tests can 
specifically test the individual beans rather than implicitly performing deep 
integration tests.

That said, you should be able to convert things over in small pieces. E.g. if 
you create a BeanUtilBean, you don't need to add support for it everywhere 
where BeanUtil is used. Maybe just start with the BeanLoaders. All the other 
places could still call the static BeanUtil methods, which would just delegate 
to the methods on a private static final BeanUtilBean (singleton) inside 
BeanUtil.

The nasty thing about setting TypeConverterManager on thread level is that the 
user will end up being responsible for doing try/finally stuff to make sure 
it's set/unset consistently. It works, but it feels a bit hacky. And setting it 
on classloader level is kind of too high, because it may be that you want 
various behaviors in the app (e.g. for different servlets in the same web app).

Original comment by alisonat...@gmail.com on 31 Jan 2012 at 7:04

[GoogleCodeExporter]

I understand what are you saying; what I was afraid in comment #8 is that 
BeanUtil is used all over the library and, in the future, it might be hard to 
track down usages. This is still something to think about, but in future;)

Anyhow, the refactoring you are suggesting is not hard to implement; in the 
same time it gives benefits and opens the library for future enhancements.

I've just uploaded the beta version that supports what we have talked about, in 
comment #7: http://jodd.org/download/index.html#beta

Example:
----------------------
TypeConverterManagerBean tcmb = new TypeConverterManagerBean();
tcmb.register(Integer.class, .....)
BeanUtilBean bub = new BeanUtilBean();
bub.setTypeConverterManager(tcmb);
// register it in the loader
----------------------

If you have time you can download it, so I can close the issue. Thank you for 
your time and contribution! 

Original comment by igor.spasic on 31 Jan 2012 at 10:52

[GoogleCodeExporter]

I've just finished with fine refactoring and improvements of type managers, 
Convert classes, BeanUtil etc. I've added a nice way to share and to wire 
converters. Also some little improvements... 

I will close this issue as the base request has been fulfilled. If there is 
some other place where TypeManager should be changeable, please open a new 
issue.

Original comment by igor.spasic on 13 Feb 2012 at 10:37

• Changed state: Fixed