Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SharedComponentPropertiesBindingsValuesProvider should support LazyBindings #2960

Open
joerghoh opened this issue Oct 5, 2022 · 1 comment

Comments

@joerghoh
Copy link
Contributor

joerghoh commented Oct 5, 2022

Required Information

  • AEM Version, including Service Packs, Cumulative Fix Packs, etc: AEM 6.5, AEM CS
  • ACS AEM Commons Version: all
  • Reproducible on Latest? yes

Expected Behavior

The SharedComponentPropertiesBindingsValuesProvider always resolves all values before they are added to the bindings; this happens irrespective if these values are required or not.
When using Lazybindings (https://sling.apache.org/apidocs/sling12/org/apache/sling/api/scripting/LazyBindings.html) this could be avoided, and values are only resolved when they are needed.

Actual Behavior

the values are always computed, leading into performance degradation.

@adamcin
Copy link
Contributor

adamcin commented Oct 7, 2022

@joerghoh TL;DR: To take advantage of LazyBindings, we have to implement the Supplier type it introduces. Unless we use reflection to access the LazyBindings type, this will require an upgrade of our org.apache.sling.api dependency version to at least 2.22.0, which will require dropping support for AEM versions prior to 6.5 SP7.

org.apache.sling.api.scripting.LazyBindings was introduced in org.apache.sling.api:2.22.0. Between this and its previous release artifact, the org.apache.sling.api.scripting package exported version was bumped from 2.3.2 to 2.5.0, which will introduce a breaking change to the Import-Package range for deployment to previous AEM releases (the current version of org.apache.sling.api we depend on is 2.16.4, which exports the .scripting package at version 2.3.1) . The latest version of this package in the apache/sling-org-apache-sling-api master branch is still 2.5.0.

In this project, the org.apache.sling.api dependency version is transitively managed by the io.wcm.maven:aem-dependencies pom import. In both our master branch and the acs-aem-commons-6.0.0 branch, that pom version is currently set to 6.4.0.0004. This pom import dependency will need to be upgraded to at least 6.5.7.0000 to bring along the necessary org.apache.sling.api version without adding an explicit dependency to the acs-aem-commons-bundle. The latest incremental version of the 6.5.7 (AEM 6.5 SP7) pom is 6.5.7.0002, while the latest AEM SP pom is 6.5.11.0000. The same org.apache.sling.api:2.22.0 dependency version is defined for the entire range between these versions, so while I will likely include an upgrade to 6.5.7.0002 in my PR, it may spark a discussion about which AEM 6.5 SP level the project should support when the decision is made to drop support for AEM 6.4.

adamcin added a commit to adamcin/acs-aem-commons that referenced this issue Oct 8, 2022
adamcin added a commit to adamcin/acs-aem-commons that referenced this issue Oct 8, 2022
adamcin added a commit to adamcin/acs-aem-commons that referenced this issue Oct 8, 2022
adamcin added a commit to adamcin/acs-aem-commons that referenced this issue Oct 8, 2022
adamcin added a commit to adamcin/acs-aem-commons that referenced this issue Oct 8, 2022
adamcin added a commit to adamcin/acs-aem-commons that referenced this issue Oct 8, 2022
adamcin added a commit to adamcin/acs-aem-commons that referenced this issue Oct 9, 2022
adamcin added a commit to adamcin/acs-aem-commons that referenced this issue Oct 9, 2022
adamcin added a commit to adamcin/acs-aem-commons that referenced this issue Oct 9, 2022
adamcin added a commit to adamcin/acs-aem-commons that referenced this issue Oct 9, 2022
adamcin added a commit to adamcin/acs-aem-commons that referenced this issue Oct 9, 2022
adamcin added a commit to adamcin/acs-aem-commons that referenced this issue Oct 9, 2022
adamcin added a commit to adamcin/acs-aem-commons that referenced this issue Oct 9, 2022
adamcin added a commit to adamcin/acs-aem-commons that referenced this issue Oct 9, 2022
adamcin added a commit to adamcin/acs-aem-commons that referenced this issue Oct 9, 2022
adamcin added a commit to adamcin/acs-aem-commons that referenced this issue Oct 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants