What is the use case of having a fake request object, adapting it to a
model and not processing it through the Sling engine?
Regards
Carsten
On 27.07.2023 09:15, Dirk Rudolph wrote:
How would this avoid the queue?
Theoretically I can have a
o.a.s.api.request.builder.impl.SlingHttpServletRequestImpl instance
and adapt it to a model class without going through the Sling Engine.
We need the queue also for adaptables that are not necessarily bound
to the request lifecycle (e.g. Resource).
Regards,
Dirk
On Thu, 27 Jul 2023 at 06:50, Carsten Ziegeler <[email protected]> wrote:
Hi,
I think at some point the servlet engine was actually reusing request
objects and we ran into trouble with an async cleanup based on request
objects and their attributes. Switching to the listener solved the issue.
I'm not sure whether this is still the same or not. But I think getting
the listener support into the Engine is not that complicated and it
avoids the queue.
Regards
Carsten
On 26.07.2023 21:17, Paul Bjorkstrand wrote:
That was very helpful, thank you. Since requests would be (eventually)
garbage collected by the JVM, I feel that a reference/queue-only approach
could be sufficient for all types of cleanup. Sure, the servlet listener
method could improve the timing of when the cleanup executes, but I imagine
it is not necessary (unless something, somewhere in the servlet framework
plumbing is caching request objects and reusing them...which I hope not).
I appreciate the insights!
//Paul
On Wed, Jul 26, 2023 at 10:50 AM Carsten Ziegeler <[email protected]>
wrote:
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]
--
Carsten Ziegeler
Adobe
[email protected]
--
Carsten Ziegeler
Adobe
[email protected]
