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
