Hi All, We have already discussed this issue in the mail thread [Dev] [AS] Error in shutting down the server : Cannot put transports into maintenance mode : NPE from RegistryConfigLoader.
Rather than checking the null point, I feel like we have to investigate why the registry's search feature bundle getting included to the products which are not using registry search feature ( BPS ). @Ajith:- You thoughts on this Regards, Firzhan On Wed, Sep 10, 2014 at 11:41 AM, Lasantha Fernando <[email protected]> wrote: > Hi, > > > On 10 September 2014 10:19, Nipuni Perera <[email protected]> wrote: > >> Hi, >> >> Yes. I can add a null check to indexingConfig element and continue only >> if the value is available (But it will add the whole method execution >> inside constructor RegistryConfigLoader() to an if block - line 75 to 150 >> of [1]). Is this preferable? >> >> public RegistryConfigLoader() { >> try { >> FileInputStream fileInputStream = new >> FileInputStream(getConfigFile()); >> StAXOMBuilder builder = new StAXOMBuilder( >> >> CarbonUtils.replaceSystemVariablesInXml(fileInputStream)); >> OMElement configElement = builder.getDocumentElement(); >> OMElement indexingConfig = >> configElement.getFirstChildWithName( >> new QName("indexingConfiguration")); >> if (indexingConfig != null) >> { //newly >> added if block >> ... >> ... >> } >> } >> } catch (FileNotFoundException e) { >> // This virtually cannot happen as registry.xml is necessary >> to start up the registry >> log.error("registry.xml has not been found", e); >> } catch (RegistryException e) { >> log.error(e.getMessage(),e); >> } catch (XMLStreamException e) { >> String msg = "error building registry.xml, check for badly >> formed xml"; >> log.error(msg, e); >> } catch (CarbonException e) { >> log.error("An error occurred during system variable >> replacement", e); >> } >> } >> >> It seems if the indexingConfig is null, no need to call this constructor. >> RegistryConfigLoader() is called inside IndexingManager() constructor. >> >> private IndexingManager() { >> try { >> registry = >> Utils.getRegistryService().getRegistry(CarbonConstants.REGISTRY_SYSTEM_USERNAME); >> registryConfig = new RegistryConfigLoader(); >> // call to constructor RegistryConfigLoader() >> indexer = new AsyncIndexer(); >> } catch (RegistryException e) { >> log.error("Could not initialize registry for indexing", e); >> } >> } >> Can't we check indexingConfig value before calling >> RegistryConfigLoader() ? >> > > +1 to add a null check and deal with this. As to not calling > RegistryConfigLoader if indexingConfig is not present, isn't > RegistryConfigLoader there to read the whole registry.xml file and > indexingConfig is only a part of that? If so, I guess we do need to call > RegistryConfigLoader() regardless of whether indexingConfig is present or > not. Maybe the GREG team can give better input on this. > > Also, this exact same code segment was discussed before in thread "[Dev] > Missing configuration in registry.xml causes OSGi services to be > unsatisfied" [1], but guess I forgot to raise a JIRA under that... :-). > Can you look into that thread as well and if it is possible to add a > touchpoint as well as suggested in that thread? > > [1] http://mail.wso2.org/mailarchive/dev/2014-April/029358.html > > Thanks, > Lasantha > >> >> [1] >> https://github.com/Nipuni/carbon-registry/blob/master/components/registry/org.wso2.carbon.registry.indexing/src/main/java/org/wso2/carbon/registry/indexing/RegistryConfigLoader.java >> >> Thanks, >> Nipuni >> >> >> On Wed, Sep 10, 2014 at 8:53 AM, Kasun Gajasinghe <[email protected]> >> wrote: >> >>> >>> As a best practice, do not catch the Exception class, instead catch >>> specific subclasses. But, you do not need to catch any exceptions in this >>> particular case? Can't you just verify whether the indexingConfiguration >>> element is available or not? >>> >>> KasunG >>> >>> On Wed, Sep 10, 2014 at 4:39 AM, Nipuni Perera <[email protected]> wrote: >>> >>>> Hi, >>>> >>>> I am working on issue[1]. I could find that the RegistryConfigLoader() >>>> returns a null value. (line 73 in [2]). >>>> >>>> OMElement indexingConfig = configElement.getFirstChildWithName(new >>>> QName("indexingConfiguration")); >>>> >>>> This value is used in several places. but NPE is thrown in line 88, >>>> >>>> lastAccessTimeLocation = indexingConfig.getFirstChildWithName(new >>>> QName("lastAccessTimeLocation")).getText(); >>>> >>>> as the catch clause catches only >>>> >>>> catch (OMException e) { >>>> ... // set default value >>>> } >>>> >>>> while in other places (eg: line 78,79) it is >>>> >>>> catch (Exception e) { >>>> ... // set default value >>>> } >>>> >>>> This exception occurred due to a missing XML elements in registry.xml. >>>> This missing element ( <indexingConfiguration> </indexingConfiguration> ) >>>> will be used if the product is using registry search feature. But this NPE >>>> should be handled for the products that does not use this feature. This can >>>> be fixed adding similar catch clause (catching Exception e ) in the >>>> catch clause(line 90,91) (current implementation use catch clauses to set >>>> default values if an exception occurs. Adding an extra catch clause to the >>>> later try block can be used to add similar behavior). Is there a better >>>> way to handle this? >>>> >>>> [1] https://wso2.org/jira/browse/CARBON-14904 >>>> [2] >>>> https://githubc.com/Nipuni/carbon-registry/blob/master/components/registry/org.wso2.carbon.registry.indexing/src/main/java/org/wso2/carbon/registry/indexing/RegistryConfigLoader.java >>>> <https://github.com/Nipuni/carbon-registry/blob/master/components/registry/org.wso2.carbon.registry.indexing/src/main/java/org/wso2/carbon/registry/indexing/RegistryConfigLoader.java> >>>> >>>> Thanks, >>>> Nipuni >>>> -- >>>> Nipuni Perera >>>> Software Engineer; WSO2 Inc.; http://wso2.com >>>> Email: [email protected] >>>> Git hub profile: https://github.com/nipuni >>>> Mobile: +94 (71) 5626680 >>>> <http://wso2.com> >>>> >>>> >>>> _______________________________________________ >>>> Dev mailing list >>>> [email protected] >>>> http://wso2.org/cgi-bin/mailman/listinfo/dev >>>> >>>> >>> >>> >>> -- >>> >>> *Kasun Gajasinghe*Senior Software Engineer, WSO2 Inc. >>> email: kasung AT spamfree wso2.com >>> linked-in: http://lk.linkedin.com/in/gajasinghe >>> blog: http://kasunbg.org >>> >>> >>> >> >> >> >> -- >> Nipuni Perera >> Software Engineer; WSO2 Inc.; http://wso2.com >> Email: [email protected] >> Git hub profile: https://github.com/nipuni >> Mobile: +94 (71) 5626680 >> <http://wso2.com> >> >> >> _______________________________________________ >> Dev mailing list >> [email protected] >> http://wso2.org/cgi-bin/mailman/listinfo/dev >> >> > > > -- > *Lasantha Fernando* > Software Engineer - Data Technologies Team > WSO2 Inc. http://wso2.com > > email: [email protected] > mobile: (+94) 71 5247551 > > _______________________________________________ > Dev mailing list > [email protected] > http://wso2.org/cgi-bin/mailman/listinfo/dev > >
_______________________________________________ Dev mailing list [email protected] http://wso2.org/cgi-bin/mailman/listinfo/dev
