-
Notifications
You must be signed in to change notification settings - Fork 33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added classes Connect.Call and StoredProcedureOutcome #53
Changes from all commits
3c0b50f
933e054
fecf961
3daf9cf
48aad25
7b2e751
bda699c
06b31f1
fd88534
5586e23
bc49e7d
d50b286
a4362ef
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,3 +2,7 @@ target/ | |
.idea/ | ||
*.iml | ||
.DS_Store | ||
bin/ | ||
.classpath | ||
.project | ||
.settings/ |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -106,6 +106,10 @@ | |
* @author Yegor Bugayenko ([email protected]) | ||
* @version $Id$ | ||
* @since 0.1.8 | ||
* @checkstyle ClassDataAbstractionCouplingCheck (500 lines) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @amihaiemil along with that we should add puzzle to break down this class, as it became too big |
||
* @todo #51:30min Refactor this class to avoid too much coupling. | ||
* For instance, CRUD operations could be performed by another class. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @amihaiemil can you also add remnidner to remove suppression? |
||
* Don't forget to remove the suppressions that become obsolete afterwards. | ||
*/ | ||
@ToString | ||
@EqualsAndHashCode(of = { "source", "connection", "args", "auto", "query" }) | ||
|
@@ -308,6 +312,26 @@ public <T> T update(final Outcome<T> outcome) | |
); | ||
} | ||
|
||
/** | ||
* Call an SQL stored procedure. | ||
* | ||
* <p>JDBC connection is opened and, optionally, commited by this | ||
* method, depending on the <b>autocommit</b> class attribute: | ||
* if it's value is true, the connection will be commited after | ||
* this call. | ||
* | ||
* @param <T> Type of result expected | ||
* @param outcome Outcome of the operation | ||
* @return Result of type T | ||
* @throws SQLException If fails | ||
*/ | ||
public <T> T call(final Outcome<T> outcome) | ||
throws SQLException { | ||
return this.run( | ||
outcome, new Connect.Call(this.query), Request.EXECUTE_UPDATE | ||
); | ||
} | ||
|
||
/** | ||
* Make SQL request expecting no response from the server. | ||
* | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,92 @@ | ||
/** | ||
* Copyright (c) 2012-2015, jcabi.com | ||
* All rights reserved. | ||
* | ||
* Redistribution and use in source and binary forms, with or without | ||
* modification, are permitted provided that the following conditions | ||
* are met: 1) Redistributions of source code must retain the above | ||
* copyright notice, this list of conditions and the following | ||
* disclaimer. 2) Redistributions in binary form must reproduce the above | ||
* copyright notice, this list of conditions and the following | ||
* disclaimer in the documentation and/or other materials provided | ||
* with the distribution. 3) Neither the name of the jcabi.com nor | ||
* the names of its contributors may be used to endorse or promote | ||
* products derived from this software without specific prior written | ||
* permission. | ||
* | ||
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS | ||
* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT | ||
* NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND | ||
* FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL | ||
* THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, | ||
* INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES | ||
* (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR | ||
* SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) | ||
* HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, | ||
* STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) | ||
* ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED | ||
* OF THE POSSIBILITY OF SUCH DAMAGE. | ||
*/ | ||
package com.jcabi.jdbc; | ||
|
||
import com.jcabi.aspects.Immutable; | ||
import java.sql.CallableStatement; | ||
import java.sql.ResultSet; | ||
import java.sql.SQLException; | ||
import java.sql.Statement; | ||
import lombok.EqualsAndHashCode; | ||
import lombok.ToString; | ||
|
||
/** | ||
* Outcome of a stored procedure with OUT parameters. | ||
* @author Mihai Andronache ([email protected]) | ||
* @version $Id$ | ||
* @since 0.17 | ||
* @param <T> Type of the returned result, which <b>has to be</b> Object[] | ||
*/ | ||
@Immutable | ||
@ToString | ||
@EqualsAndHashCode | ||
public final class StoredProcedureOutcome<T> implements Outcome<T> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @amihaiemil can we have a test for this class? Other There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mkordas is it ok of I add a todo?? Please make up ur mind about this in the beginning next time. U focus on indentation first and then u stretch my work for days... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @amihaiemil for me to-do is OK, however pdd is rather about making test first and puzzling implementation - otherwise we allow untested code for some time There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mkordas I tried creating a unit test, but it does not work. From what I read on SO and the official site, H2 database does not support stored procedures per se, only java functions declared as aliases: http://www.h2database.com/html/features.html#user_defined_functions I will leave this as it is since it's already well tested in the IT; is that ok with u? The effort of finding another mock DB only for this is simply not worth it IMO. |
||
|
||
/** | ||
* OUT parameters' indexes. | ||
*/ | ||
@Immutable.Array | ||
private final transient int[] indexes; | ||
|
||
/** | ||
* Ctor. | ||
* @param indexes Indexes of the OUT params. | ||
* <b>Index count starts from 1</b>. | ||
*/ | ||
public StoredProcedureOutcome(final int... indexes) { | ||
if (indexes.length == 0) { | ||
throw new IllegalArgumentException( | ||
"At least 1 OUT param's index needs to be specified!" | ||
); | ||
} | ||
final int size = indexes.length; | ||
this.indexes = new int[size]; | ||
for (int idx = 0; idx < size; ++idx) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @amihaiemil this looks like code duplication with the above one There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mkordas I see... it really is, and it cannot be removed. Usually we call one ctor from another one using I removed the first ctor and updated the PR description accordingly. It was in fact a convenience ctor and I figured it's not worth the duplication. |
||
this.indexes[idx] = indexes[idx]; | ||
} | ||
} | ||
|
||
@Override | ||
@SuppressWarnings("unchecked") | ||
public T handle(final ResultSet rset, final Statement stmt) | ||
throws SQLException { | ||
final int params = this.indexes.length; | ||
final Object[] outs = new Object[params]; | ||
if (stmt instanceof CallableStatement) { | ||
for (int idx = 0; idx < params; ++idx) { | ||
outs[idx] = ((CallableStatement) stmt).getObject( | ||
this.indexes[idx] | ||
); | ||
} | ||
} | ||
return (T) outs; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @amihaiemil is this always safe casting? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mkordas Right - I see in other Outcomes they solved the issue by requesting from the user the In my case I would let it fail and say in the Javadoc that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @amihaiemil maybe it's silly question, but why There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mkordas It doesn't get any better than this - I tried every combination of T and |
||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,7 +31,15 @@ | |
|
||
import com.jcabi.aspects.Tv; | ||
import com.jolbox.bonecp.BoneCPDataSource; | ||
import java.sql.CallableStatement; | ||
import java.sql.PreparedStatement; | ||
import java.sql.SQLException; | ||
import java.sql.Types; | ||
import java.util.Date; | ||
import javax.sql.DataSource; | ||
import org.apache.commons.lang3.StringUtils; | ||
import org.hamcrest.MatcherAssert; | ||
import org.hamcrest.Matchers; | ||
import org.junit.Assume; | ||
import org.junit.Before; | ||
import org.junit.Test; | ||
|
@@ -90,6 +98,93 @@ public void changesTransactionIsolationLevel() throws Exception { | |
new JdbcSession(source).sql("VACUUM").execute(); | ||
} | ||
|
||
/** | ||
* JdbcSession can run a function (stored procedure) with | ||
* output parameters. | ||
* @throws Exception If something goes wrong | ||
*/ | ||
@Test | ||
public void callsFunctionWithOutParam() throws Exception { | ||
final DataSource source = JdbcSessionITCase.source(); | ||
new JdbcSession(source).autocommit(false).sql( | ||
"CREATE TABLE IF NOT EXISTS users (name VARCHAR(50))" | ||
).execute().sql("INSERT INTO users (name) VALUES (?)") | ||
.set("Jeff Charles").execute().sql( | ||
StringUtils.join( | ||
"CREATE OR REPLACE FUNCTION fetchUser(username OUT text,", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @amihaiemil is it on purpose that here you use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mkordas Yes, since they are SQL views which hold the result of each SELECT, and they will exist at the same time; I didn't want to "step on each other's toes" |
||
" day OUT date)", | ||
" AS $$ BEGIN SELECT name, CURRENT_DATE INTO username, day", | ||
" FROM users; END; $$ LANGUAGE plpgsql;" | ||
) | ||
).execute().commit(); | ||
final Object[] result = new JdbcSession(source) | ||
.sql("{call fetchUser(?, ?)}") | ||
.prepare( | ||
new Preparation() { | ||
@Override | ||
public void | ||
prepare(final PreparedStatement stmt) | ||
throws SQLException { | ||
final CallableStatement cstmt = | ||
(CallableStatement) stmt; | ||
cstmt.registerOutParameter(1, Types.VARCHAR); | ||
cstmt.registerOutParameter(2, Types.DATE); | ||
} | ||
} | ||
) | ||
.call(new StoredProcedureOutcome<Object[]>(1, 2)); | ||
MatcherAssert.assertThat(result.length, Matchers.is(2)); | ||
MatcherAssert.assertThat( | ||
result[0].toString(), | ||
Matchers.containsString("Charles") | ||
); | ||
MatcherAssert.assertThat( | ||
(Date) result[1], | ||
Matchers.notNullValue() | ||
); | ||
} | ||
|
||
/** | ||
* JdbcSession can run a function (stored procedure) with | ||
* input and output parameters. | ||
* @throws Exception If something goes wrong | ||
*/ | ||
@Test | ||
public void callsFunctionWithInOutParam() throws Exception { | ||
final DataSource source = JdbcSessionITCase.source(); | ||
new JdbcSession(source).autocommit(false).sql( | ||
"CREATE TABLE IF NOT EXISTS usersids (id INTEGER, name VARCHAR(50))" | ||
).execute().sql("INSERT INTO usersids (id, name) VALUES (?, ?)") | ||
.set(1).set("Marco Polo").execute().sql( | ||
StringUtils.join( | ||
"CREATE OR REPLACE FUNCTION fetchUserById(uid IN INTEGER,", | ||
" usrnm OUT text) AS $$ BEGIN", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @amihaiemil probably I'm not educated enough, but what's There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mkordas To be honest, I'm not sure. It's the first time I work with PSQL and it's the same in all (I mean ALL) the stored procedure examples I could find. Search for "postgresql $$ operator" doesn't return anything either. Do you know it works without it? Even if it works, I assume there must be a reason for it. It's also there in the official examples here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @amihaiemil thanks for figuring it out, maybe we could drop a very short note about their official name (so that they become googleable) in Javadoc or comment? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @amihaiemil what about this one? |
||
" SELECT name INTO usrnm FROM usersids WHERE id=uid;", | ||
" END; $$ LANGUAGE plpgsql;" | ||
) | ||
).execute().commit(); | ||
final Object[] result = new JdbcSession(source) | ||
.sql("{call fetchUserById(?, ?)}") | ||
.set(1) | ||
.prepare( | ||
new Preparation() { | ||
@Override | ||
public void | ||
prepare(final PreparedStatement stmt) | ||
throws SQLException { | ||
((CallableStatement) stmt) | ||
.registerOutParameter(2, Types.VARCHAR); | ||
} | ||
} | ||
) | ||
.call(new StoredProcedureOutcome<Object[]>(2)); | ||
MatcherAssert.assertThat(result.length, Matchers.is(1)); | ||
MatcherAssert.assertThat( | ||
result[0].toString(), | ||
Matchers.containsString("Polo") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @amihaiemil why we check There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mkordas because then it complains of douvle literals and it is safe enough to have a contains, rahter than declare a variable for it... all the tests in all the jcabi libs are like this. Contains is used instead of equals wjere appropriate There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @amihaiemil IMO it breaks test data coupling rule, but let's follow convention then |
||
); | ||
} | ||
|
||
/** | ||
* Get data source. | ||
* @return Source | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@amihaiemil is the verision provided in parent?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mkordas yes it is
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@amihaiemil thanks for confirming