From 145061d2912c33a61b503dcb3286342b25bcb5a1 Mon Sep 17 00:00:00 2001 From: Mark Rotteveel Date: Sat, 28 Oct 2023 11:07:14 +0200 Subject: [PATCH] #774 Remove FBProcedureCall "new" behaviour which is never used --- src/docs/asciidoc/release_notes.adoc | 6 ++ .../org/firebirdsql/jdbc/FBProcedureCall.java | 57 ++++--------------- .../jdbc/escape/FBEscapedCallParserTest.java | 25 ++++++-- 3 files changed, 36 insertions(+), 52 deletions(-) diff --git a/src/docs/asciidoc/release_notes.adoc b/src/docs/asciidoc/release_notes.adoc index a60da07b0..3f963f721 100644 --- a/src/docs/asciidoc/release_notes.adoc +++ b/src/docs/asciidoc/release_notes.adoc @@ -1105,6 +1105,9 @@ there is no replacement ** `findOutParameter(String)` (protected); use `getAndAssertSingletonResultSet().findColumn(paramName)`; carefully check if that is the correct usage (the method was removed because the old usage within Jaybird resulted in mapping the wrong column) +* `FBProcedureCall` +** `mapOutParamIndexToPosition(int, boolean)`; +use `mapOutParamIndexToPosition(int)` (equivalent to passing `true`), there is no replacement for the `false` behaviour The following methods had their visibility reduced: @@ -1174,6 +1177,9 @@ there is no replacement there is no replacement ** `SQL_STATE_COMM_LINK_FAILURE` (`08S01`) -- it was unused; there is no replacement +* `FBProcedureCall` +** `OLD_CALLABLE_STATEMENT_COMPATIBILITY`; +there is no replacement [#removal-of-deprecated-classes-packages-and-methods] === Removal of deprecated classes, packages and methods diff --git a/src/main/org/firebirdsql/jdbc/FBProcedureCall.java b/src/main/org/firebirdsql/jdbc/FBProcedureCall.java index ee822c0ff..b77f5d1dd 100644 --- a/src/main/org/firebirdsql/jdbc/FBProcedureCall.java +++ b/src/main/org/firebirdsql/jdbc/FBProcedureCall.java @@ -54,12 +54,6 @@ private static Vector cloneParameters(final Vectortrue if the old callable statement compatibility mode should - * be used, otherwise - false. Current value - true. - */ - public static final boolean OLD_CALLABLE_STATEMENT_COMPATIBILITY = true; - private String name; // TODO Replace Vector with a List private Vector inputParams = new Vector<>(); @@ -137,58 +131,29 @@ private FBProcedureParam getParam(Collection params, int index } /** - * Map output parameter index to a column number of corresponding result - * set. - * - * @param index index to map. + * Map output parameter index to a column number of the corresponding result set. * - * @return mapped column number or index if no output parameter - * with the specified index found (assuming that {@link #OLD_CALLABLE_STATEMENT_COMPATIBILITY} - * constant is set to true, otherwise throws exception). - * - * @throws SQLException if compatibility mode is switched off and no - * parameter was found (see {@link #OLD_CALLABLE_STATEMENT_COMPATIBILITY} - * constant). + * @param index + * index to map + * @return mapped column number or {@code index} if no output parameter with the specified index is found + * @throws SQLException + * in current implementation: never, throws clause retained for compatibility and possibly future uses */ + @SuppressWarnings("RedundantThrows") public int mapOutParamIndexToPosition(int index) throws SQLException { - return mapOutParamIndexToPosition(index, OLD_CALLABLE_STATEMENT_COMPATIBILITY); - } - - /** - * Map output parameter index to a column number of corresponding result - * set. - * - * @param index index to map. - * @param compatibilityMode true if we should run in old compatibility mode. - * - * @return mapped column number or index if no output parameter - * with the specified index found and compatibilityMode is set. - * - * @throws SQLException if compatibility mode is switched off and no - * parameter was found. - */ - public int mapOutParamIndexToPosition(int index, boolean compatibilityMode) throws SQLException { - int position = -1; + int position = 0; for (FBProcedureParam param : outputParams) { if (param != null && param.isParam()) { position++; if (param.getIndex() == index) { - return position + 1; + return position; } } } - - // hack: if we did not find the right parameter we return - // an index that was asked if we run in compatibility mode - // - // we should switch it off as soon as people convert applications - if (compatibilityMode) { - return index; - } else { - throw new SQLException("Specified parameter does not exist", SQL_STATE_INVALID_DESC_FIELD_ID); - } + // For historic compatibility reasons we return the original requested index if there is no mapping + return index; } /** diff --git a/src/test/org/firebirdsql/jdbc/escape/FBEscapedCallParserTest.java b/src/test/org/firebirdsql/jdbc/escape/FBEscapedCallParserTest.java index 7259c5986..aea1fad81 100644 --- a/src/test/org/firebirdsql/jdbc/escape/FBEscapedCallParserTest.java +++ b/src/test/org/firebirdsql/jdbc/escape/FBEscapedCallParserTest.java @@ -52,6 +52,7 @@ class FBEscapedCallParserTest { private static final String CALL_TEST_8 = "EXECUTE PROCEDURE my_proc (UPPER(?), '11-dec-2001')"; private static final String CALL_TEST_9 = " \t EXECUTE\nPROCEDURE my_proc ( UPPER(?), '11-dec-2001') \t"; + private final FBEscapedCallParser parser = new FBEscapedCallParser(); private FBProcedureCall testProcedureCall; private final FBProcedureParam param1 = new FBProcedureParam(0, "?"); private final FBProcedureParam param2 = new FBProcedureParam(1, "UPPER(?)"); @@ -78,17 +79,15 @@ public void setUp() throws SQLException { @Test void testProcessEscapedCall() throws Exception { - FBEscapedCallParser parser = new FBEscapedCallParser(); - FBProcedureCall procedureCall1 = parser.parseCall(CALL_TEST_5); procedureCall1.registerOutParam(1, Types.INTEGER); procedureCall1.getInputParam(2).setValue("test value"); assertThrows(SQLException.class, () -> procedureCall1.registerOutParam(3, Types.CHAR), "Should not allow registering param 3 as output, since it does not exist"); - - assertEquals(1, procedureCall1.mapOutParamIndexToPosition(1, false)); - assertThrows(SQLException.class, () -> procedureCall1.mapOutParamIndexToPosition(2, false), - "Should not allow to obtain position when no compatibility mode is specified"); + // Index 1 corresponds to the first mapped OUT parameter, so it returns 1 + assertEquals(1, procedureCall1.mapOutParamIndexToPosition(1), "Should return mapped parameter"); + // Index 2 does not correspond to a mapped OUT parameter, this returns the original value (2) + assertEquals(2, procedureCall1.mapOutParamIndexToPosition(2), "Should return unmapped parameter"); FBProcedureCall procedureCall2 = parser.parseCall(CALL_TEST_5_1); procedureCall2.registerOutParam(1, Types.INTEGER); @@ -112,6 +111,20 @@ void testProcessEscapedCall() throws Exception { verifyParseSql(procedureCall6); } + @Test + void testOutParameterMapping() throws Exception { + FBProcedureCall procedureCall = parser.parseCall(CALL_TEST_5); + procedureCall.getInputParam(1).setValue("test value"); + procedureCall.registerOutParam(2, Types.INTEGER); + // Index 1 does not correspond to a mapped OUT parameter, this returns the original value (1) + assertEquals(1, procedureCall.mapOutParamIndexToPosition(1), "Should return unmapped parameter"); + // Index 2 corresponds to the first mapped OUT parameter, so it returns 1 as well + assertEquals(1, procedureCall.mapOutParamIndexToPosition(2), "Should return mapped parameter"); + // Index 3 does not correspond to a mapped OUT parameter (though it does correspond to a literal marked as OUT), + // this returns the original value (3) + assertEquals(3, procedureCall.mapOutParamIndexToPosition(3), "Should return unmapped parameter"); + } + private void verifyParseSql(FBProcedureCall procedureCall) throws SQLException { assertEquals(testProcedureCall.getSQL(false), procedureCall.getSQL(false), String.format("Should correctly parse call.\n[%s] \n[%s]",