svn commit: r1797160 - /ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityQuery.java

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

svn commit: r1797160 - /ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityQuery.java

jleroux@apache.org
Author: jleroux
Date: Thu Jun  1 08:14:41 2017
New Revision: 1797160

URL: http://svn.apache.org/viewvc?rev=1797160&view=rev
Log:
Improved: EntityListIterator closed but not in case of exception
(OFBIZ-9385)

This is an improvement only because no cases were reported. But obviously in
case of unlucky exception after the EntityListIterator creation and before it's
closed the EntityListIterator remains in memory.
It should be closed in EntityListIterator.finalize() but the less happens there
the better.

The solution is to use try-with-ressources

Modified:
    ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityQuery.java

Modified: ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityQuery.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityQuery.java?rev=1797160&r1=1797159&r2=1797160&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityQuery.java (original)
+++ ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityQuery.java Thu Jun  1 08:14:41 2017
@@ -452,9 +452,9 @@ public class EntityQuery {
         if (dynamicViewEntity == null) {
             result = delegator.findList(entityName, makeWhereCondition(useCache), fieldsToSelect, orderBy, findOptions, useCache);
         } else {
-            EntityListIterator it = queryIterator();
-            result = it.getCompleteList();
-            it.close();
+            try (EntityListIterator it = queryIterator()) {
+                result = it.getCompleteList();
+            }
         }
         if (filterByDate && useCache) {
             return EntityUtil.filterByCondition(result, this.makeDateCondition());
@@ -510,9 +510,7 @@ public class EntityQuery {
     }
 
     public <T> List<T> getFieldList(final String fieldName) throws GenericEntityException {select(fieldName);
-        EntityListIterator genericValueEli = null;
-        try {
-            genericValueEli = queryIterator();
+        try (EntityListIterator genericValueEli = queryIterator()) {
             if (this.distinct) {
                 Set<T> distinctSet = new HashSet<T>();
                 GenericValue value = null;
@@ -536,11 +534,6 @@ public class EntityQuery {
                 return fieldList;
             }
         }
-        finally {
-            if (genericValueEli != null) {
-                genericValueEli.close();
-            }
-        }
     }
 
     /**
@@ -551,16 +544,9 @@ public class EntityQuery {
      * @see EntityUtil#getPagedList
      */
     public PagedList<GenericValue> queryPagedList(final int viewIndex, final int viewSize) throws GenericEntityException {
-        EntityListIterator genericValueEli = null;
-        try {
-            genericValueEli = queryIterator();
+        try (EntityListIterator genericValueEli = queryIterator()) {
             return EntityUtil.getPagedList(genericValueEli, viewIndex, viewSize);
         }
-        finally {
-            if (genericValueEli != null) {
-                genericValueEli.close();
-            }
-        }
     }
 
 }


Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r1797160 - /ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbi z/entity/util/EntityQuery.java

Jacques Le Roux
Administrator
In this commit the bottom change slipped in and should have been part of r1797161 but anyway it's quite simple and clear

Jacques


Le 01/06/2017 à 10:14, [hidden email] a écrit :

> Author: jleroux
> Date: Thu Jun  1 08:14:41 2017
> New Revision: 1797160
>
> URL: http://svn.apache.org/viewvc?rev=1797160&view=rev
> Log:
> Improved: EntityListIterator closed but not in case of exception
> (OFBIZ-9385)
>
> This is an improvement only because no cases were reported. But obviously in
> case of unlucky exception after the EntityListIterator creation and before it's
> closed the EntityListIterator remains in memory.
> It should be closed in EntityListIterator.finalize() but the less happens there
> the better.
>
> The solution is to use try-with-ressources
>
> Modified:
>      ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityQuery.java
>
> Modified: ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityQuery.java
> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityQuery.java?rev=1797160&r1=1797159&r2=1797160&view=diff
> ==============================================================================
> --- ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityQuery.java (original)
> +++ ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityQuery.java Thu Jun  1 08:14:41 2017
> @@ -452,9 +452,9 @@ public class EntityQuery {
>           if (dynamicViewEntity == null) {
>               result = delegator.findList(entityName, makeWhereCondition(useCache), fieldsToSelect, orderBy, findOptions, useCache);
>           } else {
> -            EntityListIterator it = queryIterator();
> -            result = it.getCompleteList();
> -            it.close();
> +            try (EntityListIterator it = queryIterator()) {
> +                result = it.getCompleteList();
> +            }
>           }
>           if (filterByDate && useCache) {
>               return EntityUtil.filterByCondition(result, this.makeDateCondition());
> @@ -510,9 +510,7 @@ public class EntityQuery {
>       }
>  
>       public <T> List<T> getFieldList(final String fieldName) throws GenericEntityException {select(fieldName);
> -        EntityListIterator genericValueEli = null;
> -        try {
> -            genericValueEli = queryIterator();
> +        try (EntityListIterator genericValueEli = queryIterator()) {
>               if (this.distinct) {
>                   Set<T> distinctSet = new HashSet<T>();
>                   GenericValue value = null;
> @@ -536,11 +534,6 @@ public class EntityQuery {
>                   return fieldList;
>               }
>           }
> -        finally {
> -            if (genericValueEli != null) {
> -                genericValueEli.close();
> -            }
> -        }
>       }
>  
>       /**
> @@ -551,16 +544,9 @@ public class EntityQuery {
>        * @see EntityUtil#getPagedList
>        */
>       public PagedList<GenericValue> queryPagedList(final int viewIndex, final int viewSize) throws GenericEntityException {
> -        EntityListIterator genericValueEli = null;
> -        try {
> -            genericValueEli = queryIterator();
> +        try (EntityListIterator genericValueEli = queryIterator()) {
>               return EntityUtil.getPagedList(genericValueEli, viewIndex, viewSize);
>           }
> -        finally {
> -            if (genericValueEli != null) {
> -                genericValueEli.close();
> -            }
> -        }
>       }
>  
>   }
>
>
>