On Fri, Jul 24, 2009 at 9:35 AM, Ruwan Linton <[email protected]>wrote:
> Hiranya, > > Please go ahead with the proposed modifications, the above two methods > which adds the sequences as a Entry was there to support the cached sequence > entries from the registry but then we removed the sequences and endpoints > map from the Synapse Configuration and decided to use one single map which > is the localEntries map. > > So it is safe to remove those or may be we can deprecate them and remove in > the next release since it is a public method. +1... Let's deprecate them for the moment. Thanks, Hiranya > > > Thanks, > Ruwan > > > On Thu, Jul 23, 2009 at 4:50 PM, Hiranya Jayathilaka <[email protected] > > wrote: > >> I believe the first change is almost a trivial one hence doesn't require >> any confirmation. But the second one (removing of the unused methods) >> probably needs some insight from the devs. >> >> Thanks, >> Hiranya >> >> >> On Thu, Jul 23, 2009 at 4:17 PM, Hiranya Jayathilaka < >> [email protected]> wrote: >> >>> Hi Folks, >>> >>> When going through the SynapseConfiguration class I noticed some minor >>> design issues in the code. For instance some of the add methods in the class >>> (eg: addProxyService, addStartup) do not guarantee that adding a new item >>> does not overwirte any existing item. Let's take the addProxyService method >>> for example. >>> >>> public void addProxyService(String name, ProxyService proxy) { >>> proxyServices.put(name, proxy); >>> } >>> >>> If we call this method with two ProxyService objects having the same name >>> the second service object will overwrite the first one. It seems that >>> SynapseConfiguration class relies on higher layers to take care of such >>> issues. As a result in classes like SynapseXMLConfigurationFactory we need >>> to do this. >>> >>> if (config.getProxyService(proxy.getName()) != null) { >>> handleException("Duplicate proxy service with name : " + >>> proxy.getName()); >>> } >>> config.addProxyService(proxy.getName(), proxy); >>> >>> IMHO such validation should take place at the lowest level, ie at the >>> SynapseConfiguration class itself. Higher levels shouldn't have to dig into >>> the existing configuration before trying to add a new item. It should just >>> add the item and the lower level should perform the necessary validation and >>> throw an exception in case of an error. >>> >>> Secondly I have acome across some methods that are not used anywhere in >>> the code. >>> >>> eg: >>> public void addSequence(String key, Entry entry) >>> public void addEndpoint(String key, Entry entry) >>> >>> I think it's safe to get rid of these methods and simply the API. So what >>> do you folks think about carrying out the necessary refactoring operations >>> to fix these design issues? If you all agree I can spend some time on this. >>> >>> Thanks >>> -- >>> Hiranya Jayathilaka >>> Software Engineer; >>> WSO2 Inc.; http://wso2.org >>> E-mail: [email protected]; Mobile: +94 77 633 3491 >>> Blog: http://techfeast-hiranya.blogspot.com >>> >> >> >> >> -- >> Hiranya Jayathilaka >> Software Engineer; >> WSO2 Inc.; http://wso2.org >> E-mail: [email protected]; Mobile: +94 77 633 3491 >> Blog: http://techfeast-hiranya.blogspot.com >> > > > > -- > Ruwan Linton > Technical Lead & Product Manager; WSO2 ESB; http://wso2.org/esb > WSO2 Inc.; http://wso2.org > email: [email protected]; cell: +94 77 341 3097 > blog: http://ruwansblog.blogspot.com > -- Hiranya Jayathilaka Software Engineer; WSO2 Inc.; http://wso2.org E-mail: [email protected]; Mobile: +94 77 633 3491 Blog: http://techfeast-hiranya.blogspot.com
