users@javaserverfaces-spec-public.java.net

[jsr344-experts mirror] [jsr344-experts] Re: Flow and Loading Facelets via ResourceHandler (Re: was: PRD Review and pending issues)

From: Leonardo Uribe <lu4242_at_gmail.com>
Date: Thu, 31 Jan 2013 22:17:20 -0500

Hi

2013/1/31 Edward Burns <edward.burns_at_oracle.com>:
> SECTION: Responses relating to Flow
>
>>>>>> On Mon, 14 Jan 2013 13:41:17 -0500, Leonardo Uribe <lu4242_at_gmail.com> said:
>
> LU> I have checked PRD documentation and I have noticed some pending
> LU> points that it could be good to keep in mind. This is important
> LU> feedback, and in my opinion these points needs to be solved.
>
> LU> ------------------------------------------------------------------------
> LU> Faces Flow
> LU> ------------------------------------------------------------------------
>
> LU> - It seems there are still some examples using the old syntax inside .xhtml
> LU> files and there are people who still believes that syntax is valid, but
> LU> in the spec documentation there are only two ways to define a flow: using
> LU> xml file and using @FlowDefinition.
>
> Yes, not just some, but *all* of the examples that use XML defined flows
> still use the Facelets syntax. Until I close JAVASERVERFACES-2580 to
> that's how it will be. I've been keeping the tag handlers up to date
> with respect to the XSD, so it will be trivial to change the examples
> once 2580 is closed.
>

Ok.

> LU> - In JSF 2.2 section 7.5.1, it says something like this:
>
> LU> "... When the application containing these flows is deployed, the
> LU> runtime discovers the flow definitions and adds them to the internal
> LU> flow data structure. One flow is defined in flow-b-flow.xml. The
> LU> other flow is defined in FlowA.java, an @Named bean with the
> LU> @FlowDefinition annotation. ..."
>
> LU> That's ok, but I think it is not clear that flow-b-flow.xml is a xml
> LU> file that has a <faces-config> definition inside (note I'm
> LU> speculating here). I suppose that file should allow only a subset
> LU> of <faces-config> (navigation rules, managed beans definitions maybe
> LU> components/renderers?), or maybe it is open to define anything that
> LU> can be defined in a faces-config (which I doubt it).
>
> I wasn't planning on requiring any kind of constraint to what kinds of
> XML can be contained in such files. I have added this text.
>
> This is an XML file conforming to the Application Configuration
> Resources syntax described in Section 11.4 "Application Configuration
> Resources".
>
> I also added text at the top of 11.4 that refers to the complete XSD
> (and the XSDDoc generated HTML documentation) which is now bundled with
> the javadocs.
>

Ok.

> LU> And how @FlowDefinition works? the documentation doesn't
> LU> say anything, so I just can't say anything. In my opinion, this part has
> LU> sense but it requires some clarifications first.
>
> I will flesh out that javadoc, but here is an example that currently
> runs in our mojarra tests:
>
> public class FlowA implements Serializable {
>
> private static final long serialVersionUID = -7623501087369765218L;
>
> public FlowA() {
> }
>
> @Produces @FlowDefinition
> public Flow defineFlow(@FlowBuilderParameter FlowBuilder flowBuilder) {
> String flowId = "flow-a";
> flowBuilder.id("", flowId);
> flowBuilder.viewNode(flowId, "/" + flowId + "/" + flowId + ".xhtml").markAsStartNode();
> flowBuilder.returnNode("taskFlowReturn1").
> fromOutcome("#{flow_a_Bean.returnValue}");
> flowBuilder.methodCallNode("outcome-from-method").expression("#{flow_a_Bean.methodWithOutcome}").defaultOutcome("taskFlowReturn1");
> flowBuilder.methodCallNode("outcome-from-markup").expression("#{flow_a_Bean.voidMethod}").defaultOutcome("taskFlowReturn1");
>
> return flowBuilder.getFlow();
> }
>
> }
>

Now it has sense, thanks for the example.

> LU> - In JSF 2.2 section 11.4.3.1, it talks about "Packaging Faces Flows
> LU> in JAR Files". Take a look at these lines:
>
> LU> "... Any flow nodes included in the jar must be located within sub entries
> LU> of the META-INF/flows/<flowName> JAR entry, where <flowName> is a JAR
> LU> directory entry whose name is identical to that of a flow id in the
> LU> corresponding faces-config.xml file. ..."
>
> LU> What is a "flow node"? is it a real view that JSF can load? does that
> LU> means that the vdl should be able to load files under
> LU> META-INF/flows/<flowName> like if they were in a webapp directory?.
>
> In the non-normative example in section 7.5, I added the text, "the
> different kinds of nodes mentioned in the preceding discussion are
> defined in the javadoc for class javax.faces.flow.FlowHandler."
>
> In section 11.4.3.1, I changed "flow nodes" to "view nodes". Yes, it is
> a real JSF Facelets view.
>

Ok.

> LU> Note according to the previous answers, it should be some changes or
> LU> clarification about how ResourceHandler.createViewResource() works.
>
> EXCELLENT CATCH Leonardo. Thank you. I've fixed that. It will be in
> the next javadocs published to
> <http://jsf-spec.java.net/SNAPSHOT/javadoc/>.
>
> SECTION: Relating to ViewActions
>
> LU> ------------------------------------------------------------------------
> LU> View Actions
> LU> ------------------------------------------------------------------------
>
> LU> - The changes previously proposed are still pending, so any snapshot to
> LU> check how they should looks like is welcome.
>
> Thank you for reminding me.
>
> On 14 Decmeber 2013, Leonardo Uribe wrote:
>
> LU> I would suggest a method in ViewMetadata that do the check for
> LU> execute full lifecycle or avoid renderResponse() call, and the
> LU> implementation by default
>
> [...]
>
> LU> In conclusion, one way or another what matters here is allow
> LU> f:viewAction to be used without a dependency of f:viewParam, so any
> LU> solution that goes into that direction is ok.
>
> I did add ViewMetadata.getViewActions(), for parity with
> ViewMetadata.getViewParameters(). However, I also added
> boolean ViewMetadata.hasMetadata(UIViewRoot root) and modified 2.2.1 to
> simply call that method instead of all that other complicated logic.
>
> This was committed today on r11511 on the RI trunk.
>

Ok, any snapshot of the spec is welcome.

> SECTION: Application.createComponent().
>
> LU> ------------------------------------------------------------------------
> LU> Application.createComponent(FacesContext context, String taglibURI,
> LU> String tagName, Map<String,Object> attributes)
> LU> ------------------------------------------------------------------------
>
> LU> Here we have an implementation dependency between the VDL and the
> LU> Application object. It says that it creates a component given a VDL,
> LU> so it means Application object must call an implementation-specific
> LU> method from the VDL (in facelets case a cast to
> LU> FaceletsViewDeclarationLanguage and a call), because the VDL class
> LU> does not provide a method from the base class. I think it does not
> LU> suppose a problem, just something to take note. Maybe provide a
> LU> default method in ViewDeclarationLanguage can be useful, so if you
> LU> create a VDL wrapper, you can override that one too.
>
> I've removed the method from Application and placed it on VDL,
> specifying that only for facelets must it be implemented, but leaving
> the door open to other VDLs doing this.
>

Ok. I agree it has more sense to move the method to the VDL.

> SECTION: Loading views via ResourceHandler
>
> The discussion on this one was hard to follow.
>
> Leonardo, I've had some good luck in looking to Frank as an expert on
> this, would you mind if I had a hangout with him after you and he had a
> chance to hash it out some more? As I tried to follow the discussion it
> wandered a bit and I wasn't sure where it ended up.
>

Ok, no problem, I'll write another mail to continue the discussion.

> LU> I think the only pending point in this part is make some
> LU> clarifications about how and when
> LU> ResourceHandler.createViewResource() can be called.
>
> Yes, and how much to do here for 2.2 is the real question. At this
> point, I'd really like to get away with doing as little as possible on
> this feature, but I understand the importance of getting it right.
>

Ok.
In few words the problem is with the introduction of faces flow and
resource library contracts, it becomes necessary to define the precedence
order and the rules about how facelets load views or templates from the
specified places.

> LU> it should be 3 different methods to deal with this (better names for
> LU> the methods are welcome):
>
> Let's see if we really need that.
>

Ok.

> [...]
>
>>>>>> On Mon, 14 Jan 2013 18:32:55 -0500, Leonardo Uribe <lu4242_at_gmail.com> said:
>
> LU> Hi
> LU> Thinking about the problem in ResourceHandler.createViewResource, I have
> LU> some additional comments:
>
> LU> Q: What's the difference between createViewTemplateResource and
> LU> createResource ?
>
> I'd like to know that myself.
>

Ok.

>>>>>> On Tue, 15 Jan 2013 14:37:14 -0500, Leonardo Uribe <lu4242_at_gmail.com> said:
>
> LU> Thinking about this, I think a good idea could be add a method to
> LU> Resource interface to get the last modified time. If you take a look
> LU> at Resource implementation, these two methods:
>
> LU> public boolean userAgentNeedsUpdate(FacesContext context)
> LU> public Map<String, String> getResponseHeaders()
>
> LU> requires to calculate the last modified time.
>
> LU> DefaultFaceletFactory requires calculate the last modified time in
> LU> order to refresh the facelet, so it has sense something like this:
>
> LU> public Long getLastModified()
>
> LU> return null if no lastModified time can be returned. In the case of
> LU> load a facelet from a database this method helps to isolate the
> LU> details involved in get the lastModified time from DefaultFaceletFactory
> LU> logic related to refresh facelet instances.
>
> I can see adding this, but I think it can wait til after 2.2. Do we
> really need it now?
>

For now it is not strictly necessary, but if we don't include it,
facelets compiler
will need to calculate this value using the returned URL for
javax.faces.FACELETS_REFRESH_PERIOD. It is not nice, but it will work.

>>>>>> On Wed, 16 Jan 2013 20:23:42 +0100, Frank Caputo <frank_at_frankcaputo.de> said:
>
> FC> Hi Leonardo,
> FC> Am 15.01.2013 um 20:37 schrieb Leonardo Uribe <lu4242_at_gmail.com>:
>
> LU> Thinking about this, I think a good idea could be add a method to
> LU> Resource interface to get the last modified time.
> LU>
> LU> public Long getLastModified()
> LU>
> LU> return null if no lastModified time can be returned.
>
> FC> This method would be really helpful, but I wouldn't allow null to be returned, so I'd use the primitive long as return value.
>
> FC> There will be an issue with decorating existing resources and
> FC> overwriting only getLastModified. userAgentNeedsUpdate and
> FC> getResponseHeaders won't call the decorated version of
> FC> getLastModified. So on decorated resources all 3 methods must be
> FC> implemented, which is not very comfortable.
>
> Yes, this is a big problem one encounters when maintaining a framework
> over time that allows decoratability.
>

Ok.

>>>>>> On Wed, 16 Jan 2013 22:02:24 -0500, Leonardo Uribe <lu4242_at_gmail.com> said:
>
> LU> After thinking about the problem of ResourceHandler.createViewResource(),
> LU> I have an idea about how we can fix this part. Let's start for the
> LU> beginning (I'll do a small summary, fixing some parts to align the
> LU> concepts with the proposal, sorry if it becomes too repetitive):
>
>
>>>>>> On Wed, 16 Jan 2013 15:47:52 -0500, Leonardo Uribe <lu4242_at_gmail.com> said:
>
> LU> In conclusion, in my opinion we shouldn't worry about that detail,
> LU> because once these methods are defined, by the "nature" of the
> LU> underlying Resource those implementations does not change.
>
>>>>>> On Sun, 20 Jan 2013 16:37:41 +0100, Frank Caputo <frank_at_frankcaputo.de> said:
>
> FC> +1
>
>
>>>>>> On Wed, 23 Jan 2013 21:12:00 +0100, Frank Caputo <frank_at_frankcaputo.de> said:
>
> [...]
>
> FC> I would be happy with a method String getContract() in Resource. I
> FC> think this would help solving this problem.
>
> These kinds of comments indicate the need for a more self-contained and
> complete proposal. Leo and Frank, can you get together and I'll have a
> hangout with Frank afterward? It needs to be as soon as possible.
>

I'm on it.

regards,

Leonardo Uribe