From 6beb5acb4bdcbe07e981a6017d69b3c4f71a4480 Mon Sep 17 00:00:00 2001 From: Steve Ebersole Date: Fri, 2 Aug 2013 19:17:31 -0500 Subject: [PATCH] HHH-8415 - Throw exception types expected by JPA spec wrt StoredProcedureQuery --- .../procedure/NoSuchParameterException.java | 35 +++++ .../procedure/ParameterStrategyException.java | 35 +++++ .../hibernate/procedure/ProcedureCall.java | 6 + .../hibernate/procedure/ProcedureOutputs.java | 6 + .../procedure/internal/ProcedureCallImpl.java | 23 ++-- .../internal/ProcedureOutputsImpl.java | 8 +- .../java/org/hibernate/result/Outputs.java | 19 +-- .../result/internal/OutputsImpl.java | 129 +++++++----------- .../result/internal/ResultSetOutputImpl.java | 63 +++++++++ .../internal/UpdateCountOutputImpl.java | 49 +++++++ .../sql/storedproc/StoredProcedureTest.java | 14 +- .../internal/StoredProcedureQueryImpl.java | 72 ++++++---- .../org/hibernate/jpa/spi/BaseQueryImpl.java | 22 ++- .../jpa/test/procedure/JpaTckUsageTest.java | 81 +++++++++++ 14 files changed, 425 insertions(+), 137 deletions(-) create mode 100644 hibernate-core/src/main/java/org/hibernate/procedure/NoSuchParameterException.java create mode 100644 hibernate-core/src/main/java/org/hibernate/procedure/ParameterStrategyException.java create mode 100644 hibernate-core/src/main/java/org/hibernate/result/internal/ResultSetOutputImpl.java create mode 100644 hibernate-core/src/main/java/org/hibernate/result/internal/UpdateCountOutputImpl.java diff --git a/hibernate-core/src/main/java/org/hibernate/procedure/NoSuchParameterException.java b/hibernate-core/src/main/java/org/hibernate/procedure/NoSuchParameterException.java new file mode 100644 index 0000000000..263bd60e54 --- /dev/null +++ b/hibernate-core/src/main/java/org/hibernate/procedure/NoSuchParameterException.java @@ -0,0 +1,35 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * Copyright (c) 2013, Red Hat Inc. or third-party contributors as + * indicated by the @author tags or express copyright attribution + * statements applied by the authors. All third-party contributions are + * distributed under license by Red Hat Inc. + * + * This copyrighted material is made available to anyone wishing to use, modify, + * copy, or redistribute it subject to the terms and conditions of the GNU + * Lesser General Public License, as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY + * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this distribution; if not, write to: + * Free Software Foundation, Inc. + * 51 Franklin Street, Fifth Floor + * Boston, MA 02110-1301 USA + */ +package org.hibernate.procedure; + +import org.hibernate.HibernateException; + +/** + * @author Steve Ebersole + */ +public class NoSuchParameterException extends HibernateException { + public NoSuchParameterException(String message) { + super( message ); + } +} diff --git a/hibernate-core/src/main/java/org/hibernate/procedure/ParameterStrategyException.java b/hibernate-core/src/main/java/org/hibernate/procedure/ParameterStrategyException.java new file mode 100644 index 0000000000..e6dbafcab1 --- /dev/null +++ b/hibernate-core/src/main/java/org/hibernate/procedure/ParameterStrategyException.java @@ -0,0 +1,35 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * Copyright (c) 2013, Red Hat Inc. or third-party contributors as + * indicated by the @author tags or express copyright attribution + * statements applied by the authors. All third-party contributions are + * distributed under license by Red Hat Inc. + * + * This copyrighted material is made available to anyone wishing to use, modify, + * copy, or redistribute it subject to the terms and conditions of the GNU + * Lesser General Public License, as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY + * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this distribution; if not, write to: + * Free Software Foundation, Inc. + * 51 Franklin Street, Fifth Floor + * Boston, MA 02110-1301 USA + */ +package org.hibernate.procedure; + +import org.hibernate.HibernateException; + +/** + * @author Steve Ebersole + */ +public class ParameterStrategyException extends HibernateException { + public ParameterStrategyException(String message) { + super( message ); + } +} diff --git a/hibernate-core/src/main/java/org/hibernate/procedure/ProcedureCall.java b/hibernate-core/src/main/java/org/hibernate/procedure/ProcedureCall.java index 7ecd45d545..e4c9206cbb 100644 --- a/hibernate-core/src/main/java/org/hibernate/procedure/ProcedureCall.java +++ b/hibernate-core/src/main/java/org/hibernate/procedure/ProcedureCall.java @@ -82,6 +82,9 @@ public interface ProcedureCall extends BasicQueryContract, SynchronizeableQuery * @param position The parameter position * * @return The parameter registration memento + * + * @throws ParameterStrategyException If the ProcedureCall is defined using named parameters + * @throws NoSuchParameterException If no parameter with that position exists */ public ParameterRegistration getParameterRegistration(int position); @@ -122,6 +125,9 @@ public interface ProcedureCall extends BasicQueryContract, SynchronizeableQuery * @param name The parameter name * * @return The parameter registration memento + * + * @throws ParameterStrategyException If the ProcedureCall is defined using positional parameters + * @throws NoSuchParameterException If no parameter with that name exists */ public ParameterRegistration getParameterRegistration(String name); diff --git a/hibernate-core/src/main/java/org/hibernate/procedure/ProcedureOutputs.java b/hibernate-core/src/main/java/org/hibernate/procedure/ProcedureOutputs.java index 95e60183f5..442b33b6dc 100644 --- a/hibernate-core/src/main/java/org/hibernate/procedure/ProcedureOutputs.java +++ b/hibernate-core/src/main/java/org/hibernate/procedure/ProcedureOutputs.java @@ -53,6 +53,9 @@ public interface ProcedureOutputs extends Outputs { * * @return The output value. * + * @throws ParameterStrategyException If the ProcedureCall is defined using positional parameters + * @throws NoSuchParameterException If no parameter with that name exists + * * @see ProcedureCall#registerParameter(String, Class, javax.persistence.ParameterMode) */ public Object getOutputParameterValue(String name); @@ -64,6 +67,9 @@ public interface ProcedureOutputs extends Outputs { * * @return The output value. * + * @throws ParameterStrategyException If the ProcedureCall is defined using named parameters + * @throws NoSuchParameterException If no parameter with that position exists + * * @see ProcedureCall#registerParameter(int, Class, javax.persistence.ParameterMode) */ public Object getOutputParameterValue(int position); diff --git a/hibernate-core/src/main/java/org/hibernate/procedure/internal/ProcedureCallImpl.java b/hibernate-core/src/main/java/org/hibernate/procedure/internal/ProcedureCallImpl.java index c8db3430fc..b6639043c9 100644 --- a/hibernate-core/src/main/java/org/hibernate/procedure/internal/ProcedureCallImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/procedure/internal/ProcedureCallImpl.java @@ -50,7 +50,9 @@ import org.hibernate.internal.util.StringHelper; import org.hibernate.internal.util.collections.CollectionHelper; import org.hibernate.persister.entity.EntityPersister; import org.hibernate.procedure.NamedParametersNotSupportedException; +import org.hibernate.procedure.NoSuchParameterException; import org.hibernate.procedure.ParameterRegistration; +import org.hibernate.procedure.ParameterStrategyException; import org.hibernate.procedure.ProcedureCall; import org.hibernate.procedure.ProcedureCallMemento; import org.hibernate.procedure.ProcedureOutputs; @@ -321,21 +323,22 @@ public class ProcedureCallImpl extends AbstractBasicQueryContractImpl implements @Override public ParameterRegistrationImplementor getParameterRegistration(int position) { if ( parameterStrategy != ParameterStrategy.POSITIONAL ) { - throw new IllegalArgumentException( "Positions were not used to register parameters with this stored procedure call" ); + throw new ParameterStrategyException( + "Attempt to access positional parameter [" + position + "] but ProcedureCall using named parameters" + ); } - try { - return registeredParameters.get( position ); - } - catch ( Exception e ) { - throw new QueryException( "Could not locate parameter registered using that position [" + position + "]" ); + for ( ParameterRegistrationImplementor parameter : registeredParameters ) { + if ( position == parameter.getPosition() ) { + return parameter; + } } + throw new NoSuchParameterException( "Could not locate parameter registered using that position [" + position + "]" ); } @Override @SuppressWarnings("unchecked") public ParameterRegistration registerParameter(String name, Class type, ParameterMode mode) { - final NamedParameterRegistration parameterRegistration - = new NamedParameterRegistration( this, name, mode, type ); + final NamedParameterRegistration parameterRegistration = new NamedParameterRegistration( this, name, mode, type ); registerParameter( parameterRegistration ); return parameterRegistration; } @@ -350,14 +353,14 @@ public class ProcedureCallImpl extends AbstractBasicQueryContractImpl implements @Override public ParameterRegistrationImplementor getParameterRegistration(String name) { if ( parameterStrategy != ParameterStrategy.NAMED ) { - throw new IllegalArgumentException( "Names were not used to register parameters with this stored procedure call" ); + throw new ParameterStrategyException( "Names were not used to register parameters with this stored procedure call" ); } for ( ParameterRegistrationImplementor parameter : registeredParameters ) { if ( name.equals( parameter.getName() ) ) { return parameter; } } - throw new IllegalArgumentException( "Could not locate parameter registered under that name [" + name + "]" ); + throw new NoSuchParameterException( "Could not locate parameter registered under that name [" + name + "]" ); } @Override diff --git a/hibernate-core/src/main/java/org/hibernate/procedure/internal/ProcedureOutputsImpl.java b/hibernate-core/src/main/java/org/hibernate/procedure/internal/ProcedureOutputsImpl.java index 701f205901..bade61376d 100644 --- a/hibernate-core/src/main/java/org/hibernate/procedure/internal/ProcedureOutputsImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/procedure/internal/ProcedureOutputsImpl.java @@ -68,7 +68,7 @@ public class ProcedureOutputsImpl extends OutputsImpl implements ProcedureOutput } @Override - protected CurrentReturnState buildCurrentReturnDescriptor(boolean isResultSet, int updateCount) { + protected CurrentReturnState buildCurrentReturnState(boolean isResultSet, int updateCount) { return new ProcedureCurrentReturnState( isResultSet, updateCount, refCursorParamIndex ); } @@ -81,8 +81,8 @@ public class ProcedureOutputsImpl extends OutputsImpl implements ProcedureOutput } @Override - public boolean indicatesMoreReturns() { - return super.indicatesMoreReturns() + public boolean indicatesMoreOutputs() { + return super.indicatesMoreOutputs() || ProcedureOutputsImpl.this.refCursorParamIndex < ProcedureOutputsImpl.this.refCursorParameters.length; } @@ -106,7 +106,7 @@ public class ProcedureOutputsImpl extends OutputsImpl implements ProcedureOutput .getService( RefCursorSupport.class ) .getResultSet( ProcedureOutputsImpl.this.callableStatement, refCursorParam.getPosition() ); } - return new ResultSetOutputImpl( extractResults( resultSet ) ); + return buildResultSetOutput( extractResults( resultSet ) ); } } diff --git a/hibernate-core/src/main/java/org/hibernate/result/Outputs.java b/hibernate-core/src/main/java/org/hibernate/result/Outputs.java index 8016eb8dfd..90830161bf 100644 --- a/hibernate-core/src/main/java/org/hibernate/result/Outputs.java +++ b/hibernate-core/src/main/java/org/hibernate/result/Outputs.java @@ -38,23 +38,18 @@ public interface Outputs { * * @return The current Output object. Can be {@code null} */ - public Output getCurrentOutput(); + public Output getCurrent(); /** - * Are there any more Output objects associated with {@code this}? + * Go to the next Output object (if any), returning an indication of whether there was another (aka, will + * the next call to {@link #getCurrent()} return {@code null}? * - * @return {@code true} means there are more Output objects available via {@link #getNextOutput()}; {@code false} - * indicates that calling {@link #getNextOutput()} will certainly result in an exception. + * @return {@code true} if the next call to {@link #getCurrent()} will return a non-{@code null} value. */ - public boolean hasMoreOutput(); + public boolean goToNext(); /** - * Retrieve the next return - * - * @return The next return. - * - * @throws NoMoreReturnsException Thrown if there are no more returns associated with this Result, as would - * have been indicated by a {@code false} return from {@link #hasMoreOutput()}. + * Eagerly release any resources held by this Outputs. */ - public Output getNextOutput() throws NoMoreReturnsException; + public void release(); } diff --git a/hibernate-core/src/main/java/org/hibernate/result/internal/OutputsImpl.java b/hibernate-core/src/main/java/org/hibernate/result/internal/OutputsImpl.java index 721b215004..8320c2a1ed 100644 --- a/hibernate-core/src/main/java/org/hibernate/result/internal/OutputsImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/result/internal/OutputsImpl.java @@ -43,9 +43,7 @@ import org.hibernate.loader.custom.sql.SQLQueryReturnProcessor; import org.hibernate.loader.spi.AfterLoadAction; import org.hibernate.result.NoMoreReturnsException; import org.hibernate.result.Outputs; -import org.hibernate.result.ResultSetOutput; import org.hibernate.result.Output; -import org.hibernate.result.UpdateCountOutput; import org.hibernate.result.spi.ResultContext; /** @@ -69,14 +67,14 @@ public class OutputsImpl implements Outputs { try { final boolean isResultSet = jdbcStatement.execute(); - currentReturnState = buildCurrentReturnDescriptor( isResultSet ); + currentReturnState = buildCurrentReturnState( isResultSet ); } catch (SQLException e) { throw convert( e, "Error calling CallableStatement.getMoreResults" ); } } - private CurrentReturnState buildCurrentReturnDescriptor(boolean isResultSet) { + private CurrentReturnState buildCurrentReturnState(boolean isResultSet) { int updateCount = -1; if ( ! isResultSet ) { try { @@ -87,42 +85,57 @@ public class OutputsImpl implements Outputs { } } - return buildCurrentReturnDescriptor( isResultSet, updateCount ); + return buildCurrentReturnState( isResultSet, updateCount ); } - protected CurrentReturnState buildCurrentReturnDescriptor(boolean isResultSet, int updateCount) { + protected CurrentReturnState buildCurrentReturnState(boolean isResultSet, int updateCount) { return new CurrentReturnState( isResultSet, updateCount ); } - @Override - public Output getCurrentOutput() { - if ( currentReturnState == null ) { - return null; - } - return currentReturnState.getReturn(); + protected JDBCException convert(SQLException e, String message) { + return context.getSession().getFactory().getSQLExceptionHelper().convert( + e, + message, + context.getSql() + ); } @Override - public boolean hasMoreOutput() { + public Output getCurrent() { + if ( currentReturnState == null ) { + return null; + } + return currentReturnState.getOutput(); + } + + @Override + public boolean goToNext() { + if ( currentReturnState == null ) { + return false; + } + + if ( currentReturnState.indicatesMoreOutputs() ) // prepare the next return state try { final boolean isResultSet = jdbcStatement.getMoreResults(); - currentReturnState = buildCurrentReturnDescriptor( isResultSet ); + currentReturnState = buildCurrentReturnState( isResultSet ); } catch (SQLException e) { throw convert( e, "Error calling CallableStatement.getMoreResults" ); } - return currentReturnState != null && currentReturnState.indicatesMoreReturns(); + // and return + return currentReturnState != null && currentReturnState.indicatesMoreOutputs(); } @Override - public Output getNextOutput() { - if ( !hasMoreOutput() ) { - throw new NoMoreReturnsException( "Results have been exhausted" ); + public void release() { + try { + jdbcStatement.close(); + } + catch (SQLException e) { + log.debug( "Unable to close PreparedStatement", e ); } - - return getCurrentOutput(); } private List extractCurrentResults() { @@ -143,14 +156,6 @@ public class OutputsImpl implements Outputs { } } - protected JDBCException convert(SQLException e, String message) { - return context.getSession().getFactory().getSQLExceptionHelper().convert( - e, - message, - context.getSql() - ); - } - /** * Encapsulates the information needed to interpret the current return within a result */ @@ -165,7 +170,7 @@ public class OutputsImpl implements Outputs { this.updateCount = updateCount; } - public boolean indicatesMoreReturns() { + public boolean indicatesMoreOutputs() { return isResultSet() || getUpdateCount() >= 0; } @@ -177,14 +182,14 @@ public class OutputsImpl implements Outputs { return updateCount; } - public Output getReturn() { + public Output getOutput() { if ( rtn == null ) { - rtn = buildReturn(); + rtn = buildOutput(); } return rtn; } - protected Output buildReturn() { + protected Output buildOutput() { if ( log.isDebugEnabled() ) { log.debugf( "Building Return [isResultSet=%s, updateCount=%s, extendedReturn=%s", @@ -204,10 +209,10 @@ public class OutputsImpl implements Outputs { ); if ( isResultSet() ) { - return new ResultSetOutputImpl( extractCurrentResults() ); + return buildResultSetOutput( extractCurrentResults() ); } else if ( getUpdateCount() >= 0 ) { - return new UpdateCountOutputImpl( updateCount ); + return buildUpdateCountOutput( updateCount ); } else if ( hasExtendedReturns() ) { return buildExtendedReturn(); @@ -218,6 +223,14 @@ public class OutputsImpl implements Outputs { // hooks for stored procedure (out param) processing ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + protected Output buildResultSetOutput(List list) { + return new ResultSetOutputImpl( list ); + } + + protected Output buildUpdateCountOutput(int updateCount) { + return new UpdateCountOutputImpl( updateCount ); + } + protected boolean hasExtendedReturns() { return false; } @@ -227,53 +240,9 @@ public class OutputsImpl implements Outputs { } } - protected static class ResultSetOutputImpl implements ResultSetOutput { - private final List results; - public ResultSetOutputImpl(List results) { - this.results = results; - } - - @Override - public boolean isResultSet() { - return true; - } - - @Override - @SuppressWarnings("unchecked") - public List getResultList() { - return results; - } - - @Override - public Object getSingleResult() { - final List results = getResultList(); - if ( results == null || results.isEmpty() ) { - return null; - } - else { - return results.get( 0 ); - } - } - } - - protected static class UpdateCountOutputImpl implements UpdateCountOutput { - private final int updateCount; - - public UpdateCountOutputImpl(int updateCount) { - this.updateCount = updateCount; - } - - @Override - public int getUpdateCount() { - return updateCount; - } - - @Override - public boolean isResultSet() { - return false; - } - } + // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + // Hooks into Hibernate's Loader hierarchy for ResultSet -> Object mapping private static CustomLoaderExtension buildSpecializedCustomLoader(final ResultContext context) { // might be better to just manually construct the Return(s).. SQLQueryReturnProcessor does a lot of diff --git a/hibernate-core/src/main/java/org/hibernate/result/internal/ResultSetOutputImpl.java b/hibernate-core/src/main/java/org/hibernate/result/internal/ResultSetOutputImpl.java new file mode 100644 index 0000000000..c4c1741d0e --- /dev/null +++ b/hibernate-core/src/main/java/org/hibernate/result/internal/ResultSetOutputImpl.java @@ -0,0 +1,63 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * Copyright (c) 2013, Red Hat Inc. or third-party contributors as + * indicated by the @author tags or express copyright attribution + * statements applied by the authors. All third-party contributions are + * distributed under license by Red Hat Inc. + * + * This copyrighted material is made available to anyone wishing to use, modify, + * copy, or redistribute it subject to the terms and conditions of the GNU + * Lesser General Public License, as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY + * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this distribution; if not, write to: + * Free Software Foundation, Inc. + * 51 Franklin Street, Fifth Floor + * Boston, MA 02110-1301 USA + */ +package org.hibernate.result.internal; + +import java.util.List; + +import org.hibernate.result.ResultSetOutput; + +/** + * Implementation of ResultSetOutput + * + * @author Steve Ebersole + */ +class ResultSetOutputImpl implements ResultSetOutput { + private final List results; + + public ResultSetOutputImpl(List results) { + this.results = results; + } + + @Override + public boolean isResultSet() { + return true; + } + + @Override + @SuppressWarnings("unchecked") + public List getResultList() { + return results; + } + + @Override + public Object getSingleResult() { + final List results = getResultList(); + if ( results == null || results.isEmpty() ) { + return null; + } + else { + return results.get( 0 ); + } + } +} diff --git a/hibernate-core/src/main/java/org/hibernate/result/internal/UpdateCountOutputImpl.java b/hibernate-core/src/main/java/org/hibernate/result/internal/UpdateCountOutputImpl.java new file mode 100644 index 0000000000..842437d778 --- /dev/null +++ b/hibernate-core/src/main/java/org/hibernate/result/internal/UpdateCountOutputImpl.java @@ -0,0 +1,49 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * Copyright (c) 2013, Red Hat Inc. or third-party contributors as + * indicated by the @author tags or express copyright attribution + * statements applied by the authors. All third-party contributions are + * distributed under license by Red Hat Inc. + * + * This copyrighted material is made available to anyone wishing to use, modify, + * copy, or redistribute it subject to the terms and conditions of the GNU + * Lesser General Public License, as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY + * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this distribution; if not, write to: + * Free Software Foundation, Inc. + * 51 Franklin Street, Fifth Floor + * Boston, MA 02110-1301 USA + */ +package org.hibernate.result.internal; + +import org.hibernate.result.UpdateCountOutput; + +/** + * Implementation of UpdateCountOutput + * + * @author Steve Ebersole + */ +class UpdateCountOutputImpl implements UpdateCountOutput { + private final int updateCount; + + public UpdateCountOutputImpl(int updateCount) { + this.updateCount = updateCount; + } + + @Override + public int getUpdateCount() { + return updateCount; + } + + @Override + public boolean isResultSet() { + return false; + } +} diff --git a/hibernate-core/src/test/java/org/hibernate/test/sql/storedproc/StoredProcedureTest.java b/hibernate-core/src/test/java/org/hibernate/test/sql/storedproc/StoredProcedureTest.java index de451b842e..cc3032398a 100644 --- a/hibernate-core/src/test/java/org/hibernate/test/sql/storedproc/StoredProcedureTest.java +++ b/hibernate-core/src/test/java/org/hibernate/test/sql/storedproc/StoredProcedureTest.java @@ -169,9 +169,9 @@ public class StoredProcedureTest extends BaseCoreFunctionalTestCase { Session session = openSession(); session.beginTransaction(); - ProcedureCall query = session.createStoredProcedureCall( "user"); - ProcedureOutputs procedureResult = query.getResult(); - Output currentOutput = procedureResult.getCurrentOutput(); + ProcedureCall procedureCall = session.createStoredProcedureCall( "user"); + ProcedureOutputs procedureOutputs = procedureCall.getResult(); + Output currentOutput = procedureOutputs.getCurrent(); assertNotNull( currentOutput ); ResultSetOutput resultSetReturn = assertTyping( ResultSetOutput.class, currentOutput ); String name = (String) resultSetReturn.getSingleResult(); @@ -188,7 +188,7 @@ public class StoredProcedureTest extends BaseCoreFunctionalTestCase { ProcedureCall query = session.createStoredProcedureCall( "findOneUser" ); ProcedureOutputs procedureResult = query.getResult(); - Output currentOutput = procedureResult.getCurrentOutput(); + Output currentOutput = procedureResult.getCurrent(); assertNotNull( currentOutput ); ResultSetOutput resultSetReturn = assertTyping( ResultSetOutput.class, currentOutput ); Object result = resultSetReturn.getSingleResult(); @@ -207,7 +207,7 @@ public class StoredProcedureTest extends BaseCoreFunctionalTestCase { ProcedureCall query = session.createStoredProcedureCall( "findUsers" ); ProcedureOutputs procedureResult = query.getResult(); - Output currentOutput = procedureResult.getCurrentOutput(); + Output currentOutput = procedureResult.getCurrent(); assertNotNull( currentOutput ); ResultSetOutput resultSetReturn = assertTyping( ResultSetOutput.class, currentOutput ); List results = resultSetReturn.getResultList(); @@ -244,7 +244,7 @@ public class StoredProcedureTest extends BaseCoreFunctionalTestCase { query.registerParameter( "start", Integer.class, ParameterMode.IN ).bindValue( 1 ); query.registerParameter( "end", Integer.class, ParameterMode.IN ).bindValue( 2 ); ProcedureOutputs procedureResult = query.getResult(); - Output currentOutput = procedureResult.getCurrentOutput(); + Output currentOutput = procedureResult.getCurrent(); assertNotNull( currentOutput ); ResultSetOutput resultSetReturn = assertTyping( ResultSetOutput.class, currentOutput ); List results = resultSetReturn.getResultList(); @@ -269,7 +269,7 @@ public class StoredProcedureTest extends BaseCoreFunctionalTestCase { query.registerParameter( 1, Integer.class, ParameterMode.IN ).bindValue( 1 ); query.registerParameter( 2, Integer.class, ParameterMode.IN ).bindValue( 2 ); ProcedureOutputs procedureResult = query.getResult(); - Output currentOutput = procedureResult.getCurrentOutput(); + Output currentOutput = procedureResult.getCurrent(); assertNotNull( currentOutput ); ResultSetOutput resultSetReturn = assertTyping( ResultSetOutput.class, currentOutput ); List results = resultSetReturn.getResultList(); diff --git a/hibernate-entitymanager/src/main/java/org/hibernate/jpa/internal/StoredProcedureQueryImpl.java b/hibernate-entitymanager/src/main/java/org/hibernate/jpa/internal/StoredProcedureQueryImpl.java index 94a5fee442..6ddb0bdf4b 100644 --- a/hibernate-entitymanager/src/main/java/org/hibernate/jpa/internal/StoredProcedureQueryImpl.java +++ b/hibernate-entitymanager/src/main/java/org/hibernate/jpa/internal/StoredProcedureQueryImpl.java @@ -41,6 +41,8 @@ import java.util.List; import org.hibernate.CacheMode; import org.hibernate.FlushMode; import org.hibernate.LockMode; +import org.hibernate.procedure.NoSuchParameterException; +import org.hibernate.procedure.ParameterStrategyException; import org.hibernate.procedure.ProcedureCall; import org.hibernate.procedure.ProcedureCallMemento; import org.hibernate.procedure.ProcedureOutputs; @@ -199,27 +201,10 @@ public class StoredProcedureQueryImpl extends BaseQueryImpl implements StoredPro // outputs ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - private ProcedureOutputs outputs() { - if ( procedureResult == null ) { - procedureResult = procedureCall.getResult(); - } - return procedureResult; - } - - @Override - public Object getOutputParameterValue(int position) { - return outputs().getOutputParameterValue( position ); - } - - @Override - public Object getOutputParameterValue(String parameterName) { - return outputs().getOutputParameterValue( parameterName ); - } - @Override public boolean execute() { try { - final Output rtn = outputs().getCurrentOutput(); + final Output rtn = outputs().getCurrent(); return rtn != null && ResultSetOutput.class.isInstance( rtn ); } catch (NoMoreReturnsException e) { @@ -227,23 +212,64 @@ public class StoredProcedureQueryImpl extends BaseQueryImpl implements StoredPro } } + protected ProcedureOutputs outputs() { + if ( procedureResult == null ) { + procedureResult = procedureCall.getResult(); + } + return procedureResult; + } + @Override public int executeUpdate() { if ( ! entityManager().isTransactionInProgress() ) { throw new TransactionRequiredException( "javax.persistence.Query.executeUpdate requires active transaction" ); } - return getUpdateCount(); + + // the expectation is that there is just one Output, of type UpdateCountOutput + try { + execute(); + return getUpdateCount(); + } + finally { + outputs().release(); + } + } + + @Override + public Object getOutputParameterValue(int position) { + try { + return outputs().getOutputParameterValue( position ); + } + catch (ParameterStrategyException e) { + throw new IllegalArgumentException( "Invalid mix of named and positional parameters", e ); + } + catch (NoSuchParameterException e) { + throw new IllegalArgumentException( e.getMessage(), e ); + } + } + + @Override + public Object getOutputParameterValue(String parameterName) { + try { + return outputs().getOutputParameterValue( parameterName ); + } + catch (ParameterStrategyException e) { + throw new IllegalArgumentException( "Invalid mix of named and positional parameters", e ); + } + catch (NoSuchParameterException e) { + throw new IllegalArgumentException( e.getMessage(), e ); + } } @Override public boolean hasMoreResults() { - return outputs().hasMoreOutput() && ResultSetOutput.class.isInstance( outputs().getCurrentOutput() ); + return outputs().goToNext() && ResultSetOutput.class.isInstance( outputs().getCurrent() ); } @Override public int getUpdateCount() { try { - final Output rtn = outputs().getCurrentOutput(); + final Output rtn = outputs().getCurrent(); if ( rtn == null ) { return -1; } @@ -262,9 +288,9 @@ public class StoredProcedureQueryImpl extends BaseQueryImpl implements StoredPro @Override public List getResultList() { try { - final Output rtn = outputs().getCurrentOutput(); + final Output rtn = outputs().getCurrent(); if ( ! ResultSetOutput.class.isInstance( rtn ) ) { - throw new IllegalStateException( "Current CallableStatement return was not a ResultSet, but getResultList was called" ); + throw new IllegalStateException( "Current CallableStatement ou was not a ResultSet, but getResultList was called" ); } return ( (ResultSetOutput) rtn ).getResultList(); diff --git a/hibernate-entitymanager/src/main/java/org/hibernate/jpa/spi/BaseQueryImpl.java b/hibernate-entitymanager/src/main/java/org/hibernate/jpa/spi/BaseQueryImpl.java index ad239f8c67..9424aa265f 100644 --- a/hibernate-entitymanager/src/main/java/org/hibernate/jpa/spi/BaseQueryImpl.java +++ b/hibernate-entitymanager/src/main/java/org/hibernate/jpa/spi/BaseQueryImpl.java @@ -51,6 +51,8 @@ import org.hibernate.jpa.internal.EntityManagerMessageLogger; import org.hibernate.jpa.internal.util.CacheModeHelper; import org.hibernate.jpa.internal.util.ConfigurationHelper; import org.hibernate.jpa.internal.util.LockModeTypeHelper; +import org.hibernate.procedure.NoSuchParameterException; +import org.hibernate.procedure.ParameterStrategyException; import static org.hibernate.jpa.QueryHints.HINT_CACHEABLE; import static org.hibernate.jpa.QueryHints.HINT_CACHE_MODE; @@ -439,7 +441,8 @@ public abstract class BaseQueryImpl implements Query { protected abstract boolean isJpaPositionalParameter(int position); /** - * Hibernate specific extension to the JPA {@link javax.persistence.Parameter} contract. + * Hibernate specific extension to the JPA {@link javax.persistence.Parameter} contract. Used here to track + * information known about the parameter. */ protected static interface ParameterRegistration extends Parameter { /** @@ -450,6 +453,12 @@ public abstract class BaseQueryImpl implements Query { */ public ParameterMode getMode(); + /** + * Can we bind (set) values on this parameter? Generally this is {@code true}, but would not be in the case + * of parameters with OUT or REF_CURSOR mode. + * + * @return Whether the parameter is bindable (can set be called). + */ public boolean isBindable(); public void bindValue(T value); @@ -459,6 +468,11 @@ public abstract class BaseQueryImpl implements Query { public ParameterBind getBind(); } + /** + * Represents the value currently bound to a particular parameter. + * + * @param + */ protected static interface ParameterBind { public T getValue(); @@ -648,6 +662,12 @@ public abstract class BaseQueryImpl implements Query { try { findParameterRegistration( position ).bindValue( value, temporalType ); } + catch (ParameterStrategyException e) { + throw new IllegalArgumentException( "Invalid mix of named and positional parameters", e ); + } + catch (NoSuchParameterException e) { + throw new IllegalArgumentException( e.getMessage(), e ); + } catch (QueryParameterException e) { throw new IllegalArgumentException( e ); } diff --git a/hibernate-entitymanager/src/test/java/org/hibernate/jpa/test/procedure/JpaTckUsageTest.java b/hibernate-entitymanager/src/test/java/org/hibernate/jpa/test/procedure/JpaTckUsageTest.java index a834be7717..e734c79887 100644 --- a/hibernate-entitymanager/src/test/java/org/hibernate/jpa/test/procedure/JpaTckUsageTest.java +++ b/hibernate-entitymanager/src/test/java/org/hibernate/jpa/test/procedure/JpaTckUsageTest.java @@ -54,7 +54,9 @@ import org.hibernate.testing.junit4.BaseUnitTestCase; import static org.hibernate.testing.junit4.ExtraAssertions.assertTyping; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; /** * Tests various JPA usage scenarios for performing stored procedures. Inspired by the awesomely well-done JPA TCK @@ -107,6 +109,42 @@ public class JpaTckUsageTest extends BaseUnitTestCase { } } + @Test + @FailureExpected( jiraKey = "HHH-8416", message = "JPA TCK challenge" ) + public void testHasMoreResultsHandlingTckChallenge() { + EntityManager em = entityManagerFactory.createEntityManager(); + em.getTransaction().begin(); + + try { + StoredProcedureQuery query = em.createStoredProcedureQuery( "findOneUser", User.class ); + assertTrue( query.execute() ); + assertTrue( query.hasMoreResults() ); + query.getResultList(); + assertFalse( query.hasMoreResults() ); + } + finally { + em.getTransaction().commit(); + em.close(); + } + } + + @Test + public void testHasMoreResultsHandling() { + EntityManager em = entityManagerFactory.createEntityManager(); + em.getTransaction().begin(); + + try { + StoredProcedureQuery query = em.createStoredProcedureQuery( "findOneUser", User.class ); + assertTrue( query.execute() ); + query.getResultList(); + assertFalse( query.hasMoreResults() ); + } + finally { + em.getTransaction().commit(); + em.close(); + } + } + @Test public void testResultClassHandling() { EntityManager em = entityManagerFactory.createEntityManager(); @@ -148,6 +186,38 @@ public class JpaTckUsageTest extends BaseUnitTestCase { } } + @Test + public void testSettingNonExistingParams() { + EntityManager em = entityManagerFactory.createEntityManager(); + em.getTransaction().begin(); + + try { + // non-existing positional param + try { + StoredProcedureQuery query = em.createNamedStoredProcedureQuery( "positional-param" ); + query.setParameter( 99, 1 ); + fail( "Expecting an exception" ); + } + catch (IllegalArgumentException expected) { + // this is the expected condition + } + + // non-existing named param + try { + StoredProcedureQuery query = em.createNamedStoredProcedureQuery( "positional-param" ); + query.setParameter( "does-not-exist", 1 ); + fail( "Expecting an exception" ); + } + catch (IllegalArgumentException expected) { + // this is the expected condition + } + } + finally { + em.getTransaction().commit(); + em.close(); + } + } + @Test @FailureExpected( jiraKey = "HHH-8395", message = "Out of the frying pan into the fire: https://issues.apache.org/jira/browse/DERBY-211" ) public void testExecuteUpdate() { @@ -167,6 +237,10 @@ public class JpaTckUsageTest extends BaseUnitTestCase { } } + public void testParameterRegistration() { + + } + // todo : look at ways to allow "Auxiliary DB Objects" to the db via EMF bootstrapping. // public static final String findOneUser_CREATE_CMD = "CREATE ALIAS findOneUser AS $$\n" + @@ -317,6 +391,13 @@ public class JpaTckUsageTest extends BaseUnitTestCase { conn.close(); } + public static void findUserIds(ResultSet[] results) throws SQLException { + Connection conn = DriverManager.getConnection( "jdbc:default:connection" ); + PreparedStatement ps = conn.prepareStatement( "select id from t_user" ); + results[0] = ps.executeQuery(); + conn.close(); + } + public static void deleteAllUsers() throws SQLException { // afaict the only way to return update counts here is to actually perform some DML Connection conn = DriverManager.getConnection( "jdbc:default:connection" );