dev@javaserverfaces.java.net

Re: Call for code review for JAVASERVERFACES-3246

From: Edward Burns <edward.burns_at_oracle.com>
Date: Mon, 14 Jul 2014 07:36:23 -0700

>>>>> On Mon, 07 Jul 2014 09:05:28 +0800, Zhijun Ren <ren.zhijun_at_oracle.com> said:

ZR> Hi Ed and Manfred,
ZR> Can you confirm that you have reviewed the code? From the following
ZR> Wiki, i see that I need to get approval before I can submit the
ZR> code.

Hello Zhijun,

I would like you to try a different approach. Also, a change like this
needs an automated regression test to be contributed along with it as
well. We have made it a bit easier to create such a test with some
maven archetypes. You'll have to look on the Internet for general
instructions on how to use maven archetypes, but for this issue, I'll
walk you through what I did to create one.

First, let's talk about the heart of your change. Your proposal
accomodates the change on the late side, when the information is
requested. I think it's better to handle it on the parsing side. When
the app starts up, the faces-config.xml files are read and the config is
populated, the locale being one of those kinds of configs.

I want you to try it this way:

8<------
Index: jsf-ri/src/main/java/com/sun/faces/config/processor/ApplicationConfigProcessor.java
===================================================================
--- jsf-ri/src/main/java/com/sun/faces/config/processor/ApplicationConfigProcessor.java (revision 13412)
+++ jsf-ri/src/main/java/com/sun/faces/config/processor/ApplicationConfigProcessor.java (working copy)
@@ -305,6 +305,7 @@
                                 addVariableResolver(associate, n);
                             } else if (DEFAULT_LOCALE.equals(n.getLocalName())) {
                                 setDefaultLocale(app, n);
+ addSupportedLocale(app, n);
                             } else if (SUPPORTED_LOCALE.equals(n.getLocalName())) {
                                 addSupportedLocale(app, n);
                             } else if (RESOURCE_BUNDLE.equals(n.getLocalName())) {
8<----------

this basically says, "if you come across a <default-locale> element,
also add it to the set of supported locales."

That way when other code asks, "is this locale supported?" the answer
will be yes if it's either <default-locale> or <supported-locale>.

Now, on to the more laborious but equally important part, the test.

We have two archetypes to make a test, one for JSF 2.1 based code
another for JSF 2.2 based code.

Here are my UNIX aliases for them.

alias mvnajt21='mvn archetype:generate -DarchetypeGroupId=com.sun.faces -DarchetypeArtifactId=faces-2.1-test-war-archetype -DarchetypeVersion=0.1 -DarchetypeCatalog=https://maven.java.net/content/repositories/releases/archetype-catalog.xml'
alias mvnajt22='mvn archetype:generate -DarchetypeGroupId=com.sun.faces -DarchetypeArtifactId=faces-2.2-test-war-archetype -DarchetypeVersion=0.2 -DarchetypeCat

Don't ask me questions about proxies! I had to get on the external
Internet to get this to work.

That said, here are the steps I took to arrive at the revised
changebundle, which I am attaching to the issue.

1. Find a place in the existing test system that is the "right" place
for this test.

In the 2.1.x tree, test/agnostic/application is a clear winner.

2. cd to that directory and run the archetype command.

mvnajt21

3. Answer as follows for the groupId, artifactId, and version questions:

com.sun.faces.test.agnostic.application
defaultLocale
2.1.30-SNAPSHOT

answer this to the package question:

com.sun.faces.test.agnostic.application.defaultLocale

4. This creates a new defaultLocale project
as a child of the test/agnostic/application multi-module pom.

5. clean up the generate pom.xml to make it look like the other poms at
the similar level in the tree. I just basically took the
test/agnostic/application/localeConfig/pom.xml as a guide.

Because this test is in the "agnostic" area, you have to remove the
beans.xml that was generated by the archetype. Also delete any other
files not relevant to this test. For example, the genearted UserBean.

6. Apply the content from the mojarra-default-locale.war to this new
project.

I didn't do this step, but basically you need to extract stuff from the
war and make it so maven puts it back in the same place in the new test.
Also you have to take the important stuff from the faces-config.xml and
web.xml in the war and replace the corresponding stuff in the generated
ones. Of course, you have to take the content from the index.xhtml and
put it in the one in the new project.

7. Finally, you have to make the HtmlUnit test exercise the war and make
assertions about its correctness. The archetype creates an HtmlUnit
testcase that gets you started with this.

I think Liang would be able to help.

I'm going to attach a new changebundle that includes the changes I made
above which you can use as a starting point and then post another
changebundle.

Please let me know if you need more information.

Ed

-- 
| edward.burns_at_oracle.com | office: +1 407 458 0017
| 50 work days til JavaOne 2014