Yeah, I need to give it another read to fully grasp the proposed design.
For the initial step, I made a minimal change to test this and it works like a charm. Also, changing fields order did not cause any issues.
Here are the changes I made in case someone wants to have a look:
Edit:
Gson also does a similar thing except that it uses reflection to access Class.getRecordComponents() to keep compatibility with older Java versions.
Looks good! I do have one comment regarding line 105: this approach might make it a bit harder to evolve your designs moving forward. Whether that’s a good thing or a bad thing is a matter of opinion, but as it is if you add a field to a record this will fail to load saved data from older versions. The benefit of this is that it forces you to explicitly handle this case (probably by adding the column in the DB with a default value) rather than relying on assumptions about forward-compatibility (such as assuming that fields missing from the serialized data should be null/0/false/Optional.empty()). The downside is that probably in many cases defaulting the field to an “empty” value is a safe option and this approach forces you to be explicit about it.
All that said though, handling runtime-default values for missing fields opens up a whole can of worms that’s probably out of scope of your goal here, and it has all its own subleties.
Thanks for the review,
I think I can remove that check as it will never happen I guess.
This does not check it against DB columns mismatch. The fields in the above code is a list of FieldType (zay-es stuff) that is constructed using Class.getDeclaredFields() while statics fields, transient fields,… are filtered out. I then order that list to be in the same order with getRecordComponents().
Here is the code for reference:
I submitted a PR based on what you suggested, please take a look at it and let me know what you think.
This is how I implemented my own custom factory for record:
public class RecordComponentFactory<T> implements SqlComponentFactory<T> {
private final FieldType[] fields;
private final Constructor<T> ctor;
public RecordComponentFactory(Class<T> type) {
List<FieldType> types = FieldTypes.getFieldTypes(type);
this.fields = types.toArray(new FieldType[types.size()]);
// We use canonical constructor for records instantiating
try {
RecordComponent[] components = type.getRecordComponents();
if (fields.length != components.length) {
throw new RuntimeException("Record fields and components mismatch:" + type);
}
// Sort the fields to match with record components order
List<String> names = Arrays.stream(components).map(RecordComponent::getName).toList();
Arrays.sort(fields, Comparator.comparingInt(f -> names.indexOf(f.getFieldName())));
for (int i = 0; i < components.length; i++) {
if (!components[i].getName().equals(fields[i].getFieldName())) {
throw new RuntimeException("Record fields and components mismatch:" + type);
}
}
Class<?>[] paramTypes = Arrays.stream(components)
.map(RecordComponent::getType)
.toArray(Class<?>[]::new);
ctor = type.getDeclaredConstructor(paramTypes);
} catch (NoSuchMethodException e) {
throw new IllegalArgumentException("Record does not have a canonical constructor:" + type, e);
}
}
@Override
public FieldType[] getFieldTypes() {
return fields;
}
@Override
public T createComponent(ResultSet rs) throws SQLException {
try {
Object[] args = new Object[fields.length];
int columnIndex = 1;
for (int i = 0; i < fields.length; i++) {
FieldType field = fields[i];
columnIndex = field.readIntoArray(args, i, rs, columnIndex);
}
return ctor.newInstance(args);
} catch (InvocationTargetException | InstantiationException | IllegalAccessException e) {
throw new RuntimeException("Error in table mapping", e);
}
}
}
and this how I use it:
SqlEntityData ed = new SqlEntityData("/home/ali/Desktop/edb", 500) {
@Override
protected <T extends EntityComponent> SqlComponentFactory<T> lookupDefaultFactory(Class<T> type) {
if (type.isRecord()) {
return new RecordComponentFactory<>(type);
}
return super.lookupDefaultFactory(type);
}
};
Edit:
Note, this does not support non-record sql components that have an internal field of record type or record sql components that have an internal field of record type.
I’ll try to take a look this weekend.
A bit of explanation regarding the new FieldType.readIntoArray which is also inspired by the Gson library.
this reads the sql result set into an array instead of directly applying it to the target object fields via reflection. This is required for records, using this I first read all record fields into an array then pass it as parameters to the canonical constructor of the record in my RecordComponentFactory I posted above.
The reason I am not including RecordComponentFactory in the PR is that I am using getRecordComponents() method which is added in Java 16. I need it to grab the record canonical constructor.
If you like RecordComponentFactory to be included in the library, we can either add a new sub-project (e.g. zay-es-proto) in the zay-es repo and add this class there and set source compatibility for this sub-project to Java 16, otherwise, I can re-write this class to call Class.getRecordComponents(), Class.isRecord(), RecordComponent.getType() and RecordComponent.getName() using reflection (Gson does this) which then can be included in the main repo without bumping java version. Let me know what you think.
@pspeed have you had a chance to take a look at my PR? Is there any chance for it to get integrated?
I have not had a chance to take a look yet. It is still on my to-do list. I’m also trying to get a Mythruna release ready.
Sorry for the delay.
Finally merged this. Sorry for the delay.
I think “record” support itself was not added until Java 14… so probably even Class.isRecord(), etc. is not there in jdk8 or even jdk11. I’m hesitant to cut off these older versions as yet. The idea of an extension is an interesting one… could be zay-es-records or something. That’s up to you… it does add a tiny bit to my release burden but probably not enough to matter.
Thanks 