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: Josh Juneau <juneau001_at_gmail.com>
Date: Sat, 23 Jul 2016 16:57:01 -0500

Great news, thanks for all of your hard work in improving the API.

Josh Juneau
juneau001_at_gmail.com
http://jj-blogger.blogspot.com
https://www.apress.com/index.php/author/author/view/id/1866


On Fri, Jul 22, 2016 at 8:59 AM, Bauke Scholtz <balusc_at_gmail.com> wrote:

> 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
>> > >
>> > >
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>> >
>>
>>
>