CAY-2912 Compact SQL logger

cleanup

Author: andrus

cayenne/src/main/java/org/apache/cayenne/access/DataNode.java

@@ -262,7 +262,7 @@ public void performQueries(Collection<? extends Query> queries, OperationObserve
                 try {
                     getAdapter()
                             .getAction(nextQuery, this)
-                            .performAction(connection, new OriginalQueryObserver(instrumentedCallback, nextQuery));
+                            .performAction(connection, instrumentedCallback);
                 } catch (Exception queryEx) {
                     instrumentedCallback.nextQueryException(nextQuery, queryEx);
 

cayenne/src/main/java/org/apache/cayenne/access/OperationObserver.java

@@ -19,6 +19,7 @@
 
 package org.apache.cayenne.access;
 
+import org.apache.cayenne.CayenneRuntimeException;
 import org.apache.cayenne.ObjectId;
 import org.apache.cayenne.ResultIterator;
 import org.apache.cayenne.access.translator.TranslatedStatement;
@@ -29,6 +30,10 @@
 /**
  * Defines a set of callback methods that allow {@link DataNode} to pass back query
  * results and notify caller about exceptions.
+ * <p>
+ * All methods have default implementations: the result callbacks do nothing, the exception callbacks rethrow the
+ * reported exception wrapped in a {@link CayenneRuntimeException}, and {@link #isIteratedResult()} returns false. An
+ * implementation only needs to override the callbacks it cares about.
  */
 public interface OperationObserver extends OperationHints {
 
@@ -50,22 +55,33 @@ default void nextStatement(Query query, TranslatedStatement statement) {
     default void onSuccess() {
     }
 
+    /**
+     * Returns whether results should be returned as a {@link ResultIterator}. Defaults to false.
+     */
+    @Override
+    default boolean isIteratedResult() {
+        return false;
+    }
+
     /**
      * Callback method invoked after an updating query is executed.
      */
-    void nextCount(Query query, int resultCount);
+    default void nextCount(Query query, int resultCount) {
+    }
 
     /**
      * Callback method invoked after a batch update is executed.
      */
-    void nextBatchCount(Query query, int[] resultCount);
+    default void nextBatchCount(Query query, int[] resultCount) {
+    }
 
     /**
      * Callback method invoked for each processed ResultSet.
      *
      * @since 3.0
      */
-    void nextRows(Query query, List<?> dataRows);
+    default void nextRows(Query query, List<?> dataRows) {
+    }
 
     /**
      * Callback method invoked for each opened ResultIterator. If this observer requested
@@ -74,24 +90,31 @@ default void onSuccess() {
      *
      * @since 3.0
      */
-    void nextRows(Query q, ResultIterator<?> it);
+    default void nextRows(Query q, ResultIterator<?> it) {
+    }
 
     /**
      * Callback method invoked after each batch of generated values is read during an update.
      *
      * @since 4.2
      */
-    void nextGeneratedRows(Query query, ResultIterator<?> keys, List<ObjectId> idsToUpdate);
+    default void nextGeneratedRows(Query query, ResultIterator<?> keys, List<ObjectId> idsToUpdate) {
+    }
 
     /**
      * Callback method invoked on exceptions that happen during an execution of a specific
-     * query.
+     * query. The default implementation rethrows the exception wrapped in a {@link CayenneRuntimeException}.
      */
-    void nextQueryException(Query query, Exception ex);
+    default void nextQueryException(Query query, Exception ex) {
+        throw new CayenneRuntimeException(ex);
+    }
 
     /**
      * Callback method invoked on exceptions that are not tied to a specific query
-     * execution, such as JDBC connection exceptions, etc.
+     * execution, such as JDBC connection exceptions, etc. The default implementation rethrows the exception wrapped in
+     * a {@link CayenneRuntimeException}.
      */
-    void nextGlobalException(Exception ex);
+    default void nextGlobalException(Exception ex) {
+        throw new CayenneRuntimeException(ex);
+    }
 }

cayenne/src/main/java/org/apache/cayenne/access/OriginalQueryObserver.java

@@ -19,28 +19,27 @@
 
 package org.apache.cayenne.access.flush;
 
-import java.util.List;
-
 import org.apache.cayenne.CayenneRuntimeException;
 import org.apache.cayenne.DataRow;
 import org.apache.cayenne.ObjectId;
 import org.apache.cayenne.ResultIterator;
 import org.apache.cayenne.access.OperationObserver;
 import org.apache.cayenne.log.SqlLogger;
 import org.apache.cayenne.map.DbAttribute;
-import org.apache.cayenne.query.BatchQuery;
 import org.apache.cayenne.query.InsertBatchQuery;
 import org.apache.cayenne.query.Query;
 import org.apache.cayenne.util.Util;
 
+import java.util.List;
+
 /**
  * @since 4.2
  */
 class FlushObserver implements OperationObserver {
 
-    private SqlLogger logger;
+    private final SqlLogger logger;
 
-    FlushObserver(SqlLogger logger) {
+    public FlushObserver(SqlLogger logger) {
         this.logger = logger;
     }
 
@@ -63,13 +62,11 @@ public void nextGeneratedRows(Query query, ResultIterator<?> keysIterator, List<
 
         // read and close the iterator before doing anything else
         List<DataRow> keys;
-        try {
+        try (keysIterator) {
             keys = (List<DataRow>) keysIterator.allRows();
-        } finally {
-            keysIterator.close();
         }
 
-        if (!(query instanceof InsertBatchQuery)) {
+        if (!(query instanceof InsertBatchQuery batch)) {
             throw new CayenneRuntimeException("Generated keys only supported for InsertBatchQuery, instead got %s", query);
         }
 
@@ -80,8 +77,7 @@ public void nextGeneratedRows(Query query, ResultIterator<?> keysIterator, List<
         for (int i = 0; i < keys.size(); i++) {
 	        DataRow key = keys.get(i);
 
-	        // empty key?
-	        if (key.size() == 0) {
+	        if (key.isEmpty()) {
 	            throw new CayenneRuntimeException("Empty key generated.");
 	        }
 
@@ -91,8 +87,7 @@ public void nextGeneratedRows(Query query, ResultIterator<?> keysIterator, List<
 	            return;
 	        }
 
-	        BatchQuery batch = (BatchQuery) query;
-	        for (DbAttribute attribute : batch.getDbEntity().getGeneratedAttributes()) {
+            for (DbAttribute attribute : batch.getDbEntity().getGeneratedAttributes()) {
 
 	            // batch can have generated attributes that are not PKs, e.g.
 	            // columns with
@@ -121,26 +116,6 @@ public void nextGeneratedRows(Query query, ResultIterator<?> keysIterator, List<
         }
     }
 
-    public void setSqlLogger(SqlLogger logger) {
-        this.logger = logger;
-    }
-
-    public SqlLogger getSqlLogger() {
-        return this.logger;
-    }
-
-    @Override
-    public void nextBatchCount(Query query, int[] resultCount) {
-    }
-
-    @Override
-    public void nextCount(Query query, int resultCount) {
-    }
-
-    @Override
-    public void nextRows(Query query, List<?> dataRows) {
-    }
-
     @Override
     @SuppressWarnings("rawtypes")
     public void nextRows(Query q, ResultIterator it) {

cayenne/src/main/java/org/apache/cayenne/access/flush/FlushObserver.java

@@ -18,22 +18,27 @@
  ****************************************************************/
 package org.apache.cayenne.access.jdbc;
 
+import org.apache.cayenne.ObjectId;
+import org.apache.cayenne.ResultIterator;
 import org.apache.cayenne.access.DataNode;
 import org.apache.cayenne.access.OperationObserver;
 import org.apache.cayenne.access.translator.ejbql.EJBQLTranslationContext;
 import org.apache.cayenne.access.translator.EJBQLTranslator;
+import org.apache.cayenne.access.translator.TranslatedStatement;
 import org.apache.cayenne.dba.QuotingStrategy;
 import org.apache.cayenne.ejbql.EJBQLBaseVisitor;
 import org.apache.cayenne.ejbql.EJBQLCompiledExpression;
 import org.apache.cayenne.map.DbEntity;
 import org.apache.cayenne.ejbql.EJBQLExpression;
 import org.apache.cayenne.ejbql.EJBQLExpressionVisitor;
 import org.apache.cayenne.query.EJBQLQuery;
+import org.apache.cayenne.query.Query;
 import org.apache.cayenne.query.QueryMetadata;
 import org.apache.cayenne.query.SQLActionVisitor;
 import org.apache.cayenne.query.SQLTemplate;
 
 import java.sql.Connection;
+import java.util.List;
 
 /**
  * Parses an EJBQL statement, converting it to SQL. Executes the resulting SQL.
@@ -110,6 +115,69 @@ public boolean visitUpdate(EJBQLExpression expression) {
             sqlQuery.setQueryTimeout(queryTimeout);
         }
 
-        actionFactory.sqlAction(sqlQuery).performAction(connection, observer);
+        // the SQLTemplate is a substitute for the original EJBQLQuery; wrap the observer so that results are reported
+        // against the EJBQLQuery the caller submitted rather than the internally compiled SQLTemplate
+        actionFactory.sqlAction(sqlQuery).performAction(connection, new OriginalQueryObserver(observer, query));
+    }
+    
+    static class OriginalQueryObserver implements OperationObserver {
+
+        private final OperationObserver delegate;
+        private final Query originalQuery;
+
+        OriginalQueryObserver(OperationObserver delegate, Query originalQuery) {
+            this.delegate = delegate;
+            this.originalQuery = originalQuery;
+        }
+
+        @Override
+        public void nextStatement(Query query, TranslatedStatement statement) {
+            delegate.nextStatement(originalQuery, statement);
+        }
+
+        @Override
+        public void onSuccess() {
+            delegate.onSuccess();
+        }
+
+        @Override
+        public void nextCount(Query query, int resultCount) {
+            delegate.nextCount(originalQuery, resultCount);
+        }
+
+        @Override
+        public void nextBatchCount(Query query, int[] resultCount) {
+            delegate.nextBatchCount(originalQuery, resultCount);
+        }
+
+        @Override
+        public void nextRows(Query query, List<?> dataRows) {
+            delegate.nextRows(originalQuery, dataRows);
+        }
+
+        @Override
+        public void nextRows(Query query, ResultIterator<?> it) {
+            delegate.nextRows(originalQuery, it);
+        }
+
+        @Override
+        public void nextGeneratedRows(Query query, ResultIterator<?> keys, List<ObjectId> idsToUpdate) {
+            delegate.nextGeneratedRows(originalQuery, keys, idsToUpdate);
+        }
+
+        @Override
+        public void nextQueryException(Query query, Exception ex) {
+            delegate.nextQueryException(originalQuery, ex);
+        }
+
+        @Override
+        public void nextGlobalException(Exception ex) {
+            delegate.nextGlobalException(ex);
+        }
+
+        @Override
+        public boolean isIteratedResult() {
+            return delegate.isIteratedResult();
+        }
     }
 }