IDEMPIERE-5014 Add stream interface to Query (#1298)

org.adempiere.base/src/org/compiere/model/Query.java

@@ -28,7 +28,12 @@ import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Properties;
+import java.util.Spliterator;
+import java.util.Spliterators;
+import java.util.function.Consumer;
 import java.util.logging.Level;
+import java.util.stream.Stream;
+import java.util.stream.StreamSupport;
 
 import org.adempiere.exceptions.DBException;
 import org.compiere.util.CLogger;
@@ -645,51 +650,83 @@ public class Query
 	}
 	
 	/**
-	 * Return an Iterator implementation to fetch one PO at a time. The implementation first retrieve
+	 * Return an Iterable implementation that can be used in a <tt>for</tt> expression. Example:
-	 * all IDS that match the query criteria and issue sql query to fetch the PO when caller want to
+	 * <pre>{@code
-	 * fetch the next PO. This minimize memory usage but it is slower than the list method.
+	 * Iterable<MTable> query = new Query(...).iterable();
-	 * @return Iterator
+	 *
+	 * for (MTable table : query) {
+	 *   // Do stuff with the element
+	 * }
+	 * </pre>
+	 *
+	 * @return Iterable
 	 * @throws DBException 
 	 */
-	public <T extends PO> Iterator<T> iterate() throws DBException
+	public <T extends PO> Iterable<T> iterable() throws DBException
 	{
-		String[] keys = table.getKeyColumns();
+		return () -> iterate();
-		StringBuilder sqlBuffer = new StringBuilder(" SELECT ");
+	}
-		for (int i = 0; i < keys.length; i++) {
+	
-			if (i > 0)
+	/**
-				sqlBuffer.append(", ");
+	 * Return an Stream implementation to fetch one PO at a time. This method will only create POs on-demand and
-			if (!joinClauseList.isEmpty())
+	 * they will become eligible for garbage collection once they have been consumed by the stream, so unlike
-				sqlBuffer.append(table.getTableName()).append(".");
+	 * {@link #list()} it doesn't have to hold a copy of all the POs in the result set in memory at one time.
-			sqlBuffer.append(keys[i]);
+	 * For situations where you need to iterate over a result set and operate on the results one-at-a-time rather
-		}
+	 * than operate on the group as a whole, this method is likely to give better performance than <code>list()</code>.
-		sqlBuffer.append(" FROM ").append(table.getTableName());
+	 * @return Stream
-		String sql = buildSQL(sqlBuffer, true);
+	 * @throws DBException 
-		
+	 */
+	public <T extends PO> Stream<T> stream() throws DBException
+	{
+		String sql = buildSQL(null, true);
+
 		PreparedStatement pstmt = null;
 		ResultSet rs = null;
-		List<Object[]> idList = new ArrayList<Object[]>();
+		try {
-		try
-		{
 			pstmt = DB.prepareStatement (sql, trxName);
+			final PreparedStatement finalPstmt = pstmt;
 			rs = createResultSet(pstmt);
-			while (rs.next ())
+			final ResultSet finalRS = rs;
-			{
+			
-				Object[] ids = new Object[keys.length];
+			return StreamSupport.stream(new Spliterators.AbstractSpliterator<T>(
-				for (int i = 0; i < ids.length; i++) {
+						Long.MAX_VALUE,Spliterator.ORDERED) {
-					ids[i] = rs.getObject(i+1);
+					@Override
-				}
+					public boolean tryAdvance(Consumer<? super T> action) {
-				idList.add(ids);
+						try {
-			}
+							if(!finalRS.next()) return false;
+							@SuppressWarnings("unchecked")
+							final T newRec = (T)table.getPO(finalRS, trxName);
+							action.accept(newRec);
+							return true;
+						} catch(SQLException ex) {
+							log.log(Level.SEVERE, sql, ex);
+							throw new DBException(ex, sql);
+						}
+					}
+				}, false).onClose(() -> DB.close(finalRS, finalPstmt));
 		}
 		catch (SQLException e)
 		{
+			DB.close(rs, pstmt);
 			log.log(Level.SEVERE, sql, e);
 			throw new DBException(e, sql);
-		} finally {
-			DB.close(rs, pstmt);
-			rs = null; pstmt = null;
 		}
-		return new POIterator<T>(table, idList, trxName);
+	}
+
+	/**
+	 * Return an Iterator implementation to fetch one PO at a time. This implementation is equivalent to
+	 * <tt>stream().iterator()</tt> and has similar performance benefits compared with {@link #list()}.
+	 * Useful if the downstream API requires an <code>Iterator</code>; otherwise the additional
+	 * of the {@link #stream()} interface is likely to be more useful.
+	 *
+	 * @return Iterator
+	 * @throws DBException 
+	 * @see #stream()
+	 */
+	@SuppressWarnings("unchecked")
+	public <T extends PO> Iterator<T> iterate() throws DBException
+	{
+		return (Iterator<T>) stream().iterator();
 	}
 	
 	/**

org.idempiere.p2.targetplatform/maven.locations.xml

@@ -1355,4 +1355,11 @@ Export-Package:        *;version="${version}";-noimport:=true
       <version>4.8.138</version>
       <type>jar</type>
 	</location>
+	<!-- AssertJ -->
+	<location includeSource="true" missingManifest="error" type="Maven">
+	  <groupId>org.assertj</groupId>
+	  <artifactId>assertj-core</artifactId>
+	  <version>3.22.0</version>
+	  <type>jar</type>
+	</location>
 </locations>

org.idempiere.p2.targetplatform/org.idempiere.p2.targetplatform.target

@@ -1,7 +1,7 @@
 <?xml version="1.0" encoding="UTF-8" standalone="no"?>
 <?pde?>
 <!-- generated with https://github.com/eclipse-cbi/targetplatform-dsl -->
-<target name="idempiere-220124" sequenceNumber="1643026597">
+<target name="idempiere-220428" sequenceNumber="1643026598">
   <locations>
     <location includeMode="slicer" includeAllPlatforms="true" includeSource="true" includeConfigurePhase="true" type="InstallableUnit">
       <unit id="zcommon" version="9.6.1"/>
@@ -1492,5 +1492,12 @@ Export-Package:        *;version="${version}";-noimport:=true
       <version>4.8.138</version>
       <type>jar</type>
 	</location>
+	<!-- AssertJ -->
+	<location includeSource="true" missingManifest="error" type="Maven">
+	  <groupId>org.assertj</groupId>
+	  <artifactId>assertj-core</artifactId>
+	  <version>3.22.0</version>
+	  <type>jar</type>
+	</location>
   </locations>
 </target>

org.idempiere.p2.targetplatform/org.idempiere.p2.targetplatform.tpd

@@ -1,5 +1,5 @@
 
-target "idempiere-220124"
+target "idempiere-220428"
 
 with source configurePhase allEnvironments
 

org.idempiere.test/META-INF/MANIFEST.MF

@@ -5,7 +5,9 @@ Bundle-SymbolicName: org.idempiere.test
 Bundle-Version: 10.0.0.qualifier
 Bundle-Vendor: iDempiere
 Automatic-Module-Name: org.idempiere.test
-Import-Package: org.junit.jupiter.api;version="5.6.0",
+Import-Package: org.assertj.core.api;version="3.22.0",
+ org.assertj.core.api.junit.jupiter;version="3.22.0",
+ org.junit.jupiter.api;version="5.6.0",
  org.junit.jupiter.api.condition;version="5.6.0",
  org.junit.jupiter.api.extension;version="5.6.0",
  org.junit.jupiter.api.extension.support;version="5.6.0",

org.idempiere.test/idempiere.unit.test.launch

@@ -66,6 +66,7 @@
     <stringAttribute key="product" value="org.adempiere.server.server_product"/>
     <booleanAttribute key="run_in_ui_thread" value="true"/>
     <setAttribute key="selected_target_bundles">
+        <setEntry value="assertj-core@default:default"/>
         <setEntry value="bcmail@default:default"/>
         <setEntry value="bcpg*1.69.0@default:default"/>
         <setEntry value="bcpkix@default:default"/>

org.idempiere.test/src/org/idempiere/test/base/QueryTest.java

@@ -36,9 +36,13 @@ import java.sql.Timestamp;
 import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
+import java.util.stream.Stream;
 
 import org.adempiere.exceptions.DBException;
 import org.adempiere.model.POWrapper;
+import org.assertj.core.api.SoftAssertions;
+import org.assertj.core.api.junit.jupiter.InjectSoftAssertions;
+import org.assertj.core.api.junit.jupiter.SoftAssertionsExtension;
 import org.compiere.model.I_Test;
 import org.compiere.model.MPInstance;
 import org.compiere.model.MProcess;
@@ -53,14 +57,18 @@ import org.compiere.util.Env;
 import org.compiere.util.KeyNamePair;
 import org.idempiere.test.AbstractTestCase;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
 
 /**
  * @author hengsin
  *
  */
-
+@ExtendWith(SoftAssertionsExtension.class)
 public class QueryTest extends AbstractTestCase {
 
+	@InjectSoftAssertions
+	SoftAssertions softly;
+	
 	/**
 	 * 
 	 */
@@ -86,6 +94,41 @@ public class QueryTest extends AbstractTestCase {
 		assertEquals(list.get(1).getTableName(), "M_InOut", "Invalid object 2");
 	}
 	
+	@Test
+	public void testStream() throws Exception
+	{
+		Stream<MTable> stream = new Query(Env.getCtx(), "AD_Table", "TableName IN (?,?)", getTrxName())
+								.setParameters("C_Invoice", "M_InOut")
+								.setOrderBy("TableName")
+								.stream();
+		softly.assertThat(stream.map(MTable::getTableName)).containsExactly("C_Invoice", "M_InOut");
+	}
+	
+	@Test
+	public void testIterable() throws Exception {
+		Iterable<MTable> query = new Query(Env.getCtx(), "AD_Table", "TableName IN (?,?)", getTrxName())
+									.setParameters("C_Invoice", "M_InOut")
+									.setOrderBy("TableName")
+									.iterable();
+		int i = 0;
+		for (MTable t : query) {
+			if (i == 0)
+			{
+				softly.assertThat(t.getTableName()).as("element 0").isEqualTo("C_Invoice");
+			}
+			else if (i == 1)
+			{
+				softly.assertThat(t.getTableName()).as("element 1").isEqualTo("M_InOut");
+			}
+			else
+			{
+				softly.fail("More objects retrieved than expected: " + t.get_TableName());
+				break;
+			}
+			i++;
+		}
+	}
+	
 	@Test
 	public void testScroll() throws Exception
 	{
@@ -109,7 +152,7 @@ public class QueryTest extends AbstractTestCase {
 				}
 				else
 				{
-					fail("More objects retrived than expected");
+					fail("More objects retrieved than expected");
 				}
 				i++;
 			}
@@ -142,7 +185,7 @@ public class QueryTest extends AbstractTestCase {
 			}
 			else
 			{
-				fail("More objects retrived than expected");
+				fail("More objects retrieved than expected");
 			}
 			i++;
 		}