svn commit: r1797159 - in /ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbiz/common: FindServices.java login/LoginServices.java

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

svn commit: r1797159 - in /ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbiz/common: FindServices.java login/LoginServices.java

jleroux@apache.org
Author: jleroux
Date: Thu Jun  1 08:12:12 2017
New Revision: 1797159

URL: http://svn.apache.org/viewvc?rev=1797159&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/common/src/main/java/org/apache/ofbiz/common/FindServices.java
    ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbiz/common/login/LoginServices.java

Modified: ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbiz/common/FindServices.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbiz/common/FindServices.java?rev=1797159&r1=1797158&r2=1797159&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbiz/common/FindServices.java (original)
+++ ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbiz/common/FindServices.java Thu Jun  1 08:12:12 2017
@@ -438,11 +438,9 @@ public class FindServices {
         int start = viewIndex.intValue() * viewSize.intValue();
         List<GenericValue> list = null;
         Integer listSize = 0;
-        try {
-            EntityListIterator it = (EntityListIterator) result.get("listIt");
+        try (EntityListIterator it = (EntityListIterator) result.get("listIt")) {
             list = it.getPartialList(start+1, viewSize); // list starts at '1'
             listSize = it.getResultsSizeAfterPartialList();
-            it.close();
         } catch (Exception e) {
             Debug.logInfo("Problem getting partial list" + e,module);
         }
@@ -752,13 +750,11 @@ public class FindServices {
 
         List<GenericValue> list = null;
         GenericValue item= null;
-        try {
-            EntityListIterator it = (EntityListIterator) result.get("listIt");
+        try (EntityListIterator it = (EntityListIterator) result.get("listIt")) {
             list = it.getPartialList(1, 1); // list starts at '1'
             if (UtilValidate.isNotEmpty(list)) {
                 item = list.get(0);
             }
-            it.close();
         } catch (Exception e) {
             Debug.logInfo("Problem getting list Item" + e,module);
         }

Modified: ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbiz/common/login/LoginServices.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbiz/common/login/LoginServices.java?rev=1797159&r1=1797158&r2=1797159&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbiz/common/login/LoginServices.java (original)
+++ ofbiz/ofbiz-framework/trunk/framework/common/src/main/java/org/apache/ofbiz/common/login/LoginServices.java Thu Jun  1 08:12:12 2017
@@ -150,7 +150,7 @@ public class LoginServices {
 
                     // check the user login object again
                     try {
-                     userLogin = EntityQuery.use(delegator).from("UserLogin").where("userLoginId", username).cache(isServiceAuth).queryOne();
+                        userLogin = EntityQuery.use(delegator).from("UserLogin").where("userLoginId", username).cache(isServiceAuth).queryOne();
                     } catch (GenericEntityException e) {
                         Debug.logWarning(e, "", module);
                     }
@@ -434,29 +434,29 @@ public class LoginServices {
             // Not saving password history, so return from here.
             return;
         }
-
-        EntityListIterator eli = EntityQuery.use(delegator)
-                                            .from("UserLoginPasswordHistory")
-                                            .where("userLoginId", userLoginId)
-                                            .orderBy("-fromDate")
-                                            .cursorScrollInsensitive()
-                                            .queryIterator();
+        EntityQuery eq = EntityQuery.use(delegator)
+                .from("UserLoginPasswordHistory")
+                .where("userLoginId", userLoginId)
+                .orderBy("-fromDate")
+                .cursorScrollInsensitive();
         Timestamp nowTimestamp = UtilDateTime.nowTimestamp();
-        GenericValue pwdHist;
-        if ((pwdHist = eli.next()) != null) {
-            // updating password so set end date on previous password in history
-            pwdHist.set("thruDate", nowTimestamp);
-            pwdHist.store();
-            // check if we have hit the limit on number of password changes to be saved. If we did then delete the oldest password from history.
-            eli.last();
-            int rowIndex = eli.currentIndex();
-            if (rowIndex==passwordChangeHistoryLimit) {
-                eli.afterLast();
-                pwdHist = eli.previous();
-                pwdHist.remove();
+        
+        try (EntityListIterator eli = eq.queryIterator()) {
+            GenericValue pwdHist;
+            if ((pwdHist = eli.next()) != null) {
+                // updating password so set end date on previous password in history
+                pwdHist.set("thruDate", nowTimestamp);
+                pwdHist.store();
+                // check if we have hit the limit on number of password changes to be saved. If we did then delete the oldest password from history.
+                eli.last();
+                int rowIndex = eli.currentIndex();
+                if (rowIndex==passwordChangeHistoryLimit) {
+                    eli.afterLast();
+                    pwdHist = eli.previous();
+                    pwdHist.remove();
+                }
             }
         }
-        eli.close();
 
         // save this password in history
         GenericValue userLoginPwdHistToCreate = delegator.makeValue("UserLoginPasswordHistory", UtilMisc.toMap("userLoginId", userLoginId,"fromDate", nowTimestamp));
@@ -904,7 +904,7 @@ public class LoginServices {
 
     public static void checkNewPassword(GenericValue userLogin, String currentPassword, String newPassword, String newPasswordVerify, String passwordHint, List<String> errorMessageList, boolean ignoreCurrentPassword, Locale locale) {
         Delegator delegator = userLogin.getDelegator();
-     boolean useEncryption = "true".equals(EntityUtilProperties.getPropertyValue("security", "password.encrypt", delegator));
+        boolean useEncryption = "true".equals(EntityUtilProperties.getPropertyValue("security", "password.encrypt", delegator));
 
         String errMsg = null;