diff --git a/openjpa-lib/src/main/java/org/apache/openjpa/lib/jdbc/LoggingConnectionDecorator.java b/openjpa-lib/src/main/java/org/apache/openjpa/lib/jdbc/LoggingConnectionDecorator.java index 78f179a37..735068850 100644 --- a/openjpa-lib/src/main/java/org/apache/openjpa/lib/jdbc/LoggingConnectionDecorator.java +++ b/openjpa-lib/src/main/java/org/apache/openjpa/lib/jdbc/LoggingConnectionDecorator.java @@ -125,6 +125,7 @@ public class LoggingConnectionDecorator implements ConnectionDecorator { private int _warningAction = WARN_IGNORE; private SQLWarningHandler _warningHandler; private boolean _trackParameters = true; + private boolean _printParameters = false; /** * If set to true, pretty-print SQL by running it @@ -169,19 +170,34 @@ public class LoggingConnectionDecorator implements ConnectionDecorator { } /** - * Whether to track parameters for the purposes of reporting exceptions. + *

Whether to track parameters for the purpose of reporting exceptions.

*/ public void setTrackParameters(boolean trackParameters) { _trackParameters = trackParameters; } /** - * Whether to track parameters for the purposes of reporting exceptions. + * Whether to track parameters for the purpose of reporting exceptions. */ public boolean getTrackParameters() { return _trackParameters; } + /** + *

+ * Whether parameter values will be printed in exception messages or in trace. This is different from + * trackParameters which controls whether OpenJPA will track parameters internally (visible while debugging and used + * in batching). + *

+ */ + public boolean getPrintParameters() { + return _printParameters; + } + + public void setPrintParameters(boolean printParameters) { + _printParameters = printParameters; + } + /** * What to do with SQL warnings. */ @@ -1392,17 +1408,23 @@ public class LoggingConnectionDecorator implements ConnectionDecorator { paramBuf = new StringBuilder(); for (Iterator itr = _params.iterator(); itr .hasNext();) { - paramBuf.append(itr.next()); - if (itr.hasNext()) + if (_printParameters) { + paramBuf.append(itr.next()); + } else { + paramBuf.append("?"); + itr.next(); + } + if (itr.hasNext()) { paramBuf.append(", "); + } } } if (paramBuf != null) { - if (!_prettyPrint) + if (!_prettyPrint) { buf.append(" "); - buf.append("[params="). - append(paramBuf.toString()).append("]"); + } + buf.append("[params=").append(paramBuf.toString()).append("]"); } super.appendInfo(buf); } diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/batch/exception/TestBatchLimitException.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/batch/exception/TestBatchLimitException.java index a88d26749..b102c9a9e 100644 --- a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/batch/exception/TestBatchLimitException.java +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/batch/exception/TestBatchLimitException.java @@ -51,6 +51,7 @@ public class TestBatchLimitException extends PersistenceTestCase { "openjpa.jdbc.SynchronizeMappings", "buildSchema(ForeignKeys=true)", "openjpa.jdbc.DBDictionary", batchLimit, + "openjpa.ConnectionFactoryProperties", "PrintParameters=true", CLEAR_TABLES); assertNotNull("Unable to create EntityManagerFactory", emf); diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/exception/TestParameterLogging.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/exception/TestParameterLogging.java new file mode 100644 index 000000000..e3891951c --- /dev/null +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/exception/TestParameterLogging.java @@ -0,0 +1,185 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.openjpa.persistence.exception; + +import java.io.BufferedReader; +import java.io.File; +import java.io.FileReader; +import java.util.regex.Pattern; + +import javax.persistence.EntityManager; +import javax.persistence.EntityManagerFactory; +import javax.persistence.EntityTransaction; +import javax.persistence.RollbackException; + +import org.apache.openjpa.persistence.test.PersistenceTestCase; + +public class TestParameterLogging extends PersistenceTestCase { + + String _regex = ".*params=.*1,.*]"; + + /* + * Persist the same row twice in the same transaction - will throw an exception with the failing SQL statement + */ + private RollbackException getRollbackException(Object... props) { + EntityManagerFactory emf = createEMF(props); + EntityManager em = emf.createEntityManager(); + EntityTransaction tran = em.getTransaction(); + + PObject p1, p2; + p1 = new PObject(); + p2 = new PObject(); + + p1.setId(1); + p2.setId(1); + + try { + tran.begin(); + em.persist(p1); + em.persist(p2); + tran.commit(); + em.close(); + fail("Expected a RollbackException"); + return null; + } catch (RollbackException re) { + return re; + } finally { + if (tran.isActive()) { + tran.rollback(); + } + if (em.isOpen()) { + em.close(); + } + if (emf.isOpen()) { + emf.close(); + } + } + } + + /* + * Ensure that parameter values are not included in exception text by default. + */ + public void testNoParamsByDefault() { + RollbackException e = getRollbackException(PObject.class, CLEAR_TABLES); + + assertFalse(Pattern.matches(_regex, e.toString())); + Throwable nested = e.getCause(); + while (nested != null) { + if (Pattern.matches(".*INSERT.*", nested.toString())) { + // only check if the message contains the insert statement. + assertFalse(Pattern.matches(_regex, nested.toString())); + } + nested = nested.getCause(); + } + } + + /* + * If the EMF is created with PrintParameters=true the parameter values will be logged in exception text. + */ + public void testParamsEnabledByConfig() { + RollbackException e = + getRollbackException(PObject.class, CLEAR_TABLES, "openjpa.ConnectionFactoryProperties", + "PrintParameters=true"); + assertFalse(Pattern.matches(_regex, e.toString())); + Throwable nested = e.getCause(); + assertNotNull(nested); // expecting at least one nested exception. + while (nested != null) { + if (Pattern.matches(".*INSERT.*", nested.toString())) { + // only check if the message contains the insert statement. + assertTrue(Pattern.matches(_regex, nested.toString())); + } + nested = nested.getCause(); + } + } + + /* + * If the EMF is created with PrintParameters=false and trace is enabled for SQL the parameter values will not be + * logged in exception text. + */ + public void testParamsDisbledWithLogging() throws Exception { + RollbackException e = + getRollbackException(PObject.class, CLEAR_TABLES, "openjpa.ConnectionFactoryProperties", + "PrintParameters=false", "openjpa.Log", "SQL=TRACE,File=temp.txt"); + assertFalse(Pattern.matches(_regex, e.toString())); + Throwable nested = e.getCause(); + assertNotNull(nested); // should be at least one nested exception + while (nested != null) { + if (Pattern.matches(".*INSERT.*", nested.toString())) { + // only check if the message contains the insert statement. + assertFalse(Pattern.matches(_regex, nested.toString())); + } + nested = nested.getCause(); + } + checkAndDeleteLogFile("temp.txt", false); + } + + /* + * If the EMF is created with PrintParameters=false and trace is enabled for SQL the parameter values will not be + * logged in exception text. + */ + public void testParamsEnabledWithLogging() throws Exception { + RollbackException e = + getRollbackException(PObject.class, CLEAR_TABLES, "openjpa.ConnectionFactoryProperties", + "PrintParameters=true", "openjpa.Log", "SQL=TRACE,File=temp.txt"); + assertFalse(Pattern.matches(_regex, e.toString())); + Throwable nested = e.getCause(); + assertNotNull(nested); // should be at least one nested exception + while (nested != null) { + if (Pattern.matches(".*INSERT.*", nested.toString())) { + // only check if the message contains the insert statement. + assertTrue(Pattern.matches(_regex, nested.toString())); + } + nested = nested.getCause(); + } + checkAndDeleteLogFile("temp.txt", true); + } + + private void checkAndDeleteLogFile(String filename, boolean containsParams) throws Exception { + File f = null; + FileReader fr = null; + BufferedReader br = null; + try { + f = new File(filename); + fr = new FileReader(f); + br = new BufferedReader(fr); + + String s = br.readLine(); + while (s != null) { + if (Pattern.matches(".*INSERT.*", s)) { + if (containsParams) { + assertTrue(Pattern.matches(_regex, s)); + } else { + assertFalse(Pattern.matches(_regex, s)); + } + } + s = br.readLine(); + } + } finally { + if (br != null) { + br.close(); + } + if (fr != null) { + fr.close(); + } + if (f != null) { + f.delete(); + } + } + } +}