Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Agrawalmahin opened this issue Apr 18, 2023 · 4 comments

Comments

@Agrawalmahin
Copy link

Hello

@record
public class A {
@key
private String id;
@embed...
public List 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

@tim-aero
Copy link
Contributor

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?

@KalshuCodes
Copy link

KalshuCodes commented Apr 19, 2023

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;
}

@tim-aero
Copy link
Contributor

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.

@KalshuCodes
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants