Matthias247 / jawampa

Web Application Messaging Protocol (WAMP v2) support for Java
Apache License 2.0
148 stars 56 forks source link

Customize object mapper for WampClient (again) #85

Open the20login opened 8 years ago

the20login commented 8 years ago

Some time ago I have implemented setter for object mapper for issue https://github.com/Matthias247/jawampa/issues/79

Unfortunately, I made a mistake, my commit is useless. Jawampa client use not one, not two, but THREE different instances of ObjectMapper, and I replace only one of them.

I will provide correct implementation in several days.

the20login commented 8 years ago

There are problem with implementation. In geneneral case, client needs at least two ObjectMappers: one for json, and one for msgpack. There are several ways for obtaining configured instances, but all of them has drawbacks.

  1. Implement second setter for msgpack ObjectMapper. This is simplest solution, but it's difficult to validate that second ObjectMapper actually use MessagePackFactory as JsonFactory (classname is a hacky way to do this, I don't like it).
  2. Provide consumer-configurer for ObjectMapper. Add method to set instance with configure(ObjectMapper mapper) method, ObjectMapppers itself created inside clientBuilder and passed to that instance. This option is verbose for Java 7 and below.
  3. Instantiate both ObjectMappers inside WampClient or WampClientBuilder and expose them with getters. If you want to configure ObjectMapper, you just use existing instance and configure it as you like.

Third option looks better then others. @programmerr47, please tell me if you have any particular preferences.

the20login commented 8 years ago

After closely look at the code, I realize that ObjectMapper could be customized right now, without any changes. Just appply configuration settings to static instances in WampSerialization enum.

But using static non-constant fields is bad desing, it could be used as a quick hack, but not in general. For example, application need to receive(or send) messages with several excluding ObjectMapper propeties. It is impossinble in current implementation, but would be easy in case of per-client mappers. Of cource, it is extreamly rare case, but, since jawampa is a library, it is still possible.

Implementing per-client ObjectMappers require significant changes, and I would like to discuss it with @Matthias247 (or another mainteiner) first.

programmerr47 commented 8 years ago

@the20login thank you for this information. I think I only need to configure objectMapper during "building" process though WampClientBuilder.