Here is the latest version of the patch with the aprropriate messages.
thanks
Shreyas
Daniel John Debrunner wrote:
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
myrna wrote:
It does no longer throw an NPE, and it passes all tests....
Could we leave EmbeddedDataSource alone & only change
EmbeddedXADataSource? Only with the xa datasource did we get a NPE...
Although strictly speaking, it should behave the same, so, with
EmbeddedDataSource too, we should accept the databasename only, it's
been able to resolve the url fine. I'd be apprehensive about any
possible cloudscape customers that have mistakenly used this
functionality...
I agree with Shreyas that his patch is logically correct but I want to
ensure that your concerns are addressed. Can you be explicit about what
you believe the patch breaks for existing users of Derby. I tried this
and it results in a NullPointerException if the return from
getConnection() is referenced, e.g. in the conn.close() here.
(this is before the patch is applied)
org.apache.derby.jdbc.EmbeddedDataSource ds = new
org.apache.derby.jdbc.EmbeddedDataSource();
ds.setDatabaseName("jdbc:derby:fred");
Connection conn = ds.getConnection();
// NPE here because conn is null
conn.close();
The only issue I have with the patch is that the error thrown is
misleading, "Database Not Available" implies to the user or application
developer that they need to do something to make the database available.
A database not found exception with the name obtained from
DataSource.getDatabaseName() might make more sense. Or maybe even
better, invalid property setting error, since databaseName is a Java
Bean property of the datasource. E.g. SQLState.PROPERTY_INVALID_VALUE,
resulting in an error like
Invalid value for property 'databaseName'='jdbc:derby:fred'.
Thanks,
Dan.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.5 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iD8DBQFB7rJnIv0S4qsbfuQRApTGAKDj+MxB4kOY/GNFCMs/9R5Js3zK2wCgqYSm
DrAWFRgPOfwMHin0vUwGi70=
=zL6e
-----END PGP SIGNATURE-----
Index: java/engine/org/apache/derby/jdbc/EmbeddedDataSource.java
===================================================================
--- java/engine/org/apache/derby/jdbc/EmbeddedDataSource.java (revision
125587)
+++ java/engine/org/apache/derby/jdbc/EmbeddedDataSource.java (working copy)
@@ -34,6 +34,11 @@
import org.apache.derby.iapi.reference.Attribute;
+import org.apache.derby.iapi.reference.MessageId;
+import org.apache.derby.iapi.reference.SQLState;
+import org.apache.derby.iapi.error.ExceptionSeverity;
+import org.apache.derby.iapi.services.i18n.MessageService;
+import org.apache.derby.impl.jdbc.Util;
/**
@@ -431,6 +436,8 @@
final Connection getConnection(String username, String password,
boolean requestPassword)
throws SQLException {
+ Connection toClose = null;
+
Properties info = new Properties();
if (username != null)
info.put(Attribute.USERNAME_ATTR, username);
@@ -450,6 +457,7 @@
if (attributesAsPassword && requestPassword && password !=
null) {
+
StringBuffer sb = new StringBuffer(url.length() +
password.length() + 1);
sb.append(url);
@@ -459,8 +467,12 @@
url = sb.toString();
}
+ toClose = findDriver().connect(url, info);
- return findDriver().connect(url, info);
+ if(toClose == null)
+ throw
Util.generateCsSQLException(SQLState.PROPERTY_INVALID_VALUE,Attribute.DBNAME_ATTR,getDatabaseName());
+
+ return toClose;
}
Driver169 findDriver() throws SQLException
Index: java/engine/org/apache/derby/jdbc/EmbeddedXADataSource.java
===================================================================
--- java/engine/org/apache/derby/jdbc/EmbeddedXADataSource.java (revision
125587)
+++ java/engine/org/apache/derby/jdbc/EmbeddedXADataSource.java (working copy)
@@ -132,6 +132,9 @@
private void setupResourceAdapter(String user, String password, boolean
requestPassword) throws SQLException
{
+
+ Connection toClose = null;
+
synchronized(this)
{
if (ra == null || !ra.isActive())
@@ -157,18 +160,24 @@
// If database is not found,
try connecting to it. This
// boots and/or creates the
database. If database cannot
// be found, this throws
SQLException.
- if (requestPassword)
- getConnection(user,
password).close();
- else
- getConnection().close();
-
+ if (requestPassword) {
+ toClose =
getConnection(user, password);
+ if(toClose != null)
+ toClose.close();
+ }
+ else {
+ toClose =
getConnection();
+ if(toClose != null)
+ toClose.close();
+ }
// now try to find it again
database = (Database)
Monitor.findService(Property.DATABASE_MODULE, dbName);
}
- if (database != null)
+ if (database != null) {
ra = (ResourceAdapter)
database.getResourceAdapter();
+ }
}
if (ra == null)
@@ -178,9 +187,8 @@
// If database is already up, we need to set up
driver
- // seperately.
+ // seperately.
findDriver();
-
if (driver == null)
throw new
SQLException(MessageService.getTextMessage(MessageId.CORE_DRIVER_NOT_AVAILABLE),
"08006",
