msgpack / msgpack-java

MessagePack serializer implementation for Java / msgpack.org[Java]
http://msgpack.org/
Apache License 2.0
1.41k stars 321 forks source link

Automatic class register #19

Closed ghetolay closed 9 years ago

ghetolay commented 12 years ago

It's not really a issue it's a question :

Why can't classes be automatically registered ?

I'm actually working with JSON and I need something similar for binary data. Message Pack looks really great but the registration is a big problem for me. I'm working in a API so I don't know which classes will need to be registered. Why TemplateRegistry does not register the class if he do not find a matching template ?

muga commented 12 years ago

Hi,

Thank you for contacting us.

To generate and register templates of your classes automatically, MessagePack provides @Message annotation. Even though you cannot register your classes' templates with the annotation, it is a bug or something.

Thank, Muga

ghetolay commented 12 years ago

Hi,

I know about the annotation and it's good but we have to own the code to do it so it's not enough. Actually I've done that just to test :

private static class MyTemplateRegistry extends TemplateRegistry {

@Override
 public synchronized Template<?> lookup(Type targetType){
try{
      return super.lookup(targetType);
}catch(MessageTypeException e){
    if(e.getMessage().startsWith("Cannot find template for")
        && targetType instanceof Class<?>){

        register((Class<?>)targetType);
        return super.lookup(targetType);
    }else throw e;

It's very ugly but it seems to work pretty well. So what's I'm wondering is why the lookup method throw an exception instead of trying to register the class.

Of course this is just a test, this code is also bad because it could create an infinite loop but that's the idea : if lookup fails try to register the class and get the generated template if any (better avoid recall the lookup method).

muga commented 12 years ago

Hmm.. I see.

Could you give me your classes that you want to serialize or simple examples? I'll try to improve the impl. of TemplateRegistry.

ghetolay commented 12 years ago

I'm actually working on a Websocket protocol implementation (JWamp) so I don't have specific classes. I would like it to work with all possible classes. The register method work well (maybe except parameterized classes but this is off topic), I'm just wondering why the register method isn't called when no template are found :

public synchronized Template lookup(Type targetType) {
...

     return buildAndRegister(null, targetClass, false, null);
}

instead of

     throw new MessageTypeException(
        "Cannot find template for " + targetClass + " class.  " +
        "Try to add @Message annotation to the class or call MessagePack.register(Type).");
}
muga commented 12 years ago

That reason is to avoid unexpected template generation. Actually MessagePack 0.5.x permitted automatic template generation for any class. The specification was useful and casual, but it often caused bugs within user applications. Thus template generations without @Message was prohibited from MessagePack 0.6.x after. @Message is similar to Serializable marker interface.

BTW, I saw JWamp's source code. It is pretty good. I'm looking forward to release JWamp 0.0.1:-) For performance tuning of JWamp, you can reuse Buffer{Packer,Unpacker} objects. In the current implementation, MessagePack#createBufferPacker(..) is called within WampMessage{Serializer,Deserializer} classes whenever serialization is executed. See the following source code. https://github.com/eishay/jvm-serializers/blob/kannan/tpc/src/serializers/MsgPack.java

ghetolay commented 12 years ago

You must be right. I agree @Message and @Optional are a good design for serialization and I'll explain and permit it for my users. I still would like to offer a alternative unique and simple front end to my users not depending on Jackson nor MessagePack but it may be a impossible dream.

Maybe you could return something more specific that this exception (null ? A specific Exception Object ?), something we could easily test. This way I could recommend the use of annotation but still permit a automatic registration at his own risk (with a WARN log) if the lookup fails.

Also how do you handle parameterized classes ? i.e.

class Param<T>{ .. }

I saw template were created for List and Map so I suppose we have to create our own template for such classes ?

Thks for yours feedback about JWamp ! I've already seen these packer/unpacker optimizations on the wiki. For unpacker I can't do it because unpacker object is part of the message. In a message there is the protocol part and the user part and I'm only deserializing the protocol part then I keep the unpacker object for the user deserialisation part. As message arrive asynchronously I can't use the same unpacker for all messages. For packer I was hesitating between a synchronized method and instantiation of packer object. I've followed your advice, the method is now synchronized and reuse the same Packer object :)

ghetolay commented 12 years ago

Hi again,

I worked a bit to find a way of providing this auto-registration capability and I think I found an alternative. It's just a start and not good at all at this point. So I did this in 2 times :

Like I said it's just a start, there are still some issue like how to differentiate raw and string, licensing of TypeReference....

I had to modify several files so I created a fork here. Please give me your feedback on it and if you think it worth it I could continue to work on it. If you don't understand something (or everything) open an issue and I'll explain it.

komamitsu commented 9 years ago

I search about solution and it's seems that the only solution is the one used by JackSon with the TypeReference class

@ghetolay Very sorry for late reply. We added https://github.com/msgpack/msgpack-java/tree/v07-develop/msgpack-jackson that can be used via jackson-databind and it would resolve this kind of issue.

ghetolay commented 9 years ago

Well I'm not working on my library anymore but I'll probably work with msgpack on something else so I'll look into it. Thks.