I just reread some code after Upgrading to a newer A12 version in Claims and was surprised to find a warning being shown. It turned out to be a warning about the usage of a deprecated method.
The interface in question is this:
/**
* Provides generic (de-)serialization functionality for {@link Document}s.
*/
public interface GenericDocumentSerializer<T, S extends Document> {
@Deprecated
T serialize(S document);
default T serialize(S document, boolean skipTransientFields) {
return serialize(document);
}
S deserialize(DocumentModel documentModel, T input);
}
Regarding the deprecation of the serialize(S) method:
- Are transient fields skipped if the deprecate method is used or not? The documentation of that method needs to state this, otherwise clients of the method need to dig into implementations to find this out.
- Could we please add a
@sincetag toserialize(S, boolean)so that we actually know when that new method was added to an interface?
And regarding the changed API of GenericDocumentSerializer - wouldn’t it have been better if skipping transient fields would be a detail of the implementations of that interface so that clients won’t be bothered with this? I think that when an instance is used somewhere then in most cases you would either always skip transient fields or include them always. And depending on what you want, you would have to choose on which instance of GenericDocumentSerializer you depend on. The problem is that in future there might be additional options to control the serialization process and this would start bloating that interface.
EDIT
I noticed that <INTERNAL_LINK> marks the transient property as non-breaking. I also saw that the implementations of GenericDocumentSerializer provided by A12 skip transient fields when using the deprecated method. However if you were using an instance of the GenericDocumentSerializer to transmit your documents to another system (like creating json for a Rest interface) than skipping transient fields might be viewed as breaking. Because before the introduction of the transient fields you could be sure that the document was the same after it had gone through the roundtrip of being serialized and deserialzed again. But now with the introduction this won’t hold for transient fields. Now it depends on what exactly transient fields means in this context. If it means those fields are just not persisted, then I would treat this as a breaking change. If it means that those fields are just not being made for serialization and are to be skipped always in such cases it is non breaking. However I would encourage the use of the first interpreation since the second is not very future proof when considering cloud and especially cloud native applications using a12 components.
EDIT 2
After reading again our code in Claims, I found that we depended on the first interpretation of what transient means and that it would have broken our code if we would have started using transient fields.