adobe / aem-core-wcm-components

Standardized components to build websites with AEM.
https://docs.adobe.com/content/help/en/experience-manager-core-components/using/introduction.html
Apache License 2.0
730 stars 736 forks source link

Model interface evolution #16

Closed davidjgonzalez closed 6 years ago

davidjgonzalez commented 7 years ago

How is the evolution of a Model interface handled, since the interface itself is not versioned?

For example, in v1 suppose the Title interface defines getTitle() and getType().

If in v2, there is a desire to expose a sub-title via getSubTitle(), would this be considered a new "component" and/or require a new interface?

justinedelson commented 7 years ago

@davidjgonzalez this should be covered in the 6.3 documentation as well.

kwin commented 7 years ago

Is this already being published? I don't find any hints on this in https://docs.adobe.com/docs/en/aem/6-3/develop/components/core-components/using.html.

raducotescu commented 7 years ago

@davidjgonzalez, @kwin, I think this is covered by https://github.com/Adobe-Marketing-Cloud/aem-core-wcm-components/wiki/Versioning-policies.

kwin commented 7 years ago

How do you deal with major updates then? I.e. either a semantic change of an existing method or a new method without default implementations. Since the references from the HTL script do not indicate any version, all HTL scripts by default get bound to the most specific implementation. Therefore in such a case, you could no longer run code in parallel which still relies on the old version, because by default every script will get bound to the newest impl. Therefore the only option I see is to either rename the interface or put it in another package for a major version upgrade. Is that the policy here for major upgrades?

Also I don't see the reason here why the actual implementations have a version string in their package name (https://github.com/Adobe-Marketing-Cloud/aem-core-wcm-components/tree/master/bundles/core/src/main/java/com/adobe/cq/wcm/core/components/models/impl/v1). The implementations should never be referenced (except from classes which derive from those). Could you share why the impl package name contains the version? What advantage do you get from that?

lbaoyu commented 7 years ago

Interfaces are located in package com.adobe.cq.wcm.core.components.models with no version info. I understand it as interface is the abstraction for what a core component represents and it is stable. Changing it will require creating a completely new component.

Since these are core component to be used by all AEM projects, it is ok to expose only core or mandatory functionalities in the interface and leave the optional ones.

raducotescu commented 7 years ago

@kwin, the decision to version the models implementations by using the version info in their sub-packages was made so that it's clear for us what model supports which version of a component.

Since we rely on Sling Models >= 1.3.0, the binding between the resource and the model implementation is actually done through the resource type - https://sling.apache.org/documentation/bundles/models.html#associating-a-model-class-with-a-resource-type-since-130, so scripts will not always be bound to the latest implementation.

@lbaoyu, we're still working on the versioning policy and we'll update this soon (https://github.com/Adobe-Marketing-Cloud/aem-core-wcm-components/wiki/Versioning-policies). We will not expose only mandatory functionalities in the interface. The interface is the API contract and will provide API version information for each method. The implementations though can selectively pick which API version they want to support.

kwin commented 7 years ago

@raducotescu Thanks for your explanation. It is still not 100% clear to me, how you can run two different model interface versions in parallel on the same instance though. The HTL scripts would allow that due to the different paths in the repository but the model interface classes which are referenced from the HTL don't. So if in the future there is a 2.0 version for e.g Image (https://github.com/Adobe-Marketing-Cloud/aem-core-wcm-components/blob/master/bundles/core/src/main/java/com/adobe/cq/wcm/core/components/models/Image.java) this cannot easily be deployed at the same time.

Although OSGi allows several bundles exporting the same packages in different versions, this is not supported by

Therefore I am wondering if supporting multiple major versions in parallel is just not possible and what your policy is then about delivering updates/fixes for not the most recent major version (and the according deprecation policy). This is especially important when you extend or implement such an interface on your own.

raducotescu commented 6 years ago

Since now we've pushed the development branch I can provide you with more details:

Please let me know if you need more details.

davidjgonzalez commented 6 years ago

I'm not clear on bullet #2 - do you have an example?

raducotescu commented 6 years ago

Sure. Check the Navigation interface [0] and its implementation NavigationImpl [1]. Assuming the next version of the API would add a new method in the interface, an implementation bundle not providing the behaviour for this method would not fail to start due to API incompatibilities.

This new method will only affect a project if an HTL script starts using it, since the interface's default behaviour is to throw an UnsupportedOperationException. So technically you can install the new API with its default implementations, without affecting projects relying on the older API versions.

[0] - https://github.com/Adobe-Marketing-Cloud/aem-core-wcm-components/blob/development/bundles/core/src/main/java/com/adobe/cq/wcm/core/components/sandbox/models/Navigation.java [1] - https://github.com/Adobe-Marketing-Cloud/aem-core-wcm-components/blob/development/bundles/core/src/main/java/com/adobe/cq/wcm/core/components/sandbox/internal/models/v1/NavigationImpl.java

kwin commented 6 years ago

Thanks for the information. Just one question around the version of the java package of the interface: Do you follow semantic versioning rules there? That would mean that with each new major version the custom impl would need to be recompiled against that version.