java-object-mapper icon indicating copy to clipboard operation
java-object-mapper copied to clipboard

The class still need to implement serializable in some cases, this was fixed in #118

Open Agrawalmahin opened this issue 2 years ago • 4 comments

Hello

@Record public class A { @Key private String id; @Embed... public List<B> bList; }

public class B { @Bin... public C c; }

public class C { public String s1; public String s2;

@Embed..
public List<D> dList;

}

public class D {

public String s3;

} classes C and D are external. C and D do not implement serializable.

we are getting not serializable exception.

If the above is solved, will current case also be solved?

using aero-mapper-version : 2.2.0

Agrawalmahin avatar Apr 18 '23 06:04 Agrawalmahin

Hi

Thanks for reaching out. I'm trying to reproduce your issue, but I'm not 100% clear on your annotations. It looks like the reference to B inside A has @AerospikeEmbed, the reference to D within C has @AerospikeEmbed on it but the reference to C from B only has @AerospikeBin and not @AerospikeEmbed on it. Is this correct?

If all the references have @AerospikeEmbed on them, they should work fine even without any classes implementing Serializable. The only reason the Object Mapper tries to use Java's default serializer (and hence has the ability to throw NotSerializable exception) is when it encounters a type which it doesn't know how to handle. So in this case, if B had marked the instance of C to be @AerospikeEmbed or C had implemented @AerospikeRecord, it would have been handled correctly. But without either of these 2 conditions being met, B tries to use Java's default serializer to store the instance of C. This uses serialization and throws the exception you're seeing as C is not serializable.

We could potentially enhance the Object Mapper to automatically assume that any class it sees inside an object which is already embedded should itself be embedded. However, this is very dangerous. There's a lot of possibility of accidentally serializing classes or attributes which have internal implementations and are best not serialized. The other option would be to add the ability to mark some classes/fields as AerospikeEmbed in the AeroMapper.Builder. This could be used to augment the property file.

Can you please let me know more details of your use case and what you're trying to achieve so I can help see how we can make the Object Mapper work for you?

tim-aero avatar Apr 18 '23 23:04 tim-aero

Hey @tim-aero So basically yes this is what is happening it's going to Java's default serializer while Packing the whole Object it goes to Packer. packBlob(Object obj) as it is not able to map to any other condition and since the Object we are using is coming from 3rd party library ( C and D ) and does not implement Serializable so it's throwing an error there. So we need a way out of this so we can save both C and D in Aerospike. Since C and D are coming from 3rd party libraries we can't implement Serializable in them nor can we add AerospikeEmbed to it

The correct format

@record
public class A {
@key
private String id;
@embed...
public List<B> bList;
}

public class B {

@bin...
@embed
public C c;
}

public class C {
public String s1;
public String s2;
public List<D> dList;
}

public class D {
public String s3;
}

KalshuCodes avatar Apr 19 '23 04:04 KalshuCodes

Hi @Agrawalmahin, @kalashshah11

Thanks for the details, makes reproducing the issue much easier! So the problem you're facing (external classes which cannot be annotated but need annotations for the Object Mapper to understand them) is exactly the reason we implemented an external configuration file. This allows you to mark up your immutable library classes without having to touch their source code.

The configuration can be provided either as an external file or as a string. So in your case, I reproduced your issue using:

public class Issue140 {
    @AerospikeRecord(namespace = "test", set = "A")
    public static class A {
        @AerospikeKey
        public String id;
        
        @AerospikeEmbed
        public List<B> bList;
    }
    
    public static class B {
        @AerospikeEmbed
        public C c;
    }
    
    public static class C {
        public String s1;
        public String s2;
        public List<D> dList;
    }
    
    public static class D {
        public String s3;
    }
    
    public static void main(String[] args) throws Exception {
        IAerospikeClient client = new AerospikeClient("127.0.0.1", 3000);
        AeroMapper builder = new AeroMapper.Builder(client).build();
        
        A a = new A();
        a.id = "1";
        
        B b = new B();
        a.bList = new ArrayList<>();
        a.bList.add(b);
        
        C c = new C();
        c.s1 = "Bob";
        c.s2 = "Builder";
        c.dList = new ArrayList<>();
        b.c = c;
        
        D d = new D();
        d.s3 = "Bill";
        c.dList.add(d);
        
        builder.save(a);
        A readA = builder.read(A.class, "1");
        System.out.println(readA.bList.get(0).c.dList.get(0).s3);
        client.close();
    }
}

I then introduced a configuration string (rather than using an external file just so everything is in one file for clarity, in production I would probably use an external config file). This solved the problem:

public class Issue140 {
    @AerospikeRecord(namespace = "test", set = "A")
    public static class A {
        @AerospikeKey
        public String id;
        
        @AerospikeEmbed
        public List<B> bList;
    }
    
    public static class B {
        @AerospikeEmbed
        public C c;
    }
    
    public static class C {
        public String s1;
        public String s2;
        public List<D> dList;
    }
    
    public static class D {
        public String s3;
    }
    
    public static void main(String[] args) throws Exception {
        String config =
                "classes:\n" +
                "  - class: com.timf.Issue140$C\n" +
                "    bins:\n" + 
                "      - field: dList\n" +
                "        embed:\n" +
                "          type: LIST\n" +
                "  - class: com.timf.Issue140$D\n";
        IAerospikeClient client = new AerospikeClient("127.0.0.1", 3000);
        AeroMapper builder = new AeroMapper.Builder(client).withConfiguration(config).build();
        
        A a = new A();
        a.id = "1";
        
        B b = new B();
        a.bList = new ArrayList<>();
        a.bList.add(b);
        
        C c = new C();
        c.s1 = "Bob";
        c.s2 = "Builder";
        c.dList = new ArrayList<>();
        b.c = c;
        
        D d = new D();
        d.s3 = "Bill";
        c.dList.add(d);
        
        builder.save(a);
        A readA = builder.read(A.class, "1");
        System.out.println(readA.bList.get(0).c.dList.get(0).s3);
        client.close();
    }
}

Note that I had all my classes in one file, hence marking them as static (so they don't get an implicit reference to the outer class) and the need to refer to them as Issue140$D for example. If your classes are normal (top-level) classes, then obviously they can omit the static keyword and the configuration will just need their fully qualified class name.

Also note that the configuration can co-exist with annotations. In this example, A and B are annotated, C and D use configuration. This works fine. If there are conflicts (let's say B is both annotated and in a configuration) then the configuration will win. This allows an easy path to say change the namespace between dev/test/prod environments.

As I mentioned above, one day we may change the AeroMapper.Builder() to take a configuration which does not need a string-based approach and gives compile-time checking, but this will get you past your issue in the short term.

Hope this helps, please let me know if you have any questions.

tim-aero avatar Apr 19 '23 16:04 tim-aero

Thanks @tim-aero for the solution. As I mentioned above, one day we may change the AeroMapper.Builder() to take a configuration which does not need a string-based approach and gives compile-time checking This is what we need can you close this issue when this implementation is completed. Again thanks for the quick response.

KalshuCodes avatar Apr 20 '23 09:04 KalshuCodes