You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by "Radu Cotescu (Jira)" <ji...@apache.org> on 2021/01/15 12:32:00 UTC

[jira] [Comment Edited] (SLING-9999) Remove cyclic dependency between scripting and servlets features

    [ https://issues.apache.org/jira/browse/SLING-9999?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17266003#comment-17266003 ] 

Radu Cotescu edited comment on SLING-9999 at 1/15/21, 12:31 PM:
----------------------------------------------------------------

[~olli], I told you the reasoning for deprecating the [org.apache.sling.scripting.bundle.tracker|https://github.com/apache/sling-org-apache-sling-scripting-bundle-tracker] bundle: we wanted to bring the support for running a Sling web-app with bundled scripts into the core Sling modules. Before, that bundle was providing both the API and the implementation. We realised, though, that the implementations were better suited into {{org.apache.sling.servlets.resolver}} (for performance reasons) and {{org.apache.sling.scripting.core}} (to reduce code duplication, since we're essentially providing a {{ScriptContext}} to those bundled scripts).

Option no. 3 is actually something that would make sense: the new API would be extracted into a separate bundle, with the implementations kept like they were before your commits. This way the dependency chains are kept clean and we would still benefit from the performance improvements and reduced code duplication. So, like [~karlpauls] mentioned, if you revert your changes we can go into this direction without any issues. The API namespaces stay the same, so we don't introduce any backwards incompatible changes, at the cost of one additional API bundle. WDYT?


was (Author: radu.cotescu):
[~olli], I told you the reasoning for deprecating the [org.apache.sling.scripting.bundle.tracker|https://github.com/apache/sling-org-apache-sling-scripting-bundle-tracker] bundle: we wanted to bring the support for running a Sling web-app with bundled scripts into the core Sling modules. Before, that bundle was providing both the API and the implementation. We realised, though, that the implementations were better suited into {{org.apache.sling.servlets.resolver}} (for performance reasons) and {{org.apache.sling.scripting.core}} (to reduce code duplication, since we're essentially providing a {{ScriptContext}} to those bundled scripts).

Option no. 3 is actually something that would make sense: the new API would be extracted into a separate bundle, with the implementations kept like they were before your commits. This was the dependency chains are kept clean and we would still benefit from the performance improvements and reduced code duplication. So, like [~karlpauls] mentioned, if you revert your changes we can go into this direction without any issues. The API namespaces stay the same, so we don't introduce any backwards incompatible changes, at the cost of one additional API bundle. WDYT?

> Remove cyclic dependency between scripting and servlets features
> ----------------------------------------------------------------
>
>                 Key: SLING-9999
>                 URL: https://issues.apache.org/jira/browse/SLING-9999
>             Project: Sling
>          Issue Type: Improvement
>          Components: API, Karaf, Scripting, Servlets, Starter
>            Reporter: Oliver Lietz
>            Assignee: Oliver Lietz
>            Priority: Major
>
> Before {{org.apache.sling.scripting.core.impl.bundled}} and {{org.apache.sling.servlets.resolver.bundle}} were added the dependency chains were e.g.
> {{sling-servlets}} → {{sling-scripting}} → {{sling}}
> {{sling-scripting-jsp}} → {{sling-scripting}} → {{sling}}
> Currently several _scripting_ modules depend on {{org.apache.sling.servlets.resolver.bundle.tracker}}.
> h2. Move _Bundle_ API to Scripting and Resource packages (modules)
> ||Actual Package (Module)||Target Package (Module)||
> |*{{org.apache.sling.servlets.resolver.bundle.tracker.BundledRenderUnit}}* ({{org.apache.sling.servlets.resolver}})|*{{org.apache.sling.scripting.api.bundle.BundledRenderUnit}}* ({{org.apache.sling.scripting.api}})|
> |*{{org.apache.sling.servlets.resolver.bundle.tracker. BundledRenderUnitCapability}}* ({{org.apache.sling.servlets.resolver}})|*{{org.apache.sling.scripting.api.bundle.BundledRenderUnitCapability}}* ({{org.apache.sling.scripting.api}})|
> |*{{org.apache.sling.servlets.resolver.bundle.tracker.BundledRenderUnitFinder}}* ({{org.apache.sling.servlets.resolver}})|*{{org.apache.sling.scripting.api.bundle.BundledRenderUnitFinder}}* ({{org.apache.sling.scripting.api}})|
> |*{{org.apache.sling.servlets.resolver.bundle.tracker.ResourceType}}* ({{org.apache.sling.servlets.resolver}})|*{{org.apache.sling.api.resource.type.ResourceType}}* ({{org.apache.sling.api}})|
> |*{{org.apache.sling.servlets.resolver.bundle.tracker.TypeProvider}}* ({{org.apache.sling.servlets.resolver}})|*{{org.apache.sling.scripting.api.bundle.TypeProvider}}* (_*{color:#ff8b00}class name?{color}*_) ({{org.apache.sling.scripting.api}})|
> ----
> ||Module||Commits||Status||
> |{{org.apache.sling.api}}|[02cb7f1|https://github.com/apache/sling-org-apache-sling-api/commit/02cb7f1bbc4836865afd7e9964d3be1380daf734]|(/)|
> |{{org.apache.sling.scripting.api}}|[d38759d|https://github.com/apache/sling-org-apache-sling-scripting-api/commit/d38759dbd3d4159f70ea54bff23b037be8d07cd6]|(/)|
> |{{org.apache.sling.scripting.core}}|[b2f368a|https://github.com/apache/sling-org-apache-sling-scripting-core/commit/b2f368a90a087979c34d8072fe529675971234fe], [1467cfa|https://github.com/apache/sling-org-apache-sling-scripting-core/commit/1467cfa6a3fe4c77a628d078f9d944ce4cc42eb1]|(/)|
> |{{org.apache.sling.scripting.jsp}}|[ccdb902|https://github.com/apache/sling-org-apache-sling-scripting-jsp/commit/ccdb90266f254bb2143a5d870725a8f4b28d4d8b]|(/)|
> |{{org.apache.sling.scripting.sightly}}|[1567894|https://github.com/apache/sling-org-apache-sling-scripting-sightly/commit/1567894f6c1835a317c01e6c5b5987a260a74757], [b2f57cd|https://github.com/apache/sling-org-apache-sling-scripting-sightly/commit/b2f57cd43d7d961fcf194cf7bf61b1c79f343d44]|(/)|
> |{{org.apache.sling.servlets.resolver}}|[8262ce6|https://github.com/apache/sling-org-apache-sling-servlets-resolver/commit/8262ce63725b9ab93c1d3eb07ace6b02d79acc65]|(!) probably more classes which should be living in a scripting module|



--
This message was sent by Atlassian Jira
(v8.3.4#803005)