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]
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 <kas...@wso2.com> 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 <nip...@wso2.com> 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: nip...@wso2.com
>> Git hub profile: https://github.com/nipuni
>> Mobile: +94 (71) 5626680
>> <http://wso2.com>
>>
>>
>> _______________________________________________
>> Dev mailing list
>> Dev@wso2.org
>> 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: nip...@wso2.com
Git hub profile: https://github.com/nipuni
Mobile: +94 (71) 5626680
<http://wso2.com>
_______________________________________________
Dev mailing list
Dev@wso2.org
http://wso2.org/cgi-bin/mailman/listinfo/dev
