From 5a1faec4719c40ee0769901f2b4a959084e67cd9 Mon Sep 17 00:00:00 2001 From: Patrick Linskey Date: Thu, 24 Jan 2008 07:33:32 +0000 Subject: [PATCH] OPENJPA-502 git-svn-id: https://svn.apache.org/repos/asf/openjpa/trunk@614812 13f79535-47bb-0310-9956-ffa450edef68 --- .../apache/openjpa/conf/Compatibility.java | 60 ++++++++++++++++++- .../openjpa/kernel/ExpressionStoreQuery.java | 4 ++ .../apache/openjpa/kernel/StoreContext.java | 3 +- .../apache/openjpa/kernel/exps/Resolver.java | 8 +++ .../kernel/jpql/JPQLExpressionBuilder.java | 41 +++++++++++++ .../openjpa/kernel/jpql/ParseException.java | 12 ++++ .../org/apache/openjpa/kernel/jpql/JPQL.jjt | 7 +-- .../openjpa/kernel/jpql/localizer.properties | 8 +++ .../persistence/query/GroupingTestCase.java | 3 +- .../persistence/query/TestSubstring.java | 3 +- 10 files changed, 139 insertions(+), 10 deletions(-) diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/conf/Compatibility.java b/openjpa-kernel/src/main/java/org/apache/openjpa/conf/Compatibility.java index e8aa07b6c..45f85f9ce 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/conf/Compatibility.java +++ b/openjpa-kernel/src/main/java/org/apache/openjpa/conf/Compatibility.java @@ -23,6 +23,29 @@ package org.apache.openjpa.conf; */ public class Compatibility { + /** + * If a JPQL statement is not compliant with the JPA specification, + * fail to parse it. + * + * @since 1.1.0 + */ + public static final int JPQL_STRICT = 0; + + /** + * If a JPQL statement is not compliant with the JPA specification, + * warn the first time that statement is parsed. + * + * @since 1.1.0 + */ + public static final int JPQL_WARN = 1; + + /** + * Allow non-compliant extensions of JPQL. + * + * @since 1.1.0 + */ + public static final int JPQL_EXTENDED = 2; + private boolean _strictIdValues = false; private boolean _hollowLookups = true; private boolean _checkStore = false; @@ -30,6 +53,7 @@ public class Compatibility { private boolean _closeOnCommit = true; private boolean _quotedNumbers = false; private boolean _nonOptimisticVersionCheck = false; + private int _jpql = JPQL_STRICT; /** * Whether to require exact identity value types when creating object @@ -146,8 +170,7 @@ public class Compatibility { * in a datastore transaction. Version of OpenJPA prior to 0.4.1 always * forced a version check. */ - public void setNonOptimisticVersionCheck - (boolean nonOptimisticVersionCheck) { + public void setNonOptimisticVersionCheck(boolean nonOptimisticVersionCheck){ _nonOptimisticVersionCheck = nonOptimisticVersionCheck; } @@ -159,4 +182,37 @@ public class Compatibility { public boolean getNonOptimisticVersionCheck() { return _nonOptimisticVersionCheck; } + + /** + * Whether or not JPQL extensions are allowed. Defaults to + * {@link #JPQL_STRICT}. + * + * @since 1.1.0 + * @see #JPQL_WARN + * @see #JPQL_STRICT + * @see #JPQL_EXTENDED + */ + public int getJPQL() { + return _jpql; + } + + /** + * Whether or not JPQL extensions are allowed. Possible values: "warn", + * "strict", "extended". + * + * @since 1.1.0 + * @see #JPQL_WARN + * @see #JPQL_STRICT + * @see #JPQL_EXTENDED + */ + public void setJPQL(String jpql) { + if ("warn".equals(jpql)) + _jpql = JPQL_WARN; + else if ("strict".equals(jpql)) + _jpql = JPQL_STRICT; + else if ("extended".equals(jpql)) + _jpql = JPQL_EXTENDED; + else + throw new IllegalArgumentException(jpql); + } } diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/ExpressionStoreQuery.java b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/ExpressionStoreQuery.java index dd9520a01..e67abfb14 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/ExpressionStoreQuery.java +++ b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/ExpressionStoreQuery.java @@ -119,6 +119,10 @@ public class ExpressionStoreQuery public OpenJPAConfiguration getConfiguration() { return ctx.getStoreContext().getConfiguration(); } + + public QueryContext getQueryContext() { + return ctx; + } }; } diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/StoreContext.java b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/StoreContext.java index 05353b4b9..a05705ebc 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/StoreContext.java +++ b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/StoreContext.java @@ -46,8 +46,7 @@ public interface StoreContext { /** * Return the broker for this context, if possible. Note that a broker - * will be unavailable in remote contexts, and this method may throw - * an exception to that effect. + * will be unavailable in remote contexts, and this method may return null. */ public Broker getBroker(); diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Resolver.java b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Resolver.java index 505b94269..be0361ee2 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Resolver.java +++ b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/exps/Resolver.java @@ -19,6 +19,7 @@ package org.apache.openjpa.kernel.exps; import org.apache.openjpa.conf.OpenJPAConfiguration; +import org.apache.openjpa.kernel.QueryContext; /** * A Resolver is used to resolve listeners and class or entity names @@ -51,4 +52,11 @@ public interface Resolver { * Return the OpenJPA configuration. */ public OpenJPAConfiguration getConfiguration (); + + /** + * The {@link QueryContext} for which this resolver was created + * + * @since 1.1.0 + */ + public QueryContext getQueryContext(); } diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/jpql/JPQLExpressionBuilder.java b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/jpql/JPQLExpressionBuilder.java index acb939cd2..4642189cf 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/jpql/JPQLExpressionBuilder.java +++ b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/jpql/JPQLExpressionBuilder.java @@ -34,6 +34,8 @@ import org.apache.commons.collections.map.LinkedMap; import org.apache.openjpa.kernel.ExpressionStoreQuery; import org.apache.openjpa.kernel.QueryContext; import org.apache.openjpa.kernel.QueryOperations; +import org.apache.openjpa.kernel.StoreContext; +import org.apache.openjpa.kernel.BrokerFactory; import org.apache.openjpa.kernel.exps.AbstractExpressionBuilder; import org.apache.openjpa.kernel.exps.Expression; import org.apache.openjpa.kernel.exps.ExpressionFactory; @@ -44,12 +46,15 @@ import org.apache.openjpa.kernel.exps.QueryExpressions; import org.apache.openjpa.kernel.exps.Subquery; import org.apache.openjpa.kernel.exps.Value; import org.apache.openjpa.lib.util.Localizer; +import org.apache.openjpa.lib.log.Log; import org.apache.openjpa.meta.ClassMetaData; import org.apache.openjpa.meta.FieldMetaData; import org.apache.openjpa.meta.MetaDataRepository; import org.apache.openjpa.meta.ValueMetaData; import org.apache.openjpa.util.InternalException; import org.apache.openjpa.util.UserException; +import org.apache.openjpa.conf.Compatibility; +import org.apache.openjpa.conf.OpenJPAConfiguration; import serp.util.Numbers; /** @@ -1078,12 +1083,15 @@ public class JPQLExpressionBuilder return factory.getCurrentTimestamp(); case JJTSELECTEXTENSION: + assertQueryExtensions("SELECT"); return eval(onlyChild(node)); case JJTGROUPBYEXTENSION: + assertQueryExtensions("GROUP BY"); return eval(onlyChild(node)); case JJTORDERBYEXTENSION: + assertQueryExtensions("ORDER BY"); return eval(onlyChild(node)); default: @@ -1092,6 +1100,39 @@ public class JPQLExpressionBuilder } } + private void assertQueryExtensions(String clause) { + OpenJPAConfiguration conf = resolver.getConfiguration(); + switch(conf.getCompatibilityInstance().getJPQL()) { + case Compatibility.JPQL_WARN: + // check if we've already warned for this query-factory combo + StoreContext ctx = resolver.getQueryContext().getStoreContext(); + String query = currentQuery(); + if (ctx.getBroker() != null && query != null) { + String key = getClass().getName() + ":" + query; + BrokerFactory factory = ctx.getBroker().getBrokerFactory(); + Object hasWarned = factory.getUserObject(key); + if (hasWarned != null) + break; + else + factory.putUserObject(key, Boolean.TRUE); + } + Log log = conf.getLog(OpenJPAConfiguration.LOG_QUERY); + if (log.isWarnEnabled()) + log.warn(_loc.get("query-extensions-warning", clause, + currentQuery())); + break; + case Compatibility.JPQL_STRICT: + throw new ParseException(_loc.get("query-extensions-error", + clause, currentQuery()).getMessage()); + case Compatibility.JPQL_EXTENDED: + break; + default: + throw new IllegalStateException( + "Compatibility.getJPQL() == " + + conf.getCompatibilityInstance().getJPQL()); + } + } + protected void setImplicitTypes(Value val1, Value val2, Class expected) { super.setImplicitTypes(val1, val2, expected); diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/jpql/ParseException.java b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/jpql/ParseException.java index ecb5997be..0943ab0b1 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/jpql/ParseException.java +++ b/openjpa-kernel/src/main/java/org/apache/openjpa/kernel/jpql/ParseException.java @@ -64,6 +64,18 @@ public class ParseException super(); } + /** + * String constructor. Constructing the exception in this + * manner makes the exception behave in the normal way - i.e., as + * documented in the class "Throwable". The fields "errorToken", + * "expectedTokenSequences", and "tokenImage" do not contain + * relevant information. The JavaCC generated code does not use + * these constructors. + */ + public ParseException(String message) { + super(message); + } + /** * This method has the standard behavior when this object has been * created using the standard constructors. Otherwise, it uses diff --git a/openjpa-kernel/src/main/jjtree/org/apache/openjpa/kernel/jpql/JPQL.jjt b/openjpa-kernel/src/main/jjtree/org/apache/openjpa/kernel/jpql/JPQL.jjt index 78c9253ce..2e6d530ee 100644 --- a/openjpa-kernel/src/main/jjtree/org/apache/openjpa/kernel/jpql/JPQL.jjt +++ b/openjpa-kernel/src/main/jjtree/org/apache/openjpa/kernel/jpql/JPQL.jjt @@ -64,7 +64,6 @@ import java.io.*; public class JPQL { String jpql; - boolean extensionsEnabled = true; public JPQL (String jpql) @@ -503,7 +502,7 @@ void select_expression() #SELECTEXPRESSION : { } } -void select_extension() #SELECTEXTENSION(extensionsEnabled) : { } +void select_extension() #SELECTEXTENSION : { } { scalar_function() } @@ -627,7 +626,7 @@ void groupby_item() : { } } -void groupby_extension() #GROUPBYEXTENSION(extensionsEnabled) : { } +void groupby_extension() #GROUPBYEXTENSION : { } { scalar_function() } @@ -1098,7 +1097,7 @@ void orderby_item() #ORDERBYITEM : { } } -void orderby_extension() #ORDERBYEXTENSION(extensionsEnabled) : { } +void orderby_extension() #ORDERBYEXTENSION : { } { aggregate_select_expression() } diff --git a/openjpa-kernel/src/main/resources/org/apache/openjpa/kernel/jpql/localizer.properties b/openjpa-kernel/src/main/resources/org/apache/openjpa/kernel/jpql/localizer.properties index 17faf1443..475f5e261 100644 --- a/openjpa-kernel/src/main/resources/org/apache/openjpa/kernel/jpql/localizer.properties +++ b/openjpa-kernel/src/main/resources/org/apache/openjpa/kernel/jpql/localizer.properties @@ -58,3 +58,11 @@ unknown-identifier: Undeclared identifier "{0}". update-constant-value: Update expression "{0}" may only use literals \ or parameters as update values. bad-parse: Encountered "{0}" at character {1}, but expected: {2}. +query-extensions-warning: This JPQL query uses non-standard OpenJPA \ + extensions in the {0} clause. JPQL string: "{1}". Query execution will \ + proceed. The openjpa.Compatibility configuration setting is configured to \ + log a warning the first time a given extended query is encountered. +query-extensions-error: This JPQL query uses non-standard OpenJPA \ + extensions in the {0} clause. JPQL string: "{1}". The \ + openjpa.Compatibility configuration setting is configured to disallow \ + JPQL extensions. diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/query/GroupingTestCase.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/query/GroupingTestCase.java index aa62d5393..988ea3a38 100644 --- a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/query/GroupingTestCase.java +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/query/GroupingTestCase.java @@ -38,7 +38,8 @@ public abstract class GroupingTestCase protected abstract void prepareQuery(Query q); public void setUp() { - super.setUp(AllFieldTypes.class, CLEAR_TABLES); + super.setUp(AllFieldTypes.class, CLEAR_TABLES, + "openjpa.Compatibility", "JPQL=warn"); AllFieldTypes pc1 = new AllFieldTypes(); AllFieldTypes pc2 = new AllFieldTypes(); diff --git a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/query/TestSubstring.java b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/query/TestSubstring.java index 82d884045..d643ae573 100644 --- a/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/query/TestSubstring.java +++ b/openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/query/TestSubstring.java @@ -25,7 +25,8 @@ import org.apache.openjpa.persistence.test.SingleEMTestCase; public class TestSubstring extends SingleEMTestCase { public void setUp() { - super.setUp(SimpleEntity.class, CLEAR_TABLES, "openjpa.Log", "SQL=TRACE"); + super.setUp(SimpleEntity.class, CLEAR_TABLES, + "openjpa.Compatibility", "JPQL=extended"); EntityManager em = emf.createEntityManager(); em.getTransaction().begin();