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