Community
Participate
Working Groups
The DTP connection profile management framework currently persists all new profile instances that it maintains. There are use cases where a profile instance is needed only for a db session, and should no longer exist beyond the session. A client will have to explicitly delete an instance if it is no longer needed. The proposed enhancement is for the DTP profile management to provide the service for the creation and cleanup of a transient profile instance. The requirement for an unique profile name should also be transparent to the API client.
So to further define this a bit... We are looking for the following: * A way to create a transient connection profile **Question: Based on what criteria? Vendor/version/driver jar paths/URL/class name? **Question: Does it simply hand back an instance of an IConnectionProfile? Or are you looking for the IConnection or IManagedConnection directly? **Question: Does this transient profile have a visible component? i.e. does a transient profile appear in the DSE? **Question: The goal is then to be able to use the profile instance to use the rest of the DTP APIs, for example to get at the underlying populated SQL Model. Correct? Where will this profile be used? * A way to get a list of transient profiles * A way to delete a transient profile ** Question: All of this is API-only
**Question: Based on what criteria? All the base properties of a profile instance. This can be generic to any type of profile, not just database's. **Question: Does it simply hand back an instance of an IConnectionProfile? Returning an instance of IConnectionProfile should be sufficient. **Question: Does this transient profile have a visible component? i.e. does a transient profile appear in the DSE? ** Question: All of this is API-only No need for visible component, i.e. API-only. **Question: The goal is then to be able to use the profile instance to use the rest of the DTP APIs, for example to get at the underlying populated SQL Model. Correct? Where will this profile be used? Yes. E.g. to initialize the SQB, which requires a connected profle instance. * A way to delete a transient profile Having the API for a client to explicitly do that would be nice. However, by being transient, the profile should be automatically deleted at the end of a session (e.g. when the Profile Manager instance is disposed.)
I have to say I'm a little perplexed by a transient profile not needing a name. I'd say that the name would be optional (we could create a unique default), but until the profile is disposed, if you lose your transient profile object, you could grab it by name, could you not? At least until the profile manager closes. By providing an explicit delete, we could allow an API consumer to do more of the management as they would like, rather than just when the profile manager closes. Does that make sense at all? What does the SQB require as far as a profile handle? does it take a profile object, an ID, or a name? --Fitz
My initial description mainly refers to the "uniqueness" of a transient profile name: "The requirement for an unique profile name should also be transparent to the API client." With that said, to grab a transient profile instance, using its instance ID should be sufficient. Having an unique name automatically created does not hurt. But it should be optional for client to specify a name (unique or not) to create a transient profile instance. Providing an explicit delete method for a transient profile object is good. It is just that its call by client would ideally be optional for session cleanup. >>What does the SQB require as far as a profile handle? The SQLBuilderConnectionInfo takes an IConnectionProfile object. And the SQM takes ConnectionInfo from a connected IConnectionProfile.
I'm thinking that we would add a few new methods to ProfileManager... public IConnectionProfile createTransientProfile(String providerID, Properties basePropertie) throws ConnectionProfileException; public IConnectionProfile createTransientProfile(String name, String providerID, Properties baseProperties, String parentProfile, boolean autoConnect) throws ConnectionProfileException; The deleteProfile method would still work (we'd just make it aware of the transient profiles) and upon closing/destroying the ProfileManager object, we'd delete all transient profiles. Would that be sufficient for this requirement?
So after our chat today at the Connectivity meeting: # For new transient profiles, new method for ProfileManager * public IConnectionProfile createTransientProfile(String providerID, Properties baseProperties) throws ConnectionProfileException; # Revise the current API * public void createProfile(String name, String description, String providerID, Properties baseProperties) throws ConnectionProfileException; # to * public IConnectionProfile createProfile(String name, String description, String providerID, Properties baseProperties) throws ConnectionProfileException; * change it so it returns an IConnectionProfile instance * change it so that name and description can be passed in to be null * see if binary compatibility is broken with this change
Created attachment 121959 [details] start of patch this is just the start of this work. have to save current state so I can deliver some other fixes and come back to this.
Created attachment 122596 [details] new patch This patch adds the new methods for creating a transient profile. I'll also add an example JUnit test that uses the new functionality.
Created attachment 122597 [details] mylyn/context/zip
Created attachment 122600 [details] Example of how to use new APIs This is a JUnit class that creates a transient Derby profile and then creates a second one for good measure. In between, it connects to the Derby db.
Created attachment 123417 [details] Updated patch This patch allows a developer to skip the driver definition all together and use some profile properties to set up the driver jars in the classpath, among other things. I'll also attach another code example in a JUnit test of how to use this...
Updated test... one method to create the properties: public static Properties generateTransientDerbyProperties() { Properties baseProperties = new Properties(); baseProperties.setProperty( IDriverMgmtConstants.PROP_DEFN_JARLIST, jarList ); baseProperties.setProperty(IJDBCConnectionProfileConstants.DRIVER_CLASS_PROP_ID, driverClass); baseProperties.setProperty(IJDBCConnectionProfileConstants.URL_PROP_ID, driverURL); baseProperties.setProperty(IJDBCConnectionProfileConstants.USERNAME_PROP_ID, userName); baseProperties.setProperty(IJDBCConnectionProfileConstants.PASSWORD_PROP_ID, password); baseProperties.setProperty(IJDBCConnectionProfileConstants.DATABASE_VENDOR_PROP_ID, vendor); baseProperties.setProperty(IJDBCConnectionProfileConstants.DATABASE_VERSION_PROP_ID, version); baseProperties.setProperty( IJDBCConnectionProfileConstants.SAVE_PASSWORD_PROP_ID, String.valueOf( true ) ); return baseProperties; } And another test method: public final void testStressTestingAPIs() throws Exception { ProfileManager pm = ProfileManager.getInstance(); ArrayList<Object> profiles = new ArrayList<Object>(); for (int i = 0; i < 10; i++) { Object test = pm.createTransientProfile(providerID, generateTransientDerbyProperties()); profiles.add(test); Assert.assertTrue(test instanceof IConnectionProfile); IConnectionProfile icp = (IConnectionProfile) test; System.out.println("Created connection profile: " + icp.getInstanceID()); //$NON-NLS-1$ IStatus status = icp.connect(); while (status == null); Assert.assertTrue(status.isOK()); System.out.println("Connected to connection profile: " + icp.getInstanceID()); //$NON-NLS-1$ } Iterator<Object> iter = profiles.iterator(); while (iter.hasNext()) { Object test = iter.next(); Assert.assertTrue(test instanceof IConnectionProfile); IConnectionProfile icp2 = (IConnectionProfile) test; if (icp2.getConnectionState() == IConnectionProfile.CONNECTED_STATE) { IStatus status2 = icp2.disconnect(); while (status2 == null); Assert.assertTrue(status2.isOK()); System.out.println("Disconnected from connection profile: " + icp2.getInstanceID()); //$NON-NLS-1$ } } }
I'd done some testing of the patch in comment #11 with the org.eclipse.datatools.connectivity.db.generic.connectionProfile provider. It works fine w/o the need to use a driver instance. That is great to have, especially when using a transient database connection profile. Thanks. The outstanding issues are then 1) thread-safe usage of the ProfileManager (see bug #262270) 2) a really minor point: the generated transient profile name could use a separator after the prefix e.g. transientName = "Transient" + providerID; to transientName = "Transient." + providerID; (not that it really matters; it is just easier to read when debugging and checking on the value)
Created attachment 123777 [details] Patch addressing comment 13 This patch should handle the synchronized issue as well as the string in the ID.
RE: patch added in Comment #14, The thread-safe of mTransientProfiles in InternalProfileManager can probably use some slight fine-tuning. E.g. in createTransientProfile method, 1) it is common practice and more robust to do another if_null checking, after having gone into synchronized state, in case another thread has done the assignment right after the first if_null checking. if (mTransientProfiles == null) { synchronized (this) { if (mTransientProfiles == null) mTransientProfiles = Collections.synchronizedList( new ArrayList() ); } } 2) Since a synchronized collection is used, calling its #add method takes care of thread-safe access, hence no need to manually synchronize: synchronized (mTransientProfiles) { mTransientProfiles.add(profile); } 3) Access to the backing list, e.g. access thru its iterator, such as those in #dispose method, should however manually synchronize.
Created attachment 125313 [details] Updated patch addressing latest comment This patch should address the issues in the last comment.
I just noticed the patch seems to contain some unrelated changes in InternalProfileManager. Is this intended? // Changed to fix bug 247599 //cps[index] = profile; - if(cps[index] != profile) { - removeProfile(cps[index]); - addProfile(profile); - } + removeProfile(cps[index]); + addProfile(profile); Separately, a minor thing, it should be sufficient to lock mTransientProfiles, instead of the instance. /* package */void dispose() { + if (mTransientProfiles != null) { + synchronized (this) { --> synchronized(mTransientProfiles) The findUniqueTransientProfileName logic of iterating thru each instance to match a name seems somewhat inefficient. Wonder if it may slow performance, as this is now done in synchronized mode, and the method is called each time a profile is added. Perhaps using a Map instead, with the profile name as the key would speed up the name matching. Anyway, such tuning can be applied later, and should not hold up this BZ.
I sync'd up with the latest code, which is where the extra change might have come in. And I can definitely just synchronize on the mTransientProfiles instance. As for making the "find" method faster, we can definitely look at that in a separate BZ. Let me respin the patch in a little while and get back to you.
Created attachment 125334 [details] updated patch with tweak for dispose This is the latest patch to address the last comment. My concern with making it a map for the find function is that we're really counting the # of instances with a given provider ID. It would have to be a hashmap that used the providerID as the key and a collection as the data, which would then store all the various instances of that providerID. Something to look at for the next iteration (new BZ).
RE: - if(cps[index] != profile) { It looks like this if condition was added in the latest version 1.30 for bug #247599. The patch however removes that fix.
Created attachment 125398 [details] updated patch with correction from other bug Linda, here's the corrected patch with the fix you mentioned. You were definitely correct. Those few lines were out of whack with what was already in the codebase.
I'd briefly tested with the latest patch, and it looks good to me. Thanks! RE: the open issue related to synchronized usage, how about use bug #262270 to track it? I just spotted another potential synchronized usage issue, and will add info about it in that bug.
Delivered to o.e.d.connectivity as tag v200902120538. Will continue the threadsafe work in the other bug mentioned.
Noteworthy: We've introduced a new API to create transient instances of connection profiles. These profiles can be created under the covers by other tooling (such as BIRT) and then they are discarded when the Eclipse workbench is closed instead of persisted and accessible in the tree of the Data Source Explorer.
