Return-Path: X-Original-To: apmail-stratos-dev-archive@minotaur.apache.org Delivered-To: apmail-stratos-dev-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 34C49116E5 for ; Mon, 22 Sep 2014 11:45:24 +0000 (UTC) Received: (qmail 40890 invoked by uid 500); 22 Sep 2014 11:45:24 -0000 Delivered-To: apmail-stratos-dev-archive@stratos.apache.org Received: (qmail 40837 invoked by uid 500); 22 Sep 2014 11:45:23 -0000 Mailing-List: contact dev-help@stratos.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@stratos.apache.org Delivered-To: mailing list dev@stratos.apache.org Received: (qmail 40827 invoked by uid 99); 22 Sep 2014 11:45:23 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 22 Sep 2014 11:45:23 +0000 X-ASF-Spam-Status: No, hits=1.5 required=5.0 tests=HTML_MESSAGE,RCVD_IN_DNSWL_LOW,SPF_PASS X-Spam-Check-By: apache.org Received-SPF: pass (athena.apache.org: domain of chamilad@wso2.com designates 209.85.223.176 as permitted sender) Received: from [209.85.223.176] (HELO mail-ie0-f176.google.com) (209.85.223.176) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 22 Sep 2014 11:45:19 +0000 Received: by mail-ie0-f176.google.com with SMTP id ar1so7219535iec.21 for ; Mon, 22 Sep 2014 04:44:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=wso2.com; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :content-type; bh=367mfwzzgcNreeR/geyBhvyRTXYa2LjOnuHGtefBras=; b=f+ELRDJcR3eQQj0Jbe4sHQdwmkpD1INWD9ZpP8vbsF/hzIh7tVD37/WoC0wmt4C4cf B2fXadNYbj3VBETLpn4tlqXGA++WbO3gCt+bSqVESohzGYFdoPw8lRNvxy7GJl+aOON0 oVYMAbwCgOipTfR0u1zLJgOjFNTvxB23yx4YE= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:content-type; bh=367mfwzzgcNreeR/geyBhvyRTXYa2LjOnuHGtefBras=; b=VWnth7KdDZCtDKb0p+r+rm8+FyCs1jc8NCq9h7NqXnsPgMm4hjzkvJ4HLQ0iNUxD2j tMtq2DPv2DSQ934htQrs/+2HgKTORZ0nWjxLLKbfAaKMgFhHrqeThv4Tb24QPYwRjaek 4gOEC1baUrh+wV9j+g/7Rn0hYHwv3EDF6Z5GF3I128Gao5wfdxD7XucSeJu2yZ9Og07t 5IeLGMCW/RPZbfG59tuT7wHuv0SsfzTU3Ar4MY6FOSgFjDRPW+H2MfJc3rWK7AKjPVl6 Zw4tQPmdj+y3jqpGHpCPB+bQwSAwERHyLxyTp23KW5tjvR3nklU13JLBtLqqIO774gfp Jy3Q== X-Gm-Message-State: ALoCoQnL0ECKSmw6mWKszV7sRWz4ghwIY/ECBJ0N3aqCsuvnsLxde8P6SxayaInBlg2TUYaaUbpc X-Received: by 10.50.79.165 with SMTP id k5mr13933268igx.16.1411386298834; Mon, 22 Sep 2014 04:44:58 -0700 (PDT) MIME-Version: 1.0 Received: by 10.64.223.228 with HTTP; Mon, 22 Sep 2014 04:44:27 -0700 (PDT) In-Reply-To: References: From: Chamila De Alwis Date: Mon, 22 Sep 2014 17:14:27 +0530 Message-ID: Subject: Re: Tenant listeners added twice in the Cartridge Agent To: dev@stratos.apache.org Content-Type: multipart/alternative; boundary=089e013a032efbb8a90503a5fa13 X-Virus-Checked: Checked by ClamAV on apache.org --089e013a032efbb8a90503a5fa13 Content-Type: text/plain; charset=UTF-8 Hi Akila, I made the changes and created a PR [1]. Please review and merge. 1 - https://github.com/apache/stratos/pull/48 Regards, Chamila de Alwis Software Engineer | WSO2 | +94772207163 Blog: code.chamiladealwis.com On Mon, Sep 22, 2014 at 2:33 PM, Akila Ravihansa Perera wrote: > Hi Chamila, > > Yes, my bad. Tenant listeners in > subscribeToTopicsAndRegisterListeners() should be removed. > > Great work in finding these flaws. Appreciate if you could send a PR with > fixes. > > Thanks > > On Mon, Sep 22, 2014 at 1:08 PM, Chamila De Alwis > wrote: > > Hi Akila, > > > > The listeners in registerTenantEventListeners() are invoking extension > > handling through the DefaultExtensionHandler. The tenant event listeners > > added in subscribeToTopicsAndRegisterListeners() are not, so they must be > > removed, along with the additional methods in ExtensionUtils. > > > > > > Regards, > > Chamila de Alwis > > Software Engineer | WSO2 | +94772207163 > > Blog: code.chamiladealwis.com > > > > > > > > On Mon, Sep 22, 2014 at 12:53 PM, Akila Ravihansa Perera > > wrote: > >> > >> Hi Chamila, > >> > >> Looks like there is a flaw in the logic. The extension should be > >> invoked via DefaultExtensionHandler. This could be a merge conflict. > >> Also we should refactor cartridge agent to support customizable > >> extension handlers. I've created a JIRA to track this task [1]. > >> > >> Tenant listeners under registerTenantEventListeners() should be removed. > >> > >> [1] https://issues.apache.org/jira/browse/STRATOS-808 > >> > >> > >> Thanks. > >> > >> On Mon, Sep 22, 2014 at 12:20 PM, Chamila De Alwis > >> wrote: > >> > Hi guys, > >> > > >> > Any idea if the two paths are intentional or a flow? > >> > > >> > > >> > Regards, > >> > Chamila de Alwis > >> > Software Engineer | WSO2 | +94772207163 > >> > Blog: code.chamiladealwis.com > >> > > >> > > >> > > >> > On Sat, Sep 20, 2014 at 8:40 PM, Chamila De Alwis > >> > wrote: > >> >> > >> >> Hi, > >> >> > >> >> In org.apache.stratos.cartridge.agent.CartridgeAgent "tenant/#" topic > >> >> listeners are added twice. SubscriptionDomainAddedEventEventListener > >> >> and > >> >> SubscriptionDomainRemovedEventListener is first added in > >> >> subscribeToTopicsAndRegisterListeners() method at line 254. Then > these > >> >> two > >> >> are added again in registerTenantEventListeners() method at line 438. > >> >> > >> >> There are two overriding methods for > >> >> executeSubscriptionDomainAddedExtension() and > >> >> executeSubscriptionDomainRemovedExtension() each accepting a map of > >> >> "STRATOS_" prepended environment parameters and the other accepting > >> >> individual parameters like tenant Id and domain name. > >> >> > >> >> Furthermore, in CartridgeAgentConstants there are two entries for > each > >> >> of > >> >> the extension scripts, one hardcoding the script name and the other > >> >> taking > >> >> the script name from the stratos.sh properties. > >> >> > >> >> The extension script seems to be using the "STRATOS_" prepended > >> >> properties > >> >> for its use. Therefore, the path with individual parameters seems to > be > >> >> a > >> >> dead end. > >> >> > >> >> Furthermore, in the method subscribeToTopicsAndRegisterListeners() > >> >> method, > >> >> two threads are started with the same "instance/#" topic listener, at > >> >> line > >> >> 243 and 297. The second one seems to be mistakenly added, may be > while > >> >> merging conflicts? > >> >> > >> >> > >> >> Regards, > >> >> Chamila de Alwis > >> >> Software Engineer | WSO2 | +94772207163 > >> >> Blog: code.chamiladealwis.com > >> >> > >> >> > >> > > >> > >> > >> > >> -- > >> Akila Ravihansa Perera > >> Software Engineer, WSO2 > >> > >> Blog: http://ravihansa3000.blogspot.com > > > > > > > > -- > Akila Ravihansa Perera > Software Engineer, WSO2 > > Blog: http://ravihansa3000.blogspot.com > --089e013a032efbb8a90503a5fa13 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
Hi Akila,

I made the changes and created= a PR [1]. Please review and merge.


1 - https://github.com/apache/stratos/pull/48

<= /div>
On Mon, Sep 22, 2014 at 2:33 PM, Akila Ravih= ansa Perera <ravihansa@wso2.com> wrote:
Hi Chamila,

Yes, my bad. Tenant listeners in
subscribeToTopicsAndRegisterListeners() should be removed.

Great work in finding these flaws. Appreciate if you could send a PR with f= ixes.

Thanks

On Mon, Sep 22, 2014 at 1:08 PM, Chamila De Alwis <chamilad@wso2.com> wrote:
> Hi Akila,
>
> The listeners in registerTenantEventListeners() are invoking extension=
> handling through the DefaultExtensionHandler. The tenant event listene= rs
> added in subscribeToTopicsAndRegisterListeners() are not, so they must= be
> removed, along with the additional methods in ExtensionUtils.
>
>
> Regards,
> Chamila de Alwis
> Software Engineer | WSO2 | +94772207163
> Blog: cod= e.chamiladealwis.com
>
>
>
> On Mon, Sep 22, 2014 at 12:53 PM, Akila Ravihansa Perera
> <ravihansa@wso2.com> w= rote:
>>
>> Hi Chamila,
>>
>> Looks like there is a flaw in the logic. The extension should be >> invoked via DefaultExtensionHandler. This could be a merge conflic= t.
>> Also we should refactor cartridge agent to support customizable >> extension handlers. I've created a JIRA to track this task [1]= .
>>
>> Tenant listeners under registerTenantEventListeners() should be re= moved.
>>
>> [1] https://issues.apache.org/jira/browse/STRATOS-808
>>
>>
>> Thanks.
>>
>> On Mon, Sep 22, 2014 at 12:20 PM, Chamila De Alwis <chamilad@wso2.com>
>> wrote:
>> > Hi guys,
>> >
>> > Any idea if the two paths are intentional or a flow?
>> >
>> >
>> > Regards,
>> > Chamila de Alwis
>> > Software Engineer | WSO2 | +94772207163
>> > Blog: code.chamiladealwis.com
>> >
>> >
>> >
>> > On Sat, Sep 20, 2014 at 8:40 PM, Chamila De Alwis <chamilad@wso2.com>
>> > wrote:
>> >>
>> >> Hi,
>> >>
>> >> In org.apache.stratos.cartridge.agent.CartridgeAgent &quo= t;tenant/#" topic
>> >> listeners are added twice. SubscriptionDomainAddedEventEv= entListener
>> >> and
>> >> SubscriptionDomainRemovedEventListener is first added in<= br> >> >> subscribeToTopicsAndRegisterListeners() method at line 25= 4. Then these
>> >> two
>> >> are added again in registerTenantEventListeners() method = at line 438.
>> >>
>> >> There are two overriding methods for
>> >> executeSubscriptionDomainAddedExtension() and
>> >> executeSubscriptionDomainRemovedExtension() each acceptin= g a map of
>> >> "STRATOS_" prepended environment parameters and= the other accepting
>> >> individual parameters like tenant Id and domain name.
>> >>
>> >> Furthermore, in CartridgeAgentConstants there are two ent= ries for each
>> >> of
>> >> the extension scripts, one hardcoding the script name and= the other
>> >> taking
>> >> the script name from the stratos.sh properties.
>> >>
>> >> The extension script seems to be using the "STRATOS_= " prepended
>> >> properties
>> >> for its use. Therefore, the path with individual paramete= rs seems to be
>> >> a
>> >> dead end.
>> >>
>> >> Furthermore, in the method subscribeToTopicsAndRegisterLi= steners()
>> >> method,
>> >> two threads are started with the same "instance/#&qu= ot; topic listener, at
>> >> line
>> >> 243 and 297. The second one seems to be mistakenly added,= may be while
>> >> merging conflicts?
>> >>
>> >>
>> >> Regards,
>> >> Chamila de Alwis
>> >> Software Engineer | WSO2 | +94772207163
>> >> Blog: code.chamiladealwis.com
>> >>
>> >>
>> >
>>
>>
>>
>> --
>> Akila Ravihansa Perera
>> Software Engineer, WSO2
>>
>> Blog: http://ravihansa3000.blogspot.com
>
>



--
Akila Ravihansa Perera
Software Engineer, WSO2

Blog: http:= //ravihansa3000.blogspot.com

--089e013a032efbb8a90503a5fa13--