recreating a datasource using javax.naming.Reference from a ClientDataSource40 fails

• Key: DERBY-2559
• Type: Bug
• Status: Resolved
• Resolution: Fixed
• Priority: Major
• Assignee: Kristian Waagan
• Reporter: Myrna van Lunteren
• Component/s: Network Client
• Affects Version/s: 10.3.1.4, 10.4.1.3
• Fix Version/s: 10.3.3.0, 10.4.1.3

Description

Consider the following code snippet from test DataSourceReferenceTest:
--------------------
        Referenceable refDS = (Referenceable) ds;
        Reference dsAsReference = refDS.getReference();
        String factoryClassName = dsAsReference.getFactoryClassName();
        ObjectFactory factory =
            (ObjectFactory) Class.forName(factoryClassName).newInstance();
        Object recreatedDS =
            factory.getObjectInstance(dsAsReference, null, null, null);
---------------------

When ds is a ClientDataSource40 (i.e. when running with jdk16), recreatedDS is null.

Note, that this showed up only after converting the test to junit, because the original test hardcoded the ds to be a ClientDataSource. I confirmed this not to be related to my changes for DERBY-2296 (which prompted me to convert the test), by backing out my changes to ClientBaseDataSource and client/am/Connection and rerunning the test (needed some minor adjustments of expected values table/array).

     

Comments

'derby-2559-1a.diff' is a patch that fixes the reported problem, where attempts to (re)create a data source through Reference returned null.
The cause was an if with entries for the pre-JDBC 4.0 classes (like ClientDataSource), but no entries for the JDBC 4.0 classes. I changed the factory class to use the same approach as the embedded driver, as it seemed to be easier to use reflection because of the mix between pre-JDBC 4.0 classes and JDBC 4.0 classes.
The patch also removes the workaround (disabling) in DataSourceReferenceTest.

I am not sure if the method behaves as documented. I don't think it will ever return null, but either succeed or throw an exception. I will look at the documentation in a separate patch.

I'm running tests and will post the results later.
Patch ready for review.

[Kristian Waagan]

Committed 'derby-2559-1a.diff' to trunk with revision 617492.
This commit is an incremental fix. More thought must be put into how the object factory is to be well behaved.
The factory will now create all the client data source objects, but it might/will fail if another type of object (or null) is passed as the first argument.

I'm also thinking about porting the fix to 10.3.

[Kristian Waagan]

I have attached two more patches:
 - derby-2559-2a-defenses.diff: Does some defensive checking to ensure no exception is thrown when it should not be.
 - derby-2559-3a-documentation.diff: Fixes to the documentation, both formatting and content.

Patches ready for review. I'm especially awaiting comments on the 2a patch.

[Knut Anders Hatlen]

2a looks good. Seems like the patch makes the implementation more in line with the description in the javadoc, so that's good.

[Kristian Waagan]

Thanks for looking at the patch Knut Anders.

'derby-2559-3b-documentation.diff' adds a throw clause to the JavaDoc and corrects an invalid reference. I also refreshed the patch after applying patch 2a.

Committed 'derby-2559-2a-defenses.diff' to trunk with revision 628647.
Committed 'derby-2559-3b-documentation.diff' to trunk with revision 628654.

The previous code failed to create data sources when running with Java SE 6.
This suggest the patches could be backported to branches 10.2 and 10.3.
Patches 1 and 2 apply cleanly, the documentation patch (3) does not.

[Kristian Waagan]

Clearing patch available flag, but considering backporting at least patches 1 and 2 to 10.2 and 10.3.

[Kristian Waagan]

Merged patches 1 and 2 to 10.3 with revision 632274.
Tests ran cleanly.

For 10.2, the merge also worked (except that the test changes were skipped because the file doesn't exist), but I got one error:
derbyall/derbyall.fail:lang/timestampArith.java.
I also got this failure without the merge as well. I don't have time to investigate, so I will hold back the merge to 10.2.
If anyone wants to merge the fix, please go ahead.

If this issue is still open in one week, I'll mark it resolved.

[Kristian Waagan]

Seems I was bitten by this one, due to leap year issues: DERBY-3486.
Thus the merge also ran without failures on the 10.2 branch.
I leave it up to the release manager or someone else to do the merge if they feel it's worth the hassle.

[A B]

> Merged patches 1 and 2 to 10.3 with revision 632274.

I think this merge lead to a javadoc error:

  [javadoc] java\client\org\apache\derby\client\ClientDataSourceFactory.java:48: warning - Tag @see: reference not found: ClientDataSource

Does 628654 need to be merged back, as well?

[Kristian Waagan]

Thank you Army.
The revision you mention updates the documentation, but does not fix the JavaDoc error (didn't notice...). I didn't merge it right away because I got conflicts for 10.2, but it applies fine for 10.3.

Merged patch 3b to 10.3 with revision 635567, and also fixed the JavaDoc error in the same commit.

Resolving issue, reopen to merge the fixes back to 10.2.
Will close the issue in a few days.