[jbossseam-issues] [JBoss JIRA] Commented: (JBSEAM-2381) EntityQuery to stop fetching maxResults + 1 rows

[Brian Kalbfus (JIRA)]

    [ http://jira.jboss.com/jira/browse/JBSEAM-2381?page=comments#action_12400389 ] 
            
Brian Kalbfus commented on JBSEAM-2381:
---------------------------------------

I was looking for a bug that fit my latest issue and found this one.  It fits an interesting side effect of the current isNextExists() method:  moving the seam-gen generated "Next Page" link to the top of the page causes it to not be rendered! 

I traced this to the isNextExists() code in that it is accessing resultList before it is populated.  
The "Next Page" link is thus:
        <s:link view="/RecipientList.xhtml" 
            rendered="#{recipientList.nextExists}" 
               value="Next Page #{messages.right}"
                  id="nextPage">
            <f:param name="firstResult" 
                    value="#{recipientList.nextFirstResult}"/>
        </s:link>

The dataTable generated starts like this:
    <rich:dataTable id="recipientList" 
                var="recipient"
              value="#{recipientList.resultList}" 
           rendered="#{not empty recipientList.resultList}">

The getResultList() method:

/**
    * Get the list of results this query returns
    * 
    * Any changed restriction values will be applied
    */
   @Transactional
   @Override
   public List<E> getResultList()
   {
      if ( isAnyParameterDirty() )
      {
         refresh();
      }
      initResultList();
      return truncResultList(resultList);
   }

Having the "Next Page" link below the dataTable is rendering its "rendered" attribute after the getResultList() method populates resultList for the dataTable.  Having it above is accessing the unpopulated resultList member.  I worked around this by putting a comment before the "Next Page" link with the EL expression #{resultList.size} in it.  

Changing isNextExists() to use getResultList().size() would not do because that would get a maximum of getMaxRows() rows and you wouldn't know if there was more.  It seems to me that getRowCount() is a contract to get the row count and therefore the best place to look at for a isNextExists() query.

Thanks,
-Brian

> EntityQuery to stop fetching maxResults + 1 rows
> ------------------------------------------------
>
>                 Key: JBSEAM-2381
>                 URL: http://jira.jboss.com/jira/browse/JBSEAM-2381
>             Project: JBoss Seam
>          Issue Type: Feature Request
>          Components: Framework
>    Affects Versions: 2.0.1.CR1, 2.0.0.GA
>            Reporter: Brian Kalbfus
>             Fix For: 2.1.0.GA
>
>
> In using a hibernate interceptor to audit instantiating entities, I noticed one extra record in excess of getMaxResults() is instantiated that isn't used - thus rendering a false audit.  This was because of the following code in EntityQuery (copied from CVS head):
> <code>
> protected javax.persistence.Query createQuery()
>    {
>       parseEjbql();
>       
>       evaluateAllParameters();
>       
>       joinTransaction();
>       
>       javax.persistence.Query query = getEntityManager().createQuery( getRenderedEjbql() );
>       setParameters( query, getQueryParameterValues(), 0 );
>       setParameters( query, getRestrictionParameterValues(), getQueryParameterValues().size() );
>       if ( getFirstResult()!=null) query.setFirstResult( getFirstResult() );
>       if ( getMaxResults()!=null) query.setMaxResults( getMaxResults()+1 ); //add one, so we can tell if there is another page
>       if ( getHints()!=null )
>       {
>          for ( Map.Entry<String, String> me: getHints().entrySet() )
>          {
>             query.setHint(me.getKey(), me.getValue());
>          }
>       }
>       return query;
>    }
> </code>
> Specifically:
>       if ( getMaxResults()!=null) query.setMaxResults( getMaxResults()+1 ); //add one, so we can tell if there is another page
> The code says it is adding one so that it knows if there is another page.  I see its use here:
> <code>
> public boolean isNextExists()
>    {
>       return resultList!=null && 
>             resultList.size() > getMaxResults();
>    }
> </code>
> I propose this for isNextExists():
> <code>
> public boolean isNextExists()
>    {
>       return getResultCount() > (getFirstResult()!=null?getFirstResult():0) + getMaxResults();
>    }
> </code>
> Then we can change that line in createQuery() to:
>       if ( getMaxResults()!=null) query.setMaxResults( getMaxResults() );
> I did this in my subclass by overriding these two methods for my project; I had to copy in some of the private members of EntityQuery to do this.
> Thanks,
> Brian Kalbfus

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://jira.jboss.com/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira