History | Log In     View a printable version of the current page.  
Issue Details (XML | Word | Printable)

Key: CASTOR-650
Type: Improvement Improvement
Status: Closed Closed
Resolution: Fixed
Priority: Minor Minor
Assignee: Werner Guttmann
Reporter: Martin Fuchs
Votes: 0
Watchers: 0
Operations

If you were logged in you would be able to see more operations.
castor

Feature Request: reset database registry

Created: 27/Jan/04 11:47 AM   Updated: 18/Apr/05 10:56 PM
Component/s: JDO
Affects Version/s: 0.9.5.2
Fix Version/s: None

Time Tracking:
Not Specified

Environment:
Operating System: All
Platform: All

Bugzilla Id: 1511


 Description  « Hide
I am missing some functionality to reset the database configuration completely
or remove a specific database entry from the cache. My case current problem has
been to reconnect to a database after changing the connection password. If I'm
not wrong, this is currently not possible without restarting the process, which
uses the castor library.

Following my (simple) workaround to achieve what I need - a public clear()
function in class DatabaseRegistry. I am already using this modified code for
my application. Sure this modification is not very elaborate, it's missing at
least some locking mechanism for synchronization.

Index: engine/DatabaseRegistry.java
===================================================================
RCS
file: /cvs/castor/castor/src/main/org/exolab/castor/jdo/engine/DatabaseRegistry.
java,v
retrieving revision 1.4
diff -u -p -d -r1.4 DatabaseRegistry.java
— engine/DatabaseRegistry.java 19 Jan 2004 11:52:47 -0000 1.4
+++ engine/DatabaseRegistry.java 27 Jan 2004 19:39:16 -0000
at at -395,4 +395,10 at at public class DatabaseRegistry
}

+ public static void clear()
+ { + _databases.clear(); + _byEngine.clear(); + }
+
}



 All   Comments   Work Log   Change History      Sort Order: Ascending order - Click to sort in descending order
Werner Guttmann - 28/Jan/04 12:04 PM
Before looking into this, can you please provide a more elaborate scenario of
how you intend to be using this ? What is the main driver for this ? And why is
it not an option for you to take the JVM running Castor down ?

Martin Fuchs - 28/Jan/04 01:02 PM
> What is the main driver for this ?

I am using castor in a GUI application. At program startup there is a login
dialog, which prompts for database name, user name and password. After the user
entered this connection data the application connects with this user account to
the (Oracle) database. To pass the connection data to Castor I am using a in-
memory XML config file, which is passed as stream to JDO.loadConfiguration():

String db_name = "MyApp#" + db_url + "#" + _username;
String conf_str = "<database name=\"" + db_name + "\" engine=\"oracle\">\n" +
"<driver class-name=\"" + s_jdo_driver_name + "\" " +
"url=\"" + db_url + "\">\n" +
"<param name=\"user\" value=\"" + _username + "\" />\n" +
"<param name=\"password\" value=\"" + _password + "\" />\n" +
"</driver>\n" +
"<mapping href=\"" + map_url.toString() + "\" />\n" +
"</database>\n";

_jdo.setDatabaseName(db_name);
_jdo.setClassLoader(getClass().getClassLoader());

StringReader reader = new StringReader(conf_str);
JDO.loadConfiguration(new InputSource(reader), null, null);

[By the way: This could be made much easiear if Castor had a method for
connecting with explicit connection data instead of only being able to read XML
config files.]

At the first login the user is prompted to change its password. (for security
reasons) After the user entered a new password and it's changed in the
database, the application tries to reconnect to the database. This is not
successfully, because Castor uses the old, cached database configuration with
the old password.
So I would like to be able to reset this old cached connection information in
order to connect using the new password.

> And why is it not an option for you to take the JVM running Castor down ?

This is no option because the application should remain running - otherwise the
user would be forced to launch it again by hand.


Bruce Snyder (bugzilla) - 20/Jul/04 09:11 AM
Martin, could you please write a quick JUnit test to verify this patch? I'd like
to add this to CVS, but the lack of a test is slowing me down and I don't
currently have the time to write a test for it myself.

One other thought that I had on this subject was to overload the
JDO.setConfiguration() method that accepts an org.exolab.castor.jdo.conf.JdoConf
object. This would save you from having to create XML in a String. Instead you
would create the JdoConf object graph and pass it to the setConfiguration() method.


Martin Fuchs - 21/Jul/04 03:38 PM
Created an attachment (id=584)
DBRegistryTest.java

Martin Fuchs - 21/Jul/04 03:38 PM
Created an attachment (id=585)
oracle.xml DB-config 1

Martin Fuchs - 21/Jul/04 03:38 PM
Created an attachment (id=586)
DB config 2

Martin Fuchs - 21/Jul/04 03:39 PM
Created an attachment (id=587)
empty mapping file

Martin Fuchs - 21/Jul/04 03:42 PM
OK, I wrote a JUnit test case for the pending patch.

It logs in to the database using the default password "sectret".
Then changes the login password, and clears the Castor database registry by
calling DatabaseRegistry.clear().
After that it tries to login again using the new password.

Without the call to DatabaseRegistry.clear() the second login will fail.


Martin Fuchs - 22/Jul/04 01:43 AM
Can I rename attachements in Bugzilla after posting them?
I can't find an option for that, so here are the missing file names:
  • "DB config 2" should be called "oracle2.xml".
  • "empty mappingfile" should be called "mapping.xml".

> One other thought that I had on this subject was to overload the
> JDO.setConfiguration() method that accepts an
org.exolab.castor.jdo.conf.JdoConf
> object. This would save you from having to create XML in a String.
> Instead you would create the JdoConf object graph and pass it to
> the setConfiguration() method.

I would appreciate that very much. This would be a much cleaner way
to connect to varying databases and/or with different DB users.


Martin Fuchs - 26/Jul/04 10:54 AM
For the current version of Castor in CVS there is an additional call to
JDOConfLoader.deleteConfiguration() necessary to reset the JDO configuration
data.

Werner Guttmann - 12/Sep/04 01:37 PM
Martin, based on your last comment above, can you please re-create your patch,
and attach it to this bug report. Once that's in, I'll get this committed, and
add the setConfiguration() overload as well (if possible).

Martin Fuchs - 12/Sep/04 02:27 PM
Created an attachment (id=650)
OK, here is the updated patch.

Werner Guttmann - 13/Sep/04 12:11 PM
Thank you. I am just looking at the patch, but because of your dependency on
Oracle, it's quite hard for me to run things. Why not create and alter some
entity that maps to a simple table, and attach entity class and mapping to this
bug report.

Werner Guttmann - 14/Sep/04 02:03 AM
Martin, in your code example above, can you please give an idea how a value for
map_url might look like ?

Werner Guttmann - 14/Sep/04 02:47 AM
Created an attachment (id=657)
Revised patch.

Werner Guttmann - 14/Sep/04 02:49 AM
Above patch (in addition to Martin's contribution) provides a
JDO.setConfiguration (JdoConf) method. I have not had any time (yet) to get a
test case done for this addition, but have successfully amended the test case as
provided by Martin to access AccountNature instances based on different mapping
files.

Werner Guttmann - 14/Sep/04 02:54 AM
Actually, having said that, somehow I should make it clear that there's a
certain level of 'ugliness' in JDO.java. By introducing JDO.setConfiguration
(JdoConf), I had to find a way to store the JdoConf instance somewhere - thus I
introduced a new member named _jdoConf, and renamed the existing one to
_jdoConfURI. Have a look at JDO.getDatabase() to see how these two members are
used to load one or the other configuration. I am really tired today, so
suggestions how to improve this are welcome.

Werner Guttmann - 14/Sep/04 02:56 AM
Btw, here's one way of improving things without much work. When setting a URI
for a JDO configuration file, try to unmarshall the configuration file
immediately, and sore the result in one and one instance variable only: _jdoConf
(of type JdoConf). Of course this would remove 'lazy configuration'
instantiation, but who cares .. .

Martin Fuchs - 14/Sep/04 02:58 AM
Sure. The value of map_url is for example:
"file:/D:/src/test_app/bin/de/mentopolis/test_app/jdo/DBMapping.xml"

It's the URL from where Castor shall load the mapping configuration file. When
running a JAR'ed application it will point to a file in this JAR archive. My
example comes just out of the Eclipse debugger, so this is where the "bin"
directory is from.


Werner Guttmann - 14/Sep/04 03:15 AM
Thanks, Martin. I just wanted to make sure that you are using absolute URIs,
indeed. This way (and only this way), things should work.

Martin Fuchs - 14/Sep/04 05:32 AM
Hi Werner,

I tried your patch and fixed it to avoid NULL pointer exceptions while calling
JDO.getDatabase():

public Database getDatabase()
throws DatabaseNotFoundException, PersistenceException
{
if ( _dbName == null )
throw new IllegalStateException( "Called 'getDatabase' without
first setting database name" );

if ( DatabaseRegistry.getDatabaseRegistry( _dbName ) == null ) {
// use _jdoConfURI to load the JDO configuration
if (_jdoConfURI != null)
try { DatabaseRegistry.loadDatabase( new InputSource( _jdoConfURI ), _entityResolver, _classLoader ); } catch ( MappingException except ) { throw new DatabaseNotFoundException( except ); }
// alternatively, use a JdoConf instance to load the JDO
configuration
else if (_jdoConf != null){
try { DatabaseRegistry.loadDatabase( _jdoConf, _entityResolver, _classLoader ); } catch ( MappingException except ) { throw new DatabaseNotFoundException( except ); }
}
else
throw new DatabaseNotFoundException( Messages.format
( "jdo.dbNoMapping", _dbName ) );
}

This way I successfully used a prepared JdoConf instance in my application.
I think this is in fact a much cleaner way than setting up a in-memory XML-
config file and using InputSource to pass it to loadConfiguration().
The resulting preparation code is still a bit longer than the original, but
this can be overcome by defining some helper classes.

> When setting a URI for a JDO configuration file, try to unmarshall
> the configuration file immediately, and store the result in one and
> one instance variable only: _jdoConf (of type JdoConf).

I think this would be a good idea.


Martin Fuchs - 14/Sep/04 07:35 AM
Created an attachment (id=661)
JdoConfHelpers

Martin Fuchs - 14/Sep/04 07:44 AM
I attached a file containing a few helper classes to make it easy to configure
the Castor database connection without a XML file. Using them this task becomes
as simple as:

JDO jdo = new JDO();

jdo.setDatabaseName("db");

JdoDriverConf driverConf = new JdoDriverConf(s_jdo_driver_name, db_url,
_username, _password);
JdoDbConf jdoDbConf = new JdoDbConf(db_name, "oracle", driverConf);
jdoDbConf.addMapping(new JdoMappingConf(map_url.toString()));
jdo.setConfiguration(new JdoConfConf(jdoDbConf));

Some questions to this:

  • Do you think the overloaded constructors should be integrated directly in the
    already existing org.exolab.castor.jdo.conf classes or better remain as
    separate classes?
  • If not - should they be integrated into Castor or remain in the application
    code?
  • Should I create a new bugzilla entry for this new enhancements?

Werner Guttmann - 15/Sep/04 01:46 PM
> Do you think the overloaded constructors should be integrated directly
> in the already existing org.exolab.castor.jdo.conf classes or better
> remain as separate classes?
One should definitely not integrate them with the existing castor.jdo classes,
as they are auto-generated from an XML Schema instance.

> If not - should they be integrated into Castor or remain in the
> application code?
This is a tricky one .. imho, as per above reason (auto-generation), I thin you
should not extend any classes from this package at all. Would you mind writing a
set of stand-alone classes that did the same ? Somehow something is telling me
that we really should not extend from any of those classes.

Why not create a factory class (JdoConfFactory) that offered a couple of static
factory methods to have JdoConf instances created ?

>Should I create a new bugzilla entry for this new enhancements?
Let's discuss this here. In the end, it is related, isn't it ?


Werner Guttmann - 15/Sep/04 01:46 PM
>> When setting a URI for a JDO configuration file, try to unmarshall
>> the configuration file immediately, and store the result in one and
>> one instance variable only: _jdoConf (of type JdoConf).
>
>I think this would be a good idea.

Any objections ?




Martin Fuchs - 16/Sep/04 02:30 AM
Created an attachment (id=667)
OK, here is the new JdoConfFactory class for convenient creation of JDO configurations without the need of a database configuration XML file.

Werner Guttmann - 16/Sep/04 03:29 AM
Great. Let's see whether I can get this committed tomorrow.

Werner Guttmann - 16/Sep/04 03:30 AM
> Can I say +2, or do I only get one vote? Just on the edge of
> needing this functionality; this will be an absolutely huge time-saver
> on our side, and I'd love to see it in post-haste.
Aren't you meant to be going on holiday tomorrow ? So how come you are in
desparate need of this ?

Werner Guttmann - 16/Sep/04 03:34 AM
> However, that's a topic for another bug, if any at all ...
Don't forget to add a new feature request BEFORE you go on holiday ..,
otherwise it will never happen, as you might have experienced in the past ... ;-
).

I, for myself, will be very glad to commit this simple addition as a first
step, as well as some other bugs related closely to this one. How come there
are no dependencies defined ... ?

Maybe I should ask for money from you guys in London for all this commits ..
given that I spend 10+ hours on this a week. Actually, I have got an EC tax
number, so we could do it. Gregory ?


Werner Guttmann - 17/Sep/04 01:42 AM
Created an attachment (id=670)
Final patch

Werner Guttmann - 17/Sep/04 01:43 AM
Above patch is the total of the contributions above, incl. JdoConfFactory (which
I have moved to the castor.util package, as castor.jdo.conf only holds
auto-generated classes).

Werner Guttmann - 17/Sep/04 02:02 AM
Applied above patch as discussed.

Martin, I've already seen the class Javadocs of JdoConfFactory, but can you
please think about providing a patch for the HTML docs (src/docs) introducing
the new features.

I think that sections about JdoConfFactory and JDO.setConfiguration should be
added to src/docs/database-conf.xml at least, maybe to src/docs/jdo.xml as well
(in form of a link to database-conf.xml). Not sure where the functionality of
clearing a Database instance should go.


Werner Guttmann - 17/Sep/04 02:02 AM
And again .. patch applied .. .

Martin Fuchs - 20/Sep/04 10:49 AM
Created an attachment (id=682)
Documentation patch

Martin Fuchs - 20/Sep/04 10:55 AM
> I think that sections about JdoConfFactory and JDO.setConfiguration should be
> added to src/docs/database-conf.xml at least, maybe to src/docs/jdo.xml as
well
> (in form of a link to database-conf.xml). Not sure where the functionality of
clearing a Database instance should go.

The patch above contains the documentation about the new setConfiguration()
function and configuration factory class. I included the util package in
JavaDoc (see build.xml) to be able to link to it. The DB registry reset
function is explained on the JDO FAQ page. Hope it's all OK to be included into
the documentation.

Could you please also apply the following JavaDoc fix:

Index: JdoConfFactory.java
===================================================================
RCS file: /cvs/castor/castor/src/main/org/exolab/castor/util/JdoConfFactory.java
,v
retrieving revision 1.1
diff -u -p -d -r1.1 JdoConfFactory.java
— a/JdoConfFactory.java 17 Sep 2004 08:51:40 -0000 1.1
+++ b/JdoConfFactory.java 20 Sep 2004 17:53:23 -0000
at at -89,7 +89,7 at at import org.exolab.castor.jdo.conf.Transa

  • jdoDbConf = JdoConfFactory.createJdoDbConf(db_name, "oracle", JdoConfFactory
    .createJdoDSConf(ds));
  • </code>
    *
  • * at author Martin Fuchs <martin-fuchs at gmx dot net>
    + * at author <a href="mailto:martin-fuchs at gmx dot net">Martin Fuchs</a>
    */
    public class JdoConfFactory
    {

Thanks,

Martin


Werner Guttmann - 22/Sep/04 07:50 AM
Thanks, Martin, and I would seriously recommend to change your email address to
something like "mailto:martin-fuchs AT gmx DOT net", to avoid being spammed.

Martin Fuchs - 22/Sep/04 08:40 AM
Of course you can change the address, if you like. But it's no big problem for
a spammer to decode it then. My address is public through a few mailing lists
(and this bugzilla entries) anyway. (Btw: Same with any mail addresses here)
But my mail provider's spam filters works quite good.

Werner Guttmann - 11/Oct/04 01:56 PM
I have just noticed that JDO2 is begging for the loadConfiguration(JdoConf)
method to be added as well. Martin ?

Martin Fuchs - 11/Oct/04 04:14 PM
When looking at JDO2 I had to notice, the usage of JdoConfFactory has been
broken. I think it's ben the commit of bug 1616, because it's the call to
getDatabaseChoice(), that always returns null. Because of this there occure
NULLPointer Ecpetions in JdoConfFactory.createJdoDbConf(). Might have a closer
look tomorrow...

Werner Guttmann - 12/Oct/04 12:28 AM
Thanks, Martin, and you might be right that this is a result of committing bug
1616 .. 8-(.

Martin Fuchs - 12/Oct/04 02:05 PM
Created an attachment (id=728)
fix for JdoConfFactory's DatabaseChoice handling

Martin Fuchs - 12/Oct/04 02:38 PM
May be we should add some test cases for JdoConfFactory. This way it gets
easier to find such problems.

Werner Guttmann - 13/Oct/04 02:26 AM
Patch applied, incl. a couple of code improvements in JDO2 (exception handling,
broken JavaDocs, ...).

Werner Guttmann - 13/Oct/04 02:27 AM
Martin, it looks like JDO is working again, but JDO2 still lacks a
loadConfiguration(JdoConf) method. Whilst this is not high priority, I think we
should try to make it for the 0.9.6 release.

Werner Guttmann - 13/Oct/04 02:28 AM
And mabe we should try to pick up on this old idea below as well.

> When setting a URI for a JDO configuration file, try to unmarshall
> the configuration file immediately, and store the result in one and
> one instance variable only: _jdoConf (of type JdoConf).


Martin Fuchs - 14/Oct/04 05:02 PM
Created an attachment (id=732)
first patch version for JDO and JDO2

Martin Fuchs - 14/Oct/04 05:08 PM
The new patch above includes the requested method JDO2.loadConfiguration
(JdoConf) as well as a revised version of the old JDO class to load the JDO
configuration file immediately when calling JDO.setConfiguration(). This
already works for my test applications, both using XML configuration files and
JDOConfFactory.

However the JTF tests don't yet run - they print the following exception
message:

Caused by: unable to add attribute "configuration"
to 'org.exolab.castor.jdo.JDO' due to the following error: org.exolab
.castor.mapping.MappingRuntimeException: Nested error:
org.exolab.castor.mapping.MappingException: Nested error: Parsing
Error : File "file:///D:/java/castor-1511/sybase.xml" not found.
Line : 0
Column : 0

{file: [not available]; line: 11; column: 62}

at org.exolab.castor.xml.Unmarshaller.unmarshal(Unmarshaller.java:622)
at org.exolab.castor.xml.Unmarshaller.unmarshal(Unmarshaller.java:516)
at Main.run(Main.java:146)
at Main.main(Main.java:226)
Caused by: org.xml.sax.SAXException: unable to add attribute "configuration"
to 'org.exolab.castor.jdo.JDO' due to the f
ollowing error: org.exolab.castor.mapping.MappingRuntimeException: Nested
error: org.exolab.castor.mapping.MappingExcept
ion: Nested error: Parsing Error : File "file:///D:/java/castor-
1511/sybase.xml" not found.
Line : 0
Column : 0

It seems the BASE URI used to load the config file is the current directory
instead of the class directory.
May be some one else can have a look at this?


Werner Guttmann - 14/Oct/04 11:57 PM
Thanks, Martin.

Martin Fuchs - 15/Oct/04 01:00 AM
Werner,
if you look into the patch you will see it also addresses some other small
issues I found on the way:
  • JDO.getReference() asked for _jdoConf, but then used _jdoConfURI to create a
    string reference.
  • JDO2_jdoConfURI was incorrectly used as string in
    JDO2.getReference/getObjectInstance(). I removed it completely.
  • Some JDO2 comments contained "JDOold", I think this was a thing left over
    from creating JDO2.
  • some more comment and white space changes

I asked myself it it would not be better to remove the static variable
JDOConfLoader._jdoConf completely. Providing the method unmarshalConfiguration
() to unmarshal the JDO configuration from an input source and returning the
configuration object immediatelly should be enough. Anything other, like the
database or transaction configuration can be queried directly from this
returned JdoConf.

An additional comment to JDO2:
Creating an JDO2 instance using JdoConfFactory is achieved by calling a new
JDO2.createInstance() overload with both parameters "JdoConf jdoConf"
and "String databaseName" at one time, not by using two distinct calls, because
we need both informations at the same time.

Here is a small usage example:

org.exolab.castor.jdo.conf.Database jdoDbConf;

String db_url = "jdbc:mysql://localhost:3306/test";
String username = "test";
String password = "";

URL map_url = DeptEmp.class.getResource("mapping.xml");

// Set up JDO with the configuration the specified database driver and
connection information
jdoDbConf = JdoConfFactory.createJdoDbConf("db", "mysql",
JdoConfFactory.createJdoDriverConf("com.mysql.jdbc.Driver",
db_url, username, password));

// Store mapping file location in the configuration
jdoDbConf.addMapping(JdoConfFactory.createJdoMappingConf
(map_url.toString()));

// Activate the JDO configuration
JDO2 jdo = JDO2.createInstance(JdoConfFactory.createJdoConf
(jdoDbConf), "db");


Martin Fuchs - 16/Oct/04 08:44 AM
Created an attachment (id=734)
documentation update

Martin Fuchs - 16/Oct/04 08:48 AM
This documentation update contains the JDO2 example from above. It also fixes a
few small mistakes of the old JDO examples, and indents them to make it a bit
easier to read.

Martin Fuchs - 31/Oct/04 01:37 AM
Werner, do you think, the JDO2/JdoConfFactory adjustments should still go into
the 0.9.6 release? If not, we might move this into a new bug entry to avoid
forgetting about it - this bug here is already closed.

Martin Fuchs - 31/Oct/04 02:37 AM
... and if it doesn't make it into 0.9.6, I would like to propose not to
deprecate the whole old JDO class. It should be enough to deprecate only the
JDO.setConfiguration(String url) function. Otherwise any usage of
JdoConfFactory results in warning messages about the deprecated JDO class.

Werner Guttmann - 03/Nov/04 12:56 AM
As it stands right now, I honestly do not feel comfortable about making JDO2
available but in an experimental form. As a result, here's what I suggest:

a) de-deprecate JDO.java.
b) flag JDO2.java as experimental.
c) resolve a couple of minor issues with JDO.java (mainly Javadocs, JNDI-related
methods, etc.).
d) amend the DOCS to reflect a), b) and c).

And now the 'more difficult' part. We need to find a way to prevent users
binding a JDO instance that has been configured by providing a JdoConf
instance, as such an instance by definition is not prepared for being bound to a
naming context. The obvious solution would be to use the instance
variables 'jdoConf' and 'jdoConfURI' as part of a check.


Werner Guttmann - 03/Nov/04 01:21 AM
Created an attachment (id=758)
Proposed (final) patch

This is a final patch for the 0.9.6 RC, implementing all of the items discussed
above.


Werner Guttmann - 03/Nov/04 01:22 AM
Created an attachment (id=759)
Proposed (final) patch

And once again ... this time without JAR files, etc.