A while back we had a similar discussion. Now I think this distinction
between real and fake requests could be completely removed, simplifying
the models impl a lot - if we enhance the contract for servlet listeners
being also called by Sling Engine for internal requests (which often use
those fake request objects).
Regards
Carsten
On 26.07.2023 17:29, Dirk Rudolph wrote:
Hey Paul,
afaik the marker attribute it used to distinguish between real and
synthetic request objects.
The ServletRequestListner implementation of the ModelAdapterFactory is
only called for requests created by the servlet container.
There are use cases that do not have access to a request object and
create a synthetic instance to adapt it to a Sling Model (directly or
indirectly), for example using the SlingHttpServletRequestBuilder [1].
In this case requestDestroyed is never called with that request object
and hence the cleanup mechanism needs to fall back to the one used for
non-Request adaptables.
Best regards,
Dirk
[1]:
https://sling.apache.org/apidocs/sling12/org/apache/sling/api/request/builder/SlingHttpServletRequestBuilder.html
On Wed, 19 Jul 2023 at 22:11, Paul Bjorkstrand
<[email protected]> wrote:
I'm looking into a potential refactor of the Sling Models Impl to make the
code easier to manage, but also with an eye to enhancing the functionality
in some ways that are hard to do with the way the code is currently
structured Examples of such improvements: support other "container types"
in a generic fashion (Optional, WeakReference, and all Collections/arrays)
and implement a java.lang.invoke.MethodHandle based injection system. One
area I have almost no knowledge about is the cleanup side with the disposal
registry, and its related functionality.
As I have been reading through the code, I immediately noticed that there
are two paths that are taken, depending on a couple factors, most notably
whether the adaptable is a request or not. At first glance, it seems that
this decision is a necessary difference between request adapted models or
resource adapted models. Digging in deeper, I believe this is not the case,
and it is just a performance/reliability improvement. This makes it not
necessary to rely on PhantomReference for request adapted models, since
there is a servlet request listener that notifies creation and cleanup of
the requests. This seems to be confirmed after reading [1].
What I don't quite understand is the purpose of REQUEST_MARKER_ATTRIBUTE
[2] [3] [4] [5]. What does that do, and why is it needed? Any help
understanding this is appreciated!
[1]: https://issues.apache.org/jira/browse/SLING-5668
[2]:
https://github.com/apache/sling-org-apache-sling-models-impl/blob/master/src/main/java/org/apache/sling/models/impl/ModelAdapterFactory.java#L106C33-L106C58
[3]:
https://github.com/apache/sling-org-apache-sling-models-impl/blob/master/src/main/java/org/apache/sling/models/impl/ModelAdapterFactory.java#L688
[4]:
https://github.com/apache/sling-org-apache-sling-models-impl/blob/master/src/main/java/org/apache/sling/models/impl/ModelAdapterFactory.java#L774
[5]:
https://github.com/apache/sling-org-apache-sling-models-impl/blob/master/src/main/java/org/apache/sling/models/impl/ModelAdapterFactory.java#L1392
// Paul
--
Carsten Ziegeler
Adobe
[email protected]
