ICEfaces
  1. ICEfaces
  2. ICE-4667

UISeries.keepSaved performance

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.8.1
    • Fix Version/s: 1.8.2-RC1, 1.8.2
    • Component/s: ICE-Components
    • Labels:
      None
    • Environment:
      All

      Description

      In Mojarra2's UIData.keepSaved, they just do the following, instead of that whole FacesMessage while loop that we do in UISeries.keepSaved:

          FacesMessage.Severity sev = context.getMaximumSeverity();
          return (sev != null && (FacesMessage.SEVERITY_ERROR.compareTo(sev) >= 0));

      The difference of course being that we track whether a child of the container has failed validation, whereas stock JSF sees if any component in the view has failed validation.

      One bad scenario with our existing way, would be if you have two different dataTables in one form. Something in dataTable A fails validation, so the whole form, and thus both dataTable A and B have failed validation. But dataTable B detects no FacesMessages for its components, so it throws out its state, losing any inputted values therein. I'd rather keep state potentially too long, that apps can just clear themselves, than lose state that unrecoverable. Maybe we should adopt their optimisation.

        Issue Links

          Activity

          Hide
          Mark Collette added a comment -

          I ran in some challenges while trying to create a test case for this issue.

          1. When I made an inputText fail validation, it threw an exception about not finding some resource bundle, which I'd ran into before in ICE-3896. I dug into that, seeing if my browser locale was correct, and if I needed to include the locale configuration in my faces-config.xml, but then I found out it was my fault, I'd built my own JSF jars that somehow didn't include the required Messages.properties files.

          2. I tried using List<String> as my data models for my test case, which didn't seem to work properly. I remembered running into this in the past, and replaced them with List<ClassWithStringField>, and changed the facelets page to use a field off of the List element instead of the List element directly, and then I started getting the expected behaviour.

          3. For my test case, I made two separate forms, each with their own UISeries with required inputText child. I tried covering all of the possible input value combinations, but that wasn't useful, since the necessary test involved multiples interactions in a specific sequence, and not a single step with a given set of inputs.

          Show
          Mark Collette added a comment - I ran in some challenges while trying to create a test case for this issue. 1. When I made an inputText fail validation, it threw an exception about not finding some resource bundle, which I'd ran into before in ICE-3896 . I dug into that, seeing if my browser locale was correct, and if I needed to include the locale configuration in my faces-config.xml, but then I found out it was my fault, I'd built my own JSF jars that somehow didn't include the required Messages.properties files. 2. I tried using List<String> as my data models for my test case, which didn't seem to work properly. I remembered running into this in the past, and replaced them with List<ClassWithStringField>, and changed the facelets page to use a field off of the List element instead of the List element directly, and then I started getting the expected behaviour. 3. For my test case, I made two separate forms, each with their own UISeries with required inputText child. I tried covering all of the possible input value combinations, but that wasn't useful, since the necessary test involved multiples interactions in a specific sequence, and not a single step with a given set of inputs.
          Hide
          Mark Collette added a comment -

          I inspected the JSF RI code that I have already downloaded, and found that 1.2_04 and 1.2_05 do the slow FacesMessage approach, and 1.2_10, 1.2_11, 1.2_12 do the faster maxSeverity approach.

          Show
          Mark Collette added a comment - I inspected the JSF RI code that I have already downloaded, and found that 1.2_04 and 1.2_05 do the slow FacesMessage approach, and 1.2_10, 1.2_11, 1.2_12 do the faster maxSeverity approach.
          Hide
          Mark Collette added a comment -

          When compiling test scenarios, I found a case where ICEfaces failed to do what I consider the correct behaviour. Further investigation showed it to not be a flaw with the FacesMessage or MaximumSeverity approaches, but rather something intrinsic to how UIForm, UIData and UIInput/EditableValueHolder interact together. I made a stock JSF example app demonstrating this, and filed a bug with the Sun RI:

          Data loss with multiple forms
          https://javaserverfaces.dev.java.net/issues/show_bug.cgi?id=1236

          Show
          Mark Collette added a comment - When compiling test scenarios, I found a case where ICEfaces failed to do what I consider the correct behaviour. Further investigation showed it to not be a flaw with the FacesMessage or MaximumSeverity approaches, but rather something intrinsic to how UIForm, UIData and UIInput/EditableValueHolder interact together. I made a stock JSF example app demonstrating this, and filed a bug with the Sun RI: Data loss with multiple forms https://javaserverfaces.dev.java.net/issues/show_bug.cgi?id=1236
          Hide
          Mark Collette added a comment -

          TRUNK
          Subversion 19144
          icefaces\component\src\com\icesoft\faces\component\panelseries\UISeries.java

          GLIMMER
          Subversion 19145
          compat\components\src\main\java\com\icesoft\faces\component\panelseries\UISeries.java

          Show
          Mark Collette added a comment - TRUNK Subversion 19144 icefaces\component\src\com\icesoft\faces\component\panelseries\UISeries.java GLIMMER Subversion 19145 compat\components\src\main\java\com\icesoft\faces\component\panelseries\UISeries.java
          Hide
          Mark Collette added a comment -

          The test involves having two forms, a top form and a bottom form, each with their own panelSeries that contains an inputText with required validation. The panelSeries datamodel is just 2 strings, which means that each form will show an inputText on the left and on the right.

          1. Type "xx" into all 4 inputTexts
          2. Press both submit buttons. The order doesn't matter
          3. Clear the left inputText in each of the two forms, and type "yy" into the right inputText in each form
          4. Press the bottom submit button. As expected, what you types remains, and the bottom left inputText gets a FacesMessage for requiring input.
          5. Press the top submit button.
          OLD/WRONG: With the old code, the top inputText components retain their typed in values, but the bottom inputText components lose their values, and show the old bean values of "xx". The top left inputText gets a FacesMessage for requiring input, as one would expect
          NEW/CORRECT: With the new code, all of the inputText components retain their typed in values, and the empty inputTexts get required FacesMessages.

          <ice:form>
          <ice:panelSeries value="#

          {bean.topList}

          " var="topEntry" id="topLst">
          <ice:inputText value="#

          {topEntry.held}

          " id="topIn" required="true"/>
          </ice:panelSeries>
          <br/>
          <ice:commandButton value="Submit top form only"/>
          <br/>
          <ice:messages/>
          </ice:form>
          <ice:form>
          <ice:panelSeries value="#

          {bean.bottomList}

          " var="bottomEntry" id="bottomLst">
          <ice:inputText value="#

          {bottomEntry.held}

          " id="bottomIn" required="true"/>
          </ice:panelSeries>
          <br/>
          <ice:commandButton value="Submit bottom form only"/>
          <br/>
          <ice:messages/>
          </ice:form>

          import java.util.List;
          import java.util.ArrayList;

          public class Bean {
          private List topList;
          private List bottomList;

          public Bean ()

          { topList = new ArrayList(); bottomList = new ArrayList(); topList.add(new ItemHolder("")); topList.add(new ItemHolder("")); bottomList.add(new ItemHolder("")); bottomList.add(new ItemHolder("")); }

          public List getTopList()

          { return topList; }

          public void setTopList(List top)

          { topList = top; }

          public List getBottomList()

          { return bottomList; }

          public void setBottomList(List bottom)

          { bottomList = bottom; }

          public static class ItemHolder {
          private String held;

          public ItemHolder(String h)

          { held = h; }
          public String getHeld() { return held; }
          public void setHeld(String h) { held = h; }

          }
          }

          Show
          Mark Collette added a comment - The test involves having two forms, a top form and a bottom form, each with their own panelSeries that contains an inputText with required validation. The panelSeries datamodel is just 2 strings, which means that each form will show an inputText on the left and on the right. 1. Type "xx" into all 4 inputTexts 2. Press both submit buttons. The order doesn't matter 3. Clear the left inputText in each of the two forms, and type "yy" into the right inputText in each form 4. Press the bottom submit button. As expected, what you types remains, and the bottom left inputText gets a FacesMessage for requiring input. 5. Press the top submit button. OLD/WRONG: With the old code, the top inputText components retain their typed in values, but the bottom inputText components lose their values, and show the old bean values of "xx". The top left inputText gets a FacesMessage for requiring input, as one would expect NEW/CORRECT: With the new code, all of the inputText components retain their typed in values, and the empty inputTexts get required FacesMessages. <ice:form> <ice:panelSeries value="# {bean.topList} " var="topEntry" id="topLst"> <ice:inputText value="# {topEntry.held} " id="topIn" required="true"/> </ice:panelSeries> <br/> <ice:commandButton value="Submit top form only"/> <br/> <ice:messages/> </ice:form> <ice:form> <ice:panelSeries value="# {bean.bottomList} " var="bottomEntry" id="bottomLst"> <ice:inputText value="# {bottomEntry.held} " id="bottomIn" required="true"/> </ice:panelSeries> <br/> <ice:commandButton value="Submit bottom form only"/> <br/> <ice:messages/> </ice:form> import java.util.List; import java.util.ArrayList; public class Bean { private List topList; private List bottomList; public Bean () { topList = new ArrayList(); bottomList = new ArrayList(); topList.add(new ItemHolder("")); topList.add(new ItemHolder("")); bottomList.add(new ItemHolder("")); bottomList.add(new ItemHolder("")); } public List getTopList() { return topList; } public void setTopList(List top) { topList = top; } public List getBottomList() { return bottomList; } public void setBottomList(List bottom) { bottomList = bottom; } public static class ItemHolder { private String held; public ItemHolder(String h) { held = h; } public String getHeld() { return held; } public void setHeld(String h) { held = h; } } }
          Hide
          Isuru Perera added a comment -

          Mark Collette,

          Can you please look at my forum post about an issue with the fix applied http://www.icefaces.org/JForum/posts/list/16345.page

          Thanks.

          Show
          Isuru Perera added a comment - Mark Collette, Can you please look at my forum post about an issue with the fix applied http://www.icefaces.org/JForum/posts/list/16345.page Thanks.
          Hide
          Mark Collette added a comment -

          Thanks Isuru, I've investigated this, and I believe that the solution is to change UISeries.maximumSeverityAtLeastError from >= 0 to <= 0. Comparing with Mojarra 1.2 through 2.0, I believe they have it incorrect as well. I will make a new jira for this change, and link to it in your forum thread.

          Show
          Mark Collette added a comment - Thanks Isuru, I've investigated this, and I believe that the solution is to change UISeries.maximumSeverityAtLeastError from >= 0 to <= 0. Comparing with Mojarra 1.2 through 2.0, I believe they have it incorrect as well. I will make a new jira for this change, and link to it in your forum thread.
          Hide
          Isuru Perera added a comment -

          Hi Mark,

          Thanks for taking time to see my forum post. I will also check your solution.

          Show
          Isuru Perera added a comment - Hi Mark, Thanks for taking time to see my forum post. I will also check your solution.

            People

            • Assignee:
              Mark Collette
              Reporter:
              Mark Collette
            • Votes:
              1 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: