jsr372-experts@javaserverfaces-spec-public.java.net

[jsr372-experts] Re: [jsr372-experts mirror] Re: Re: Set and check component resource rendered via standard API

From: Bauke Scholtz <balusc_at_gmail.com>
Date: Fri, 22 Jul 2016 15:59:49 +0200

All,

It's now in the API as per
https://java.net/jira/browse/JAVASERVERFACES_SPEC_PUBLIC-1404 and Mojarra
implementation has been altered as per
https://java.net/jira/browse/JAVASERVERFACES-4166

This enables us to advance with
https://java.net/jira/browse/JAVASERVERFACES_SPEC_PUBLIC-1423

Cheers, B

On Fri, Jul 22, 2016 at 12:24 AM, Neil Griffin <
neil.griffin_at_portletfaces.org> wrote:

> +1
>
> The improved proposal and default implementation look good.
>
> Thanks!
>
> > On Jul 21, 2016, at 2:01 PM, Leonardo Uribe <leonardo.uribe_at_irian.at>
> wrote:
> >
> > Hi
> >
> > It has sense to use library / name instead Resource instance. In
> MyFaces, there is a cache holding Resource instances, so the instantiation
> is only a bit expensive the first time. In MyFaces there is also a
> ResourceIdentifier interface class called ResourceMeta, but its use is
> internal only. It helps to identify a resource in a unique way.
> >
> > But the map we are discussing here only have library / name into
> account, without take into account libraryVersion/resourceVersion or
> locale/contract. Its only purpose is to keep track of the rendered
> resources and avoid duplicates, at least when the full page is built. You
> only need library / name at that point of the algorithm, so if the methods
> uses library / name, it looks good to me.
> >
> > +1
> >
> > regards,
> >
> > Leonardo Uribe
> >
> >
> >
> > 2016-07-21 4:10 GMT-05:00 Bauke Scholtz <balusc_at_gmail.com>:
> > Here's a kickoff of the default implementation:
> >
> > /**
> > * <p class="changed_added_2_3">
> > * Mark the resource as identified by given resource and library name as
> rendered. The default implementation must
> > * ensure that {_at_link #isResourceRendered(FacesContext, String, String)}
> will return <code>true</code> when the
> > * resource has already been rendered during the render response phase
> of the current view.
> > * </p>
> > * @param context The {_at_link FacesContext} for this request.
> > * @param resourceName The name of the resource.
> > * @param libraryName The name of the library in which the resource
> resides, may be <code>null</code>.
> > * @since 2.3
> > */
> > public void markResourceRendered(FacesContext context, String
> resourceName, String libraryName) {
> > String resourceIdentifier = libraryName + ":" + resourceName;
> > Set<String> resourceIdentifiers = (Set<String>)
> context.getAttributes().computeIfAbsent(ResourceHandler.class.getName(), k
> -> new HashSet<>());
> > resourceIdentifiers.add(resourceIdentifier);
> > }
> >
> > /**
> > * <p class="changed_added_2_3">
> > * Returns whether the resource as identified by given resource and
> library name has been rendered. The default
> > * implementation must return <code>true</code> when the resource has
> been marked as rendered via
> > * {_at_link #markResourceRendered(FacesContext, String, String)} during
> the render response phase of the current view.
> > * </p>
> > * @param context The {_at_link FacesContext} for this request.
> > * @param resourceName The name of the resource.
> > * @param libraryName The name of the library in which this resource
> resides, may be <code>null</code>.
> > * @return Whether the resource as identified by given resource and
> library name has been rendered.
> > * @since 2.3
> > */
> > public boolean isResourceRendered(FacesContext context, String
> resourceName, String libraryName) {
> > String resourceIdentifier = libraryName + ":" + resourceName;
> > Set<String> resourceIdentifiers = (Set<String>)
> context.getAttributes().get(ResourceHandler.class.getName());
> > return resourceIdentifiers != null &&
> resourceIdentifiers.contains(resourceIdentifier);
> > }
> >
> >
> > Cheers, B
> >
> >
> > On Thu, Jul 21, 2016 at 11:02 AM, Bauke Scholtz <balusc_at_gmail.com>
> wrote:
> > While at it, should there be a default implementation too? Both Mojarra
> and MyFaces sets it as context attribute. They only do it differently.
> Mojarra puts name+library right away as key while MyFaces holds them all in
> a Map which in turn is stored as context attribute.
> >
> > I personally suggest holding a Set<String> or perhaps
> Set<ResourceIdentifier> as context attribute.
> >
> > Cheers, B
> >
> > On Thu, Jul 21, 2016 at 10:51 AM, Bauke Scholtz <balusc_at_gmail.com>
> wrote:
> > Hi,
> >
> > I do agree that UIComponent argument should be replaced. I only don't
> strongly agree that it should be a Resource instance as creating it is not
> cheap per se. Basically only the resource identifier (library+name) is
> sufficient for the task. There is however no such model in Java API. In
> OmniFaces there's one:
> https://github.com/omnifaces/omnifaces/blob/625ed0d52c640d7b8a25ba02295c2c77b8f75fe1/src/main/java/org/omnifaces/resourcehandler/ResourceIdentifier.java
> This should also reduce the need of carrying around loose but closely
> related name+library variables through the JSF impl.
> >
> > What do you guys think about taking over this in JSF API as
> javax.faces.model.ResourceIdentifier? Only, the current API such as
> createResource() would need another overload for that.
> >
> > Alternatively, we could also just ask for resource name and library
> directly.
> >
> > abstract void markComponentResourceRendered(FacesContext context, String
> library, String name);
> > abstract boolean isComponentResourceRendered(FacesContext context,
> String library, String name);
> >
> > That's also more consistent with existing ResourceHandler API. I only
> don't particularly like it, but I feel it's kind of too late to introduce a
> ResourceIdentifier as the loose variables are already used over all place
> in JSF API.
> >
> > Cheers, B
> >
> > On Wed, Jul 20, 2016 at 8:28 AM, Bauke Scholtz <balusc_at_gmail.com> wrote:
> > Hi,
> >
> > I was already doubting between UIComponent or a new ResourceIdentifier
> model with a.o. name and library properties. This should skip the need for
> a createResource call.
> >
> > Cheers, B
> >
> >
> > On Wed, Jul 20, 2016, 02:55 Leonardo Uribe <leonardo.uribe_at_irian.at>
> wrote:
> > Hi
> >
> > I have to change my +1 to +0.
> >
> > I just notice there is always a calculation of the underlying Resource
> instance,
> > before call the method, so it looks better if we pass the Resource
> instance
> > as parameter.
> >
> > public void markComponentResourceRendered(FacesContext context,
> > Resource resource, UIComponent componentResource)
> >
> > public boolean isComponentResourceRendered(FacesContext context,
> > Resource resource, UIComponent componentResource)
> >
> > The UIComponent instance could be optional. So it could be valid to call:
> >
> > ResourceHandler.isComponentResourceRendered(context, resource, null);
> >
> > The Resource instance is required, from that one it is easier to
> > get the libraryName/resourceName. If you have two different component
> > instances pointing to the same Resource, once the resource is rendered
> > you don't need to render the other one.
> >
> > From a theoretical perspective, a componentResource is a component
> > that:
> >
> > - Can be added to a specific target ("head", "body", "form").
> > - Could have an associated Resource instance (optional).
> >
> > It is just that if you only have the UIComponent instance, how you
> calculate
> > the underlying Resource instance is not clear, and that step can be
> different
> > between components.
> >
> > regards,
> >
> > Leonardo Uribe
> >
> >
> >
> > 2016-07-19 18:52 GMT-05:00 Leonardo Uribe <leonardo.uribe_at_irian.at>:
> > +1
> >
> > 2016-07-19 5:55 GMT-05:00 Bauke Scholtz <balusc_at_gmail.com>:
> > Hi,
> >
> > I have rephrased the javadocs on those methods.
> >
> >
> > /**
> > * <p class="changed_added_2_3">
> > * Mark given component resource as rendered. The default implementation
> must ensure that
> > * {_at_link #isComponentResourceRendered(FacesContext, UIComponent)} will
> return <code>true</code> when given
> > * component resource has already been rendered during the render
> response phase of the current view.
> > * </p>
> > * @param context The {_at_link FacesContext} for this request.
> > * @param componentResource The {_at_link UIComponent} representing a
> {_at_link Resource} instance.
> > * @since 2.3
> > */
> > public abstract void markComponentResourceRendered(FacesContext context,
> UIComponent componentResource);
> >
> > /**
> > * <p class="changed_added_2_3">
> > * Returns whether given component resource has been rendered. The
> default implementation must return
> > * <code>true</code> when given component resource has been marked as
> rendered via
> > * {_at_link #markComponentResourceRendered(FacesContext, UIComponent)}
> during the render response phase of the current
> > * view.
> > * </p>
> > * @param context The {_at_link FacesContext} for this request.
> > * @param componentResource The {_at_link UIComponent} representing a
> {_at_link Resource} instance.
> > * @return Whether given component resource has been rendered.
> > * @since 2.3
> > */
> > public abstract boolean isComponentResourceRendered(FacesContext
> context, UIComponent componentResource);
> >
> >
> > The literal part "current view" is important as this forces the
> implementation to clear out the marks when the view changes during render
> response, usually due to an exception which would show an error page view.
> >
> > Cheers, B
> >
> > On Tue, Jul 19, 2016 at 10:18 AM, Bauke Scholtz <balusc_at_gmail.com>
> wrote:
> > Hi,
> >
> > After all, adding those methods to ResourceHandler is cleaner API and
> allows better abstraction with a custom ResourceHandler.
> >
> >
> > /**
> > * <p class="changed_added_2_3">
> > * Mark given component resource as rendered.
> > * The default implementation stores the resource identifier as context
> attribute.
> > * </p>
> > * @param context The {_at_link FacesContext} for this request.
> > * @param componentResource The {_at_link UIComponent} representing a
> {_at_link Resource} instance.
> > * @since 2.3
> > */
> > public void markComponentResourceRendered(FacesContext context,
> UIComponent componentResource) {
> > // TODO
> > }
> >
> > /**
> > * <p class="changed_added_2_3">
> > * Returns whether given component resource has been rendered.
> > * The default implementation checks if the resource identifier has been
> stored as context attribute.
> > * </p>
> > * @param context The {_at_link FacesContext} for this request.
> > * @param componentResource The {_at_link UIComponent} representing a
> {_at_link Resource} instance.
> > * @return Whether given component resource has been rendered.
> > * @since 2.3
> > */
> > public boolean isComponentResourceRendered(FacesContext context,
> UIComponent componentResource) {
> > // TODO
> > }
> >
> >
> > If there are no objections, I will take care of it.
> >
> > Cheers, B
> >
> >
> >
> > On Thu, Sep 3, 2015 at 11:08 PM, Bauke Scholtz <balusc_at_gmail.com> wrote:
> > I created https://java.net/jira/browse/JAVASERVERFACES_SPEC_PUBLIC-1404
> on this.
> >
> > Cheers, B
> >
> > On Wed, Mar 4, 2015 at 6:11 PM, Bauke Scholtz <balusc_at_gmail.com> wrote:
> > By just adding it to the set.
> >
> > Cheers, B
> >
> >
> > On 16:25, Wed, Mar 4, 2015 arjan tijms <arjan.tijms_at_gmail.com> wrote:
> > Hi,
> >
> > Sounds good, but one question:
> >
> > If the proposed API just returns a Set<String> where presence of a
> > resource identifier means that resource has been rendered, how does
> > the library indicate it wants to suppress rendering of a certain
> > resource?
> >
> > Kind regards,
> > Arjan
> >
> >
> >
> >
> > On Wed, Mar 4, 2015 at 4:09 PM, Bauke Scholtz <balusc_at_gmail.com> wrote:
> > > In hindsight, that boolean is superfluous. Make it a mutable
> > > Set<ResourceIdentifier> or Set<String> and just consider the presence
> in the
> > > set as already rendered.
> > >
> > > Cheers, B
> > >
> > > On Wed, Mar 4, 2015 at 4:05 PM, Bauke Scholtz <balusc_at_gmail.com>
> wrote:
> > >>
> > >> Hi,
> > >>
> > >> Both Mojarra and MyFaces have an internal way to mark a script or
> > >> stylesheet resource as rendered (to avoid duplicate rendering).
> Mojarra sets
> > >> it as a context attribute with name+library as key and a boolean true
> as
> > >> value (see a.o. StylesheetRenderer#encodeEnd()). MyFaces sets it as a
> > >> context attribute via a Map<String, Boolean> (see a.o.
> > >> HtmlStylesheetRenderer#encodeEnd() via a ResourceUtils helper class).
> > >>
> > >> For component library developers it would be very useful if this
> > >> information is available by standard API means so that the component
> library
> > >> can if necessary check and/or suppress the rendering of those
> resources. For
> > >> example to combine those resources via a special resource handler, or
> to
> > >> automatically delegate to a CDN host, or to turn a script resource
> into a
> > >> deferred script, etcetera.
> > >>
> > >> I'd propose adding UIViewRoot#getRenderedComponentResources() which
> > >> returns a mutable Map<ResourceIdentifier, Boolean> for the purpose
> where
> > >> ResourceIdentifier could also be a String in the standard format
> > >> library+":"+name.
> > >>
> > >> What do others think about it?
> > >>
> > >> Cheers, B
> > >
> > >
> >
> >
> >
> >
> >
> >
> >
> >
> >
>
>