This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.
| Summary: | Add method to Database Explorer API to remove an existing connection | ||
|---|---|---|---|
| Product: | db | Reporter: | David Vancouvering <davidvc> |
| Component: | Code | Assignee: | David Vancouvering <davidvc> |
| Status: | RESOLVED FIXED | ||
| Severity: | blocker | Keywords: | API_REVIEW_FAST |
| Priority: | P3 | ||
| Version: | 6.x | ||
| Hardware: | All | ||
| OS: | All | ||
| Issue Type: | DEFECT | Exception Reporter: | |
| Bug Depends on: | |||
| Bug Blocks: | 128369 | ||
| Attachments: | Diff output for proposed change | ||
|
Description
David Vancouvering
2008-02-26 21:16:34 UTC
Requesting a fast review as this is a pretty non-controversial incremental addition.
Here is the javadoc comment for the new method, I am also attaching the full diff for the change.
/**
* Remove an existing connection from the Database Explorer. This method
* unregisters the connection from the the explorer so it will
* no longer appear as a connection in the UI, and disconnects the underlying
* JDBC connection if it is connected.
*
* @param dbconn the connection to be removed
*/
public void removeConnection(DatabaseConnection dbconn) throws DatabaseException
Created attachment 57310 [details]
Diff output for proposed change
One small change. After further thought, I don't believe this API should have the side-effect of closing the JDBC connection. Doing so could have unintended consequences if some other part of the user code is holding a reference to the connection. AB01. Not all clients close the connection when they are done with it (often because they expect to use it again soon -- think SQL editor). So if removeConnection() doesn't close the connection, who does? AB02. Please increment the specification number and add a @since tag to the new method. AB03. Not sure about the implementation. Do you really need the removeSubNode() method? Can't you just call destroy() on the connection node? (I haven't seen the DB Explorer in a while, so perhaps I'm wrong though.) AB04. Looks a space is missing before the opening usecase tag: + </usecase> + <usecase id="connect-database" name="Connecting to a database"> AB01. Not all clients close the connection when they are done with it (often because they expect to use it again soon -- think SQL editor). So if removeConnection() doesn't close the connection, who does? DVC: OK, good argument, I'll have it close the connection. AB02. Please increment the specification number and add a @since tag to the new method. DVC: OK AB03. Not sure about the implementation. Do you really need the removeSubNode() method? Can't you just call destroy() on the connection node? (I haven't seen the DB Explorer in a while, so perhaps I'm wrong though.) DVC: I'll look into that... AB04. Looks a space is missing before the opening usecase tag: + </usecase> + <usecase id="connect-database" name="Connecting to a database"> DVC: Sheesh! :) OK, will fix. It looks like the spec version is already at 25, even though the apichanges file wasn't modified. I found this changset 355394df5911 where Hrjebek bumped the spec version when branching for 6.0. So I'll make this revision 26 of the API. The review is closed, I made the changes requested and there were no further comments Subsequent fixes, we weren't closing the connection or removing it from the underlying connection list http://hg.netbeans.org/main?cmd=changeset;node=ad7bfce64029 http://hg.netbeans.org/main?cmd=changeset;node=d71d7d64ed98 |