[ http://wso2.org/jira/browse/REGISTRY-23?page=all ]
Chathura Ekanayake resolved REGISTRY-23.
----------------------------------------
Resolution: Fixed
Added the RegistryDataSource class to encapsulate database parameters and
provide the DataSource interface.
Changed the constructors to always use DataSource interface.
Used Apache DBCP inside the RegisrtyDataSource to manage connection pools.
> Code Review Feedback
> --------------------
>
> Key: REGISTRY-23
> URL: http://wso2.org/jira/browse/REGISTRY-23
> Project: WSO2 Registry
> Issue Type: Improvement
> Reporter: Sanjaya Karunasena
> Assigned To: Chathura Ekanayake
> Priority: Critical
>
> I went through the code in SVN couple of days ago. Here are few problems I
> have
> noticed.
> * There is a "init" method which does lot of datasource related work.
> First of all having a init method in a OO code doesn't make sense. YES, IOC
> containers use this since they can't handle any thing else while doing
> constructor injection. But we shouldn't follow that.
> This code actually belong to a RegistryDatasource class, which should ideally
> implement java.sql.Datasource. Then we are clean.
> * There is a ConnectionFactory class which does a trick.
> Given a java.sql.Datasource or a DB URI it gives a java.sql.connection.
> However, when a DB URI is given it uses the DriverManager. Please read the
> Java doc. The connections you get have very different QoS properties. If I am
> a user I will assume the Registry nicely encapsulate all of that and give me
> a very reliable connectivity to the datasource, which is not the case.
> Another reason to have our own RegistryDatasource class.
> So that way we need one constructor;
> JDBCRegistry(DataSource dataSource)
> If what user has is a DB URI... he will,
> new JDBCRegistry( new RegistryDatasource(DB URI, ...) )
> * The SecureRegistry doesn't completely encapsulate the Registry
> Right now the user has to construct a Registry and pass it to the
> SecureRegistry. The problem here is, there is insecure access to the same
> registry since the use build that externally. If we aggregate the Registry in
> to the SecureRegistry, an instance of a SecureRegistry is always secure.
> BTW, I was thinking about Paul's reason to have a JDBCRegistry class and a
> InMemory Registry class where he would like the developer to know what
> exactly he is going to get just looking at the class name (without reading
> the Javadoc). In that case how about naming them as, "PersistentRegistry"
> and "TransientRegistry"?
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://wso2.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
_______________________________________________
Registry-dev mailing list
[email protected]
http://wso2.org/cgi-bin/mailman/listinfo/registry-dev
