Hi,
Adding null check can avoid the NPE exception but may not solve the issue
properly.
@Lasantha,
I will check with GREG team and see if this can be solved with touchpoints.
@Firzhan.
I have gone through the discussion "[AS] Error in shutting down the server
: Cannot put transports into maintenance mode : NPE from
RegistryConfigLoader." earlier and your findings, will check with GREG
team.
Thanks,
Nipuni
On Wed, Sep 10, 2014 at 11:44 AM, Firzhan Naqash <firz...@wso2.com> wrote:
> 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 <lasan...@wso2.com>
> wrote:
>
>> Hi,
>>
>>
>> On 10 September 2014 10:19, Nipuni Perera <nip...@wso2.com> 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 <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
>>>
>>>
>>
>>
>> --
>> *Lasantha Fernando*
>> Software Engineer - Data Technologies Team
>> WSO2 Inc. http://wso2.com
>>
>> email: lasan...@wso2.com
>> mobile: (+94) 71 5247551
>>
>> _______________________________________________
>> Dev mailing list
>> Dev@wso2.org
>> http://wso2.org/cgi-bin/mailman/listinfo/dev
>>
>>
>
--
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
