The fix looks good Raj! Thanks!

On Mon, Mar 2, 2015 at 7:14 PM, Rajkumar Rajaratnam <[email protected]>
wrote:

> Fixed in db532859732794b9dd7512cea999ad84eb4355ec.
>
> On Mon, Mar 2, 2015 at 6:23 PM, Rajkumar Rajaratnam <[email protected]>
> wrote:
>
>> It should be even simplified as below.
>>
>>     private void findSubscribableInfoOfGroupContexts(String
>> applicationId,
>>             Map<String, SubscribableInfoContext>
>> subscribableInfoContextMap, GroupContext[] groupContexts)
>>                     throws ApplicationDefinitionException {
>>
>>         if (groupContexts != null) {
>>             for (GroupContext groupContext : groupContexts) {
>>                 // finding SubscribableInfo in group
>>                 if (groupContext.getGroupContexts() != null) {
>>                     findSubscribableInfoOfGroupContexts(applicationId,
>> subscribableInfoContextMap, groupContext.getGroupContexts());
>>                 }
>>                 // finding SubscribableInfo in cartridge
>>                 if (groupContext.getCartridgeContexts() != null) {
>>
>> findSubscribableInfoOfCartridgeContexts(applicationId,
>> subscribableInfoContextMap, groupContext.getCartridgeContexts());
>>                 }
>>             }
>>         }
>>     }
>>
>> Thanks.
>>
>> On Mon, Mar 2, 2015 at 6:18 PM, Rajkumar Rajaratnam <[email protected]>
>> wrote:
>>
>>> Hi Devs,
>>>
>>> Seems there is a small mistake in 
>>> DefaultApplicationParser#findSubscribableInfoOfGroupContexts()
>>> method [1].
>>>
>>>     private void findSubscribableInfoOfGroupContexts(String
>>> applicationId,
>>>             Map<String, SubscribableInfoContext>
>>> subscribableInfoContextMap, GroupContext[] groupContexts)
>>>                     throws ApplicationDefinitionException {
>>>
>>>         if (groupContexts != null) {
>>>             for (GroupContext groupContext : groupContexts) {
>>>                 if (groupContext.getGroupContexts() != null) {
>>>                     findSubscribableInfoOfGroupContexts(applicationId,
>>> subscribableInfoContextMap, groupContext.getGroupContexts());
>>>                 } else {
>>>                     CartridgeContext[] cartridgeContexts =
>>> groupContext.getCartridgeContexts();
>>>                     for (CartridgeContext cartridgeContext :
>>> cartridgeContexts) {
>>>                         if(cartridgeContext != null) {
>>>                             SubscribableInfoContext
>>> subscribableInfoContext = cartridgeContext.getSubscribableInfoContext();
>>>                             addSubscribableInfo(applicationId,
>>> cartridgeContext.getType(),
>>>                                     subscribableInfoContextMap,
>>> subscribableInfoContext);
>>>                         }
>>>                     }
>>>                 }
>>>             }
>>>         }
>>>     }
>>>
>>> According to the above logic, some cartridge's subscribableInfo will
>>> not be collected. There shouldn't be an *else* block there. Is this
>>> intentional or a mistake? I guess the correct logic should be like below.
>>>
>>>     private void findSubscribableInfoOfGroupContexts(String
>>> applicationId,
>>>             Map<String, SubscribableInfoContext>
>>> subscribableInfoContextMap, GroupContext[] groupContexts)
>>>                     throws ApplicationDefinitionException {
>>>
>>>         if (groupContexts != null) {
>>>             for (GroupContext groupContext : groupContexts) {
>>>                 // finding SubscribableInfo in group
>>>                 if (groupContext.getGroupContexts() != null) {
>>>                     findSubscribableInfoOfGroupContexts(applicationId,
>>> subscribableInfoContextMap, groupContext.getGroupContexts());
>>>                 }
>>>                 // finding SubscribableInfo in cartridge
>>>                 if (groupContext.getCartridgeContexts() != null) {
>>>                     CartridgeContext[] cartridgeContexts =
>>> groupContext.getCartridgeContexts();
>>>                     for (CartridgeContext cartridgeContext :
>>> cartridgeContexts) {
>>>                         if(cartridgeContext != null) {
>>>                             SubscribableInfoContext
>>> subscribableInfoContext = cartridgeContext.getSubscribableInfoContext();
>>>                             addSubscribableInfo(applicationId,
>>> cartridgeContext.getType(),
>>>                                     subscribableInfoContextMap,
>>> subscribableInfoContext);
>>>                         }
>>>                     }
>>>                 }
>>>             }
>>>         }
>>>     }
>>>
>>> WDYT?
>>>
>>> 1.
>>> https://github.com/apache/stratos/blob/6a407bd501e5409d27f0a1a7fb031820dccf95d3/components/org.apache.stratos.autoscaler/src/main/java/org/apache/stratos/autoscaler/applications/parser/DefaultApplicationParser.java#L184-L205
>>>
>>> --
>>> Rajkumar Rajaratnam
>>> Committer & PMC Member, Apache Stratos
>>> Software Engineer, WSO2
>>>
>>> Mobile : +94777568639
>>> Blog : rajkumarr.com
>>>
>>
>>
>>
>> --
>> Rajkumar Rajaratnam
>> Committer & PMC Member, Apache Stratos
>> Software Engineer, WSO2
>>
>> Mobile : +94777568639
>> Blog : rajkumarr.com
>>
>
>
>
> --
> Rajkumar Rajaratnam
> Committer & PMC Member, Apache Stratos
> Software Engineer, WSO2
>
> Mobile : +94777568639
> Blog : rajkumarr.com
>



-- 
Imesh Gunaratne

Technical Lead, WSO2
Committer & PMC Member, Apache Stratos

Reply via email to