Core Component interface methods should not throw UnsupportedOperationException by default

[stefanseifert]

with the new version 1.1.0 of the core componts all interface methods where enhances with a default implementation throwing and "UnsupportedOperationException" by default.

this seems quite useless to me and dangerous regarding semantic versioning. i assume this was introduced to support interface changes in the future when new methods added to @ConsumerType-annotated classes would lead to a major version increase of the package version, requiring all code that uses it to recompile or relax their osgi import instructions.

but adding a new interface method throwing an UnsupportedOperationException by default "solves" the problem with the baseline check of the maven-bundle-plugin - but in a bad way. but it just moves the potential problem from compile time to run time - where it is much harder to detect. code relying on the latest version of the interface may call one of the new methods and just getting an unexpected exception because the implementation was compiled against the old interface version.

my proposal is:

• for the existing interfaces, remove all default implementations throwing UnsupportedOperationException
• if a new interface methods is added add a default implementation that really makes sense - e.g. calculation the result form the other interface methods, or returning null if the result is optional and this makes sense for existing implementation
• if this is not possible for a new interface methods, we need to update the package version to signal all users and implementors of the interface that they have to recompile and recheck their code against the new contract

[vladbailescu]

Hi Stefan,

The models defined by the Core Components are intended to be consumed from the rendering scripts. We expect people to maybe provide their own implementation as an extension. Having said that, we had two scenarios that we wanted to cover:

1. You start developing siteA with version 1.x of the Core Components. After that is launched you begin to develop siteB on the same AEM infrastructure. By that time version 2.x of the Core Components is available and you would like to use that. We wanted to enable you to replace version 1x of the Core Components with version 2.x, without having to touch siteA. That's why we decided to add default methods.

Being able to have multiple versions of the Core Components running simultaneously and sharing the same models was one of the design goals.

2. Suppose you start extending one of the Core Components and override the rendering script and the model implementation. In case you want to simplify things, you might only implement a few of the methods in the model interface, those that are being used by your script. If someone revisits the script and tries to make use of an unimplemented method, they should get an error when they call that method. That's why by default we throw UnsupportedOperationException.

As a side note, the rendering scripts are using HTL which uses reflection to resolve use-object methods so any missing method would only be discovered at runtime anyway. We believe having the default methods return something meaningful (or at least harmless) would make mismatches between model implementation and rendering scripts less obvious.

We presented this pattern at adaptTo() 2017 and was a bit surprised it didn't stir much controversy. Maybe I went over it too quickly or it got lost in the huge sea of information available during the conference. It was also used in the development branch for some time already.

This is the best compromise we managed to come up with, so far. We are very interested in any ideas on how we can improve this pattern while also keeping the aforementioned scenarios valid.

[stefanseifert]

i see your points - but still, from an OSGi perspective this feels "wrong" for me.

In case you want to simplify things, you might only implement a few of the methods in the model interface, those that are being used by your script

in this case you are still effectively breaking the contract the interface defines, but making it silent so neither the compiler nor the baseline plugin complains. if you decide you need only a subset, drop the interface at all and create a custom component with it's own model class.

why are the model interface are not versioned in the package name as well? so you can switch within the script from v1 core component interface to v2 core component interface. and you can still neatly deploy core components v1 and v2 on the same instance, and choose in each application (to be precise in each component of each application) which version to use.

[raducotescu]

While in a purely OSGi context this would be an anti-pattern (for the reasons mentioned by @stefanseifert above), for the Core Components this is a pattern that would allow developers to seamlessly upgrade to the latest version of the Core Component project without affecting pre-existing projects relying on Core Components deployed on the same AEM instance. I agree that this could also be solved with versioning Java packages (e.g. com.adobe.cq.wcm.core.components.models.v1...com.adobe.cq.wcm.core.components.models.vN), but something like this would lead to exporting too many API packages, with artificial API versions (1.x.y).

Maintaining multiple API packages would also lead to code duplication, since a new API version for a model might still keep some of the old methods.

By using the default methods approach an UnsupportedOperationException would only be thrown if a developer tries to use new API in a script relying on a custom implementation that hasn't been adapted to use the new API: this is a highly unlikely case.

Let's also not forget that if a custom implementation for a Core Components Sling Model exists, it's bound to a specific resource type (the proxy component used by the project), so an upgrade of the Core Components bundle would not affect the implementation resolution.