PersistentComponent with Java Records deserializing issue

Hi,

I noticed when getting a PersistentComponent (I created using Java record) from SqlEntityData in Zay-ES it throws IllegalAccessException.

Exception in thread "main" java.lang.RuntimeException: Error in field mapping
	at com.simsilica.es.sql.FieldTypes$StringField.load(FieldTypes.java:281)
	at com.simsilica.es.sql.ComponentTable.getComponent(ComponentTable.java:343)
	at com.simsilica.es.sql.SqlComponentHandler.getComponent(SqlComponentHandler.java:94)
	at com.simsilica.es.base.DefaultEntityData.getComponent(DefaultEntityData.java:178)
	at app.RecordPersistentComponentTest.main(RecordPersistentComponentTest.java:28)
Caused by: java.lang.IllegalAccessException: Can not set final java.lang.String field app.RecordPersistentComponentTest$Person.name to java.lang.String
	at java.base/jdk.internal.reflect.FieldAccessorImpl.throwFinalFieldIllegalAccessException(FieldAccessorImpl.java:137)
	at java.base/jdk.internal.reflect.FieldAccessorImpl.throwFinalFieldIllegalAccessException(FieldAccessorImpl.java:141)
	at java.base/jdk.internal.reflect.MethodHandleObjectFieldAccessorImpl.set(MethodHandleObjectFieldAccessorImpl.java:103)
	at java.base/java.lang.reflect.Field.set(Field.java:834)
	at com.simsilica.es.sql.FieldTypes$StringField.load(FieldTypes.java:278)
	... 4 more

Has someone else also encountered this issue?

Apparently using reflection is not allowed with records since Java 15.

https://bugs.openjdk.org/browse/JDK-8247517

This change impacts 3rd-party frameworks including 3rd-party
serialization framework that rely on core reflection setAccessible or
sun.misc.Unsafe::allocateInstance and objectFieldOffset etc to
construct records but not using the canonical constructor.
These frameworks would need to be updated to construct records via its
canonical constructor as done by Java serialization.

This also affects other libraries like GSON,…

Looks like they solved it by switching to using the canonical constructor for records.

Would it be possible to solve this in a similar way in Zay-ES as well?

This is a minimal test case to reproduce the issue:

public class TestPersistentComponentWithRecord {

    public record Person(String name) implements PersistentComponent {

        protected Person() {
            this(null);
        }
    }

    public static void main(String[] args) throws SQLException {
        EntityData ed = new SqlEntityData("/home/ali/Desktop/edb", 500);

        EntityId id = ed.createEntity();
        ed.setComponent(id, new Person("ali"));

        Person person = ed.getComponent(id, Person.class);
        System.out.println(person.name());

        ed.close();
    }
}

I am using OpenJDK 20.

I’m not sure how best to solve this.

In your example, the “canonical constructor” to field mapping is obvious but that is almost never the case for anything other than these single-field components.

Probably we need to augment the SQL support with the ability to support custom object factories.

Curious: I’ve never used records. What are they buying you in this case?

Well, in the above example, no thing I think, (except the getter) but in general I guess I can get rid of some boilerplate codes (hashcode, equals,…) and also “getters” are there out of the box.

I started to convert my game components to record a while back without being aware of this issue ahead of me. For now, I think I need to revert them back for persistent components to not use record…

By the way, it is still possible to use reflection to read fields and parameter types,… from records that can be used to detect the appropriate canonical constructor, I guess. (?)

Only modifications to fields are not allowed.

The question mark is concerning. Do you not already know if saving your objects works? Do you know how to open the hsql UI for looking at your saved database to see if the fields were really stored?

To be certain I re-checked in HSQL UI and the component successfully persisted in the database. So yes using reflection to read fields from the record works fine.

1 Like

I have looked at the code and adding factory support would not be too difficult to add. I don’t know when/if I will get to it myself because of time constraints and because I get nervous when I implement something that I don’t use and can’t properly test.

Just in case, here is the outline as I see it.

  1. Make a new SqlComponentFactory<T> interface that has a createComponent method takes a ResultSet and returns a T
  2. Create a DefaultComponentFactory that looks up the no-arg constructor for the class and essentially copies the code from two places in ComponentTable:
                T target = ctor.newInstance(); 
                for( FieldType t : fields ) {
                    index = t.load(target, rs, index);
                }
  1. Modify ComponentTable to take this SqlComponentFactory instead of the ctor argument and update the two places where the loop is to call the factory instead.
  2. Modify it’s create() factory method to create the DefaultComponentFactory instead of manually looking up the ctor.
  3. Add a constructor to SqlComponentHandler to can take a factory… probably leave the type-based one.
  4. Add a method to SqlEntityData for setting the SqlComponentFactory for a type. Probably the easiest way is to have it add the class to persistentTypes, create the appropriate SqlComponentHandler, and call super.registerComponentHandler()

Custom factories could then be used by calling sqlEntityData.setSqlComponentFactory()… or if we want an alternate makePersistence() method that takes a second factory argument I’m ok with that, too.

It does mean that an application that uses records would have to add a factory for every component that is a record. I don’t see a way around that, though.

1 Like

What is the issue with a general-purpose RecordComponentFactory that uses canonical constructor to initialize components?

Class<?> recordClass = ...;

RecordComponent[] components = recordClass.getRecordComponents();
Class<?>[] parameterTypes = new Class<?>[components.length];
for (int i = 0; i < components.length; i++) {
    parameterTypes[i] = components[i].getType();
}

Constructor<?> constructor = recordClass.getDeclaredConstructor(parameterTypes);
constructor.newInstance(args)

I guess records only have generated constructors and the parameters are in field order?

What if the user reorders the fields of the class?

1 Like

Good point, let me try this now. Will let you know of the result.

Looks like they can not reorder the fields, I am getting a compiler error when reorder them

Non-canonical record constructor must delegate to another constructor

You misunderstand. The fields of the record itself. Reordering the fields of a class is not usually something the users will think breaks persistence.

How does GSON solve this again?

Ok, tried it, and persisting record to DB still works fine after I reordered the fields.

And for constructing the record (loading component from DB), class.getDeclaredFields() should resolve the fields in the order they are at runtime (yes?) so it should read fields from the database table in the same order:

So if previously it was SELECT x, y from PERSON, after reordering fields (reversing the order in this case), it automatically should be SELECT y, x from PERSON which should read fields from tables in the correct order, I guess.

It gets record canonical constructor and use that constructor to initialize the object.

Like the below code example:

Edit:

Notice the use of Class.getRecordComponents() for finding the canonical constructor. See

To expand a bit on what @Ali_RS is saying; the names of the parameters could be used rather than their order (directly)

public record SomeRecord(int alpha, String beta, List<String> gamma){
}
public class RecordTest{
    public static void main(String[] args) {
        Class<?> recordClass = SomeRecord.class;
        RecordComponent[] componentTypes = recordClass.getRecordComponents();

        for(RecordComponent componentType : componentTypes){
            System.out.println(componentType.getType() + " " +  componentType.getName());
        }
    }
}

This prints out:

int alpha
class java.lang.String beta
interface java.util.List gamma

So it would be easy to figure out that gamma is the third parameter for the canonical constructor (even if when the class was serialised it was the second parameter).

1 Like

Having dealt with persistence for records and traditional classes, I prefer records because they’re generally very unambiguous:

  1. No ambiguity about which constructor to call - you always have one available that accepts all parameters, and as long as someone doesn’t do something odd with other constructors you’ll have no issues always invoking the canonical constructor.

  2. No ambiguity about which fields you should/should not serialize - records are meant to be data objects, so 99% of the time you should serialize all fields as given. If someone has set a field of a record to an object that should not be serialized (such as a reference to a service), that’s poor design to begin with and you probably have larger problems to address than which fields get serialized.

  3. No inheritance - both of the issues above can become even more of a problem in the face of inheritance. Records cannot explicitly extend anything (they always implicitly extend Record), so you don’t need to worry about discovering/serializing the fields of a base class.

  4. Reflection that directly fits the data model - RecordComponents make it easy to discover the correct constructor and resolve which field goes where. As long as you have a schema with name/type for your stored data, it’s usually easy to resolve stored data against changes to the record type that aren’t immediately obvious (such as field reordering or even some field type changes - say changing a float to a double).

Data serialization is never foolproof, but records go a long ways towards removing some of the bigger headaches as long as they’re used consistently with a few minimal guidelines.

3 Likes

Thank you @danielp

I have a question regarding this, in the case of records can you please confirm if the only difference (in terms of the fields they return) between Class.getDeclaredFields() and Class.getRecordComponents() is in the static fields?

Supposing that I am already filtering out static fields from the Class.getDeclaredFields() result, would it be safe to use getDeclaredFields to grab the canonical constructor on record? (as Class.getRecordComponents() is not available on the older Java versions)

Unfortunately you cannot use getDeclaredFields() as a substitute for getRecordComponents() without a fair bit of extra finagling that may be brittle. The issue is the order of fields - getRecordComponents() always returns the components in the order that they are declared in the header but getDeclaredFields() does not guarantee any particular order. This isn’t (in general) an easy problem to solve without getRecordComponents() because inspecting the constructor will get you as far as parameter type/position/count but not names - so if you have multiple parameters of the same type in the record (which is pretty common) then you’re 100% reliant on the ordering never changing between the serialized data and the record header.

If you’re going to use records for serialized data, I’d look heavily at upgrading your Java version if at all possible. jlink helps a ton in this regard because you can easily build a custom VM image just for your app.

2 Likes

I see, thanks for the help.

1 Like

So it seems like with this there could be some automation of selecting which factory to use. I think the rest of my design probably still stands.