Hi Shenglin

Our IRC chat is not going well as out IT connections are in bad shape today.
So, as you discovered,
/{property}/{value} catches /mbean/{id} - /{property}/{value} is
preferred because  /mbean/{id}
is a subresource locator even though it is less specific.

Thus I suggested to introduce a slightly diff scheme
/list
/list/{propertyName}/{propertyValue}
/list/objectName/{objectValue}

The above 3 resource methods return MbeanCollection

/mbean/{id} returns MBean (via MBeanResource) as discussed earlier on.

Please hurry up as you need to get /counters dealt with as well
Cheers, Sergey

On Thu, Jun 9, 2011 at 1:17 PM, Sergey Beryozkin <sberyoz...@gmail.com> wrote:
> Hi Shenglin
>
> One thing which you are doing well is introducing subresources, given
> that both MBean and MBeanCotellection have become the ones.
> Understanding how subresources work is good in itself. I can see you
> have unique ids allocated to MBeans now. So there's some progress.
> IMHO some more code cleanup is needed.
>
> 1. At this stage please have only a ***single*** subresource,
> intoroduce a dedicated class called MBeanResource which should have
> only a single member, MBean instance. This MBeanResource should only
> have right now setMBean(MBean) and getMBean methods. getMBean method
> should be annotated with @GET.
>
> 2. MBean and MBeanCollection should have no static methods, and no
> JAX-RS resource methods at all. These are plain JAXB beans only.
>
> 3. JMXServer should have:
>  -  /mbean/{id}
>
> Note, Path value should be '/mbean/{id}'
>
>    You should use a captured {id}, such as 0, 1, 2, etc (as opposed
> to manually parsing path segments) for locating MBean, set this MBean
> on a new MBeanResource instance and return  MBeanResource,
> MBeanResource.getMBean will complete the processing. we will think
> later on how to keep MBeanResources between requests. The reason I'd
> like a dedicated MBeanResource wrap MBean is because possible
> modifications, update notifications, etc may need to be dealt with and
> thus IMHO it can be cleaner to keep MBean as a plain JAXB bean and
> have a dedicated JMX-aware handler around it as opposed to making
> MBean both JAXB and JMX-aware at the same time.
>
>  - /list
>
>  - /{property}/{value}
>
> Please note this method, /{property}/{value}, is on JMXServer. Remove
> @Path("") and replace it with  /{property}/{value}.
>
>  - /objectname/{objectname}
>
> This method should iterate over the existing MBeanCollection only
> without quering the instrumentation manager again.
>
> 3. All the methods should use the same code for accessing the map -
> which is what you already do. Some cleanup is needed there too, but
> for now please address the above
>
> thanks, Sergey
>
>
> On Thu, Jun 9, 2011 at 4:05 AM, Shenglin Qiu <dabaip...@hotmail.com> wrote:
>> Hi Sergey:
>>
>> Sorry for replying a little bit late.
>> But I reverted the code as you mentioned /mbean/{id} must not be changed.
>>
>> Code has be synced to github:
>> https://github.com/dabaipang/services
>>
>> /**
>>  *  Specification:
>>  *  Function 1:
>>  *    Jmx Server:
>>  *    sub-resource locator
>>  *    http://localhost:8080/services/jmx/mbean/0
>>  *    http://localhost:8080/services/jmx/mbean/1
>>  *    ...
>>  *
>>  *
>>  *    Function 2:
>>  *    Note: search by bus.id, bus.id will be changed on every time server
>> had been bounced.
>>  *  http://localhost:8080/services/jmx/objectname/org.apache.cxf:*
>>  *
>> http://localhost:8080/services/jmx/objectname/org.apache.cxf:bus.id=cxf28619341,*
>>  *    ...
>>  *
>> http://localhost:8080/services/jmx/objectname/org.apache.cxf:port=%22CustomerServiceImpl%22,*
>>  *
>> http://localhost:8080/services/jmx/objectname/org.apache.cxf:bus.id=cxf28619341,port=%22SoapPort%22,type=Bus.Service.Endpoint,*
>>  *
>> http://localhost:8080/services/jmx/objectname/org.apache.cxf:type=Bus.Service.Endpoint,*
>>  *
>> http://localhost:8080/services/jmx/objectname/org.apache.cxf:bus.id=cxf28619341,service=%22%7Bhttp%3A%2F%2Fserver.gsoc.apache.org%2F%7DCustomerServiceImpl%22,type=Bus.Service.Endpoint,*
>>  *
>> http://localhost:8080/services/jmx/objectname/org.apache.cxf:service=%22%7Bhttp%3A%2F%2Fserver.gsoc.apache.org%2F%7DCustomerServiceImpl%22,*
>>  *
>>  *    Function 3:
>>  *    Search by service:
>>  *
>> http://localhost:8080/services/jmx/service/%7Bhttp:%2F%2Fserver.gsoc.apache.org%2F%7DCustomerServiceImpl
>>  *
>> http://localhost:8080/services/jmx/service/%7Bhttp:%2F%2Fserver.gsoc.apache.org%2F%7DUserServiceImpl
>>  *
>>  *  Note, above queries are searching these 2, but replace by url encoding
>> from:
>>  *  {http://server.gsoc.apache.org/}UserServiceImpl
>>  *  {http://server.gsoc.apache.org/}CustomerServiceImpl
>>  *
>>  *    Search by type:
>>  *    http://localhost:8080/services/jmx/type/Bus.Service.Endpoint
>>  *
>>  *    Search by port:
>>  *    http://localhost:8080/services/jmx/port/UserServiceImpl
>>  *    http://localhost:8080/services/jmx/port/CustomerServiceImpl
>>  *    http://localhost:8080/services/jmx/port/SoapPort
>>  *
>>  *    Search by domain
>>  *    http://localhost:8080/services/jmx/domain/org.apache.cxf
>>  *
>>  *    Function 4:
>>  *    http://localhost:8080/services/jmx/list
>>  *
>>  * */
>>
>> Source is also attached.
>> Thank you very much on the reviewing, I am very dedicated to what you have
>> mentioned. Though there was a time in last 2 days, I reformatted the code a
>> little too much:)
>>
>>
>> Regards:
>> Shenglin Qiu
>>
>>
>>> Date: Wed, 8 Jun 2011 10:23:58 +0100
>>> Subject: Re: Expose MBeans in CXF
>>> From: sberyoz...@gmail.com
>>> To: dabaip...@hotmail.com
>>> CC: dev@cxf.apache.org
>>>
>>> Hi Shenglin
>>>
>>> I'm not sure if I'm confusing you with the comments I'm making...The
>>> source has become cleaner, good progress there,
>>> but I'm not sure what are you doing there at all now.
>>> Why MBeanCollection has become a subresource ? What happened to our
>>> /mbean/{id} handler ? Recall, you had a subresource locator method
>>> dealing with '/mbean/{id}' and you were delegating to MBean to finish
>>> the processing and the only thing I suggested is to have a dedicated
>>> MBeanResource handler which I guess should wrap an individual MBean
>>> and work with it (get its state for now, etc)
>>>
>>> What does
>>>
>>> http://localhost:8080/services/jmx/property/id/0
>>>
>>> mean ?
>>>
>>> it has to be
>>>
>>> http://localhost:8080/services/jmx/mbean/0
>>> http://localhost:8080/services/jmx/mbean/1
>>>
>>> and MBeanResource needs to deal with these URIs.
>>>
>>> Please check my previous email.
>>> have only
>>> 1. list all MBeans in org.apache.cxf domain
>>> /list
>>> 2. find MBeans in org.apache.cxf domain which have a given attribute value
>>> /{attribute}/{value}
>>> 3. find MBeans in org.apache.cxf domain their object names
>>> /objectname/{objectname}
>>> 4. get the represenation of the individual MBean (and later update it,
>>> register event listeners, etc)
>>> /mbean/{id}
>>>
>>> // we haven't discussed it yet - need to be done
>>> 5. Return the counter statistics
>>>
>>> /counters
>>>
>>> 6. Counters for a specific endpoint
>>>
>>> /counters/{servicename}
>>>
>>> Sergey
>>>
>>> On Wed, Jun 8, 2011 at 5:46 AM, Shenglin Qiu <dabaip...@hotmail.com>
>>> wrote:
>>> > Hi Sergey:
>>> >
>>> > I attached the src and due to I am working on the mvn project only sync
>>> > to
>>> > svn, so I will take some time and sync this to github.
>>> >
>>> > And here is an update on the request:
>>> >
>>> > Self-defined demo inbound service:
>>> > http://localhost:8080/services/greeter?wsdl
>>> > http://localhost:8080/services/customerservice/customers
>>> > http://localhost:8080/services/customerservice/customer/firstname/Lois
>>> > http://localhost:8080/services/customerservice/customer/id/1
>>> > http://localhost:8080/services/userservice/users
>>> > http://localhost:8080/services/userservice/user/1
>>> > /**
>>> >  *  Specification:
>>> >  *  Function 1:
>>> >  *    Jmx Server:
>>> >  *    sub-resource locator
>>> >  *    http://localhost:8080/services/jmx/property/id/0
>>> >  *    http://localhost:8080/services/jmx/property/id/1
>>> >  *    ...
>>> >  *
>>> >  *
>>> >  *    Function 2:
>>> >  *    Note: search by bus.id, bus.id will be changed on every time
>>> > server
>>> > had been bounced.
>>> >  *    http://localhost:8080/services/jmx/objectname/org.apache.cxf:*
>>> >  *
>>> >
>>> > http://localhost:8080/services/jmx/objectname/org.apache.cxf:bus.id=cxf28619341,*
>>> >  *    ...
>>> >  *
>>> >
>>> > http://localhost:8080/services/jmx/objectname/org.apache.cxf:port=%22CustomerServiceImpl%22,*
>>> >  *
>>> >
>>> > http://localhost:8080/services/jmx/objectname/org.apache.cxf:bus.id=cxf28619341,port=%22SoapPort%22,type=Bus.Service.Endpoint,*
>>> >  *
>>> >
>>> > http://localhost:8080/services/jmx/objectname/org.apache.cxf:type=Bus.Service.Endpoint,*
>>> >  *
>>> >
>>> > http://localhost:8080/services/jmx/objectname/org.apache.cxf:bus.id=cxf28619341,service=%22%7Bhttp%3A%2F%2Fserver.gsoc.apache.org%2F%7DCustomerServiceImpl%22,type=Bus.Service.Endpoint,*
>>> >  *
>>> >
>>> > http://localhost:8080/services/jmx/objectname/org.apache.cxf:service=%22%7Bhttp%3A%2F%2Fserver.gsoc.apache.org%2F%7DCustomerServiceImpl%22,*
>>> >  *
>>> >  *    Function 3:
>>> >  *    Search by service:
>>> >  *
>>> >
>>> > http://localhost:8080/services/jmx/property/service/%7Bhttp:%2F%2Fserver.gsoc.apache.org%2F%7DCustomerServiceImpl
>>> >  *
>>> >
>>> > http://localhost:8080/services/jmx/property/service/%7Bhttp:%2F%2Fserver.gsoc.apache.org%2F%7DUserServiceImpl
>>> >  *
>>> >  *  Note, above queries are searching these 2, but replace by url
>>> > encoding
>>> > from:
>>> >  *  {http://server.gsoc.apache.org/}UserServiceImpl
>>> >  *  {http://server.gsoc.apache.org/}CustomerServiceImpl
>>> >  *
>>> >  *    Search by type:
>>> >  *
>>> > http://localhost:8080/services/jmx/property/type/Bus.Service.Endpoint
>>> >  *
>>> >  *    Search by port:
>>> >  *    http://localhost:8080/services/jmx/property/port/UserServiceImpl
>>> >  *
>>> > http://localhost:8080/services/jmx/property/port/CustomerServiceImpl
>>> >  *    http://localhost:8080/services/jmx/property/port/SoapPort
>>> >  *
>>> >  *    Function 4:
>>> >  *    http://localhost:8080/services/jmx/list
>>> >  *
>>> >  * */
>>> >
>>> > I will ping you tomorrow morning.
>>> >
>>> > Thank you.
>>> > Regards:
>>> >
>>> > Shenglin Qiu
>>> >
>>> >
>>> >> Date: Wed, 1 Jun 2011 15:32:30 +0100
>>> >> Subject: Re: Expose MBeans in CXF
>>> >> From: sberyoz...@gmail.com
>>> >> To: dev@cxf.apache.org
>>> >>
>>> >> Hi Shenglin
>>> >>
>>> >> It's a step in the right direction, thanks, but JMXServer still needs
>>> >> to be cleaned up quite a bit.
>>> >> - as I said earlier you have 4 big functions basically duplicating
>>> >> each other. We can't have a method per every attribute a given MBean
>>> >> may have. Have a single function only, max two (one capturing all the
>>> >> attributes, another one - dedicated to objectname/{value}).
>>> >> - Introduce MBeanResource subresource instead of overloading MBean, it
>>> >> will be cleaner and easier to work with
>>> >> - as suggested earlier, update the domain MBean and/or MBeanAttribute
>>> >> such that you could drop a big chunk of code in JMXServer where
>>> >> individual attribute values are set, currently in getMbeanMap (if
>>> >> (attrName.equals("service") then/else)). That is because a number of
>>> >> attributes is basically unlimited, even though we have some well-known
>>> >> ones
>>> >>
>>> >> - getMBeanMap - this is so complex I can't understand what it does. I
>>> >> do understand it returns a thread-safe Map - but I'm finding it
>>> >> difficult to understand how the method achieves that. Please remove
>>> >> all those synchronized blocks and have ConcurrentHashMap. Have atomic
>>> >> integer counter, synchronized exlicitly (in a block) if you prefer,
>>> >> and try to get a simple and effective function implemented. Don't make
>>> >> the logic of that function dependent on a n
>>> >> I don't understand why you use "list" for creating MBean hrefs;
>>> >>
>>> >> - there's no need for having Mbean.id and MBean.href because
>>> >> MBean.href is identifying a given MBean uniquely and thus you could
>>> >> use that calculated href as a key.
>>> >>
>>> >> Thanks, Sergey
>>> >>
>>> >>
>>> >> On Tue, May 31, 2011 at 5:36 PM, Shenglin Qiu <dabaip...@hotmail.com>
>>> >> wrote:
>>> >> >
>>> >> > Hi Sergey:
>>> >> >
>>> >> > Project has been synced to Github:
>>> >> >
>>> >> > Browser:
>>> >> > https://github.com/dabaipang/services
>>> >> >
>>> >> >
>>> >> > Git address:
>>> >> > https://dabaip...@github.com/dabaipang/services.git
>>> >> > or
>>> >> > g...@github.com:dabaipang/services.git
>>> >> >
>>> >> >
>>> >> > Thank you.
>>> >> >
>>> >> >
>>> >> > Regards:
>>> >> > Shenglin Qiu
>>> >> >
>>> >> >
>>> >> >
>>> >> >> From: dabaip...@hotmail.com
>>> >> >> To: sberyoz...@gmail.com
>>> >> >> CC: dev@cxf.apache.org
>>> >> >> Subject: RE: Expose MBeans in CXF
>>> >> >> Date: Tue, 31 May 2011 11:23:38 -0400
>>> >> >>
>>> >> >>
>>> >> >> Hi Sergey:
>>> >> >>
>>> >> >> > > Function 1: (0 - 5 is continuous, no gap)
>>> >> >> > > Jmx Server:
>>> >> >> > > sub-resource locator
>>> >> >> > > http://localhost:8080/services/jmx/mbean/0
>>> >> >> > > http://localhost:8080/services/jmx/mbean/1
>>> >> >> >
>>> >> >> > The id allocation (0, 1, etc) has to be thread safe
>>> >> >>
>>> >> >> Done with:
>>> >> >> private synchronized Map<String, MBean> getMBeansMap(){
>>> >> >>  ....
>>> >> >> }
>>> >> >> My reason not using synchronized block but using synchronized method
>>> >> >> is:
>>> >> >> this guarantees the order on mbeansMap's initialization,  so when
>>> >> >> it's
>>> >> >> called by a lot of requests, the followings will have to wait, unit
>>> >> >> the
>>> >> >> first one finished,  after that, following requests will see
>>> >> >> mbeansMap has
>>> >> >> been initialized, and simply return mbeansMap without initializing
>>> >> >> it again.
>>> >> >>
>>> >> >>
>>> >> >> > http://localhost:8080/services/jmx/objectname/org.apache.cxf:*
>>> >> >> > or
>>> >> >> > http://localhost:8080/services/jmx/objectname/org.apache.cxf
>>> >> >> >
>>> >> >> > should also work
>>> >> >> >
>>> >> >> > >
>>> >> >>
>>> >> >> http://localhost:8080/services/jmx/objectname/org.apache.cxf:*
>>> >> >> is working.
>>> >> >>
>>> >> >> > > Due to the URL duplication with /mbean/{id}, I can't put
>>> >> >> > > {searchtype}/{query}, so I put things like:
>>> >> >> >
>>> >> >> > Personally I'd  prefer to keep a URI as minimal as possible.
>>> >> >> >
>>> >> >> > /mbean/{id}
>>> >> >> > and
>>> >> >> > {searchtype}/{query}
>>> >> >> >
>>> >> >> > do not duplicate each other given that /mbean/{id} is more
>>> >> >> > specific
>>> >> >> > than {searchtype}/{query}.
>>> >> >> > Likewise /objectname/{objectname} is more specific than
>>> >> >> > {searchtype}/{query}, so consider dropping /search/ part,
>>> >> >> > we may have a dedicated /search handler later on.
>>> >> >>
>>> >> >> Done by dedicating each request per method:
>>> >> >>     @GET
>>> >> >>     @POST
>>> >> >>     @Path("/list")
>>> >> >>     public MBeanCollection list()
>>> >> >>
>>> >> >>     @Path("/mbean/")
>>> >> >>     public MBean locateMBean()
>>> >> >>
>>> >> >>
>>> >> >>     @GET
>>> >> >>     @POST
>>> >> >>     @Path("/type/{query}")
>>> >> >>     public MBeanCollection searchMBeansByType(@PathParam("query")
>>> >> >> String query)
>>> >> >>
>>> >> >>
>>> >> >>     @GET
>>> >> >>     @POST
>>> >> >>     @Path("/port/{query}")
>>> >> >>     public MBeanCollection searchMBeansByPort(@PathParam("query")
>>> >> >> String query)
>>> >> >>
>>> >> >>
>>> >> >>     @GET
>>> >> >>     @POST
>>> >> >>     @Path("/service/{query}")
>>> >> >>     public MBeanCollection searchMBeansByService(@PathParam("query")
>>> >> >> String query)
>>> >> >>
>>> >> >>
>>> >> >>     @GET
>>> >> >>     @POST
>>> >> >>     @Path("/objectname/{objectname}")
>>> >> >>     public  MBeanCollection getComponent(@PathParam("objectname")
>>> >> >> String objectname)
>>> >> >>
>>> >> >> I remembered you mentioned to handle Exceptions once, currently I am
>>> >> >> still 'throws' -ing them out, I will make the try catch block.
>>> >> >> Should I also add a new object which display the error or simply
>>> >> >> return
>>> >> >> empty?
>>> >> >>
>>> >> >>
>>> >> >> Thank you very much.
>>> >> >>
>>> >> >>
>>> >> >> Regards:
>>> >> >> Shenglin Qiu
>>> >> >>
>>> >> >>
>>> >> >> > Date: Tue, 31 May 2011 10:35:36 +0100
>>> >> >> > Subject: Re: Expose MBeans in CXF
>>> >> >> > From: sberyoz...@gmail.com
>>> >> >> > To: dev@cxf.apache.org
>>> >> >> >
>>> >> >> > Hi Shenglin
>>> >> >> >
>>> >> >> > Good progress, some comments below
>>> >> >> > >
>>> >> >> > > Function 1: (0 - 5 is continuous, no gap)
>>> >> >> > > Jmx Server:
>>> >> >> > > sub-resource locator
>>> >> >> > > http://localhost:8080/services/jmx/mbean/0
>>> >> >> > > http://localhost:8080/services/jmx/mbean/1
>>> >> >> >
>>> >> >> > The id allocation (0, 1, etc) has to be thread safe
>>> >> >> >
>>> >> >> > >
>>> >> >> > >
>>> >> >> > > Function 2:
>>> >> >> > > Note: search by bus.id, bus.id will be changed on every time
>>> >> >> > > server
>>> >> >> > > had been
>>> >> >> > > bounced.
>>> >> >> > >
>>> >> >> > >
>>> >> >> > > http://localhost:8080/services/jmx/objectname/org.apache.cxf:bus.id=cxf28619341,*
>>> >> >> > > ...
>>> >> >> > that is ok, a user does not have to specify them as you
>>> >> >> > demonstrated
>>> >> >> > with queries like
>>> >> >> >
>>> >> >> > >
>>> >> >> > >
>>> >> >> > > http://localhost:8080/services/jmx/objectname/org.apache.cxf:port=%22CustomerServiceImpl%22,*
>>> >> >> > I guess
>>> >> >> >
>>> >> >> > http://localhost:8080/services/jmx/objectname/org.apache.cxf:*
>>> >> >> > or
>>> >> >> > http://localhost:8080/services/jmx/objectname/org.apache.cxf
>>> >> >> >
>>> >> >> > should also work
>>> >> >> >
>>> >> >> > >
>>> >> >> > >
>>> >> >> > > I am only now use 2 methods to fulfill:
>>> >> >> > > 1 method for service/{serivce}, port/{port}, type/{type}
>>> >> >> > > 1 method for objectname/{objectname}
>>> >> >> >
>>> >> >> > OK
>>> >> >> >
>>> >> >> > >
>>> >> >> > > Due to the URL duplication with /mbean/{id}, I can't put
>>> >> >> > > {searchtype}/{query}, so I put things like:
>>> >> >> >
>>> >> >> > Personally I'd  prefer to keep a URI as minimal as possible.
>>> >> >> >
>>> >> >> > /mbean/{id}
>>> >> >> > and
>>> >> >> > {searchtype}/{query}
>>> >> >> >
>>> >> >> > do not duplicate each other given that /mbean/{id} is more
>>> >> >> > specific
>>> >> >> > than {searchtype}/{query}.
>>> >> >> > Likewise /objectname/{objectname} is more specific than
>>> >> >> > {searchtype}/{query}, so consider dropping /search/ part,
>>> >> >> > we may have a dedicated /search handler later on.
>>> >> >> >
>>> >> >> > The other thing I forgot to mention, please remove all those
>>> >> >> > System.out.println, ping me please if you need some help with
>>> >> >> > setting
>>> >> >> > up a remote debugging session
>>> >> >> >
>>> >> >> > Thanks, Sergey
>>> >> >> >
>>> >> >> > >
>>> >> >> > > Thank you so much!
>>> >> >> > >
>>> >> >> > > Regards:
>>> >> >> > > Shenglin Qiu
>>> >> >> > >
>>> >> >>
>>> >> >
>>> >>
>>> >>
>>> >>
>>> >> --
>>> >> Sergey Beryozkin
>>> >>
>>> >> Application Integration Division of Talend
>>> >> http://sberyozkin.blogspot.com
>>> >
>>>
>>>
>>>
>>> --
>>> Sergey Beryozkin
>>>
>>> Application Integration Division of Talend
>>> http://sberyozkin.blogspot.com
>>
>
>
>
> --
> Sergey Beryozkin
>
> Application Integration Division of Talend
> http://sberyozkin.blogspot.com
>



-- 
Sergey Beryozkin

Application Integration Division of Talend
http://sberyozkin.blogspot.com

Reply via email to