Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 361030

Summary: Support SQL JNDI data sources
Product: z_Archived Reporter: Justin Spadea <jspadea>
Component: EDTAssignee: Justin Spadea <jspadea>
Status: CLOSED FIXED QA Contact:
Severity: enhancement    
Priority: P3 CC: mayunf, smythew, svihovec
Version: unspecified   
Target Milestone: ---   
Hardware: PC   
OS: Linux   
Whiteboard: ibmi jndi
Bug Depends on:    
Bug Blocks: 367078    
Attachments:
Description Flags
SQL binding detail page none

Description Justin Spadea CLA 2011-10-14 16:16:30 EDT
See the TODO in SysLib.doGetResource(). JNDI data sources are not implemented yet.
Comment 1 Will Smythe CLA 2011-12-15 11:59:51 EST
Think we need to consider this for 0.8. The current database implementation initiates new connections for every database request (i.e. no pooling), and any app beyond a basic app will benefit from the database connection pooling capabilities provided by most app servers.
Comment 2 Will Smythe CLA 2011-12-19 09:56:27 EST
We need to think about how we make this capability available in the DD. 

I *think* a new resource binding (of type JNDI) needs to be created. This option would have 1 parameter: "name", which would be something like "jdbc/mydb". The assumption is that a resource with this name has been properly configured in the application server. 

A common/typical development/production scenario might look like this:
1) Development DD would be configured to use a workspace db connection or configured with specific db connection settings. 
2) Production DD would have a JNDI resource binding with the same name as defined in #1. 

During development, the app would use the connection specified in the development DD, but at runtime (assuming it was deployed using the production DD), it would use the JNDI resource.
Comment 3 Will Smythe CLA 2011-12-19 10:04:10 EST
In addition to my comment #2 and because of Bug 367078 (Create Tomcat context.xml during deployment), we should consider allowing the developer to specify a JNDI resource name (e.g. "jdbc/mydb") as part of the SQL database resource binding (by default, this value would be blank). If a resource name is specified, the deployment process would create files like context.xml and configure it using the specified JNDI name and connection settings. The runtime would then use this JNDI name at runtime (versus using the 'hard coded' database connection settings).
Comment 4 Yun Feng Ma CLA 2012-01-05 03:35:12 EST
Which milestone is this enhancement targeted? My enhancement 367078 is depending on this. Thanks.
Comment 5 Justin Spadea CLA 2012-01-05 17:28:47 EST
I'm still investigating what's required for JNDI support across all aspects of the product (deployment, test server, java runtime, editors, wizards) so it's not yet known if it will fit in the current milestone. It might get broken up with core support in the first milestone and the tooling in the next one.
Comment 6 Justin Spadea CLA 2012-01-06 15:30:57 EST
Proposal for SQL JNDI support:

1. EGL Language
A new type will be added:

externalType SQLJNDIDataSource extends SQLDataSource type NativeType
end

It has no new API, it just obtains the Connection differently than plain JDBC (SQLDataSource). Users can manually instantiate this, or they can use resource bindings. It has the same constructors as SQLDataSource, but you pass in the JNDI name instead of the JDBC URL. If the data source is using Application authentication instead of Container authentication, the user can pass in a dictionary with the 'user' and 'password' attributes.

2. Java runtime
Implementation for SQLJNDIDataSource will be added, subclassing SQLDataSource. SysLib.getResource() will create SQLJNDIDataSource when the jndi:// URI for a binding is used.
A runtime exception will be thrown if attempting to create a connection outside of a JEE environment.

3. Deployment Descriptor
The model for the DD is unchanged. The editor and wizard, however, will be modified to be more user friendly and support the full requirements for JNDI. I've attached a mockup of the SQL binding detail page. The "Add" wizard for SQL bindings will have similar updates. Summary of the changes:

a) The URI section is no longer generic. Different URIs have different requirements, and a generic input field means the user must know each URI format. This is now split into two radio buttons.

    a1) For pointing to a workspace connection we provide a combo with the available choices (if the DD's connection profile doesn't exist in the current workspace, it will display as "myConnectionProfile <Not found>" so that the user is aware and can update the setting or create a connection with that name.

    a2) The second radio button is when the server has a data source already defined (e.g. using a server that we don't support configuring, such as WAS, so the user defines the DS manually on the server and has the DD point to it). The username and password attributes are optional and are only needed if the connection needs different credentials than what's on the data source definition.

b) There is a new check box at the bottom that tells us to define a data source on the server for the user. Initially this will only support Tomcat. If the "Use an existing JNDI data source" radio button is selected then the check box is disabled; when either of the other options is selected this check box becomes available (if using an existing data source, then there is no definition to be done on the server). When this check box is enabled then the application will use JNDI to access the (new) data source, instead of using plain JDBC.

You'll note it asks for more information than the design proposal in bug 367078. The combo for data source class lets a user manually input a value, or they can pick from the drop down, which will be prefilled with all the classes that are in the jars for the database implementing javax.sql.CommonDataSource.

4. Deployment

a) Support the new check box described in section 3b - defining the data source on the server. If the target project is not a Web project, or is not a server runtime that we can configure (i.e. it's not Tomcat) then a warning should be issued in the deployment results indicating that the data source was not defined and JNDI will not be used for the resource binding.

b) When "Use an existing JNDI data source" is selected, OR the new check box is selected, update web.xml to add a resource ref so that the data source can be accessed by the client.

5. Test server
The test server should make use of JNDI so that test is as similar to the production environment as possible. One example of runtime difference if the test server does not support JNDI is that "if (myDS isa SQLJNDIDataSource)" would evaluate differently than production code. Jetty has built-in JNDI support, we just need to configure it so that data sources are available. Work involved:

a) For each SQL resource binding of each DD on the test server, if it has the new check box for defining data sources selected, use the information to automatically define such a data source.

b) For the condition in 5a, OR the condition that the user selected "Use an existing JNDI data source", a resource ref needs to be configured.

c) Update IDEResourceLocator to create SQLJNDIDataSource objects as necessary.

d) The code that determines the server needs to be updated as changes are made to DD files will need to handle any JNDI-related changes (new/changed data sources, or new/changed resource refs).

e) To support the "Use an existing JNDI data source" option in test mode, we can add a preference page for the test server that lets users define data sources that get added to all test servers (they would need to enter the JNDI name, the data source class, and pick a connection profile that provides the rest of the information).
Comment 7 Justin Spadea CLA 2012-01-06 15:31:29 EST
Created attachment 209149 [details]
SQL binding detail page
Comment 8 Justin Spadea CLA 2012-01-11 12:53:27 EST
The following bugs were opened to track each piece of work:

368366
367078
368371 	
368373
Comment 9 Justin Spadea CLA 2012-02-01 13:43:25 EST
All work has been completed.
Comment 10 Justin Spadea CLA 2012-02-07 16:32:55 EST
Also see https://bugs.eclipse.org/bugs/show_bug.cgi?id=370746
Comment 11 Justin Spadea CLA 2012-02-22 08:52:59 EST
Closing