You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cayenne.apache.org by nt...@apache.org on 2023/11/09 14:08:02 UTC

(cayenne) branch master updated (f6b08c747 -> b72b1f4f8)

This is an automated email from the ASF dual-hosted git repository.

ntimofeev pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/cayenne.git


    from f6b08c747 Add support for Java 21 to GitHub Actions
     new b48705158 CAY-2817 Pagination flow refactoring
     new b72b1f4f8 CAY-2819 `DataContext.performIteratedQuery()` method should be unified with `iterator()` method

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 RELEASE-NOTES.txt                                  |   2 +
 .../org/apache/cayenne/access/DataContext.java     |  95 +++++-----------
 .../cayenne/access/DataContextQueryAction.java     |  58 ++++++----
 .../cayenne/access/DataDomainQueryAction.java      |  13 ++-
 .../cayenne/access/IncrementalFaultList.java       | 126 +++++++++++++++------
 .../access/MixedResultIncrementalFaultList.java    |  20 +---
 .../access/SimpleIdIncrementalFaultList.java       |   5 +-
 .../cayenne/query/IteratedQueryDecorator.java      |  17 ++-
 .../cayenne/access/IncrementalFaultListIT.java     |   4 +-
 .../SimpleIdIncrementalFaultListDataRowsIT.java    |   7 +-
 .../access/SimpleIdIncrementalFaultListIT.java     |  44 ++++---
 11 files changed, 222 insertions(+), 169 deletions(-)


(cayenne) 01/02: CAY-2817 Pagination flow refactoring

Posted by nt...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

ntimofeev pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/cayenne.git

commit b487051586e5f7859a92e735c5f5b0b6d3a11f74
Author: stariy95 <st...@gmail.com>
AuthorDate: Thu Nov 9 18:03:25 2023 +0400

    CAY-2817 Pagination flow refactoring
---
 RELEASE-NOTES.txt                                  |   1 +
 .../cayenne/access/DataContextQueryAction.java     |  58 ++++++----
 .../cayenne/access/DataDomainQueryAction.java      |  12 +-
 .../cayenne/access/IncrementalFaultList.java       | 126 +++++++++++++++------
 .../access/MixedResultIncrementalFaultList.java    |  20 +---
 .../access/SimpleIdIncrementalFaultList.java       |   5 +-
 .../cayenne/access/IncrementalFaultListIT.java     |   4 +-
 .../SimpleIdIncrementalFaultListDataRowsIT.java    |   7 +-
 .../access/SimpleIdIncrementalFaultListIT.java     |  44 ++++---
 9 files changed, 178 insertions(+), 99 deletions(-)

diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt
index 23868ae11..e1ef21051 100644
--- a/RELEASE-NOTES.txt
+++ b/RELEASE-NOTES.txt
@@ -40,6 +40,7 @@ CAY-2795 Add unit tests for the Json type
 CAY-2802 Upgrade Gradle to 7.6.1
 CAY-2803 Test infrastructure: declarative custom DI modules in ServerCase
 CAY-2805 Stop calling exp parser internally
+CAY-2817 Pagination flow refactoring
 
 Bug Fixes:
 
diff --git a/cayenne-server/src/main/java/org/apache/cayenne/access/DataContextQueryAction.java b/cayenne-server/src/main/java/org/apache/cayenne/access/DataContextQueryAction.java
index a1d9263a5..bf276f22e 100644
--- a/cayenne-server/src/main/java/org/apache/cayenne/access/DataContextQueryAction.java
+++ b/cayenne-server/src/main/java/org/apache/cayenne/access/DataContextQueryAction.java
@@ -23,6 +23,7 @@ import java.util.Collection;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 
 import org.apache.cayenne.ObjectContext;
 import org.apache.cayenne.PersistenceState;
@@ -104,30 +105,14 @@ class DataContextQueryAction extends ObjectContextQueryAction {
     @Override
     protected boolean interceptPaginatedQuery() {
         if (metadata.getPageSize() > 0) {
-            Integer maxIdQualifierSize = actingDataContext.getParentDataDomain().getMaxIdQualifierSize();
-            List<?> paginatedList;
-            List<Object> rsMapping = metadata.getResultSetMapping();
-            boolean mixedResults = false;
-            if(rsMapping != null) {
-                if(rsMapping.size() > 1) {
-                    mixedResults = true;
-                } else if(rsMapping.size() == 1) {
-                    mixedResults = !(rsMapping.get(0) instanceof EntityResultSegment)
-                            || !metadata.isSingleResultSetMapping();
-                }
-            }
+            // this will select raw ids
+            runQuery();
 
-            if(mixedResults) {
-                paginatedList = new MixedResultIncrementalFaultList<>(actingDataContext, query, maxIdQualifierSize);
-            } else {
-                DbEntity dbEntity = metadata.getDbEntity();
-                if (dbEntity != null && dbEntity.getPrimaryKeys().size() == 1) {
-                    paginatedList = new SimpleIdIncrementalFaultList<>(actingDataContext, query, maxIdQualifierSize);
-                } else {
-                    paginatedList = new IncrementalFaultList<>(actingDataContext, query, maxIdQualifierSize);
-                }
-            }
+            List<?> rawIds = response.firstList();
+            int maxIdQualifierSize = actingDataContext.getParentDataDomain().getMaxIdQualifierSize();
+            IncrementalFaultList<?> paginatedList = createIncrementalFaultList(rawIds, maxIdQualifierSize);
 
+            // replace result with a paginated list that will deal with id-to-object resolution
             response = new ListResponse(paginatedList);
             return DONE;
         }
@@ -135,6 +120,35 @@ class DataContextQueryAction extends ObjectContextQueryAction {
         return !DONE;
     }
 
+    private IncrementalFaultList<?> createIncrementalFaultList(List<?> rawIds, int maxIdQualifierSize) {
+        // just a sanity check
+        Objects.requireNonNull(rawIds, "Trying to execute paginated query that is not a select query");
+        if(isMixedResultsForPaginatedQuery()) {
+            return new MixedResultIncrementalFaultList<>(actingDataContext, query, maxIdQualifierSize, rawIds);
+        } else {
+            DbEntity dbEntity = metadata.getDbEntity();
+            if (dbEntity != null && dbEntity.getPrimaryKeys().size() == 1) {
+                return new SimpleIdIncrementalFaultList<>(actingDataContext, query, maxIdQualifierSize, rawIds);
+            } else {
+                return new IncrementalFaultList<>(actingDataContext, query, maxIdQualifierSize, rawIds);
+            }
+        }
+    }
+
+    private boolean isMixedResultsForPaginatedQuery() {
+        boolean mixedResults = false;
+        List<Object> rsMapping = metadata.getResultSetMapping();
+        if(rsMapping != null) {
+            if(rsMapping.size() > 1) {
+                mixedResults = true;
+            } else if(rsMapping.size() == 1) {
+                mixedResults = !(rsMapping.get(0) instanceof EntityResultSegment)
+                        || !metadata.isSingleResultSetMapping();
+            }
+        }
+        return mixedResults;
+    }
+
     @Override
     protected boolean interceptRefreshQuery() {
         if (query instanceof RefreshQuery) {
diff --git a/cayenne-server/src/main/java/org/apache/cayenne/access/DataDomainQueryAction.java b/cayenne-server/src/main/java/org/apache/cayenne/access/DataDomainQueryAction.java
index 0b01cb620..e6778c442 100644
--- a/cayenne-server/src/main/java/org/apache/cayenne/access/DataDomainQueryAction.java
+++ b/cayenne-server/src/main/java/org/apache/cayenne/access/DataDomainQueryAction.java
@@ -139,9 +139,7 @@ class DataDomainQueryAction implements QueryRouter, OperationObserver {
             }
         }
 
-        if (!noObjectConversion) {
-            interceptObjectConversion();
-        }
+        interceptObjectConversion();
 
         return response;
     }
@@ -528,7 +526,7 @@ class DataDomainQueryAction implements QueryRouter, OperationObserver {
 
     @SuppressWarnings({"unchecked", "rawtypes"})
     private void interceptObjectConversion() {
-        if (context == null) {
+        if (noObjectConversion()) {
             return;
         }
 
@@ -550,6 +548,12 @@ class DataDomainQueryAction implements QueryRouter, OperationObserver {
         response.reset();
     }
 
+    private boolean noObjectConversion() {
+        return context == null
+                || noObjectConversion
+                || metadata.getPageSize() > 0;
+    }
+
     private ObjectConversionStrategy<?, ?> getConverter() {
         ObjectConversionStrategy<?, ?> converter;
 
diff --git a/cayenne-server/src/main/java/org/apache/cayenne/access/IncrementalFaultList.java b/cayenne-server/src/main/java/org/apache/cayenne/access/IncrementalFaultList.java
index c4648d3ea..e07dd4372 100644
--- a/cayenne-server/src/main/java/org/apache/cayenne/access/IncrementalFaultList.java
+++ b/cayenne-server/src/main/java/org/apache/cayenne/access/IncrementalFaultList.java
@@ -21,7 +21,6 @@ package org.apache.cayenne.access;
 
 import org.apache.cayenne.CayenneRuntimeException;
 import org.apache.cayenne.Persistent;
-import org.apache.cayenne.ResultIterator;
 import org.apache.cayenne.exp.Expression;
 import org.apache.cayenne.exp.ExpressionFactory;
 import org.apache.cayenne.map.ObjEntity;
@@ -58,20 +57,18 @@ import java.util.NoSuchElementException;
  */
 public class IncrementalFaultList<E> implements List<E>, Serializable {
 
-	protected int pageSize;
+	protected final int pageSize;
 	protected final List elements;
-	protected DataContext dataContext;
-	protected ObjEntity rootEntity;
-	protected int unfetchedObjects;
+	protected final DataContext dataContext;
+	protected final ObjEntity rootEntity;
+	protected volatile int unfetchedObjects;
 
 	/**
 	 * Stores a hint allowing to distinguish data rows from unfetched ids when
 	 * the query fetches data rows.
 	 */
-	protected int idWidth;
-
-	IncrementalListHelper helper;
-	protected QueryMetadata metadata;
+	protected final int idWidth;
+	protected final QueryMetadata metadata;
 
 	/**
 	 * Defines the upper limit on the size of fetches. This is needed to avoid
@@ -79,15 +76,13 @@ public class IncrementalFaultList<E> implements List<E>, Serializable {
 	 */
 	protected int maxFetchSize;
 
-	// Don't confuse this with the JDBC ResultSet fetch size setting - this
-	// controls
-	// the where clause generation that is necessary to fetch specific records a
-	// page
-	// at a time. Some JDBC Drivers/Databases may have limits on statement
-	// length
-	// or complexity of the where clause - e.g., PostgreSQL having a default
-	// limit of
-	// 10,000 nested expressions.
+	IncrementalListHelper helper;
+
+	// Don't confuse this with the JDBC ResultSet fetch size setting -
+	// this controls the where clause generation that is necessary to fetch specific records a
+	// page at a time. Some JDBC Drivers/Databases may have limits on statement
+	// length or complexity of the where clause - e.g., PostgreSQL having a default
+	// limit of 10,000 nested expressions.
 
 	/**
 	 * Creates a new IncrementalFaultList using a given DataContext and query.
@@ -101,7 +96,7 @@ public class IncrementalFaultList<E> implements List<E>, Serializable {
 	 * @param maxFetchSize
 	 *            maximum number of fetches in one query
 	 */
-	public IncrementalFaultList(DataContext dataContext, Query query, int maxFetchSize) {
+	IncrementalFaultList(DataContext dataContext, Query query, int maxFetchSize, List<?> data) {
 		this.metadata = query.getMetaData(dataContext.getEntityResolver());
 		if (metadata.getPageSize() <= 0) {
 			throw new CayenneRuntimeException("Not a paginated query; page size: " + metadata.getPageSize());
@@ -116,12 +111,10 @@ public class IncrementalFaultList<E> implements List<E>, Serializable {
 		}
 
 		this.idWidth = metadata.getDbEntity().getPrimaryKeys().size();
-
-		List<Object> elementsUnsynced = new ArrayList<>();
-		fillIn(query, elementsUnsynced);
-		this.elements = Collections.synchronizedList(elementsUnsynced);
-
 		this.maxFetchSize = maxFetchSize;
+		// make a copy of data, as we need to modify content of this list later
+		this.elements = Collections.synchronizedList(new ArrayList<>(data));
+		this.unfetchedObjects = elements.size();
 	}
 
 	/**
@@ -147,18 +140,20 @@ public class IncrementalFaultList<E> implements List<E>, Serializable {
 	 * fully resolved. For the rest of the list, only ObjectIds are read.
 	 * 
 	 * @since 3.0
+	 * @deprecated since 5.0, does nothing
 	 */
+	@Deprecated(forRemoval = true)
 	protected void fillIn(final Query query, List<Object> elementsList) {
+	}
 
-		elementsList.clear();
-
-		try (ResultIterator<?> it = dataContext.performIteratedQuery(query)) {
-			while (it.hasNextRow()) {
-				elementsList.add(it.nextRow());
-			}
-		}
+	/**
+	 * Sets initial data (i.e. list of ObjectIds) for this FaultList
+	 *
+	 * @param elementsList initial data to fill this list with
+	 * @since 5.0
+	 */
+	public void fillIn(List<?> elementsList) {
 
-		unfetchedObjects = elementsList.size();
 	}
 
 	/**
@@ -173,6 +168,9 @@ public class IncrementalFaultList<E> implements List<E>, Serializable {
 	 * (DataObject or DataRows depending on the query type).
 	 */
 	private void validateListObject(Object object) throws IllegalArgumentException {
+		if(elements == null) {
+			throw new IllegalStateException("IncrementalFaultList is not initialized");
+		}
 
 		// I am not sure if such a check makes sense???
 
@@ -197,6 +195,10 @@ public class IncrementalFaultList<E> implements List<E>, Serializable {
 			return;
 		}
 
+		if(elements == null) {
+			throw new IllegalStateException("IncrementalFaultList is not initialized");
+		}
+
 		synchronized (elements) {
 			if (elements.size() == 0) {
 				return;
@@ -295,7 +297,7 @@ public class IncrementalFaultList<E> implements List<E>, Serializable {
 			throw new CayenneRuntimeException("Expected %d objects, retrieved %d", ids.size(), objects.size());
 		}
 
-		// We have less objects then ids
+		// We have fewer objects than ids
 		// check that we are really missing some ids and throw an exception in that case
 		StringBuilder buffer = null;
 		boolean first = true;
@@ -409,9 +411,13 @@ public class IncrementalFaultList<E> implements List<E>, Serializable {
 	 * next().
 	 */
 	public Iterator<E> iterator() {
+		if(elements == null) {
+			throw new IllegalStateException("IncrementalFaultList is not initialized");
+		}
+
 		// by virtue of get(index)'s implementation, resolution of ids into
 		// objects will occur on pageSize boundaries as necessary.
-		return new Iterator<E>() {
+		return new Iterator<>() {
 
 			int listIndex = 0;
 
@@ -459,6 +465,10 @@ public class IncrementalFaultList<E> implements List<E>, Serializable {
 	 * @see java.util.Collection#addAll(Collection)
 	 */
 	public boolean addAll(Collection<? extends E> c) {
+		if(elements == null) {
+			throw new IllegalStateException("IncrementalFaultList is not initialized");
+		}
+
 		synchronized (elements) {
 			return elements.addAll(c);
 		}
@@ -468,6 +478,10 @@ public class IncrementalFaultList<E> implements List<E>, Serializable {
 	 * @see java.util.List#addAll(int, Collection)
 	 */
 	public boolean addAll(int index, Collection<? extends E> c) {
+		if(elements == null) {
+			throw new IllegalStateException("IncrementalFaultList is not initialized");
+		}
+
 		synchronized (elements) {
 			return elements.addAll(index, c);
 		}
@@ -477,6 +491,10 @@ public class IncrementalFaultList<E> implements List<E>, Serializable {
 	 * @see java.util.Collection#clear()
 	 */
 	public void clear() {
+		if(elements == null) {
+			throw new IllegalStateException("IncrementalFaultList is not initialized");
+		}
+
 		synchronized (elements) {
 			elements.clear();
 		}
@@ -486,6 +504,10 @@ public class IncrementalFaultList<E> implements List<E>, Serializable {
 	 * @see java.util.Collection#contains(Object)
 	 */
 	public boolean contains(Object o) {
+		if(elements == null) {
+			throw new IllegalStateException("IncrementalFaultList is not initialized");
+		}
+
 		synchronized (elements) {
 			return elements.contains(o);
 		}
@@ -495,12 +517,20 @@ public class IncrementalFaultList<E> implements List<E>, Serializable {
 	 * @see java.util.Collection#containsAll(Collection)
 	 */
 	public boolean containsAll(Collection<?> c) {
+		if(elements == null) {
+			throw new IllegalStateException("IncrementalFaultList is not initialized");
+		}
+
 		synchronized (elements) {
 			return elements.containsAll(c);
 		}
 	}
 
 	public E get(int index) {
+		if(elements == null) {
+			throw new IllegalStateException("IncrementalFaultList is not initialized");
+		}
+
 		synchronized (elements) {
 			Object o = elements.get(index);
 
@@ -527,6 +557,10 @@ public class IncrementalFaultList<E> implements List<E>, Serializable {
 	 * @see java.util.Collection#isEmpty()
 	 */
 	public boolean isEmpty() {
+		if(elements == null) {
+			throw new IllegalStateException("IncrementalFaultList is not initialized");
+		}
+
 		synchronized (elements) {
 			return elements.isEmpty();
 		}
@@ -537,6 +571,10 @@ public class IncrementalFaultList<E> implements List<E>, Serializable {
 	}
 
 	public E remove(int index) {
+		if(elements == null) {
+			throw new IllegalStateException("IncrementalFaultList is not initialized");
+		}
+
 		synchronized (elements) {
 			// have to resolve the page to return correct object
 			E object = get(index);
@@ -546,18 +584,30 @@ public class IncrementalFaultList<E> implements List<E>, Serializable {
 	}
 
 	public boolean remove(Object o) {
+		if(elements == null) {
+			throw new IllegalStateException("IncrementalFaultList is not initialized");
+		}
+
 		synchronized (elements) {
 			return elements.remove(o);
 		}
 	}
 
 	public boolean removeAll(Collection<?> c) {
+		if(elements == null) {
+			throw new IllegalStateException("IncrementalFaultList is not initialized");
+		}
+
 		synchronized (elements) {
 			return elements.removeAll(c);
 		}
 	}
 
 	public boolean retainAll(Collection<?> c) {
+		if(elements == null) {
+			throw new IllegalStateException("IncrementalFaultList is not initialized");
+		}
+
 		synchronized (elements) {
 			return elements.retainAll(c);
 		}
@@ -578,12 +628,20 @@ public class IncrementalFaultList<E> implements List<E>, Serializable {
 	 * @see java.util.Collection#size()
 	 */
 	public int size() {
+		if(elements == null) {
+			throw new IllegalStateException("IncrementalFaultList is not initialized");
+		}
+
 		synchronized (elements) {
 			return elements.size();
 		}
 	}
 
 	public List<E> subList(int fromIndex, int toIndex) {
+		if(elements == null) {
+			throw new IllegalStateException("IncrementalFaultList is not initialized");
+		}
+
 		synchronized (elements) {
 			resolveInterval(fromIndex, toIndex);
 			return elements.subList(fromIndex, toIndex);
diff --git a/cayenne-server/src/main/java/org/apache/cayenne/access/MixedResultIncrementalFaultList.java b/cayenne-server/src/main/java/org/apache/cayenne/access/MixedResultIncrementalFaultList.java
index 33c5da5b1..471259ca3 100644
--- a/cayenne-server/src/main/java/org/apache/cayenne/access/MixedResultIncrementalFaultList.java
+++ b/cayenne-server/src/main/java/org/apache/cayenne/access/MixedResultIncrementalFaultList.java
@@ -70,8 +70,8 @@ class MixedResultIncrementalFaultList<E> extends IncrementalFaultList<E> {
      * @param query        Main query used to retrieve data. Must have "pageSize"
      *                     property set to a value greater than zero.
      */
-    MixedResultIncrementalFaultList(DataContext dataContext, Query query, int maxFetchSize) {
-        super(dataContext, query, maxFetchSize);
+    MixedResultIncrementalFaultList(DataContext dataContext, Query query, int maxFetchSize, List<?> data) {
+        super(dataContext, query, maxFetchSize, data);
     }
 
     @Override
@@ -89,8 +89,8 @@ class MixedResultIncrementalFaultList<E> extends IncrementalFaultList<E> {
             }
         }
 
-        // if there is no entities in this results,
-        // than all data is already there and we don't need to resolve any objects
+        // if there is no entities in these results,
+        // then all data is already there, and we don't need to resolve any objects
         if(indexToEntity.isEmpty()) {
             return new ScalarArrayListHelper();
         } else {
@@ -98,18 +98,6 @@ class MixedResultIncrementalFaultList<E> extends IncrementalFaultList<E> {
         }
     }
 
-    @Override
-    protected void fillIn(final Query query, List<Object> elementsList) {
-        elementsList.clear();
-        try (ResultIterator it = dataContext.performIteratedQuery(query)) {
-            while (it.hasNextRow()) {
-                elementsList.add(it.nextRow());
-            }
-        }
-
-        unfetchedObjects = elementsList.size();
-    }
-
     @Override
     protected void resolveInterval(int fromIndex, int toIndex) {
         if (fromIndex >= toIndex || scalarResult) {
diff --git a/cayenne-server/src/main/java/org/apache/cayenne/access/SimpleIdIncrementalFaultList.java b/cayenne-server/src/main/java/org/apache/cayenne/access/SimpleIdIncrementalFaultList.java
index d52e8cafa..c9c586048 100644
--- a/cayenne-server/src/main/java/org/apache/cayenne/access/SimpleIdIncrementalFaultList.java
+++ b/cayenne-server/src/main/java/org/apache/cayenne/access/SimpleIdIncrementalFaultList.java
@@ -19,6 +19,7 @@
 package org.apache.cayenne.access;
 
 import java.util.Collection;
+import java.util.List;
 import java.util.Map;
 
 import org.apache.cayenne.Persistent;
@@ -38,8 +39,8 @@ class SimpleIdIncrementalFaultList<E> extends IncrementalFaultList<E> {
 
     protected DbAttribute pk;
 
-    SimpleIdIncrementalFaultList(DataContext dataContext, Query query, int maxFetchSize) {
-        super(dataContext, query, maxFetchSize);
+    SimpleIdIncrementalFaultList(DataContext dataContext, Query query, int maxFetchSize, List<?> data) {
+        super(dataContext, query, maxFetchSize, data);
 
         Collection<DbAttribute> pks = rootEntity.getDbEntity().getPrimaryKeys();
         if (pks.size() != 1) {
diff --git a/cayenne-server/src/test/java/org/apache/cayenne/access/IncrementalFaultListIT.java b/cayenne-server/src/test/java/org/apache/cayenne/access/IncrementalFaultListIT.java
index 51822def5..388cce6ac 100644
--- a/cayenne-server/src/test/java/org/apache/cayenne/access/IncrementalFaultListIT.java
+++ b/cayenne-server/src/test/java/org/apache/cayenne/access/IncrementalFaultListIT.java
@@ -27,6 +27,8 @@ import org.apache.cayenne.unit.di.server.UseServerRuntime;
 import org.apache.cayenne.util.Util;
 import org.junit.Test;
 
+import java.util.List;
+
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 
@@ -41,7 +43,7 @@ public class IncrementalFaultListIT extends ServerCase {
 		ObjectSelect<Artist> query = ObjectSelect.query(Artist.class)
 				.pageSize(10);
 
-		IncrementalFaultList<Artist> i1 = new IncrementalFaultList<Artist>(context, query, 10);
+		IncrementalFaultList<Artist> i1 = new IncrementalFaultList<Artist>(context, query, 10, List.of());
 		IncrementalFaultList<Artist> i2 = Util.cloneViaSerialization(i1);
 
 		assertNotNull(i2);
diff --git a/cayenne-server/src/test/java/org/apache/cayenne/access/SimpleIdIncrementalFaultListDataRowsIT.java b/cayenne-server/src/test/java/org/apache/cayenne/access/SimpleIdIncrementalFaultListDataRowsIT.java
index 3808fe113..fa9110de8 100644
--- a/cayenne-server/src/test/java/org/apache/cayenne/access/SimpleIdIncrementalFaultListDataRowsIT.java
+++ b/cayenne-server/src/test/java/org/apache/cayenne/access/SimpleIdIncrementalFaultListDataRowsIT.java
@@ -64,8 +64,11 @@ public class SimpleIdIncrementalFaultListDataRowsIT extends ServerCase {
         createArtistsDataSet();
 
         ObjectSelect<DataRow> q = ObjectSelect.dataRowQuery(Artist.class)
-                .pageSize(6).orderBy("db:ARTIST_ID", SortOrder.ASCENDING);
-        list = new SimpleIdIncrementalFaultList<>(context, q, 10000);
+                .pageSize(6).orderBy(Artist.ARTIST_ID_PK_PROPERTY.asc());
+        List<Long> select = Artist.SELF.columnQuery(Artist.ARTIST_ID_PK_PROPERTY)
+                .orderBy(Artist.ARTIST_ID_PK_PROPERTY.asc())
+                .select(context);
+        list = new SimpleIdIncrementalFaultList<>(context, q, 10000, select);
     }
 
     protected void createArtistsDataSet() throws Exception {
diff --git a/cayenne-server/src/test/java/org/apache/cayenne/access/SimpleIdIncrementalFaultListIT.java b/cayenne-server/src/test/java/org/apache/cayenne/access/SimpleIdIncrementalFaultListIT.java
index 972712826..a8fc5c408 100644
--- a/cayenne-server/src/test/java/org/apache/cayenne/access/SimpleIdIncrementalFaultListIT.java
+++ b/cayenne-server/src/test/java/org/apache/cayenne/access/SimpleIdIncrementalFaultListIT.java
@@ -83,17 +83,32 @@ public class SimpleIdIncrementalFaultListIT extends ServerCase {
         tArtist.insert(33025, "artist25");
     }
 
+    private SimpleIdIncrementalFaultList<?> prepareList(int pageSize) throws Exception {
+        createArtistsDataSet();
+
+        ObjectSelect<Artist> query = Artist.SELF.query()
+                // make sure total number of objects is not dividable
+                // by the page size, to test the last smaller page
+                .pageSize(pageSize)
+                .orderBy(Artist.ARTIST_ID_PK_PROPERTY.asc());
+        // we need manually fill data here
+        List<Long> ids = Artist.SELF.columnQuery(Artist.ARTIST_ID_PK_PROPERTY)
+                .orderBy(Artist.ARTIST_ID_PK_PROPERTY.asc())
+                .select(context);
+
+        return new SimpleIdIncrementalFaultList<>(context, query, 10000, ids);
+    }
+
     @Test
     public void testRemoveDeleted() throws Exception {
         createArtistsDataSet();
 
-        // DataContext context = createDataContext();
-
-        ObjectSelect<Artist> query = ObjectSelect.query(Artist.class)
-                .pageSize(10);
-        SimpleIdIncrementalFaultList<Artist> list = new SimpleIdIncrementalFaultList<Artist>(
+        ObjectSelect<Artist> query = Artist.SELF.query().pageSize(10);
+        SimpleIdIncrementalFaultList<Artist> list = new SimpleIdIncrementalFaultList<>(
                 context,
-                query, 10000);
+                query,
+                10000,
+                Artist.SELF.columnQuery(Artist.ARTIST_ID_PK_PROPERTY).select(context));
 
         assertEquals(25, list.size());
 
@@ -105,17 +120,6 @@ public class SimpleIdIncrementalFaultListIT extends ServerCase {
         assertEquals(24, list.size());
     }
 
-    private SimpleIdIncrementalFaultList<?> prepareList(int pageSize) throws Exception {
-        createArtistsDataSet();
-
-        ObjectSelect<Artist> query = ObjectSelect.query(Artist.class)
-                // make sure total number of objects is not divisable
-                // by the page size, to test the last smaller page
-                .pageSize(pageSize)
-                .orderBy("db:ARTIST_ID", SortOrder.ASCENDING);
-        return new SimpleIdIncrementalFaultList<>(context, query, 10000);
-    }
-
     @Test
     public void testSize() throws Exception {
         SimpleIdIncrementalFaultList<?> list = prepareList(6);
@@ -173,7 +177,11 @@ public class SimpleIdIncrementalFaultListIT extends ServerCase {
 
         SimpleIdIncrementalFaultList<Artist> list = new SimpleIdIncrementalFaultList<>(
                 context,
-                q, 10000);
+                q,
+                10000,
+                Artist.SELF.columnQuery(Artist.ARTIST_ID_PK_PROPERTY)
+                        .orderBy(Artist.ARTIST_NAME.asc())
+                        .select(context));
 
         assertSame(newArtist, list.get(25));
     }


(cayenne) 02/02: CAY-2819 `DataContext.performIteratedQuery()` method should be unified with `iterator()` method

Posted by nt...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

ntimofeev pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/cayenne.git

commit b72b1f4f8be3ee22e09ae4300ff696af6081adf6
Author: stariy95 <st...@gmail.com>
AuthorDate: Thu Nov 9 18:07:33 2023 +0400

    CAY-2819 `DataContext.performIteratedQuery()` method should be unified with `iterator()` method
---
 RELEASE-NOTES.txt                                  |  1 +
 .../org/apache/cayenne/access/DataContext.java     | 95 +++++++---------------
 .../cayenne/access/DataDomainQueryAction.java      |  1 +
 .../cayenne/query/IteratedQueryDecorator.java      | 17 +++-
 4 files changed, 44 insertions(+), 70 deletions(-)

diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt
index e1ef21051..8d74cc967 100644
--- a/RELEASE-NOTES.txt
+++ b/RELEASE-NOTES.txt
@@ -41,6 +41,7 @@ CAY-2802 Upgrade Gradle to 7.6.1
 CAY-2803 Test infrastructure: declarative custom DI modules in ServerCase
 CAY-2805 Stop calling exp parser internally
 CAY-2817 Pagination flow refactoring
+CAY-2819 DataContext.performIteratedQuery() method should be unified with iterator() method
 
 Bug Fixes:
 
diff --git a/cayenne-server/src/main/java/org/apache/cayenne/access/DataContext.java b/cayenne-server/src/main/java/org/apache/cayenne/access/DataContext.java
index ec930c81a..4206edfd5 100644
--- a/cayenne-server/src/main/java/org/apache/cayenne/access/DataContext.java
+++ b/cayenne-server/src/main/java/org/apache/cayenne/access/DataContext.java
@@ -792,84 +792,45 @@ public class DataContext extends BaseContext {
 
     }
 
-    @SuppressWarnings("unchecked")
+    /**
+     * Performs a single database select query returning result as a {@link ResultIterator}.
+     * <p>
+     * It is caller's responsibility to explicitly close the {@link ResultIterator}.
+     * A failure to do so will result in a <b>database connection not being released</b>.
+     * Another side effect of an open {@link ResultIterator} is that an internal Cayenne transaction
+     * that originated in this method stays open until the iterator is closed.
+     * So users should normally close the iterator within the same thread that opened it.
+     * <p>
+     */
     @Override
     public <T> ResultIterator<T> iterator(final Select<T> query) {
-        IteratedQueryDecorator queryDecorator = new IteratedQueryDecorator(query);
-        Query queryToRun = nonNullDelegate().willPerformQuery(this, queryDecorator);
-        QueryResponse queryResponse = onQuery(this, queryToRun);
-        return (ResultIterator<T>) queryResponse.firstIterator();
+        return performIteratedQueryInternal(query, false);
     }
 
     /**
-     * Performs a single database select query returning result as a
-     * ResultIterator. It is caller's responsibility to explicitly close the
-     * ResultIterator. A failure to do so will result in a database connection
-     * not being released. Another side effect of an open ResultIterator is that
-     * an internal Cayenne transaction that originated in this method stays open
-     * until the iterator is closed. So users should normally close the iterator
-     * within the same thread that opened it.
+     * Performs a single database select query returning result as a {@link ResultIterator}.
+     * <p>
+     * It is caller's responsibility to explicitly close the {@link ResultIterator}.
+     * A failure to do so will result in a <b>database connection not being released</b>.
+     * Another side effect of an open {@link ResultIterator} is that an internal Cayenne transaction
+     * that originated in this method stays open until the iterator is closed.
+     * So users should normally close the iterator within the same thread that opened it.
      * <p>
-     * Note that 'performIteratedQuery' always returns ResultIterator over
-     * DataRows. Use
-     * {@link #iterate(Select, org.apache.cayenne.ResultIteratorCallback)} to
-     * get access to objects.
+     * Note that {@code performIteratedQuery} always returns {@link ResultIterator} over DataRows.
+     * <p>
+     * Use {@link #iterate(Select, org.apache.cayenne.ResultIteratorCallback)} to get access to objects.
      */
-    // TODO: deprecate once all selecting queries start implementing Select<T> interface
     @SuppressWarnings({ "rawtypes" })
     public ResultIterator performIteratedQuery(Query query) {
-
-        if (BaseTransaction.getThreadTransaction() != null) {
-            return internalPerformIteratedQuery(query);
-        } else {
-
-
-            // can't use TransactionManger here as it would attempt to commit the transaction at the end...
-
-            // manually manage a transaction, so that a ResultIterator wrapper
-            // could close it when it is done.
-            Transaction tx = getTransactionFactory().createTransaction();
-            BaseTransaction.bindThreadTransaction(tx);
-
-            ResultIterator<?> result;
-            try {
-                result = internalPerformIteratedQuery(query);
-            } catch (Exception e) {
-
-                tx.setRollbackOnly();
-                throw new CayenneRuntimeException(e);
-            } finally {
-
-                // unbind thread tx before returning to the caller. Transaction will be managed internally by the
-                // ResultIterator and should not get in the way of other DB operations that may be performed
-                // within the iterator....
-
-                // TODO: there was an older comment about Ingres breaking when we unbind thread transaction
-                // before closing the iterator. As we have no test environment for ingres, we can't
-                // confirm this here...
-                BaseTransaction.bindThreadTransaction(null);
-
-                if (tx.isRollbackOnly()) {
-                    try {
-                        tx.rollback();
-                    } catch (Exception ignored) {
-                    }
-                }
-            }
-
-            return new TransactionResultIteratorDecorator<>(result, tx);
-        }
+        return performIteratedQueryInternal(query, true);
     }
 
-    /**
-     * Runs an iterated query in a transactional context provided by the caller.
-     */
-    ResultIterator <?> internalPerformIteratedQuery(Query query) {
-        // note that for now DataChannel API does not support cursors (aka
-        // ResultIterator), so we have to go directly to the DataDomain.
-        IteratedSelectObserver observer = new IteratedSelectObserver();
-        getParentDataDomain().performQueries(Collections.singletonList(query), observer);
-        return observer.getResultIterator();
+    @SuppressWarnings("unchecked")
+    private <T> ResultIterator<T> performIteratedQueryInternal(Query query, boolean fetchDataRows) {
+        IteratedQueryDecorator queryDecorator = new IteratedQueryDecorator(query, fetchDataRows);
+        Query queryToRun = nonNullDelegate().willPerformQuery(this, queryDecorator);
+        QueryResponse queryResponse = onQuery(this, queryToRun);
+        return (ResultIterator<T>)queryResponse.firstIterator();
     }
 
     /**
diff --git a/cayenne-server/src/main/java/org/apache/cayenne/access/DataDomainQueryAction.java b/cayenne-server/src/main/java/org/apache/cayenne/access/DataDomainQueryAction.java
index e6778c442..22f07a2da 100644
--- a/cayenne-server/src/main/java/org/apache/cayenne/access/DataDomainQueryAction.java
+++ b/cayenne-server/src/main/java/org/apache/cayenne/access/DataDomainQueryAction.java
@@ -146,6 +146,7 @@ class DataDomainQueryAction implements QueryRouter, OperationObserver {
 
     private boolean interceptIteratedQuery() {
         if (query instanceof IteratedQueryDecorator) {
+            noObjectConversion = ((IteratedQueryDecorator) query).isFetchingDataRows();
             validateIteratedQuery();
             performIteratedQuery();
             return DONE;
diff --git a/cayenne-server/src/main/java/org/apache/cayenne/query/IteratedQueryDecorator.java b/cayenne-server/src/main/java/org/apache/cayenne/query/IteratedQueryDecorator.java
index 9093bd7f9..bbb2e318e 100644
--- a/cayenne-server/src/main/java/org/apache/cayenne/query/IteratedQueryDecorator.java
+++ b/cayenne-server/src/main/java/org/apache/cayenne/query/IteratedQueryDecorator.java
@@ -21,12 +21,19 @@ package org.apache.cayenne.query;
 
 import org.apache.cayenne.map.EntityResolver;
 
-public class IteratedQueryDecorator implements Query{
+/**
+ * A simple decorator for an iterated query.
+ * @see org.apache.cayenne.access.DataContext#iterator(Select)
+ * @since 5.0
+ */
+public class IteratedQueryDecorator implements Query {
 
     private final Query query;
+    private final boolean fetchDataRows;
 
-    public IteratedQueryDecorator(Query query) {
+    public IteratedQueryDecorator(Query query, boolean fetchDataRows) {
         this.query = query;
+        this.fetchDataRows = fetchDataRows;
     }
 
     @Override
@@ -36,7 +43,7 @@ public class IteratedQueryDecorator implements Query{
 
     @Override
     public void route(QueryRouter router, EntityResolver resolver, Query substitutedQuery) {
-        query.route(router,resolver,substitutedQuery);
+        query.route(router, resolver, substitutedQuery);
     }
 
     @Override
@@ -47,4 +54,8 @@ public class IteratedQueryDecorator implements Query{
     public Query getQuery() {
         return query;
     }
+
+    public boolean isFetchingDataRows() {
+        return fetchDataRows;
+    }
 }