5 Replies Latest reply on Mar 17, 2008 11:07 PM by wiggy

    Seam framework EntityQuery Bug ?

    wiggy

      think I have a bug the seam frameworks when using the EntityQuery seam pojo.


      I tried to build a page that did next/previous/last page etc when running a listing and kept getting a null pointer error - when falls over in the the isNextExists() test.


      I had two problems (first one took me a while to twig - cos i'm new at the game) 


      essentially if you dont make your class that extends EntityQuery conversational or longer scope, what the framework does is inject a new EntityQuery subclass each time you hit a button on the browser.  This means that any context was lost across button presses. 


      I guess this should have been obvious - but foxed me for ages.  By default the jboss tools IDE doesnt set your query class as conversation scope.


      Next problem is the null pointer error.  this is i think a genuine bug.  I have fixed it locally by cloning EntityQuery as WillsEntityQuery and inheriting from the modified version till i got it to work.


      I think the main problem is that


      1. firstResult and maxResults are Integer in the base Query class.  And hence can be null.


      2.  next the isNextExists test.  in the original code this is


      
         @Override
      
         @Transactional
      
         public boolean isNextExists()
      
         {
      
            return resultList!=null && 
      
                  resultList.size() > getMaxResults();
      
         }
      
      



      however getMaxResults by default is null so fails on null pointer.  In addition this code doesnt work through what it means if firstResult is null.


      heres my not so beautiful fix


      
         public boolean isNextExists()
      
         {
      
              List<E> rl = getResultList(); //sets # records got
      
              String rls = (rl == null) ? "empty RL" : rl.toString();
      
              log.info("next exists:  resultList: #0, size #1, and max results #2 and first result #3", rls, rl.size(), getMaxResults(), getFirstResult());
      
              
      
                
      
                 //fixes a defect in framework code - can be null when created
      
                 //so set the maxResults with a default
      
                  boolean rlsizeGTmaxresults = false;  
      
                 if (getMaxResults() == null)  
      
                      rlsizeGTmaxresults = false; //no max results defined, say no
      
                 else
      
                      rlsizeGTmaxresults = (resultCount != null) ? resultCount > getMaxResults () : false;
      
             log.info ("next exists : returning  " + rlsizeGTmaxresults);
      
              return rl != null && rlsizeGTmaxresults ;
      
         }
      
      



      first declare a boolean rlsizeGTMaxresults at set false - default result.


      thus if maxResults not defined return this (presumes page windows are not in use - just full listing.)


      next problem is that resultCount (in Entity ) can be zero so check before testing resultCount > getMaxResults()


      I had to pull another trick here.  resultCount can be zero so in the


      public List<E> getResultList()



      class I have to set this with the size of the resultList attribute thats set by initResultList as follows



      
         public List<E> getResultList()
      
         {
      
            if ( isAnyParameterDirty() )
      
            {
      
               refresh();
      
            }
      
            initResultList(); //get resultList from DB
      
            //set number of records retrieved
      
            //added to make the forward backward work
      
            if (resultList != null)
      
            {
      
                 resultCount = Long.valueOf(resultList.size());
      
            }
      
            else
      
                 resultCount = null;
      
            //truncate the list to max record size if defined 
      
            return truncResultList(resultList);
      
         }
      
      
      



      lastly the getPreviousFirstResult() logic looks dubious to me - i've recoded it and in expanded form seems to work as follows


      
          public int getPreviousFirstResult()
      
          {
      
             Integer fr = getFirstResult();
      
             Integer mr = getMaxResults();
      
             
      
             if (fr == null)
      
             {
      
                  return 0;
      
             }
      
             else if (fr > mr)
      
             {
      
                  return fr - mr;
      
             }
      
             else if (fr <= mr)
      
             {
      
                  return 0;
      
             }
      
             else
      
                  return 0;
      
             
      
             /*orig code : return (mr >= ( fr==null ? 0 : fr )) ? 
      
                      0 : fr - mr;*/
      
          }
      
      
      


      remember maxResults or firstResult can return null - so test for that first.
      if firstResult is not set - make it zero.
      if fr > max results push the view results back up. 
      if fr <= mr query from zero as first result.


      With these fixes in place - the code works as expected here is my form


      
      <ui:define name="body">
      
          
      
          <h:messages globalOnly="true" styleClass="message"/>
      
          
      
          <h1>Enhanced Node Manager</h1>
      
          
      
          <h:form id="searchCriteria">
      
          <fieldset>
      
               <h:inputText id="searchString" 
      
                               value="#{nodeList.searchString}" >
      
               <!-- comment out <a4j:support event="onkeyup" actionListener="#{nodeList.find()}"
      
                               reRender="searchResults" /> -->
      
               </h:inputText>
      
               &#160;
      
               <a4j:commandButton id="findNodes" value="Find nodes" 
      
                                        action="#{nodeList.find}"
      
                                        reRender="searchResults" />
      
               &#160;
      
               <a4j:status id="searchStringStatus">
      
                    <f:facet name="start">
      
                         <h:graphicImage value="/img/spinner.gif" />
      
                    </f:facet>
      
               </a4j:status>
      
               <br/>
      
               <h:outputLabel for="pageSize" >Maximium Results</h:outputLabel>
      
               &#160;
      
               <!--  set the maxResults attribute which determines the page size to display -->
      
               <h:selectOneMenu value="#{nodeList.maxResults}" id="pageSize" >
      
                    <f:selectItem itemLabel="2" itemValue="2"/>
      
                    <f:selectItem itemLabel="4" itemValue="4"/>
      
                      <f:selectItem itemLabel="5" itemValue="5"/>
      
                    <f:selectItem itemLabel="10" itemValue="10"/>
      
                    <f:selectItem itemLabel="20" itemValue="20"/>
      
               </h:selectOneMenu>     
      
               &#160;<br />
      
               <!--  start a new node -->
      
               <s:button propagation="false" view="/node.xhtml" value="New Node"/>  
      
               
      
          </fieldset>
      
          </h:form>
      
          
      
          <a4j:outputPanel id="searchResults">
      
          <div class="section">
      
               <h:outputText value="no nodes found"
      
                                rendered="#{nodeList.resultList != null and nodeList.resultList.size==0}" />
      
              <rich:dataTable id="nodeList" var="node"
      
                         value="#{nodeList.resultList}" 
      
                         rendered="#{not empty nodeList.resultList}">
      
                  <rich:column width="10" >
      
                      <f:facet name="header">Id</f:facet>
      
                      #{node.id}
      
                  </rich:column>
      
                  <rich:column>
      
                  <f:facet name="header">Name</f:facet>
      
                      <s:link  id="editNodeLink"
      
                               value="#{node.name}" 
      
                               view="/node.xhtml" />
      
                  </rich:column>
      
                  <rich:column>
      
                  <f:facet name="header">Date</f:facet>
      
                      #{node.lastUpdated}
      
                  </rich:column>
      
                  <rich:column width="3">
      
                      <f:facet name="header">Action</f:facet>
      
                      <s:button id="editNode" 
      
                               value="edit" 
      
                               view="/node.xhtml">
      
                      </s:button>
      
                  </rich:column>
      
              </rich:dataTable>
      
          </div>
      
          <div>
      
            <fieldset>
      
               <s:button view="/enhanced node.xhtml"
      
                         action="#{nodeList.myFirst}"
      
                         rendered="#{nodeList.previousExists}"
      
                         value="First Page">
      
               </s:button>
      
               <s:button view="/enhanced node.xhtml"
      
                         action="#{nodeList.myPrevious}"
      
                         rendered="#{nodeList.previousExists}"
      
                         value="Previous Page">
      
               </s:button>
      
               <s:button view="/enhanced node.xhtml"
      
                         action="#{nodeList.myNext}"
      
                         rendered="#{nodeList.nextExists}"
      
                         value="Next Page">
      
               </s:button>
      
               <s:button view="/enhanced node.xhtml"
      
                         action="#{nodeList.myLast}"
      
                         rendered="#{nodeList.nextExists}"
      
                         value="Last Page">
      
               </s:button>
      
            </fieldset>
      
          </div>
      
          <div>
      
               <!-- end conversation -->
      
               <s:button view="/home.xhtml"
      
                         action="#{nodeList.clear}"
      
                             value="Finish">
      
               </s:button>
      
          
      
          </div>
      
          </a4j:outputPanel>
      
      
       
      
      
      </ui:define>
      
      
      </ui:composition>
      
      
      




      finally heres my nodeList code that inherits from the fixed WillsEntityQuery



      
      
      @Name("nodeList")
      
      @Scope (ScopeType.CONVERSATION)
      
      //@Conversational()
      
      public class NodeList extends WillsEntityQuery
      
      {
      
           private static final long serialVersionUID = 1;
      
           
      
           private String searchString;
      
           
      
           @Logger
      
           private Log log;
      
           
      
          @In FacesMessages facesMessages;
      
           
      
           //set default query for this entity query subclass
      
          @Override
      
          public String getEjbql() 
      
          { 
      
              return "select node from Node node";
      
          }
      
      
          @Factory(value="pattern", scope=ScopeType.EVENT)
      
          public String getSearchPattern()
      
          {
      
               return searchString == null ? "%" : 
      
                    '%' + searchString.toLowerCase().replace('*', '%') + '%';
      
          }
      
          
      
          @Override //overides @create in base class 
      
          @Begin (join=true) //join or begin conversation
      
          public void validate()
      
          {
      
               log.info (" new nodeList created, joined conversation");
      
             super.validate();
      
             //setMaxResults (10); //default return number
      
                 //setFirstResult (0);
      
          }
      
      
          //@End
      
          public String clear ()
      
          {
      
               searchString = null;
      
               return "main";
      
          }
      
          
      
          public void myFirst ()
      
          {
      
               setFirstResult (0);
      
               facesMessages.add("my first: setting first result #{nodeList.firstResult}");
      
               facesMessages.add("my first: max results #{nodeList.maxResults}");
      
               facesMessages.add("my first: record count #{nodeList.resultCount}");
      
          }
      
          
      
          public void myLast ()
      
          {
      
               setFirstResult (getLastFirstResult().intValue());
      
               facesMessages.add("my last: setting first result #{nodeList.firstResult}");
      
               facesMessages.add("my last: max results #{nodeList.maxResults}");
      
               facesMessages.add("my last: record count #{nodeList.resultCount}");
      
          }
      
          
      
          public void myNext ()
      
          {
      
               setFirstResult (getNextFirstResult());
      
               facesMessages.add("my next: setting next first result #0", getFirstResult());
      
               facesMessages.add("my next: max results #{nodeList.maxResults}");
      
               facesMessages.add("my next: record count #{nodeList.resultCount}");
      
          }
      
       
      
          public void myPrevious ()
      
          {
      
               setFirstResult (getPreviousFirstResult());
      
               facesMessages.add("my previous: setting previous first result #{nodeList.firstResult}");
      
               facesMessages.add("my previous: max results #{nodeList.maxResults}");
      
               facesMessages.add("my previous: record count #{nodeList.resultCount}");
      
          }
      
       
      
          @Transactional
      
          @Override
      
          public Long getLastFirstResult()
      
          {
      
             Integer pc = getPageCount();
      
             return pc==null ? 0 : ( pc.longValue()-1 ) * getMaxResults();
      
          }
      
          
      
          /**
      
           * Get the index of the first result of the next page
      
           * 
      
           */
      
          @Override
      
          public int getNextFirstResult()
      
          {
      
             Integer fr = getFirstResult();
      
             Integer mr = getMaxResults()!= null ? getMaxResults() : 0;
      
      
             facesMessages.add("nxt first result : first result #{nodeList.maxResults}");
      
             facesMessages.add("my first: max results calc #{nodeList.maxResults}");
      
      
             return ( fr==null ? 0 : fr ) + mr;
      
          }
      
      
          /**
      
           * Get the index of the first result of the previous page
      
           * 
      
           */
      
          @Override
      
          public int getPreviousFirstResult()
      
          {
      
             Integer fr = getFirstResult();
      
             Integer mr = getMaxResults();
      
             
      
             if (fr == null)
      
             {
      
                  return 0;
      
             }
      
             else if (fr > mr)
      
             {
      
                  return fr - mr;
      
             }
      
             else if (fr <= mr)
      
             {
      
                  return 0;
      
             }
      
             else
      
                  return 0;
      
             
      
             /*orig code : return (mr >= ( fr==null ? 0 : fr )) ? 
      
                      0 : fr - mr;*/
      
          }
      
          
      
        
      
          /*
      
           * set the query, run the query and set the resultList inherited attribute.
      
           */
      
          private List<Node> queryNodes ()
      
          {
      
               /*List<Node> nodeList = 
      
                    getEntityManager().createQuery("select n from Node n where lower(n.name) like #{pattern} ")
      
                    .setMaxResults(pageSize)
      
                    .setFirstResult(page * pageSize)
      
                    .getResultList(); */
      
               this.setEjbql("select n from Node n");
      
               List<String> restrictions = new ArrayList();
      
               restrictions.add ("lower(n.name) like #{pattern}");
      
               //this.setRestrictions(restrictions );
      
      
               return this.getResultList();
      
          }
      
       
      
          // call the queryNodes which will set the resultList attribute.
      
          public void find ()
      
          {
      
                queryNodes();
      
          }
      
        
      
          public String getSearchString ()
      
          {
      
               return searchString;
      
          }
      
          
      
          public void setSearchString (String search)
      
          {
      
               searchString = search != null ? search : "";
      
               log.debug("setting the query constraint #0", searchString);
      
          }
      
          
      
          @Destroy
      
          public void destroy () {}
      
      }
      
      
      



      hope that makes sense

        • 1. Re: Seam framework EntityQuery Bug ?
          wiggy

          how does one post a bug to the seam jira?

          • 2. Re: Seam framework EntityQuery Bug ?
            fernando_jmt

            I don't think it's a bug.


            If you are calling the isNextExtists() method from your template, then I assume you are trying to show the data in a paged way. So, the maxResults is mandatory in that case. So, you must tell EntityQuery how many rows you want to recover from the database (this is the essence of pagination right?).


            So, if you are getting NPE when the call to isNextExists() is done is because you are doing something wrong.



            You have two ways to define the maxResults.


            a) Declarative:



            <framework:entity-query name="languageList" ejbql="select language from Language language" auto-create="true"
                                        max-results="10"
                                        scope="EVENT">
                </framework:entity-query>
            



            b) Programmatic:



            @Name("roomList")
            public class RoomList extends EntityQuery
            {
               @Override
               public String getEjbql() 
               { 
                  return "select room from Room room";
               }
               @Override
               public Integer getMaxResults()
               {
                   return 10;
               }
            } 
            




            Are you still thinking this is bug?


            • 3. Re: Seam framework EntityQuery Bug ?
              pmuir

              Not a bug, but a crappy error message ;-)


              File an issue in JIRA so we can make it better.

              • 4. Re: Seam framework EntityQuery Bug ?
                fernando_jmt

                Pete Muir wrote on Mar 15, 2008 05:21 PM:


                Not a bug, but a crappy error message ;-)

                File an issue in JIRA so we can make it better.


                Done.
                http://jira.jboss.com/jira/browse/JBSEAM-2747

                • 5. Re: Seam framework EntityQuery Bug ?
                  wiggy

                  Okay -


                  thanks for that - I followed your jira link.


                  HOwever I added a comment to the effect that I think making the code robustly work in any scenario might be a better option than just reporting an error to the user when you consider the usage not to be as expected.


                  Thanks for helping post that across.